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 c0d7d1011b..da94baaa8b 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc @@ -282,12 +282,10 @@ void LossBasedBweV2::UpdateBandwidthEstimate( // Bound the best candidate by the acked bitrate. if (increasing_when_loss_limited && IsValid(acknowledged_bitrate_)) { best_candidate.loss_limited_bandwidth = - IsValid(best_candidate.loss_limited_bandwidth) - ? std::min(best_candidate.loss_limited_bandwidth, - config_->bandwidth_rampup_upper_bound_factor * - (*acknowledged_bitrate_)) - : config_->bandwidth_rampup_upper_bound_factor * - (*acknowledged_bitrate_); + std::max(current_best_estimate_.loss_limited_bandwidth, + std::min(best_candidate.loss_limited_bandwidth, + config_->bandwidth_rampup_upper_bound_factor * + (*acknowledged_bitrate_))); } } @@ -323,11 +321,13 @@ void LossBasedBweV2::UpdateResult() { if (IsEstimateIncreasingWhenLossLimited( /*old_estimate=*/loss_based_result_.bandwidth_estimate, /*new_estimate=*/bounded_bandwidth_estimate) && - bounded_bandwidth_estimate < delay_based_estimate_) { + bounded_bandwidth_estimate < delay_based_estimate_ && + bounded_bandwidth_estimate < max_bitrate_) { loss_based_result_.state = LossBasedState::kIncreasing; - } else if (bounded_bandwidth_estimate < delay_based_estimate_) { + } else if (bounded_bandwidth_estimate < delay_based_estimate_ && + bounded_bandwidth_estimate < max_bitrate_) { loss_based_result_.state = LossBasedState::kDecreasing; - } else if (bounded_bandwidth_estimate >= delay_based_estimate_) { + } else { loss_based_result_.state = LossBasedState::kDelayBasedEstimate; } loss_based_result_.bandwidth_estimate = bounded_bandwidth_estimate; 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 3ce750039c..a803f7d54d 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 @@ -711,7 +711,7 @@ TEST_F(LossBasedBweV2Test, } TEST_F(LossBasedBweV2Test, - LossBasedStateIsNotDelayBasedEstimateIfDelayBasedEsimtateInfinite) { + LossBasedStateIsNotDelayBasedEstimateIfDelayBasedEstimateInfinite) { ExplicitKeyValueConfig key_value_config( ShortObservationConfig("CandidateFactors:100|1|0.5," "InstantUpperBoundBwBalance:10000kbps," @@ -750,17 +750,14 @@ TEST_F(LossBasedBweV2Test, // a factor of acked bitrate. TEST_F(LossBasedBweV2Test, IncreaseByFactorOfAckedBitrateAfterLossBasedBweBacksOff) { - ExplicitKeyValueConfig key_value_config( - ShortObservationConfig("LossThresholdOfHighBandwidthPreference:0.99," - "BwRampupUpperBoundFactor:1.2," - "InherentLossUpperBoundOffset:0.9")); + ExplicitKeyValueConfig key_value_config(ShortObservationConfig( + "LossThresholdOfHighBandwidthPreference:0.99," + "BwRampupUpperBoundFactor:1.2," + // Set InstantUpperBoundBwBalance high to disable InstantUpperBound cap. + "InstantUpperBoundBwBalance:10000kbps,")); std::vector enough_feedback_1 = CreatePacketResultsWith100pLossRate( /*first_packet_timestamp=*/Timestamp::Zero()); - std::vector enough_feedback_2 = - CreatePacketResultsWith10pLossRate( - /*first_packet_timestamp=*/Timestamp::Zero() + - kObservationDurationLowerBound); LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); DataRate delay_based_estimate = DataRate::KilobitsPerSec(5000); @@ -771,19 +768,42 @@ TEST_F(LossBasedBweV2Test, loss_based_bandwidth_estimator.UpdateBandwidthEstimate(enough_feedback_1, delay_based_estimate, /*in_alr=*/false); + ASSERT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDecreasing); + LossBasedBweV2::Result result = + loss_based_bandwidth_estimator.GetLossBasedResult(); + DataRate estimate_1 = result.bandwidth_estimate; + ASSERT_LT(estimate_1.kbps(), 600); - // Change the acked bitrate to make sure that the estimate is bounded by a - // factor of acked bitrate. - DataRate acked_bitrate = DataRate::KilobitsPerSec(50); - loss_based_bandwidth_estimator.SetAcknowledgedBitrate(acked_bitrate); - loss_based_bandwidth_estimator.UpdateBandwidthEstimate(enough_feedback_2, - delay_based_estimate, - /*in_alr=*/false); + loss_based_bandwidth_estimator.SetAcknowledgedBitrate(estimate_1 * 0.9); + + int feedback_count = 1; + while (feedback_count < 5 && result.state != LossBasedState::kIncreasing) { + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + feedback_count++ * kObservationDurationLowerBound), + delay_based_estimate, + /*in_alr=*/false); + result = loss_based_bandwidth_estimator.GetLossBasedResult(); + } + ASSERT_EQ(result.state, LossBasedState::kIncreasing); // The estimate is capped by acked_bitrate * BwRampupUpperBoundFactor. - DataRate estimate_2 = - loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate; - EXPECT_EQ(estimate_2, acked_bitrate * 1.2); + EXPECT_EQ(result.bandwidth_estimate, estimate_1 * 0.9 * 1.2); + + // But if acked bitrate decrease, BWE does not decrease when there is no + // loss. + loss_based_bandwidth_estimator.SetAcknowledgedBitrate(estimate_1 * 0.9); + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + feedback_count++ * kObservationDurationLowerBound), + delay_based_estimate, + /*in_alr=*/false); + EXPECT_EQ( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + result.bandwidth_estimate); } // After loss based bwe backs off, the estimate is bounded during the delayed @@ -1368,5 +1388,52 @@ TEST_F(LossBasedBweV2Test, HasIncreaseStateBecauseOfLowerBound) { LossBasedState::kIncreasing); } +TEST_F(LossBasedBweV2Test, HasDelayBasedStateIfLossBasedBweIsMax) { + ExplicitKeyValueConfig key_value_config(ShortObservationConfig("")); + LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config); + loss_based_bandwidth_estimator.SetMinMaxBitrate( + /*min_bitrate=*/DataRate::KilobitsPerSec(10), + /*max_bitrate=*/DataRate::KilobitsPerSec(1000)); + + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + /*feedback = */ CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero()), + /*delay_based_estimate=*/DataRate::KilobitsPerSec(2000), + /*in_alr=*/false); + EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state, + LossBasedState::kDelayBasedEstimate); + EXPECT_EQ( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + DataRate::KilobitsPerSec(1000)); + + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + /*feedback=*/CreatePacketResultsWith50pLossRate( + /*first_packet_timestamp=*/Timestamp::Zero() + + kObservationDurationLowerBound), + /*delay_based_estimate=*/DataRate::KilobitsPerSec(2000), + /*in_alr=*/false); + LossBasedBweV2::Result result = + loss_based_bandwidth_estimator.GetLossBasedResult(); + ASSERT_EQ(result.state, LossBasedState::kDecreasing); + ASSERT_LT(result.bandwidth_estimate, DataRate::KilobitsPerSec(1000)); + + // Eventually the estimator recovers to delay based state. + int feedback_count = 2; + while (feedback_count < 5 && + result.state != LossBasedState::kDelayBasedEstimate) { + loss_based_bandwidth_estimator.UpdateBandwidthEstimate( + /*feedback = */ CreatePacketResultsWithReceivedPackets( + /*first_packet_timestamp=*/Timestamp::Zero() + + feedback_count++ * kObservationDurationLowerBound), + /*delay_based_estimate=*/DataRate::KilobitsPerSec(2000), + /*in_alr=*/false); + result = loss_based_bandwidth_estimator.GetLossBasedResult(); + } + EXPECT_EQ(result.state, LossBasedState::kDelayBasedEstimate); + EXPECT_EQ( + loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate, + DataRate::KilobitsPerSec(1000)); +} + } // namespace } // namespace webrtc