1361756 - WebM video format not working if site triggers network download for invisible media elements

Status: VERIFIED FIXED

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

Description

[Tim]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/603.1.30 (KHTML, like Gecko) Version/10.1 Safari/603.1.30

Steps to reproduce:

Visit stanrobak.com or chriscorley.com in Firefox latest version (v53.0 through v55.0 nightly), try to play any video or audio on the home or media pages. You can use Windows or Mac, issue is present in both. Have not tested in Linux.


Actual results:

Video/audio does not play. Seems to have trouble loading the media file both locally from the server, and remotely such as from YouTube and Vimeo.


Expected results:

HTML5 audio/video should play in the browser as they have in the past. Everything was working fine until Firefox was updated to v53.0, both Windows and Mac. Tested the sites successfully in a prior version, then updated to v53.0 and this issue appeared.

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ec7fb4f14d3ec23ded7eea40ff49ebbcbec6bde1&tochange=8d2eecb7ea5a16e02862dd326ce4519082ce9901

Regressed by: Bug 1271765


I think that the bunch of problematic patch Bug 1271765 should be backed out completely.

Comment 2

[Alice0775 White]

Wait, please ignore comment#1.

Comment 3

[Alice0775 White]

Steps to reproduce:
1. Open http://stanrobak.com/
2. Wait to stop tab icon spinning
3. Click play button of TRUNP 2016 video

Actual Results:
The video would not start to playback.

Expected Results:
The bideo should start to playback.


Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fd57bcd5daf2e1dd79021f4fa7ec659ad4b40d21&tochange=09fb9831233593b5c88028ff9400993acd680559

Regressed by: Bug 1331329

@:jwwang
Your bunch of patch causes the regression. Please look this.

Comment 4

[Blake Wu [:bwu][:blakewu]]

I can see this bug on 53 and the latest nightly, but not on 52.

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

No. This is not a regression from bug 1331329 which just makes the symptom more obvious. Going to about:config and changing "media.num-decode-threads" to 9 or above will fix the issue.

The site somehow blocks network download for invisible media elements. All media threads are blocked by network and no more thread available for new decoding/seeking request and therefore playback seems to get stuck.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

You can also use some extra steps to recover playback:

4. scroll down to Heck 2016 and click play
5. wait for Heck 2016 to start playback
6. scroll up to TRUNP 2016 and click play again
   (it is curious that the site seems to pause a media element when it becomes invisible)
7. TRUNP 2016 should now play without any problem

Comment 7

[Blake Wu [:bwu][:blakewu]]

Bug 1329490 looks like having similar problem. Once I new a integer "media.num-decode-threads" and set it to be 10. The problem is gone.

Comment 8

[Gerry Chang [:gchang]]

Track 54+/55+ as important audio/video playback issue.

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1361756 - don't reset decoders when entering dormant.

Resetting decoders somehow cause the WebM demuxer to seek and initiate
network download which is blocked by the site for invisible media elements.

See comment 5 for how this blocks playback.

We work around this issue by not resetting decoders when entering dormant
since we will reset them anyway during seeking when exiting dormant.

This change is a workaround for this bug though. However it is also an
improvement for the dormant state in general since it removes unnecessary
works to improve performance.

Review commit: https://reviewboard.mozilla.org/r/136428/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136428/

Comment 10

[Jon K]

I'm experiencing the same bug with all of these HTML 5 video players:

http://videojs.com

http://jplayer.org/

http://easyhtml5video.com

http://www.html5videoplayer.net

Comment 11

[Jon K]

Just downloaded today's new Firefox update; playback issue still persists with all of these HTML players.

http://videojs.com

http://jplayer.org/

http://easyhtml5video.com

http://www.html5videoplayer.net

Comment 12

[Tzuhao Kuo [:kaku]]

Comment on attachment 8864770 [details]
Bug 1361756 - don't reset decoders when entering dormant.

