1351087 - Firefox requests internal audio focus for no-audio media (e.g. GIFV)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect)

Description

[Jan Henning [:JanH]]

+++ This bug was initially created as a clone of Bug #1347648 +++

Bug 1347648 fixed this with regards to external apps, but at the moment playing no-audio media will still interrupt audible background playback coming from another tab *within* Firefox.

Comment 1

[Alastor Wu [:alwu]]

It's intend to do, because we would also show the media control for non-audible media. Media control is only used for one specific tab, so we don't allow play multiple tabs at the same time.

Comment 2

[Jan Henning [:JanH]]

Yes, that makes things more convoluted, but the behaviour of the media control is certainly not set in stone for all times going forward and I don't think we should let us get painted into a corner that easily.

Furthermore,
- as their name implies, GIFV (and GIFV-like) videos are intended as a higher quality/smaller file size replacement for animated images. We don't interrupt background audio playback for animated images, so why should we do this now just because an implementation detail (using a video instead of an image file format) changed?
- in bug 1347648, this was already fixed with regards to external apps. Having a different behaviour for audio playback within Firefox is just inconsistent UX. Why should we care whether the user is using another tab or completely different app for background playback?
- Chrome doesn't do this, either. Only one tab at a time can play sound, but a silent video doesn't interrupt background playback on other tabs.

Comment 3

[Alastor Wu [:alwu]]

Ok, after more investigate, I could do not to request the audio focus for GIFV within the firefox, but it's not mean non-audible video won't interrupt other tabs, because we have different kinds of non-audible media.
However, it would have some side effect, I mention that in the last paragraph.

There are different kinds of non-audible media,
(1) Media without audio track (GIFV-like) [1]
(2) Media with silent audio track [1]
(3) Audible Media, but get muted or changed volume to ZERO

For these kinds of media, that's how audio focus should work,

Audio focus between apps (Android audio focus)
- We won't request audio focus for all these media.

Audio focus between tabs (Firefox internal audio focus)
- We would only request audio focus for (2) and (3), not for (1).

But now, we treat (1) as same as (2) and (3) so that cause this bug.

[1] https://people-mozilla.org/~alwu/Non-audible%20files/

---

> - Chrome doesn't do this, either. Only one tab at a time can play sound, but
> a silent video doesn't interrupt background playback on other tabs.

Actually, non-audible media would also affect other background playback on Chrome, if you're playing (2) or (3). Since your case is playing (1), so the background tab won't be affect.

---

But if I implement that, that means we can allow multiple playing tab, but one of them won't have a media control. And that might cause potential mobile data problem, because Firefox allow background video playing, but Chrome doesn't.

Comment 4

[Alastor Wu [:alwu]]

> But if I implement that, that means we can allow multiple playing tab, but
> one of them won't have a media control. And that might cause potential
> mobile data problem, because Firefox allow background video playing, but
> Chrome doesn't.

It would also have power consumption concern.

Comment 5

[Alastor Wu [:alwu]]

