Cleanup AudioManager initialization on OSX to reduce startup delay.

Cleanup AudioManager initialization on OSX to reduce startup delay.

Since the UI thread is the audio thread on OSX, we should be careful
about what we execute during startup.  Device enumeration and creation
of the device listener can be expensive, so don't do them until the
first stream is created.

BUG=288759
TEST=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251443

[DaleCurtis, 2014-02-12 23:31:46]

Another CL I had in the queue which aimed to reduce initialization costs, though
this one is solving a known issue.

The UMA stat previously discussed wouldn't really help here since we were
already deferring startup, just until a later time on the UI thread.

Moving it to first use may increase first stream creation costs, but IMHO those
are acceptable to UI startup delays.

[tommi (sloooow) - chröme, 2014-02-13 06:56:20]

https://codereview.chromium.org/154543002/diff/60001/media/audio/mac/audio_ma...
File media/audio/mac/audio_manager_mac.cc (right):

https://codereview.chromium.org/154543002/diff/60001/media/audio/mac/audio_ma...
media/audio/mac/audio_manager_mac.cc:588: if (device_id.empty())
Check for AudioManagerBase::kDefaultDeviceId?

[DaleCurtis, 2014-02-13 19:24:35]

https://codereview.chromium.org/154543002/diff/60001/media/audio/mac/audio_ma...
File media/audio/mac/audio_manager_mac.cc (right):

https://codereview.chromium.org/154543002/diff/60001/media/audio/mac/audio_ma...
media/audio/mac/audio_manager_mac.cc:588: if (device_id.empty())
On 2014/02/13 06:56:20, tommi wrote:
> Check for AudioManagerBase::kDefaultDeviceId?

That's only for input no?

[tommi (sloooow) - chröme, 2014-02-13 19:45:55]

On 2014/02/13 19:24:35, DaleCurtis wrote:
>
https://codereview.chromium.org/154543002/diff/60001/media/audio/mac/audio_ma...
> File media/audio/mac/audio_manager_mac.cc (right):
> 
>
https://codereview.chromium.org/154543002/diff/60001/media/audio/mac/audio_ma...
> media/audio/mac/audio_manager_mac.cc:588: if (device_id.empty())
> On 2014/02/13 06:56:20, tommi wrote:
> > Check for AudioManagerBase::kDefaultDeviceId?
> 
> That's only for input no?

Maybe I'm misparsing this somehow but it looks like we use that for output as
well:
https://code.google.com/p/chromium/codesearch#chromium/src/media/audio/mac/au...

Personally I would like to simply use the empty string to indicate a default
device and get rid of kDefaultDeviceId.

[DaleCurtis, 2014-02-13 20:31:34]

Ah, you're right.  I don't think that's always true though. We should definitely
unify on a single identifier.

[DaleCurtis, 2014-02-13 20:31:53]

https://codereview.chromium.org/154543002/diff/60001/media/audio/mac/audio_ma...
File media/audio/mac/audio_manager_mac.cc (right):

https://codereview.chromium.org/154543002/diff/60001/media/audio/mac/audio_ma...
media/audio/mac/audio_manager_mac.cc:588: if (device_id.empty())
On 2014/02/13 19:24:35, DaleCurtis wrote:
> On 2014/02/13 06:56:20, tommi wrote:
> > Check for AudioManagerBase::kDefaultDeviceId?
> 
> That's only for input no?

Done.

[tommi (sloooow) - chröme, 2014-02-13 20:55:13]

lgtm

[DaleCurtis, 2014-02-14 01:45:59]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2014-02-14 01:52:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/154543002/240001

[commit-bot: I haz the power, 2014-02-14 01:52:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-14 01:52:30]

Failed to apply patch for media/audio/mac/audio_manager_mac.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file media/audio/mac/audio_manager_mac.cc
  Hunk #1 succeeded at 261 (offset -12 lines).
  Hunk #2 FAILED at 579.
  Hunk #3 succeeded at 674 (offset -66 lines).
  1 out of 3 hunks FAILED -- saving rejects to file
media/audio/mac/audio_manager_mac.cc.rej

