From cc92b6e1bd101f69517da7fa0c80eacf37b65621 Mon Sep 17 00:00:00 2001 From: Per K Date: Tue, 7 May 2024 15:35:09 +0000 Subject: [PATCH] Add ProbeController::EnableRepeatedInitialProbing This adds a new mode to the ProbeController that sends probes every second the first 5seconds. Intented usage is before normal media has started flowing. If enabled, the max allocated bitrat is ignored during these initial probes. The purpose is to have a more accurate estimate at the beginning of a call. The cl also removes ProbeController::SetFirstProbeToMaxBitrate. Bug: webrtc:14928 Change-Id: I04feefb2f1b82ff38b35ee2be810f9119c53536a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349924 Reviewed-by: Diep Bui Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#42252} --- api/transport/network_types.h | 8 +-- call/rtp_transport_controller_send.cc | 3 +- .../goog_cc/goog_cc_network_control.cc | 7 ++- .../goog_cc/probe_controller.cc | 47 ++++++++++---- .../goog_cc/probe_controller.h | 19 ++++-- .../goog_cc/probe_controller_unittest.cc | 62 +++++++------------ 6 files changed, 81 insertions(+), 65 deletions(-) diff --git a/api/transport/network_types.h b/api/transport/network_types.h index 46470d6b5d..c922cb322d 100644 --- a/api/transport/network_types.h +++ b/api/transport/network_types.h @@ -46,10 +46,10 @@ struct StreamsConfig { ~StreamsConfig(); Timestamp at_time = Timestamp::PlusInfinity(); absl::optional requests_alr_probing; - // If `initial_probe_to_max_bitrate` is set to true, the first probe - // may probe up to the max configured bitrate and can ignore - // max_total_allocated_bitrate. - absl::optional initial_probe_to_max_bitrate; + // If `enable_repeated_initial_probing` is set to true, Probes are sent + // periodically every 1s during the first 5s after the network becomes + // available. The probes ignores max_total_allocated_bitrate. + absl::optional enable_repeated_initial_probing; absl::optional pacing_factor; // TODO(srte): Use BitrateAllocationLimits here. diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index d739ed1ddd..6a168de924 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -263,9 +263,10 @@ void RtpTransportControllerSend::ReconfigureBandwidthEstimation( RTC_DCHECK_RUN_ON(&sequence_checker_); bwe_settings_ = settings; + streams_config_.enable_repeated_initial_probing = + bwe_settings_.allow_probe_without_media; bool allow_probe_without_media = bwe_settings_.allow_probe_without_media && packet_router_.SupportsRtxPayloadPadding(); - streams_config_.initial_probe_to_max_bitrate = allow_probe_without_media; pacer_.SetAllowProbeWithoutMediaPacket(allow_probe_without_media); if (controller_) { diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc index 09ac8caef2..e485f73514 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -221,9 +221,10 @@ NetworkControlUpdate GoogCcNetworkController::OnProcessInterval( probe_controller_->EnablePeriodicAlrProbing( *initial_config_->stream_based_config.requests_alr_probing); } - if (initial_config_->stream_based_config.initial_probe_to_max_bitrate) { - probe_controller_->SetFirstProbeToMaxBitrate( - *initial_config_->stream_based_config.initial_probe_to_max_bitrate); + if (initial_config_->stream_based_config.enable_repeated_initial_probing) { + probe_controller_->EnableRepeatedInitialProbing( + *initial_config_->stream_based_config + .enable_repeated_initial_probing); } absl::optional total_bitrate = initial_config_->stream_based_config.max_total_allocated_bitrate; diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 8961675f9c..1a20807756 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -89,6 +89,8 @@ ProbeControllerConfig::ProbeControllerConfig( further_exponential_probe_scale("step_size", 2), further_probe_threshold("further_probe_threshold", 0.7), abort_further_probe_if_max_lower_than_current("abort_further", false), + repeated_initial_probing_duration("initial_probing_duration", + TimeDelta::Seconds(5)), alr_probing_interval("alr_interval", TimeDelta::Seconds(5)), alr_probe_scale("alr_scale", 2), network_state_estimate_probing_interval("network_state_interval", @@ -118,6 +120,7 @@ ProbeControllerConfig::ProbeControllerConfig( &further_exponential_probe_scale, &further_probe_threshold, &abort_further_probe_if_max_lower_than_current, + &repeated_initial_probing_duration, &alr_probing_interval, &alr_probe_scale, &probe_on_max_allocated_bitrate_change, @@ -304,6 +307,13 @@ std::vector ProbeController::InitiateExponentialProbing( start_bitrate_); } waiting_for_initial_probe_result_ = true; + if (repeated_initial_probing_enabled_) { + last_allowed_repeated_initial_probe_ = + at_time + config_.repeated_initial_probing_duration; + RTC_LOG(LS_INFO) << "Repeated initial probing enabled, last allowed probe: " + << last_allowed_repeated_initial_probe_ + << "now: " << at_time; + } return InitiateProbing(at_time, probes, true); } @@ -325,7 +335,8 @@ std::vector ProbeController::SetEstimatedBitrate( if (config_.abort_further_probe_if_max_lower_than_current && (bitrate > max_bitrate_ || (!max_total_allocated_bitrate_.IsZero() && - !(waiting_for_initial_probe_result_ && first_probe_to_max_bitrate_) && + !(waiting_for_initial_probe_result_ && + repeated_initial_probing_enabled_) && bitrate > 2 * max_total_allocated_bitrate_))) { // No need to continue probing. min_bitrate_to_probe_further_ = DataRate::PlusInfinity(); @@ -354,9 +365,8 @@ void ProbeController::EnablePeriodicAlrProbing(bool enable) { enable_periodic_alr_probing_ = enable; } -void ProbeController::SetFirstProbeToMaxBitrate( - bool first_probe_to_max_bitrate) { - first_probe_to_max_bitrate_ = first_probe_to_max_bitrate; +void ProbeController::EnableRepeatedInitialProbing(bool enable) { + repeated_initial_probing_enabled_ = enable; } void ProbeController::SetAlrStartTimeMs( @@ -472,18 +482,22 @@ bool ProbeController::TimeForNetworkStateProbe(Timestamp at_time) const { return false; } +bool ProbeController::TimeForNextRepeatedInitialProbe(Timestamp at_time) const { + if (state_ != State::kWaitingForProbingResult && + last_allowed_repeated_initial_probe_ > at_time) { + Timestamp next_probe_time = + time_last_probing_initiated_ + kMaxWaitingTimeForProbingResult; + if (at_time >= next_probe_time) { + return true; + } + } + return false; +} + std::vector ProbeController::Process(Timestamp at_time) { if (at_time - time_last_probing_initiated_ > kMaxWaitingTimeForProbingResult) { if (state_ == State::kWaitingForProbingResult) { - // If the initial probe timed out, and the estimate has not changed by - // other means, (likely because normal media packets are not being sent - // yet), then send a probe again. - if (waiting_for_initial_probe_result_ && - estimated_bitrate_ == start_bitrate_ && first_probe_to_max_bitrate_) { - UpdateState(State::kInit); - return InitiateExponentialProbing(at_time); - } RTC_LOG(LS_INFO) << "kWaitingForProbingResult: timeout"; UpdateState(State::kProbingComplete); } @@ -491,6 +505,12 @@ std::vector ProbeController::Process(Timestamp at_time) { if (estimated_bitrate_.IsZero() || state_ != State::kProbingComplete) { return {}; } + if (TimeForNextRepeatedInitialProbe(at_time)) { + waiting_for_initial_probe_result_ = true; + return InitiateProbing( + at_time, {estimated_bitrate_ * config_.first_exponential_probe_scale}, + true); + } if (TimeForAlrProbe(at_time) || TimeForNetworkStateProbe(at_time)) { return InitiateProbing( at_time, {estimated_bitrate_ * config_.alr_probe_scale}, true); @@ -519,7 +539,8 @@ std::vector ProbeController::InitiateProbing( DataRate max_probe_bitrate = max_bitrate_; if (max_total_allocated_bitrate_ > DataRate::Zero() && - !(first_probe_to_max_bitrate_ && waiting_for_initial_probe_result_)) { + !(repeated_initial_probing_enabled_ && + waiting_for_initial_probe_result_)) { // If a max allocated bitrate has been configured, allow probing up to 2x // that rate. This allows some overhead to account for bursty streams, // which otherwise would have to ramp up when the overshoot is already in diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h index ec078adbc1..4a13d17a14 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -43,6 +43,9 @@ struct ProbeControllerConfig { FieldTrialParameter further_exponential_probe_scale; FieldTrialParameter further_probe_threshold; FieldTrialParameter abort_further_probe_if_max_lower_than_current; + // Duration of time from the first initial probe where repeated initial probes + // are sent if repeated initial probing is enabled. + FieldTrialParameter repeated_initial_probing_duration; // Configures how often we send ALR probes and how big they are. FieldTrialParameter alr_probing_interval; @@ -50,7 +53,7 @@ struct ProbeControllerConfig { // Configures how often we send probes if NetworkStateEstimate is available. FieldTrialParameter network_state_estimate_probing_interval; - // Periodically probe as long as the the ratio beteeen current estimate and + // Periodically probe as long as the ratio between current estimate and // NetworkStateEstimate is lower then this. FieldTrialParameter probe_if_estimate_lower_than_network_state_estimate_ratio; @@ -72,7 +75,7 @@ struct ProbeControllerConfig { // The minimum probing duration. FieldTrialParameter min_probe_duration; FieldTrialParameter loss_limited_probe_scale; - // Dont send a probe if min(estimate, network state estimate) is larger than + // Don't send a probe if min(estimate, network state estimate) is larger than // this fraction of the set max bitrate. FieldTrialParameter skip_if_estimate_larger_than_fraction_of_max; }; @@ -121,9 +124,11 @@ class ProbeController { Timestamp at_time); void EnablePeriodicAlrProbing(bool enable); - // The first initial probe ignores allocated bitrate constraints and probe up - // to max configured bitrate configured via SetBitrates. - void SetFirstProbeToMaxBitrate(bool first_probe_to_max_bitrate); + + // Probes are sent periodically every 1s during the first 5s after the network + // becomes available. The probes ignores allocated bitrate constraints and + // probe up to max configured bitrate configured via SetBitrates. + void EnableRepeatedInitialProbing(bool enable); void SetAlrStartTimeMs(absl::optional alr_start_time); void SetAlrEndedTimeMs(int64_t alr_end_time); @@ -160,10 +165,12 @@ class ProbeController { bool probe_further); bool TimeForAlrProbe(Timestamp at_time) const; bool TimeForNetworkStateProbe(Timestamp at_time) const; + bool TimeForNextRepeatedInitialProbe(Timestamp at_time) const; bool network_available_; bool waiting_for_initial_probe_result_ = false; - bool first_probe_to_max_bitrate_ = false; + bool repeated_initial_probing_enabled_ = false; + Timestamp last_allowed_repeated_initial_probe_ = Timestamp::MinusInfinity(); BandwidthLimitedCause bandwidth_limited_cause_ = BandwidthLimitedCause::kDelayBasedLimited; State state_; diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index 6691e87d3d..48214851b6 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -378,10 +378,8 @@ TEST(ProbeControllerTest, ExponentialProbingStopIfMaxAllocatedBitrateLow) { EXPECT_THAT(probes, IsEmpty()); } -TEST(ProbeControllerTest, - InitialProbingIgnoreLowMaxAllocatedbitrateIfSetFirstProbeToMaxBitrate) { - ProbeControllerFixture fixture( - "WebRTC-Bwe-ProbingConfiguration/abort_further:true/"); +TEST(ProbeControllerTest, RepeatedInitialProbingIgnoreLowMaxAllocatedbitrate) { + ProbeControllerFixture fixture; std::unique_ptr probe_controller = fixture.CreateController(); ASSERT_THAT( @@ -390,7 +388,7 @@ TEST(ProbeControllerTest, auto probes = probe_controller->SetBitrates( kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); ASSERT_THAT(probes, SizeIs(Gt(0))); - probe_controller->SetFirstProbeToMaxBitrate(true); + probe_controller->EnableRepeatedInitialProbing(true); // Repeated probe is sent when estimated bitrate climbs above // 0.7 * 6 * kStartBitrate = 1260. During the initial probe, we ignore the @@ -411,8 +409,7 @@ TEST(ProbeControllerTest, EXPECT_EQ(probes.size(), 1u); } -TEST(ProbeControllerTest, - InitialProbingToLowMaxAllocatedbitrateIfNotSetFirstProbeToMaxBitrate) { +TEST(ProbeControllerTest, InitialProbingToLowMaxAllocatedbitrate) { ProbeControllerFixture fixture; std::unique_ptr probe_controller = fixture.CreateController(); @@ -458,46 +455,35 @@ TEST(ProbeControllerTest, InitialProbingTimeout) { EXPECT_THAT(probes, IsEmpty()); } -TEST( - ProbeControllerTest, - InitialProbingRetriedAfterTimeoutIfFirstProbeToMaxBitrateAndBweNotUpdated) { +TEST(ProbeControllerTest, RepeatedInitialProbingSendsNewProbeAfterTimeout) { ProbeControllerFixture fixture; std::unique_ptr probe_controller = fixture.CreateController(); - probe_controller->SetFirstProbeToMaxBitrate(true); + probe_controller->EnableRepeatedInitialProbing(true); ASSERT_THAT( probe_controller->OnNetworkAvailability({.network_available = true}), IsEmpty()); auto probes = probe_controller->SetBitrates( kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); EXPECT_THAT(probes, SizeIs(Gt(0))); - // Advance far enough to cause a time out in waiting for probing result. - fixture.AdvanceTime(kExponentialProbingTimeout); - probes = probe_controller->Process(fixture.CurrentTime()); - EXPECT_THAT(probes, SizeIs(Gt(0))); -} - -TEST(ProbeControllerTest, - InitialProbingNotRetriedAfterTimeoutIfFirstProbeAndBweUpdated) { - ProbeControllerFixture fixture; - std::unique_ptr probe_controller = - fixture.CreateController(); - probe_controller->SetFirstProbeToMaxBitrate(true); - ASSERT_THAT( - probe_controller->OnNetworkAvailability({.network_available = true}), - IsEmpty()); - auto probes = probe_controller->SetBitrates( - kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); - EXPECT_THAT(probes, SizeIs(Gt(0))); - fixture.AdvanceTime(TimeDelta::Millis(700)); - probes = probe_controller->SetEstimatedBitrate( - DataRate::BitsPerSec(180), BandwidthLimitedCause::kDelayBasedLimited, - fixture.CurrentTime()); - EXPECT_THAT(probes, IsEmpty()); - // Advance far enough to cause a time out in waiting for probing result. - fixture.AdvanceTime(kExponentialProbingTimeout); - probes = probe_controller->Process(fixture.CurrentTime()); - EXPECT_THAT(probes, IsEmpty()); + Timestamp start_time = fixture.CurrentTime(); + Timestamp last_probe_time = fixture.CurrentTime(); + while (fixture.CurrentTime() < start_time + TimeDelta::Seconds(5)) { + fixture.AdvanceTime(TimeDelta::Millis(100)); + probes = probe_controller->Process(fixture.CurrentTime()); + if (!probes.empty()) { + // Expect a probe every second. + EXPECT_EQ(fixture.CurrentTime() - last_probe_time, + TimeDelta::Seconds(1.1)); + last_probe_time = fixture.CurrentTime(); + } else { + EXPECT_LT(fixture.CurrentTime() - last_probe_time, + TimeDelta::Seconds(1.1)); + } + } + fixture.AdvanceTime(TimeDelta::Seconds(1)); + // After 5s, repeated initial probing stops. + EXPECT_THAT(probe_controller->Process(fixture.CurrentTime()), IsEmpty()); } TEST(ProbeControllerTest, RequestProbeInAlr) {