From 75a131f39c8cfa6471d29ae9c42e204a70f793d3 Mon Sep 17 00:00:00 2001 From: Diep Bui Date: Mon, 23 Oct 2023 08:22:41 +0000 Subject: [PATCH] Introduce hold duration in loss based BWE. The initial hold duration is 300ms. Whenever it enters kDecreasing state, it will double the current hold duration. The hold duration will be reset as soon as the delay based estimate works, e.g. the state is kDelayBased to avoid getting stuck at low bitrate. Bug: webrtc:12707 Change-Id: I3906ff80b071ba3eb6274b012fb31922f4cbc7b6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324304 Commit-Queue: Diep Bui Reviewed-by: Per Kjellander Cr-Commit-Position: refs/heads/main@{#40991} --- .../goog_cc/loss_based_bwe_v2.cc | 31 +++- .../goog_cc/loss_based_bwe_v2.h | 3 + .../goog_cc/loss_based_bwe_v2_test.cc | 141 ++++++++++++++++++ 3 files changed, 174 insertions(+), 1 deletion(-) diff --git a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc index 78f3cba78d..f79164ebf0 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc @@ -35,6 +35,8 @@ namespace webrtc { namespace { +constexpr TimeDelta kInitHoldDuration = TimeDelta::Millis(300); + bool IsValid(DataRate datarate) { return datarate.IsFinite(); } @@ -125,6 +127,7 @@ LossBasedBweV2::LossBasedBweV2(const FieldTrialsView* key_value_config) instant_upper_bound_temporal_weights_.resize( config_->observation_window_size); CalculateTemporalWeights(); + hold_duration_ = kInitHoldDuration; } bool LossBasedBweV2::IsEnabled() const { @@ -318,6 +321,16 @@ void LossBasedBweV2::UpdateResult() { GetInstantUpperBound())); } + if (loss_based_result_.state == LossBasedState::kDecreasing && + last_hold_timestamp_ > last_send_time_most_recent_observation_ && + bounded_bandwidth_estimate < delay_based_estimate_) { + // BWE is not allowed to increase during the HOLD duration. The purpose of + // HOLD is to not immediately ramp up BWE to a rate that may cause loss. + loss_based_result_.bandwidth_estimate = std::min( + loss_based_result_.bandwidth_estimate, bounded_bandwidth_estimate); + return; + } + if (IsEstimateIncreasingWhenLossLimited( /*old_estimate=*/loss_based_result_.bandwidth_estimate, /*new_estimate=*/bounded_bandwidth_estimate) && @@ -328,8 +341,21 @@ void LossBasedBweV2::UpdateResult() { : LossBasedState::kIncreasing; } else if (bounded_bandwidth_estimate < delay_based_estimate_ && bounded_bandwidth_estimate < max_bitrate_) { + if (loss_based_result_.state != LossBasedState::kDecreasing && + config_->hold_duration_factor > 0) { + RTC_LOG(LS_INFO) << this << " " + << "Switch to HOLD. Bounded BWE: " + << bounded_bandwidth_estimate.kbps() + << ", duration: " << hold_duration_.seconds(); + last_hold_timestamp_ = + last_send_time_most_recent_observation_ + hold_duration_; + hold_duration_ = hold_duration_ * config_->hold_duration_factor; + } loss_based_result_.state = LossBasedState::kDecreasing; } else { + // Reset the HOLD duration if delay based estimate works to avoid getting + // stuck in low bitrate. + hold_duration_ = kInitHoldDuration; loss_based_result_.state = LossBasedState::kDelayBasedEstimate; } loss_based_result_.bandwidth_estimate = bounded_bandwidth_estimate; @@ -417,6 +443,7 @@ absl::optional LossBasedBweV2::CreateConfig( "LowerBoundByAckedRateFactor", 0.0); FieldTrialParameter use_padding_for_increase("UsePadding", false); + FieldTrialParameter hold_duration_factor("HoldDurationFactor", 0.0); if (key_value_config) { ParseFieldTrial({&enabled, &bandwidth_rampup_upper_bound_factor, @@ -454,7 +481,8 @@ absl::optional LossBasedBweV2::CreateConfig( &use_in_start_phase, &min_num_observations, &lower_bound_by_acked_rate_factor, - &use_padding_for_increase}, + &use_padding_for_increase, + &hold_duration_factor}, key_value_config->Lookup("WebRTC-Bwe-LossBasedBweV2")); } @@ -517,6 +545,7 @@ absl::optional LossBasedBweV2::CreateConfig( config->lower_bound_by_acked_rate_factor = lower_bound_by_acked_rate_factor.Get(); config->use_padding_for_increase = use_padding_for_increase.Get(); + config->hold_duration_factor = hold_duration_factor.Get(); return config; } diff --git a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h index 9221028ddd..0468e16525 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h @@ -119,6 +119,7 @@ class LossBasedBweV2 { int min_num_observations = 0; double lower_bound_by_acked_rate_factor = 0.0; bool use_padding_for_increase = false; + double hold_duration_factor = 0.0; }; struct Derivatives { @@ -191,6 +192,8 @@ class LossBasedBweV2 { DataRate max_bitrate_ = DataRate::PlusInfinity(); DataRate delay_based_estimate_ = DataRate::PlusInfinity(); LossBasedBweV2::Result loss_based_result_ = LossBasedBweV2::Result(); + Timestamp last_hold_timestamp_ = Timestamp::MinusInfinity(); + TimeDelta hold_duration_ = TimeDelta::Zero(); }; } // namespace webrtc diff --git a/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc b/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc index 8416f29f9c..d9480d00d5 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc @@ -1491,5 +1491,146 @@ TEST_F(LossBasedBweV2Test, IncreaseUsingPaddingStateIfFieldTrial) { LossBasedState::kIncreaseUsingPadding); } +TEST_F(LossBasedBweV2Test, IncreaseEstimateIfNotHold) { + ExplicitKeyValueConfig key_value_config( + ShortObservationConfig("HoldDurationFactor:0")); + LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); + loss_based_bandwidth_estimator.SetBandwidthEstimate( + DataRate::KilobitsPerSec(2500)); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWith50pLossRate( + /*first_packet_timestamp=*/Timestamp::Zero()), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + ASSERT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + DataRate estimate = + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate; + + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kIncreasing); + EXPECT_GT( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + estimate); +} + +TEST_F(LossBasedBweV2Test, IncreaseEstimateAfterHoldDuration) { + ExplicitKeyValueConfig key_value_config( + ShortObservationConfig("HoldDurationFactor:3")); + LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); + loss_based_bandwidth_estimator.SetBandwidthEstimate( + DataRate::KilobitsPerSec(2500)); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWith50pLossRate( + /*first_packet_timestamp=*/Timestamp::Zero()), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + ASSERT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + DataRate estimate = + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate; + + // During the hold duration, e.g. first 300ms, the estimate cannot increase. + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + EXPECT_EQ( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + estimate); + + // After the hold duration, the estimate can increase. + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound * 2), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kIncreasing); + EXPECT_GE( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + estimate); + + // Get another 50p packet loss. + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWith50pLossRate( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound * 3), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + estimate = + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate; + + // During the hold duration, e.g. next 900ms, the estimate cannot increase. + for (int i = 4; i <= 6; ++i) { + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound * i), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + EXPECT_EQ( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + estimate); + } + + // After the hold duration, the estimate can increase again. + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound * 7), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kIncreasing); + EXPECT_GE( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + estimate); +} + +TEST_F(LossBasedBweV2Test, EndHoldDurationIfDelayBasedEstimateWorks) { + ExplicitKeyValueConfig key_value_config( + ShortObservationConfig("HoldDurationFactor:3")); + LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); + loss_based_bandwidth_estimator.SetBandwidthEstimate( + DataRate::KilobitsPerSec(2500)); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWith50pLossRate( + /*first_packet_timestamp=*/Timestamp::Zero()), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + ASSERT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + DataRate estimate = + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate; + + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound), + /*delay_based_estimate=*/estimate + DataRate::KilobitsPerSec(10), + /*in_alr=*/false); + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDelayBasedEstimate); + EXPECT_EQ( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + estimate + DataRate::KilobitsPerSec(10)); +} + } // namespace } // namespace webrtc