Rename AudioRenderer::Play/Pause() to Start/StopRendering().

This makes a clearer separation between methods for controlling the
actual rendering of audio versus methods for controlling the state of the
renderer (i.e,. flushing and seeking).

Consequently, clean up AudioRendererImpl to reflect the separation
between rendering methods and state change methods.

BUG=370634

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270017 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 157 insertions, 85 deletions

diff --git a/media/filters/audio_renderer_impl.cc b/media/filters/audio_renderer_impl.cc
index e189c61..48da818 100644
--- a/media/filters/audio_renderer_impl.cc
+++ b/media/filters/audio_renderer_impl.cc
@@ -54,6 +54,7 @@ AudioRendererImpl::AudioRendererImpl(
       hardware_config_(hardware_config),
       now_cb_(base::Bind(&base::TimeTicks::Now)),
       state_(kUninitialized),
+      rendering_(false),
       sink_playing_(false),
       pending_read_(false),
       received_end_of_stream_(false),
@@ -73,65 +74,72 @@ AudioRendererImpl::~AudioRendererImpl() {
   DCHECK(!algorithm_.get());
 }
 
-void AudioRendererImpl::Play() {
+void AudioRendererImpl::StartRendering() {
+  DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(!rendering_);
+  rendering_ = true;
 
   base::AutoLock auto_lock(lock_);
-  DCHECK_EQ(state_, kPaused);
-  ChangeState_Locked(kPlaying);
-  earliest_end_time_ = now_cb_.Run();
-
-  if (algorithm_->playback_rate() != 0)
-    DoPlay_Locked();
-  else
+  // Wait for an eventual call to SetPlaybackRate() to start rendering.
+  if (algorithm_->playback_rate() == 0) {
     DCHECK(!sink_playing_);
+    return;
+  }
+
+  StartRendering_Locked();
 }
 
-void AudioRendererImpl::DoPlay_Locked() {
+void AudioRendererImpl::StartRendering_Locked() {
+  DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(state_ == kPlaying || state_ == kRebuffering || state_ == kUnderflow)
+      << "state_=" << state_;
+  DCHECK(!sink_playing_);
+  DCHECK_NE(algorithm_->playback_rate(), 0);
   lock_.AssertAcquired();
-  earliest_end_time_ = now_cb_.Run();
 
-  if ((state_ == kPlaying || state_ == kRebuffering || state_ == kUnderflow) &&
-      !sink_playing_) {
-    {
-      base::AutoUnlock auto_unlock(lock_);
-      sink_->Play();
-    }
+  earliest_end_time_ = now_cb_.Run();
+  sink_playing_ = true;
 
-    sink_playing_ = true;
-  }
+  base::AutoUnlock auto_unlock(lock_);
+  sink_->Play();
 }
 
-void AudioRendererImpl::Pause() {
+void AudioRendererImpl::StopRendering() {
+  DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(rendering_);
+  rendering_ = false;
 
   base::AutoLock auto_lock(lock_);
-  DCHECK(state_ == kPlaying || state_ == kUnderflow ||
-         state_ == kRebuffering) << "state_ == " << state_;
-  ChangeState_Locked(kPaused);
+  // Rendering should have already been stopped with a zero playback rate.
+  if (algorithm_->playback_rate() == 0) {
+    DCHECK(!sink_playing_);
+    return;
+  }
 
-  DoPause_Locked();
+  StopRendering_Locked();
 }
 
-void AudioRendererImpl::DoPause_Locked() {
+void AudioRendererImpl::StopRendering_Locked() {
   DCHECK(task_runner_->BelongsToCurrentThread());
+  DCHECK(state_ == kPlaying || state_ == kRebuffering || state_ == kUnderflow)
+      << "state_=" << state_;
+  DCHECK(sink_playing_);
   lock_.AssertAcquired();
 
-  if (sink_playing_) {
-    {
-      base::AutoUnlock auto_unlock(lock_);
-      sink_->Pause();
-    }
-    sink_playing_ = false;
-  }
+  sink_playing_ = false;
+
+  base::AutoUnlock auto_unlock(lock_);
+  sink_->Pause();
 }
 
 void AudioRendererImpl::Flush(const base::Closure& callback) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   base::AutoLock auto_lock(lock_);
-  DCHECK_EQ(state_, kPaused);
+  DCHECK_EQ(state_, kPlaying);
   DCHECK(flush_cb_.is_null());
 
   flush_cb_ = callback;
@@ -141,6 +149,7 @@ void AudioRendererImpl::Flush(const base::Closure& callback) {
     return;
   }
 
+  ChangeState_Locked(kFlushed);
   DoFlush_Locked();
 }
 
@@ -149,7 +158,7 @@ void AudioRendererImpl::DoFlush_Locked() {
   lock_.AssertAcquired();
 
   DCHECK(!pending_read_);
-  DCHECK_EQ(state_, kPaused);
+  DCHECK_EQ(state_, kFlushed);
 
   audio_buffer_stream_.Reset(base::Bind(&AudioRendererImpl::ResetDecoderDone,
                                         weak_factory_.GetWeakPtr()));
@@ -162,7 +171,7 @@ void AudioRendererImpl::ResetDecoderDone() {
     if (state_ == kStopped)
       return;
 
-    DCHECK_EQ(state_, kPaused);
+    DCHECK_EQ(state_, kFlushed);
     DCHECK(!flush_cb_.is_null());
 
     audio_clock_.reset(new AudioClock(audio_parameters_.sample_rate()));
@@ -215,7 +224,7 @@ void AudioRendererImpl::Preroll(base::TimeDelta time,
 
   base::AutoLock auto_lock(lock_);
   DCHECK(!sink_playing_);
-  DCHECK_EQ(state_, kPaused);
+  DCHECK_EQ(state_, kFlushed);
   DCHECK(!pending_read_) << "Pending read must complete before seeking";
   DCHECK(preroll_cb_.is_null());
 
@@ -321,7 +330,7 @@ void AudioRendererImpl::OnAudioBufferStreamInitialized(bool success) {
   algorithm_.reset(new AudioRendererAlgorithm());
   algorithm_->Initialize(0, audio_parameters_);
 
-  ChangeState_Locked(kPaused);
+  ChangeState_Locked(kFlushed);
 
   HistogramRendererEvent(INITIALIZED);
 
@@ -365,7 +374,7 @@ void AudioRendererImpl::SetVolume(float volume) {
 void AudioRendererImpl::DecodedAudioReady(
     AudioBufferStream::Status status,
     const scoped_refptr<AudioBuffer>& buffer) {
-  DVLOG(1) << __FUNCTION__ << "(" << status << ")";
+  DVLOG(2) << __FUNCTION__ << "(" << status << ")";
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   base::AutoLock auto_lock(lock_);
@@ -389,7 +398,7 @@ void AudioRendererImpl::DecodedAudioReady(
   DCHECK(buffer.get());
 
   if (state_ == kFlushing) {
-    ChangeState_Locked(kPaused);
+    ChangeState_Locked(kFlushed);
     DoFlush_Locked();
     return;
   }
@@ -463,14 +472,14 @@ bool AudioRendererImpl::HandleSplicerBuffer(
       NOTREACHED();
       return false;
 
-    case kPaused:
+    case kFlushed:
       DCHECK(!pending_read_);
       return false;
 
     case kPrerolling:
       if (!buffer->end_of_stream() && !algorithm_->IsQueueFull())
         return true;
-      ChangeState_Locked(kPaused);
+      ChangeState_Locked(kPlaying);
       base::ResetAndReturn(&preroll_cb_).Run(PIPELINE_OK);
       return false;
 
@@ -513,7 +522,7 @@ bool AudioRendererImpl::CanRead_Locked() {
   switch (state_) {
     case kUninitialized:
     case kInitializing:
-    case kPaused:
+    case kFlushed:
     case kFlushing:
     case kStopped:
       return false;
@@ -541,12 +550,20 @@ void AudioRendererImpl::SetPlaybackRate(float playback_rate) {
   // Play: current_playback_rate == 0 && playback_rate != 0
   // Pause: current_playback_rate != 0 && playback_rate == 0
   float current_playback_rate = algorithm_->playback_rate();
-  if (current_playback_rate == 0 && playback_rate != 0)
-    DoPlay_Locked();
-  else if (current_playback_rate != 0 && playback_rate == 0)
-    DoPause_Locked();
-
   algorithm_->SetPlaybackRate(playback_rate);
+
+  if (!rendering_)
+    return;
+
+  if (current_playback_rate == 0 && playback_rate != 0) {
+    StartRendering_Locked();
+    return;
+  }
+
+  if (current_playback_rate != 0 && playback_rate == 0) {
+    StopRendering_Locked();
+    return;
+  }
 }
 
 bool AudioRendererImpl::IsBeforePrerollTime(
@@ -697,12 +714,8 @@ void AudioRendererImpl::HandleAbortedReadOrDecodeError(bool is_decode_error) {
     case kInitializing:
       NOTREACHED();
       return;
-    case kPaused:
-      if (status != PIPELINE_OK)
-        error_cb_.Run(status);
-      return;
     case kFlushing:
-      ChangeState_Locked(kPaused);
+      ChangeState_Locked(kFlushed);
 
       if (status == PIPELINE_OK) {
         DoFlush_Locked();
@@ -715,9 +728,10 @@ void AudioRendererImpl::HandleAbortedReadOrDecodeError(bool is_decode_error) {
     case kPrerolling:
       // This is a signal for abort if it's not an error.
       preroll_aborted_ = !is_decode_error;
-      ChangeState_Locked(kPaused);
+      ChangeState_Locked(kPlaying);
       base::ResetAndReturn(&preroll_cb_).Run(status);
       return;
+    case kFlushed:
     case kPlaying:
     case kUnderflow:
     case kRebuffering:
diff --git a/media/filters/audio_renderer_impl.h b/media/filters/audio_renderer_impl.h
index 0748f0f..c98ead8 100644
--- a/media/filters/audio_renderer_impl.h
+++ b/media/filters/audio_renderer_impl.h
@@ -72,8 +72,8 @@ class MEDIA_EXPORT AudioRendererImpl
                           const TimeCB& time_cb,
                           const base::Closure& ended_cb,
                           const PipelineStatusCB& error_cb) OVERRIDE;
-  virtual void Play() OVERRIDE;
-  virtual void Pause() OVERRIDE;
+  virtual void StartRendering() OVERRIDE;
+  virtual void StopRendering() OVERRIDE;
   virtual void Flush(const base::Closure& callback) OVERRIDE;
   virtual void Stop(const base::Closure& callback) OVERRIDE;
   virtual void SetPlaybackRate(float rate) OVERRIDE;
@@ -97,12 +97,33 @@ class MEDIA_EXPORT AudioRendererImpl
  private:
   friend class AudioRendererImplTest;
 
-  // TODO(acolwell): Add a state machine graph.
+  // Important detail: being in kPlaying doesn't imply that audio is being
+  // rendered. Rather, it means that the renderer is ready to go. The actual
+  // rendering of audio is controlled via Start/StopRendering().
+  //
+  //   kUninitialized
+  //         | Initialize()
+  //         |
+  //         V
+  //    kInitializing
+  //         | Decoders initialized
+  //         |
+  //         V            Decoders reset
+  //      kFlushed <------------------ kFlushing
+  //         | Preroll()                  ^
+  //         |                            |
+  //         V                            | Flush()
+  //     kPrerolling ----------------> kPlaying ---------.
+  //           Enough data buffered       ^              | Not enough data
+  //                                      |              | buffered
+  //                 Enough data buffered |              V
+  //                                 kRebuffering <--- kUnderflow
+  //                                      ResumeAfterUnderflow()
   enum State {
     kUninitialized,
     kInitializing,
-    kPaused,
     kFlushing,
+    kFlushed,
     kPrerolling,
     kPlaying,
     kStopped,
@@ -127,8 +148,8 @@ class MEDIA_EXPORT AudioRendererImpl
                                     const base::TimeDelta& playback_delay,
                                     const base::TimeTicks& time_now);
 
-  void DoPlay_Locked();
-  void DoPause_Locked();
+  void StartRendering_Locked();
+  void StopRendering_Locked();
 
   // AudioRendererSink::RenderCallback implementation.
   //
@@ -235,7 +256,9 @@ class MEDIA_EXPORT AudioRendererImpl
   // Simple state tracking variable.
   State state_;
 
-  // Keep track of whether or not the sink is playing.
+  // Keep track of whether or not the sink is playing and whether we should be
+  // rendering.
+  bool rendering_;
   bool sink_playing_;
 
   // Keep track of our outstanding read to |decoder_|.
diff --git a/media/filters/audio_renderer_impl_unittest.cc b/media/filters/audio_renderer_impl_unittest.cc
index 5937870..f472ae9 100644
--- a/media/filters/audio_renderer_impl_unittest.cc
+++ b/media/filters/audio_renderer_impl_unittest.cc
@@ -228,17 +228,17 @@ class AudioRendererImplTest : public ::testing::Test {
     EXPECT_TRUE(decode_cb_.is_null());
   }
 
-  void Play() {
-    renderer_->Play();
+  void StartRendering() {
+    renderer_->StartRendering();
     renderer_->SetPlaybackRate(1.0f);
   }
 
-  void Pause() {
-    renderer_->Pause();
+  void StopRendering() {
+    renderer_->StopRendering();
   }
 
   void Seek() {
-    Pause();
+    StopRendering();
     Flush();
     Preroll();
   }
@@ -375,7 +375,7 @@ class AudioRendererImplTest : public ::testing::Test {
   void EndOfStreamTest(float playback_rate) {
     Initialize();
     Preroll();
-    Play();
+    StartRendering();
     renderer_->SetPlaybackRate(playback_rate);
 
     // Drain internal buffer, we should have a pending read.
@@ -516,10 +516,10 @@ TEST_F(AudioRendererImplTest, Preroll) {
   Preroll();
 }
 
-TEST_F(AudioRendererImplTest, Play) {
+TEST_F(AudioRendererImplTest, StartRendering) {
   Initialize();
   Preroll();
-  Play();
+  StartRendering();
 
   // Drain internal buffer, we should have a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered(), NULL));
@@ -544,7 +544,7 @@ TEST_F(AudioRendererImplTest, Underflow) {
 
   int initial_capacity = buffer_capacity();
 
-  Play();
+  StartRendering();
 
   // Drain internal buffer, we should have a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered(), NULL));
@@ -578,7 +578,7 @@ TEST_F(AudioRendererImplTest, Underflow_FollowedByFlush) {
 
   int initial_capacity = buffer_capacity();
 
-  Play();
+  StartRendering();
 
   // Drain internal buffer, we should have a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered(), NULL));
@@ -606,7 +606,7 @@ TEST_F(AudioRendererImplTest, Underflow_FollowedByFlush) {
 TEST_F(AudioRendererImplTest, Underflow_EndOfStream) {
   Initialize();
   Preroll();
-  Play();
+  StartRendering();
 
   // Figure out how long until the ended event should fire.  Since
   // ConsumeBufferedData() doesn't provide audio delay information, the time
@@ -652,7 +652,7 @@ TEST_F(AudioRendererImplTest, Underflow_EndOfStream) {
 TEST_F(AudioRendererImplTest, Underflow_ResumeFromCallback) {
   Initialize();
   Preroll();
-  Play();
+  StartRendering();
 
   // Drain internal buffer, we should have a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered(), NULL));
@@ -679,7 +679,7 @@ TEST_F(AudioRendererImplTest, Underflow_ResumeFromCallback) {
 TEST_F(AudioRendererImplTest, Underflow_SetPlaybackRate) {
   Initialize();
   Preroll();
-  Play();
+  StartRendering();
 
   // Drain internal buffer, we should have a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered(), NULL));
@@ -715,7 +715,7 @@ TEST_F(AudioRendererImplTest, Underflow_SetPlaybackRate) {
 TEST_F(AudioRendererImplTest, Underflow_PausePlay) {
   Initialize();
   Preroll();
-  Play();
+  StartRendering();
 
   // Drain internal buffer, we should have a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered(), NULL));
@@ -765,13 +765,13 @@ TEST_F(AudioRendererImplTest, AbortPendingRead_Flush) {
   Initialize();
 
   Preroll();
-  Play();
+  StartRendering();
 
   // Partially drain internal buffer so we get a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered() / 2, NULL));
   WaitForPendingRead();
 
-  Pause();
+  StopRendering();
 
   EXPECT_TRUE(IsReadPending());
 
@@ -793,13 +793,13 @@ TEST_F(AudioRendererImplTest, PendingRead_Flush) {
   Initialize();
 
   Preroll();
-  Play();
+  StartRendering();
 
   // Partially drain internal buffer so we get a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered() / 2, NULL));
   WaitForPendingRead();
 
-  Pause();
+  StopRendering();
 
   EXPECT_TRUE(IsReadPending());
 
@@ -821,13 +821,13 @@ TEST_F(AudioRendererImplTest, PendingRead_Stop) {
   Initialize();
 
   Preroll();
-  Play();
+  StartRendering();
 
   // Partially drain internal buffer so we get a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered() / 2, NULL));
   WaitForPendingRead();
 
-  Pause();
+  StopRendering();
 
   EXPECT_TRUE(IsReadPending());
 
@@ -846,13 +846,13 @@ TEST_F(AudioRendererImplTest, PendingFlush_Stop) {
   Initialize();
 
   Preroll();
-  Play();
+  StartRendering();
 
   // Partially drain internal buffer so we get a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered() / 2, NULL));
   WaitForPendingRead();
 
-  Pause();
+  StopRendering();
 
   EXPECT_TRUE(IsReadPending());
 
@@ -879,7 +879,7 @@ TEST_F(AudioRendererImplTest, InitializeThenStopDuringDecoderInit) {
 TEST_F(AudioRendererImplTest, ConfigChangeDrainsConverter) {
   Initialize();
   Preroll();
-  Play();
+  StartRendering();
 
   // Drain internal buffer, we should have a pending read.
   EXPECT_TRUE(ConsumeBufferedData(frames_buffered(), NULL));
@@ -901,7 +901,7 @@ TEST_F(AudioRendererImplTest, ConfigChangeDrainsConverter) {
 TEST_F(AudioRendererImplTest, TimeUpdatesOnFirstBuffer) {
   Initialize();
   Preroll();
-  Play();
+  StartRendering();
 
   AudioTimestampHelper timestamp_helper(kOutputSamplesPerSecond);
   EXPECT_EQ(kNoTimestamp(), last_time_update());
@@ -933,7 +933,7 @@ TEST_F(AudioRendererImplTest, ImmediateEndOfStream) {
     DeliverEndOfStream();
     event.RunAndWaitForStatus(PIPELINE_OK);
   }
-  Play();
+  StartRendering();
 
   // Read a single frame. We shouldn't be able to satisfy it.
   EXPECT_FALSE(ConsumeBufferedData(1, NULL));
@@ -943,10 +943,45 @@ TEST_F(AudioRendererImplTest, ImmediateEndOfStream) {
 TEST_F(AudioRendererImplTest, OnRenderErrorCausesDecodeError) {
   Initialize();
   Preroll();
-  Play();
+  StartRendering();
 
   EXPECT_CALL(*this, OnError(PIPELINE_ERROR_DECODE));
   sink_->OnRenderError();
 }
 
+// Test for AudioRendererImpl calling Pause()/Play() on the sink when the
+// playback rate is set to zero and non-zero.
+TEST_F(AudioRendererImplTest, SetPlaybackRate) {
+  Initialize();
+  Preroll();
+
+  // Rendering hasn't started. Sink should always be paused.
+  EXPECT_EQ(FakeAudioRendererSink::kPaused, sink_->state());
+  renderer_->SetPlaybackRate(0.0f);
+  EXPECT_EQ(FakeAudioRendererSink::kPaused, sink_->state());
+  renderer_->SetPlaybackRate(1.0f);
+  EXPECT_EQ(FakeAudioRendererSink::kPaused, sink_->state());
+
+  // Rendering has started with non-zero rate. Rate changes will affect sink
+  // state.
+  renderer_->StartRendering();
+  EXPECT_EQ(FakeAudioRendererSink::kPlaying, sink_->state());
+  renderer_->SetPlaybackRate(0.0f);
+  EXPECT_EQ(FakeAudioRendererSink::kPaused, sink_->state());
+  renderer_->SetPlaybackRate(1.0f);
+  EXPECT_EQ(FakeAudioRendererSink::kPlaying, sink_->state());
+
+  // Rendering has stopped. Sink should be paused.
+  renderer_->StopRendering();
+  EXPECT_EQ(FakeAudioRendererSink::kPaused, sink_->state());
+
+  // Start rendering with zero playback rate. Sink should be paused until
+  // non-zero rate is set.
+  renderer_->SetPlaybackRate(0.0f);
+  renderer_->StartRendering();
+  EXPECT_EQ(FakeAudioRendererSink::kPaused, sink_->state());
+  renderer_->SetPlaybackRate(1.0f);
+  EXPECT_EQ(FakeAudioRendererSink::kPlaying, sink_->state());
+}
+
 }  // namespace media