Closed Bug 809558 Opened 12 years ago Closed 11 years ago

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

Categories

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

ARM
Android
defect

Tracking

(firefox19 affected, firefox23 verified)

VERIFIED FIXED
Firefox 23
Tracking Status
firefox19 --- affected
firefox23 --- verified

People

(Reporter: cpeterson, Assigned: wesj)

References

()

Details

(Whiteboard: A4A)

Attachments

(1 file, 1 obsolete file)

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.
Priority: P2 → --
I thought browser.js used to suspend timeouts on the content window if it's not in the foreground?
Chris - Does this work correctly when using Firefox and running in a tab?
Flags: needinfo?(cpeterson)
Priority: -- → P2
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/
Flags: needinfo?(cpeterson)
Whiteboard: A4A?
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
Whiteboard: A4A? → A4A
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.
Attached patch Patch (obsolete) — Splinter Review
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?
Attachment #725070 - Flags: review?
Maybe Entanglement is playing audio through flash? That could explain the audio playing during a phone call as well?
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.
Attached patch Patch v2Splinter Review
Updated from the mass changes that just happened.
Attachment #725070 - Attachment is obsolete: true
Attachment #725070 - Flags: review?
Attachment #727386 - Flags: review?(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.
Attachment #727386 - Flags: review?(kinetik)
Attachment #727386 - Flags: review?(jonas)
Attachment #727386 - Flags: review?(amarchesini)
Attachment #727386 - Flags: review+
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.
Attachment #727386 - Flags: review?(jonas)
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.
> 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.
Attachment #727386 - Flags: review?(amarchesini)
https://hg.mozilla.org/mozilla-central/rev/55a6fc3b5f42
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
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.
Flags: needinfo?(wjohnston)
Whoops. Thanks. Does the central push need to be backed out?
Flags: needinfo?(wjohnston)
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.
Status: RESOLVED → VERIFIED
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.
Depends on: 888836
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.