Fix incorrect output audio buffer size setting when device goes away on Mac.

- Before changing the IO buffer size on the AudioUnit, see if the device still
  is present.
- Initialize min and max variables for buffer size range.
- Check return value when getting buffer size range; return at failure.

Bug: 960736
Change-Id: Iddf1d1736c801c46b690e6fe4a570b62f304830c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1602493
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658550}

media/audio/mac/audio_manager_mac.cc

@@ -74,17 +74,17 @@ static OSStatus GetIOBufferFrameSizeRange(AudioDeviceID device_id,
       kAudioDevicePropertyBufferFrameSizeRange, is_input);
   AudioValueRange range = {0, 0};
   UInt32 data_size = sizeof(AudioValueRange);
-  OSStatus error = AudioObjectGetPropertyData(device_id, &address, 0, NULL,
-                                              &data_size, &range);
-  if (error != noErr) {
-    OSSTATUS_DLOG(WARNING, error)
+  OSStatus result = AudioObjectGetPropertyData(device_id, &address, 0, NULL,
+                                               &data_size, &range);
+  if (result != noErr) {
+    OSSTATUS_DLOG(WARNING, result)
         << "Failed to query IO buffer size range for device: " << std::hex
         << device_id;
   } else {
     *minimum = range.mMinimum;
     *maximum = range.mMaximum;
   }
-  return error;
+  return result;
 }
 
 static bool HasAudioHardware(AudioObjectPropertySelector selector) {
@@ -1026,8 +1026,13 @@ bool AudioManagerMac::MaybeChangeBufferSize(AudioDeviceID device_id,
   // does in fact do this limitation internally and report noErr even if the
   // user tries to set an invalid size. As an example, asking for a size of
   // 4410 will on most devices be limited to 4096 without any further notice.
-  UInt32 minimum, maximum;
-  GetIOBufferFrameSizeRange(device_id, is_input, &minimum, &maximum);
+  UInt32 minimum = buffer_size;
+  UInt32 maximum = buffer_size;
+  result = GetIOBufferFrameSizeRange(device_id, is_input, &minimum, &maximum);
+  if (result != noErr) {
+    // OS error is logged in GetIOBufferFrameSizeRange().
+    return false;
+  }
   DVLOG(1) << "valid IO buffer size range: [" << minimum << ", " << maximum
            << "]";
   buffer_size = desired_buffer_size;
@@ -1187,19 +1192,19 @@ void AudioManagerMac::UnsuppressNoiseReduction(AudioDeviceID device_id) {
   }
 }
 
-bool AudioManagerMac::IncreaseIOBufferSizeIfPossible(AudioDeviceID device_id) {
+void AudioManagerMac::IncreaseIOBufferSizeIfPossible(AudioDeviceID device_id) {
   DCHECK(GetTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << "IncreaseIOBufferSizeIfPossible(id=0x" << std::hex << device_id
            << ")";
   if (in_shutdown_) {
     DVLOG(1) << "Disabled since we are shutting down";
-    return false;
+    return;
   }
-  // Start by storing the actual I/O buffer size. Then scan all active output
+
+  // Start by getting the actual I/O buffer size. Then scan all active output
   // streams using the specified |device_id| and find the minimum requested
   // buffer size. In addition, store a reference to the audio unit of the first
   // output stream using |device_id|.
-  DCHECK(!output_io_buffer_size_map_.empty());
   // All active output streams use the same actual I/O buffer size given
   // a unique device ID.
   // TODO(henrika): it would also be possible to use AudioUnitGetProperty(...,
@@ -1207,7 +1212,12 @@ bool AudioManagerMac::IncreaseIOBufferSizeIfPossible(AudioDeviceID device_id) {
   // buffer size but I have chosen to use the map instead to avoid possibly
   // expensive Core Audio API calls and the risk of failure when asking while
   // closing a stream.
-  const size_t& actual_size = output_io_buffer_size_map_[device_id];
+  // TODO(http://crbug.com/961629): There seems to be bugs in the caching.
+  const size_t actual_size =
+      output_io_buffer_size_map_.find(device_id) !=
+              output_io_buffer_size_map_.end()
+          ? output_io_buffer_size_map_[device_id]
+          : 0;  // This leads to trying to update the buffer size below.
   AudioUnit audio_unit;
   size_t min_requested_size = std::numeric_limits<std::size_t>::max();
   for (auto* stream : output_streams_) {
@@ -1225,7 +1235,7 @@ bool AudioManagerMac::IncreaseIOBufferSizeIfPossible(AudioDeviceID device_id) {
 
   if (min_requested_size == std::numeric_limits<std::size_t>::max()) {
     DVLOG(1) << "No action since there is no active stream for given device id";
-    return false;
+    return;
   }
 
   // It is only possible to revert to a larger buffer size if the lowest
@@ -1236,22 +1246,20 @@ bool AudioManagerMac::IncreaseIOBufferSizeIfPossible(AudioDeviceID device_id) {
   if (min_requested_size == actual_size) {
     DVLOG(1) << "No action since lowest possible size is already in use: "
              << actual_size;
-    return false;
+    return;
   }
 
   // It should now be safe to increase the I/O buffer size to a new (higher)
   // value using the |min_requested_size|. Doing so will save system resources.
   // All active output streams with the same |device_id| are affected by this
   // change but it is only required to apply the change to one of the streams.
+  // We ignore the result from MaybeChangeBufferSize(). Logging is done in that
+  // function and it could fail if the device was removed during the operation.
   DVLOG(1) << "min_requested_size: " << min_requested_size;
   bool size_was_changed = false;
   size_t io_buffer_frame_size = 0;
-  bool result =
-      MaybeChangeBufferSize(device_id, audio_unit, 0, min_requested_size,
-                            &size_was_changed, &io_buffer_frame_size);
-  DCHECK_EQ(io_buffer_frame_size, min_requested_size);
-  DCHECK(size_was_changed);
-  return result;
+  MaybeChangeBufferSize(device_id, audio_unit, 0, min_requested_size,
+                        &size_was_changed, &io_buffer_frame_size);
 }
 
 bool AudioManagerMac::AudioDeviceIsUsedForInput(AudioDeviceID device_id) {
@@ -1297,12 +1305,19 @@ void AudioManagerMac::ReleaseOutputStreamUsingRealDevice(
   if (output_streams_.empty())
     return;
 
-  if (!AudioDeviceIsUsedForInput(device_id)) {
-    // The current audio device is not used for input. See if it is possible to
-    // increase the IO buffer size (saves power) given the remaining output
-    // audio streams and their buffer size requirements.
+  // If the audio device exists (i.e. has not been removed from the system) and
+  // is not used for input, see if it is possible to increase the IO buffer size
+  // (saves power) given the remaining output audio streams and their buffer
+  // size requirements.
+  // TODO(grunell): When closing several idle streams
+  // (AudioOutputDispatcherImpl::CloseIdleStreams), we should ideally only
+  // update the buffer size once after closing all those streams.
+  std::vector<AudioObjectID> device_ids =
+      core_audio_mac::GetAllAudioDeviceIDs();
+  const bool device_exists = std::find(device_ids.begin(), device_ids.end(),
+                                       device_id) != device_ids.end();
+  if (device_exists && !AudioDeviceIsUsedForInput(device_id))
     IncreaseIOBufferSizeIfPossible(device_id);
-  }
 }
 
 void AudioManagerMac::ReleaseInputStream(AudioInputStream* stream) {

media/audio/mac/audio_manager_mac.h

@@ -182,7 +182,7 @@ class MEDIA_EXPORT AudioManagerMac : public AudioManagerBase {
   // false otherwise.
   // TODO(henrika): possibly extend the scheme to also take input streams into
   // account.
-  bool IncreaseIOBufferSizeIfPossible(AudioDeviceID device_id);
+  void IncreaseIOBufferSizeIfPossible(AudioDeviceID device_id);
 
   std::string GetDefaultDeviceID(bool is_input);