I'm not consistently getting progress events from <video> on current nightly trunk / builds. EG, no "X"s appear on http://www.double.co.nz/video_test/events.html and I'm not seeing them in a video control patch that was previously working (so this presumably regressed recently). Oddly enough, I do get progress events when the video src is a local file, but event.loaded is always 0.

Shift-reload will sometimes give me one small progress event around 4K, but the initial load doesn't (even when the cache has been cleared).

Fixes progress events for http resources. Still need to resolve local file issue and write tests.

Should this be blocking1.9.1? It blocks a blocking1.9.1 bug.

Yes.

+ if (mDataTime != 0 && + PR_IntervalToMilliseconds(PR_IntervalNow() - mDataTime) >= STALL_MS) { + mElement->DispatchProgressEvent(NS_LITERAL_STRING("stalled")); + mDataTime = 0; + } + + if (!aTimer) { + mDataTime = PR_IntervalNow(); + } It looks like we could have a situation where data arrives just after STALL_MS causing us to send a "stalled" event. Factor out PR_IntervalNow() into a shared local so we don't call it more than necessary. + if (mElement) { + mElement->DispatchProgressEvent(NS_LITERAL_STRING("progress")); + } Fix indent void nsOggDecoder::UpdateBytesDownloaded(PRUint64 aBytes) { - mBytesDownloaded = aBytes; + nsAutoMonitor mon(mMonitor); + if (mPlayState != PLAY_STATE_LOADING) { + mBytesDownloaded = aBytes; + } } Why is this needed? Don't we need to update the Wave backend too? Seems like we'll need a test for that too.

> Why is this needed? During the loading of the metadata the value of the bytes downloaded can be skewed by the results of the seek to get the duration. If progress events are sent during this time then we get the progress.loaded value of the seek to eof to get the duration - which makes it look as if the download has completed. So during the loading of metadata for Ogg files I don't display progress. I've addressed your other comments and added wav test.

Don't we need to StartProgress in nsWaveDecoder::MetadataLoaded instead of ::Load, like we are for Ogg? And stop/start progress in SeekingStarted/SeekingStopped?

StartProgress is done during Load in the wav backend. Since the wav backend doesn't seek when loading metadata like the ogg backend does it doesn't need to wait until then to do it. Seeks immediately go to the correct location in the wav backend, it doesn't oscillate to find the seek point so doesn't need to start/stop the progress eents.

http://hg.mozilla.org/mozilla-central/rev/34f6eb1300f1 http://hg.mozilla.org/mozilla-central/rev/3e3feb158fad http://hg.mozilla.org/mozilla-central/rev/1b5995ee5192

test_progress1 timed out, so backed out commits: *** 29953 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29954 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29955 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29956 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29957 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29958 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29959 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29960 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29961 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29962 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29963 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29964 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29965 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29966 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29967 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29968 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29969 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29970 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29971 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29972 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29973 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29974 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29975 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29976 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29977 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29978 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29979 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29980 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress lengthComputable *** 29981 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress loaded: 0 *** 29982 INFO TEST-PASS | /tests/content/media/video/test/test_progress1.html | Check progress total: 285310 *** 29983 ERROR TEST-UNEXPECTED-FAIL | /tests/content/media/video/test/test_progress1.html | Test timed out.

This changes things to make sure that events aren't raised during metadata/duration retrieval, and ensures that a final event is always sent containing the length of the file. This fixes the spurious test failure that was occurring.

Rebase to trunk, carry forward r+sr

This wav file needs to be added to content/media/video/test

Pushed 70b57b58afe2

Applies on top of existing patch

I backed this out because of a test failure on the tinderbox. We decided we need to make some more fixes to WAV on top of attachment 353611 [details] [diff] [review], so we'll wait for that to be done before relanding.

Checked in again as 1a4e7f55120d but backed out due to test timeouts on all platforms, e.g. http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1229588582.1229592245.10136.gz#err5

I've made the progress event dispatches occur asynchronously and add some dump statements to the failing test. If the async change doesn't fix the breakage the dump's will help track it down. They can be removed if the test passes.

Pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/f155c8f39715 I'm holding off marking as resolved until I see the results of the tests.

Backed out due to failures: http://tinderbox.mozilla.org/showlog.cgi?tree=Firefox&errorparser=unittest&logfile=1231293364.1231298743.27619.gz&buildtime=1231293364&buildname=Linux%20mozilla-central%20unit%20test&fulltext=1 http://tinderbox.mozilla.org/showlog.cgi?tree=Firefox&errorparser=unittest&logfile=1231294086.1231297427.24331.gz&buildtime=1231294086&buildname=WINNT%205.2%20mozilla-central%20unit%20test&fulltext=1

Making P1 since it blocks the video controls work, which is P1

Hmm, I just rediscovered the ".loaded == 0 for file://" problem I noted in comment 0. Should we spin off a new bug for that problem, or will this patch eventually address that?

I'll address it in this patch. Do you have a testcase?

Testcase: Save this locally along with the linked .ogg, progress events are dumped to the console.

Adds two tests with smaller files to test the case of resource being fully loaded before metadataloaded fires. Fixes issue of not firing last progress event when resource is loaded before metadataloaded fires.

Comment on attachment 357052 [details] [diff] [review] Fix test failure OK once we've cleaned up nsOggDecoder::ResourceLoaded

Clean up ResourceLoaded, carrying forward r+sr=roc

Pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/a9cd6dc8dbe0

I haven't been able to get test_progress1.html working 100% on Linux (although it passes on Mac and Windows always, as far as I can tell), but I think we should just push this to 1.9.1 as-is since we can't reproduce the failures.

Pushed to mozilla-1.9.1; http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b926512423bd Also pushed to disable progress1 test as per comment 33: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b57b064e920f

Great testcase. Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090527 Shiretoko/3.5pre Ubiquity/0.1.5 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090527 Minefield/3.6a1pre