Merge 189654 "Don't change devices unless the sample rate changes."

> Don't change devices unless the sample rate changes.
> 
> Reduces the number of times we'd change devices and thus by proxy
> should reduce the number of crashes we see.
> 
> Unlike on Windows, OSX will route device changes appropriately behind
> the scenes. However if the sample rate doesn't match, the penalties include:
> - OSX may dynamically adjust the buffer size.
> - OSX may issue back to back requests for data.
> - other badness?
> 
> BUG=158170
> TEST=device changes.
> 
> Review URL: https://codereview.chromium.org/12611027

TBR=dalecurtis@google.com
Review URL: https://codereview.chromium.org/13144016

git-svn-id: svn://svn.chromium.org/chrome/branches/1410/src@191617 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 19 insertions, 2 deletions

diff --git a/media/audio/mac/audio_device_listener_mac.cc b/media/audio/mac/audio_device_listener_mac.cc
index 6121dc6f..c5f2de1 100644
--- a/media/audio/mac/audio_device_listener_mac.cc
+++ b/media/audio/mac/audio_device_listener_mac.cc
@@ -12,6 +12,7 @@
 #include "base/mac/mac_util.h"
 #include "base/message_loop.h"
 #include "base/pending_task.h"
+#include "media/audio/mac/audio_low_latency_output_mac.h"
 
 namespace media {
 
@@ -104,7 +105,16 @@ OSStatus AudioDeviceListenerMac::OnDefaultDeviceChanged(
         addresses[i].mScope == kDeviceChangePropertyAddress.mScope &&
         addresses[i].mElement == kDeviceChangePropertyAddress.mElement &&
         context) {
-      static_cast<AudioDeviceListenerMac*>(context)->listener_cb_.Run();
+      AudioDeviceListenerMac* p_this =
+          static_cast<AudioDeviceListenerMac*>(context);
+      // Device changes on Mac are risky, the OSX API is not thread safe, so
+      // only change devices if we have to.  Again, see http://crbug.com/158170.
+      // TODO(crogers): Remove this once the AUHAL output driver is in.
+      int sample_rate = AUAudioOutputStream::HardwareSampleRate();
+      if (p_this->current_sample_rate_ != sample_rate) {
+        p_this->current_sample_rate_ = sample_rate;
+        p_this->listener_cb_.Run();
+      }
       break;
     }
   }
@@ -115,7 +125,8 @@ OSStatus AudioDeviceListenerMac::OnDefaultDeviceChanged(
 AudioDeviceListenerMac::AudioDeviceListenerMac(const base::Closure& listener_cb)
     : listener_block_(NULL),
       add_listener_block_func_(NULL),
-      remove_listener_block_func_(NULL) {
+      remove_listener_block_func_(NULL),
+      current_sample_rate_(AUAudioOutputStream::HardwareSampleRate()) {
   // Device changes are hard, lets go shopping!  Sadly OSX does not handle
   // property listener callbacks in a thread safe manner.  On 10.6 we can set
   // kAudioHardwarePropertyRunLoop to account for this.  On 10.7 this is broken
diff --git a/media/audio/mac/audio_device_listener_mac.h b/media/audio/mac/audio_device_listener_mac.h
index d2d5870..24ecf26 100644
--- a/media/audio/mac/audio_device_listener_mac.h
+++ b/media/audio/mac/audio_device_listener_mac.h
@@ -68,6 +68,10 @@ class MEDIA_EXPORT AudioDeviceListenerMac {
   // thread.
   base::ThreadChecker thread_checker_;
 
+  // If the sample rate hasn't changed, don't fire a device change.  OSX will
+  // handle the routing under the hood.
+  int current_sample_rate_;
+
   DISALLOW_COPY_AND_ASSIGN(AudioDeviceListenerMac);
 };
 
diff --git a/media/audio/mac/audio_device_listener_mac_unittest.cc b/media/audio/mac/audio_device_listener_mac_unittest.cc
index 1f884bc..c88c339 100644
--- a/media/audio/mac/audio_device_listener_mac_unittest.cc
+++ b/media/audio/mac/audio_device_listener_mac_unittest.cc
@@ -63,6 +63,8 @@ class AudioDeviceListenerMacTest : public testing::Test {
         kAudioObjectPropertyElementMaster }
     };
 
+    // Force sample rate change so the listener fires.
+    output_device_listener_->current_sample_rate_++;
     return noErr == output_device_listener_->OnDefaultDeviceChanged(
         kAudioObjectSystemObject, 1, addresses, output_device_listener_.get());
   }