Closed
Bug 693131
Opened 10 years ago
Closed 10 years ago
Problems playing 11kHz WAV audio and short files on Android
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: cjones, Assigned: kinetik)
References
Details
Attachments
(3 files, 2 obsolete files)
STR (1) Open the attached testcase (2) Press the "Beep" button You should hear the DTMF tone for "0", and do, on my Ubuntu desktop. In firefox-android, I don't hear anything. In a bit more detail: I tried playing this audio in both a b2g build (which is still very close to firefox-android under the covers) on an unbranded Galaxy S II, and nothing happened. This is equivalent to trying to play audio from the chrome process. I got this error in logcat W/System.err( 2826): java.lang.IllegalArgumentException: Invalid audio buffer size. W/System.err( 2826): at android.media.AudioTrack.audioBuffSizeCheck(AudioTrack.java:436) W/System.err( 2826): at android.media.AudioTrack.<init>(AudioTrack.java:314) W/System.err( 2826): at android.media.AudioTrack.<init>(AudioTrack.java:265) W/System.err( 2826): at dalvik.system.NativeStart.run(Native Method) W/System.err( 2826): at dalvik.system.NativeStart.run(Native Method) Not very helpful. I tried the same test page in a vanilla firefox-android nightly on another phone of my, Galaxy S II Epic 4G, and got the same result: nothing played. In this setup, the audio would have been played from a content process. There were no errors in the logcat this time. In even more detail: the audio format is 16-bit single-channel 11KHz PCM (WAV), ~240ms. It's embedded in the testcase as a base64-encoded data URI. I'll attach the raw .wav file it was encoded from in a second.
Reporter | ||
Comment 1•10 years ago
|
||
Reporter | ||
Comment 2•10 years ago
|
||
Another datapoint: on my Galaxy S II Epic 4G, I tried to play the raw audio file in firefox-android, but it downloaded and played the file in my music player instead. The file played just fine. So android and my HW are able to play the file. I tried modifying the testcase above to load the raw .wav file (<audio src="0.wav">) instead of using a base64 URI, but I got the same result: no audio played.
Reporter | ||
Comment 3•10 years ago
|
||
In my b2g build, the check that's failing is private void audioBuffSizeCheck(int audioBufferSize) { // NB: this section is only valid with PCM data. // To update when supporting compressed formats int frameSizeInBytes = mChannelCount * (mAudioFormat == AudioFormat.ENCODING_PCM_8BIT ? 1 : 2); if ((audioBufferSize % frameSizeInBytes != 0) || (audioBufferSize < 1)) { throw (new IllegalArgumentException("Invalid audio buffer size.")); }
Assignee | ||
Comment 4•10 years ago
|
||
AudioTrack's buffer size parameters is supposed to be in bytes, but the code in sydney_audio_android.c#sa_stream_create_pcm is calculating the buffer size in samples. Changing s->bufferSize = rate * channels; to s->bufferSize = rate * channels * sizeof(short); should fix this.
Assignee | ||
Comment 5•10 years ago
|
||
sa_stream_get_write_size is also mixing units: *size = s->bufferSize - ((s->timePlaying * s->channels * s->rate / MILLISECONDS_PER_SECOND) - s->amountWritten); should be *size = s->bufferSize - ((s->timePlaying * s->channels * s->rate * sizeof(short) / MILLISECONDS_PER_SECOND) - s->amountWritten);
Reporter | ||
Comment 6•10 years ago
|
||
This makes the error go away, but I still don't get any audio :/.
Assignee | ||
Comment 7•10 years ago
|
||
The logic in sa_stream_write to start the stream playing after the first write won't run if the first write is small enough to complete in a single loop iteration. With that fixed the audio still isn't audible. This can be worked around by writing additional data in sa_stream_drain (e.g. s->bufferSize) before draining, suggesting a minimum write size before data becomes audible--but I don't see such a requirement in the AudioTrack documentation.
Reporter | ||
Comment 8•10 years ago
|
||
Matthew/Mike, is that a reasonable workaround? Matthew, do you have a patch I can try?
Assignee | ||
Comment 9•10 years ago
|
||
I'll post a patch soon, I'm trying to work out exactly what the minimum-write requirements might be.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → kinetik
Assignee | ||
Comment 10•10 years ago
|
||
This is the best I can come up with for now. The original testcase works on every button press for me on a Nexus S. Includes the fixes described in this bug so far, plus a workaround in sa_stream_drain that fills the audio buffers with silence to force a flush of any already buffered data. To avoid drain being delayed by the additional silence, compute the drain wait based on the amount of data written before the silence. Also includes a fix to sa_stream_destroy where the previous code was leaving the stream active, resulting in obtainBuffer timeouts appearing in logcat.
Attachment #566096 -
Attachment is obsolete: true
Attachment #566958 -
Flags: review?(doug.turner)
Assignee | ||
Comment 12•10 years ago
|
||
Minor changes: - use sizeof(int16_t) rather sizeof(short) to match existing code - use SetByteArrayRegion rather than GetByteArrayElements/ReleaseByteArrayElements (reported to be fasted--I couldn't measure a difference, but the code is simpler)
Attachment #566958 -
Attachment is obsolete: true
Attachment #566958 -
Flags: review?(doug.turner)
Attachment #567973 -
Flags: review?(doug.turner)
Assignee | ||
Updated•10 years ago
|
Summary: Problems playing WAV audio → Problems playing 11kHz WAV audio and short files on Android
Comment 13•10 years ago
|
||
Comment on attachment 567973 [details] [diff] [review] patch v1 Review of attachment 567973 [details] [diff] [review]: ----------------------------------------------------------------- R+ if you address the leak (either fix it or comment here), and add a comment to the extra sa_stream_write() ::: media/libsydneyaudio/src/sydney_audio_android.c @@ +311,2 @@ > > jbyteArray bytearray = (*jenv)->NewByteArray(jenv, nbytes); why don't we free this any longer? @@ +463,5 @@ > size_t available; > sa_stream_get_write_size(s, &available); > > + void *p = calloc(1, available); > + sa_stream_write(s, p, available); i don't understand this bit completely. maybe a comment would help.
Attachment #567973 -
Flags: review?(doug.turner) → review+
Assignee | ||
Comment 14•10 years ago
|
||
> why don't we free this any longer? The object allocated by NewByteArray is (and was) freed by the GC. The removed Release was paired with the removed Get. > i don't understand this bit completely. maybe a comment would help. Good point, I'll add this comment when I check in: /* This is somewhat of a hack (see bug 693131). The AudioTrack documentation doesn't make it clear how much data must be written before a chunk of data is played, and experimentation with short streams required filling the entire allocated buffer. To guarantee that short streams (and the end of longer streams) are audible, fill the remaining space in the AudioTrack with silence before sleeping. Note that the sleep duration is calculated from the duration of audio written before filling the buffer with silence. */
Assignee | ||
Comment 15•10 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/92900f4e19f1
Target Milestone: --- → mozilla10
Comment 16•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/92900f4e19f1
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•