From c1d75875b883fb33981eadcbc6e0af7c5e65775a Mon Sep 17 00:00:00 2001 From: dalecurtis Date: Wed, 6 May 2015 14:04:18 -0700 Subject: [PATCH] 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} --- media/filters/video_renderer_algorithm.cc | 28 ++++++++++++---------- media/filters/video_renderer_algorithm.h | 12 ++++++---- media/filters/video_renderer_algorithm_unittest.cc | 16 +++++++++---- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/media/filters/video_renderer_algorithm.cc b/media/filters/video_renderer_algorithm.cc index a78342aca9e1..5bad074339eb 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 61603b3e95a3..88e8766dab15 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 3fb658a5569a..fb748066d507 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 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 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()); } -- 2.11.4.GIT