Closed Bug 1160321 Opened 8 years ago Closed 8 years ago

All YouTube videos fail to start for affected users (readyState == 0)

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
Unspecified
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 + verified
firefox38.0.5 --- wontfix
firefox40 + verified

People

(Reporter: cpeterson, Assigned: cpearce)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(3 files)

Patch 1: Try to instantiate decoders in canPlayType
12.10 KB, patch
mattwoodrow
: review+
lizzard
: approval-mozilla-beta+
Details | Diff | Splinter Review
mozilla-inbound_yosemite_test-mochitest-gl-bm107-tests1-macosx-build814.txt.gz
69.00 KB, application/gzip
Details
Patch 2: Report trackbuffer initialization error back to HTML element
1.26 KB, patch
karlt
: review+
Details | Diff | Splinter Review 
After YouTube re-enabled HTML5 video for Firefox 37 users on Windows Vista+, problems were reported on Reddit about audio playing without video:

http://www.reddit.com/r/youtube/comments/34cyca/firefox_is_now_using_the_html5_player_by_default/


strobe at YouTube says:

the video has gotten past the "trying to play an ad" state (no preroll=1, none of the ad_v* params).

Looking at one of the debug infos, we see that:

vns (network state) is 2 - mediasource is attached
vrs (ready state) is 0 - loadedmetadata never fired

vd (video duration) is at 890 - a media source is attached and provided duration info
laa, lva - media was appended
lar, lvr - media continues to be requested without any surprising events

vpl, vbu is empty - nothing's getting buffered
lab, lvb is empty - the source buffer 'buffred' properties are empty

the video ID is in the debug info as 'docid' (and again as debug_videoId, for legacy reasons)

Could something be failing or getting wedged during decoder init?


Anthony asked jya:

Looks like we're not appending audio due to a 34.266ms gap in the video. I thought we fixed the threshold in 37.0.2.

http://www.reddit.com/r/youtube/comments/34cyca/firefox_is_now_using_the_html5_player_by_default/cqtmuec

https://www.youtube.com/watch?v=wTcNtgA6gHs
mediasource:https://www.youtube.com/1e06f81f-1d74-4d93-b57d-6f8d838f2921
        currentTime: 0
                SourceBuffer 0
                SourceBuffer 1
                        start=0 end=19.986633
        Internal Data:
        Dumping data for reader 8879ea0:
                Dumping Video Track Decoders - mLastVideoTime: 0,000000
                        Reader 1: 1a9dd400 ranges=[(4,971633, 15,915900)] active=false size=465670
                        Reader 0: 1a67dc00 ranges=[(0,000000, 19,919900), (19,953266, 19,986633)]
OS: Unspecified → Windows 7
strobe says:

About the persistent complaints of "MediaSource sourceopen fired but readyState is still 0", I'm going to experiment with and then launch a workaround where we fall back to Flash if sourceopen has happened, readyState is null, metadata has been appended, and we've waited more than 15 seconds for a video to start. The 15 second delay should prevent incorrect triggering and keep the problem visible until we can root-cause it.
I think the issue title and description are mismatched; the primary complaint we're seeing is that all videos fail to start for affected users (with readyState == 0), and secondary complaints about videos with low framerate not starting are probably an independent complaint related to the question posed to jya.

The fall-back-after-15-seconds thing should go live tonight (2-ish PST), if the push goes as planned. I'll look into a safe way to do it with less delay as well, although hopefully we can just fix this in 38.
Summary: YouTube audio playing with black video frame → All YouTube videos fail to start for affected users (readyState == 0)
http://www.reddit.com/r/firefox/comments/34ivca/youtube_videos_dont_load/

Users note that windows-media-foundation might be defaulting to off. There's no copy debug info in the thread so I don't know if they're readyState == 0 but I'll check on a Windows box and also ask a few users to try it.
Ah, yes, that's definitely the issue. And I think I know why Reddit has a relatively high set of users in this boat:

http://www.fluxbytes.com/tag/media-windows-media-foundation-enabled/

I'm considering just putting in an alert() that instructs users in this situation to flip that flag. Heinous, but I don't have a better idea right now. Any objections?
If WMF is disabled, ReadMetadata would have returned an error, and would have been reported back to the HTMLMediaElement. Additionally, the message "couldn't display because the file was corrupted" (or something to that extent) would have been displayed.

Is the error lost somehow? It would have been issued prior the updateend event due for the init segment.
The sourcebuffer would have received an error as well (with the events error / updateend)
I downloaded and installed "CodecTweakTool", and was able to disable Media Foundation and a bunch of the Windows' codecs, and I got the same behaviour on YouTube. This renamed the H.264/AAC DLLs and made MFStartup() fail. It's not obvious how we can recover from this, so we should just make our HTMLMediaElement.canPlayType() return can't play when we're unable to instantiate & init the decoders.
The error message at the video element level seems to be lost, and we've been hitherto counting on step 5 from MSE 3.5.3 to throw the error at the element level. 

