Open Bug 665000 Opened 13 years ago Updated 2 years ago

Playback of javascript mp3 decoder suffers in quality when tab loses focus

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect

Tracking

()

Tracking Flags:
Tracking Status
firefox5 - affected
firefox6 - affected
firefox7 - affected

People

(Reporter: lsblakk, Unassigned)

References

(
URL
)

Details

(Keywords: regression) 

STR:

1. Go to http://jsmad.org/ and click the play button
2. After music starts, go to another tab in the same window as that tab
3. Audio playback becomes choppy and returns to normal when you return focus to the tab with http://jsmad.org/ in it

Note that if you have it open in another *window* and shift focus to another *window* the audio remains consistent in its playback.

Expected results would be that switching tabs would not create choppiness in the audio playback.

Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0) Gecko/20100101 Firefox/5.0
I'm presuming this is due to the changes we made to deprioritizing background tabs.
Component: Video/Audio → DOM
QA Contact: video.audio → general
Sound like a bug in the js mp3 decoder, IMHO.

Is this perhaps evang bug?
I think this is the sort of thing that bug 615946 is trying to address.  It's a perfect use case, imho.
> I'm presuming this is due to the changes we made to deprioritizing background
> tabs.

Almost certainly because it uses a timeout or interval timer to drive things somehow... and that's now firing more rarely.  The audiolib script certainly seems to involve some timers that might be affected.

It looks like it's relying on 250ms timeouts or so: if I set the background tab clamp at 250ms it skips very rarely.

I can't test how Chrome behaves here, because this site doesn't work in Chrome at all (no sound).
Blocks: 633421
tracking-firefox5: --- → ?
tracking-firefox6: --- → ?
tracking-firefox7: --- → ?
Keywords: regression
Per discussion at http://news.ycombinator.com/item?id=2665607 Chrome does the same thing.  The developers of this mp3 decoder say that the problem is the limited buffer size of the library they're using, so they can't actually buffer up a full second's worth of audio at once.
Hi, lead jsmad dev here, answering a few questions :)

> Sound like a bug in the js mp3 decoder, IMHO.

No. The *decoder* has nothing to do with how playback is handled. If there's a bug in the playback, it's in player.js - which, I agree is not optimal anyway.

The demo works beautifully in Firefox 4.0.1, even when in a background tab. So as far as I'm concerned, this is a regression (I've encountered this bug on Firefox 6.0a2 at least).

This is where I wish Mozilla Devs in general cared a little more about multimedia in general. https://github.com/jussi-kalliokoski/audiolib.js/ has been around for some time, and it's what jsmad's demo is using.

So please, Olli, don't be so fast on the judgement guns.

> Almost certainly because it uses a timeout or interval timer to drive things somehow... and that's now firing more rarely.  The audiolib script certainly seems to involve some timers that might be affected.

That's my understanding as well. However, the 250ms timeout in player.js is *not* directly related to audio playback - it's here to update the graphical progress bar, and work around some audio bugs that sometimes stop the refill function from being called *at all*. In that case I just reinitialize the device and refill is being called again.

> I can't test how Chrome behaves here, because this site doesn't work in Chrome at all (no sound).

Although I haven't personally got it to work under any version of Chrome, we have had reports that it works on Chrome 13.0 (Canary channel) and above if you go in 'about:flags', enable Web Audio API, restart, and try the demo again.

> The developers of this mp3 decoder say that the problem is the limited buffer size of the library they're using, so they can't actually buffer up a full second's worth of audio at once.

Indeed, if we could buffer 1s of audio at once the background tab bug in Firefox would probably not affect us anymore. But then the "pause" button would suffer some serious delay...

> I think this is the sort of thing that bug 615946 is trying to address.  It's a perfect use case, imho.

Audio workers certainly look really interesting - please don't tie them to the HTML5 audio tag, though, that would really suck.

I'm looking forward to a time where we can just get rid of all those binary dependences because the JavaScript language and its implementations have evolved enough that the performance penalty is irrelevant.

Cheers!
A proposed workaround, from http://www.opennet.ru/opennews/art.shtml?num=30921 comments, is to set 'dom.min_background_timeout_value' back to a low value such as 10, in about:config - then the demo works perfectly.

Isn't there a way to *not* clamp audio callbacks? That would be enough for jsmad's usecase.
Jussi, as a temporary workaround, could the API use the postMessage()
hack I mentioned yesterday.

The new Audio APIs should be designed so that timers aren't needed.
> Isn't there a way to *not* clamp audio callbacks?

