From e43edec62de68fe081b545e734c81f1eed5c4830 Mon Sep 17 00:00:00 2001 From: Diep Bui Date: Fri, 27 Oct 2023 11:17:11 +0000 Subject: [PATCH] Add 1s as padding duration limit in loss based BWE. If we have been sending padding for 1s and estimate still is unchanged, then stop padding by transitioning to decrease state. Bug: webrtc:12707 Change-Id: I0dca2e5cd98263fc7fae9882c23c21634413c7a0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324740 Reviewed-by: Per Kjellander Commit-Queue: Diep Bui Cr-Commit-Position: refs/heads/main@{#41018} --- .../goog_cc/loss_based_bwe_v2.cc | 36 +++++++++--- .../goog_cc/loss_based_bwe_v2.h | 9 ++- .../goog_cc/loss_based_bwe_v2_test.cc | 57 ++++++++++++++++++- 3 files changed, 93 insertions(+), 9 deletions(-) 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 2ebbe3338e..1a90aa089f 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc @@ -338,9 +338,17 @@ void LossBasedBweV2::UpdateResult() { if (IsEstimateIncreasingWhenLossLimited( /*old_estimate=*/loss_based_result_.bandwidth_estimate, /*new_estimate=*/bounded_bandwidth_estimate) && + CanKeepIncreasingState(bounded_bandwidth_estimate) && bounded_bandwidth_estimate < delay_based_estimate_ && bounded_bandwidth_estimate < max_bitrate_) { - loss_based_result_.state = config_->use_padding_for_increase + if (config_->padding_duration > TimeDelta::Zero() && + bounded_bandwidth_estimate > last_padding_info_.padding_rate) { + // Start a new padding duration. + last_padding_info_.padding_rate = bounded_bandwidth_estimate; + last_padding_info_.padding_timestamp = + last_send_time_most_recent_observation_; + } + loss_based_result_.state = config_->padding_duration > TimeDelta::Zero() ? LossBasedState::kIncreaseUsingPadding : LossBasedState::kIncreasing; } else if (bounded_bandwidth_estimate < delay_based_estimate_ && @@ -355,11 +363,13 @@ void LossBasedBweV2::UpdateResult() { last_send_time_most_recent_observation_ + hold_duration_; hold_duration_ = hold_duration_ * config_->hold_duration_factor; } + last_padding_info_ = PaddingInfo(); 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; + last_padding_info_ = PaddingInfo(); loss_based_result_.state = LossBasedState::kDelayBasedEstimate; } loss_based_result_.bandwidth_estimate = bounded_bandwidth_estimate; @@ -445,11 +455,10 @@ absl::optional LossBasedBweV2::CreateConfig( FieldTrialParameter min_num_observations("MinNumObservations", 3); FieldTrialParameter lower_bound_by_acked_rate_factor( "LowerBoundByAckedRateFactor", 0.0); - FieldTrialParameter use_padding_for_increase("UsePadding", false); - FieldTrialParameter hold_duration_factor("HoldDurationFactor", 0.0); FieldTrialParameter use_byte_loss_rate("UseByteLossRate", false); - + FieldTrialParameter padding_duration("PaddingDuration", + TimeDelta::Zero()); if (key_value_config) { ParseFieldTrial({&enabled, &bandwidth_rampup_upper_bound_factor, @@ -487,9 +496,9 @@ absl::optional LossBasedBweV2::CreateConfig( &use_in_start_phase, &min_num_observations, &lower_bound_by_acked_rate_factor, - &use_padding_for_increase, &hold_duration_factor, - &use_byte_loss_rate}, + &use_byte_loss_rate, + &padding_duration}, key_value_config->Lookup("WebRTC-Bwe-LossBasedBweV2")); } @@ -551,9 +560,9 @@ absl::optional LossBasedBweV2::CreateConfig( config->min_num_observations = min_num_observations.Get(); 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(); config->use_byte_loss_rate = use_byte_loss_rate.Get(); + config->padding_duration = padding_duration.Get(); return config; } @@ -1135,4 +1144,17 @@ bool LossBasedBweV2::IsInLossLimitedState() const { return loss_based_result_.state != LossBasedState::kDelayBasedEstimate; } +bool LossBasedBweV2::CanKeepIncreasingState(DataRate estimate) const { + if (config_->padding_duration == TimeDelta::Zero() || + loss_based_result_.state != LossBasedState::kIncreaseUsingPadding) + return true; + + // Keep using the kIncreaseUsingPadding if either the state has been + // kIncreaseUsingPadding for less than kPaddingDuration or the estimate + // increases. + return last_padding_info_.padding_timestamp + config_->padding_duration >= + last_send_time_most_recent_observation_ || + last_padding_info_.padding_rate < estimate; +} + } // namespace webrtc 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 b39657c7bb..425ca2a0c8 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h @@ -118,9 +118,9 @@ class LossBasedBweV2 { bool use_in_start_phase = false; 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; bool use_byte_loss_rate = false; + TimeDelta padding_duration = TimeDelta::Zero(); }; struct Derivatives { @@ -147,6 +147,11 @@ class LossBasedBweV2 { DataSize lost_size = DataSize::Zero(); }; + struct PaddingInfo { + DataRate padding_rate = DataRate::MinusInfinity(); + Timestamp padding_timestamp = Timestamp::MinusInfinity(); + }; + static absl::optional CreateConfig( const FieldTrialsView* key_value_config); bool IsConfigValid() const; @@ -179,6 +184,7 @@ class LossBasedBweV2 { bool IsEstimateIncreasingWhenLossLimited(DataRate old_estimate, DataRate new_estimate); bool IsInLossLimitedState() const; + bool CanKeepIncreasingState(DataRate estimate) const; absl::optional acknowledged_bitrate_; absl::optional config_; @@ -200,6 +206,7 @@ class LossBasedBweV2 { LossBasedBweV2::Result loss_based_result_ = LossBasedBweV2::Result(); Timestamp last_hold_timestamp_ = Timestamp::MinusInfinity(); TimeDelta hold_duration_ = TimeDelta::Zero(); + PaddingInfo last_padding_info_ = PaddingInfo(); }; } // 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 fc1f410bd6..347e2a86d1 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 @@ -1472,7 +1472,7 @@ TEST_F(LossBasedBweV2Test, HasDelayBasedStateIfLossBasedBweIsMax) { TEST_F(LossBasedBweV2Test, IncreaseUsingPaddingStateIfFieldTrial) { ExplicitKeyValueConfig key_value_config( - ShortObservationConfig("UsePadding:true")); + ShortObservationConfig("PaddingDuration:1000ms")); LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); loss_based_bandwidth_estimator.SetBandwidthEstimate( DataRate::KilobitsPerSec(2500)); @@ -1494,6 +1494,61 @@ TEST_F(LossBasedBweV2Test, IncreaseUsingPaddingStateIfFieldTrial) { LossBasedState::kIncreaseUsingPadding); } +TEST_F(LossBasedBweV2Test, DecreaseAfterPadding) { + ExplicitKeyValueConfig key_value_config(ShortObservationConfig( + "PaddingDuration:1000ms,BwRampupUpperBoundFactor:2.0")); + LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); + loss_based_bandwidth_estimator.SetBandwidthEstimate( + DataRate::KilobitsPerSec(2500)); + DataRate acknowledged_bitrate = DataRate::KilobitsPerSec(51); + loss_based_bandwidth_estimator.SetAcknowledgedBitrate(acknowledged_bitrate); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWith50pPacketLossRate( + /*first_packet_timestamp=*/Timestamp::Zero()), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + ASSERT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + ASSERT_EQ( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + acknowledged_bitrate); + + acknowledged_bitrate = DataRate::KilobitsPerSec(26); + loss_based_bandwidth_estimator.SetAcknowledgedBitrate(acknowledged_bitrate); + int feedback_id = 1; + while (loss_based_bandwidth_estimator.GetLossBasedResult().state != + LossBasedState::kIncreaseUsingPadding) { + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound * feedback_id), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + feedback_id++; + } + + const Timestamp estimate_increased = + Timestamp::Zero() + kObservationDurationLowerBound * feedback_id; + // The state is kIncreaseUsingPadding for a while without changing the + // estimate, which is limited by 2 * acked rate. + while (loss_based_bandwidth_estimator.GetLossBasedResult().state == + LossBasedState::kIncreaseUsingPadding) { + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound * feedback_id), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/false); + feedback_id++; + } + + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + const Timestamp start_decreasing = + Timestamp::Zero() + kObservationDurationLowerBound * (feedback_id - 1); + EXPECT_EQ(start_decreasing - estimate_increased, TimeDelta::Seconds(1)); +} + TEST_F(LossBasedBweV2Test, IncreaseEstimateIfNotHold) { ExplicitKeyValueConfig key_value_config( ShortObservationConfig("HoldDurationFactor:0"));