Last Comment Bug 693905 - Android audio has numerous issues
: Android audio has numerous issues
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 9 Branch
: ARM Android
-- normal (vote)
: mozilla10
Assigned To: Matthew Gregan [:kinetik]
: Maire Reavy [:mreavy] Please needinfo me
Depends on: 695986
Blocks: 643454
  Show dependency treegraph
Reported: 2011-10-11 21:32 PDT by Chris Pearce (:cpearce)
Modified: 2011-10-20 03:12 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch v0 (4.47 KB, patch)
2011-10-18 22:31 PDT, Matthew Gregan [:kinetik]
cpearce: review+
Details | Diff | Splinter Review

Description User image Chris Pearce (:cpearce) 2011-10-11 21:32:14 PDT
Try comparing playback performance on android of the following two videos:
1. and

(1) is the contains no audio track, it only has the video track from (2).

On my Samsung Galaxy S GT-I9000 2.3.3 perf of (2) is terrible, but (1) is watchable.

Something in our audio playback code on android is hurting us.
Comment 1 User image Henri Sivonen (:hsivonen) 2011-10-12 03:49:36 PDT
This also happens on Android 3.1 on Galaxy Tab 10.1 with trunk builds. doesn't have audio and plays fine. Every video that has audio that I've tried plays terribly.
Comment 2 User image Matthew Gregan [:kinetik] 2011-10-13 16:15:45 PDT
Problems are observable with an audio-only WebM/Vorbis stream, so I changed the bug title.

The initial problem is that the stream is constantly hitting xrun.  Since sa_stream_write is expected to start the stream as soon as it is called, the stream is being started with only 23ms of audio buffered.  We already know this model is not suitable for our use (bug 623444), so that's no surprise.  Adding a hack to start playback later (once at least bufferSize/2 has been written) helps slightly, but the stream begins to xrun within a second or so, suggesting problems with when the sa_stream_write calls are being scheduled.
Comment 3 User image Matthew Gregan [:kinetik] 2011-10-13 22:11:47 PDT
Ugh, the patch in bug 669556 actually makes the situation much worse on Android because it relies on audio writes blocking when the backend has full buffers.  The remoted audio stream never blocks, it just shunts the audio data over to the chrome process and returns immediately.  This results in the same "hit audio event queue max" issue mentioned in that bug, and also confuses the decoder buffering logic by always being in a "low audio" state.
Comment 4 User image Matthew Gregan [:kinetik] 2011-10-18 16:35:54 PDT
The position estimation incorrectly treats PRIntervalTime as if it is in nanoseconds, causing the playback position to be correct only when a new reliable position is received from the parent (which happens at 1Hz).
Comment 5 User image Matthew Gregan [:kinetik] 2011-10-18 22:31:36 PDT
Created attachment 567972 [details] [diff] [review]
patch v0

Fix audio stream position estimation for remoted streams.  Also resurrect audio thread wait removed in bug 669556 when using remoted audio streams.

With this patch, the patch in bug 693131, and the patch in bug 695240 applied, the testcase linked in the first bug is no longer completely broken.
Comment 6 User image Matthew Gregan [:kinetik] 2011-10-19 15:53:25 PDT
Comment 7 User image Matthew Gregan [:kinetik] 2011-10-19 15:58:08 PDT
Oops, the checkin comment for this refers to bug 693095. :-(
Comment 8 User image Oleg Romashin (:romaxa) 2011-10-19 16:02:45 PDT
Does it make sense to do the same for Maemo? or all mobile devices? how is remote audio related to this problem?
Comment 9 User image Matthew Gregan [:kinetik] 2011-10-19 16:09:17 PDT
It's only a problem for users of nsRemotedAudio, which is only used by Android (because JNI, and therefore the sound API, is only available in the parent process).  The problem is that nsAudioStream's Write call is expected to block when the audio buffers are full, but the remoted implementation doesn't support that functionality (bug 695612 is for fixing that).
Comment 10 User image Marco Bonardo [::mak] 2011-10-20 03:12:02 PDT
(In reply to Matthew Gregan [:kinetik] from comment #7)
> Oops, the checkin comment for this refers to bug 693095. :-(

yes, in these cases it's usually better to backout and reland with the correct commit message

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