Created attachment 565777 [details] test 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.
Created attachment 565778 [details] raw audio file used in test above
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.
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.")); }
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.
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);
Created attachment 566096 [details] [diff] [review] WIP This makes the error go away, but I still don't get any audio :/.
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.
Matthew/Mike, is that a reasonable workaround? Matthew, do you have a patch I can try?
I'll post a patch soon, I'm trying to work out exactly what the minimum-write requirements might be.
Created attachment 566958 [details] [diff] [review] patch v0 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.
*** Bug 694770 has been marked as a duplicate of this bug. ***
Created attachment 567973 [details] [diff] [review] patch v1 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)
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.
> 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. */
http://hg.mozilla.org/integration/mozilla-inbound/rev/92900f4e19f1
https://hg.mozilla.org/mozilla-central/rev/92900f4e19f1
or
