Introduce buffering state change callback to Pipeline.

This is the next step after r269828 to communicate changes in buffering
state to clients. The most notable difference is that a seek completion
callback no longer indicates that prerolling has also completed. Instead
clients must listen for a buffering state change callback.

BUG=144683

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

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

10 files changed, 77 insertions, 54 deletions

diff --git a/content/renderer/media/webmediaplayer_impl.cc b/content/renderer/media/webmediaplayer_impl.cc
index 9e22d4f..a6b0fd9 100644
--- a/content/renderer/media/webmediaplayer_impl.cc
+++ b/content/renderer/media/webmediaplayer_impl.cc
@@ -165,6 +165,7 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       playback_rate_(0.0f),
       pending_seek_(false),
       pending_seek_seconds_(0.0f),
+      should_notify_time_changed_(false),
       client_(client),
       delegate_(delegate),
       defer_load_cb_(params.defer_load_cb()),
@@ -922,9 +923,7 @@ void WebMediaPlayerImpl::OnPipelineSeeked(bool time_changed,
   if (paused_)
     paused_time_ = pipeline_.GetMediaTime();
 
-  // Blink expects a timeChanged() in response to a seek().
-  if (time_changed)
-    client_->timeChanged();
+  should_notify_time_changed_ = time_changed;
 }
 
 void WebMediaPlayerImpl::OnPipelineEnded() {
@@ -979,19 +978,26 @@ void WebMediaPlayerImpl::OnPipelineMetadata(
   InvalidateOnMainThread();
 }
 
-void WebMediaPlayerImpl::OnPipelinePrerollCompleted() {
-  DVLOG(1) << __FUNCTION__;
+void WebMediaPlayerImpl::OnPipelineBufferingStateChanged(
+    media::BufferingState buffering_state) {
+  DVLOG(1) << __FUNCTION__ << "(" << buffering_state << ")";
 
-  // Only transition to ReadyStateHaveEnoughData if we don't have
-  // any pending seeks because the transition can cause Blink to
-  // report that the most recent seek has completed.
-  if (!pending_seek_) {
-    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+  // Ignore buffering state changes until we've completed all outstanding seeks.
+  if (seeking_ || pending_seek_)
+    return;
 
-    // TODO(scherkus): This should be handled by HTMLMediaElement and controls
-    // should know when to invalidate themselves http://crbug.com/337015
-    InvalidateOnMainThread();
-  }
+  // TODO(scherkus): Handle other buffering states when Pipeline starts using
+  // them and translate them ready state changes http://crbug.com/144683
+  DCHECK_EQ(buffering_state, media::BUFFERING_HAVE_ENOUGH);
+  SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+
+  // Blink expects a timeChanged() in response to a seek().
+  if (should_notify_time_changed_)
+    client_->timeChanged();
+
+  // TODO(scherkus): This should be handled by HTMLMediaElement and controls
+  // should know when to invalidate themselves http://crbug.com/337015
+  InvalidateOnMainThread();
 }
 
 void WebMediaPlayerImpl::OnDemuxerOpened() {
@@ -1211,7 +1217,7 @@ void WebMediaPlayerImpl::StartPipeline() {
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineError),
       BIND_TO_RENDER_LOOP1(&WebMediaPlayerImpl::OnPipelineSeeked, false),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineMetadata),
-      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelinePrerollCompleted),
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineBufferingStateChanged),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnDurationChanged));
 }
 
diff --git a/content/renderer/media/webmediaplayer_impl.h b/content/renderer/media/webmediaplayer_impl.h
index aa9d40d..d8e83ff 100644
--- a/content/renderer/media/webmediaplayer_impl.h
+++ b/content/renderer/media/webmediaplayer_impl.h
@@ -167,7 +167,7 @@ class WebMediaPlayerImpl
   void OnPipelineEnded();
   void OnPipelineError(media::PipelineStatus error);
   void OnPipelineMetadata(media::PipelineMetadata metadata);
