5.18 MB, text/plain
7.71 KB, patch
|Details | Diff | Splinter Review|
1.00 KB, patch
|Details | Diff | Splinter Review|
Created attachment 8544882 [details] charliebrown.txt Sheila found this bug and showed it to me, and I was able to reproduce it and grab some logging. Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:37.0) Gecko/20100101 Firefox/37.0 ID:20150106030201 CSet: 2a193b7f395c STR: 1. Load https://www.youtube.com/watch?v=VayAyAr-xqI 2. About 9 seconds in it seems to stutter, and at the 14 second mark the video stops playing. This can be consistently reproduced on my Mac which is running 10.10.2 and is using an ethernet connection. Logging is attached.
I can reproduce on MacOS X 10.8.5.
I can reproduce on Aurora36.0a2 as well as Nightly37.0a1 on Windows7. However, I cannot reproduce on Firefox35.0b8 even if media.mediasource.enabled =true.
status-firefox35: --- → unaffected
status-firefox36: --- → affected
status-firefox37: --- → affected
tracking-firefox36: --- → ?
tracking-firefox37: --- → ?
OS: Mac OS X → All
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=903fd76cc118&tochange=fe5ae33fc62d Triggered by: Bug 1097436
Looks like there's a few issues here at least. The gaps between audio segments are larger than our current fuzz factor (1ms), so we're getting lots of new decoders. Bumping the fuzz factor up to 10ms fixes the majority of the badness, though we should try to fix as much as possible before doing that. The first thing I've found is that we don't seem to have initialized the second audio decoder when we hit EOS on the first one. When initialization completes we attempt to notify that we now have data, but MediaSourceReader::MaybeNotifyHaveData doesn't account for any sort of fuzz factor so we still think we're waiting for audio.
Created attachment 8545681 [details] [diff] [review] Use a fuzzy check to determine if we have data
Attachment #8545681 - Flags: review?(cajbir.bugzilla)
Anthony, we need to increase the fuzzy tolerance for this video to work properly. You mentioned that one of the demuxers computed a value for this using the timescale, where is the code for that? I haven't been able to find it. Failing that, we need to bump up it to at least 5011 for this video (it's currently 1000).
Created attachment 8546195 [details] [diff] [review] Bump fuzz factor for mp4 up to 20ms
Attachment #8546195 - Flags: review?(ajones)
Comment on attachment 8545681 [details] [diff] [review] Use a fuzzy check to determine if we have data Review of attachment 8545681 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/mediasource/MediaSourceReader.h @@ +128,5 @@ > nsresult SetCDMProxy(CDMProxy* aProxy); > #endif > > private: > + bool SwitchAudioReader(int64_t aTarget, int64_t aError = 0); Add comment explaining what unit aError is. Might as well do aTarget too. Do this in all methods that take aError.
Attachment #8545681 - Flags: review?(cajbir.bugzilla) → review+
Attachment #8546195 - Flags: review?(ajones) → review+
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment on attachment 8545681 [details] [diff] [review] Use a fuzzy check to determine if we have data Landed on 36 beta with irc approval from lsblakk. https://hg.mozilla.org/releases/mozilla-beta/rev/d9a5b0ec6226 https://hg.mozilla.org/releases/mozilla-beta/rev/6231299d9787 These changes are low risk, and fix a specific issue with youtube.
Attachment #8545681 - Flags: approval-mozilla-beta?
status-firefox36: affected → fixed
status-firefox37: affected → fixed
tracking-firefox36: ? → +
tracking-firefox37: ? → +
Attachment #8545681 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reproduced with Nightly 2015-01-06 on Mac OS X 10.9.5 with str from comment 0. Verified as fixed with Fx 36 beta 1 build 2 (20150114125146) and DevEd 37.0a2 (20150118004006) on Window 8 32-bit, Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit.
Status: RESOLVED → VERIFIED
status-firefox36: fixed → verified
status-firefox37: fixed → verified
\o/ Thanks, Alexandra.
You need to log in before you can comment on or make changes to this bug.