https://reviewboard.mozilla.org/r/136428/#review139894

::: commit-message-93168:3
(Diff revision 1)
> +Resetting decoders somehow cause the WebM demuxer to seek and initiate
> +network download which is blocked by the site for invisible media elements.

> Resetting decoders somehow cause the WebM demuxer to seek and initiate

Is it expected? should we have an separated bug for it?

::: dom/media/MediaDecoderStateMachine.cpp:536
(Diff revision 1)
> +  void HandleWaitingForAudio() override
> +  {
> +    MaybeReleaseResources();
> +  }
> +  void HandleWaitingForVideo() override
> +  {
> +    MaybeReleaseResources();
> +  }

You have disconneted `mMaster->mAudioWaitRequest` and `mMaster->mVideoWaitRequest` at `Enter()`, we should not override these two handlers because we're not expecting these two events, right? We should use the versions in `StateObject` to crash.

Comment 13

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8864770 [details]
Bug 1361756 - don't reset decoders when entering dormant.

https://reviewboard.mozilla.org/r/136428/#review139894

> > Resetting decoders somehow cause the WebM demuxer to seek and initiate
> 
> Is it expected? should we have an separated bug for it?

The typical usage is to call ResetDecode() on a MediaDecoderReader to put it in a stable state before calling ReleaseResources(). Since we want to bypass the ResetDecode() call, we need to wait for it to become idle.

> You have disconneted `mMaster->mAudioWaitRequest` and `mMaster->mVideoWaitRequest` at `Enter()`, we should not override these two handlers because we're not expecting these two events, right? We should use the versions in `StateObject` to crash.

No. the disconnection prevents Handle{Audio,Video}Waited from being called instead of HandleWaitingFor{Audio,Video}.

Comment 14

[Tzuhao Kuo [:kaku]]

Comment on attachment 8864770 [details]
Bug 1361756 - don't reset decoders when entering dormant.

https://reviewboard.mozilla.org/r/136428/#review139894

> No. the disconnection prevents Handle{Audio,Video}Waited from being called instead of HandleWaitingFor{Audio,Video}.

Sorry, my fault.

Comment 15

[Tzuhao Kuo [:kaku]]

Comment on attachment 8864770 [details]
Bug 1361756 - don't reset decoders when entering dormant.

https://reviewboard.mozilla.org/r/136428/#review139902

Comment 16

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Jon K from comment #11)
> Just downloaded today's new Firefox update; playback issue still persists
> with all of these HTML players.
> 
> http://videojs.com
> 
> http://jplayer.org/
> 
> http://easyhtml5video.com
> 
> http://www.html5videoplayer.net

Do you experience the same issue with Beta(54) or Release(53)?

Comment 17

[Jon K]

I do experience the issue with 53.0.2 (64-bit) - I haven't downloaded 54 beta to test yet.

