Allow initial BWE probing with Scream without configured media Similar to Goog CC, allow padding packets to be sent the first 6s seconds, up to the configured max target bitrate, even if no max allocated bitrate per streams is set. Probing without media must be allowed. These "probes" are sent every 3s during the first 6s. Ie, if the initial attempt is aborted due to congestion, a second attempt will be made 3s later. Bug: webrtc:447037083 Change-Id: I8d32e1ac2eebe5cc84142995d2332ffdc954ecb4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/458300 Reviewed-by: Björn Terelius <terelius@webrtc.org> Commit-Queue: Per Kjellander <perkj@webrtc.org> Cr-Commit-Position: refs/heads/main@{#47213}
diff --git a/modules/congestion_controller/scream/scream_network_controller.cc b/modules/congestion_controller/scream/scream_network_controller.cc index 396c2be..e20e589 100644 --- a/modules/congestion_controller/scream/scream_network_controller.cc +++ b/modules/congestion_controller/scream/scream_network_controller.cc
@@ -46,8 +46,7 @@ streams_config_(config.stream_based_config), max_seen_total_allocated_bitrate_( config.stream_based_config.max_total_allocated_bitrate.value_or( - DataRate::Zero())), - last_padding_interval_started_(Timestamp::Zero()) { + DataRate::Zero())) { UpdateScreamTargetBitrateConstraints(); } @@ -65,6 +64,9 @@ RTC_DCHECK(network_available_); RTC_DCHECK(!first_update_created_); first_update_created_ = true; + if (allow_initial_bwe_before_media_) { + initial_bwe_probe_end_time_ = now + params_.initial_probing_duration.Get(); + } NetworkControlUpdate update = CreateUpdate(now); if (allow_initial_bwe_before_media_) { @@ -87,8 +89,7 @@ NetworkControlUpdate ScreamNetworkController::OnNetworkAvailability( NetworkAvailability msg) { network_available_ = msg.network_available; - if (!first_update_created_ && network_available_ && - streams_config_.max_total_allocated_bitrate > DataRate::Zero()) { + if (!first_update_created_ && network_available_) { return CreateFirstUpdate(msg.at_time); } return NetworkControlUpdate(); @@ -112,8 +113,9 @@ NetworkControlUpdate ScreamNetworkController::OnProcessInterval( ProcessInterval msg) { - // Scream currently has no need for periodic processing. - return NetworkControlUpdate(); + NetworkControlUpdate update; + update.pacer_config = MaybeCreatePacerConfig(msg.at_time); + return update; } NetworkControlUpdate ScreamNetworkController::OnRemoteBitrateReport( @@ -204,12 +206,13 @@ target_rate_msg.network_estimate.bwe_period = TimeDelta::Millis(25); update.target_rate = target_rate_msg; } - update.pacer_config = MaybeCreatePacerConfig(); + update.pacer_config = MaybeCreatePacerConfig(now); update.congestion_window = scream_->max_data_in_flight(); return update; } -std::optional<PacerConfig> ScreamNetworkController::MaybeCreatePacerConfig() { +std::optional<PacerConfig> ScreamNetworkController::MaybeCreatePacerConfig( + Timestamp now) { // Allow sending packets in larger bursts if some time has passed since last // congestion event. TimeDelta pacing_window = @@ -218,29 +221,42 @@ params_.allow_large_pacing_bursts_after_congestion_time.Get()) ? default_pacing_window_ : TimeDelta::Millis(10); - DataRate target_rate = scream_->target_rate(); - Timestamp now = env_.clock().CurrentTime(); - DataRate padding_rate = DataRate::Zero(); - // Allow padding if needed. Note that current max needed by streams may be - // lower than what the user intended since it depends on video resolution - // that may be scaled down due to low quality. - DataRate max_padding_rate = - std::min({max_target_rate_, max_seen_total_allocated_bitrate_, - 2 * streams_config_.max_total_allocated_bitrate.value_or( - DataRate::Zero()), - remote_bitrate_report_.value_or(DataRate::PlusInfinity())}); - if (target_rate < max_padding_rate && + + bool allow_padding = false; + if (params_.time_between_periodic_padding.Get().IsFinite() && now - scream_->last_reference_window_decrease_time() > params_.allow_padding_after_last_congestion_time) { - if (params_.periodic_padding_interval->IsFinite() && - (now - last_padding_interval_started_ > - params_.periodic_padding_interval.Get())) { - last_padding_interval_started_ = now; + if (now < padding_interval_end_time_) { + // We are currently inside an active padding duration. + allow_padding = true; + } else if (now - padding_interval_end_time_ >= + params_.time_between_periodic_padding.Get()) { + // Enough time has passed since the end of the last padding interval; + // start a new one. + padding_interval_end_time_ = + now + params_.periodic_padding_duration.Get(); + allow_padding = true; } - if (now - last_padding_interval_started_ < - params_.periodic_padding_duration.Get()) { - padding_rate = target_rate; + } else { + // Stop padding immediately if a congestion event occurred recently. + padding_interval_end_time_ = Timestamp::MinusInfinity(); + } + + DataRate padding_rate = DataRate::Zero(); + if (allow_padding) { + // Padding is allowed. + DataRate max_padding_rate = + std::min({max_target_rate_, max_seen_total_allocated_bitrate_, + 2 * streams_config_.max_total_allocated_bitrate.value_or( + DataRate::Zero()), + remote_bitrate_report_.value_or(DataRate::PlusInfinity())}); + if (max_padding_rate.IsZero() && now < initial_bwe_probe_end_time_) { + // If initial BWE probing is allowed, probe up to the max target rate. + max_padding_rate = max_target_rate_; } + DataRate target_rate = scream_->target_rate(); + padding_rate = + target_rate < max_padding_rate ? target_rate : DataRate::Zero(); } DataRate pacing_rate = scream_->pacing_rate();
diff --git a/modules/congestion_controller/scream/scream_network_controller.h b/modules/congestion_controller/scream/scream_network_controller.h index 4dfebff..2a03ce6 100644 --- a/modules/congestion_controller/scream/scream_network_controller.h +++ b/modules/congestion_controller/scream/scream_network_controller.h
@@ -50,7 +50,7 @@ void UpdateScreamTargetBitrateConstraints(); NetworkControlUpdate CreateFirstUpdate(Timestamp now); NetworkControlUpdate CreateUpdate(Timestamp now); - std::optional<PacerConfig> MaybeCreatePacerConfig(); + std::optional<PacerConfig> MaybeCreatePacerConfig(Timestamp now); Environment env_; const ScreamV2Parameters params_; @@ -66,8 +66,8 @@ std::optional<DataRate> remote_bitrate_report_; StreamsConfig streams_config_; DataRate max_seen_total_allocated_bitrate_ = DataRate::Zero(); - - Timestamp last_padding_interval_started_; + Timestamp initial_bwe_probe_end_time_ = Timestamp::MinusInfinity(); + Timestamp padding_interval_end_time_ = Timestamp::MinusInfinity(); // Values last reported in a NetworkControlUpdate. Used for finding out if an // update needs to be reported.
diff --git a/modules/congestion_controller/scream/scream_network_controller_unittest.cc b/modules/congestion_controller/scream/scream_network_controller_unittest.cc index 1a9dcab..9dc4280 100644 --- a/modules/congestion_controller/scream/scream_network_controller_unittest.cc +++ b/modules/congestion_controller/scream/scream_network_controller_unittest.cc
@@ -48,8 +48,8 @@ DataRate::KilobitsPerSec(456); ScreamNetworkController scream_controller(config); - NetworkControlUpdate update = - scream_controller.OnNetworkAvailability({.network_available = true}); + NetworkControlUpdate update = scream_controller.OnNetworkAvailability( + {.at_time = clock.CurrentTime(), .network_available = true}); ASSERT_TRUE(update.has_updates()); ASSERT_TRUE(update.target_rate.has_value()); EXPECT_EQ(update.target_rate->target_rate, config.constraints.starting_rate); @@ -92,7 +92,8 @@ config.stream_based_config.max_total_allocated_bitrate = DataRate::KilobitsPerSec(1000); ScreamNetworkController scream_controller(config); - scream_controller.OnNetworkAvailability({.network_available = true}); + scream_controller.OnNetworkAvailability( + {.at_time = clock.CurrentTime(), .network_available = true}); CcFeedbackGenerator feedback_generator({}); DataRate send_rate = DataRate::KilobitsPerSec(100); @@ -312,6 +313,8 @@ StreamsConfig streams_config; streams_config.max_total_allocated_bitrate = DataRate::KilobitsPerSec(1000); scream_controller.OnStreamsConfig(streams_config); + NetworkControlUpdate update = scream_controller.OnNetworkAvailability( + {.at_time = clock.CurrentTime(), .network_available = true}); DataRate send_rate = DataRate::KilobitsPerSec(50); DataRate target_rate = DataRate::Zero(); @@ -323,8 +326,7 @@ feedback_generator.ProcessUntilNextFeedback( send_rate, clock, [&](SentPacket packet) { scream_controller.OnSentPacket(packet); }); - NetworkControlUpdate update = - scream_controller.OnTransportPacketsFeedback(feedback); + update = scream_controller.OnTransportPacketsFeedback(feedback); if (update.pacer_config.has_value()) { if (update.pacer_config->pad_rate() != DataRate::Zero()) { padding_set = true; @@ -355,6 +357,35 @@ EXPECT_LT(padding_stop - start_time, TimeDelta::Seconds(1)); } +TEST(ScreamControllerTest, InitialProbingWithoutMaxTotalAllocatedBitrate) { + SimulatedClock clock(Timestamp::Seconds(1'234)); + Environment env = CreateTestEnvironment({.time = &clock}); + NetworkControllerConfig config(env); + config.stream_based_config.enable_repeated_initial_probing = true; + config.constraints.starting_rate = DataRate::KilobitsPerSec(100); + config.constraints.max_data_rate = DataRate::KilobitsPerSec(1000); + // Do not set config.stream_based_config.max_total_allocated_bitrate + + ScreamNetworkController scream_controller(config); + NetworkControlUpdate update = scream_controller.OnNetworkAvailability( + {.at_time = clock.CurrentTime(), .network_available = true}); + + ASSERT_TRUE(update.pacer_config.has_value()); + // Padding is allowed during the first 6 seconds even if + // max_total_allocated_bitrate is zero. + EXPECT_EQ(update.pacer_config->pad_rate(), config.constraints.starting_rate); + + // Advance clock past the 6s initial BWE probe window. + clock.AdvanceTime(TimeDelta::Seconds(7)); + update = + scream_controller.OnProcessInterval({.at_time = clock.CurrentTime()}); + // Since max_total_allocated_bitrate is not set, padding should stop after the + // initial probe window. + if (update.pacer_config.has_value()) { + EXPECT_EQ(update.pacer_config->pad_rate(), DataRate::Zero()); + } +} + struct PaddingTestResult { DataRate target_rate; Timestamp padding_start; @@ -450,7 +481,7 @@ TimeDelta time_between_padding = result_2.padding_start - result_1.padding_stop; EXPECT_GT(padding_duration, TimeDelta::Millis(2500)); - EXPECT_LT(padding_duration, TimeDelta::Millis(3100)); + EXPECT_LT(padding_duration, TimeDelta::Millis(3300)); EXPECT_GT(time_between_padding, TimeDelta::Millis(2500)); EXPECT_LT(time_between_padding, TimeDelta::Millis(3300));
diff --git a/modules/congestion_controller/scream/scream_v2_parameters.cc b/modules/congestion_controller/scream/scream_v2_parameters.cc index 7c70de1..68f6fe3 100644 --- a/modules/congestion_controller/scream/scream_v2_parameters.cc +++ b/modules/congestion_controller/scream/scream_v2_parameters.cc
@@ -52,11 +52,12 @@ queue_delay_drain_threshold("QDelayDrainThreshold", TimeDelta::Millis(5)), queue_delay_drain_period("QDelayDrainPeriod", TimeDelta::Seconds(20)), queue_delay_drain_rtts("QDelayDrainRtts", 5), - periodic_padding_interval("PeriodicPadding", TimeDelta::Seconds(6)), + time_between_periodic_padding("PeriodicPadding", TimeDelta::Seconds(3)), periodic_padding_duration("PaddingDuration", TimeDelta::Seconds(3)), allow_padding_after_last_congestion_time( "AllowPaddingAfterLastCongestionTimeout", TimeDelta::Seconds(1)), + initial_probing_duration("InitialProbingDuration", TimeDelta::Seconds(6)), pacing_factor("PacingFactor", 1.1), feedback_hold_time_avg_g("FeedbackHoldTimeAvgG", 1.0 / 8.0), allow_large_pacing_bursts_after_congestion_time( @@ -87,9 +88,10 @@ &queue_delay_drain_threshold, &queue_delay_drain_period, &queue_delay_drain_rtts, - &periodic_padding_interval, + &time_between_periodic_padding, &periodic_padding_duration, &allow_padding_after_last_congestion_time, + &initial_probing_duration, &pacing_factor, &feedback_hold_time_avg_g, &allow_large_pacing_bursts_after_congestion_time},
diff --git a/modules/congestion_controller/scream/scream_v2_parameters.h b/modules/congestion_controller/scream/scream_v2_parameters.h index 9dafdba..bd4c129 100644 --- a/modules/congestion_controller/scream/scream_v2_parameters.h +++ b/modules/congestion_controller/scream/scream_v2_parameters.h
@@ -108,8 +108,9 @@ FieldTrialParameter<int> queue_delay_drain_rtts; // Padding is periodically used in order to increase target rate even if a - // stream does not produce a high enough rate. - FieldTrialParameter<TimeDelta> periodic_padding_interval; + // stream does not produce a high enough rate. Time between periodic padding + // is at least this long. + FieldTrialParameter<TimeDelta> time_between_periodic_padding; // Max duration padding is used when periodic padding start. // Padding is stopped if congestion occur. FieldTrialParameter<TimeDelta> periodic_padding_duration; @@ -117,6 +118,9 @@ // time reference window was reduced but at least `periodic_padding_interval` // must have passed since last time padding was used. FieldTrialParameter<TimeDelta> allow_padding_after_last_congestion_time; + // If initial BWE probing is allowed, allow periodic probing for this duration + // even of no streams have been configured. + FieldTrialParameter<TimeDelta> initial_probing_duration; // Factor multiplied by the current target rate to decide the pacing rate. FieldTrialParameter<double> pacing_factor;
diff --git a/test/peer_scenario/bwe_integration_tests/scream_test.cc b/test/peer_scenario/bwe_integration_tests/scream_test.cc index de61e23..ef5f84c 100644 --- a/test/peer_scenario/bwe_integration_tests/scream_test.cc +++ b/test/peer_scenario/bwe_integration_tests/scream_test.cc
@@ -364,7 +364,7 @@ SendMediaTestResult result = SendMediaInOneDirection(std::move(params), s); EXPECT_THAT(result.caller().subspan(1), Each(AvailableSendBitrateIsBetween( DataRate::KilobitsPerSec(200), - DataRate::KilobitsPerSec(800)))); + DataRate::KilobitsPerSec(900)))); } TEST(ScreamTest, MaybeTest(LinkCapacity600KbpsRtt100msEcn)) { @@ -467,7 +467,7 @@ SendMediaTestResult result = SendMediaInOneDirection(std::move(params), s); EXPECT_THAT(result.caller().subspan(1), Each(AvailableSendBitrateIsBetween( DataRate::KilobitsPerSec(800), - DataRate::KilobitsPerSec(2000)))); + DataRate::KilobitsPerSec(2200)))); } TEST(ScreamTest, MaybeTest(LinkCapacity2MbpsRtt50msNoEcn)) { @@ -564,8 +564,8 @@ // BWE rampup is quite slow since feedback is only sent every 90ms // approximately. EXPECT_THAT(result.caller().subspan(5), Each(AvailableSendBitrateIsBetween( - DataRate::KilobitsPerSec(1200), - DataRate::KilobitsPerSec(2400)))); + DataRate::KilobitsPerSec(700), + DataRate::KilobitsPerSec(2600)))); } TEST(ScreamTest, MaybeTest(CallerPauseSendingVideoIfFeedbackNotReceived)) {