Last Comment Bug 779914 - Using the Audio Data API on otoro slow down the device
: Using the Audio Data API on otoro slow down the device
Status: RESOLVED FIXED
: perf
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla17
Assigned To: Chris Pearce (:cpearce)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-02 10:25 PDT by Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)
Modified: 2012-08-21 06:27 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Patch: reset mAudioStream in nsHTMLMediaElement::AbortExistingLoads() (555 bytes, patch)
2012-08-20 20:51 PDT, Chris Pearce (:cpearce)
cajbir.bugzilla: review+
Details | Diff | Splinter Review
Patch 2: Remove redundant mAudioStream reset (1.03 KB, patch)
2012-08-20 22:02 PDT, Chris Pearce (:cpearce)
kinetik: review+
Details | Diff | Splinter Review

Description Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-08-02 10:25:54 PDT
Steps to reproduce:
 - Have an otoro device with Gaia on it
 - pan the homescreen and see how fast it is
 - launch the dialer application and hit one of the number key, it should play a sound with the Audio API.
 - Hit the home key to go back to the homescreen 
 - pan again

Actual result:
 - The homescreen is really slow down

Expected result:
 - The homescreen speed has not changed
Comment 1 Chris Pearce (:cpearce) 2012-08-02 12:10:52 PDT
Just to be clear, you're referring to the HTML5 <audio> API, not the Audio Data API which enables you to play raw samples?
Comment 2 cajbir (:cajbir) 2012-08-02 12:20:55 PDT
The Gaia dialer app uses the Audio Data API (mozWriteAudio) to generate the tones.
Comment 3 Chris Pearce (:cpearce) 2012-08-02 14:58:10 PDT
Presumably the Gaia developer's chose to use the Audio Data API rather than the standard <audio> elements because <audio> has too high latency?

Can we use the new media streams API instead to work around this issue?
Comment 4 Matthew Gregan [:kinetik] 2012-08-02 15:50:41 PDT
Does this disappear after some time (i.e., when a GC happens)?  How about if you force the audio stream to close by doing an null load (audioelement.src = null) when navigating away from the dialer?
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-08-02 16:44:28 PDT
Yeah, once the tone has finished we should do something to ensure callbacks are no longer being fired.
Comment 6 Andrew Overholt [:overholt] 2012-08-20 10:59:40 PDT
Chris, who should own this?  What needs to do something, the web page or Gecko?
Comment 7 Chris Pearce (:cpearce) 2012-08-20 15:21:21 PDT
I can own this for now. I'm not sure if we should work around this in Gaia for the time being, or fix it in Gecko, but I can either of those things.
Comment 8 cajbir (:cajbir) 2012-08-20 16:56:13 PDT
Andrew, given the million Chris' that work for Mozilla, two of which actively commenting in this bug, can you be more specific about which Chris you're asking for a response from?
Comment 9 Andrew Overholt [:overholt] 2012-08-20 17:26:34 PDT
(In reply to Chris Double (:doublec) from comment #8)
> Andrew, given the million Chris' that work for Mozilla, two of which
> actively commenting in this bug, can you be more specific about which Chris
> you're asking for a response from?

Will do.  In this case I assigned it to Chris Pearce at the same time so I thought it was clear.
Comment 10 Chris Pearce (:cpearce) 2012-08-20 20:38:40 PDT
Since we're planning to implement the Web Audio API and move away from the Audio Data API I don't think it's worth us spending time fixing the Audio Data API; I think we should just work around this issue.

If we shutdown the audio stream when the dialer app goes into the background, and recreate it if/when the dialer app is brought back to the forefront, the problem goes away.

So I propose we make the following changes to work around this problem:
* Change nsHTMLMediaElement::AbortExistingLoads() to shutdown and reset mAudioStream if it's non-null. Then setting HTMLMediaElement.src=""; will cause the Audio Data API to be reset.
* Change the Gaia dialer app to reset/recreate the audio stream (_audio.src=null;delete _audio;) in a mozvisibilitychange event handler, and recreate it in the same handler if !document.mozHidden.

Something like:


I'll prepare a patch an a pull request to make these changes.
Comment 11 Chris Pearce (:cpearce) 2012-08-20 20:40:52 PDT
Bah, there was a failure somewhere between my brain the my keyboard. What I meant to say was:

Since we're planning to implement the Web Audio API and move away from the Audio Data API I don't think it's worth us spending time fixing the Audio Data API; I think we should just work around this issue.

If we shutdown the audio stream when the dialer app goes into the background, and recreate it if/when the dialer app is brought back to the forefront, the problem goes away.

So I propose we make the following changes to work around this problem:
* Change nsHTMLMediaElement::AbortExistingLoads() to shutdown and reset mAudioStream if it's non-null. Then setting HTMLMediaElement.src=""; will cause the Audio Data API to be reset.
* Change the Gaia dialer app to reset/recreate the audio stream in a mozvisibilitychange event handler, the handler would do something like:

  if (document.mozHidden) {
     this._audio.src = null;
     delete this._audio;
  } else {
     // init audio...
  }

I'll prepare a patch an a pull request to make these changes.
Comment 12 Chris Pearce (:cpearce) 2012-08-20 20:51:24 PDT
Created attachment 653641 [details] [diff] [review]
Patch: reset mAudioStream in nsHTMLMediaElement::AbortExistingLoads()
Comment 13 Chris Pearce (:cpearce) 2012-08-20 21:21:03 PDT
Gaia pull request:
https://github.com/mozilla-b2g/gaia/pull/3623
Comment 14 Chris Pearce (:cpearce) 2012-08-20 22:02:56 PDT
Created attachment 653653 [details] [diff] [review]
Patch 2: Remove redundant mAudioStream reset

We also don't need the existing mAudioStream reset in nsHTMLMediaElement::LoadResource() once we reset AbortExistingLoads(), as the AbortExistingLoads() will always run before LoadResource() is called.

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