Don't modify the ideal render count for a frame for overages.

These values are overwritten when new frames are enqueued because
ordering may change, which wipes overage adjustments

BUG=439548
TEST=modified tests.

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

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

3 files changed, 35 insertions, 21 deletions

diff --git a/media/filters/video_renderer_algorithm.cc b/media/filters/video_renderer_algorithm.cc
index a78342a..5bad074 100644
--- a/media/filters/video_renderer_algorithm.cc
+++ b/media/filters/video_renderer_algorithm.cc
@@ -460,29 +460,33 @@ int VideoRendererAlgorithm::FindBestFrameByCadence() {
   if (!cadence_estimator_.has_cadence())
     return -1;
 
-  int new_ideal_render_count = 0;
+  int remaining_overage = 0;
   const int best_frame =
-      FindBestFrameByCadenceInternal(&new_ideal_render_count);
+      FindBestFrameByCadenceInternal(&remaining_overage);
   if (best_frame < 0)
     return -1;
 
-  DCHECK_GT(new_ideal_render_count, 0);
-  frame_queue_[best_frame].ideal_render_count = new_ideal_render_count;
+  DCHECK_GE(remaining_overage, 0);
+
+  // Overage is treated as having been displayed and dropped for each count.
+  frame_queue_[best_frame].render_count += remaining_overage;
+  frame_queue_[best_frame].drop_count += remaining_overage;
   return best_frame;
 }
 
 int VideoRendererAlgorithm::FindBestFrameByCadenceInternal(
-    int* adjusted_ideal_render_count) const {
+    int* remaining_overage) const {
   DCHECK(!frame_queue_.empty());
   DCHECK(cadence_estimator_.has_cadence());
   const ReadyFrame& current_frame = frame_queue_[last_frame_index_];
 
+  if (remaining_overage) {
+    DCHECK_EQ(*remaining_overage, 0);
+  }
+
   // If the current frame is below cadence, we should prefer it.
-  if (current_frame.render_count < current_frame.ideal_render_count) {
-    if (adjusted_ideal_render_count)
-      *adjusted_ideal_render_count = current_frame.ideal_render_count;
+  if (current_frame.render_count < current_frame.ideal_render_count)
     return last_frame_index_;
-  }
 
   // For over-rendered frames we need to ensure we skip frames and subtract
   // each skipped frame's ideal cadence from the over-render count until we
@@ -495,10 +499,8 @@ int VideoRendererAlgorithm::FindBestFrameByCadenceInternal(
   for (size_t i = last_frame_index_ + 1; i < frame_queue_.size(); ++i) {
     const ReadyFrame& frame = frame_queue_[i];
     if (frame.ideal_render_count > render_count_overage) {
-      if (adjusted_ideal_render_count) {
-        *adjusted_ideal_render_count =
-            frame.ideal_render_count - render_count_overage;
-      }
+      if (remaining_overage)
+        *remaining_overage = render_count_overage;
       return i;
     } else {
       // The ideal render count should always be zero or smaller than the
diff --git a/media/filters/video_renderer_algorithm.h b/media/filters/video_renderer_algorithm.h
index 61603b3..88e8766 100644
--- a/media/filters/video_renderer_algorithm.h
+++ b/media/filters/video_renderer_algorithm.h
@@ -194,10 +194,14 @@ class MEDIA_EXPORT VideoRendererAlgorithm {
   int FindBestFrameByCadence();
 
   // Similar to FindBestFrameByCadence(), but instead of adjusting the last
-  // rendered frame's ideal render count in the case of over selection,
-  // optionally returns the new ideal render count via
-  // |adjusted_ideal_render_count|.
-  int FindBestFrameByCadenceInternal(int* adjusted_ideal_render_count) const;
+  // rendered frame's ideal render count in the case of over selection.
+  // Optionally returns the number of times a prior frame was over displayed and
+  // ate into the returned frames ideal render count via |remaining_overage|.
+  //
+  // For example, if we have 2 frames and each has an ideal display count of 3,
+  // but the first was displayed 4 times, the best frame is the second one, but
+  // it should only be displayed twice instead of thrice, so it's overage is 1.
+  int FindBestFrameByCadenceInternal(int* remaining_overage) const;
 
   // Iterates over |frame_queue_| and finds the frame which covers the most of
   // the deadline interval.  If multiple frames have coverage of the interval,
diff --git a/media/filters/video_renderer_algorithm_unittest.cc b/media/filters/video_renderer_algorithm_unittest.cc
index 3fb658a..fb74806 100644
--- a/media/filters/video_renderer_algorithm_unittest.cc
+++ b/media/filters/video_renderer_algorithm_unittest.cc
@@ -225,7 +225,7 @@ class VideoRendererAlgorithmTest : public testing::Test {
       } else if (is_using_cadence() && !IsUsingFractionalCadence()) {
         // If there was no glitch in the last render, the two queue sizes should
         // be off by exactly one frame; i.e., the current frame doesn't count.
-        if (!last_render_had_glitch())
+        if (!last_render_had_glitch() && fresh_algorithm)
           ASSERT_EQ(frames_queued() - 1, algorithm_.EffectiveFramesQueued());
       } else if (IsUsingFractionalCadence()) {
         // The frame estimate should be off by at most one frame.
@@ -691,7 +691,7 @@ TEST_F(VideoRendererAlgorithmTest, BestFrameByCadenceOverdisplayed) {
   algorithm_.EnqueueFrame(CreateFrame(frame_tg.interval(1)));
 
   // Render frames until we've exhausted available frames and the last frame is
-  // forced to be overdisplayed.
+  // forced to be over displayed.
   for (int i = 0; i < 5; ++i) {
     size_t frames_dropped = 0;
     scoped_refptr<VideoFrame> frame =
@@ -709,18 +709,26 @@ TEST_F(VideoRendererAlgorithmTest, BestFrameByCadenceOverdisplayed) {
   algorithm_.EnqueueFrame(CreateFrame(frame_tg.interval(3)));
 
   // The next frame should only be displayed once, since the previous one was
-  // overdisplayed by one frame.
+  // over displayed by one frame.
   size_t frames_dropped = 0;
   scoped_refptr<VideoFrame> frame = RenderAndStep(&display_tg, &frames_dropped);
   ASSERT_TRUE(frame);
   EXPECT_EQ(frame_tg.interval(2), frame->timestamp());
   EXPECT_EQ(0u, frames_dropped);
-  ASSERT_EQ(1, GetCurrentFrameIdealDisplayCount());
+
+  // Enqueuing a new frame should keep the correct cadence values.
+  algorithm_.EnqueueFrame(CreateFrame(frame_tg.interval(4)));
+
+  ASSERT_EQ(2, GetCurrentFrameDisplayCount());
+  ASSERT_EQ(1, GetCurrentFrameDropCount());
+  ASSERT_EQ(2, GetCurrentFrameIdealDisplayCount());
 
   frame = RenderAndStep(&display_tg, &frames_dropped);
   ASSERT_TRUE(frame);
   EXPECT_EQ(frame_tg.interval(3), frame->timestamp());
   EXPECT_EQ(0u, frames_dropped);
+  ASSERT_EQ(1, GetCurrentFrameDisplayCount());
+  ASSERT_EQ(0, GetCurrentFrameDropCount());
   ASSERT_EQ(2, GetCurrentFrameIdealDisplayCount());
 }