Reland "SCReAM: Scale backoff by feedback interval and reset ref window on congestion" This is a reland of commit 6214048b9dccb3abf7e1ab858396c4cfb0849703 Internal tests are fixed. Original change's description: > SCReAM: Scale backoff by feedback interval and reset ref window on congestion > > This CL: > 1. Tracks feedback interval in ScreamV2 and scales backoff relative > to effective virtual RTT (max of virtual RTT and feedback interval) > to account for how often feedback actually arrives. This avoids > stalled backoff during low bitrate audio with large feedback intervals. > 2. Clamps the reference window to data in flight on first congestion > in delay-based congestion control to eliminate initial rampup overshoot. > 3. Adds 'FeedbackIntervalAvgG' and 'UseFeedbackIntervalForVirtualRtt' > parameters to ScreamV2Parameters. > 4. Tightens expectations in AudioOnlyBothDirectionsLinkCapacity40To90KbpsRtt200ms > to reflect lower RTT and tighter BWE. > > Bug: webrtc:447037083 > Change-Id: I79d8cfff9e4ac034d8192a3668ea3342812ce141 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/506900 > Commit-Queue: Per Kjellander <perkj@webrtc.org> > Reviewed-by: Björn Terelius <terelius@webrtc.org> > Cr-Commit-Position: refs/heads/main@{#48777} Bug: webrtc:447037083 Change-Id: I2c09879692744a39720a52b5916757a4797e1f05 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/507460 Reviewed-by: Erik Språng <sprang@webrtc.org> Commit-Queue: Per Kjellander <perkj@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48788}
diff --git a/modules/congestion_controller/scream/scream_network_controller_unittest.cc b/modules/congestion_controller/scream/scream_network_controller_unittest.cc index d5a34e9..21e1c69 100644 --- a/modules/congestion_controller/scream/scream_network_controller_unittest.cc +++ b/modules/congestion_controller/scream/scream_network_controller_unittest.cc
@@ -1145,5 +1145,60 @@ PacerConfig::kDefaultTimeInterval); } +void TestClampsRefWindowOnFirstCongestionWhenStartingRateIsHigh( + bool send_as_ect1) { + SimulatedClock clock(Timestamp::Seconds(1'234)); + Environment env = CreateTestEnvironment({.time = &clock}); + NetworkControllerConfig config(env); + config.constraints.starting_rate = DataRate::KilobitsPerSec(300); + config.constraints.min_data_rate = DataRate::KilobitsPerSec(20); + config.constraints.max_data_rate = DataRate::KilobitsPerSec(1000); + config.stream_based_config.max_total_allocated_bitrate = + DataRate::KilobitsPerSec(1000); + ScreamNetworkController scream_controller(config); + scream_controller.OnNetworkAvailability( + {.at_time = clock.CurrentTime(), .network_available = true}); + + // RTT is 200ms (100ms one-way delay), link capacity is 30 kbps. + CcFeedbackGenerator feedback_generator({ + .network_config = {.queue_delay_ms = 100, + .link_capacity = DataRate::KilobitsPerSec(30)}, + .send_as_ect1 = send_as_ect1, + .packet_size = DataSize::Bytes(150), + }); + + TimeDelta rtt = TimeDelta::Millis(200); + Timestamp start_time = clock.CurrentTime(); + DataRate target_rate = config.constraints.starting_rate.value(); + while (target_rate > DataRate::KilobitsPerSec(60) && + clock.CurrentTime() - start_time < 10 * rtt) { + TransportPacketsFeedback feedback = + feedback_generator.ProcessUntilNextFeedback( + target_rate, clock, [&](const SentPacket& packet) { + scream_controller.OnSentPacket(packet); + }); + NetworkControlUpdate update = + scream_controller.OnTransportPacketsFeedback(feedback); + if (update.target_rate.has_value()) { + target_rate = update.target_rate->target_rate; + } + } + EXPECT_LE(target_rate, DataRate::KilobitsPerSec(60)); + EXPECT_GE(target_rate, DataRate::KilobitsPerSec(20)); + EXPECT_LE(clock.CurrentTime() - start_time, 2 * rtt); +} + +TEST(ScreamControllerTest, + ClampsRefWindowOnFirstDelayCongestionWhenStartingRateIsHigh) { + TestClampsRefWindowOnFirstCongestionWhenStartingRateIsHigh( + /*send_as_ect1=*/false); +} + +TEST(ScreamControllerTest, + ClampsRefWindowOnFirstCeCongestionWhenStartingRateIsHigh) { + TestClampsRefWindowOnFirstCongestionWhenStartingRateIsHigh( + /*send_as_ect1=*/true); +} + } // namespace } // namespace webrtc
diff --git a/modules/congestion_controller/scream/scream_v2.cc b/modules/congestion_controller/scream/scream_v2.cc index 33ece85..ec23cbb 100644 --- a/modules/congestion_controller/scream/scream_v2.cc +++ b/modules/congestion_controller/scream/scream_v2.cc
@@ -28,13 +28,13 @@ namespace webrtc { - - ScreamV2::ScreamV2(const Environment& env) : env_(env), params_(env_.field_trials()), ref_window_(params_.min_ref_window.Get()), loss_estimator_(params_), + allow_initial_ref_window_clamping_( + params_.allow_initial_ref_window_clamping.Get()), delay_based_congestion_control_(params_) {} void ScreamV2::SetTargetBitrateConstraints(DataRate min, @@ -93,6 +93,7 @@ first_feedback_processed_ = true; } + UpdateFeedbackInterval(feedback.feedback_time); UpdateL4SAlpha(feedback); UpdateRefWindow(feedback); UpdateTargetRate(feedback); @@ -132,33 +133,46 @@ bool is_ce = parsed.num_ce_marked_packets > 0; bool is_loss = loss_estimator_.Update(parsed, delay_based_congestion_control_.rtt()); - bool is_virtual_ce = false; - if (delay_based_congestion_control_.IsQueueDelayDetected()) { - is_virtual_ce = true; - } + bool is_virtual_ce = delay_based_congestion_control_.IsQueueDelayDetected(); if (is_loss) { - if (!loss_estimator_.congested() && - !delay_based_congestion_control_.IsQueueDelayDetected()) { + if (!loss_estimator_.congested() && !is_virtual_ce) { is_loss = false; } } DataSize previous_ref_window = ref_window_; + TimeDelta time_since_last_reaction = + parsed.feedback_time - last_reaction_to_congestion_time_; + TimeDelta reaction_interval = + params_.use_feedback_interval_for_virtual_rtt.Get() + ? std::max(params_.virtual_rtt.Get(), feedback_interval_) + : params_.virtual_rtt.Get(); + if ((is_virtual_ce || is_ce || is_loss) && - parsed.feedback_time - last_reaction_to_congestion_time_ >= - std::min(delay_based_congestion_control_.rtt(), - params_.virtual_rtt.Get())) { + time_since_last_reaction >= + std::min(delay_based_congestion_control_.rtt(), reaction_interval)) { last_reaction_to_congestion_time_ = parsed.feedback_time; + if (allow_initial_ref_window_clamping_ && received_rate_.IsFinite()) { + // Clamp ref_window to received rate on the first congestion event, + // to adjust if starting rate was much higher than link capacity. + ref_window_ = + std::clamp(received_rate_ * (delay_based_congestion_control_.rtt() + + feedback_hold_time_), + params_.min_ref_window.Get(), ref_window_); + allow_initial_ref_window_clamping_ = false; + } + double backoff = 0.0; if (is_loss) { // Back off due to loss backoff = 1.0 - params_.beta_loss.Get(); } else if (is_ce) { // Backoff due to ECN-CE marking backoff = l4s_alpha_ / 2.0; // Scale down backoff when RTT is high as several backoff events occur - // per RTT + // per RTT. Scaling is relative to reaction_interval to account for + // how often feedback is actually received. backoff /= std::max( - 1.0, delay_based_congestion_control_.rtt() / params_.virtual_rtt); + 1.0, delay_based_congestion_control_.rtt() / reaction_interval); if (!delay_based_congestion_control_.IsQueueDelayDetected()) { // Scale down backoff if close to the last known max reference window @@ -174,16 +188,18 @@ .ref_window_scale_factor_due_to_avg_min_delay()); } - if (parsed.feedback_time - last_reaction_to_congestion_time_ > - params_.number_of_rtts_between_reset_ref_window_i_on_congestion - .Get() * - std::max(params_.virtual_rtt.Get(), - delay_based_congestion_control_.rtt())) { + if (time_since_last_reaction.IsFinite() && + time_since_last_reaction > + params_.number_of_rtts_between_reset_ref_window_i_on_congestion + .Get() * + std::max(params_.virtual_rtt.Get(), + delay_based_congestion_control_.rtt()) && + max_data_in_flight_prev_rtt_ > DataSize::Zero()) { // A long time(> 100 RTTs) since last congested because - // link throughput exceeds max video bitrate. (or first congestion) + // link throughput exceeds max video bitrate. // There is a certain risk that ref_wnd has increased way above // bytes in flight, so we reduce it here to get it better on - // track and thus the congestion episode is shortened + // track and thus the congestion episode is shortened. ref_window_ = std::clamp(max_data_in_flight_prev_rtt_, params_.min_ref_window.Get(), ref_window_); // In addition, bump up l4sAlpha to a more credible value @@ -193,8 +209,8 @@ } } else if (is_virtual_ce) { // Back off due to delay backoff = delay_based_congestion_control_.l4s_alpha_v() / 2.0; - backoff /= std::max(1.0, delay_based_congestion_control_.rtt() / - params_.virtual_rtt.Get()); + backoff /= std::max( + 1.0, delay_based_congestion_control_.rtt() / reaction_interval); } ref_window_ = (1.0 - backoff) * ref_window_; } @@ -323,6 +339,21 @@ (1.0 - params_.feedback_hold_time_avg_g.Get()) * feedback_hold_time_; } +void ScreamV2::UpdateFeedbackInterval(Timestamp feedback_time) { + if (last_feedback_time_.IsFinite()) { + TimeDelta delta = feedback_time - last_feedback_time_; + if (delta > TimeDelta::Zero()) { + if (feedback_interval_.IsZero()) { + feedback_interval_ = delta; + } else { + const double g = params_.feedback_interval_avg_g.Get(); + feedback_interval_ = delta * g + feedback_interval_ * (1.0 - g); + } + } + } + last_feedback_time_ = feedback_time; +} + void ScreamV2::UpdateTargetRate(const ScreamFeedback& parsed) { // Avoid division by zero. const TimeDelta non_zero_smoothed_rtt =
diff --git a/modules/congestion_controller/scream/scream_v2.h b/modules/congestion_controller/scream/scream_v2.h index 59add75..87c2f37 100644 --- a/modules/congestion_controller/scream/scream_v2.h +++ b/modules/congestion_controller/scream/scream_v2.h
@@ -132,6 +132,7 @@ void UpdateL4SAlpha(const ScreamFeedback& parsed); void UpdateRefWindow(const ScreamFeedback& parsed); void UpdateFeedbackHoldTime(TimeDelta feedback_hold_time); + void UpdateFeedbackInterval(Timestamp feedback_time); void UpdateTargetRate(const ScreamFeedback& parsed); void UpdateReceiveRate(const ScreamFeedback& parsed); @@ -163,11 +164,14 @@ LossEstimator loss_estimator_; TimeDelta feedback_hold_time_ = TimeDelta::Zero(); + Timestamp last_feedback_time_ = Timestamp::MinusInfinity(); + TimeDelta feedback_interval_ = TimeDelta::Zero(); // Per-RTT stats Timestamp last_data_in_flight_update_ = Timestamp::MinusInfinity(); DataSize max_data_in_flight_this_rtt_ = DataSize::Zero(); DataSize max_data_in_flight_prev_rtt_ = DataSize::Zero(); + bool allow_initial_ref_window_clamping_ = true; DataRate received_rate_ = DataRate::Zero(); DataSize accumulated_received_bytes_ = DataSize::Zero(); Timestamp last_window_receive_time_ = Timestamp::MinusInfinity();
diff --git a/modules/congestion_controller/scream/scream_v2_parameters.cc b/modules/congestion_controller/scream/scream_v2_parameters.cc index 13d72de..8e1e4df 100644 --- a/modules/congestion_controller/scream/scream_v2_parameters.cc +++ b/modules/congestion_controller/scream/scream_v2_parameters.cc
@@ -28,7 +28,6 @@ l4s_avg_g_down("L4sAvgGDown", 1.0 / 128.0), rtts_with_loss_before_backoff("RttsWithLossBeforeBackoff", 3), lossless_rtts_before_clear("LosslessRttsBeforeClear", 2), - smoothed_rtt_avg_g("SmoothedRttAvgG", 1.0 / 8.0), smoothed_rtt_avg_in_alr_g("SmoothedRttAvgInAlrG", 1.0 / 128.0), max_segment_size("MaxSegmentSize", DataSize::Bytes(1280)), @@ -72,6 +71,10 @@ pacing_factor("PacingFactor", 1.1), pacing_rate_received_factor("PacingRateReceivedFactor", 0.8), feedback_hold_time_avg_g("FeedbackHoldTimeAvgG", 1.0 / 8.0), + feedback_interval_avg_g("FeedbackIntervalAvgG", 0.1), + use_feedback_interval_for_virtual_rtt("UseFeedbackIntervalForVirtualRtt", + true), + allow_initial_ref_window_clamping("AllowInitialRefWindowClamping", true), allow_large_pacing_bursts_after_congestion_time( "AllowLargePacingBurstsAfterCongestionTime", TimeDelta::Seconds(15)), @@ -122,6 +125,9 @@ &pacing_factor, &pacing_rate_received_factor, &feedback_hold_time_avg_g, + &feedback_interval_avg_g, + &use_feedback_interval_for_virtual_rtt, + &allow_initial_ref_window_clamping, &allow_large_pacing_bursts_after_congestion_time, &enable_alr, &alr_threshold,
diff --git a/modules/congestion_controller/scream/scream_v2_parameters.h b/modules/congestion_controller/scream/scream_v2_parameters.h index ee822d0..a919c67 100644 --- a/modules/congestion_controller/scream/scream_v2_parameters.h +++ b/modules/congestion_controller/scream/scream_v2_parameters.h
@@ -154,6 +154,18 @@ // received until feedback is sent. If zero, this delay is ignored. FieldTrialParameter<double> feedback_hold_time_avg_g; + // Exponentially Weighted Moving Average (EWMA) factor for tracking average + // feedback interval. + FieldTrialParameter<double> feedback_interval_avg_g; + + // If true, effective virtual_rtt is calculated as + // std::max(virtual_rtt, feedback_interval). Otherwise, virtual_rtt is used. + FieldTrialParameter<bool> use_feedback_interval_for_virtual_rtt; + + // If true, reference window is clamped to received rate on the first + // congestion event after the first RTT. + FieldTrialParameter<bool> allow_initial_ref_window_clamping; + // If the time since last reaction to congestion is larger than this, the // pacing window is increased. I.e. packets are allowed to be sent in larger // bursts.
diff --git a/test/peer_scenario/bwe_integration_tests/scream_test.cc b/test/peer_scenario/bwe_integration_tests/scream_test.cc index f708265..2e4b966 100644 --- a/test/peer_scenario/bwe_integration_tests/scream_test.cc +++ b/test/peer_scenario/bwe_integration_tests/scream_test.cc
@@ -1159,17 +1159,17 @@ ASSERT_GT(result.caller().subspan(2).size(), 0u); EXPECT_THAT(result.caller().subspan(2), Each(CurrentRoundTripTimeIsBetween(TimeDelta::Millis(180), - TimeDelta::Millis(700)))); + TimeDelta::Millis(500)))); ASSERT_GT(result.callee().subspan(2).size(), 0u); EXPECT_THAT(result.callee().subspan(2), Each(CurrentRoundTripTimeIsBetween(TimeDelta::Millis(180), - TimeDelta::Millis(700)))); + TimeDelta::Millis(500)))); EXPECT_THAT(result.caller().subspan(2), Each(AvailableSendBitrateIsBetween( - DataRate::KilobitsPerSec(5), DataRate::KilobitsPerSec(90)))); + DataRate::KilobitsPerSec(5), DataRate::KilobitsPerSec(60)))); EXPECT_THAT(result.callee().subspan(2), Each(AvailableSendBitrateIsBetween( - DataRate::KilobitsPerSec(5), DataRate::KilobitsPerSec(90)))); + DataRate::KilobitsPerSec(5), DataRate::KilobitsPerSec(60)))); } } // namespace