0

[M135] Revert "fix: prevent capture halting when content_sampler_ detects wrong region."

This reverts commit 626bcb3aae.

Reason for revert: Breaks chromeOS tests: capturer produces more than configured 30fps.

Original change's description:
> fix: prevent capture halting when content_sampler_ detects wrong region.
>
> The content sampler may not sample the frame, if the
> (animated_content_sampler.cc:93) 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 may use the smoothing sampler as a fallback to
> prevent the bad output, while still limits the overall capture frequency by the min capture time.
>
> Bug: 391118566
> Change-Id: I9e17190ea53750626d9c9bb699f0db412fcf33f1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6232721
> Commit-Queue: Mark Foltz <mfoltz@chromium.org>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@chromium.org>
> Reviewed-by: Mark Foltz <mfoltz@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1418410}

(cherry picked from commit c5ca2218c8)

Bug: 391118566, 397958082
Fixed: 402025432
Change-Id: Icf75e99b7e432a7fe920229017bb8a03d88c539b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6330304
Reviewed-by: Mark Foltz <mfoltz@chromium.org>
Auto-Submit: Ilya Nikolaevskiy <ilnik@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1429656}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6336110
Commit-Queue: Mark Foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/branch-heads/7049@{#410}
Cr-Branched-From: 2dab7846d0951a552bdc4f350dad497f986e6fed-refs/heads/main@{#1427262}
This commit is contained in:
Ilya Nikolaevskiy
2025-03-10 10:58:05 -07:00
committed by Chromium LUCI CQ
parent 2495e45040
commit 42a40baa50
2 changed files with 35 additions and 58 deletions

@ -118,9 +118,8 @@ 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 "
@ -128,22 +127,19 @@ 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<int>::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(
@ -176,15 +172,6 @@ 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 {
@ -211,9 +198,8 @@ 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.
@ -387,18 +373,16 @@ 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
@ -569,14 +553,12 @@ 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<int>(
current_area / buffer_pool_utilization_.current());
@ -611,9 +593,8 @@ 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

@ -158,26 +158,21 @@ TEST(VideoCaptureOracleTest, TransitionsSmoothlyBetweenSamplers) {
const bool provide_animated_content_event =
(i % 100) >= 25 && (i % 100) < 75;
// 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.
// 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;
const bool oracle_says_sample = oracle.ObserveEventAndDecideCapture(
VideoCaptureOracle::kCompositorUpdate,
provide_animated_content_event ? animation_damage_rect : gfx::Rect(),
t);
// 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);
if (require_oracle_says_sample)
ASSERT_TRUE(oracle_says_sample);
if (!oracle_says_sample) {
ASSERT_EQ(base::TimeDelta(), oracle.estimated_frame_duration());
continue;
}
ASSERT_LT(base::TimeDelta(), oracle.estimated_frame_duration());
const int frame_number = oracle.next_frame_number();
@ -189,9 +184,12 @@ 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());
// The delta between frame timestamps should never be more than 2X the
// 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
// |event_increment|.
const base::TimeDelta max_acceptable_delta = event_increment * 2;
const base::TimeDelta max_acceptable_delta =
(i % 100) == 78 ? event_increment * 5 : event_increment * 2;
EXPECT_GE(max_acceptable_delta.InMicroseconds(), delta.InMicroseconds());
}
last_frame_timestamp = frame_timestamp;
@ -446,9 +444,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);
@ -473,10 +471,9 @@ 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));
}
}
}
@ -485,9 +482,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);
@ -516,10 +513,9 @@ 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));
}
}
}
}