Tab Audio Capture: Browser-side connect/disconnect functionality.

media/audio/audio_output_controller.cc

Index: media/audio/audio_output_controller.cc
diff --git a/media/audio/audio_output_controller.cc b/media/audio/audio_output_controller.cc
index 50850c9265cdd2b0f503e97994a122b70167047d..ac1ca355eb0110da0becb6fd79b57fef080f1d50 100644
--- a/media/audio/audio_output_controller.cc
+++ b/media/audio/audio_output_controller.cc
@@ -29,6 +29,7 @@ AudioOutputController::AudioOutputController(AudioManager* audio_manager,
                                              const AudioParameters& params,
                                              SyncReader* sync_reader)
     : audio_manager_(audio_manager),
+      params_(params),
       handler_(handler),
       stream_(NULL),
       volume_(1.0),
@@ -36,7 +37,6 @@ AudioOutputController::AudioOutputController(AudioManager* audio_manager,
       sync_reader_(sync_reader),
       message_loop_(audio_manager->GetMessageLoop()),
       number_polling_attempts_left_(0),
-      params_(params),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_this_(this)) {
 }
 
@@ -70,13 +70,10 @@ scoped_refptr<AudioOutputController> AudioOutputController::Create(
   if (!params.IsValid() || !audio_manager)
     return NULL;
 
-  // Starts the audio controller thread.
   scoped_refptr<AudioOutputController> controller(new AudioOutputController(
       audio_manager, event_handler, params, sync_reader));
-
   controller->message_loop_->PostTask(FROM_HERE, base::Bind(
       &AudioOutputController::DoCreate, controller));
-
   return controller;
 }
 
@@ -138,10 +135,9 @@ void AudioOutputController::DoCreate() {
     return;
   }
 
-  // Everything started okay, so register for state change callbacks if we have
-  // not already done so.
-  if (state_ != kRecreating)
-    audio_manager_->AddOutputDeviceChangeListener(this);
+  // Everything started okay, so re-register for state change callbacks (the
+  // call to DoStopCloseAndClearStream() above de-registered this instance).
+  audio_manager_->AddOutputDeviceChangeListener(this);

[DaleCurtis, 2012/12/05 23:35:14]

I don't see where you're removing the device change listener? Won't this add a
new listener everytime?

[miu, 2012/12/11 02:30:45]

The call to DoStopCloseAndClearStream() above called
RemoveOutputDeviceChangeListener().

I've clarified the comment on the line above.

 
   // We have successfully opened the stream. Set the initial volume.
   stream_->SetVolume(volume_);