-  void OnPipelinePrerollCompleted();
+  void OnPipelineBufferingStateChanged(media::BufferingState buffering_state);
   void OnDemuxerOpened();
   void OnKeyAdded(const std::string& session_id);
   void OnKeyError(const std::string& session_id,
@@ -290,6 +290,10 @@ class WebMediaPlayerImpl
   bool pending_seek_;
   double pending_seek_seconds_;
 
+  // Tracks whether to issue time changed notifications during buffering state
+  // changes.
+  bool should_notify_time_changed_;
+
   blink::WebMediaPlayerClient* client_;
 
   base::WeakPtr<WebMediaPlayerDelegate> delegate_;
diff --git a/media/base/buffering_state.h b/media/base/buffering_state.h
index 3140505..935922c 100644
--- a/media/base/buffering_state.h
+++ b/media/base/buffering_state.h
@@ -21,6 +21,9 @@ enum BufferingState {
   BUFFERING_HAVE_ENOUGH,
 };
 
+// Used to indicate changes in buffering state;
+typedef base::Callback<void(BufferingState)> BufferingStateCB;
+
 }  // namespace media
 
 #endif  // MEDIA_BASE_BUFFERING_STATE_H_
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index ca015a7..792fa03 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -72,13 +72,13 @@ void Pipeline::Start(scoped_ptr<FilterCollection> collection,
                      const PipelineStatusCB& error_cb,
                      const PipelineStatusCB& seek_cb,
                      const PipelineMetadataCB& metadata_cb,
-                     const base::Closure& preroll_completed_cb,
+                     const BufferingStateCB& buffering_state_cb,
                      const base::Closure& duration_change_cb) {
   DCHECK(!ended_cb.is_null());
   DCHECK(!error_cb.is_null());
   DCHECK(!seek_cb.is_null());
   DCHECK(!metadata_cb.is_null());
-  DCHECK(!preroll_completed_cb.is_null());
+  DCHECK(!buffering_state_cb.is_null());
 
   base::AutoLock auto_lock(lock_);
   CHECK(!running_) << "Media pipeline is already running";
@@ -89,7 +89,7 @@ void Pipeline::Start(scoped_ptr<FilterCollection> collection,
   error_cb_ = error_cb;
   seek_cb_ = seek_cb;
   metadata_cb_ = metadata_cb;
-  preroll_completed_cb_ = preroll_completed_cb;
+  buffering_state_cb_ = buffering_state_cb;
   duration_change_cb_ = duration_change_cb;
 
   task_runner_->PostTask(
@@ -399,6 +399,8 @@ void Pipeline::StateTransitionTask(PipelineStatus status) {
       return DoInitialPreroll(done_cb);
 
     case kPlaying:
+      base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
+
       PlaybackRateChangedTask(GetPlaybackRate());
       VolumeChangedTask(GetVolume());
 
@@ -891,12 +893,16 @@ void Pipeline::BufferingStateChanged(BufferingState* buffering_state,
   // Renderer underflowed.
   if (!was_waiting_for_enough_data && WaitingForEnoughData()) {
     StartWaitingForEnoughData();
+
+    // TODO(scherkus): Fire BUFFERING_HAVE_NOTHING callback to alert clients of
+    // underflow state http://crbug.com/144683
     return;
   }
 
   // Renderer prerolled.
   if (was_waiting_for_enough_data && !WaitingForEnoughData()) {
     StartPlayback();
+    buffering_state_cb_.Run(BUFFERING_HAVE_ENOUGH);
     return;
   }
 }
@@ -939,10 +945,6 @@ void Pipeline::StartPlayback() {
     clock_->SetMaxTime(clock_->Duration());
     clock_->Play();
   }
-
-  preroll_completed_cb_.Run();
-  if (!seek_cb_.is_null())
-    base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
 }
 
 void Pipeline::PauseClockAndStopRendering_Locked() {
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index dab0255..b5bba8d 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -85,7 +85,7 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost {
   virtual ~Pipeline();
 
   // Build a pipeline to using the given filter collection to construct a filter
-  // chain, executing |seek_cb| when the initial seek/preroll has completed.
+  // chain, executing |seek_cb| when the initial seek has completed.
   //
   // |filter_collection| must be a complete collection containing a demuxer,
   // audio/video decoders, and audio/video renderers. Failing to do so will
@@ -99,9 +99,8 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost {
   //   |metadata_cb| will be executed when the content duration, container video
   //                 size, start time, and whether the content has audio and/or
   //                 video in supported formats are known.
-  //   |preroll_completed_cb| will be executed when all renderers have buffered
-  //                          enough data to satisfy preroll and are ready to
-  //                          start playback.
+  //   |buffering_state_cb| will be executed whenever there are changes in the
+  //                        overall buffering state of the pipeline.
   //   |duration_change_cb| optional callback that will be executed whenever the
   //                        presentation duration changes.
   // It is an error to call this method after the pipeline has already started.
@@ -110,7 +109,7 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost {
              const PipelineStatusCB& error_cb,
              const PipelineStatusCB& seek_cb,
              const PipelineMetadataCB& metadata_cb,
-             const base::Closure& preroll_completed_cb,
+             const BufferingStateCB& buffering_state_cb,
              const base::Closure& duration_change_cb);
 
   // Asynchronously stops the pipeline, executing |stop_cb| when the pipeline
@@ -409,7 +408,7 @@ class MEDIA_EXPORT Pipeline : public DemuxerHost {
   base::Closure ended_cb_;
   PipelineStatusCB error_cb_;
   PipelineMetadataCB metadata_cb_;
-  base::Closure preroll_completed_cb_;
+  BufferingStateCB buffering_state_cb_;
   base::Closure duration_change_cb_;
 
   // Contains the demuxer and renderers to use when initializing.
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index 05ffc8c..a877143 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -64,7 +64,7 @@ class CallbackHelper {
   MOCK_METHOD0(OnEnded, void());
   MOCK_METHOD1(OnError, void(PipelineStatus));
   MOCK_METHOD1(OnMetadata, void(PipelineMetadata));
-  MOCK_METHOD0(OnPrerollCompleted, void());
+  MOCK_METHOD1(OnBufferingStateChange, void(BufferingState));
   MOCK_METHOD0(OnDurationChange, void());
 
  private:
@@ -208,7 +208,7 @@ class PipelineTest : public ::testing::Test {
             .WillOnce(RunCallback<1>(PIPELINE_OK));
         EXPECT_CALL(*audio_renderer_, StartRendering());
       }
-      EXPECT_CALL(callbacks_, OnPrerollCompleted());
+      EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
     }
 
     pipeline_->Start(
@@ -217,7 +217,7 @@ class PipelineTest : public ::testing::Test {
         base::Bind(&CallbackHelper::OnError, base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnStart, base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnMetadata, base::Unretained(&callbacks_)),
-        base::Bind(&CallbackHelper::OnPrerollCompleted,
+        base::Bind(&CallbackHelper::OnBufferingStateChange,
                    base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnDurationChange,
                    base::Unretained(&callbacks_)));
@@ -277,10 +277,9 @@ class PipelineTest : public ::testing::Test {
           .WillOnce(RunClosure<0>());
     }
 
-    EXPECT_CALL(callbacks_, OnPrerollCompleted());
-
-    // We expect a successful seek callback.
+    // We expect a successful seek callback followed by a buffering update.
     EXPECT_CALL(callbacks_, OnSeek(PIPELINE_OK));
+    EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
   }
 
   void DoSeek(const base::TimeDelta& seek_time) {
@@ -376,7 +375,7 @@ TEST_F(PipelineTest, NeverInitializes) {
         base::Bind(&CallbackHelper::OnError, base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnStart, base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnMetadata, base::Unretained(&callbacks_)),
-        base::Bind(&CallbackHelper::OnPrerollCompleted,
+        base::Bind(&CallbackHelper::OnBufferingStateChange,
                    base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnDurationChange,
                    base::Unretained(&callbacks_)));
@@ -781,8 +780,8 @@ TEST_F(PipelineTest, AudioTimeUpdateDuringSeek) {
   EXPECT_CALL(*audio_renderer_, SetVolume(_));
   EXPECT_CALL(*audio_renderer_, StartRendering());
 
-  EXPECT_CALL(callbacks_, OnPrerollCompleted());
   EXPECT_CALL(callbacks_, OnSeek(PIPELINE_OK));
+  EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
   DoSeek(seek_time);
 
   EXPECT_EQ(pipeline_->GetMediaTime(), seek_time);
@@ -872,7 +871,7 @@ class PipelineTeardownTest : public PipelineTest {
         base::Bind(&CallbackHelper::OnError, base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnStart, base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnMetadata, base::Unretained(&callbacks_)),
-        base::Bind(&CallbackHelper::OnPrerollCompleted,
+        base::Bind(&CallbackHelper::OnBufferingStateChange,
                    base::Unretained(&callbacks_)),
         base::Bind(&CallbackHelper::OnDurationChange,
                    base::Unretained(&callbacks_)));
@@ -966,7 +965,7 @@ class PipelineTeardownTest : public PipelineTest {
         .WillOnce(RunClosure<0>());
 
     if (status == PIPELINE_OK)
-      EXPECT_CALL(callbacks_, OnPrerollCompleted());
+      EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
 
     return status;
   }
diff --git a/media/filters/pipeline_integration_test.cc b/media/filters/pipeline_integration_test.cc
index f991dc3..3fac5ca 100644
--- a/media/filters/pipeline_integration_test.cc
+++ b/media/filters/pipeline_integration_test.cc
@@ -535,9 +535,11 @@ class PipelineIntegrationTest
       public PipelineIntegrationTestBase {
  public:
   void StartPipelineWithMediaSource(MockMediaSource* source) {
-    EXPECT_CALL(*this, OnMetadata(_)).Times(AtMost(1))
+    EXPECT_CALL(*this, OnMetadata(_))
+        .Times(AtMost(1))
         .WillRepeatedly(SaveArg<0>(&metadata_));
-    EXPECT_CALL(*this, OnPrerollCompleted()).Times(AtMost(1));
+    EXPECT_CALL(*this, OnBufferingStateChanged(BUFFERING_HAVE_ENOUGH))
+        .Times(AtMost(1));
     pipeline_->Start(
         CreateFilterCollection(source->GetDemuxer(), NULL),
         base::Bind(&PipelineIntegrationTest::OnEnded, base::Unretained(this)),
@@ -545,7 +547,7 @@ class PipelineIntegrationTest
         QuitOnStatusCB(PIPELINE_OK),
         base::Bind(&PipelineIntegrationTest::OnMetadata,
                    base::Unretained(this)),
-        base::Bind(&PipelineIntegrationTest::OnPrerollCompleted,
+        base::Bind(&PipelineIntegrationTest::OnBufferingStateChanged,
                    base::Unretained(this)),
         base::Closure());
 
@@ -560,9 +562,11 @@ class PipelineIntegrationTest
   void StartPipelineWithEncryptedMedia(
       MockMediaSource* source,
       FakeEncryptedMedia* encrypted_media) {
-    EXPECT_CALL(*this, OnMetadata(_)).Times(AtMost(1))
+    EXPECT_CALL(*this, OnMetadata(_))
+        .Times(AtMost(1))
         .WillRepeatedly(SaveArg<0>(&metadata_));
-    EXPECT_CALL(*this, OnPrerollCompleted()).Times(AtMost(1));
+    EXPECT_CALL(*this, OnBufferingStateChanged(BUFFERING_HAVE_ENOUGH))
+        .Times(AtMost(1));
     pipeline_->Start(
         CreateFilterCollection(source->GetDemuxer(),
                                encrypted_media->decryptor()),
@@ -571,7 +575,7 @@ class PipelineIntegrationTest
         QuitOnStatusCB(PIPELINE_OK),
         base::Bind(&PipelineIntegrationTest::OnMetadata,
                    base::Unretained(this)),
-        base::Bind(&PipelineIntegrationTest::OnPrerollCompleted,
+        base::Bind(&PipelineIntegrationTest::OnBufferingStateChanged,
                    base::Unretained(this)),
         base::Closure());
 
diff --git a/media/filters/pipeline_integration_test_base.cc b/media/filters/pipeline_integration_test_base.cc
index c179903..3dae6a5 100644
--- a/media/filters/pipeline_integration_test_base.cc
+++ b/media/filters/pipeline_integration_test_base.cc
@@ -104,9 +104,11 @@ void PipelineIntegrationTestBase::OnError(PipelineStatus status) {
 
 bool PipelineIntegrationTestBase::Start(const base::FilePath& file_path,
                                         PipelineStatus expected_status) {
-  EXPECT_CALL(*this, OnMetadata(_)).Times(AtMost(1))
+  EXPECT_CALL(*this, OnMetadata(_))
+      .Times(AtMost(1))
       .WillRepeatedly(SaveArg<0>(&metadata_));
-  EXPECT_CALL(*this, OnPrerollCompleted()).Times(AtMost(1));
+  EXPECT_CALL(*this, OnBufferingStateChanged(BUFFERING_HAVE_ENOUGH))
+      .Times(AtMost(1));
   pipeline_->Start(
       CreateFilterCollection(file_path, NULL),
       base::Bind(&PipelineIntegrationTestBase::OnEnded, base::Unretained(this)),
@@ -114,7 +116,7 @@ bool PipelineIntegrationTestBase::Start(const base::FilePath& file_path,
       QuitOnStatusCB(expected_status),
       base::Bind(&PipelineIntegrationTestBase::OnMetadata,
                  base::Unretained(this)),
-      base::Bind(&PipelineIntegrationTestBase::OnPrerollCompleted,
+      base::Bind(&PipelineIntegrationTestBase::OnBufferingStateChanged,
                  base::Unretained(this)),
       base::Closure());
   message_loop_.Run();
@@ -138,9 +140,11 @@ bool PipelineIntegrationTestBase::Start(const base::FilePath& file_path) {
 
 bool PipelineIntegrationTestBase::Start(const base::FilePath& file_path,
                                         Decryptor* decryptor) {
-  EXPECT_CALL(*this, OnMetadata(_)).Times(AtMost(1))
+  EXPECT_CALL(*this, OnMetadata(_))
+      .Times(AtMost(1))
       .WillRepeatedly(SaveArg<0>(&metadata_));
-  EXPECT_CALL(*this, OnPrerollCompleted()).Times(AtMost(1));
+  EXPECT_CALL(*this, OnBufferingStateChanged(BUFFERING_HAVE_ENOUGH))
+      .Times(AtMost(1));
   pipeline_->Start(
       CreateFilterCollection(file_path, decryptor),
       base::Bind(&PipelineIntegrationTestBase::OnEnded, base::Unretained(this)),
@@ -149,7 +153,7 @@ bool PipelineIntegrationTestBase::Start(const base::FilePath& file_path,
                  base::Unretained(this)),
       base::Bind(&PipelineIntegrationTestBase::OnMetadata,
                  base::Unretained(this)),
-      base::Bind(&PipelineIntegrationTestBase::OnPrerollCompleted,
+      base::Bind(&PipelineIntegrationTestBase::OnBufferingStateChanged,
                  base::Unretained(this)),
       base::Closure());
   message_loop_.Run();
@@ -167,7 +171,7 @@ void PipelineIntegrationTestBase::Pause() {
 bool PipelineIntegrationTestBase::Seek(base::TimeDelta seek_time) {
   ended_ = false;
 
-  EXPECT_CALL(*this, OnPrerollCompleted());
+  EXPECT_CALL(*this, OnBufferingStateChanged(BUFFERING_HAVE_ENOUGH));
   pipeline_->Seek(seek_time, QuitOnStatusCB(PIPELINE_OK));
   message_loop_.Run();
   return (pipeline_status_ == PIPELINE_OK);
diff --git a/media/filters/pipeline_integration_test_base.h b/media/filters/pipeline_integration_test_base.h
index 10cf262..37f744c 100644
--- a/media/filters/pipeline_integration_test_base.h
+++ b/media/filters/pipeline_integration_test_base.h
@@ -134,7 +134,7 @@ class PipelineIntegrationTestBase {
   void OnVideoRendererPaint(const scoped_refptr<VideoFrame>& frame);
 
   MOCK_METHOD1(OnMetadata, void(PipelineMetadata));
-  MOCK_METHOD0(OnPrerollCompleted, void());
+  MOCK_METHOD1(OnBufferingStateChanged, void(BufferingState));
 };
 
 }  // namespace media
diff --git a/media/tools/player_x11/player_x11.cc b/media/tools/player_x11/player_x11.cc
index b691743d..94aac8a 100644
--- a/media/tools/player_x11/player_x11.cc
+++ b/media/tools/player_x11/player_x11.cc
@@ -86,6 +86,8 @@ static void OnStatus(media::PipelineStatus status) {}
 
 static void OnMetadata(media::PipelineMetadata metadata) {}
 
+static void OnBufferingStateChanged(media::BufferingState buffering_state) {}
+
 static void NeedKey(const std::string& type,
                     const std::vector<uint8>& init_data) {
   std::cout << "File is encrypted." << std::endl;
@@ -146,7 +148,7 @@ void InitPipeline(
   pipeline->Start(
       collection.Pass(), base::Bind(&DoNothing), base::Bind(&OnStatus),
       base::Bind(&SaveStatusAndSignal, &event, &status),
-      base::Bind(&OnMetadata), base::Bind(&DoNothing),
+      base::Bind(&OnMetadata), base::Bind(&OnBufferingStateChanged),
       base::Bind(&DoNothing));
 
   // Wait until the pipeline is fully initialized.