1348432 - Missing audio in YouTube videos that were requested within the same tab. Refreshing page fixes audio.

Status: VERIFIED FIXED

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

Description

[smartfon.reddit]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170317111607

Steps to reproduce:

Visit YouTube and play any video. It plays fine. Click to play another video.


Actual results:

That, and all future videos play without audio, unless the page is refreshed. The page must be refreshed for every new video. Started after 2017-03-17 Nightly. Most likely after the second (~2PM PST) update - not the morning one. Tested with new profile. Not related to YouTube mute button.


Expected results:

You should be able to request a video in the same tab without having to refresh the tab, to play the audio.

Comment 1

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=aecbad803701a0d8eab8bad19348df37dcb42435&tochange=23cfb067130f6225f9b63c752c349fd607e69c46

Regressed by: Bug 1347402

Comment 2

[Tzuhao Kuo [:kaku]]

Thanks for reporting this regression in quickly!

The root cause is that we moved MediaDecoder::ConstructMediaTracks() , MediaDecoder::RemoveMediaTracks(), and MediaDecoder::mMediaTracksConstructed (which is a boolean) into HTMLMediaElement in bug 1347402 patch 4. It's fine to the two methods but not right to the MediaDecoder::mMediaTracksConstructed flag. This flag should belong to a MediaDecoder, every time a HTMLMediaElement switches its MediaDecoder, the flag should be reset to false again. 

So, we have two solutions to this bug:
(1) Keep the flag in the HTMLMediaElement and reset it while shouting down a decoder.
(2) Keep the flag in the MediaDecoder and it will be initialized to be false while we have a new MediaDecoder.

I think the 2nd proposal is better since it won't mess up the code in HTMLMediaElement::ShutdownDecoder().

Comment 3

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1348432 - move the mMediaTracksConstructed flag back to MediaDecoder;

This mMediaTracksConstructed flag should belong to a MediaDecoder,
every time a HTMLMediaElement switches its MediaDecoder, the flag should be reset to false again.

So, we move the mMediaTracksConstructed flag back to MediaDecoder, by this way,
HTMLMediaElement provides only the mechanism to construct and remove media tracks,
and MediaDecoder uses the flag, mMediaTracksConstructed, to provide policy.

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

Comment 4

