Update AudioRenderer API to fire changes in BufferingState.

media/filters/audio_renderer_impl.cc

Index: media/filters/audio_renderer_impl.cc
diff --git a/media/filters/audio_renderer_impl.cc b/media/filters/audio_renderer_impl.cc
index 0dff76f37d7f3a222a30befb0bb057ee54f66d6c..dfa4163761059009cd1b646eabc5482760aab01e 100644
--- a/media/filters/audio_renderer_impl.cc
+++ b/media/filters/audio_renderer_impl.cc
@@ -52,6 +52,7 @@ AudioRendererImpl::AudioRendererImpl(
                            decoders.Pass(),
                            set_decryptor_ready_cb),
       hardware_config_(hardware_config),
+      buffering_state_(BUFFERING_HAVE_NOTHING),
       now_cb_(base::Bind(&base::TimeTicks::Now)),
       state_(kUninitialized),
       rendering_(false),
@@ -59,7 +60,6 @@ AudioRendererImpl::AudioRendererImpl(
       pending_read_(false),
       received_end_of_stream_(false),
       rendered_end_of_stream_(false),
-      preroll_aborted_(false),
       weak_factory_(this) {
   audio_buffer_stream_.set_splice_observer(base::Bind(
       &AudioRendererImpl::OnNewSpliceBuffer, weak_factory_.GetWeakPtr()));
@@ -92,8 +92,7 @@ void AudioRendererImpl::StartRendering() {
 void AudioRendererImpl::StartRendering_Locked() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK(state_ == kPlaying || state_ == kRebuffering || state_ == kUnderflow)
-      << "state_=" << state_;
+  DCHECK_EQ(state_, kPlaying);
   DCHECK(!sink_playing_);
   DCHECK_NE(algorithm_->playback_rate(), 0);
   lock_.AssertAcquired();
@@ -123,8 +122,7 @@ void AudioRendererImpl::StopRendering() {
 
 void AudioRendererImpl::StopRendering_Locked() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  DCHECK(state_ == kPlaying || state_ == kRebuffering || state_ == kUnderflow)
-      << "state_=" << state_;
+  DCHECK_EQ(state_, kPlaying);
   DCHECK(sink_playing_);
   lock_.AssertAcquired();
 
@@ -139,16 +137,14 @@ void AudioRendererImpl::Flush(const base::Closure& callback) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   base::AutoLock auto_lock(lock_);
-  DCHECK(state_ == kPlaying || state_ == kRebuffering || state_ == kUnderflow)
-      << "state_=" << state_;
+  DCHECK_EQ(state_, kPlaying);
   DCHECK(flush_cb_.is_null());
 
   flush_cb_ = callback;
+  ChangeState_Locked(kFlushing);
 
-  if (pending_read_) {
-    ChangeState_Locked(kFlushing);
+  if (pending_read_)
     return;
-  }
 
   ChangeState_Locked(kFlushed);
   DoFlush_Locked();
@@ -176,9 +172,9 @@ void AudioRendererImpl::ResetDecoderDone() {
     DCHECK(!flush_cb_.is_null());
 
     audio_clock_.reset(new AudioClock(audio_parameters_.sample_rate()));
+    buffering_state_ = BUFFERING_HAVE_NOTHING;

[acolwell GONE FROM CHROMIUM, 2014/05/16 17:25:08]

Why don't we dispatch a callback in this case? The 2 other sets send a callback.

[scherkus (not reviewing), 2014/05/22 17:49:12]

as discussed offline ... updated API docs + impls + tests to have clients expect
this callback

     received_end_of_stream_ = false;
     rendered_end_of_stream_ = false;
-    preroll_aborted_ = false;
 
     earliest_end_time_ = now_cb_.Run();
     splicer_->Reset();
@@ -207,7 +203,6 @@ void AudioRendererImpl::Stop(const base::Closure& callback) {
 
     ChangeState_Locked(kStopped);
     algorithm_.reset();
-    underflow_cb_.Reset();
     time_cb_.Reset();
     flush_cb_.Reset();
   }
@@ -220,20 +215,18 @@ void AudioRendererImpl::Stop(const base::Closure& callback) {
   audio_buffer_stream_.Stop(callback);
 }
 
-void AudioRendererImpl::Preroll(base::TimeDelta time,
-                                const PipelineStatusCB& cb) {
-  DVLOG(1) << __FUNCTION__ << "(" << time.InMicroseconds() << ")";
+void AudioRendererImpl::StartPlayingFrom(base::TimeDelta timestamp) {
+  DVLOG(1) << __FUNCTION__ << "(" << timestamp.InMicroseconds() << ")";
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   base::AutoLock auto_lock(lock_);
   DCHECK(!sink_playing_);
   DCHECK_EQ(state_, kFlushed);
+  DCHECK_EQ(buffering_state_, BUFFERING_HAVE_NOTHING);
   DCHECK(!pending_read_) << "Pending read must complete before seeking";
-  DCHECK(preroll_cb_.is_null());
 
-  ChangeState_Locked(kPrerolling);
-  preroll_cb_ = cb;
-  preroll_timestamp_ = time;
+  ChangeState_Locked(kPlaying);
+  start_timestamp_ = timestamp;
 
   AttemptRead_Locked();
 }
@@ -241,8 +234,8 @@ void AudioRendererImpl::Preroll(base::TimeDelta time,
 void AudioRendererImpl::Initialize(DemuxerStream* stream,
                                    const PipelineStatusCB& init_cb,
                                    const StatisticsCB& statistics_cb,
-                                   const base::Closure& underflow_cb,
                                    const TimeCB& time_cb,
+                                   const BufferingStateCB& buffering_state_cb,
                                    const base::Closure& ended_cb,
                                    const PipelineStatusCB& error_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
@@ -250,8 +243,8 @@ void AudioRendererImpl::Initialize(DemuxerStream* stream,
   DCHECK_EQ(stream->type(), DemuxerStream::AUDIO);
   DCHECK(!init_cb.is_null());
   DCHECK(!statistics_cb.is_null());
-  DCHECK(!underflow_cb.is_null());
   DCHECK(!time_cb.is_null());
+  DCHECK(!buffering_state_cb.is_null());
   DCHECK(!ended_cb.is_null());
   DCHECK(!error_cb.is_null());
   DCHECK_EQ(kUninitialized, state_);
@@ -260,8 +253,9 @@ void AudioRendererImpl::Initialize(DemuxerStream* stream,
   state_ = kInitializing;
 
   init_cb_ = init_cb;
-  underflow_cb_ = underflow_cb;
   time_cb_ = time_cb;
+  // TODO(scherkus): Enforce AudioRendererImpl to call on right thread.

[acolwell GONE FROM CHROMIUM, 2014/05/16 17:25:08]

Is this still a TODO? It looks like you are ensuring this right here.

[scherkus (not reviewing), 2014/05/22 17:49:12]

yeah .. I guess so :\

would rather have ARI be single threaded I guess, but it's not really
TODO-worthy

changed to comment describing why it's BTCL'd

+  buffering_state_cb_ = BindToCurrentLoop(buffering_state_cb);
   ended_cb_ = ended_cb;
   error_cb_ = error_cb;
 
@@ -351,23 +345,6 @@ void AudioRendererImpl::OnAudioBufferStreamInitialized(bool success) {
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
-void AudioRendererImpl::ResumeAfterUnderflow() {
-  DCHECK(task_runner_->BelongsToCurrentThread());
-  base::AutoLock auto_lock(lock_);
-  if (state_ == kUnderflow) {
-    // The "!preroll_aborted_" is a hack. If preroll is aborted, then we
-    // shouldn't even reach the kUnderflow state to begin with. But for now
-    // we're just making sure that the audio buffer capacity (i.e. the
-    // number of bytes that need to be buffered for preroll to complete)
-    // does not increase due to an aborted preroll.
-    // TODO(vrk): Fix this bug correctly! (crbug.com/151352)
-    if (!preroll_aborted_)
-      algorithm_->IncreaseQueueCapacity();
-
-    ChangeState_Locked(kRebuffering);
-  }
-}
-
 void AudioRendererImpl::SetVolume(float volume) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(sink_);
@@ -441,19 +418,13 @@ bool AudioRendererImpl::HandleSplicerBuffer(
     const scoped_refptr<AudioBuffer>& buffer) {
   if (buffer->end_of_stream()) {

[acolwell GONE FROM CHROMIUM, 2014/05/16 17:25:08]

nit: add lock_.AssertAcquired() and possibly _Locked to the method name.

[scherkus (not reviewing), 2014/05/22 17:49:12]

Done.

     received_end_of_stream_ = true;
-
-    // Transition to kPlaying if we are currently handling an underflow since
-    // no more data will be arriving.
-    if (state_ == kUnderflow || state_ == kRebuffering)
-      ChangeState_Locked(kPlaying);
   } else {
-    if (state_ == kPrerolling) {
-      if (IsBeforePrerollTime(buffer))
+    if (state_ == kPlaying) {
+      if (IsBeforeStartTime(buffer))
         return true;
 
-      // Trim off any additional time before the preroll timestamp.
-      const base::TimeDelta trim_time =
-          preroll_timestamp_ - buffer->timestamp();
+      // Trim off any additional time before the start timestamp.
+      const base::TimeDelta trim_time = start_timestamp_ - buffer->timestamp();
       if (trim_time > base::TimeDelta()) {
         buffer->TrimStart(buffer->frame_count() *
                           (static_cast<double>(trim_time.InMicroseconds()) /
@@ -479,22 +450,15 @@ bool AudioRendererImpl::HandleSplicerBuffer(
       DCHECK(!pending_read_);
       return false;
 
-    case kPrerolling:
-      if (!buffer->end_of_stream() && !algorithm_->IsQueueFull())
-        return true;
-      ChangeState_Locked(kPlaying);
-      base::ResetAndReturn(&preroll_cb_).Run(PIPELINE_OK);
-      return false;
-
     case kPlaying:
-    case kUnderflow:
-      return false;
-
-    case kRebuffering:
-      if (!algorithm_->IsQueueFull())
-        return true;
-      ChangeState_Locked(kPlaying);
-      return false;
+      if (buffer->end_of_stream() || algorithm_->IsQueueFull()) {
+        if (buffering_state_ == BUFFERING_HAVE_NOTHING) {
+          buffering_state_ = BUFFERING_HAVE_ENOUGH;
+          buffering_state_cb_.Run(BUFFERING_HAVE_ENOUGH);

[acolwell GONE FROM CHROMIUM, 2014/05/16 17:25:08]

nit: Put these 2 operations in a SetBufferingState_Locked() method so it's easy
to debug transitions like with ChangeState_Locked()?

[scherkus (not reviewing), 2014/05/22 17:49:12]

Done.

+        }
+        return false;
+      }
+      return true;
 
     case kStopped:
       return false;
@@ -525,15 +489,12 @@ bool AudioRendererImpl::CanRead_Locked() {
   switch (state_) {
     case kUninitialized:
     case kInitializing:
-    case kFlushed:
     case kFlushing:
+    case kFlushed:
     case kStopped:
       return false;
 
-    case kPrerolling:
     case kPlaying:
-    case kUnderflow:
-    case kRebuffering:
       break;
   }
 
@@ -569,11 +530,11 @@ void AudioRendererImpl::SetPlaybackRate(float playback_rate) {
   }
 }
 
-bool AudioRendererImpl::IsBeforePrerollTime(
+bool AudioRendererImpl::IsBeforeStartTime(
     const scoped_refptr<AudioBuffer>& buffer) {
-  DCHECK_EQ(state_, kPrerolling);
+  DCHECK_EQ(state_, kPlaying);
   return buffer && !buffer->end_of_stream() &&
-         (buffer->timestamp() + buffer->duration()) < preroll_timestamp_;
+         (buffer->timestamp() + buffer->duration()) < start_timestamp_;
 }
 
 int AudioRendererImpl::Render(AudioBus* audio_bus,
@@ -585,7 +546,6 @@ int AudioRendererImpl::Render(AudioBus* audio_bus,
                                             audio_parameters_.sample_rate());
   int frames_written = 0;
   base::Closure time_cb;
-  base::Closure underflow_cb;
   {
     base::AutoLock auto_lock(lock_);
 
@@ -639,8 +599,11 @@ int AudioRendererImpl::Render(AudioBus* audio_bus,
         rendered_end_of_stream_ = true;
         ended_cb_.Run();
       } else if (!received_end_of_stream_ && state_ == kPlaying) {
-        ChangeState_Locked(kUnderflow);
-        underflow_cb = underflow_cb_;
+        if (buffering_state_ != BUFFERING_HAVE_NOTHING) {
+          algorithm_->IncreaseQueueCapacity();
+          buffering_state_ = BUFFERING_HAVE_NOTHING;
+          buffering_state_cb_.Run(BUFFERING_HAVE_NOTHING);
+        }
       } else {
         // We can't write any data this cycle. For example, we may have
         // sent all available data to the audio device while not reaching
@@ -673,9 +636,6 @@ int AudioRendererImpl::Render(AudioBus* audio_bus,
   if (!time_cb.is_null())
     time_cb.Run();
 
-  if (!underflow_cb.is_null())
-    underflow_cb.Run();
-
   DCHECK_LE(frames_written, requested_frames);
   return frames_written;
 }
@@ -714,7 +674,6 @@ void AudioRendererImpl::HandleAbortedReadOrDecodeError(bool is_decode_error) {
       return;
     case kFlushing:
       ChangeState_Locked(kFlushed);
-
       if (status == PIPELINE_OK) {
         DoFlush_Locked();
         return;
@@ -723,16 +682,9 @@ void AudioRendererImpl::HandleAbortedReadOrDecodeError(bool is_decode_error) {
       error_cb_.Run(status);
       base::ResetAndReturn(&flush_cb_).Run();
       return;
-    case kPrerolling:
-      // This is a signal for abort if it's not an error.
-      preroll_aborted_ = !is_decode_error;
-      ChangeState_Locked(kPlaying);
-      base::ResetAndReturn(&preroll_cb_).Run(status);
-      return;
+
     case kFlushed:
     case kPlaying:
-    case kUnderflow:
-    case kRebuffering:
     case kStopped:
       if (status != PIPELINE_OK)
         error_cb_.Run(status);