1202296 - HTML5 Video on YouTube are not playing with FF 41 beta 7

Status: RESOLVED FIXED

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

Description

[giopeppe]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150831172306

Steps to reproduce:

Update from Firefox 41 beta 6 to Firefox beta 7 and go to Youtube to playsome videos.


Actual results:

I can't play video with HTML5 player anymore, i get the message "An error is occurred". Using Adobe Flash works. 
Rolling back to beta 6 they work again. 

While doing some backtracking (at first i tought it was due to an extension) i found that it's probably related to hardware video decoding. 

Noticing that all seems to works with a new profile i found that the culprit is 

media.hardware-video-decoding.failed : false

In Beta 7, If i set it to "true", all seem to work (but hw video decoding should be gone, right?). 


Expected results:

Videos should just play normally.

Comment 1

[Loic]

WFM, could you test with a fresh profile, please.
https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles

Comment 2

[giopeppe]

(In reply to Loic from comment #1)
> WFM, could you test with a fresh profile, please.
> https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-
> firefox-profiles

Sure, i already do, here the results:

With 41 beta 6: 

When i create a fresh profile "media.hardware-video-decoding.failed" is automatically set to "false" and html5 video playback is working.

With 41 beta 7, using the fresh profile created with beta 6:
Video playback is not working and "media.hardware-video-decoding.failed : false", if i reset the value in about:config, it's not recreated, if i add it manually and set to "true", videoplaback works again.

With 41 beta 7, creating a freshly profile:
Video playback works and "media.hardware-video-decoding.failed : true" is automatically set to "true".


I have the about:support data of all three conditions, could they help?

Comment 3

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to giopeppe from comment #2)
> I have the about:support data of all three conditions, could they help?

The graphics section of about:support would help to identify which GPU we're dealing with so we can try to repro it. You could use http://mozilla.github.io/mozregression/ to figure out which change broke it.

Matt/Mason - are you aware of anything that changed in b7?

Comment 4

[giopeppe]

Attachment: Graphic portion of about:support, on FF 41 beta 7

This is the about support of Beta 7 with a freshly created profile, it have h264 HW support: False and videoplayback of html5 videos works. 

With beta 6, H264 HW support is: True and videoplayback works, updating to beta 7 form an older profile, HW support is setted to "True" but video playback don't work at all.

Comment 5

[Mason Chang [Inactive] [:mchang]]

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #3)
> (In reply to giopeppe from comment #2)
> > I have the about:support data of all three conditions, could they help?
> 
> The graphics section of about:support would help to identify which GPU we're
> dealing with so we can try to repro it. You could use
> http://mozilla.github.io/mozregression/ to figure out which change broke it.
> 
> Matt/Mason - are you aware of anything that changed in b7?

I'm not aware of anything that changed in b7.

Comment 6

[Matt Woodrow (:mattwoodrow)]

Bug 1193547 landed for b7 I believe, that's my guess.

We should probably back that out for now.

Comment 7

[Matt Woodrow (:mattwoodrow)]

I can reproduce this on a machine in the TO office.

Looks like reconfiguring the MFTransform when we decide we can't use DXVA doesn't work sometime, it may be specific to the windows 7 version of the library (since it definitely works on other machines).

Comment 8

[Benjamin Smedberg]

[Tracking Requested - why for this release]: we cannot tolerate any regressions on youtube.

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked.

Comment 10

[Matt Woodrow (:mattwoodrow)]

Attachment: Hide MFTransform so we can recreate it opaquely

Comment 11

[Matt Woodrow (:mattwoodrow)]

Attachment: Recreate MFT rather than re-configuring it

This works much better for windows 7, and seems simpler too.

Comment 12

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

Comment on attachment 8658945 [details] [diff] [review]
Recreate MFT rather than re-configuring it

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

r+ with explanations or fixes for my comments.

::: dom/media/platforms/wmf/WMFVideoMFTManager.cpp
@@ +330,2 @@
>    // SupportsConfig only checks for valid h264 decoders currently.
> +  if (mStreamType != H264) {

Why are you returning true that non-H264 supports DXVA? We haven't really tested non-H264 DXVA have we? Shouldn't this remain at "return false"?

@@ +358,1 @@
>  

You deleted the line:
    HRESULT hr = mDecoder->GetOutputMediaType(mediaType);

Do we need to keep that line? Can the output type change after we re-init? Wouldn't the YUV format need to change from NV12 to YV12 when we switch from DXVA to non-DXVA? Or are you getting a MF_E_TRANSFORM_STREAM_CHANGE on first output that is causing us to re-get the output types next cycle?

Comment 13

[Matt Woodrow (:mattwoodrow)]

(In reply to Chris Pearce (:cpearce) from comment #12)
> Comment on attachment 8658945 [details] [diff] [review]
> Recreate MFT rather than re-configuring it
> 
> Review of attachment 8658945 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with explanations or fixes for my comments.
> 
> ::: dom/media/platforms/wmf/WMFVideoMFTManager.cpp
> @@ +330,2 @@
> >    // SupportsConfig only checks for valid h264 decoders currently.
> > +  if (mStreamType != H264) {
> 
> Why are you returning true that non-H264 supports DXVA? We haven't really
> tested non-H264 DXVA have we? Shouldn't this remain at "return false"?

We're returning true to mean that we have no idea if it's supported (since the test below doesn't take it into account), and we shouldn't take any action.


> 
> @@ +358,1 @@
> >  
> 
> You deleted the line:
>     HRESULT hr = mDecoder->GetOutputMediaType(mediaType);
> 
> Do we need to keep that line? Can the output type change after we re-init?
> Wouldn't the YUV format need to change from NV12 to YV12 when we switch from
> DXVA to non-DXVA? Or are you getting a MF_E_TRANSFORM_STREAM_CHANGE on first
> output that is causing us to re-get the output types next cycle?

Yep, we're re-inputting on the brand new MFT and returning immediately, which should once again trigger the STREAM_CHANGE and we'll go through this function again.

Comment 14

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

OK, cool.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a7d924a0f34
https://hg.mozilla.org/integration/mozilla-inbound/rev/e7e0427710e1

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Would you be able to nominate a patch for this soon? We will gtb 41 RC on Monday morning PST.

Comment 17

[Matthew Gregan [:kinetik]]

I assume you meant to ask the other Matt. :-)

Comment 18

[Matt Woodrow (:mattwoodrow)]

Bug 1193547 got backed out of beta/aurora, so would shouldn't need to uplift anything here.

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8a7d924a0f34
https://hg.mozilla.org/mozilla-central/rev/e7e0427710e1