1627999 - Windows volume + media info shows incorrect info when multiple videos on page

Status: VERIFIED FIXED

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

Description

[Andrew Overholt [:overholt]]

I was looking at the main page of https://www.reddit.com/ and there was a Youtube video of 'Never Gonna Give You Up' which I scrolled past and played another video down the page. When pressing volume up/down with the (lower on the page) video playing, the media info thing in Windows showed 'Rick Astley - ...' (for 'Never Gonna Give You Up').

Today's Nightly on Windows 10

Comment 1

[Alastor Wu [:alwu]]

Do you mean the media metadata showing on SMTC [1] is incorrect?

From your description, do you mean there were two video ('Never Gonna Give You Up' and 'Rick Astley - ...' playing at the same time, but the information showed 'Rick Astley - ...', but you expected to see 'Never Gonna Give You Up'? Is 'Rick Astley - ...', also a youtube video? or is it a normal HTML video?

Thank you.

[1] https://bug1615665.bmoattachments.org/attachment.cgi?id=9129321

Comment 2

[Andrew Overholt [:overholt]]

(In reply to Alastor Wu [:alwu] from comment #1)

Do you mean the media metadata showing on SMTC [1] is incorrect?

Yes.

From your description, do you mean there were two video ('Never Gonna Give You Up' and 'Rick Astley - ...' playing at the same time, but the information showed 'Rick Astley - ...', but you expected to see 'Never Gonna Give You Up'? Is 'Rick Astley - ...', also a youtube video? or is it a normal HTML video?

Sorry, I did a poor job explaining. The page looked something like this:

[... page content ...]
(embedded YouTube video of "Never Gonna Give You Up") [note: I never played this video]
[... more page content ...]
<video> that I did play and while it was playing, pressed the volume up key on my laptop keyboard

So it was indeed (1) a YouTube video followed by (2) a normal HTML video and I played (2) and never played (1).

Comment 3

[Alastor Wu [:alwu]]

Target the fix on this release cycle, mark it as P1.

Comment 4

[Alastor Wu [:alwu]]

Quick update, I'm still working on this bug and doing some small reflectoring before implementing the solution.

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part4 : use only one delegate in the media controller.

Comment 6

[Phabricator Automation]

Comment on attachment 9143250 [details]
Bug 1627999 - part4 : use only one delegate in the media controller.

Revision D72474 was moved to bug 1633010. Setting attachment 9143250 [details] to obsolete.

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part1 : manage the audio focus in `MediaPlaybackStatus`

Comment 8

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part2 : update the active media session when the owner of the audio focus changes.

Comment 9

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part3 : activate controller when it first time becomes audible.

Comment 10

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part4 : listen to the playback change from the event source.

Comment 11

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part5 : remove out-of-date test.

Comment 12

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part6 : modify 'test_trigger_actionhanlder.html'.

Comment 13

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part7 : add test.

Comment 14

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part8 : remove audible check in media element.

This patch will do :

• remove audible check from the logic of registering controller
• include audio channel affect on the media element's audible state

The advantage of doing so :

• it can help to reduce the intermittent failure during testing by earlier hooking media elements in the content process to the media controller in the chrome process

More details :
In D72497, we have added the audible check to postpone the activation of the media controller, which would ensure that we only control media after it become audible. Therefore, we can remove the previous implementation which we use to achieve that in media element.

Therefore, when having that audible check in media, that would postpone the timing of adding media element to ContentMediaController. That causes some intermitent failures when I was writing test for bug1633565. When removing these checks, we can ensure that the media element would have always been added into ContentMediaController after calling video.play(). If the element haven't been added into ContentMediaController, then it would miss to handle the media key events when test generates a fake media key event, which causes an intermitent failure.

We use controller's audible state to decide if the controller should be activated, but the old audible state doesn't include the effect from the audio channel. Therefore, we have to add it to prevent activating a controller in a muted tab.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/039ecd63e3fb
part1 : manage the audio focus in `MediaPlaybackStatus` r=bryce
https://hg.mozilla.org/integration/autoland/rev/10e3550226eb
part2 : update the active media session when the owner of the audio focus changes. r=bryce
https://hg.mozilla.org/integration/autoland/rev/72a1340bc112
part3 : activate controller when it first time becomes audible. r=bryce
https://hg.mozilla.org/integration/autoland/rev/433a0eb70fc6
part4 : listen to the playback change from the event source. r=bryce
https://hg.mozilla.org/integration/autoland/rev/37b8f7a47ede
part5 : remove out-of-date test. r=bryce
https://hg.mozilla.org/integration/autoland/rev/c9c9f78e131d
part6 : modify 'test_trigger_actionhanlder.html'. r=bryce
https://hg.mozilla.org/integration/autoland/rev/5533b1c9e3ed
part7 : add test. r=bryce
https://hg.mozilla.org/integration/autoland/rev/98e98e2ecb56
part8 : remove audible check in media element. r=bryce

Comment 16

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Cpending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&revision=98e98e2ecb56b0bd6acef33e8d702ec606abc35c&searchStr=wpt&selectedTaskRun=Je0btUZtQPejM6Hm1gREcg-0

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=302222674&repo=autoland

Back-out link: https://hg.mozilla.org/integration/autoland/rev/a96011ce18741c016a0e985736c0f50ec96b73b7

[task 2020-05-14T08:34:57.147Z] 08:34:57 INFO - TEST-OK | /html/semantics/embedded-content/media-elements/playing-the-media-resource/loop-from-ended.tentative.html | took 3829ms
[task 2020-05-14T08:34:57.148Z] 08:34:57 INFO - TEST-START | /html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html
[task 2020-05-14T08:34:57.151Z] 08:34:57 INFO - Closing window 10737418605
[task 2020-05-14T08:34:58.148Z] 08:34:58 INFO - PID 7030 | Assertion failure: mAudibleMediaNum < mPlayingMediaNum, at /builds/worker/checkouts/gecko/dom/media/mediacontrol/MediaPlaybackStatus.h:76
[task 2020-05-14T08:34:58.175Z] 08:34:58 INFO - STDOUT: Initializing stack-fixing for the first stack frame, this may take a while...
[task 2020-05-14T08:34:58.482Z] 08:34:58 INFO - IOError on command, setting status to CRASH
[task 2020-05-14T08:34:58.527Z] 08:34:58 INFO - mozcrash Copy/paste: /builds/worker/fetches/minidump_stackwalk/minidump_stackwalk /tmp/tmpOak_14/minidumps/796f861d-3116-1bd0-deef-eb557ae49a2c.dmp /builds/worker/workspace/build/symbols
[task 2020-05-14T08:35:05.359Z] 08:35:05 INFO - mozcrash Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/796f861d-3116-1bd0-deef-eb557ae49a2c.dmp
[task 2020-05-14T08:35:05.359Z] 08:35:05 INFO - mozcrash Saved app info as /builds/worker/workspace/build/blobber_upload_dir/796f861d-3116-1bd0-deef-eb557ae49a2c.extra
[task 2020-05-14T08:35:05.522Z] 08:35:05 INFO - PROCESS-CRASH | /html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html | application crashed [@ mozilla::dom::MediaPlaybackStatus::ContextMediaInfo::IncreaseAudibleMediaNum()]
[task 2020-05-14T08:35:05.522Z] 08:35:05 INFO - Mozilla crash reason: MOZ_DIAGNOSTIC_ASSERT(mAudibleMediaNum < mPlayingMediaNum)
[task 2020-05-14T08:35:05.522Z] 08:35:05 INFO - Crash dump filename: /tmp/tmpOak_14/minidumps/796f861d-3116-1bd0-deef-eb557ae49a2c.dmp
[task 2020-05-14T08:35:05.522Z] 08:35:05 INFO - Operating system: Linux
[task 2020-05-14T08:35:05.523Z] 08:35:05 INFO - 0.0.0 Linux 4.4.0-1014-aws #14taskcluster1-Ubuntu SMP Tue Apr 3 10:27:00 UTC 2018 x86_64
[task 2020-05-14T08:35:05.523Z] 08:35:05 INFO - CPU: amd64
[task 2020-05-14T08:35:05.523Z] 08:35:05 INFO - family 6 model 62 stepping 4
[task 2020-05-14T08:35:05.523Z] 08:35:05 INFO - 4 CPUs
[task 2020-05-14T08:35:05.523Z] 08:35:05 INFO -
[task 2020-05-14T08:35:05.523Z] 08:35:05 INFO - GPU: UNKNOWN
[task 2020-05-14T08:35:05.523Z] 08:35:05 INFO -
[task 2020-05-14T08:35:05.524Z] 08:35:05 INFO - Crash reason: SIGSEGV /SEGV_MAPERR
[task 2020-05-14T08:35:05.524Z] 08:35:05 INFO - Crash address: 0x0
[task 2020-05-14T08:35:05.524Z] 08:35:05 INFO - Process uptime: not available
[task 2020-05-14T08:35:05.524Z] 08:35:05 INFO -
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - Thread 0 (crashed)
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - 0 libxul.so!mozilla::dom::MediaPlaybackStatus::ContextMediaInfo::IncreaseAudibleMediaNum() [MediaPlaybackStatus.h:8a21ea441fc78be36c13f78fc3da9fb41985ed3a : 76 + 0x29]
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - rax = 0x00007fe764033d1e rdx = 0x0000000000000000
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - rcx = 0x000055d49b853e30 rbx = 0x00007fe742751fe0
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - rsi = 0x00007fe77005f8b0 rdi = 0x00007fe77005e680
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - rbp = 0x00007ffef5832920 rsp = 0x00007ffef5832920
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - r8 = 0x00007fe77005f8b0 r9 = 0x00007fe770fc6780
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - r10 = 0x0000000000000002 r11 = 0x0000000000000000
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - r12 = 0x00007fe7420ea2e8 r13 = 0x00007fe7420ea248
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - r14 = 0x00000002800000f5 r15 = 0x0000000000000001
[task 2020-05-14T08:35:05.525Z] 08:35:05 INFO - rip = 0x00007fe75f1d7d6a
[task 2020-05-14T08:35:05.526Z] 08:35:05 INFO - Found by: given as instruction pointer in context

Comment 17

[Alastor Wu [:alwu]]

Attachment: Bug 1627999 - part9 : handle the owner browsing context change for the media element.

This patch will do :

• update the media status when media changes its owner browsing context

The advantage of doing so :

• make the media status in ContextMediaInfo correcly

More details :
ContextMediaInfo stores the media status of each browsing context, but actually the media doesn't always need to stay in one browsing context. We can move it to other browsing contexts (iframe) by appending it to other browsing context's document body.
For example, in [1], we move the video from the main frame to another iframe.

Therefore, when we move the media to a new browsing context, we should also update its media status (controlledMedia/playing/audio number) for its previous owner browsing context.

[1] https://searchfox.org/mozilla-central/source/testing/web-platform/tests/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/76c6b64928e9
part1 : manage the audio focus in `MediaPlaybackStatus` r=bryce
https://hg.mozilla.org/integration/autoland/rev/021c014ba882
part2 : update the active media session when the owner of the audio focus changes. r=bryce
https://hg.mozilla.org/integration/autoland/rev/84434656006c
part3 : activate controller when it first time becomes audible. r=bryce
https://hg.mozilla.org/integration/autoland/rev/f4d85f0b61d9
part4 : listen to the playback change from the event source. r=bryce
https://hg.mozilla.org/integration/autoland/rev/e0fa806a6942
part5 : remove out-of-date test. r=bryce
https://hg.mozilla.org/integration/autoland/rev/55ac831f54f3
part6 : modify 'test_trigger_actionhanlder.html'. r=bryce
https://hg.mozilla.org/integration/autoland/rev/2e1c8f9e8f88
part7 : add test. r=bryce
https://hg.mozilla.org/integration/autoland/rev/b9025bab4036
part8 : remove audible check in media element. r=bryce
https://hg.mozilla.org/integration/autoland/rev/05ba58699a1b
part9 : handle the owner browsing context change for the media element. r=chunmin

Comment 19

[Natalia Csoregi [:nataliaCs]]

Backed out for bustages on HTMLMediaElement.cpp

backout: https://hg.mozilla.org/integration/autoland/rev/80de33f1eb8d34ce3e26ca695c19dcb8e1f07fb1

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=05ba58699a1b74e53b04ad3ca3330ec70714654e&searchStr=build&selectedTaskRun=WUoLO0BqR8a873rptWtZdw-0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302508913&repo=autoland&lineNumber=46411

[task 2020-05-15T21:19:41.535Z] 21:19:41 INFO - In file included from Unified_cpp_dom_html2.cpp:20:
[task 2020-05-15T21:19:41.535Z] 21:19:41 ERROR - /builds/worker/checkouts/gecko/dom/html/HTMLMediaElement.cpp:535:10: error: unused variable 'rv' [-Werror,-Wunused-variable]
[task 2020-05-15T21:19:41.535Z] 21:19:41 INFO - bool rv = Start();
[task 2020-05-15T21:19:41.535Z] 21:19:41 INFO - ^
[task 2020-05-15T21:19:41.535Z] 21:19:41 INFO - 1 error generated.
[task 2020-05-15T21:19:41.535Z] 21:19:41 INFO - /builds/worker/checkouts/gecko/config/rules.mk:746: recipe for target 'Unified_cpp_dom_html2.o' failed
[task 2020-05-15T21:19:41.535Z] 21:19:41 ERROR - make[4]: *** [Unified_cpp_dom_html2.o] Error 1
[task 2020-05-15T21:19:41.535Z] 21:19:41 INFO - make[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/html'
[task 2020-05-15T21:19:41.536Z] 21:19:41 INFO - make[4]: *** Waiting for unfinished jobs....

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/615c99ce554d
part1 : manage the audio focus in `MediaPlaybackStatus` r=bryce
https://hg.mozilla.org/integration/autoland/rev/6f6bdde73e3a
part2 : update the active media session when the owner of the audio focus changes. r=bryce
https://hg.mozilla.org/integration/autoland/rev/18ce43558e56
part3 : activate controller when it first time becomes audible. r=bryce
https://hg.mozilla.org/integration/autoland/rev/d1d2eb0af75e
part4 : listen to the playback change from the event source. r=bryce
https://hg.mozilla.org/integration/autoland/rev/cc9fcd608bec
part5 : remove out-of-date test. r=bryce
https://hg.mozilla.org/integration/autoland/rev/88b8944c99bd
part6 : modify 'test_trigger_actionhanlder.html'. r=bryce
https://hg.mozilla.org/integration/autoland/rev/39e3cf0e7204
part7 : add test. r=bryce
https://hg.mozilla.org/integration/autoland/rev/7df1ff108c67
part8 : remove audible check in media element. r=bryce
https://hg.mozilla.org/integration/autoland/rev/75e66ea73949
part9 : handle the owner browsing context change for the media element. r=chunmin

Comment 21

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/615c99ce554d
https://hg.mozilla.org/mozilla-central/rev/6f6bdde73e3a
https://hg.mozilla.org/mozilla-central/rev/18ce43558e56
https://hg.mozilla.org/mozilla-central/rev/d1d2eb0af75e
https://hg.mozilla.org/mozilla-central/rev/cc9fcd608bec
https://hg.mozilla.org/mozilla-central/rev/88b8944c99bd
https://hg.mozilla.org/mozilla-central/rev/39e3cf0e7204
https://hg.mozilla.org/mozilla-central/rev/7df1ff108c67
https://hg.mozilla.org/mozilla-central/rev/75e66ea73949

Comment 22

[Catalin Sasca, Desktop Test Engineering [:csasca]]

I successfully reproduced the issue on Firefox Nightly 77.0a1 (2020-04-07) under Windows 10 (x64).

The issue is fixed on Firefox Nightly 78.0a1 (2020-05-18) and Nightly 79.0a1 (2020-06-16) under Windows 10 (x64).