Change the TimeSource interface to return wallclock time for video.

media/renderers/video_renderer_impl.cc

 // Copyright 2013 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/renderers/video_renderer_impl.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/platform_thread.h"
+#include "base/time/default_tick_clock.h"
 #include "base/trace_event/trace_event.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/buffers.h"
 #include "media/base/limits.h"
 #include "media/base/pipeline.h"
 #include "media/base/video_frame.h"
 
 namespace media {
 
 VideoRendererImpl::VideoRendererImpl(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
     ScopedVector<VideoDecoder> decoders,
     bool drop_frames,
     const scoped_refptr<MediaLog>& media_log)
     : task_runner_(task_runner),
       video_frame_stream_(
           new VideoFrameStream(task_runner, decoders.Pass(), media_log)),
       low_delay_(false),
       received_end_of_stream_(false),
       rendered_end_of_stream_(false),
       frame_available_(&lock_),
       state_(kUninitialized),
       thread_(),
       pending_read_(false),
       drop_frames_(drop_frames),
       buffering_state_(BUFFERING_HAVE_NOTHING),
-      last_timestamp_(kNoTimestamp()),
-      last_painted_timestamp_(kNoTimestamp()),
       frames_decoded_(0),
       frames_dropped_(0),
       is_shutting_down_(false),
+      tick_clock_(new base::DefaultTickClock()),
       weak_factory_(this) {
 }
 
 VideoRendererImpl::~VideoRendererImpl() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   {
     base::AutoLock auto_lock(lock_);
     is_shutting_down_ = true;
     frame_available_.Signal();
@@ skipping 48 matching lines @@
 
 void VideoRendererImpl::Initialize(
     DemuxerStream* stream,
     const PipelineStatusCB& init_cb,
     const SetDecryptorReadyCB& set_decryptor_ready_cb,
     const StatisticsCB& statistics_cb,
     const BufferingStateCB& buffering_state_cb,
     const PaintCB& paint_cb,
     const base::Closure& ended_cb,
     const PipelineStatusCB& error_cb,
-    const TimeDeltaCB& get_time_cb,
+    const TimeConverterCB& time_converter_cb,
     const base::Closure& waiting_for_decryption_key_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
   DCHECK(stream);
   DCHECK_EQ(stream->type(), DemuxerStream::VIDEO);
   DCHECK(!init_cb.is_null());
   DCHECK(!statistics_cb.is_null());
   DCHECK(!buffering_state_cb.is_null());
   DCHECK(!paint_cb.is_null());
   DCHECK(!ended_cb.is_null());
-  DCHECK(!get_time_cb.is_null());
+  DCHECK(!time_converter_cb.is_null());
   DCHECK_EQ(kUninitialized, state_);
 
   low_delay_ = (stream->liveness() == DemuxerStream::LIVENESS_LIVE);
 
   // Always post |init_cb_| because |this| could be destroyed if initialization
   // failed.
   init_cb_ = BindToCurrentLoop(init_cb);
 
   statistics_cb_ = statistics_cb;
   buffering_state_cb_ = buffering_state_cb;
   paint_cb_ = paint_cb,
   ended_cb_ = ended_cb;
   error_cb_ = error_cb;
-  get_time_cb_ = get_time_cb;
+  time_converter_cb_ = time_converter_cb;
   state_ = kInitializing;
 
   video_frame_stream_->Initialize(
       stream, base::Bind(&VideoRendererImpl::OnVideoFrameStreamInitialized,
                          weak_factory_.GetWeakPtr()),
       set_decryptor_ready_cb, statistics_cb, waiting_for_decryption_key_cb);
 }
 
 void VideoRendererImpl::CreateVideoThread() {
   // This may fail and cause a crash if there are too many threads created in
@@ skipping 35 matching lines @@
 
   // The number of milliseconds to idle when we do not have anything to do.
   // Nothing special about the value, other than we're being more OS-friendly
   // than sleeping for 1 millisecond.
   //
   // TODO(scherkus): switch to pure event-driven frame timing instead of this
   // kIdleTimeDelta business http://crbug.com/106874
   const base::TimeDelta kIdleTimeDelta =
       base::TimeDelta::FromMilliseconds(10);
 
-  // If we have no frames and haven't painted any frame for certain amount of
-  // time, declare BUFFERING_HAVE_NOTHING.
-  const base::TimeDelta kTimeToDeclareHaveNothing =
-      base::TimeDelta::FromSeconds(3);
-
   for (;;) {
     base::AutoLock auto_lock(lock_);
 
     // Thread exit condition.
     if (is_shutting_down_)
       return;
 
     // Remain idle as long as we're not playing.
     if (state_ != kPlaying || buffering_state_ != BUFFERING_HAVE_ENOUGH) {
       UpdateStatsAndWait_Locked(kIdleTimeDelta);
       continue;
     }
 
-    base::TimeDelta now = get_time_cb_.Run();
+    base::TimeTicks now = tick_clock_->NowTicks();
 
     // Remain idle until we have the next frame ready for rendering.
     if (ready_frames_.empty()) {
       if (received_end_of_stream_) {
         if (!rendered_end_of_stream_) {
           rendered_end_of_stream_ = true;
           task_runner_->PostTask(FROM_HERE, ended_cb_);
         }
-      } else if (last_painted_timestamp_ != kNoTimestamp() &&
-                 now - last_painted_timestamp_ >= kTimeToDeclareHaveNothing) {
+      } else {
         buffering_state_ = BUFFERING_HAVE_NOTHING;
         task_runner_->PostTask(
             FROM_HERE, base::Bind(buffering_state_cb_, BUFFERING_HAVE_NOTHING));
       }
 
       UpdateStatsAndWait_Locked(kIdleTimeDelta);
       continue;
     }
 
-    base::TimeDelta target_paint_timestamp = ready_frames_.front()->timestamp();
-    base::TimeDelta latest_paint_timestamp;
+    base::TimeTicks target_paint_timestamp =
+        time_converter_cb_.Run(ready_frames_.front()->timestamp());
+
+    // If media time has stopped, don't attempt to paint any more frames.
+    if (target_paint_timestamp.is_null()) {
+      UpdateStatsAndWait_Locked(kIdleTimeDelta);
+      continue;
+    }
+
+    base::TimeTicks latest_paint_timestamp;

[xhwang, 2015/03/23 22:20:14]

bikesheddings not related to your change:

|latest| can also mean "most recent", which is a bit confusing given we also
have "last_painted_timestamp_"....

We seem to use timestamp for all time variables. I feel it's a bit weird to use
"timestamp" for a future wall clock time (TimeTicks)... Also, by looking at the
implementation, it's hard to tell which is media time (TimeDelta) and which is
wall time (TimeTicks). How about we use "timestamp" for media time, and just
"time" for wall clock time?

[DaleCurtis, 2015/03/25 00:31:45]

Done; great suggestion!

 
     // Deadline is defined as the duration between this frame and the next
     // frame, using the delta between this frame and the previous frame as the
     // assumption for frame duration.
     //
     // TODO(scherkus): This can be vastly improved. Use a histogram to measure
     // the accuracy of our frame timing code. http://crbug.com/149829
-    if (last_timestamp_ == kNoTimestamp()) {
-      latest_paint_timestamp = base::TimeDelta::Max();
+    if (last_timestamp_.is_null()) {
+      latest_paint_timestamp = now;
     } else {
       base::TimeDelta duration = target_paint_timestamp - last_timestamp_;
       latest_paint_timestamp = target_paint_timestamp + duration;
     }
 
     // Remain idle until we've reached our target paint window.
     if (now < target_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.
     //
     // At this point enough time has passed that the next frame that ready for
     // rendering.
     PaintNextReadyFrame_Locked();
   }
 }
 
+void VideoRendererImpl::SetTickClockForTesting(
+    scoped_ptr<base::TickClock> tick_clock) {
+  tick_clock_.swap(tick_clock);
+}
+
 void VideoRendererImpl::PaintNextReadyFrame_Locked() {
   lock_.AssertAcquired();
 
   scoped_refptr<VideoFrame> next_frame = ready_frames_.front();
   ready_frames_.pop_front();
   frames_decoded_++;
 
-  last_timestamp_ = next_frame->timestamp();
-  last_painted_timestamp_ = next_frame->timestamp();
+  last_timestamp_ = last_painted_timestamp_ =
+      time_converter_cb_.Run(next_frame->timestamp());

[xhwang, 2015/03/23 22:20:14]

hmm, do we allow chained assignment? I don't see it banned anywhere, but I
rarely (if ever) see it in our code base.

[DaleCurtis, 2015/03/25 00:31:45]

Hmm, I tend to use it in the audio pathways, but I'm happy to change it if you
prefer otherwise.

 
   paint_cb_.Run(next_frame);
 
   task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&VideoRendererImpl::AttemptRead, weak_factory_.GetWeakPtr()));
 }
 
 void VideoRendererImpl::DropNextReadyFrame_Locked() {
   TRACE_EVENT0("media", "VideoRendererImpl:frameDropped");
 
   lock_.AssertAcquired();
 
-  last_timestamp_ = ready_frames_.front()->timestamp();
+  last_timestamp_ = time_converter_cb_.Run(ready_frames_.front()->timestamp());
+
   ready_frames_.pop_front();
   frames_decoded_++;
   frames_dropped_++;
 
   task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&VideoRendererImpl::AttemptRead, weak_factory_.GetWeakPtr()));
 }
 
 void VideoRendererImpl::FrameReady(VideoFrameStream::Status status,
@@ skipping 120 matching lines @@
 void VideoRendererImpl::OnVideoFrameStreamResetDone() {
   base::AutoLock auto_lock(lock_);
   DCHECK_EQ(kFlushing, state_);
   DCHECK(!pending_read_);
   DCHECK(ready_frames_.empty());
   DCHECK(!received_end_of_stream_);
   DCHECK(!rendered_end_of_stream_);
   DCHECK_EQ(buffering_state_, BUFFERING_HAVE_NOTHING);
 
   state_ = kFlushed;
-  last_timestamp_ = kNoTimestamp();
-  last_painted_timestamp_ = kNoTimestamp();
+  last_timestamp_ = last_painted_timestamp_ = base::TimeTicks();

[xhwang, 2015/03/23 22:20:14]

ditto

[DaleCurtis, 2015/03/25 00:31:45]

Acknowledged.

   base::ResetAndReturn(&flush_cb_).Run();
 }
 
 void VideoRendererImpl::UpdateStatsAndWait_Locked(
     base::TimeDelta wait_duration) {
   lock_.AssertAcquired();
   DCHECK_GE(frames_decoded_, 0);
   DCHECK_LE(frames_dropped_, frames_decoded_);
 
   if (frames_decoded_) {
     PipelineStatistics statistics;
     statistics.video_frames_decoded = frames_decoded_;
     statistics.video_frames_dropped = frames_dropped_;
     task_runner_->PostTask(FROM_HERE, base::Bind(statistics_cb_, statistics));
 
     frames_decoded_ = 0;
     frames_dropped_ = 0;
   }
 
   frame_available_.TimedWait(wait_duration);
 }
 
 }  // namespace media