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 614ae5b0b2..898fdaea35 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc @@ -316,33 +316,27 @@ void LossBasedBweV2::UpdateBandwidthEstimate( } } - current_best_estimate_ = best_candidate; - UpdateResult(); - - if (IsInLossLimitedState() && - (recovering_after_loss_timestamp_.IsInfinite() || - recovering_after_loss_timestamp_ + config_->delayed_increase_window < - last_send_time_most_recent_observation_)) { - bandwidth_limit_in_current_window_ = - std::max(kCongestionControllerMinBitrate, - current_best_estimate_.loss_limited_bandwidth * - config_->max_increase_factor); - recovering_after_loss_timestamp_ = last_send_time_most_recent_observation_; - } -} - -void LossBasedBweV2::UpdateResult() { DataRate bounded_bandwidth_estimate = DataRate::PlusInfinity(); if (IsValid(delay_based_estimate_)) { bounded_bandwidth_estimate = std::max(GetInstantLowerBound(), - std::min({current_best_estimate_.loss_limited_bandwidth, + std::min({best_candidate.loss_limited_bandwidth, GetInstantUpperBound(), delay_based_estimate_})); } else { - bounded_bandwidth_estimate = - std::max(GetInstantLowerBound(), - std::min(current_best_estimate_.loss_limited_bandwidth, - GetInstantUpperBound())); + bounded_bandwidth_estimate = std::max( + GetInstantLowerBound(), std::min(best_candidate.loss_limited_bandwidth, + GetInstantUpperBound())); + } + if (config_->bound_best_candidate && + bounded_bandwidth_estimate < best_candidate.loss_limited_bandwidth) { + RTC_LOG(LS_INFO) << "Resetting loss based BWE to " + << bounded_bandwidth_estimate.kbps() + << "due to loss. Avg loss rate: " + << GetAverageReportedLossRatio(); + current_best_estimate_.loss_limited_bandwidth = bounded_bandwidth_estimate; + current_best_estimate_.inherent_loss = 0; + } else { + current_best_estimate_ = best_candidate; } if (loss_based_result_.state == LossBasedState::kDecreasing && @@ -399,6 +393,17 @@ void LossBasedBweV2::UpdateResult() { loss_based_result_.state = LossBasedState::kDelayBasedEstimate; } loss_based_result_.bandwidth_estimate = bounded_bandwidth_estimate; + + if (IsInLossLimitedState() && + (recovering_after_loss_timestamp_.IsInfinite() || + recovering_after_loss_timestamp_ + config_->delayed_increase_window < + last_send_time_most_recent_observation_)) { + bandwidth_limit_in_current_window_ = + std::max(kCongestionControllerMinBitrate, + current_best_estimate_.loss_limited_bandwidth * + config_->max_increase_factor); + recovering_after_loss_timestamp_ = last_send_time_most_recent_observation_; + } } bool LossBasedBweV2::IsEstimateIncreasingWhenLossLimited( @@ -483,6 +488,7 @@ absl::optional LossBasedBweV2::CreateConfig( FieldTrialParameter use_byte_loss_rate("UseByteLossRate", false); FieldTrialParameter padding_duration("PaddingDuration", TimeDelta::Zero()); + FieldTrialParameter bound_best_candidate("BoundBestCandidate", false); if (key_value_config) { ParseFieldTrial({&enabled, &bandwidth_rampup_upper_bound_factor, @@ -521,7 +527,8 @@ absl::optional LossBasedBweV2::CreateConfig( &lower_bound_by_acked_rate_factor, &hold_duration_factor, &use_byte_loss_rate, - &padding_duration}, + &padding_duration, + &bound_best_candidate}, key_value_config->Lookup("WebRTC-Bwe-LossBasedBweV2")); } @@ -586,7 +593,7 @@ absl::optional LossBasedBweV2::CreateConfig( config->hold_duration_factor = hold_duration_factor.Get(); config->use_byte_loss_rate = use_byte_loss_rate.Get(); config->padding_duration = padding_duration.Get(); - + config->bound_best_candidate = bound_best_candidate.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 850ce8ef05..9afbb11f1f 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.h @@ -123,6 +123,7 @@ class LossBasedBweV2 { double hold_duration_factor = 0.0; bool use_byte_loss_rate = false; TimeDelta padding_duration = TimeDelta::Zero(); + bool bound_best_candidate = false; }; struct Derivatives { @@ -188,7 +189,6 @@ class LossBasedBweV2 { // Returns false if no observation was created. bool PushBackObservation(rtc::ArrayView packet_results); - void UpdateResult(); bool IsEstimateIncreasingWhenLossLimited(DataRate old_estimate, DataRate new_estimate); bool IsInLossLimitedState() const; 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 2399108dc6..84c889ace7 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 @@ -1383,6 +1383,42 @@ TEST_F(LossBasedBweV2Test, IncreaseUsingPaddingStateIfFieldTrial) { LossBasedState::kIncreaseUsingPadding); } +TEST_F(LossBasedBweV2Test, BestCandidateResetsToUpperBoundInFieldTrial) { + ExplicitKeyValueConfig key_value_config( + ShortObservationConfig("PaddingDuration:1000ms,BoundBestCandidate:true")); + LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); + loss_based_bandwidth_estimator.SetBandwidthEstimate( + DataRate::KilobitsPerSec(2500)); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWith50pPacketLossRate( + /*first_packet_timestamp=*/Timestamp::Zero()), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/true); + LossBasedBweV2::Result result_after_loss = + loss_based_bandwidth_estimator.GetLossBasedResult(); + ASSERT_EQ(result_after_loss.state, LossBasedState::kDecreasing); + + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/true); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + 2 * kObservationDurationLowerBound), + /*delay_based_estimate=*/DataRate::PlusInfinity(), + /*in_alr=*/true); + // After a BWE decrease due to large loss, BWE is expected to ramp up slowly + // and follow the acked bitrate. + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kIncreaseUsingPadding); + EXPECT_NEAR(loss_based_bandwidth_estimator.GetLossBasedResult() + .bandwidth_estimate.kbps(), + result_after_loss.bandwidth_estimate.kbps(), 100); +} + TEST_F(LossBasedBweV2Test, DecreaseToAckedCandidateIfPaddingInAlr) { ExplicitKeyValueConfig key_value_config(ShortObservationConfig( "PaddingDuration:1000ms,"