Remove media::VideoRenderer::SetPlaybackRate().

Video renderers don't need the information as they are already making
audio/video synchronization decisions based on a media timeline that
already incorporates the current playback rate via the time callback.

In particular, VideoRendererImpl only used playback rate to estimate
a better duration to sleep until the current frame was ready ...
except that in most cases we'd sleep for kIdleTimeDelta and wait
until the media timeline had progressed past the current frame's
timestamp. In other words, there's absolutely no reason to even try
to estimate the sleep duration based on the playback rate.

BUG=370634

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

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

media/base/mock_filters.h

@@ -130,7 +130,6 @@ class MockVideoRenderer : public VideoRenderer {
   MOCK_METHOD1(Flush, void(const base::Closure& callback));
   MOCK_METHOD1(StartPlayingFrom, void(base::TimeDelta timestamp));
   MOCK_METHOD1(Stop, void(const base::Closure& callback));
-  MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockVideoRenderer);

media/base/pipeline.cc

@@ -649,8 +649,6 @@ void Pipeline::PlaybackRateChangedTask(float playback_rate) {
 
   if (audio_renderer_)
     audio_renderer_->SetPlaybackRate(playback_rate_);
-  if (video_renderer_)
-    video_renderer_->SetPlaybackRate(playback_rate_);
 }
 
 void Pipeline::VolumeChangedTask(float volume) {

media/base/pipeline_unittest.cc

@@ -208,7 +208,6 @@ class PipelineTest : public ::testing::Test {
       }
 
       if (video_stream_) {
-        EXPECT_CALL(*video_renderer_, SetPlaybackRate(0.0f));
         EXPECT_CALL(*video_renderer_, StartPlayingFrom(base::TimeDelta()))
             .WillOnce(SetBufferingState(&video_buffering_state_cb_,
                                         BUFFERING_HAVE_ENOUGH));
@@ -285,7 +284,6 @@ class PipelineTest : public ::testing::Test {
       EXPECT_CALL(*video_renderer_, StartPlayingFrom(seek_time))
           .WillOnce(SetBufferingState(&video_buffering_state_cb_,
                                       BUFFERING_HAVE_ENOUGH));
-      EXPECT_CALL(*video_renderer_, SetPlaybackRate(_));
     }
 
     // We expect a successful seek callback followed by a buffering update.
@@ -631,7 +629,6 @@ TEST_F(PipelineTest, AudioStreamShorterThanVideo) {
   EXPECT_EQ(0, pipeline_->GetMediaTime().ToInternalValue());
 
   float playback_rate = 1.0f;
-  EXPECT_CALL(*video_renderer_, SetPlaybackRate(playback_rate));
   EXPECT_CALL(*audio_renderer_, SetPlaybackRate(playback_rate));
   pipeline_->SetPlaybackRate(playback_rate);
   message_loop_.RunUntilIdle();
@@ -1000,7 +997,6 @@ class PipelineTeardownTest : public PipelineTest {
                                     BUFFERING_HAVE_ENOUGH));
 
     EXPECT_CALL(*audio_renderer_, SetPlaybackRate(0.0f));
-    EXPECT_CALL(*video_renderer_, SetPlaybackRate(0.0f));
     EXPECT_CALL(*audio_renderer_, SetVolume(1.0f));
     EXPECT_CALL(*audio_renderer_, StartRendering());
 

media/base/video_renderer.h

@@ -75,9 +75,6 @@ class MEDIA_EXPORT VideoRenderer {
   // when complete.
   virtual void Stop(const base::Closure& callback) = 0;
 
-  // Updates the current playback rate.
-  virtual void SetPlaybackRate(float playback_rate) = 0;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(VideoRenderer);
 };

media/filters/video_renderer_impl.cc

@@ -34,7 +34,6 @@ VideoRendererImpl::VideoRendererImpl(
       thread_(),
       pending_read_(false),
       drop_frames_(drop_frames),
-      playback_rate_(0),
       buffering_state_(BUFFERING_HAVE_NOTHING),
       paint_cb_(paint_cb),
       last_timestamp_(kNoTimestamp()),
@@ -106,12 +105,6 @@ void VideoRendererImpl::Stop(const base::Closure& callback) {
   video_frame_stream_.Stop(callback);
 }
 
-void VideoRendererImpl::SetPlaybackRate(float playback_rate) {
-  DCHECK(task_runner_->BelongsToCurrentThread());
-  base::AutoLock auto_lock(lock_);
-  playback_rate_ = playback_rate;
-}
-
 void VideoRendererImpl::StartPlayingFrom(base::TimeDelta timestamp) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
@@ -221,8 +214,7 @@ void VideoRendererImpl::ThreadMain() {
       return;
 
     // Remain idle as long as we're not playing.
-    if (state_ != kPlaying || playback_rate_ == 0 ||
-        buffering_state_ != BUFFERING_HAVE_ENOUGH) {
+    if (state_ != kPlaying || buffering_state_ != BUFFERING_HAVE_ENOUGH) {
       UpdateStatsAndWait_Locked(kIdleTimeDelta);
       continue;
     }
@@ -238,16 +230,10 @@ void VideoRendererImpl::ThreadMain() {
       continue;
     }
 
-    base::TimeDelta remaining_time =
-        CalculateSleepDuration(ready_frames_.front(), playback_rate_);
-
-    // Sleep up to a maximum of our idle time until we're within the time to
-    // render the next frame.
-    if (remaining_time.InMicroseconds() > 0) {
-      remaining_time = std::min(remaining_time, kIdleTimeDelta);
-      UpdateStatsAndWait_Locked(remaining_time);
-      continue;
-    }
+    base::TimeDelta now = get_time_cb_.Run();
+    base::TimeDelta target_timestamp = ready_frames_.front()->timestamp();
+    base::TimeDelta earliest_paint_timestamp;
+    base::TimeDelta latest_paint_timestamp;
 
     // Deadline is defined as the midpoint between this frame and the next
     // frame, using the delta between this frame and the previous frame as the
@@ -260,15 +246,24 @@ void VideoRendererImpl::ThreadMain() {
     //
     // TODO(scherkus): This can be vastly improved. Use a histogram to measure
     // the accuracy of our frame timing code. http://crbug.com/149829
-    if (drop_frames_ && last_timestamp_ != kNoTimestamp()) {
-      base::TimeDelta now = get_time_cb_.Run();
-      base::TimeDelta deadline = ready_frames_.front()->timestamp() +
-          (ready_frames_.front()->timestamp() - last_timestamp_) / 2;
+    if (last_timestamp_ == kNoTimestamp()) {
+      earliest_paint_timestamp = target_timestamp;
+      latest_paint_timestamp = base::TimeDelta::Max();
+    } else {
+      base::TimeDelta duration = target_timestamp - last_timestamp_;
+      earliest_paint_timestamp = target_timestamp - duration / 2;
+      latest_paint_timestamp = target_timestamp + duration / 2;
+    }
 
-      if (now > deadline) {
-        DropNextReadyFrame_Locked();
+    // Remain idle until we've reached our target paint window.
+    if (now < earliest_paint_timestamp) {
+      UpdateStatsAndWait_Locked(kIdleTimeDelta);
       continue;
     }
+
+    if (now > latest_paint_timestamp && drop_frames_) {
+      DropNextReadyFrame_Locked();
+      continue;
     }
 
     // Congratulations! You've made it past the video frame timing gauntlet.
@@ -477,19 +472,6 @@ void VideoRendererImpl::OnVideoFrameStreamResetDone() {
   base::ResetAndReturn(&flush_cb_).Run();
 }
 
-base::TimeDelta VideoRendererImpl::CalculateSleepDuration(
-    const scoped_refptr<VideoFrame>& next_frame,
-    float playback_rate) {
-  // Determine the current and next presentation timestamps.
-  base::TimeDelta now = get_time_cb_.Run();
-  base::TimeDelta next_pts = next_frame->timestamp();
-
-  // Scale our sleep based on the playback rate.
-  base::TimeDelta sleep = next_pts - now;
-  return base::TimeDelta::FromMicroseconds(
-      static_cast<int64>(sleep.InMicroseconds() / playback_rate));
-}
-
 void VideoRendererImpl::DoStopOrError_Locked() {
   lock_.AssertAcquired();
   last_timestamp_ = kNoTimestamp();

media/filters/video_renderer_impl.h

@@ -69,7 +69,6 @@ class MEDIA_EXPORT VideoRendererImpl
   virtual void Flush(const base::Closure& callback) OVERRIDE;
   virtual void StartPlayingFrom(base::TimeDelta timestamp) OVERRIDE;
   virtual void Stop(const base::Closure& callback) OVERRIDE;
-  virtual void SetPlaybackRate(float playback_rate) OVERRIDE;
 
   // PlatformThread::Delegate implementation.
   virtual void ThreadMain() OVERRIDE;
@@ -95,14 +94,6 @@ class MEDIA_EXPORT VideoRendererImpl
   // Called when VideoFrameStream::Reset() completes.
   void OnVideoFrameStreamResetDone();
 
-  // Calculates the duration to sleep for based on |last_timestamp_|,
-  // the next frame timestamp (may be NULL), and the provided playback rate.
-  //
-  // We don't use |playback_rate_| to avoid locking.
-  base::TimeDelta CalculateSleepDuration(
-      const scoped_refptr<VideoFrame>& next_frame,
-      float playback_rate);
-
   // Helper function that flushes the buffers when a Stop() or error occurs.
   void DoStopOrError_Locked();
 
@@ -186,8 +177,6 @@ class MEDIA_EXPORT VideoRendererImpl
 
   bool drop_frames_;
 
-  float playback_rate_;
-
   BufferingState buffering_state_;
 
   // Playback operation callbacks.

media/filters/video_renderer_impl_unittest.cc

@@ -27,7 +27,6 @@
 using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::AtLeast;
-using ::testing::InSequence;
 using ::testing::Invoke;
 using ::testing::NiceMock;
 using ::testing::NotNull;
@@ -95,11 +94,6 @@ class VideoRendererImplTest : public ::testing::Test {
     EXPECT_CALL(*decoder_, Reset(_))
         .WillRepeatedly(Invoke(this, &VideoRendererImplTest::FlushRequested));
 
-    InSequence s;
-
-    // Set playback rate before anything else happens.
-    renderer_->SetPlaybackRate(1.0f);
-
     // Initialize, we shouldn't have any reads.
     InitializeRenderer(PIPELINE_OK, low_delay);
   }
@@ -549,7 +543,6 @@ TEST_F(VideoRendererImplTest, StopDuringOutstandingRead) {
 }
 
 TEST_F(VideoRendererImplTest, VideoDecoder_InitFailure) {
-  InSequence s;
   InitializeRenderer(DECODER_ERROR_NOT_SUPPORTED, false);
   Stop();
 }