Switch GetWallClockTime to using vectors for input and output.

media/filters/video_renderer_algorithm.cc

Index: media/filters/video_renderer_algorithm.cc
diff --git a/media/filters/video_renderer_algorithm.cc b/media/filters/video_renderer_algorithm.cc
index 9402c09c818b6cfbb4675bfb4fe45a9c2ebc61a2..813d6a02fcc9b4c69eea7ed88e4aa1ab48d7a412 100644
--- a/media/filters/video_renderer_algorithm.cc
+++ b/media/filters/video_renderer_algorithm.cc
@@ -16,6 +16,7 @@ const int kMovingAverageSamples = 25;
 VideoRendererAlgorithm::ReadyFrame::ReadyFrame(
     const scoped_refptr<VideoFrame>& ready_frame)
     : frame(ready_frame),
+      estimated_end_time(true),

[miu, 2015/05/12 02:46:57]

Do you need this boolean?  It looks like estimated_end_time will always be equal
to end_time.is_null().

[DaleCurtis, 2015/05/12 02:48:49]

end_time is set to start_time + average_frame_duration for the last frame in the
queue, so that won't work.

[miu, 2015/05/12 02:52:46]

OIC.  So, my only comment now is naming nit: Can we call this
has_estimated_end_time or end_time_was_estimated?

[DaleCurtis, 2015/05/12 02:58:59]

has_estimated_end_time sgtm, I'll change it shortly.

       ideal_render_count(0),
       render_count(0),
       drop_count(0) {
@@ -30,13 +31,13 @@ bool VideoRendererAlgorithm::ReadyFrame::operator<(
 }
 
 VideoRendererAlgorithm::VideoRendererAlgorithm(
-    const TimeConverterCB& time_converter_cb)
+    const TimeSource::WallClockTimeCB& time_converter_cb)
     : cadence_estimator_(base::TimeDelta::FromSeconds(
           kMinimumAcceptableTimeBetweenGlitchesSecs)),
-      time_converter_cb_(time_converter_cb),
+      wall_clock_time_cb_(time_converter_cb),
       frame_duration_calculator_(kMovingAverageSamples),
       frame_dropping_disabled_(false) {
-  DCHECK(!time_converter_cb_.is_null());
+  DCHECK(!wall_clock_time_cb_.is_null());
   Reset();
 }
 
@@ -393,31 +394,31 @@ void VideoRendererAlgorithm::AccountForMissedIntervals(
 }
 
 bool VideoRendererAlgorithm::UpdateFrameStatistics() {
-  // Figure out all current ready frame times at once so we minimize the drift
-  // relative to real time as the code below executes.
-  for (size_t i = 0; i < frame_queue_.size(); ++i) {
-    ReadyFrame& frame = frame_queue_[i];
-    const bool new_frame = frame.start_time.is_null();
-    frame.start_time = time_converter_cb_.Run(frame.frame->timestamp());
+  DCHECK(!frame_queue_.empty());
 
-    // If time stops or never started, exit immediately.
-    if (frame.start_time.is_null()) {
-      frame.end_time = base::TimeTicks();
-      return false;
-    }
+  // Figure out all current ready frame times at once.
+  std::vector<base::TimeDelta> media_timestamps;
+  media_timestamps.reserve(frame_queue_.size());
+  for (const auto& ready_frame : frame_queue_)
+    media_timestamps.push_back(ready_frame.frame->timestamp());
 
-    // TODO(dalecurtis): An unlucky tick of a playback rate change could cause
-    // this to skew so much that time goes backwards between calls.  Fix this by
-    // either converting all timestamps at once or with some retry logic.
-    if (i > 0) {
-      frame_queue_[i - 1].end_time = frame.start_time;
-      const base::TimeDelta delta =
-          frame.start_time - frame_queue_[i - 1].start_time;
-      CHECK_GT(delta, base::TimeDelta());
-      if (new_frame)
-        frame_duration_calculator_.AddSample(delta);
-    }
+  // If time has stopped, we can bail out early.
+  std::vector<base::TimeTicks> wall_clock_times;
+  if (!wall_clock_time_cb_.Run(media_timestamps, &wall_clock_times))
+    return false;
+
+  // Transfer the converted wall clock times into our frame queue.
+  DCHECK_EQ(wall_clock_times.size(), frame_queue_.size());
+  for (size_t i = 0; i < frame_queue_.size() - 1; ++i) {
+    ReadyFrame& frame = frame_queue_[i];
+    const bool new_sample = frame.estimated_end_time;
+    frame.start_time = wall_clock_times[i];
+    frame.end_time = wall_clock_times[i + 1];
+    frame.estimated_end_time = false;
+    if (new_sample)
+      frame_duration_calculator_.AddSample(frame.end_time - frame.start_time);
   }
+  frame_queue_.back().start_time = wall_clock_times.back();

[miu, 2015/05/12 02:46:57]

nit: DCHECK(frame_queue_.back().end_time.is_null());

[DaleCurtis, 2015/05/12 02:48:48]

See l.431, this is not correct.

 
   if (!frame_duration_calculator_.count())
     return false;