Switch OnMoreData() to use AudioBus.

media/audio/mac/audio_output_mac.cc

Index: media/audio/mac/audio_output_mac.cc
diff --git a/media/audio/mac/audio_output_mac.cc b/media/audio/mac/audio_output_mac.cc
index 7f96735a495d6771b1e027740129cb7dcc95806a..5414c64457b7e892f1b66ebc45db579bafd0fec0 100644
--- a/media/audio/mac/audio_output_mac.cc
+++ b/media/audio/mac/audio_output_mac.cc
@@ -53,7 +53,8 @@ PCMQueueOutAudioOutputStream::PCMQueueOutAudioOutputStream(
       should_swizzle_(false),
       should_down_mix_(false),
       stopped_event_(true /* manual reset */, false /* initial state */),
-      num_buffers_left_(kNumBuffers) {
+      num_buffers_left_(kNumBuffers),
+      audio_bus_(AudioBus::Create(params)) {
   // We must have a manager.
   DCHECK(manager_);
   // A frame is one sample across all channels. In interleaved audio the per
@@ -409,11 +410,25 @@ void PCMQueueOutAudioOutputStream::RenderCallback(void* p_this,
   // Adjust the number of pending bytes by subtracting the amount played.
   if (!static_cast<AudioQueueUserData*>(buffer->mUserData)->empty_buffer)
     audio_stream->pending_bytes_ -= buffer->mAudioDataByteSize;
+  uint32 filled = 0;
   uint32 capacity = buffer->mAudioDataBytesCapacity;
-  // TODO(sergeyu): Specify correct hardware delay for AudioBuffersState.
-  uint32 filled = source->OnMoreData(
-      reinterpret_cast<uint8*>(buffer->mAudioData), capacity,
-      AudioBuffersState(audio_stream->pending_bytes_, 0));
+  {
+    // Since RenderCallback may be reentrant, we need to ensure the AudioBus
+    // we're sharing is safe for this instance.  OnMoreData() offers no
+    // guarantee of thread safety as well.
+    base::AutoLock lock(audio_stream->audio_bus_lock_);

[Chris Rogers, 2012/08/24 20:20:26]

Are you sure this is right?  If this function is re-entrant then we don't want a
lock since we'll get dead-lock (see comment on line 374).  I'm also pretty sure
that this method won't be called from multiple threads in the same time window
so the lock shouldn't be needed.

[DaleCurtis, 2012/08/24 23:53:12]

That indicates that a lock shouldn't be held across AudioQueue / OS API calls,
within the calling range of OnMoreData() I believe this is safe (otherwise no
function all the way down OnMoreData could hold a lock).

Without the lock if multiple OnMoreData() calls occur the shared audio_bus may
be trampled as locks further down the OnMoreData chain cycle to allow the next
pass through.

+

[Chris Rogers, 2012/08/24 20:20:26]

You use "audio_stream->audio_bus_" three times below.  It would be nice to use a
local AudioBus*

[DaleCurtis, 2012/08/24 23:53:12]

Done. Presumably PCMQueueOutAudioOutputStream won't be destroyed while we're in
the middle of this call?

+    DCHECK_EQ(audio_stream->audio_bus_->frames() *
+              audio_stream->format_.mBytesPerFrame, capacity);
+    // TODO(sergeyu): Specify correct hardware delay for AudioBuffersState.
+    int frames_filled = source->OnMoreData(
+        audio_stream->audio_bus_.get(),
+        AudioBuffersState(audio_stream->pending_bytes_, 0));
+    filled = frames_filled * audio_stream->format_.mBytesPerFrame;
+    audio_stream->audio_bus_->ToInterleaved(
+        frames_filled, audio_stream->format_.mBitsPerChannel / 8,
+        buffer->mAudioData);
+  }
 
   // In order to keep the callback running, we need to provide a positive amount
   // of data to the audio queue. To simulate the behavior of Windows, we write