(In reply to Alastor Wu [:alwu] from comment #4)
> > But if I implement that, that means we can allow multiple playing tab, but
> > one of them won't have a media control. And that might cause potential
> > mobile data problem, because Firefox allow background video playing, but
> > Chrome doesn't.
> 
> It would also have power consumption concern.

But we have zombify mechanism on Fennec, it might be helpful on this issue. This mechanism would clean up the background GIFV-like video after a period.

---

Hi, Blake,
Do you have any suggestion for this issue?
I would like to allow that the GIV-like video can be play with other background playing tab and we won't show the media control for it.
The zombify mechanism might be slightly help us to avoid mobile data and power consumption problem. 
Thanks!

Comment 6

[Jan Henning [:JanH]]

(In reply to Alastor Wu [:alwu] from comment #4)
> It would also have power consumption concern.

Zombification could work as a last resort, but it seems a bit heavy-handed just for stopping media playback given that our audio competition mechanism can already start and stop playback in other tabs without having to resort to zombification. Can't we simply stop non-audible playground directly for no-audio media?

That is across Firefox, only one audible [1] media stream/file/... may play at the same time (even in background) just as today, but the *foreground* tab may additionally start playback of any kind of non-audible media without interrupting the audible background playback. When a different tab gets selected and that tab looses its foreground status, then all non-audible playback in the former foreground tab is interrupted/paused, just as happens today through audio competition/focus mechanism or the user stopping via the media control.

As for the display of the media control, off-hand I can think of
[A] Audible media always takes precedence, that is the media control is shown for non-audio content only if nothing audible is playing in the background.
[B] The foreground tab always takes precedence if playback is started - if playback is stopped again, the media control might want to switch back to controlling the background playback.
[C] The most recently started media takes precedence. Depending on the actual implementation, this might effectively be the same as [B] if we *don't* allow background playback of non-audible media, as proposed above.
[D] Maybe somebody has an even better idea?


[1] Wherever we draw the line between audible and non-audible among your categories (1), (2) and (3). I need to check its behaviour again, but personally, I'd probably say we should use the same logic as the audio playing icon that's displayed in the tabs list.

Comment 7

[Alastor Wu [:alwu]]

(In reply to Jan Henning [:JanH] from comment #6)
> (In reply to Alastor Wu [:alwu] from comment #4)
> > It would also have power consumption concern.
> 
> Zombification could work as a last resort, but it seems a bit heavy-handed
> just for stopping media playback given that our audio competition mechanism
> can already start and stop playback in other tabs without having to resort
> to zombification. Can't we simply stop non-audible playground directly for
> no-audio media?

Shutdown decoder can achieve that without zombification, but I don't know when would it land on Fennec, see bug1293963 for more details.

> That is across Firefox, only one audible [1] media stream/file/... may play
> at the same time (even in background) just as today, but the *foreground*
> tab may additionally start playback of any kind of non-audible media without
> interrupting the audible background playback. When a different tab gets
> selected and that tab looses its foreground status, then all non-audible
> playback in the former foreground tab is interrupted/paused, just as happens
> today through audio competition/focus mechanism or the user stopping via the
> media control.

Now I don't want to introduce the completed audio competition mechanism (including resume) in gecko, because no other browser support that now. In addition, the related spec is just a draft.[1]

[1] https://wicg.github.io/mediasession/

> As for the display of the media control, off-hand I can think of
> [A] Audible media always takes precedence, that is the media control is
> shown for non-audio content only if nothing audible is playing in the
> background.
> [B] The foreground tab always takes precedence if playback is started - if
> playback is stopped again, the media control might want to switch back to
> controlling the background playback.
> [C] The most recently started media takes precedence. Depending on the
> actual implementation, this might effectively be the same as [B] if we
> *don't* allow background playback of non-audible media, as proposed above.
> [D] Maybe somebody has an even better idea?

Before discussing that, I think the most important thing is to have completely UX spec first.

> [1] Wherever we draw the line between audible and non-audible among your
> categories (1), (2) and (3). I need to check its behaviour again, but
> personally, I'd probably say we should use the same logic as the audio
> playing icon that's displayed in the tabs list.

The audio playing icon should be totally different with audio focus and media control. You can have media control to control non-audible tab.

Chrome do the same behavior like I said in comment3, when you try to play (2) and (3), it would interrupt the background playback.

Comment 8

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

Anthony, 
What do you think?

Comment 9

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

Personally, IMO we should not request audio focus on (1) in comment 3 and for (2)(3) keep the current design.

Comment 10

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Blake Wu [:bwu][:blakewu] from comment #8)
> Anthony, 
> What do you think?

Sorry about the delay, just got back from PTO.

There seems to be a lot of edge cases here and technicality that I'm having trouble following along with. 

What's the problem we're trying to solve? and what is the current behavior for users that we need to "fix"?

Comment 11

[Kevin Brosnan [Ex-Mozilla]]

Firefox steals focus from itself. So you have a YouTube video playing and you visit a GIFV or other video only media will lower the volume of the YouTube tab.

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Blake Wu [:bwu][:blakewu] from comment #9)
> Personally, IMO we should not request audio focus on (1) in comment 3 and
> for (2)(3) keep the current design.

I agree, this seems easy and in line with what I believe users would expect.

Comment 13

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Makes sense to me!

Comment 14

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

Great to see we reach a consensus!

Comment 15

[Alastor Wu [:alwu]]

Attachment: Bug 1351087 - part1 : don't register agent for audio without audio track.

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

Comment 16

[Alastor Wu [:alwu]]

I got the WebRTC test case fail, but I can't reproduce it on my local environment, it only happened on try-server.
Now I'm still trying to figure out what's happen...

Comment 17

[Alastor Wu [:alwu]]

writing test...

Comment 18

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=137625a038b89fef9e8e620f8f08d290c05c3640

Comment 19

[Alastor Wu [:alwu]]

Attachment: Bug 1351087 - part2 : add tests.

Add test for handling the cases, media with silent audio track and media without audio track.

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

Comment 20

[Alastor Wu [:alwu]]

Comment on attachment 8862371 [details]
Bug 1351087 - part1 : don't register agent for audio without audio track.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134320/diff/1-2/

Comment 21

[Alastor Wu [:alwu]]

Comment on attachment 8862371 [details]
Bug 1351087 - part1 : don't register agent for audio without audio track.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134320/diff/2-3/

Comment 22

[Alastor Wu [:alwu]]

Comment on attachment 8865348 [details]
Bug 1351087 - part2 : add tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136982/diff/1-2/

Comment 23

[Andrea Marchesini [:baku]]

Comment on attachment 8862371 [details]
Bug 1351087 - part1 : don't register agent for audio without audio track.

https://reviewboard.mozilla.org/r/134320/#review140444

Comment 24

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8865348 [details]
Bug 1351087 - part2 : add tests.

https://reviewboard.mozilla.org/r/136982/#review140608

Comment 25

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ede4c518a6f2269cc751657d9f79d4f05080bebc

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/83623d23255e
part1 : don't register agent for audio without audio track. r=baku
https://hg.mozilla.org/integration/autoland/rev/d661115e6032
part2 : add tests. r=sebastian

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/83623d23255e
https://hg.mozilla.org/mozilla-central/rev/d661115e6032

Comment 28

[Nathan Froyd [:froydnj]]

Please request uplift on this when you get a chance.

Comment 29

[Alastor Wu [:alwu]]

Comment on attachment 8862371 [details]
Bug 1351087 - part1 : don't register agent for audio without audio track.

Approval Request Comment
[Feature/Bug causing the regression]: Introduce Android audio focus from bug1249579.
[User impact if declined]: When user starts watching GIF-like video, it would pause background music playback.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No
[Why is the change risky/not risky?]: Only change the behavior for GIF-like video
[String changes made/needed]: No

Comment 30

[Gerry Chang [:gchang]]

Hi Ioana & Mihai,
Can you help check if the issue is fixed in the latest nightly?

Comment 31

[Gerry Chang [:gchang]]

Comment on attachment 8862371 [details]
Bug 1351087 - part1 : don't register agent for audio without audio track.

Fix an issue that requests of internal audio focus from firefox for no-audio media. Beta54+. Should be in 54 beta 10.

Comment 32

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/659ecca63ea6
https://hg.mozilla.org/releases/mozilla-beta/rev/2adc92ca8808

Comment 33

[Carsten Book [:Tomcat]]

note i had to backout the test patch for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=100874795&repo=mozilla-beta

Comment 34

[Alastor Wu [:alwu]]

Should only uplift the first patch, the second one is testing and it doesn't need to be uplift.

Comment 35

[Sorina Florean [:sflorean]]

Verified as fixed in build: Nightly (55.0a1/2017-05-19) and 54 Beta 10. Media control is not displayed for (1) comment 3. 
Device: Asus ZenPad 8(Android 6.0.1) and Huawei Honor (Android 5.1.1).