@@ -215,7 +211,8 @@ void AudioOutputController::StartStream() {
   state_ = kPlaying;
 
   // We start the AudioOutputStream lazily.
-  stream_->Start(this);
+  if (stream_)

[DaleCurtis, 2012/12/05 23:35:14]

stream_ should always be valid here. why not check() instead.

[miu, 2012/12/11 02:30:45]

I had changed the semantics so that stream_ is NULL whenever audio data is not
destined for the AudioManager-owned AudioOutputStream.

Having simplified my approach, I reverted to the original semantics: stream_ is
always non-NULL and until closed.

+    stream_->Start(this);
 
   // Tell the event handler that we are now playing.
   handler_->OnPlaying(this);
@@ -305,11 +302,12 @@ int AudioOutputController::OnMoreIOData(AudioBus* source,
   TRACE_EVENT0("audio", "AudioOutputController::OnMoreIOData");
 
   {
-    // Check state and do nothing if we are not playing.
+    // Check state.  When not playing, fill the destination with zeros.
     // We are on the hardware audio thread, so lock is needed.
     base::AutoLock auto_lock(lock_);
     if (state_ != kPlaying) {
-      return 0;
+      dest->Zero();
+      return dest->frames();
     }
   }
 
@@ -353,7 +351,6 @@ void AudioOutputController::DoStopCloseAndClearStream(WaitableEvent* done) {
     stream_ = NULL;
 
     audio_manager_->RemoveOutputDeviceChangeListener(this);
-    audio_manager_ = NULL;
 
     weak_this_.InvalidateWeakPtrs();
   }
@@ -366,16 +363,11 @@ void AudioOutputController::DoStopCloseAndClearStream(WaitableEvent* done) {
 void AudioOutputController::OnDeviceChange() {
   DCHECK(message_loop_->BelongsToCurrentThread());
 
-  // We should always have a stream by this point.

[DaleCurtis, 2012/12/05 23:35:14]

Is this not true anymore?

[miu, 2012/12/11 02:30:45]

Done.  (Reverted back to original behavior.)

-  CHECK(stream_);
-
-  // Preserve the original state and shutdown the stream.
+  // Preserve the original state.
   State original_state = state_;
-  stream_->Stop();
-  stream_->Close();
-  stream_ = NULL;
 
-  // Recreate the stream, exit if we ran into an error.
+  // Recreate the stream (DoCreate() will first shut down an existing stream).
+  // Exit if we ran into an error.
   state_ = kRecreating;
   DoCreate();
   if (!stream_ || state_ == kError)
@@ -385,6 +377,7 @@ void AudioOutputController::OnDeviceChange() {
   switch (original_state) {
     case kStarting:
     case kPlaying:
+      DoSetVolume(volume_);
       DoPlay();
       return;
     case kCreated:
@@ -397,4 +390,39 @@ void AudioOutputController::OnDeviceChange() {
   }
 }
 
+AudioOutputStream::AudioSourceCallback* AudioOutputController::Divert() {
+  // Assumption: AudioRendererHost is calling this method on the IO
+  // BrowserThread.
+  DCHECK(!message_loop_->BelongsToCurrentThread());

[DaleCurtis, 2012/12/05 23:35:14]

Is this helpful? It doesn't really matter what thread it's called from right?

[miu, 2012/12/11 02:30:45]

Done.

+
+  // Block until stream closure is complete because, otherwise, it's possible
+  // for two entities to be pulling audio data at the same time.  In addition,
+  // a side effect of closing the stream is that this controller will be
+  // de-registered from device change events (from AudioManager) during the
+  // "divert."
+  base::WaitableEvent blocker(true, false);
+  message_loop_->PostTask(
+      FROM_HERE,
+      base::Bind(&AudioOutputController::DoStopCloseAndClearStream, this,
+                 &blocker));
+  blocker.Wait();

[DaleCurtis, 2012/12/05 23:35:14]

Hmmm, seems risky. You're blocking the IO thread with this. Would a callback
based solution be better?

[miu, 2012/12/11 02:30:45]

I changed my approach, and came up with something cleaner that doesn't block any
threads:

1. AudioOutputController no longer extends AudioSourceCallback.  Created a
"Glue" helper class that implements AudioSourceCallback instead.

2. When diverting/reverting audio, Glue::PassSoon() will ensure a non-blocking
"pointer swap."  Hopefully the code is clear enough that you can easily see how
this is accomplished.

3. For simplicity, I don't mess with stream_ at all to do a Divert/Revert. 
stream_ simply gets zeros while mirroring is active (i.e., muted audio).  Look
how simple AOC::Divert() and AOC::Revert() look now!  :)

+
+  return this;
+}
+
+void AudioOutputController::Revert(
+    AudioOutputStream::AudioSourceCallback* asc) {
+  // Assumption: AudioRendererHost is calling this method on the IO
+  // BrowserThread.
+  DCHECK(!message_loop_->BelongsToCurrentThread());
+
+  DCHECK_EQ(this, asc);

[DaleCurtis, 2012/12/05 23:35:14]

Again seems odd.

[miu, 2012/12/11 02:30:45]

Done.

+
+  // The following re-creates the normal audio output stream and places it in
+  // the correct playback state.  It also re-registers this controller for
+  // device change events from AudioManager.
+  message_loop_->PostTask(
+      FROM_HERE, base::Bind(&AudioOutputController::OnDeviceChange, this));
+}
+
 }  // namespace media