1207376 - canPlayType("video/mp4; codecs="avc1.42E01E, mp4a.40.2"") erroneously returns "probably" on Windows 8.1 N

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

Our canPlayType logic is failing on Windows N.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

This is a regression in 42. I bet from jya making the H264Converter not create the decoder until a sample is input.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Netflix rely on isTypeSupported working, so we'll need this fixed and uplifted to beta42.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch: Ensure PlatformDecoderModule::CreateDecoder reports failure to create H264Converter

If WMF is broken or not available, we still end up creating the H264Converter in PlatformDecoderModule::CreateDecoder(), but it wraps a null decoder. We need to report the error out to the caller, otherwise MP4Decoder::CanHandleMediaType() ends up reporting the wrong value out.

This means on Windows N, and on Windows where Codec Tweak Tool has buggered WMF, our HTMLMediaElement.canPlayType() and everything that depends on that is wrong.

So we need to propogate the error if when the H264Converter::CreateDecoder() call made by the H264Converter() constructor fails.

The alternative to exposing the H264Converter::mLastError like this patch does would be to make H264Converter::CreateDecoder() public, and call that directly outside of the H264Converter constructor in PlatformDecoderModule::CreateDecoder() right after constructing the H264Converter.

We'll need to uplift this to Beta 42. We need this for Netflix.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Pearce (:cpearce) from comment #1)
> This is a regression in 42. I bet from jya making the H264Converter not
> create the decoder until a sample is input.

This statement is incorrect. jya is awesome.

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8664639 [details] [diff] [review]
Patch: Ensure PlatformDecoderModule::CreateDecoder reports failure to create H264Converter

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

I don't really like it because it breaks the abstraction; but I can't see another way to handle this without calling Init()

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f16dbf52986

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Needs uplift.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2f16dbf52986

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8664639 [details] [diff] [review]
Patch: Ensure PlatformDecoderModule::CreateDecoder reports failure to create H264Converter

Approval Request Comment
[Feature/regressing bug #]: Regression from MSE uplifts to 42, affects Netflix/EME

[User impact if declined]: Websites won't be able to correctly detect whether Firefox cannot play MP4/AAC/H264. This means Netflix will try to use EME when it's not going to work, and they won't be able to detect that it's not going to work.

[Describe test coverage new/current, TreeHerder]: We have lots of MSE/EME mochitests

[Risks and why]: Low, just checks an error code.

[String/UUID change made/needed]: None.

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8664639 [details] [diff] [review]
Patch: Ensure PlatformDecoderModule::CreateDecoder reports failure to create H264Converter

Fix a regression, taking it

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Aurora Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb4e98d185d5

Aurora patch rebased, ready to be uplift:
https://hg.mozilla.org/try/raw-rev/c07b819b4341

---

Beta Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=da564fd4c6d2

Beta patch rebased, ready to be uplift:
https://hg.mozilla.org/try/raw-rev/11d842cf31d4

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d3923a72848e

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4c2d05f5523c