As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 752141 - Big pauses when playing sounds in BananaBread
: Big pauses when playing sounds in BananaBread
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: mozilla15
Assigned To: Chris Pearce (:cpearce)
:
: Maire Reavy [:mreavy] Please needinfo me
Mentors:
Depends on:
Blocks: gecko-games
  Show dependency treegraph
 
Reported: 2012-05-04 21:15 PDT by Alon Zakai (:azakai)
Modified: 2012-05-08 10:18 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch: don't hold media decoder monitor while finishing audio stream (5.53 KB, patch)
2012-05-06 16:57 PDT, Chris Pearce (:cpearce)
kinetik: review+
Details | Diff | Splinter Review
Patch with `diff -w` (3.05 KB, patch)
2012-05-06 16:58 PDT, Chris Pearce (:cpearce)
kinetik: feedback+
Details | Diff | Splinter Review

Description User image Alon Zakai (:azakai) 2012-05-04 21:15:58 PDT
http://www.syntensity.com/static/bb/client.html

The game has some gc/recompilation pauses, every few seconds the frame rate drops. But aside from that, when you click to fire the gun, there is a very noticeable pause - longer and worse than the gc pauses - right after the sound plays, pretty much every time.

These are *NOT* gc pauses, no gc'ing shows up in the logs. Also, a CPU monitor with high frequency shows that the CPU is inactive during these pauses, as if sleeping as it waits for a lock or something like that. Commenting out the line that does .play() on the audio element prevents the stalls, so it seems to be audio related.

(If you want to comment it out, save a local copy of the html file and also the same url with .data instead of .html, that's all it needs. The relevant play() happens in _Mix_PlayChannel.)
Comment 1 User image Alon Zakai (:azakai) 2012-05-04 21:21:10 PDT
Bill, this sounds similar to something we saw in the past on a (private) game-related demo, I seem to recall you profiled it and found audio is to blame, and when audio was disabled the demo got much faster? Do you remember any other details that might help here?
Comment 2 User image Bill McCloskey (:billm) 2012-05-04 22:27:59 PDT
Yeah, the problem was that we were spending a lot of time waiting for a lock in the media code. It was a Linux-only problem, and the media people didn't seem to have much interest in fixing it. I'm not sure there's much that can be done.
Comment 3 User image Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-04 22:35:12 PDT
What lock are blocking on that can be held for ~seconds?
Comment 4 User image Chris Pearce (:cpearce) 2012-05-05 01:02:12 PDT
(In reply to Bill McCloskey (:billm) from comment #2)
> Yeah, the problem was that we were spending a lot of time waiting for a lock
> in the media code.

What's the bug number for this?

If we're stuck waiting for several seconds on a lock, we'll want to fix that.

> It was a Linux-only problem, and the media people didn't
> seem to have much interest in fixing it. I'm not sure there's much that can
> be done.

Matthew Gregan has been in the process of writing our own sound library (libcubeb) which should alleviate much of our sound issues. It's been slow going, in part because sound on Linux is a pretty broken.

libcubeb has landed for Windows and Mac, but it's disabled on Aurora+ so we can test it more. Linux still needs work I understand, so hasn't landed.
Comment 5 User image Alon Zakai (:azakai) 2012-05-06 12:22:32 PDT
(In reply to Chris Pearce (:cpearce) from comment #4)
> (In reply to Bill McCloskey (:billm) from comment #2)
> > Yeah, the problem was that we were spending a lot of time waiting for a lock
> > in the media code.
> 
> What's the bug number for this?
> 

Other people know more, but I suspect since the demo where we saw the problem was non-public, we couldn't open a bug on it.

But anyhow, we do have a usable testcase now in this bug.
Comment 6 User image Chris Pearce (:cpearce) 2012-05-06 16:36:20 PDT
This is easy to fix. We're holding the decoder monitor when we write silence to the audio stream in order to ensure we've written more than the block size in AudioLoop(), and that's blocking event runners acquiring the monitor on the main thread.
Comment 7 User image Chris Pearce (:cpearce) 2012-05-06 16:57:22 PDT
Created attachment 621478 [details] [diff] [review]
Patch: don't hold media decoder monitor while finishing audio stream

In addition to holding the media decoder monitor while writing silence to fill up the last block in the stream, we're *not* holding the monitor while reading mState and mStopAudioThread; we should be.

Tests pass locally, I've pushed to Try:
https://tbpl.mozilla.org/?tree=Try&rev=6c8fb3cdfeb5

I'll upload a `diff -w` patch, so it's clearer what I've changed.
Comment 8 User image Chris Pearce (:cpearce) 2012-05-06 16:58:26 PDT
Created attachment 621479 [details] [diff] [review]
Patch with `diff -w`

Same as previous patch, but produced with `diff -w`, so it's clearer what's going on.
Comment 11 User image Alon Zakai (:azakai) 2012-05-08 10:18:59 PDT
Works great, thanks for the quick resolution!

Note You need to log in before you can comment on or make changes to this bug.