We don't know they're "audio callbacks".  All we know is that the page is trying to run stuff off a timeout; in 99.99% of cases on the web right now doing that at the 4ms clamp is a waste of CPU power (and hence battery).... which is why the background tab clamp for timers is higher, just as it is in Chrome.

For a setInterval I suppose we could try to detect whether it touches audio APIs when it runs and if so not clamp it to the background clamp, but for nested setTimeout that's not really feasible either (because there is no good correlation between the setTimeout call and the code that runs when the timer fires).
Ok, I've made an optional workaround to audiolib.js , using a home-baked postMessage-based setInterval replacement.

I don't know if it makes sense to detect whether it touches the Audio API, it would make more sense to have some way of creating a timer that works in the background as well, such as

setTimeout.createFixed(callback, timeout);

Then on the other hand, if you know what you're doing, you can make it work anyway, with the postMessage, but that's a bit intruding, so if you need that functionality, you can't make it a default in a library, for instance.
> Then on the other hand, if you know what you're doing, you can make it work
> anyway, with the postMessage, but that's a bit intruding, so if you need
> that functionality, you can't make it a default in a library, for instance.

Jussi, you should try having it use the APIs and builds that Yury made here: http://async5.org/audiodata/workerAudio/play.html.
By the way, Jussi added the postMessage workaround, and I've enabled it on jsmad.org - it works, but the UI is less responsive and it nows sometimes blocks for a split second whereas it did not do that before (with setTimeout)
David: That's otherwise a great solution, but it would leave Fx4 and and Chrome out of the picture. That said, kudos to Yury and the rest of you for all the great work you've been doing with that, I've been watching the progress with enthusiasm.

I have to confirm Amos' statement, using postMessage instead of setTimeout in this case is very intrusive and not so nice performance-wise, in this case, that's why I decided from the start to disable the behaviour in audiolib.js by default.

