Switch back to using VideoRenderer::StartPlayingFrom().

Turns out dropping the time argument in 08602875 doesn't work well in a
video frame scheduling based world as a VideoRenderer is no longer
able to poll for the current time. Instead, go back to passing in the
start timestamp so that VideoRenderer implementations can do frame
accurate seeking.

BUG=110814,370634

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

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

media/base/mock_filters.h

@@ -127,7 +127,7 @@ class MockVideoRenderer : public VideoRenderer {
                                 const PipelineStatusCB& error_cb,
                                 const TimeDeltaCB& get_time_cb));
   MOCK_METHOD1(Flush, void(const base::Closure& callback));
-  MOCK_METHOD0(StartPlaying, void());
+  MOCK_METHOD1(StartPlayingFrom, void(base::TimeDelta));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockVideoRenderer);

media/base/video_renderer.h

@@ -57,10 +57,11 @@ class MEDIA_EXPORT VideoRenderer {
   // BUFFERING_HAVE_NOTHING while flushing is in progress.
   virtual void Flush(const base::Closure& callback) = 0;
 
-  // Starts playback by reading from |stream| and decoding and rendering video.
+  // Starts playback at |timestamp| by reading from |stream| and decoding and
+  // rendering video.
   //
   // Only valid to call after a successful Initialize() or Flush().
-  virtual void StartPlaying() = 0;
+  virtual void StartPlayingFrom(base::TimeDelta timestamp) = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(VideoRenderer);

media/filters/renderer_impl.cc

@@ -104,7 +104,7 @@ void RendererImpl::StartPlayingFrom(base::TimeDelta time) {
   if (audio_renderer_)
     audio_renderer_->StartPlaying();
   if (video_renderer_)
-    video_renderer_->StartPlaying();
+    video_renderer_->StartPlayingFrom(time);
 }
 
 void RendererImpl::SetPlaybackRate(float playback_rate) {

media/filters/renderer_impl_unittest.cc

@@ -172,10 +172,11 @@ class RendererImplTest : public ::testing::Test {
     DCHECK(audio_stream_ || video_stream_);
     EXPECT_CALL(callbacks_, OnBufferingStateChange(BUFFERING_HAVE_ENOUGH));
 
+    base::TimeDelta start_time(
+        base::TimeDelta::FromMilliseconds(kStartPlayingTimeInMs));
+
     if (audio_stream_) {
-      EXPECT_CALL(time_source_,
-                  SetMediaTime(base::TimeDelta::FromMilliseconds(
-                      kStartPlayingTimeInMs)));
+      EXPECT_CALL(time_source_, SetMediaTime(start_time));
       EXPECT_CALL(time_source_, StartTicking());
       EXPECT_CALL(*audio_renderer_, StartPlaying())
           .WillOnce(SetBufferingState(&audio_buffering_state_cb_,
@@ -183,13 +184,12 @@ class RendererImplTest : public ::testing::Test {
     }
 
     if (video_stream_) {
-      EXPECT_CALL(*video_renderer_, StartPlaying())
+      EXPECT_CALL(*video_renderer_, StartPlayingFrom(start_time))
           .WillOnce(SetBufferingState(&video_buffering_state_cb_,
                                       BUFFERING_HAVE_ENOUGH));
     }
 
-    renderer_impl_->StartPlayingFrom(
-        base::TimeDelta::FromMilliseconds(kStartPlayingTimeInMs));
+    renderer_impl_->StartPlayingFrom(start_time);
     base::RunLoop().RunUntilIdle();
   }
 

media/filters/video_renderer_impl.cc

@@ -92,8 +92,8 @@ void VideoRendererImpl::Flush(const base::Closure& callback) {
                  weak_factory_.GetWeakPtr()));
 }
 
-void VideoRendererImpl::StartPlaying() {
-  DVLOG(1) << __FUNCTION__;
+void VideoRendererImpl::StartPlayingFrom(base::TimeDelta timestamp) {
+  DVLOG(1) << __FUNCTION__ << "(" << timestamp.InMicroseconds() << ")";
   DCHECK(task_runner_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
   DCHECK_EQ(state_, kFlushed);
@@ -102,7 +102,7 @@ void VideoRendererImpl::StartPlaying() {
   DCHECK_EQ(buffering_state_, BUFFERING_HAVE_NOTHING);
 
   state_ = kPlaying;
-  start_timestamp_ = get_time_cb_.Run();
+  start_timestamp_ = timestamp;
   AttemptRead_Locked();
 }
 

media/filters/video_renderer_impl.h

@@ -68,7 +68,7 @@ class MEDIA_EXPORT VideoRendererImpl
                           const PipelineStatusCB& error_cb,
                           const TimeDeltaCB& get_time_cb) OVERRIDE;
   virtual void Flush(const base::Closure& callback) OVERRIDE;
-  virtual void StartPlaying() OVERRIDE;
+  virtual void StartPlayingFrom(base::TimeDelta timestamp) OVERRIDE;
 
   // PlatformThread::Delegate implementation.
   virtual void ThreadMain() OVERRIDE;
@@ -140,7 +140,7 @@ class MEDIA_EXPORT VideoRendererImpl
 
   // Important detail: being in kPlaying doesn't imply that video is being
   // rendered. Rather, it means that the renderer is ready to go. The actual
-  // rendering of video is controlled by time advancing via |time_cb_|.
+  // rendering of video is controlled by time advancing via |get_time_cb_|.
   //
   //   kUninitialized
   //         | Initialize()
@@ -151,7 +151,7 @@ class MEDIA_EXPORT VideoRendererImpl
   //         |
   //         V            Decoders reset
   //      kFlushed <------------------ kFlushing
-  //         | StartPlaying()             ^
+  //         | StartPlayingFrom()         ^
   //         |                            |
   //         |                            | Flush()
   //         `---------> kPlaying --------'

media/filters/video_renderer_impl_unittest.cc

@@ -110,9 +110,10 @@ class VideoRendererImplTest : public ::testing::Test {
         base::Bind(&VideoRendererImplTest::GetTime, base::Unretained(this)));
   }
 
-  void StartPlaying() {
-    SCOPED_TRACE("StartPlaying()");
-    renderer_->StartPlaying();
+  void StartPlayingFrom(int milliseconds) {
+    SCOPED_TRACE(base::StringPrintf("StartPlayingFrom(%d)", milliseconds));
+    renderer_->StartPlayingFrom(
+        base::TimeDelta::FromMilliseconds(milliseconds));
     message_loop_.RunUntilIdle();
   }
 
@@ -330,12 +331,12 @@ TEST_F(VideoRendererImplTest, Initialize) {
   Destroy();
 }
 
-TEST_F(VideoRendererImplTest, InitializeAndStartPlaying) {
+TEST_F(VideoRendererImplTest, InitializeAndStartPlayingFrom) {
   Initialize();
   QueueFrames("0 10 20 30");
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(0)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  StartPlaying();
+  StartPlayingFrom(0);
   Destroy();
 }
 
@@ -349,7 +350,7 @@ TEST_F(VideoRendererImplTest, DestroyWhileFlushing) {
   QueueFrames("0 10 20 30");
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(0)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  StartPlaying();
+  StartPlayingFrom(0);
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_NOTHING));
   renderer_->Flush(NewExpectedClosure());
   Destroy();
@@ -360,13 +361,13 @@ TEST_F(VideoRendererImplTest, Play) {
   QueueFrames("0 10 20 30");
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(0)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  StartPlaying();
+  StartPlayingFrom(0);
   Destroy();
 }
 
 TEST_F(VideoRendererImplTest, FlushWithNothingBuffered) {
   Initialize();
-  StartPlaying();
+  StartPlayingFrom(0);
 
   // We shouldn't expect a buffering state change since we never reached
   // BUFFERING_HAVE_ENOUGH.
@@ -379,7 +380,7 @@ TEST_F(VideoRendererImplTest, DecodeError_Playing) {
   QueueFrames("0 10 20 30");
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(0)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  StartPlaying();
+  StartPlayingFrom(0);
 
   QueueFrames("error");
   SatisfyPendingRead();
@@ -387,47 +388,44 @@ TEST_F(VideoRendererImplTest, DecodeError_Playing) {
   Destroy();
 }
 
-TEST_F(VideoRendererImplTest, DecodeError_DuringStartPlaying) {
+TEST_F(VideoRendererImplTest, DecodeError_DuringStartPlayingFrom) {
   Initialize();
   QueueFrames("error");
-  StartPlaying();
+  StartPlayingFrom(0);
   Destroy();
 }
 
-TEST_F(VideoRendererImplTest, StartPlaying_Exact) {
+TEST_F(VideoRendererImplTest, StartPlayingFrom_Exact) {
   Initialize();
   QueueFrames("50 60 70 80 90");
 
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(60)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  AdvanceTimeInMs(60);
-  StartPlaying();
+  StartPlayingFrom(60);
   Destroy();
 }
 
-TEST_F(VideoRendererImplTest, StartPlaying_RightBefore) {
+TEST_F(VideoRendererImplTest, StartPlayingFrom_RightBefore) {
   Initialize();
   QueueFrames("50 60 70 80 90");
 
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(50)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  AdvanceTimeInMs(59);
-  StartPlaying();
+  StartPlayingFrom(59);
   Destroy();
 }
 
-TEST_F(VideoRendererImplTest, StartPlaying_RightAfter) {
+TEST_F(VideoRendererImplTest, StartPlayingFrom_RightAfter) {
   Initialize();
   QueueFrames("50 60 70 80 90");
 
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(60)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  AdvanceTimeInMs(61);
-  StartPlaying();
+  StartPlayingFrom(61);
   Destroy();
 }
 
-TEST_F(VideoRendererImplTest, StartPlaying_LowDelay) {
+TEST_F(VideoRendererImplTest, StartPlayingFrom_LowDelay) {
   // In low-delay mode only one frame is required to finish preroll.
   InitializeWithLowDelay(true);
   QueueFrames("0");
@@ -438,7 +436,7 @@ TEST_F(VideoRendererImplTest, StartPlaying_LowDelay) {
       .Times(AnyNumber());
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_NOTHING))
       .Times(AnyNumber());
-  StartPlaying();
+  StartPlayingFrom(0);
 
   QueueFrames("10");
   SatisfyPendingRead();
@@ -452,26 +450,13 @@ TEST_F(VideoRendererImplTest, StartPlaying_LowDelay) {
   Destroy();
 }
 
-TEST_F(VideoRendererImplTest, PlayAfterStartPlaying) {
-  Initialize();
-  QueueFrames("0 10 20 30");
-  EXPECT_CALL(mock_cb_, Display(HasTimestamp(0)));
-  EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  StartPlaying();
-
-  // Check that there is an outstanding Read() request.
-  EXPECT_TRUE(IsReadPending());
-
-  Destroy();
-}
-
 // Verify that a late decoder response doesn't break invariants in the renderer.
 TEST_F(VideoRendererImplTest, DestroyDuringOutstandingRead) {
   Initialize();
   QueueFrames("0 10 20 30");
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(0)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  StartPlaying();
+  StartPlayingFrom(0);
 
   // Check that there is an outstanding Read() request.
   EXPECT_TRUE(IsReadPending());
@@ -489,7 +474,7 @@ TEST_F(VideoRendererImplTest, Underflow) {
   QueueFrames("0 10 20 30");
   EXPECT_CALL(mock_cb_, Display(HasTimestamp(0)));
   EXPECT_CALL(mock_cb_, BufferingStateChange(BUFFERING_HAVE_ENOUGH));
-  StartPlaying();
+  StartPlayingFrom(0);
 
   // Advance time slightly. Frames should be dropped and we should NOT signal
   // having nothing.