Dynamically FIFO when OSX requests unexpected frame counts.

media/audio/mac/audio_auhal_mac.cc

Index: media/audio/mac/audio_auhal_mac.cc
diff --git a/media/audio/mac/audio_auhal_mac.cc b/media/audio/mac/audio_auhal_mac.cc
index 6badbe9d285a20bbf54f9eb1e0f5c6802b241a65..aeea39c83dceacc45cc88e43b9601b6b78c24cd6 100644
--- a/media/audio/mac/audio_auhal_mac.cc
+++ b/media/audio/mac/audio_auhal_mac.cc
@@ -58,8 +58,8 @@ AUHALStream::AUHALStream(
       volume_(1),
       hardware_latency_frames_(0),
       stopped_(false),
-      notified_for_possible_device_change_(false),
-      input_buffer_list_(NULL) {
+      input_buffer_list_(NULL),
+      current_hardware_pending_bytes_(0) {
   // We must have a manager.
   DCHECK(manager_);
 
@@ -150,7 +150,7 @@ void AUHALStream::Start(AudioSourceCallback* callback) {
   }
 
   stopped_ = false;
-  notified_for_possible_device_change_ = false;
+  audio_fifo_.reset();
   {
     base::AutoLock auto_lock(source_lock_);
     source_ = callback;
@@ -200,63 +200,68 @@ OSStatus AUHALStream::Render(
     AudioBufferList* io_data) {
   TRACE_EVENT0("audio", "AUHALStream::Render");
 
+  // If the stream parameters change for any reason, we need to insert a FIFO
+  // since our pipeline can't handle frame size changes.  Generally this is a

[scherkus (not reviewing), 2013/11/13 18:35:19]

is this referring to media::Pipeline, or something else? it'd be good to clarify
/ use different terminology

also thinking long term ... is this something we _should_ support? I know
regular audio/video playback has no issues dealing w/ variable buffer sizes but
that might not be true for other types of applications.

[DaleCurtis, 2013/11/13 20:06:24]

Comment clarified.

The shared memory is sized to fit, so no audio clients are currently prepared to
handle variable buffer sizes. Resolving that has a number of issues; e.g. slow
shared memory creation, signaling around current buffer size, etc. I expect the
situation would be difficult for WebRTC and WebAudio without incurring a latency
penalty.

+  // temporary situation which can occur after a device change has occurred but
+  // the AudioManager hasn't received the notification yet.
   if (number_of_frames != number_of_frames_) {
-    // This can happen if we've suddenly changed sample-rates.
-    // The stream should be stopping very soon.
-    //
-    // Unfortunately AUAudioInputStream and AUHALStream share the frame
-    // size set by kAudioDevicePropertyBufferFrameSize above on a per process
-    // basis.  What this means is that the |number_of_frames| value may be
-    // larger or smaller than the value set during ConfigureAUHAL().
-    // In this case either audio input or audio output will be broken,
-    // so just output silence.
-    ZeroBufferList(io_data);
-    return noErr;
-  }
+    // Create a FIFO on the fly to handle any discrepancies in callback rates.
+    if (!audio_fifo_) {
+      audio_fifo_.reset(new AudioPullFifo(

[scherkus (not reviewing), 2013/11/13 18:35:19]

OOC does using the fifo have a perf impact? (i.e., is there a reason why we
couldn't use it all the time instead of special-casing it?)

I'm wondering about situations where, say, this only happens a few times before
going back to being normal and now we're stuck using the FIFO.

[DaleCurtis, 2013/11/13 20:06:24]

Yes it has a perf impact of at least 6.5% (see AudioConverter perf test) due to
the multiple memcpys involved. Even if the correct frame size returned it'd be
impossible to drop the FIFO, since some remainder of audio data would always be 
present.

+          output_channels_,
+          number_of_frames_,
+          base::Bind(&AUHALStream::ProvideInput, base::Unretained(this))));
+    }
 
-  if (input_channels_ > 0 && input_buffer_list_) {
-    // Get the input data.  |input_buffer_list_| is wrapped
-    // to point to the data allocated in |input_bus_|.
-    OSStatus result = AudioUnitRender(
-        audio_unit_,
-        flags,
-        output_time_stamp,
-        1,
-        number_of_frames,
-        input_buffer_list_);
-    if (result != noErr)
-      ZeroBufferList(input_buffer_list_);
+    // Synchronous IO is not supported in this state.
+    if (input_channels_ > 0)
+      input_bus_->Zero();
+  } else {
+    if (input_channels_ > 0 && input_buffer_list_) {
+      // Get the input data.  |input_buffer_list_| is wrapped
+      // to point to the data allocated in |input_bus_|.
+      OSStatus result = AudioUnitRender(audio_unit_,
+                                        flags,
+                                        output_time_stamp,
+                                        1,
+                                        number_of_frames,
+                                        input_buffer_list_);
+      if (result != noErr)
+        ZeroBufferList(input_buffer_list_);
+    }
   }
 
   // Make |output_bus_| wrap the output AudioBufferList.
   WrapBufferList(io_data, output_bus_.get(), number_of_frames);
 
   // Update the playout latency.
-  double playout_latency_frames = GetPlayoutLatency(output_time_stamp);
+  const double playout_latency_frames = GetPlayoutLatency(output_time_stamp);
+  current_hardware_pending_bytes_ = static_cast<uint32>(
+      (playout_latency_frames + 0.5) * params_.GetBytesPerFrame());
 
-  uint32 hardware_pending_bytes = static_cast<uint32>
-      ((playout_latency_frames + 0.5) * output_format_.mBytesPerFrame);
+  if (audio_fifo_)
+    audio_fifo_->Consume(output_bus_.get(), output_bus_->frames());
+  else
+    ProvideInput(0, output_bus_.get());
 
-  {
-    // Render() shouldn't be called except between AudioOutputUnitStart() and
-    // AudioOutputUnitStop() calls, but crash reports have shown otherwise:
-    // http://crbug.com/178765.  We use |source_lock_| to prevent races and
-    // crashes in Render() when |source_| is cleared.
-    base::AutoLock auto_lock(source_lock_);
-    if (!source_) {
-      ZeroBufferList(io_data);
-      return noErr;
-    }
+  return noErr;
+}
 
-    // Supply the input data and render the output data.
-    source_->OnMoreIOData(
-        input_bus_.get(),
-        output_bus_.get(),
-        AudioBuffersState(0, hardware_pending_bytes));
-    output_bus_->Scale(volume_);
+void AUHALStream::ProvideInput(int frame_delay, AudioBus* dest) {
+  base::AutoLock auto_lock(source_lock_);
+  if (!source_) {
+    dest->Zero();
+    return;
   }
 
-  return noErr;
+  // Supply the input data and render the output data.
+  source_->OnMoreIOData(
+      input_bus_.get(),
+      dest,
+      AudioBuffersState(0,
+                        current_hardware_pending_bytes_ +
+                            frame_delay * params_.GetBytesPerFrame()));
+  dest->Scale(volume_);
 }
 
 // AUHAL callback.