Is there a way to feature detect audio workers?
As Jussi said, Audio Workers look really neat, but we have to worry about cross-browser here, and browser detection is not a viable solution.
All the methods are browser-dependent at the moment.
(In reply to comment #15)
> All the methods are browser-dependent at the moment.

Yes, obviously, but feature detection is much more desirable than browser detection. That way, if other vendors implement it, client code works without any modification. That was my point :)
Jussi, what about using postMessage + workers as a hack.
http://mozilla.pettay.fi/moztests/audio/audio.html seems to work in 
background tabs.
It uses a trivial worker
http://mozilla.pettay.fi/moztests/audio/audioworker.js
That was a very quick hack and the example itself (copied from
https://wiki.mozilla.org/Audio_Data_API#Complete_Example:_Creating_a_Web_Based_Tone_Generator)
doesn't seem to handle errors in all the cases.
Olli, I implemented the inline worker (with blob urls) that we discussed about into the hack in audiolib.js, where BlobBuilder is available, and that's working nice, otherwise it will fall back to the postMessage loveliness. But I think the options are still not so nice:

1. Use postMessage for Fx4 and Fx5, and Fx6+ will have the worker hack. The problem is that Fx4 doesn't need the background work hack to be enabled, and actually would be better without it.

2. Browser detect (I wish there was some way to feature detect whether you can trust the timeouts to do what you tell them to), and disable the hack for Fx4.
This is 5-specific since we added the clamping in FF5, but we're not going to track this for tomorrow's release.
tracking-firefox5: ? → -
We'll be tracking this for 5 and 6 since we may learn from 5 that we should adjust how aggressively we clamp timeouts in background tabs and adjust in 6 based on what we learn.
status-firefox5: --- → affected
status-firefox6: --- → affected
tracking-firefox5: - → +
tracking-firefox6: ? → +
no longer going to track this instance of the problem.
status-firefox7: --- → affected
tracking-firefox5: + → -
tracking-firefox6: + → -
tracking-firefox7: ? → -
What we need is a DOM event to be fired for when this clamping to 1000 ms happens, so that the web app can pause itself. Also another DOM event to un-pause itself for when the 1000 ms throttle removes itself. Affects my js emulator stuff too (and I use a single 16 ms setInterval to drive everything).
> What we need is a DOM event to be fired for when this clamping to 1000 ms happens, so
> that the web app can pause itself. Also another DOM event to un-pause itself for when
> the 1000 ms throttle removes itself.

http://dvcs.w3.org/hg/webperf/raw-file/tip/specs/PageVisibility/Overview.html#sec-visibilitychange-event
Yup, Jussi and I should use that then to pause the core of the web apps.
Well then... if we assume that realtime audio applications will pause themselves when they're in the background, then we lose an edge to Flash (ie. I pretty much always have the Grooveshark player as a background tab as I do other stuff.) I know the answer will be "you need to use the HTML5 <audio> tag to play music tracks anyway" but this approach has shortcomings as well..
bz: Is document.visibilityState implemented yet?
https://github.com/grantgalitz/GameBoy-Online/commit/c4d0a132f98f1a2cf4b3cd0e40718fdb589beed1 has landed into the GBC emulator, though I don't see the API linked working (Seems to return null).
> bz: Is document.visibilityState implemented yet?

Yes, as you could trivially test...
Though note that all that stuff uses moz prefixes so far.
Oh, it's only in Firefox 10 and up it seems (Was using Firefox 9 earlier). Yeah, also implemented the vendor-specific checks too:

(!document.hidden && !document.msHidden && !document.mozHidden && !document.webkitHidden)
Is there any way to do this for Firefox versions 5 through 9? This API seems to be absent in anything before Firefox 10.
> Is there any way to do this for Firefox versions 5 through 9?

Not really.
This is a non-issue in sink.js in Firefox (excluding Firefox 5 because of this bug, and Firefox 8 because of https://bugzilla.mozilla.org/show_bug.cgi?id=699633 ). It can be worked around using inline workers that manage the timers instead, this allows for accurate timers in background tabs. Working around it by switching off the sounds/music is a bad idea, imho. Music especially is hardly ever something you have as the main focus when on a computer. For Firefox 5 and 8, sink.js just doesn't work properly in background tabs, though. :/
Since my bug 717413 is marked as dupe, below is copy of its description to prevent the info (including provided possible solutions) provided in that bug from getting lost.

-----

Music cannot be listened in inactive tab when custom pure-JavaScript audio format decoder is used. Annoying.

Live examples of pages where the issue reproduces:

A. http://codecs.ofmlabs.org/ Pure-JavaScript MP3/ALAC decoders;
B. http://www.abyss-online.de/disissid4/ Chip-tune JavaScript player.

Example A reproduce the bug in Firefox 10b3 and does not in latest nightly 20120111031049, but example B reproduces the bug in _both_ Firefox 10b3 and latest nightly 20120111031049.

This bug makes it literally _impossible_ to listen custom-format music since music in the web is almost _always_ is listened _exactly_ in inactive (background) tab while viewing another sites in another tabs.

And while with MP3 case music can be just reencoded to Ogg Vorbis, it's _absolutely impossible_ to workaround with synthesizing sound from non-wave-like format _on-the-fly_.

The issue is in place even if tab is pinned (via "Pin as App Tab" tab context-menu item).

I currently see following probable solutions for this issue:

1. (probably most proper) disable automatic CPU-saving measure for inactive tab (such CPU saving is implemented in bug 633421) if specific script running in such tab involves continuous _generating sound_;

2. (relatively acceptable) disable automatic CPU-saving measure for _pinned_ inactive tabs;

3. (relatively acceptable) add new checkbox item like "Disable CPU saving for this tab" to tab context-menu. Once the checkbox is turned on, tab works identically for both active and inactive states.

Google Chrome has no such issue and plays generated sound smoothly in inactive tabs for both examples provided above. Flash-based players and synths (like that on http://www.photonstorm.com/flod page) work properly in inactive tabs too.

So this issue unfortunately makes Firefox less usable/competitive product.

Thanks.
> Google Chrome has no such issue and plays generated sound smoothly in inactive tabs

That's quite odd, since it's more aggressive than we are in terms of what it turns off in background tabs.  Are those testcases running the same code in Firefox and Chrome?
Actually, I just tried running the ALAC example at http://codecs.ofmlabs.org/ in a background tab in my Firefox nightly, and it works just fine.  Which explains why it works fine in Chrome: the code there deals with being in a background tab ok.

I'm going to reopen bug 717413 since it's about a different problem than this bug: this bug was about code that _doesn't_ handle the background tab clamping.
http://codecs.ofmlabs.org/ works just fine in background in Firefox (excluding 8 [because of https://bugzilla.mozilla.org/show_bug.cgi?id=699633] and 5 [because of this bug]), because it's using the latest version of sink.js

@Marat A really cool demo you've got there, if you want to make it use sink.js (which has a function called doInterval for timers that work in background tabs too, and protection against several other bugs in the browser audio minefield as well), and need help doing so, drop me a line on GitHub, my handle is jussi-kalliokoski. Or you can come to #ofmlabs on freenode IRC. It should be pretty straightforward though.
Oh, wait.  You said that the ofmlabs code works on nightly.  OK, so no problem there; I can confirm that.

The testcase at http://www.abyss-online.de/disissid4/ browser-sniffs and runs different code in Chrome and Firefox: it uses the webkitAudioContext stuff in webkit, but uses setInterval in Gecko.  All that site needs is for us to finish up the new audio API stuff, which is specifically designed to allow audio to play without dropping out (e.g. not even running on the main browser thread) and then to use the new APIs.
Oh, and even while using setInterval the disissid4 code would work fine if it used a longer buffer, of course.
It makes sense to add that Audio Workers seems to be not a too good idea since:

1. inventing a new setInterval/setTimeout alternative for each new usecase is propectless dead-end road;

2. necessity to create separate _file_ just to attach some JS _code_ to audio is annoying and harmful since it inevitably forces to make another request to server thus slowing down page-loading.

Also, it likely cannot be impossible to determine that tab is generating audio data. Human can hear that sound plays, computer's audio mixer shows peaking levels, so nothing prevents browser to determine this as well.

postMessage() workaround looks like not too good idea sinde that is based on specifics of current implementation of browser and probably can get same clamping problem in future.

So, it likely makes sense to first consider disabling automatic CPU-saving measure for inactive tab if specific script running in such tab involves continuous generating sound.

Thanks.
(In reply to Jussi Kalliokoski from comment #39)

> @Marat A really cool demo you've got there, if you want to make it use
> sink.js (which has a function called doInterval for timers that work in
> background tabs too, and protection against several other bugs in the
> browser audio minefield as well), and need help doing so, drop me a line on
> GitHub, my handle is jussi-kalliokoski. Or you can come to #ofmlabs on
> freenode IRC. It should be pretty straightforward though.

I am not author of the example, so I'm just a user (not web-developer) in this case.
(In reply to Boris Zbarsky (:bz) from comment #41)

> Oh, and even while using setInterval the disissid4 code would work fine if
> it used a longer buffer, of course.

As it mentioned by Amos above in comment 6, using longer buffer may cause issues with pausing.
Yes, long buffering doesn't work except in rare cases, because the current Audio Data API doesn't handle the tail, so you may not be (and probably aren't) able to buffer a whole second at a time.
(In reply to Marat Tanalin | tanalin.com from comment #42)
> 2. necessity to create separate _file_ just to attach some JS _code_ to
> audio is annoying and harmful since it inevitably forces to make another
> request to server thus slowing down page-loading.

It's actually possible to work around this by copying script content into a Blob and using that Blob as the source for the worker.
(In reply to comment #46)

Inventing a feature that knowingly have to be worked around right after being implemented looks like wrong way.

This is in addition to being wrong way according to argument #1 (in comment 42) that is fundamentally more important. It's browser's work to determine if audio is generated/played and to _transparently_ tweak timeout interval for specific tab without any additional efforts on web-developer side.
The problem here is that the web developer isn't given the option to initially override the throttles. There should be an extension of sort to disable the throttles and to further configure the aspects of the timer. Working around a feature that's also a bug in some cases is so ugly. Why not just extend the timer APIs instead?
Possibly in the near future there could be a clean break from setInterval/setTimeout  (while keeping these older APIs around) that are much more configurable. Think event queue priority, type of timer accuracy algorithm to use, etc.
(In reply to comment #48)
Web developer does not need and should not be forced to make extra efforts here. Since there is generally no sense in playing more than one sound source (coming from different tabs) simultaneously (user will appearently not have more than one sound-generating tab opened at once), it's enough to just automatically disable timer clamping for tab that generates sound.

Clamping is browser's feature; it's potentially good, but if it causes problems for user experience, then implementation of clamping (not of web-app) should be improved.

The way to misfortune is paved with good intentions. Good intention is clamping here. What matters is implementation.
(In reply to Grant Galitz from comment #49)
> Possibly in the near future there could be a clean break from
> setInterval/setTimeout  (while keeping these older APIs around) that are
> much more configurable. Think event queue priority, type of timer accuracy
> algorithm to use, etc.

If there will be new setTimeout-like feature that will allow web-developer to force disabling timer clamping, then nothing will prevent web-developers from disable clamping _always_. This would lead browser vendors to invent a new way to clamp (with same good intention to save CPU power), and the story will repeat. Dead-end road.
(In reply to Marat Tanalin | tanalin.com from comment #50)
> here. Since there is generally no sense in playing more than one sound
> source (coming from different tabs) simultaneously (user will appearently
> not have more than one sound-generating tab opened at once), it's enough to
> just automatically disable timer clamping for tab that generates sound.

And right there you've killed all unborn webDJs =(
(In reply to alex_mayorga from comment #52)
Either you've missed "coming from _different_ tabs" part, or you just don't understand the subject adequately.
See Also: → 666219
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.