Restores larger output buffer size when output stream requiring smaller size is closed

media/audio/mac/audio_auhal_mac.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/audio/mac/audio_auhal_mac.h"
 
 #include <CoreServices/CoreServices.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 26 matching lines @@
   bus->set_frames(frames);
 }
 
 AUHALStream::AUHALStream(AudioManagerMac* manager,
                          const AudioParameters& params,
                          AudioDeviceID device)
     : manager_(manager),
       params_(params),
       output_channels_(params_.channels()),
       number_of_frames_(params_.frames_per_buffer()),
+      actual_io_buffer_frame_size_(0),
       number_of_frames_requested_(0),
       source_(NULL),
       device_(device),
       audio_unit_(0),
       volume_(1),
       hardware_latency_frames_(0),
       stopped_(true),
       current_hardware_pending_bytes_(0),
       current_lost_frames_(0),
       last_sample_time_(0.0),
@@ skipping 51 matching lines @@
 
   // The output bus will wrap the AudioBufferList given to us in
   // the Render() callback.
   DCHECK_GT(output_channels_, 0);
   output_bus_ = AudioBus::CreateWrapper(output_channels_);
 
   bool configured = ConfigureAUHAL();
   if (configured)
     hardware_latency_frames_ = GetHardwareLatency();
 
+#if !defined(NDEBUG)
+  // Print a list of all existing output streams and there current buffer size
+  // state (required and actual). Does nothing in Release builds.
+  manager_->PrintOutputBufferSizes();
+#endif
+
   return configured;
 }
 
 void AUHALStream::Close() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(1) << "Close";
   CloseAudioUnit();
   // Inform the audio manager that we have been closed. This will cause our
   // destruction.
   manager_->ReleaseOutputStream(this);
@@ skipping 40 matching lines @@
   OSSTATUS_DLOG(ERROR, result) << "AudioOutputUnitStart() failed.";
   callback->OnError(this);
 }
 
 void AUHALStream::Stop() {
   DCHECK(thread_checker_.CalledOnValidThread());
   deferred_start_cb_.Cancel();
   if (stopped_)
     return;
   DVLOG(1) << "Stop";
+  DVLOG(2) << "number_of_frames: " << number_of_frames_;
   OSStatus result = AudioOutputUnitStop(audio_unit_);
   OSSTATUS_DLOG_IF(ERROR, result != noErr, result)
       << "AudioOutputUnitStop() failed.";
   if (result != noErr)
     source_->OnError(this);
   ReportAndResetStats();
   base::AutoLock auto_lock(source_lock_);
   source_ = NULL;
   stopped_ = true;
 }
@@ skipping 16 matching lines @@
     UInt32 bus_number,
     UInt32 number_of_frames,
     AudioBufferList* data) {
   TRACE_EVENT0("audio", "AUHALStream::Render");
 
   UpdatePlayoutTimestamp(output_time_stamp);
 
   // If the stream parameters change for any reason, we need to insert a FIFO
   // since the OnMoreData() pipeline can't handle frame size changes.
   if (number_of_frames != number_of_frames_) {
+    // Update the actual buffer size given the recent change.
+    base::subtle::Release_Store(&actual_io_buffer_frame_size_,

[o1ka, 2016/04/27 13:24:40]

What is the guarantee that actual size we get in
IncreaseIOBufferSizeIfPossible() from the first output stream is up-to-date?
Isn't it a race?

[henrika (OOO until Aug 14), 2016/04/27 13:36:43]

Sorry, don't understand. Race between what?

[o1ka, 2016/04/27 15:04:31]

See my question: what is the guarantee that we updated _all_ the streams here in
Render() before executing IncreaseIOBufferSizeIfPossible()? Otherwise we may end
up with an outdated actual size there.

[Henrik Grunell, 2016/04/28 07:51:38]

I'm not following either. Olga, do you mean that the buffer size the OS requests
could change when a stream is removed? Maybe we can discuss offline.

[henrika (OOO until Aug 14), 2016/04/28 11:28:44]

Redesigned. Stream no longer contains actual buffer size, hence no race. Instead
the audio manager keeps track of actual size per device id. Much better now
IMHO. Thanks ;-)

+                                number_of_frames);
     // Create a FIFO on the fly to handle any discrepancies in callback rates.
     if (!audio_fifo_) {
       // TODO(grunell): We'll only care about the first buffer size change,
       // any further changes will be ignored. It would be nice to have all
       // changes reflected in UMA stats.
       number_of_frames_requested_ = number_of_frames;
       DVLOG(1) << "Audio frame size changed from " << number_of_frames_
-               << " to " << number_of_frames
-               << "; adding FIFO to compensate.";
+               << " to " << number_of_frames << " adding FIFO to compensate.";
       audio_fifo_.reset(new AudioPullFifo(
           output_channels_,
           number_of_frames_,
           base::Bind(&AUHALStream::ProvideInput, base::Unretained(this))));
     }
   }
 
   // Make |output_bus_| wrap the output AudioBufferList.
   WrapBufferList(data, output_bus_.get(), number_of_frames);
 
