1617033 - Stop listening to media control keys for some media element to prevent intercepting key from other background music apps

Status: RESOLVED FIXED

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

Description

[Alastor Wu [:alwu]]

So basically this is aim to fix the issue described in bug1615665 comment0 (which I would leave it open in order to discuss UX behavior there)

As we would create media controller for a silent video as well, so playing a silent video can claim the audio focus on platform which allows us to use media keys to control those silent video.

However, we only destroy media controller when closing that tab, which says it one tab is constantly playing silent video, then the controllability of media keys would be always hijacking by that tab.

So I would like to do in this bug are,

(1) Not to listen media control keys for video without audio track
Those kind of video are ususally being used as a background video or GIF-like video, so it's not worth to control them.

(2) Start to listen to media control keys after media become audible
If media start inaudibly, such as setting its volume to zero, being muted, audio track only containing silence or the whole tab is being muted by sound indicator, then we probaly won't want to control them until they become audible.

(3) Stop to listen to media control keys after media has been paused for a while
If media has been paused long enough, we could probably think it's no need to be controlled any more. So we should stop listening to the event and let other apps to handle keys properly.

Comment 1

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part1 : start listening to the event after media becomes audible.

Comment 2

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part3 : add comment to explicitly mention some functions should only be used after starting listener.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part4 : add a timer to stop listening to media control key events.

Comment 4

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part7 : modify tests to allow simulating media keys would always happen after controller's playback changes.

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part9 : add test.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part2 : only update audible state after starting listener successfully.

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part5 : remove unecessary async.

Comment 8

[Alastor Wu [:alwu]]

Now I'm still dealing with the test failure on try server, which I couldn't reproduce locally. Will ask for a review after fixing this issue.

Comment 9

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part6 : make utility functions more general.

By providing element Id, we can use these functions to access video element on other files.

Comment 10

[Alastor Wu [:alwu]]

Attachment: Bug 1617033 - part8 : add an eligible media duration value to filter out notification sound.

Intercepting media control keys would also come up with the virtual control interface in most of platforms, and for the notification sound we don't want either to show the interface or control them.

Currently we use 3s as a threshold to filter those short duration media which are possible to be a notification sound.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6acd67af0e32
part1 : start listening to the event after media becomes audible. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/a33e9c0dcea1
part2 : only update audible state after starting listener successfully. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/10d38000b11c
part3 : add comment to explicitly mention some functions should only be used after starting listener. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/5f1478173ec6
part4 : add a timer to stop listening to media control key events. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/b92e7ec309ac
part5 : remove unecessary async. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/6d749b36ef6c
part6 : make utility functions more general. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/02a74ae77f0a
part7 : modify tests to allow simulating media keys would always happen after controller's playback changes. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/83c4c8f23b2d
part8 : add an eligible media duration value to filter out notification sound. r=chunmin
https://hg.mozilla.org/integration/autoland/rev/11c9465d2b02
part9 : add test. r=chunmin

Comment 12

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/6acd67af0e32
https://hg.mozilla.org/mozilla-central/rev/a33e9c0dcea1
https://hg.mozilla.org/mozilla-central/rev/10d38000b11c
https://hg.mozilla.org/mozilla-central/rev/5f1478173ec6
https://hg.mozilla.org/mozilla-central/rev/b92e7ec309ac
https://hg.mozilla.org/mozilla-central/rev/6d749b36ef6c
https://hg.mozilla.org/mozilla-central/rev/02a74ae77f0a
https://hg.mozilla.org/mozilla-central/rev/83c4c8f23b2d
https://hg.mozilla.org/mozilla-central/rev/11c9465d2b02

Comment 13

[Florin Strugariu [:Bebe]]

== Change summary for alert #25259 (as of Fri, 06 Mar 2020 20:55:50 GMT) ==

Improvements:

26% build times windows2012-64 debug taskcluster-c5.4xlarge 2,321.62 -> 1,710.08

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=25259