From b3d4ff0b8f2e1233c8741f302363fe1e4b2ebb5c Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Thu, 20 Feb 2025 08:57:59 +0100 Subject: [PATCH] fix: osr stutter fix backport for electron. (#45657) * fix: osr stutter fix backport for electron. Co-authored-by: reito * Update patches/chromium/.patches Co-authored-by: reito --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: reito Co-authored-by: Shelley Vohr --- patches/chromium/.patches | 1 + ..._gpu_capture_when_page_has_animation.patch | 236 ++++++++++++++++++ 2 files changed, 237 insertions(+) create mode 100644 patches/chromium/fix_osr_stutter_in_both_cpu_and_gpu_capture_when_page_has_animation.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index f327f6100f27..6a7b22f5766e 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -146,3 +146,4 @@ cherry-pick-3a6ff45cc3f4.patch cherry-pick-a51e7ebb7663.patch cherry-pick-f3300abe2fcd.patch remove_persistentmemoryallocator_getallocsize.patch +fix_osr_stutter_in_both_cpu_and_gpu_capture_when_page_has_animation.patch diff --git a/patches/chromium/fix_osr_stutter_in_both_cpu_and_gpu_capture_when_page_has_animation.patch b/patches/chromium/fix_osr_stutter_in_both_cpu_and_gpu_capture_when_page_has_animation.patch new file mode 100644 index 000000000000..a8d4bc910a8f --- /dev/null +++ b/patches/chromium/fix_osr_stutter_in_both_cpu_and_gpu_capture_when_page_has_animation.patch @@ -0,0 +1,236 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: reito +Date: Wed, 12 Feb 2025 20:42:02 +0800 +Subject: fix: osr stutter in both cpu and gpu capture when page has animation. + +https://crrev.org/c/6232721 +https://crbug.com/391118566 +There's bug in VideoCaptureOracle that cause stutter in both cpu and gpu capture when page has animation. +The upstream has a fix, which will be available in Chromium M135. +Backport this fix for Electron versions before that. + +diff --git a/media/capture/content/video_capture_oracle.cc b/media/capture/content/video_capture_oracle.cc +index dad9598799a670b3cfb14965bc8a7b4ea3b4f95f..05a82788ae1e5c304ee150a2390f346d7b454630 100644 +--- a/media/capture/content/video_capture_oracle.cc ++++ b/media/capture/content/video_capture_oracle.cc +@@ -118,8 +118,9 @@ void VideoCaptureOracle::SetCaptureSizeConstraints( + void VideoCaptureOracle::SetAutoThrottlingEnabled(bool enabled) { + const bool was_enabled = + (capture_size_throttling_mode_ != kThrottlingDisabled); +- if (was_enabled == enabled) ++ if (was_enabled == enabled) { + return; ++ } + capture_size_throttling_mode_ = + enabled ? kThrottlingEnabled : kThrottlingDisabled; + VLOG(1) << "Capture size auto-throttling is now " +@@ -127,19 +128,22 @@ void VideoCaptureOracle::SetAutoThrottlingEnabled(bool enabled) { + + // When not auto-throttling, have the CaptureResolutionChooser target the max + // resolution within constraints. +- if (!enabled) ++ if (!enabled) { + resolution_chooser_.SetTargetFrameArea(std::numeric_limits::max()); ++ } + +- if (next_frame_number_ > 0) ++ if (next_frame_number_ > 0) { + CommitCaptureSizeAndReset(GetFrameTimestamp(next_frame_number_ - 1)); ++ } + } + + void VideoCaptureOracle::SetSourceSize(const gfx::Size& source_size) { + resolution_chooser_.SetSourceSize(source_size); + // If the |resolution_chooser_| computed a new capture size, that will become + // visible via a future call to ObserveEventAndDecideCapture(). +- source_size_change_time_ = (next_frame_number_ == 0) ? +- base::TimeTicks() : GetFrameTimestamp(next_frame_number_ - 1); ++ source_size_change_time_ = (next_frame_number_ == 0) ++ ? base::TimeTicks() ++ : GetFrameTimestamp(next_frame_number_ - 1); + } + + bool VideoCaptureOracle::ObserveEventAndDecideCapture( +@@ -172,6 +176,15 @@ bool VideoCaptureOracle::ObserveEventAndDecideCapture( + if (should_sample) { + event_time = content_sampler_.frame_timestamp(); + duration_of_next_frame_ = content_sampler_.sampling_period(); ++ } else { ++ // https://crbug.com/391118566 ++ // The content sampler may not sample the frame, if the ++ // `detected_region_` does not match the `damage_rect`. In this case, ++ // the capture may halt up to kNonAnimatingThreshold (250ms) and cause ++ // the video stutter, until it recovers and do another animation ++ // detection. To avoid this, we should use the smoothing sampler as a ++ // fallback to prevent the bad output. ++ should_sample = smoothing_sampler_.ShouldSample(); + } + last_time_animation_was_detected_ = event_time; + } else { +@@ -198,8 +211,9 @@ bool VideoCaptureOracle::ObserveEventAndDecideCapture( + NOTREACHED(); + } + +- if (!should_sample) ++ if (!should_sample) { + return false; ++ } + + // If the exact duration of the next frame has not been determined, estimate + // it using the difference between the current and last frame. +@@ -373,16 +387,18 @@ void VideoCaptureOracle::RecordConsumerFeedback( + + // resource_utilization feedback. + +- if (capture_size_throttling_mode_ == kThrottlingDisabled) ++ if (capture_size_throttling_mode_ == kThrottlingDisabled) { + return; ++ } + + if (!std::isfinite(feedback.resource_utilization)) { + LOG(DFATAL) << "Non-finite utilization provided by consumer for frame #" + << frame_number << ": " << feedback.resource_utilization; + return; + } +- if (feedback.resource_utilization <= 0.0) ++ if (feedback.resource_utilization <= 0.0) { + return; // Non-positive values are normal, meaning N/A. ++ } + + if (capture_size_throttling_mode_ != kThrottlingActive) { + VLOG(1) << "Received consumer feedback at frame #" << frame_number +@@ -553,12 +569,14 @@ int VideoCaptureOracle::AnalyzeForIncreasedArea(base::TimeTicks analyze_time) { + const int current_area = capture_size_.GetArea(); + const int increased_area = + resolution_chooser_.FindLargerFrameSize(current_area, 1).GetArea(); +- if (increased_area <= current_area) ++ if (increased_area <= current_area) { + return -1; ++ } + + // Determine whether the buffer pool could handle an increase in area. +- if (!HasSufficientRecentFeedback(buffer_pool_utilization_, analyze_time)) ++ if (!HasSufficientRecentFeedback(buffer_pool_utilization_, analyze_time)) { + return -1; ++ } + if (buffer_pool_utilization_.current() > 0.0) { + const int buffer_capable_area = base::saturated_cast( + current_area / buffer_pool_utilization_.current()); +@@ -593,8 +611,9 @@ int VideoCaptureOracle::AnalyzeForIncreasedArea(base::TimeTicks analyze_time) { + + // At this point, the system is currently under-utilized. Reset the start + // time if the system was not under-utilized when the last analysis was made. +- if (start_time_of_underutilization_.is_null()) ++ if (start_time_of_underutilization_.is_null()) { + start_time_of_underutilization_ = analyze_time; ++ } + + // If the under-utilization started soon after the last source size change, + // permit an immediate increase in the capture area. This allows the system +diff --git a/media/capture/content/video_capture_oracle_unittest.cc b/media/capture/content/video_capture_oracle_unittest.cc +index 066676fa998db6782270ddbf42fe176d88eb30d4..6cd7567e91bc8c496846a685aa1506c7548f3a21 100644 +--- a/media/capture/content/video_capture_oracle_unittest.cc ++++ b/media/capture/content/video_capture_oracle_unittest.cc +@@ -158,21 +158,26 @@ TEST(VideoCaptureOracleTest, TransitionsSmoothlyBetweenSamplers) { + const bool provide_animated_content_event = + (i % 100) >= 25 && (i % 100) < 75; + +- // Only the few events that trigger the lock-out transition should be +- // dropped, because the AnimatedContentSampler doesn't yet realize the +- // animation ended. Otherwise, the oracle should always decide to sample +- // because one of its samplers says to. +- const bool require_oracle_says_sample = (i % 100) < 75 || (i % 100) >= 78; ++ // https://crbug.com/391118566 ++ // Previously the AnimatedContentSampler has a bug that cause jank. ++ // The oracle should always use SmoothEventSampler as a fallback. If ++ // AnimatedContentSampler doesn't yet realize the animation ended or ++ // doesn't keep up with the prediction it make, and it will wait for ++ // kNonAnimatingThreshold before it lock-out and hand over to smooth ++ // handler. This will cause the video to stutter and it is unacceptable. ++ // So, when the AnimatedContentSampler goes into wrong state, we now ++ // use SmoothEventSampler's decision as a fallback to prevent jank output ++ // and still has a overall limit on capture frequency. + const bool oracle_says_sample = oracle.ObserveEventAndDecideCapture( + VideoCaptureOracle::kCompositorUpdate, + provide_animated_content_event ? animation_damage_rect : gfx::Rect(), + t); +- if (require_oracle_says_sample) +- ASSERT_TRUE(oracle_says_sample); +- if (!oracle_says_sample) { +- ASSERT_EQ(base::TimeDelta(), oracle.estimated_frame_duration()); +- continue; +- } ++ ++ // Because we now use SmoothEventSampler as a fallback, oracle should ++ // always say sample. The previous AnimatedContentSampler lock-out ++ // dropped frame are now revived by SmoothEventSampler, since this test's ++ // capture frequency always meets min capture limit requirement. ++ ASSERT_TRUE(oracle_says_sample); + ASSERT_LT(base::TimeDelta(), oracle.estimated_frame_duration()); + + const int frame_number = oracle.next_frame_number(); +@@ -184,12 +189,9 @@ TEST(VideoCaptureOracleTest, TransitionsSmoothlyBetweenSamplers) { + if (!last_frame_timestamp.is_null()) { + const base::TimeDelta delta = frame_timestamp - last_frame_timestamp; + EXPECT_LE(event_increment.InMicroseconds(), delta.InMicroseconds()); +- // Right after the AnimatedContentSampler lock-out transition, there were +- // a few frames dropped, so allow a gap in the timestamps. Otherwise, the +- // delta between frame timestamps should never be more than 2X the ++ // The delta between frame timestamps should never be more than 2X the + // |event_increment|. +- const base::TimeDelta max_acceptable_delta = +- (i % 100) == 78 ? event_increment * 5 : event_increment * 2; ++ const base::TimeDelta max_acceptable_delta = event_increment * 2; + EXPECT_GE(max_acceptable_delta.InMicroseconds(), delta.InMicroseconds()); + } + last_frame_timestamp = frame_timestamp; +@@ -444,9 +446,9 @@ void RunAutoThrottleTest(bool is_content_animating, + // expect the resolution to remain constant. Repeat. + for (int i = 0; i < 2; ++i) { + const gfx::Size starting_size = oracle.capture_size(); +- SCOPED_TRACE(::testing::Message() << "Stepping down from " +- << starting_size.ToString() +- << ", i=" << i); ++ SCOPED_TRACE(::testing::Message() ++ << "Stepping down from " << starting_size.ToString() ++ << ", i=" << i); + + gfx::Size stepped_down_size; + end_t = t + base::Seconds(10); +@@ -471,9 +473,10 @@ void RunAutoThrottleTest(bool is_content_animating, + oracle.RecordCapture(with_consumer_feedback ? 0.25 : utilization); + base::TimeTicks ignored; + ASSERT_TRUE(oracle.CompleteCapture(frame_number, true, &ignored)); +- if (with_consumer_feedback) ++ if (with_consumer_feedback) { + oracle.RecordConsumerFeedback(frame_number, + media::VideoCaptureFeedback(utilization)); ++ } + } + } + +@@ -482,9 +485,9 @@ void RunAutoThrottleTest(bool is_content_animating, + // utilization and expect the resolution to remain constant. Repeat. + for (int i = 0; i < 2; ++i) { + const gfx::Size starting_size = oracle.capture_size(); +- SCOPED_TRACE(::testing::Message() << "Stepping up from " +- << starting_size.ToString() +- << ", i=" << i); ++ SCOPED_TRACE(::testing::Message() ++ << "Stepping up from " << starting_size.ToString() ++ << ", i=" << i); + + gfx::Size stepped_up_size; + end_t = t + base::Seconds(is_content_animating ? 90 : 10); +@@ -513,9 +516,10 @@ void RunAutoThrottleTest(bool is_content_animating, + oracle.RecordCapture(with_consumer_feedback ? 0.25 : utilization); + base::TimeTicks ignored; + ASSERT_TRUE(oracle.CompleteCapture(frame_number, true, &ignored)); +- if (with_consumer_feedback) ++ if (with_consumer_feedback) { + oracle.RecordConsumerFeedback(frame_number, + media::VideoCaptureFeedback(utilization)); ++ } + } + } + }