We'll make use of the 'error' event starting in the next YouTube branch cut and push. Ideally this will be pushed late Tuesday night PST.
It looks like we fire the "error" event at the SourceBuffer (3.5.3.3), but not at the Element.

The latter is part of 3.5.3.5, which goes into 2.4.7.3 and requests that we "Run the "If the media data can be fetched but is found by inspection to be in an unsupported format, or can otherwise not be rendered at all" steps of the resource fetch algorithm.".

That resource fetch algorithm step fires an "error" event at the Element, and we don't implement that bit yet.

http://mxr.mozilla.org/mozilla-central/source/dom/media/mediasource/MediaSource.cpp#316
Test instantiating decoders in HTMLMediaElement.canPlayType() and in MediaKeySystemAccess.

I moved the statics to MP4Decoder, since MP4Reader is supposed to be off-main-thread only, and most of the "do we support codec X" logic is in MP4Decoder already anyway.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eaf0fc8e6008
Assignee: nobody → cpearce
Status: NEW → ASSIGNED
Attachment #8601760 - Flags: review?(matt.woodrow)
Comment on attachment 8601760 [details] [diff] [review]
Patch 1: Try to instantiate decoders in canPlayType

Review of attachment 8601760 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/fmp4/MP4Decoder.cpp
@@ +253,5 @@
> +};
> +
> +static already_AddRefed<MediaDataDecoder>
> +CreateTestH264Decoder(layers::LayersBackend aBackend,
> +VideoInfo& aConfig)

Indent

@@ +261,5 @@
> +  aConfig.mDuration = 40000;
> +  aConfig.mMediaTime = 0;
> +  aConfig.mDisplay = aConfig.mImage = nsIntSize(64, 64);
> +  aConfig.mExtraData = new MediaByteBuffer();
> +  aConfig.mExtraData->AppendElements(sTestH264ExtraData, 40);

Might as well switch to MOZ_ARRAY_LENGTH here too.
Attachment #8601760 - Flags: review?(matt.woodrow) → review+
Uplift to 39 needed.
Flags: needinfo?(cpearce)
This is a very rough fix, and doesn't fully implement how we're supposed to report errors back to the HTML MediaElement nor handling the different conditions based on the readyState value. However, we can only reach that point when a reader fail to initialize. Which is always when readyState is still 0. So calling MediaDecoder::DecoderError() will do the job
Attachment #8602444 - Flags: review?(cpearce)
Assignee: cpearce → jyavenard
was just trying to offer another approach.
Assignee: jyavenard → cpearce
Comment on attachment 8602444 [details] [diff] [review]
Patch 2: Report trackbuffer initialization error back to HTML element

