Update AudioRenderer API to fire changes in BufferingState.

media/base/pipeline.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/pipeline.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/location.h"
 #include "base/metrics/histogram.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/synchronization/condition_variable.h"
 #include "media/base/audio_decoder.h"
 #include "media/base/audio_renderer.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/base/clock.h"
 #include "media/base/filter_collection.h"
 #include "media/base/media_log.h"
 #include "media/base/text_renderer.h"
 #include "media/base/text_track_config.h"
 #include "media/base/video_decoder.h"
 #include "media/base/video_decoder_config.h"
 #include "media/base/video_renderer.h"
 
 using base::TimeDelta;
@@ skipping 373 matching lines @@
           metadata.natural_size = stream->video_decoder_config().natural_size();
         metadata_cb_.Run(metadata);
       }
 
       return DoInitialPreroll(done_cb);
 
     case kPlaying:
       PlaybackRateChangedTask(GetPlaybackRate());
       VolumeChangedTask(GetVolume());
 
+      // Handle renderers that immediately signal they have enough data.
+      if (!WaitingForEnoughData())
+        TransitionToNonWaiting();
+
       // We enter this state from either kInitPrerolling or kSeeking. As of now
       // both those states call Preroll(), which means by time we enter this
       // state we've already buffered enough data. Forcefully update the
       // buffering state, which start the clock and renderers and transition
       // into kPlaying state.
       //
       // TODO(scherkus): Remove after renderers are taught to fire buffering
       // state callbacks http://crbug.com/144683
