809558 - Web app audio does not stop playing when backgrounded (or taking a phone call)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P2)

Description

[Chris Peterson [:cpeterson]]

STR:
1. Install "Entanglement" game web app.
2. Load the game. (This can take a long time!)
3. Once the game's music is playing, switch to a different app.

RESULT:
The music continues playing. I also received a phone call while testing and the music continued to play during my phone call!

I'm using Nightly 19 on Galaxy Nexus JB.

Comment 1

[Chris Peterson [:cpeterson]]

https://marketplace.mozilla.org/app/entanglement/

Comment 2

[Aaron Train [:aaronmt]]

I thought browser.js used to suspend timeouts on the content window if it's not in the foreground?

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Chris - Does this work correctly when using Firefox and running in a tab?

Comment 4

[Chris Peterson [:cpeterson]]

When I run the Entanglement game [1] in a tab, the sound continues to play when I switch to a different tab.

[1] http://entanglement.gopherwoodstudios.com/

Comment 6

[Erin Lancaster [:elan]]

We want to stop playing audio when Fx is not active, let's use this bug to track that work. The webapi to allow for playing audio in the background is being tracked here: https://bugzilla.mozilla.org/show_bug.cgi?id=796333

Comment 7

[Erin Lancaster [:elan]]

Correction to the above: the b2g implementation is tracked above, we still need to file one to track the API implementation on Android. End stop.

Comment 8

[Wesley Johnston (:wesj)]

Attachment: Patch

This is a pretty simple implementation that basically just flips this on. It doesn't seem to support dynamic changes to the channel. Looking at b2g, I don't think they're supported there either? I removed the check in CheckAudioChannelPermissions to test, but put it back here.

Right now audio channels require permissions only allowed for certified apps. I think we should relax that somewhat at least. For instance, content (for music) and telephony (for webrtc) would be useful and relatively benign for most pages. By default audio is "normal" and gets the "Pause when I leave the page" behavior. 

The notification channels seem to have problems on Android. Ringer and alarm override default volume settings and I can see some sense in not allowing them.

Since this mutes when the user switches apps, it will essentially pause things on calls except it does not mute "music" when those channels are backgrounded. Android (on my S3 at least) automatically mutes (but doesn't pause) all other channels when something is playing through STREAM_VOICE_CALL, so I'm not sure we need to do anything special for that?

Comment 9

[Wesley Johnston (:wesj)]

Maybe Entanglement is playing audio through flash? That could explain the audio playing during a phone call as well?

Comment 10

[Wesley Johnston (:wesj)]

Whoops. Nope. Flash would show a click-to-play prompt that we don't see.

I also sent a message to the HTML Working Group and the WhatWG with a slightly simplified proposal for audiochannel. Comments from anyone are welcome.

Comment 11

[Wesley Johnston (:wesj)]

Attachment: Patch v2

Updated from the mass changes that just happened.

Comment 12

[Matthew Gregan [:kinetik]]

Comment on attachment 727386 [details] [diff] [review]
Patch v2

Review of attachment 727386 [details] [diff] [review]:
-----------------------------------------------------------------

Note that we only use AudioTrack on Android now for 2.2, all later versions should be using the OpenSL backend.  Adding support for stream_type to AudioTrack makes sense, though.  Thanks!  Can you please submit the cubeb changes upstream to http://github.com/kinetiknz/cubeb, then reintegrate them into our tree via media/libcubeb/update.sh?

Bug 852831 has a patch to always pass CUBEB_STREAM_TYPE_MUSIC for Android and only pass other values for B2G because we regressed the media volume rocker by accidentally switching to CUBEB_STREAM_TYPE_SYSTEM as the default.  If we're planning to pass the right values for Android, we won't need the patch in bug 852831.

The HTMLMediaElement changes are fine with me, but I'd like sicking or baku to cast an eye over this since they've been working on the web API side of this.

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 727386 [details] [diff] [review]
Patch v2

Review of attachment 727386 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/HTMLMediaElement.cpp
@@ +2156,5 @@
>  }
>  
>  bool HTMLMediaElement::CheckAudioChannelPermissions(const nsAString& aString)
>  {
> +#ifdef ANDROID

Don't we need this both on ANDROID and MOZ_B2G?

::: media/libcubeb/src/cubeb_audiotrack.c
@@ +145,5 @@
>    status_t status;
>    /* Recent Android have a getMinFrameCount method. On Froyo, we have to compute it by hand. */
>    if (audiotrack_version_is_froyo(ctx)) {
>      int samplerate, frame_count, latency, min_buffer_count;
> +    status = ctx->klass.get_output_frame_count(&frame_count, params->stream_type);

I don't know this code well enough that I can review this file.

Comment 14

[Matthew Gregan [:kinetik]]

Sorry, I should've been clearer (and probably asked for feedback? instead), but I just wanted to make sure people involved in the web API side of the audiochanneltype stuff were aware of/okay with this being enabled on Android too.

Comment 15

[Wesley Johnston (:wesj)]

> Note that we only use AudioTrack on Android now for 2.2, all later versions
> should be using the OpenSL backend.

Hmm.. I saw this: http://mxr.mozilla.org/mozilla-central/source/media/libcubeb/src/Makefile.in#29 and assumed we are using AudioTrack for all Android.

Like I mentioned earlier, I DO want to change the audio channel stuff so at least the main music channel is available to web content (with or without a prompt?), and possibly the telephony channel as well. I sent an email to the HTMl5 and the WhatWG working group proposing both but haven't heard much in the way of a response. If you have comments, happy to hear them.

Comment 16

[Wesley Johnston (:wesj)]

landed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/55a6fc3b5f42

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/55a6fc3b5f42

Comment 18

[Paul Adenot (:padenot)]

Wes, this needs to go on the github repo [1], so your changes don't get overwritten when using the update.sh script when updating the library.

Comment 19

[Wesley Johnston (:wesj)]

Whoops. Thanks. Does the central push need to be backed out?

Comment 20

[Paul Adenot (:padenot)]

Wes, no, just rebase the cubeb part of you patch on top of kinetik's cubeb repo master (I have merged a patch in the meantime, so it won't apply cleanly), and ask for a pull request to kinetik.

Comment 21

[Wesley Johnston (:wesj)]

pull request https://github.com/wesj/cubeb/commit/62632198752fee08693fea1c412a21e8d98e58b8

Comment 22

[Justin Lebar (not reading bugmail)]

Note that I'm switching this away from an ifdef to a pref in bug 844323.  I've enabled the pref in android/app/mobile.js, so we should be good here.