(In reply to JW Wang [:jwwang] [:jw_wang] from comment #16)
> (In reply to Jon K from comment #11)
> > Just downloaded today's new Firefox update; playback issue still persists
> > with all of these HTML players.
> > 
> > http://videojs.com
> > 
> > http://jplayer.org/
> > 
> > http://easyhtml5video.com
> > 
> > http://www.html5videoplayer.net
> 
> Do you experience the same issue with Beta(54) or Release(53)?

Comment 18

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the review!

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cba5c15d4c98
don't reset decoders when entering dormant. r=kaku

Comment 20

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

https://hg.mozilla.org/mozilla-central/rev/cba5c15d4c98

Comment 21

[Jon K]

I downloaded the latest beta and it looks like the playback issue is resolved with it! Thank you so much!

Comment 22

[JW Wang [:jwwang] [:jw_wang]]

Thanks for testing! We will need to uplift the fix to 53&54.

Comment 23

[Jon K]

Seriously a HUGE THANK YOU all who worked on this - I'm so relieved that I can tell my clients their videos will work again soon!

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

If this fix isn't on 54 yet, how is it fixed in the latest beta? 
Can you request uplift please?

JW, do you think this problem is widespread enough that we should do a 53 dot release for it?

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

Andrei, can your team test this issue, with the current beta ? Thanks!

Comment 26

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1361756_fix_beta_54.patch

Approval Request Comment
[Feature/Bug causing the regression]:1331329 which doesn't cause regression but reveal a hidden bug that's been there long before.
[User impact if declined]:video won't play in some sites.
[Is this code covered by automated tests?]:yes.
[Has the fix been verified in Nightly?]:yes.
[Needs manual test from QE? If yes, steps to reproduce]: yes. See comment 3 and comment 10.
[List of other uplifts needed for the feature/fix]:none
[Is the change risky?]:low
[Why is the change risky/not risky?]:the change is quite simple and manually tested on Nightly.
[String changes made/needed]:none

Comment 27

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #24)
> JW, do you think this problem is widespread enough that we should do a 53
> dot release for it?

Yes. I've requested uplift to 54. If QA finds it good after testing, we should uplift it to 53 as well.

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you explain how broadly, what platforms are affected, and so on? Do we have telemetry that we could look at to explain the user impact?  I need more information to make a decision about a dot release. I'm not seeing any duplicate bugs or reports from users about this issue.

Comment 29

[Liz Henry (:lizzard) (relman/hg->git project)]

Is bug 1329490 a duplicate?

Comment 30

[Liz Henry (:lizzard) (relman/hg->git project)]

jya, cpearce, checking in with you too for data on user impact here.

Comment 31

[Blake Wu [:bwu][:blakewu]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #28)
> Can you explain how broadly, what platforms are affected, and so on? Do we
> have telemetry that we could look at to explain the user impact?  I need
> more information to make a decision about a dot release. I'm not seeing any
> duplicate bugs or reports from users about this issue.
Kaku,
Since JW is on PTO, can you help answer this question?

Comment 32

[Tzuhao Kuo [:kaku]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #28)
> Can you explain how broadly, what platforms are affected, and so on? 
Yes, the phenomenon we observed here somehow depends on the site's own behavior. 
The site blocks network download for invisible media elements and then blocks all media threads.
Our solution here is not triggering unnecessary network requests for invisible media elements. 
This issue is platform independent, so it is universal but only be triggered by specific website behavior. 
We still recommend to uplift the patch considering the patch is low risk (Comment 26) and benefits a lot.

> Do we have telemetry that we could look at to explain the user impact?  
No, sorry.

> I need more information to make a decision about a dot release. 
> I'm not seeing any duplicate bugs or reports from users about this issue.
Here are two factors: (1) I think it's because that not every site tries to block networking for invisible medias, and (2) this only happens to WebM which is relatively not as popular as mp4.



(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #29)
> Is bug 1329490 a duplicate?
Not exactly. Bug 1329490 has multiple root causes which include the one in this bug.

Comment 33

[Emil Ghitta, QA [:emilghitta]]

Hi Liz!

This issue seems fixed in Firefox Nightly 55.0a1 (Build Id:20170511063838) using Windows 10 64bit , MacOS 10.11.6 and Ubuntu 16.04 64bit.

On Firefox 54.0b7 the issue mentioned in comment 3 can be reproduced (apparently it is not reproducible with the links provided in comment 10), I think that maybe John K missed comment 6 while testing it on beta.

So in conclusion:

The issue is reproducible on Beta (The video  fails to playback and it can be recovered by following the steps mentioned in comment 6).
The issue is fixed in Firefox Nightly 55.0a1 (The video playbacks as soon as the play button is pressed).

I am leaving the qe-verify+ until it is verified fixed in 54 as well.
Please needinfo me if you have any questions.

Comment 34

[Liz Henry (:lizzard) (relman/hg->git project)]

One more question! Would this also affect WebM playback on Fennec?

Comment 35

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8866602 [details] [diff] [review]
1361756_fix_beta_54.patch

Let's uplift this to beta 54. It should be in Monday's beta 8 build.

Comment 36

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

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #30)
> jya, cpearce, checking in with you too for data on user impact here.

I'm hesitant to spin a dot release for this as, if I understand correctly, this affects only non-MSE WebM, which I think should be a smallish proportion of the population of web video.

That is to say, YouTube isn't affected by this, it's only sites that have this "throttle invisible videos' network requests" behaviour, and we don't know how prevalent that is.

telemetry.mozilla.org has been dropping requests and unresponsive for about a month now, so I can't access the MEDIA_CODEC_USED probe to back up my claim that WebM non-MSE video isn't that common.

Comment 37

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

(In reply to JW Wang [:jwwang] [:jw_wang] from comment #13)
> Comment on attachment 8864770 [details]
> Bug 1361756 - don't reset decoders when entering dormant.
> 
> https://reviewboard.mozilla.org/r/136428/#review139894
> 
> > > Resetting decoders somehow cause the WebM demuxer to seek and initiate
> > 
> > Is it expected? should we have an separated bug for it?
> 
> The typical usage is to call ResetDecode() on a MediaDecoderReader to put it
> in a stable state before calling ReleaseResources(). Since we want to bypass
> the ResetDecode() call, we need to wait for it to become idle.


It seems to me that the real bug here lies in the WebM demuxer? It should not need to do network requests when it's been reset.

Comment 38

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

(In reply to Chris Pearce (:cpearce) from comment #36)
> telemetry.mozilla.org has been dropping requests and unresponsive for about
> a month now, so I can't access the MEDIA_CODEC_USED probe to back up my
> claim that WebM non-MSE video isn't that common.

I filed bug 1364817 for telemetry.mozilla.org dropping requests.

Comment 39

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Chris Pearce (:cpearce) from comment #37)
> (In reply to JW Wang [:jwwang] [:jw_wang] from comment #13)
> > Comment on attachment 8864770 [details]
> > Bug 1361756 - don't reset decoders when entering dormant.
> > 
> > https://reviewboard.mozilla.org/r/136428/#review139894
> > 
> > > > Resetting decoders somehow cause the WebM demuxer to seek and initiate
> > > 
> > > Is it expected? should we have an separated bug for it?
> > 
> > The typical usage is to call ResetDecode() on a MediaDecoderReader to put it
> > in a stable state before calling ReleaseResources(). Since we want to bypass
> > the ResetDecode() call, we need to wait for it to become idle.
> 
> 
> It seems to me that the real bug here lies in the WebM demuxer? It should
> not need to do network requests when it's been reset.

Exactly. As said in the comment of https://hg.mozilla.org/mozilla-central/rev/cba5c15d4c98, the change is a workaround. However it is also an improvement to the dormant state in general.

Comment 40

[Jean-Yves Avenard [:jya]]

I believe the WebM demuxer issue has been fixed.

Comment 41

[Liz Henry (:lizzard) (relman/hg->git project)]

jya: I'm not sure what you mean. Fixed somewhere else? With what effect for the way this bug is showing up for some sites/users?

chutten is looking up the data for us now, but my impression so far is that we can let this fix ride with 54.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5e5e0d19cf12

Comment 43

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

(In reply to Jean-Yves Avenard [:jya] from comment #40)
> I believe the WebM demuxer issue has been fixed.

In what bug was that? Should we uplift that too?

Comment 44

[Jean-Yves Avenard [:jya]]

Maybe I'm confused... I was referring to bug 1361964, which causes extra GET  

Yes, that should be uplifted (but that uplift request we denied for some reason)

Comment 45

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using Fx 55.0a1, build ID:20170503030212, on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 54.0b8 on Windows 10 x64, Mac OS X 10.11.6 and Ubuntu 14.04 LTS.

Cheers!