-      DCHECK(WaitingForEnoughData());
-      if (audio_renderer_)
-        BufferingStateChanged(&audio_buffering_state_, BUFFERING_HAVE_ENOUGH);
-      if (video_renderer_)
+      if (video_renderer_) {
+        DCHECK(WaitingForEnoughData());
         BufferingStateChanged(&video_buffering_state_, BUFFERING_HAVE_ENOUGH);
+      }
       return;
 
     case kStopping:
     case kStopped:
     case kCreated:
     case kSeeking:
       NOTREACHED() << "State has no transition: " << state_;
       return;
   }
 }
 
 // Note that the usage of base::Unretained() with the audio/video renderers
 // in the following DoXXX() functions is considered safe as they are owned by
 // |pending_callbacks_| and share the same lifetime.
 //
 // That being said, deleting the renderers while keeping |pending_callbacks_|
 // running on the media thread would result in crashes.
 void Pipeline::DoInitialPreroll(const PipelineStatusCB& done_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!pending_callbacks_.get());
   SerialRunner::Queue bound_fns;
 
   base::TimeDelta seek_timestamp = demuxer_->GetStartTime();
 
   // Preroll renderers.
   if (audio_renderer_) {
-    bound_fns.Push(base::Bind(
-        &AudioRenderer::Preroll, base::Unretained(audio_renderer_.get()),
-        seek_timestamp));
+    bound_fns.Push(base::Bind(&AudioRenderer::StartPlayingFrom,
+                              base::Unretained(audio_renderer_.get()),
+                              seek_timestamp));
   }
 
   if (video_renderer_) {
     bound_fns.Push(base::Bind(
         &VideoRenderer::Preroll, base::Unretained(video_renderer_.get()),
         seek_timestamp));
 
     // TODO(scherkus): Remove after VideoRenderer is taught to fire buffering
     // state callbacks http://crbug.com/144683
     bound_fns.Push(base::Bind(&VideoRenderer::Play,
@@ skipping 48 matching lines @@
     bound_fns.Push(base::Bind(
         &TextRenderer::Flush, base::Unretained(text_renderer_.get())));
   }
 
   // Seek demuxer.
   bound_fns.Push(base::Bind(
       &Demuxer::Seek, base::Unretained(demuxer_), seek_timestamp));
 
   // Preroll renderers.
   if (audio_renderer_) {
-    bound_fns.Push(base::Bind(
-        &AudioRenderer::Preroll, base::Unretained(audio_renderer_.get()),
-        seek_timestamp));
+    bound_fns.Push(base::Bind(&AudioRenderer::StartPlayingFrom,
+                              base::Unretained(audio_renderer_.get()),
+                              seek_timestamp));
   }
 
   if (video_renderer_) {
     bound_fns.Push(base::Bind(
         &VideoRenderer::Preroll, base::Unretained(video_renderer_.get()),
         seek_timestamp));
 
     // TODO(scherkus): Remove after renderers are taught to fire buffering
     // state callbacks http://crbug.com/144683
     bound_fns.Push(base::Bind(&VideoRenderer::Play,
@@ skipping 312 matching lines @@
 }
 
 void Pipeline::InitializeAudioRenderer(const PipelineStatusCB& done_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   audio_renderer_ = filter_collection_->GetAudioRenderer();
   audio_renderer_->Initialize(
       demuxer_->GetStream(DemuxerStream::AUDIO),
       done_cb,
       base::Bind(&Pipeline::OnUpdateStatistics, base::Unretained(this)),
-      base::Bind(&Pipeline::OnAudioUnderflow, base::Unretained(this)),
       base::Bind(&Pipeline::OnAudioTimeUpdate, base::Unretained(this)),
+      // TODO(scherkus): Enforce AudioRendererImpl to call on right thread.
+      BindToCurrentLoop(base::Bind(&Pipeline::BufferingStateChanged,

[acolwell GONE FROM CHROMIUM, 2014/05/13 20:44:27]

nit: Shouldn't we hide this wrapping in the AudioRendererImpl? That way the
followup CL only has to include renderer changes.

[scherkus (not reviewing), 2014/05/15 23:28:43]

Done.

+                                   base::Unretained(this),
+                                   &audio_buffering_state_)),
       base::Bind(&Pipeline::OnAudioRendererEnded, base::Unretained(this)),
       base::Bind(&Pipeline::SetError, base::Unretained(this)));
 }
 
 void Pipeline::InitializeVideoRenderer(const PipelineStatusCB& done_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   video_renderer_ = filter_collection_->GetVideoRenderer();
   video_renderer_->Initialize(
       demuxer_->GetStream(DemuxerStream::VIDEO),
       demuxer_->GetLiveness() == Demuxer::LIVENESS_LIVE,
       done_cb,
       base::Bind(&Pipeline::OnUpdateStatistics, base::Unretained(this)),
       base::Bind(&Pipeline::OnVideoTimeUpdate, base::Unretained(this)),
       base::Bind(&Pipeline::OnVideoRendererEnded, base::Unretained(this)),
       base::Bind(&Pipeline::SetError, base::Unretained(this)),
       base::Bind(&Pipeline::GetMediaTime, base::Unretained(this)),
       base::Bind(&Pipeline::GetMediaDuration, base::Unretained(this)));
 }
 
-void Pipeline::OnAudioUnderflow() {
-  if (!task_runner_->BelongsToCurrentThread()) {

[acolwell GONE FROM CHROMIUM, 2014/05/13 20:44:27]

\o/ I'm happy to see this method die. It lived WAY longer than I had originally
intended.

-    task_runner_->PostTask(FROM_HERE, base::Bind(
-        &Pipeline::OnAudioUnderflow, base::Unretained(this)));
-    return;
-  }
-
-  if (state_ != kPlaying)
-    return;
-
-  if (audio_renderer_)
-    audio_renderer_->ResumeAfterUnderflow();
-}
-
 void Pipeline::BufferingStateChanged(BufferingState* buffering_state,
                                      BufferingState new_buffering_state) {
   DVLOG(1) << __FUNCTION__ << "(" << *buffering_state << ", "
            << " " << new_buffering_state << ") "
            << (buffering_state == &audio_buffering_state_ ? "audio" : "video");
   DCHECK(task_runner_->BelongsToCurrentThread());
   bool was_waiting_for_enough_data = WaitingForEnoughData();
   *buffering_state = new_buffering_state;
 
   // Renderer underflowed.
   if (!was_waiting_for_enough_data && WaitingForEnoughData()) {
-    StartWaitingForEnoughData();
+    TransitionToWaiting();
     return;
   }
 
   // Renderer prerolled.
   if (was_waiting_for_enough_data && !WaitingForEnoughData()) {
-    StartPlayback();
+    TransitionToNonWaiting();
     return;
   }
 }
 
 bool Pipeline::WaitingForEnoughData() const {
   DCHECK(task_runner_->BelongsToCurrentThread());
   if (state_ != kPlaying)
     return false;
   if (audio_renderer_ && audio_buffering_state_ != BUFFERING_HAVE_ENOUGH)
     return true;
   if (video_renderer_ && video_buffering_state_ != BUFFERING_HAVE_ENOUGH)
     return true;
   return false;
 }
 
-void Pipeline::StartWaitingForEnoughData() {
+void Pipeline::TransitionToWaiting() {
   DVLOG(1) << __FUNCTION__;
   DCHECK_EQ(state_, kPlaying);
   DCHECK(WaitingForEnoughData());
+  task_runner_->PostTask(
+      FROM_HERE, base::Bind(&Pipeline::PausePlayback, base::Unretained(this)));
+}
+
+void Pipeline::TransitionToNonWaiting() {
+  DVLOG(1) << __FUNCTION__;
+  DCHECK_EQ(state_, kPlaying);
+  DCHECK(!WaitingForEnoughData());
+  task_runner_->PostTask(

[acolwell GONE FROM CHROMIUM, 2014/05/13 20:44:27]

Why do you need to PostTask() here and above now? Can't this open the door for
WaitingForEnoughData() to change state before StartPlayback() is called? 

[scherkus (not reviewing), 2014/05/15 23:28:43]

it's to avoid reentrancy into renderers ... but I think this was before I did
the BindToCurrentLoop() so this shouldn't be necessary anymore

+      FROM_HERE, base::Bind(&Pipeline::StartPlayback, base::Unretained(this)));
+}
+
+void Pipeline::PausePlayback() {
+  DVLOG(1) << __FUNCTION__;
+  DCHECK(task_runner_->BelongsToCurrentThread());
 
   if (audio_renderer_)
     audio_renderer_->StopRendering();
 
   base::AutoLock auto_lock(lock_);
   clock_->Pause();
 }
 
 void Pipeline::StartPlayback() {
   DVLOG(1) << __FUNCTION__;
-  DCHECK_EQ(state_, kPlaying);
-  DCHECK(!WaitingForEnoughData());
+  DCHECK(task_runner_->BelongsToCurrentThread());
 
   if (audio_renderer_) {
     // We use audio stream to update the clock. So if there is such a
     // stream, we pause the clock until we receive a valid timestamp.
     base::AutoLock auto_lock(lock_);
     waiting_for_clock_update_ = true;
     audio_renderer_->StartRendering();
   } else {
     base::AutoLock auto_lock(lock_);
     DCHECK(!waiting_for_clock_update_);
     clock_->SetMaxTime(clock_->Duration());
     clock_->Play();
   }
 
   preroll_completed_cb_.Run();
   if (!seek_cb_.is_null())
     base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
 }
 
 void Pipeline::StartClockIfWaitingForTimeUpdate_Locked() {
   lock_.AssertAcquired();
   if (!waiting_for_clock_update_)
     return;
 
   waiting_for_clock_update_ = false;
   clock_->Play();
 }
 
 }  // namespace media