@@ skipping 272 matching lines @@
 
   bool size_was_changed = false;
   size_t io_buffer_frame_size = 0;
   if (!manager_->MaybeChangeBufferSize(device_, audio_unit_, 0,
                                        number_of_frames_, &size_was_changed,
                                        &io_buffer_frame_size)) {
     CloseAudioUnit();
     return false;
   }
 
+  // Get the actual number of frames in the IO buffers after the audio manager
+  // has tried to set the requested size. We could use |io_buffer_frame_size|
+  // here instead of doing these extra steps but I prefer to play it safe and
+  // to ensure that the change done by the audio manager is really active.
+  // TODO(henrika): it might be possible to remove this part.

[Henrik Grunell, 2016/04/28 07:51:38]

What is required to do to figure out if this part is needed? Could it be done
now? Iiuc, doing this will (hopefully) ensure we have the correct actual size as
early as possible. Does that matter? Would be nice to not have code if it
doesn't add any value.

[henrika (OOO until Aug 14), 2016/04/28 11:28:44]

Removed now ;-)

+  UInt32 actual_io_buffer_frame_size;
+  UInt32 property_size = sizeof(actual_io_buffer_frame_size);
+  result = AudioUnitGetProperty(
+      audio_unit_, kAudioDevicePropertyBufferFrameSize, kAudioUnitScope_Global,
+      0, &actual_io_buffer_frame_size, &property_size);
+  LOG_IF(WARNING, result != noErr)
+      << "Failed to get the actual I/O buffer size";
+  if (result == noErr) {
+    base::subtle::Release_Store(&actual_io_buffer_frame_size_,
+                                actual_io_buffer_frame_size);
+    DVLOG(1) << "actual_io_buffer_frame_size: " << actual_io_buffer_frame_size;
+  }
+  LOG_IF(WARNING, actual_io_buffer_frame_size != number_of_frames_)
+      << "Failed to set the requested buffer frame size exactly";
+  DCHECK_EQ(io_buffer_frame_size, actual_io_buffer_frame_size);
+
   // Setup callback.
   AURenderCallbackStruct callback;
   callback.inputProc = InputProc;
   callback.inputProcRefCon = this;
   result = AudioUnitSetProperty(
       audio_unit_,
       kAudioUnitProperty_SetRenderCallback,
       kAudioUnitScope_Input,
       0,
       &callback,
@@ skipping 21 matching lines @@
   OSStatus result = AudioUnitUninitialize(audio_unit_);
   OSSTATUS_DLOG_IF(ERROR, result != noErr, result)
       << "AudioUnitUninitialize() failed.";
   result = AudioComponentInstanceDispose(audio_unit_);
   OSSTATUS_DLOG_IF(ERROR, result != noErr, result)
       << "AudioComponentInstanceDispose() failed.";
   audio_unit_ = 0;
 }
 
 }  // namespace media