Last Comment Bug 693905 - Android audio has numerous issues
: Android audio has numerous issues
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: 9 Branch
: ARM Android
: -- normal (vote)
: mozilla10
Assigned To: Matthew Gregan [:kinetik]
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


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

Description Chris Pearce (:cpearce) 2011-10-11 21:32:14 PDT
Try comparing playback performance on android of the following two videos:
1. http://pearce.org.nz/video/arctic_giant.theora.ogg and
2. http://pearce.org.nz/video/arctic_giant.ogg

(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 Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-09-26) 2011-10-12 03:49:36 PDT
This also happens on Android 3.1 on Galaxy Tab 10.1 with trunk builds.

http://hsivonen.iki.fi/html5-lecture/2011/sintel.webm doesn't have audio and plays fine. Every video that has audio that I've tried plays terribly.
Comment 2 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 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 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 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 Matthew Gregan [:kinetik] 2011-10-19 15:53:25 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/959ff7fea1e8
Comment 7 Matthew Gregan [:kinetik] 2011-10-19 15:58:08 PDT
Oops, the checkin comment for this refers to bug 693095. :-(
Comment 8 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 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 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

https://hg.mozilla.org/mozilla-central/rev/959ff7fea1e8

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