Make VideoRendererAlgorithm::RemoveExpiredFrames() more aggressive.

There's no need to keep more than 1 frame if none are effective. If any
are effective, there's no reason to keep any of the ones that aren't
effective.

R=eugene

Bug: 1111273
Change-Id: I88bd309f7bc347fbfa84ee5f8a0e40d393a86cc4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462666
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815901}

media/filters/video_renderer_algorithm.cc

@@ -243,42 +243,32 @@ size_t VideoRendererAlgorithm::RemoveExpiredFrames(base::TimeTicks deadline) {
   // Even though we may not be able to remove anything due to having only one
   // frame, correct any estimates which may have been set during EnqueueFrame().
   UpdateFrameStatistics();
+  UpdateEffectiveFramesQueued();
 
   // We always leave at least one frame in the queue, so if there's only one
   // frame there's nothing we can expire.
-  if (frame_queue_.size() == 1) {
-    UpdateEffectiveFramesQueued();
+  if (frame_queue_.size() == 1)
     return 0;
-  }
 
   DCHECK_GT(average_frame_duration_, base::TimeDelta());
 
-  // Finds and removes all frames which are too old to be used; I.e., the end of
-  // their render interval is further than |max_acceptable_drift_| from the
-  // given |deadline|.  We also always expire anything inserted before the last
-  // rendered frame.
+  // Finds and removes all frames which are too old to be used.
   size_t frames_dropped_without_rendering = 0;
-  size_t frames_to_expire = 0;
-  const base::TimeTicks minimum_start_time =
-      deadline - max_acceptable_drift_ - average_frame_duration_;
-  for (; frames_to_expire < frame_queue_.size() - 1; ++frames_to_expire) {
-    const ReadyFrame& frame = frame_queue_[frames_to_expire];
-    if (frame.start_time >= minimum_start_time)
-      break;
-    if (frame.render_count == frame.drop_count)
-      ++frames_dropped_without_rendering;
-  }
+  size_t frames_to_expire = std::min(
+      frame_queue_.size() - 1, frame_queue_.size() - effective_frames_queued_);
 
-  if (!frames_to_expire) {
-    UpdateEffectiveFramesQueued();
+  if (!frames_to_expire)
     return 0;
+
+  for (size_t i = 0; i < frames_to_expire; ++i) {
+    const ReadyFrame& frame = frame_queue_[i];
+    if (frame.render_count == frame.drop_count)
+      ++frames_dropped_without_rendering;
   }
 
   cadence_frame_counter_ += frames_to_expire;
   frame_queue_.erase(frame_queue_.begin(),
                      frame_queue_.begin() + frames_to_expire);
-
-  UpdateEffectiveFramesQueued();
   return frames_dropped_without_rendering;
 }
 

media/filters/video_renderer_algorithm_unittest.cc

@@ -1245,12 +1245,12 @@ TEST_F(VideoRendererAlgorithmTest, RemoveExpiredFrames) {
   tg.step(2);
   // Two frames are removed, one displayed frame (which should not be counted as
   // dropped) and one undisplayed one.
-  ASSERT_EQ(1u, algorithm_.RemoveExpiredFrames(tg.current()));
+  ASSERT_EQ(2u, algorithm_.RemoveExpiredFrames(tg.current()));
   // Since we just removed the last rendered frame, OnLastFrameDropped() should
   // be ignored.
   algorithm_.OnLastFrameDropped();
   frame = RenderAndStep(&tg, &frames_dropped);
-  EXPECT_EQ(1u, frames_dropped);
+  EXPECT_EQ(0u, frames_dropped);
   EXPECT_EQ(2u, frames_queued());
   EXPECT_EQ(1u, EffectiveFramesQueued());
   ASSERT_TRUE(frame);