Handle incoming frames with the same timestamp.

Crash analysis shows frames with the same timestamp getting through.

Replaces the old frame if the old frame hasn't yet been rendered or
drops the new frame if the old frame has already been shown.

BUG=485042
TEST=new unittest.

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

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

3 files changed, 66 insertions, 6 deletions

diff --git a/media/filters/video_renderer_algorithm.cc b/media/filters/video_renderer_algorithm.cc
index 5bad074..9402c09 100644
--- a/media/filters/video_renderer_algorithm.cc
+++ b/media/filters/video_renderer_algorithm.cc
@@ -53,7 +53,8 @@ scoped_refptr<VideoFrame> VideoRendererAlgorithm::Render(
     return nullptr;
 
   if (frames_dropped)
-    *frames_dropped = 0;
+    *frames_dropped = frames_dropped_during_enqueue_;
+  frames_dropped_during_enqueue_ = 0;
 
   // Once Render() is called |last_frame_index_| has meaning and should thus be
   // preserved even if better frames come in before it due to out of order
@@ -252,7 +253,7 @@ void VideoRendererAlgorithm::OnLastFrameDropped() {
 }
 
 void VideoRendererAlgorithm::Reset() {
-  last_frame_index_ = 0;
+  frames_dropped_during_enqueue_ = last_frame_index_ = 0;
   have_rendered_frames_ = last_render_had_glitch_ = false;
   last_deadline_max_ = base::TimeTicks();
   average_frame_duration_ = render_interval_ = base::TimeDelta();
@@ -316,14 +317,30 @@ void VideoRendererAlgorithm::EnqueueFrame(
 
   // If a frame was inserted before the first frame, update the index. On the
   // next call to Render() it will be dropped.
-  if (static_cast<size_t>(it - frame_queue_.begin()) <= last_frame_index_ &&
-      have_rendered_frames_) {
+  const size_t new_frame_index = it - frame_queue_.begin();
+  if (new_frame_index <= last_frame_index_ && have_rendered_frames_) {
+    if (new_frame_index == last_frame_index_ &&
+        frame->timestamp() ==
+            frame_queue_[last_frame_index_].frame->timestamp()) {
+      DVLOG(2) << "Ignoring frame with the same timestamp as the most recently "
+                  "rendered frame.";
+      ++frames_dropped_during_enqueue_;
+      return;
+    }
     ++last_frame_index_;
+  } else if (new_frame_index < frame_queue_.size() &&
+             frame->timestamp() ==
+                 frame_queue_[new_frame_index].frame->timestamp()) {
+    DVLOG(2) << "Replacing existing frame with the same timestamp with most "
+             << "recently received frame.";
+    frame_queue_[new_frame_index].frame = frame;
+    ++frames_dropped_during_enqueue_;
+    return;
   }
 
   // The vast majority of cases should always append to the back, but in rare
   // circumstance we get out of order timestamps, http://crbug.com/386551.
-  it = frame_queue_.insert(it, ready_frame);
+  frame_queue_.insert(it, ready_frame);
 
   // Project the current cadence calculations to include the new frame.  These
   // may not be accurate until the next Render() call.  These updates are done
diff --git a/media/filters/video_renderer_algorithm.h b/media/filters/video_renderer_algorithm.h
index 88e8766..3c37e7f 100644
--- a/media/filters/video_renderer_algorithm.h
+++ b/media/filters/video_renderer_algorithm.h
@@ -91,6 +91,10 @@ class MEDIA_EXPORT VideoRendererAlgorithm {
   // stream frames.  Frames inserted prior to the last rendered frame will not
   // be used.  They will be discarded on the next call to Render(), counting as
   // dropped frames, or by RemoveExpiredFrames(), counting as expired frames.
+  //
+  // Attempting to enqueue a frame with the same timestamp as a previous frame
+  // will result in the previous frame being replaced if it has not been
+  // rendered yet.  If it has been rendered, the new frame will be dropped.
   void EnqueueFrame(const scoped_refptr<VideoFrame>& frame);
 
   // Removes all frames from the |frame_queue_| and clears predictors.  The
@@ -279,9 +283,14 @@ class MEDIA_EXPORT VideoRendererAlgorithm {
   // more frames, or chosen a frame which exceeded acceptable drift.
   bool last_render_had_glitch_;
 
-  // For testing functionality which enables clockless playback of all frames.
+  // For testing functionality which enables clockless playback of all frames,
+  // does not prevent frame dropping due to equivalent timestamps.
   bool frame_dropping_disabled_;
 
+  // Tracks frames dropped during enqueue when identical timestamps are added
+  // to the queue.  Callers are told about these frames during Render().
+  size_t frames_dropped_during_enqueue_;
+
   DISALLOW_COPY_AND_ASSIGN(VideoRendererAlgorithm);
 };
 
diff --git a/media/filters/video_renderer_algorithm_unittest.cc b/media/filters/video_renderer_algorithm_unittest.cc
index fb74806..20ce6ef 100644
--- a/media/filters/video_renderer_algorithm_unittest.cc
+++ b/media/filters/video_renderer_algorithm_unittest.cc
@@ -1132,4 +1132,38 @@ TEST_F(VideoRendererAlgorithmTest, UglyTimestampsHaveCadence) {
   }
 }
 
+TEST_F(VideoRendererAlgorithmTest, EnqueueFrames) {
+  TickGenerator tg(base::TimeTicks(), 50);
+  time_source_.StartTicking();
+
+  EXPECT_EQ(0u, frames_queued());
+  scoped_refptr<VideoFrame> frame_1 = CreateFrame(tg.interval(0));
+  algorithm_.EnqueueFrame(frame_1);
+  EXPECT_EQ(1u, frames_queued());
+
+  // Enqueuing a frame with the same timestamp should not increase the queue and
+  // just replace the existing frame if we haven't rendered it.
+  scoped_refptr<VideoFrame> frame_2 = CreateFrame(tg.interval(0));
+  algorithm_.EnqueueFrame(frame_2);
+  EXPECT_EQ(1u, frames_queued());
+
+  size_t frames_dropped = 0;
+  scoped_refptr<VideoFrame> rendered_frame =
+      RenderAndStep(&tg, &frames_dropped);
+  EXPECT_EQ(1u, frames_queued());
+  EXPECT_EQ(frame_2, rendered_frame);
+
+  // The replaced frame should count as dropped.
+  EXPECT_EQ(1u, frames_dropped);
+
+  // Trying to replace frame_2 with frame_1 should do nothing.
+  algorithm_.EnqueueFrame(frame_1);
+  EXPECT_EQ(1u, frames_queued());
+
+  rendered_frame = RenderAndStep(&tg, &frames_dropped);
+  EXPECT_EQ(1u, frames_queued());
+  EXPECT_EQ(frame_2, rendered_frame);
+  EXPECT_EQ(1u, frames_dropped);
+}
+
 }  // namespace media