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 898fdaea35..f50700cc5f 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc @@ -342,6 +342,11 @@ void LossBasedBweV2::UpdateBandwidthEstimate( if (loss_based_result_.state == LossBasedState::kDecreasing && last_hold_info_.timestamp > last_send_time_most_recent_observation_ && bounded_bandwidth_estimate < delay_based_estimate_) { + // Ensure that acked rate is the lower bound of HOLD rate. + if (config_->lower_bound_by_acked_rate_factor > 0.0) { + last_hold_info_.rate = + std::max(GetInstantLowerBound(), last_hold_info_.rate); + } // BWE is not allowed to increase above the HOLD rate. The purpose of // HOLD is to not immediately ramp up BWE to a rate that may cause loss. loss_based_result_.bandwidth_estimate = @@ -868,10 +873,17 @@ DataRate LossBasedBweV2::GetCandidateBandwidthUpperBound() const { std::vector LossBasedBweV2::GetCandidates( bool in_alr) const { + ChannelParameters best_estimate = current_best_estimate_; + // Ensure that acked rate is the lower bound of best estimate. + if (config_->lower_bound_by_acked_rate_factor > 0.0 && + IsValid(best_estimate.loss_limited_bandwidth)) { + best_estimate.loss_limited_bandwidth = + std::max(GetInstantLowerBound(), best_estimate.loss_limited_bandwidth); + } std::vector bandwidths; for (double candidate_factor : config_->candidate_factors) { bandwidths.push_back(candidate_factor * - current_best_estimate_.loss_limited_bandwidth); + best_estimate.loss_limited_bandwidth); } if (acknowledged_bitrate_.has_value() && @@ -887,13 +899,13 @@ std::vector LossBasedBweV2::GetCandidates( if (IsValid(delay_based_estimate_) && config_->append_delay_based_estimate_candidate) { - if (delay_based_estimate_ > current_best_estimate_.loss_limited_bandwidth) { + if (delay_based_estimate_ > best_estimate.loss_limited_bandwidth) { bandwidths.push_back(delay_based_estimate_); } } if (in_alr && config_->append_upper_bound_candidate_in_alr && - current_best_estimate_.loss_limited_bandwidth > GetInstantUpperBound()) { + best_estimate.loss_limited_bandwidth > GetInstantUpperBound()) { bandwidths.push_back(GetInstantUpperBound()); } @@ -903,10 +915,10 @@ std::vector LossBasedBweV2::GetCandidates( std::vector candidates; candidates.resize(bandwidths.size()); for (size_t i = 0; i < bandwidths.size(); ++i) { - ChannelParameters candidate = current_best_estimate_; - candidate.loss_limited_bandwidth = std::min( - bandwidths[i], std::max(current_best_estimate_.loss_limited_bandwidth, - candidate_bandwidth_upper_bound)); + ChannelParameters candidate = best_estimate; + candidate.loss_limited_bandwidth = + std::min(bandwidths[i], std::max(best_estimate.loss_limited_bandwidth, + candidate_bandwidth_upper_bound)); candidate.inherent_loss = GetFeasibleInherentLoss(candidate); candidates[i] = candidate; } 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 84c889ace7..d19464d1ee 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 @@ -1650,6 +1650,36 @@ TEST_F(LossBasedBweV2Test, IncreaseEstimateAfterHoldDuration) { } } +TEST_F(LossBasedBweV2Test, HoldRateNotLowerThanAckedRate) { + ExplicitKeyValueConfig key_value_config(ShortObservationConfig( + "HoldDurationFactor:10,LowerBoundByAckedRateFactor:1.0")); + 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=*/false); + ASSERT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + + // During the hold duration, hold rate is not lower than the acked rate. + loss_based_bandwidth_estimator.SetAcknowledgedBitrate( + DataRate::KilobitsPerSec(1000)); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWith50pPacketLossRate( + /*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, + DataRate::KilobitsPerSec(1000)); +} + TEST_F(LossBasedBweV2Test, EndHoldDurationIfDelayBasedEstimateWorks) { ExplicitKeyValueConfig key_value_config( ShortObservationConfig("HoldDurationFactor:3"));