Merge to M44: "Drop frames less than a millisecond apart in media time."

Our rendering path has no reasonable expectation of being able to
render 1000fps content, so these are bad timestamps that end up
wrecking assumptions throughout the rendering pathway.

Instead of trying to guard against these frames everywhere, just
discard them immediately during enqueue.

BUG=488302
TEST=new unittest
TBR=xhwang

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

Cr-Commit-Position: refs/heads/master@{#330458}
(cherry picked from commit b581477)

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

Cr-Commit-Position: refs/branch-heads/2403@{#44}
Cr-Branched-From: f54b809-refs/heads/master@{#330231}

media/filters/video_renderer_algorithm.cc

@@ -363,25 +363,30 @@ void VideoRendererAlgorithm::EnqueueFrame(
                                                     frame_queue_.end(), frame);
   DCHECK_GE(it - frame_queue_.begin(), 0);
 
-  // If a frame was inserted before the first frame, update the index. On the
-  // next call to Render() it will be dropped.
+  // Drop any frames inserted before or at the last rendered frame if we've
+  // already rendered any 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;
+    DVLOG(2) << "Dropping frame inserted before the last rendered frame.";
+    ++frames_dropped_during_enqueue_;
+    return;
+  }
+
+  // Drop any frames which are less than a millisecond apart in media time (even
+  // those with timestamps matching an already enqueued frame), there's no way
+  // we can reasonably render these frames; it's effectively a 1000fps limit.
+  const base::TimeDelta delta =
+      std::min(new_frame_index < frame_queue_.size()
+                   ? frame_queue_[new_frame_index].frame->timestamp() -
+                         frame->timestamp()
+                   : base::TimeDelta::Max(),
+               new_frame_index > 0
+                   ? frame->timestamp() -
+                         frame_queue_[new_frame_index - 1].frame->timestamp()
+                   : base::TimeDelta::Max());
+  if (delta < base::TimeDelta::FromMilliseconds(1)) {
+    DVLOG(2) << "Dropping frame too close to an already enqueued frame: "
+             << delta.InMicroseconds() << " us";
     ++frames_dropped_during_enqueue_;
     return;
   }

media/filters/video_renderer_algorithm_unittest.cc

@@ -633,9 +633,10 @@ TEST_F(VideoRendererAlgorithmTest, SortedFrameQueue) {
 
   // Since a frame has already been rendered, queuing this frame and calling
   // Render() should result in it being dropped; even though it's a better
-  // candidate for the desired interval.
+  // candidate for the desired interval.  The frame is dropped during enqueue so
+  // it won't show up in frames_queued().
   algorithm_.EnqueueFrame(CreateFrame(tg.interval(1)));
-  EXPECT_EQ(3u, frames_queued());
+  EXPECT_EQ(2u, frames_queued());
   EXPECT_EQ(2u, algorithm_.EffectiveFramesQueued());
   frame = RenderAndStep(&tg, &frames_dropped);
   EXPECT_EQ(1u, frames_dropped);
@@ -1190,8 +1191,7 @@ TEST_F(VideoRendererAlgorithmTest, EnqueueFrames) {
   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.
+  // Enqueuing a frame with the same timestamp should always be dropped.
   scoped_refptr<VideoFrame> frame_2 = CreateFrame(tg.interval(0));
   algorithm_.EnqueueFrame(frame_2);
   EXPECT_EQ(1u, frames_queued());
@@ -1200,18 +1200,37 @@ TEST_F(VideoRendererAlgorithmTest, EnqueueFrames) {
   scoped_refptr<VideoFrame> rendered_frame =
       RenderAndStep(&tg, &frames_dropped);
   EXPECT_EQ(1u, frames_queued());
-  EXPECT_EQ(frame_2, rendered_frame);
+  EXPECT_EQ(frame_1, 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);
+  // Trying to replace frame_1 with frame_2 should do nothing.
+  algorithm_.EnqueueFrame(frame_2);
+  EXPECT_EQ(1u, frames_queued());
+
+  rendered_frame = RenderAndStep(&tg, &frames_dropped);
   EXPECT_EQ(1u, frames_queued());
+  EXPECT_EQ(frame_1, rendered_frame);
+  EXPECT_EQ(1u, frames_dropped);
+
+  // Trying to add a frame < 1 ms after the last frame should drop the frame.
+  algorithm_.EnqueueFrame(CreateFrame(base::TimeDelta::FromMicroseconds(999)));
+  rendered_frame = RenderAndStep(&tg, &frames_dropped);
+  EXPECT_EQ(1u, frames_queued());
+  EXPECT_EQ(frame_1, rendered_frame);
+  EXPECT_EQ(1u, frames_dropped);
+
+  scoped_refptr<VideoFrame> frame_3 = CreateFrame(tg.interval(1));
+  algorithm_.EnqueueFrame(frame_3);
+  EXPECT_EQ(2u, frames_queued());
 
+  // Trying to add a frame < 1 ms before the last frame should drop the frame.
+  algorithm_.EnqueueFrame(
+      CreateFrame(tg.interval(1) - base::TimeDelta::FromMicroseconds(999)));
   rendered_frame = RenderAndStep(&tg, &frames_dropped);
   EXPECT_EQ(1u, frames_queued());
-  EXPECT_EQ(frame_2, rendered_frame);
+  EXPECT_EQ(frame_3, rendered_frame);
   EXPECT_EQ(1u, frames_dropped);
 }