Add RestartableAudioRendererSink, a restartable AudioRendererSink.

This is the start of a refactor of the semantics of the
AudioRendererSink interface to allow Start() to be called again
after Stop(). This change defines the new interface and ports
MockAudioRendererSink, NullAudioSink, AudioRendererMixerInput,
AudioOutputStreamSink, and WebAudioSourceProviderImpl) to it. This
allows WebMediaPlayerImpl to switch to using
RestartableAudioRendererSink.

BUG=516850

Review URL: https://codereview.chromium.org/1428313003

Cr-Commit-Position: refs/heads/master@{#358666}

16 files changed, 103 insertions, 62 deletions

diff --git a/components/html_viewer/media_factory.cc b/components/html_viewer/media_factory.cc
index 4d155d3..64426c1 100644
--- a/components/html_viewer/media_factory.cc
+++ b/components/html_viewer/media_factory.cc
@@ -160,7 +160,7 @@ const media::AudioHardwareConfig& MediaFactory::GetAudioHardwareConfig() {
   return audio_hardware_config_;
 }
 
-scoped_refptr<media::AudioRendererSink>
+scoped_refptr<media::RestartableAudioRendererSink>
 MediaFactory::CreateAudioRendererSink() {
   // TODO(dalecurtis): Replace this with an interface to an actual mojo service;
   // the AudioOutputStreamSink will not work in sandboxed processes.
diff --git a/components/html_viewer/media_factory.h b/components/html_viewer/media_factory.h
index 70d1f7d..edae380 100644
--- a/components/html_viewer/media_factory.h
+++ b/components/html_viewer/media_factory.h
@@ -30,7 +30,7 @@ class WebMediaPlayerEncryptedMediaClient;
 
 namespace media {
 class AudioManager;
-class AudioRendererSink;
+class RestartableAudioRendererSink;
 class CdmFactory;
 class MediaPermission;
 class WebEncryptedMediaClientImpl;
@@ -70,7 +70,7 @@ class MediaFactory {
 
 #if !defined(OS_ANDROID)
   const media::AudioHardwareConfig& GetAudioHardwareConfig();
-  scoped_refptr<media::AudioRendererSink> CreateAudioRendererSink();
+  scoped_refptr<media::RestartableAudioRendererSink> CreateAudioRendererSink();
   scoped_refptr<base::SingleThreadTaskRunner> GetMediaThreadTaskRunner();
 
   base::Thread media_thread_;
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 5a9da31..bc482c6 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -2150,10 +2150,10 @@ blink::WebMediaPlayer* RenderFrameImpl::createMediaPlayer(
   RenderThreadImpl* render_thread = RenderThreadImpl::current();
 
 #if defined(OS_ANDROID) && !defined(ENABLE_MEDIA_PIPELINE_ON_ANDROID)
-  scoped_refptr<media::AudioRendererSink> audio_renderer_sink;
+  scoped_refptr<media::RestartableAudioRendererSink> audio_renderer_sink;
   media::WebMediaPlayerParams::Context3DCB context_3d_cb;
 #else
-  scoped_refptr<media::AudioRendererSink> audio_renderer_sink =
+  scoped_refptr<media::RestartableAudioRendererSink> audio_renderer_sink =
       render_thread->GetAudioRendererMixerManager()->CreateInput(
           routing_id_, sink_id.utf8(), frame->securityOrigin());
   media::WebMediaPlayerParams::Context3DCB context_3d_cb =
diff --git a/media/audio/audio_output_stream_sink.cc b/media/audio/audio_output_stream_sink.cc
index cd08fdd..ceb943f 100644
--- a/media/audio/audio_output_stream_sink.cc
+++ b/media/audio/audio_output_stream_sink.cc
@@ -12,11 +12,12 @@
 namespace media {
 
 AudioOutputStreamSink::AudioOutputStreamSink()
-    : render_callback_(NULL),
+    : initialized_(false),
+      started_(false),
+      render_callback_(NULL),
+      active_render_callback_(NULL),
       audio_task_runner_(AudioManager::Get()->GetTaskRunner()),
-      stream_(NULL),
-      active_render_callback_(NULL) {
-}
+      stream_(NULL) {}
 
 AudioOutputStreamSink::~AudioOutputStreamSink() {
 }
@@ -24,18 +25,27 @@ AudioOutputStreamSink::~AudioOutputStreamSink() {
 void AudioOutputStreamSink::Initialize(const AudioParameters& params,
                                        RenderCallback* callback) {
   DCHECK(callback);
-  DCHECK(!render_callback_);
+  DCHECK(!started_);
   params_ = params;
   render_callback_ = callback;
+  initialized_ = true;
 }
 
 void AudioOutputStreamSink::Start() {
+  DCHECK(initialized_);
+  DCHECK(!started_);
+  {
+    base::AutoLock al(callback_lock_);
+    active_render_callback_ = render_callback_;
+  }
+  started_ = true;
   audio_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&AudioOutputStreamSink::DoStart, this));
+      FROM_HERE, base::Bind(&AudioOutputStreamSink::DoStart, this, params_));
 }
 
 void AudioOutputStreamSink::Stop() {
   ClearCallback();
+  started_ = false;
   audio_task_runner_->PostTask(
       FROM_HERE, base::Bind(&AudioOutputStreamSink::DoStop, this));
 }
@@ -47,8 +57,10 @@ void AudioOutputStreamSink::Pause() {
 }
 
 void AudioOutputStreamSink::Play() {
-  base::AutoLock al(callback_lock_);
-  active_render_callback_ = render_callback_;
+  {
+    base::AutoLock al(callback_lock_);
+    active_render_callback_ = render_callback_;
+  }
   audio_task_runner_->PostTask(
       FROM_HERE, base::Bind(&AudioOutputStreamSink::DoPlay, this));
 }
@@ -71,7 +83,7 @@ int AudioOutputStreamSink::OnMoreData(AudioBus* dest,
     return 0;
 
   return active_render_callback_->Render(
-      dest, total_bytes_delay * 1000.0 / params_.GetBytesPerSecond());
+      dest, total_bytes_delay * 1000.0 / active_params_.GetBytesPerSecond());
 }
 
 void AudioOutputStreamSink::OnError(AudioOutputStream* stream) {
@@ -81,15 +93,20 @@ void AudioOutputStreamSink::OnError(AudioOutputStream* stream) {
     active_render_callback_->OnRenderError();
 }
 
-void AudioOutputStreamSink::DoStart() {
+void AudioOutputStreamSink::DoStart(const AudioParameters& params) {
   DCHECK(audio_task_runner_->BelongsToCurrentThread());
 
   // Create an AudioOutputStreamProxy which will handle any and all resampling
   // necessary to generate a low latency output stream.
-  stream_ =
-      AudioManager::Get()->MakeAudioOutputStreamProxy(params_, std::string());
+  active_params_ = params;
+  stream_ = AudioManager::Get()->MakeAudioOutputStreamProxy(active_params_,
+                                                            std::string());
   if (!stream_ || !stream_->Open()) {
-    render_callback_->OnRenderError();
+    {
+      base::AutoLock al(callback_lock_);
+      if (active_render_callback_)
+        active_render_callback_->OnRenderError();
+    }
     if (stream_)
       stream_->Close();
     stream_ = NULL;
diff --git a/media/audio/audio_output_stream_sink.h b/media/audio/audio_output_stream_sink.h
index 6ab5dc1..1a5701b 100644
--- a/media/audio/audio_output_stream_sink.h
+++ b/media/audio/audio_output_stream_sink.h
@@ -23,12 +23,12 @@ namespace media {
 // TODO(dalecurtis): Delete this class once we have a proper mojo audio service;
 // tracked by http://crbug.com/425368
 class MEDIA_EXPORT AudioOutputStreamSink
-    : NON_EXPORTED_BASE(public AudioRendererSink),
+    : NON_EXPORTED_BASE(public RestartableAudioRendererSink),
       public AudioOutputStream::AudioSourceCallback {
  public:
   AudioOutputStreamSink();
 
-  // AudioRendererSink implementation.
+  // RestartableAudioRendererSink implementation.
   void Initialize(const AudioParameters& params,
                   RenderCallback* callback) override;
   void Start() override;
@@ -44,37 +44,39 @@ class MEDIA_EXPORT AudioOutputStreamSink
 
  private:
   ~AudioOutputStreamSink() override;
+  void ClearCallback();
 
   // Helper methods for running AudioManager methods on the audio thread.
-  void DoStart();
+  void DoStart(const AudioParameters& params);
   void DoStop();
   void DoPause();
   void DoPlay();
   void DoSetVolume(double volume);
 
-  // Clears |active_render_callback_| under lock, synchronously stopping render
-  // callbacks from any thread.  Must be called before Pause() and Stop()
-  // trampoline to their helper methods on the audio thread.
-  void ClearCallback();
+  bool initialized_;
+  bool started_;
 
-  // Parameters provided by Initialize(), cached for use on other threads.
+  // Parameters provided by Initialize().
   AudioParameters params_;
-
-  // Since Initialize() is only called once for AudioRenderSinks, save the
-  // callback both here and under |active_render_callback_| which will be
-  // cleared during Pause() and Stop() to achieve "synchronous" stoppage.
   RenderCallback* render_callback_;
 
+  // State latched for the audio thread.
+  // |active_render_callback_| allows Stop()/Pause() to synchronously prevent
+  // callbacks. Access is synchronized by |callback_lock_|.
+  // |active_params_| is set on the audio thread and therefore does not need
+  // synchronization.
+  AudioParameters active_params_;
+  RenderCallback* active_render_callback_;
+
+  // Lock to synchronize setting and clearing of |active_render_callback_|.
+  base::Lock callback_lock_;
+
   // The task runner for the audio thread.
   const scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner_;
 
   // The actual AudioOutputStream, must only be accessed on the audio thread.
   AudioOutputStream* stream_;
 
-  // Lock and callback for forwarding OnMoreData() calls into Render() calls.
-  base::Lock callback_lock_;
-  RenderCallback* active_render_callback_;
-
   DISALLOW_COPY_AND_ASSIGN(AudioOutputStreamSink);
 };
 
diff --git a/media/audio/null_audio_sink.cc b/media/audio/null_audio_sink.cc
index ba25718..5b043a0 100644
--- a/media/audio/null_audio_sink.cc
+++ b/media/audio/null_audio_sink.cc
@@ -15,16 +15,16 @@ namespace media {
 NullAudioSink::NullAudioSink(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
     : initialized_(false),
+      started_(false),
       playing_(false),
       callback_(NULL),
-      task_runner_(task_runner) {
-}
+      task_runner_(task_runner) {}
 
 NullAudioSink::~NullAudioSink() {}
 
 void NullAudioSink::Initialize(const AudioParameters& params,
                                RenderCallback* callback) {
-  DCHECK(!initialized_);
+  DCHECK(!started_);
   fake_worker_.reset(new FakeAudioWorker(task_runner_, params));
   audio_bus_ = AudioBus::Create(params);
   callback_ = callback;
@@ -33,12 +33,14 @@ void NullAudioSink::Initialize(const AudioParameters& params,
 
 void NullAudioSink::Start() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK(!playing_);
+  DCHECK(initialized_);
+  DCHECK(!started_);
+  started_ = true;
 }
 
 void NullAudioSink::Stop() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-
+  started_ = false;
   // Stop may be called at any time, so we have to check before stopping.
   if (fake_worker_)
     fake_worker_->Stop();
@@ -46,18 +48,20 @@ void NullAudioSink::Stop() {
 
 void NullAudioSink::Play() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK(initialized_);
+  DCHECK(started_);
 
   if (playing_)
     return;
 
   fake_worker_->Start(base::Bind(
       &NullAudioSink::CallRender, base::Unretained(this)));
+
   playing_ = true;
 }
 
 void NullAudioSink::Pause() {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(started_);
 
   if (!playing_)
     return;
diff --git a/media/audio/null_audio_sink.h b/media/audio/null_audio_sink.h
index 85c7731..9bfa518 100644
--- a/media/audio/null_audio_sink.h
+++ b/media/audio/null_audio_sink.h
@@ -21,7 +21,7 @@ class FakeAudioWorker;
 class OutputDevice;
 
 class MEDIA_EXPORT NullAudioSink
-    : NON_EXPORTED_BASE(public AudioRendererSink) {
+    : NON_EXPORTED_BASE(public RestartableAudioRendererSink) {
  public:
   NullAudioSink(const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
 
@@ -49,6 +49,7 @@ class MEDIA_EXPORT NullAudioSink
   void CallRender();
 
   bool initialized_;
+  bool started_;
   bool playing_;
   RenderCallback* callback_;
 
diff --git a/media/base/audio_renderer_mixer_input.cc b/media/base/audio_renderer_mixer_input.cc
index 73166d3..9c5c372 100644
--- a/media/base/audio_renderer_mixer_input.cc
+++ b/media/base/audio_renderer_mixer_input.cc
@@ -15,8 +15,8 @@ AudioRendererMixerInput::AudioRendererMixerInput(
     const RemoveMixerCB& remove_mixer_cb,
     const std::string& device_id,
     const url::Origin& security_origin)
-    : playing_(false),
-      initialized_(false),
+    : initialized_(false),
+      playing_(false),
       volume_(1.0f),
       get_mixer_cb_(get_mixer_cb),
       remove_mixer_cb_(remove_mixer_cb),
@@ -28,15 +28,14 @@ AudioRendererMixerInput::AudioRendererMixerInput(
                            base::Unretained(this))) {}
 
 AudioRendererMixerInput::~AudioRendererMixerInput() {
-  DCHECK(!playing_);
   DCHECK(!mixer_);
 }
 
 void AudioRendererMixerInput::Initialize(
     const AudioParameters& params,
     AudioRendererSink::RenderCallback* callback) {
+  DCHECK(!mixer_);
   DCHECK(callback);
-  DCHECK(!initialized_);
 
   params_ = params;
   callback_ = callback;
@@ -45,7 +44,6 @@ void AudioRendererMixerInput::Initialize(
 
 void AudioRendererMixerInput::Start() {
   DCHECK(initialized_);
-  DCHECK(!playing_);
   DCHECK(!mixer_);
   mixer_ = get_mixer_cb_.Run(params_, device_id_, security_origin_, nullptr);
   if (!mixer_) {
diff --git a/media/base/audio_renderer_mixer_input.h b/media/base/audio_renderer_mixer_input.h
index e2c5932..9ec4518 100644
--- a/media/base/audio_renderer_mixer_input.h
+++ b/media/base/audio_renderer_mixer_input.h
@@ -17,7 +17,7 @@ namespace media {
 class AudioRendererMixer;
 
 class MEDIA_EXPORT AudioRendererMixerInput
-    : NON_EXPORTED_BASE(public AudioRendererSink),
+    : NON_EXPORTED_BASE(public RestartableAudioRendererSink),
       NON_EXPORTED_BASE(public OutputDevice),
       public AudioConverter::InputCallback {
  public:
@@ -36,7 +36,7 @@ class MEDIA_EXPORT AudioRendererMixerInput
                           const std::string& device_id,
                           const url::Origin& security_origin);
 
-  // AudioRendererSink implementation.
+  // RestartableAudioRendererSink implementation.
   void Start() override;
   void Stop() override;
   void Play() override;
@@ -62,8 +62,8 @@ class MEDIA_EXPORT AudioRendererMixerInput
  private:
   friend class AudioRendererMixerInputTest;
 
-  bool playing_;
   bool initialized_;
+  bool playing_;
   double volume_;
 
   // AudioConverter::InputCallback implementation.
diff --git a/media/base/audio_renderer_mixer_input_unittest.cc b/media/base/audio_renderer_mixer_input_unittest.cc
index c268578..2f8f977 100644
--- a/media/base/audio_renderer_mixer_input_unittest.cc
+++ b/media/base/audio_renderer_mixer_input_unittest.cc
@@ -147,13 +147,21 @@ TEST_F(AudioRendererMixerInputTest, StopBeforeInitializeOrStart) {
 }
 
 // Test that Start() can be called after Stop().
-// TODO(dalecurtis): We shouldn't allow this.  See http://crbug.com/151051
 TEST_F(AudioRendererMixerInputTest, StartAfterStop) {
   mixer_input_->Stop();
   mixer_input_->Start();
   mixer_input_->Stop();
 }
 
+// Test that Initialize() can be called again after Stop().
+TEST_F(AudioRendererMixerInputTest, InitializeAfterStop) {
+  mixer_input_->Initialize(audio_parameters_, fake_callback_.get());
+  mixer_input_->Start();
+  mixer_input_->Stop();
+  mixer_input_->Initialize(audio_parameters_, fake_callback_.get());
+  mixer_input_->Stop();
+}
+
 // Test SwitchOutputDevice().
 TEST_F(AudioRendererMixerInputTest, SwitchOutputDevice) {
   mixer_input_->Start();
diff --git a/media/base/audio_renderer_sink.h b/media/base/audio_renderer_sink.h
index 1e091af..30cbfef 100644
--- a/media/base/audio_renderer_sink.h
+++ b/media/base/audio_renderer_sink.h
@@ -79,6 +79,16 @@ class AudioRendererSink
   virtual ~AudioRendererSink() {}
 };
 
+// Same as AudioRendererSink except that Initialize() and Start() can be called
+// again after Stop().
+// TODO(sandersd): Fold back into AudioRendererSink once all subclasses support
+// this.
+
+class RestartableAudioRendererSink : public AudioRendererSink {
+ protected:
+  ~RestartableAudioRendererSink() override {}
+};
+
 }  // namespace media
 
 #endif  // MEDIA_BASE_AUDIO_RENDERER_SINK_H_
diff --git a/media/base/mock_audio_renderer_sink.h b/media/base/mock_audio_renderer_sink.h
index c44ac40..a966f71 100644
--- a/media/base/mock_audio_renderer_sink.h
+++ b/media/base/mock_audio_renderer_sink.h
@@ -15,7 +15,7 @@ namespace media {
 
 class FakeOutputDevice;
 
-class MockAudioRendererSink : public AudioRendererSink {
+class MockAudioRendererSink : public RestartableAudioRendererSink {
  public:
   MockAudioRendererSink();
   explicit MockAudioRendererSink(OutputDeviceStatus device_status);
diff --git a/media/blink/webaudiosourceprovider_impl.cc b/media/blink/webaudiosourceprovider_impl.cc
index 6efda72..ddc79fe 100644
--- a/media/blink/webaudiosourceprovider_impl.cc
+++ b/media/blink/webaudiosourceprovider_impl.cc
@@ -47,7 +47,7 @@ class AutoTryLock {
 }  // namespace
 
 WebAudioSourceProviderImpl::WebAudioSourceProviderImpl(
-    const scoped_refptr<AudioRendererSink>& sink)
+    const scoped_refptr<RestartableAudioRendererSink>& sink)
     : channels_(0),
       sample_rate_(0),
       volume_(1.0),
@@ -126,6 +126,7 @@ void WebAudioSourceProviderImpl::provideInput(
 
 void WebAudioSourceProviderImpl::Start() {
   base::AutoLock auto_lock(sink_lock_);
+  DCHECK(renderer_);
   DCHECK_EQ(state_, kStopped);
   state_ = kStarted;
   if (!client_)
@@ -172,7 +173,6 @@ void WebAudioSourceProviderImpl::Initialize(
     const AudioParameters& params,
     RenderCallback* renderer) {
   base::AutoLock auto_lock(sink_lock_);
-  CHECK(!renderer_);
   renderer_ = renderer;
 
   DCHECK_EQ(state_, kStopped);
diff --git a/media/blink/webaudiosourceprovider_impl.h b/media/blink/webaudiosourceprovider_impl.h
index 675be92..947c33c 100644
--- a/media/blink/webaudiosourceprovider_impl.h
+++ b/media/blink/webaudiosourceprovider_impl.h
@@ -32,17 +32,17 @@ namespace media {
 // All calls are protected by a lock.
 class MEDIA_BLINK_EXPORT WebAudioSourceProviderImpl
     : NON_EXPORTED_BASE(public blink::WebAudioSourceProvider),
-      NON_EXPORTED_BASE(public AudioRendererSink) {
+      NON_EXPORTED_BASE(public RestartableAudioRendererSink) {
  public:
   explicit WebAudioSourceProviderImpl(
-      const scoped_refptr<AudioRendererSink>& sink);
+      const scoped_refptr<RestartableAudioRendererSink>& sink);
 
   // blink::WebAudioSourceProvider implementation.
   void setClient(blink::WebAudioSourceProviderClient* client) override;
   void provideInput(const blink::WebVector<float*>& audio_data,
                     size_t number_of_frames) override;
 
-  // AudioRendererSink implementation.
+  // RestartableAudioRendererSink implementation.
   void Start() override;
   void Stop() override;
   void Play() override;
@@ -79,7 +79,7 @@ class MEDIA_BLINK_EXPORT WebAudioSourceProviderImpl
 
   // Where audio ends up unless overridden by |client_|.
   base::Lock sink_lock_;
-  scoped_refptr<AudioRendererSink> sink_;
+  scoped_refptr<RestartableAudioRendererSink> sink_;
   scoped_ptr<AudioBus> bus_wrapper_;
 
   // NOTE: Weak pointers must be invalidated before all other member variables.
diff --git a/media/blink/webmediaplayer_params.cc b/media/blink/webmediaplayer_params.cc
index ffe3c1b..530d5c9 100644
--- a/media/blink/webmediaplayer_params.cc
+++ b/media/blink/webmediaplayer_params.cc
@@ -13,7 +13,7 @@ namespace media {
 
 WebMediaPlayerParams::WebMediaPlayerParams(
     const DeferLoadCB& defer_load_cb,
-    const scoped_refptr<AudioRendererSink>& audio_renderer_sink,
+    const scoped_refptr<RestartableAudioRendererSink>& audio_renderer_sink,
     const scoped_refptr<MediaLog>& media_log,
     const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
     const scoped_refptr<base::TaskRunner>& worker_task_runner,
diff --git a/media/blink/webmediaplayer_params.h b/media/blink/webmediaplayer_params.h
index 35f5134..8022431 100644
--- a/media/blink/webmediaplayer_params.h
+++ b/media/blink/webmediaplayer_params.h
@@ -22,7 +22,7 @@ class WebMediaPlayerClient;
 
 namespace media {
 
-class AudioRendererSink;
+class RestartableAudioRendererSink;
 class MediaLog;
 class MediaPermission;
 
@@ -44,7 +44,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
   // |context_3d_cb| may be null.
   WebMediaPlayerParams(
       const DeferLoadCB& defer_load_cb,
-      const scoped_refptr<AudioRendererSink>& audio_renderer_sink,
+      const scoped_refptr<RestartableAudioRendererSink>& audio_renderer_sink,
       const scoped_refptr<MediaLog>& media_log,
       const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
       const scoped_refptr<base::TaskRunner>& worker_task_runner,
@@ -58,7 +58,8 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
 
   DeferLoadCB defer_load_cb() const { return defer_load_cb_; }
 
-  const scoped_refptr<AudioRendererSink>& audio_renderer_sink() const {
+  const scoped_refptr<RestartableAudioRendererSink>& audio_renderer_sink()
+      const {
     return audio_renderer_sink_;
   }
 
@@ -93,7 +94,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerParams {
 
  private:
   DeferLoadCB defer_load_cb_;
-  scoped_refptr<AudioRendererSink> audio_renderer_sink_;
+  scoped_refptr<RestartableAudioRendererSink> audio_renderer_sink_;
   scoped_refptr<MediaLog> media_log_;
   scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
   scoped_refptr<base::TaskRunner> worker_task_runner_;