Review of attachment 8602444 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with taking this patch too (but I needed my other patch to ensure this doesn't bite EME as well), but I think someone who knows more about whether this is in line with the MSE spec should review it... karl?
Attachment #8602444 - Flags: review?(cpearce) → review?(karlt)
https://hg.mozilla.org/mozilla-central/rev/03dc784d5456
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
[Tracking Requested - why for this release]:

Youtube has deployed a fix on their end for this issue (that detect the failure and fallback to flash), but these patches will improve the user experience by not reporting that we support HTML5 video in the first place.
tracking-firefox39: --- → ?
tracking-firefox40: --- → ?
Comment on attachment 8601760 [details] [diff] [review]
Patch 1: Try to instantiate decoders in canPlayType

Approval Request Comment
[Feature/regressing bug #]: HTML5 Video
[User impact if declined]: Users with broken HTML5 video (disabled windows media foundation somehow) will see the youtube error screen before falling back to flash.
[Describe test coverage new/current, TreeHerder]: Tested manually.
[Risks and why]: Low risk, just checks if the decoders work before reporting that we support video.
[String/UUID change made/needed]: None
Attachment #8601760 - Flags: approval-mozilla-beta?
Attachment #8601760 - Flags: approval-mozilla-aurora?
Comment on attachment 8602444 [details] [diff] [review]
Patch 2: Report trackbuffer initialization error back to HTML element

>+      // We can only reach this state during a track buffer initialization.
>+      // This is a non recoverable error.
>+      mDecoder->DecodeError();

Please remove the first comment.
This is a public method, so AFAICS this code is reached when endOfStream() decode is called.

I would call DecoderError() directly on mMediaElement instead of using the MediaDecoder proxy, which I assume is mostly for objects that don't have a direct reference to the MediaElement.

>     // TODO: If media element has a readyState of:
>     //   HAVE_NOTHING -> run "unsupported" steps of resource fetch
>     // > HAVE_NOTHING -> run "corrupted" steps of resource fetch

Is there anything here not handled by DecodeError?
Please remove or update the comment as appropriate.
Attachment #8602444 - Flags: review?(karlt) → review+
Blocks: 1163509
Matt, from the review in comment 22, would you like to update this patch before we uplift it to 39? Let me know. Thanks.
Flags: needinfo?(matt.woodrow)
(In reply to Liz Henry (:lizzard) from comment #23)
> Matt, from the review in comment 22, would you like to update this patch
> before we uplift it to 39? Let me know. Thanks.

We're only looking to uplift the first patch (which has already landed on m-c), so those comments don't apply.
Flags: needinfo?(matt.woodrow)
this patch appears to have caused regression in bug 1164480. May want to hold on the uplift (if mac is expected to be supported)
Patch 1 also caused bug 1163814. So we'll need to take the fixes to bug 1164480 and bug 1163814 when we uplift Patch 1.
Depends on: 1163814
Attachment #8601760 - Attachment description: Patch: Try to instantiate decoders in canPlayType → Patch 1: Try to instantiate decoders in canPlayType
Attachment #8602444 - Attachment description: Report trackbuffer initialization error back to HTML element → Patch 2: Report trackbuffer initialization error back to HTML element
Still holding off on uplift for this waiting on a fix for bug 1164480.
Liz: bug 1164480 was fixed by the patch in bug 1163814. Can we get this bug 1163814 and this bug approved?

I also have a bunch of other approval requests (Bug 1156566, bug 1139125, bug 1165163, and Bug 1160914) which I'd like to get into 39 before 39 beta1 goes out if possible...
Flags: needinfo?(lhenry)
Comment on attachment 8601760 [details] [diff] [review]
Patch 1: Try to instantiate decoders in canPlayType

Approved for uplift to aurora (40) and beta (39).
Flags: needinfo?(lhenry)
Attachment #8601760 - Flags: approval-mozilla-beta?
Attachment #8601760 - Flags: approval-mozilla-beta+
Attachment #8601760 - Flags: approval-mozilla-aurora?
Attachment #8601760 - Flags: approval-mozilla-aurora+
Andrei: Can your team verify this fix once it lands in 39 and we have build1 for 39.0b1? That should be somewhat later today if everything goes well.
Flags: qe-verify+
Comment on attachment 8601760 [details] [diff] [review]
Patch 1: Try to instantiate decoders in canPlayType

This landed prior to the uplift, so it's already on Aurora.
Attachment #8601760 - Flags: approval-mozilla-aurora+
If we can land this on beta before Friday then we can still get it into 39 beta 1.
Flags: needinfo?(cpearce)
Chris would you like there to be a release note for this for 39 or 40 depending on where it ends up?
Flags: needinfo?(cpearce)
I don't think that's necessary, the uplifte stuff only covers EME, and that's basically a bug fix.
Flags: needinfo?(cpearce)
I was not able to reproduce the initial issue using Firefox 38.0.5 build 1 on Windows 7 64-bit in order to verify if the issue was fixed.
I used CodecTweakTool and disabled Media Foundation but with no success. 
Chris can you tell me what codecs I need to disable in order to reproduce the initial issue? I noticed that in comment 6 you said you also disable a bunch of other windows codecs.
Flags: needinfo?(cpearce)
It can no longer be reproduced on YouTube becuase YouTube added a work around.
YouTube have deployed a work around, but you can verify the behaviour like so:

1. Open http://pearce.org.nz/video/h264.html
2. Observe "v.canPlayType("video/mp4; codecs="avc1.42E01E, mp4a.40.2"") = probably" appears on that page and that the video plays.
3. Disable codecs using codec tweak tool.
4. Restart Firefox.
5. Open http://pearce.org.nz/video/h264.html
6. Observe "v.canPlayType("video/mp4; codecs="avc1.42E01E, mp4a.40.2"") = " appears on that page, and the the video doesn't play.
Flags: needinfo?(cpearce)
(In reply to Chris Pearce (:cpearce) from comment #41)
> YouTube have deployed a work around, but you can verify the behaviour like
> so:
> 
> 1. Open http://pearce.org.nz/video/h264.html
> 2. Observe "v.canPlayType("video/mp4; codecs="avc1.42E01E, mp4a.40.2"") =
> probably" appears on that page and that the video plays.
> 3. Disable codecs using codec tweak tool.
> 4. Restart Firefox.
> 5. Open http://pearce.org.nz/video/h264.html
> 6. Observe "v.canPlayType("video/mp4; codecs="avc1.42E01E, mp4a.40.2"") = "
> appears on that page, and the the video doesn't play.

Thanks Chris, confirmed that is verified and also based on youtube`s work around, marking this as verified fixed.
Status: RESOLVED → VERIFIED
status-firefox39: fixed → verified
status-firefox40: fixed → verified
You need to log in before you can comment on or make changes to this bug.