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"));