Patch:       media/audio/mac/audio_manager_mac.cc
Index: media/audio/mac/audio_manager_mac.cc
diff --git a/media/audio/mac/audio_manager_mac.cc
b/media/audio/mac/audio_manager_mac.cc
index
c08efff93363beb5cd62d1bc2f606c664f7558ee..d4004caba4cde628dd5299a7350f7028ba5baf8e
100644
--- a/media/audio/mac/audio_manager_mac.cc
+++ b/media/audio/mac/audio_manager_mac.cc
@@ -273,26 +273,25 @@ class AudioManagerMac::AudioPowerObserver : public
base::PowerObserver {
 
 AudioManagerMac::AudioManagerMac(AudioLogFactory* audio_log_factory)
     : AudioManagerBase(audio_log_factory),
-      current_sample_rate_(0) {
-  current_output_device_ = kAudioDeviceUnknown;
-
+      current_sample_rate_(0),
+      current_output_device_(kAudioDeviceUnknown) {
   SetMaxOutputStreamsAllowed(kMaxOutputStreams);
 
   // Task must be posted last to avoid races from handing out "this" to the
   // audio thread.  Always PostTask even if we're on the right thread since
   // AudioManager creation is on the startup path and this may be slow.
   GetTaskRunner()->PostTask(FROM_HERE, base::Bind(
-      &AudioManagerMac::CreateDeviceListener, base::Unretained(this)));
+      &AudioManagerMac::InitializeOnAudioThread, base::Unretained(this)));
 }
 
 AudioManagerMac::~AudioManagerMac() {
   if (GetTaskRunner()->BelongsToCurrentThread()) {
-    DestroyDeviceListener();
+    ShutdownOnAudioThread();
   } else {
     // It's safe to post a task here since Shutdown() will wait for all tasks
to
     // complete before returning.
     GetTaskRunner()->PostTask(FROM_HERE, base::Bind(
-        &AudioManagerMac::DestroyDeviceListener, base::Unretained(this)));
+        &AudioManagerMac::ShutdownOnAudioThread, base::Unretained(this)));
   }
 
   Shutdown();
@@ -580,6 +579,19 @@ AudioOutputStream*
AudioManagerMac::MakeLowLatencyOutputStream(
       DLOG(ERROR) << "Failed to open output device: " << device_id;
       return NULL;
     }
+
+    // Lazily create the audio device listener on the first stream creation.
+    if (!output_device_listener_) {
+      output_device_listener_.reset(new AudioDeviceListenerMac(base::Bind(
+          &AudioManagerMac::HandleDeviceChanges, base::Unretained(this))));
+      // Only set the current output device for the default device.
+      if (device_id == AudioManagerBase::kDefaultDeviceId || device_id.empty())
+        current_output_device_ = device;
+      // Just use the current sample rate since we don't allow non-native
sample
+      // rates on OSX.
+      current_sample_rate_ = params.sample_rate();
+    }
+
     return new AUHALStream(this, params, device);
   }
 
@@ -741,21 +753,12 @@ AudioParameters
AudioManagerMac::GetPreferredOutputStreamParameters(
       AudioParameters::NO_EFFECTS);
 }
 
-void AudioManagerMac::CreateDeviceListener() {
+void AudioManagerMac::InitializeOnAudioThread() {
   DCHECK(GetTaskRunner()->BelongsToCurrentThread());
-
-  // Get a baseline for the sample-rate and current device,
-  // so we can intelligently handle device notifications only when necessary.
-  current_sample_rate_ = HardwareSampleRate();
-  if (!GetDefaultOutputDevice(&current_output_device_))
-    current_output_device_ = kAudioDeviceUnknown;
-
-  output_device_listener_.reset(new AudioDeviceListenerMac(base::Bind(
-      &AudioManagerMac::HandleDeviceChanges, base::Unretained(this))));
   power_observer_.reset(new AudioPowerObserver());
 }
 
-void AudioManagerMac::DestroyDeviceListener() {
+void AudioManagerMac::ShutdownOnAudioThread() {
   DCHECK(GetTaskRunner()->BelongsToCurrentThread());
   output_device_listener_.reset();
   power_observer_.reset();

[DaleCurtis, 2014-02-14 02:01:20]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2014-02-14 02:02:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/154543002/360001

[commit-bot: I haz the power, 2014-02-14 03:20:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-14 03:20:17]

Try jobs failed on following builders: linux_chromium_rel

[tommi (sloooow) - chröme, 2014-02-14 08:23:33]

The CQ bit was checked by tommi@chromium.org

[commit-bot: I haz the power, 2014-02-14 10:09:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/154543002/360001

[commit-bot: I haz the power, 2014-02-14 10:10:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-14 10:10:02]

Try jobs failed on following builders: linux_chromium_rel

[DaleCurtis, 2014-02-14 20:52:24]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2014-02-14 20:53:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/154543002/360001

[commit-bot: I haz the power, 2014-02-14 23:58:02]

Change committed as 251443