[Tzuhao Kuo [:kaku]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f7df09912faab115655284ba73e794781ea7461
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2877f3bf6e35fb0c895475917b657187a7d1f12a

Comment 6

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

Comment on attachment 8848831 [details]
Bug 1348432 - move the mMediaTracksConstructed flag back to MediaDecoder;

https://reviewboard.mozilla.org/r/121708/#review123776

It is a bad smell that MediaDecoder shows too much interest in HTMLMediaElement's properties because it breaks encapsulation. I think the issue can be resolved by calling GetOwner()->RemoveMediaTracks() in MediaDecoder::Shutdown().

Comment 7

[Tzuhao Kuo [:kaku]]

Comment on attachment 8848831 [details]
Bug 1348432 - move the mMediaTracksConstructed flag back to MediaDecoder;

https://reviewboard.mozilla.org/r/121708/#review123776

Good idea, will do it.

Comment 8

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1348432 - while shutting down a MediaDecoder, ask its owner to remove all media tracks;

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

Comment 9

[Tzuhao Kuo [:kaku]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba0f5c924e223558483b445d3889b44809c60438
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4fbc6ebe4c98476c3ebd1e2ab6415f1bebe2025

Comment 10

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

Comment on attachment 8848968 [details]
Bug 1348432 - while shutting down a MediaDecoder, ask its owner to remove all media tracks;

https://reviewboard.mozilla.org/r/121814/#review123782

Comment 12

[Xavier Delgado]

I've got this bug too. Sound doesn't work more or less 1 time each 3 videos with automatic play switch "on". I don't have a strict str.

browser:      Nightly 55.0a1 (2017-03-19) (64 bits)
OS:           Windows 10
hd:           SSD Crucial 256 GO
graphic card: NVIDIA GeForce GTX 960

Comment 14

[Shayne Jewers]

This isn't just YouTube, it also happens for me on the Steam store when attempting to put a trailer in fullscreen.

Comment 17

[Serphentas]

This problem also occurs when new content is loaded by the user within a page (without loading), for example when playing two videos in a row on Facebook or listen to two tracks in a row on VK.

Comment 19

[Tzuhao Kuo [:kaku]]

(In reply to Tzuhao Kuo [:kaku] from comment #9)
> Try:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba0f5c924e223558483b445d3889b44809c60438
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4fbc6ebe4c98476c3ebd1e2ab6415f1bebe2025

Try looks very crazy, and I am not able to figure it out in a short period, so, I would like to land attachment 8848831 [details] at this moment as a workaround. Attachment 8848831 [details] is kind of reverting the root cause of this bug which was introduced at bug 1347402 patch 4. 

Will upload a new patch soon.

Comment 20

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1348432 - move the mMediaTracksConstructed flag back to MediaDecoder;

This mMediaTracksConstructed flag should belong to a MediaDecoder,
every time a HTMLMediaElement switches its MediaDecoder, the flag should be reset to false again.

So, we move the mMediaTracksConstructed flag back to MediaDecoder, by this way,
HTMLMediaElement provides only the mechanism to construct and remove media tracks,
and MediaDecoder uses the flag, mMediaTracksConstructed, to provide policy.

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

Comment 21

[Tzuhao Kuo [:kaku]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2823d84b2693074edc08c106bfbf0f58d9f749cc
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d9df7e78a54dbc0ac512cbf344aa0f943e02d00

Comment 22

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

Comment on attachment 8849824 [details]
Bug 1348432 - move the mMediaTracksConstructed flag back to MediaDecoder;

https://reviewboard.mozilla.org/r/122570/#review124760

Comment 23

[Tzuhao Kuo [:kaku]]

Try looks good, thanks for the review!

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c979d49bdbc5
move the mMediaTracksConstructed flag back to MediaDecoder; r=jwwang

Comment 25

[Carsten Book [:Tomcat]]

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

Comment 29

[Jan Sonntag]

Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
BuildID: 20170323030203

Bug seems to be fixed in the new Nightly Build.

Thank you very much :)

Comment 30

[Darkspirit]

My reproducible ways to this bug (on other websites than youtube when changing the video quality) are no longer working. Audio is now working as expected. Thank you ;-)

Comment 31

[Darkspirit]

(In reply to bugzilla from comment #30)
> My reproducible ways to this bug (on other websites than youtube when
> changing the video quality) are no longer working. Audio is now working as
> expected. Thank you ;-)

Sorry, I've forgotten: I am using BuildID: 20170323110205 @ Debian testing = Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

Comment 32

[smartfon.reddit]

Works for me after 3/23/2017 update.

Comment 33

[Tzuhao Kuo [:kaku]]

Thank you all for keep monitoring this bug and verifying the patch.

I marked this bug as "leave-open" since I want to evolve the solution, however, now, I think I should just open a new bug and close this one for this issue is indeed fixed.

Comment 34

[u591426]

Thanks so much, the problem is fixed for me! Works great now for me on today's build, keep up the amazing work!

Comment 35

[laugeo]

Not fixed in Xubuntu 14.04 , fixed in Xubuntu 16.04:
Pulseaudio Nightly volume stills muted when new video is played using Xubuntu 14.04:
(as described in this initial report:
 https://bugzilla.mozilla.org/show_bug.cgi?id=1349331

Xubuntu 14.04 data:
Firefox
Version 	55.0a1
Identifiant de compilation 	20170324110206
nightly
Agent utilisateur 	Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Système d’exploitation 	Linux 3.13.0-101-generic

Comment 36

[laugeo]

The previous 35 comment is bad :
It 's ok also with Xubuntu 14.04 using a new profile .

Comment 37

[Darkspirit]

(In reply to laugeo from comment #36)
> The previous 35 comment is bad :
> It 's ok also with Xubuntu 14.04 using a new profile .

Are you sure that a new profile is fixing this? Maybe this is working because you had running only one application with audio output at the time when you tested it, preventing to tap into a possible pulseaudio bug I also have.

https://bugzilla.mozilla.org/show_bug.cgi?id=1349331#c8

Comment 38

[laugeo]

Sure it was bad profile.  it was a Nightly bug, there was no problem with other audio pulseaudio clients running at the same time.