Ensure that acked rate is the lower bound of estimate and candidates.
After https://webrtc-review.googlesource.com/c/src/+/329141, best candidate can still be less than acked rate if not_increase_if_inherent_loss_less_than_average_loss, or the selected candidate is 95% of current estimate. This cl/ is ensure the previous cl works as intended. And add unit test.
Bug: webrtc:12707
Change-Id: Ie5683ca8ea51f6d80c4c59cbf08c22e8b24c0cb9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/329441
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41298}
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 f50700c..95f8213 100644
--- a/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc
+++ b/modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc
@@ -337,6 +337,11 @@
current_best_estimate_.inherent_loss = 0;
} else {
current_best_estimate_ = best_candidate;
+ if (config_->lower_bound_by_acked_rate_factor > 0.0) {
+ current_best_estimate_.loss_limited_bandwidth =
+ std::max(current_best_estimate_.loss_limited_bandwidth,
+ GetInstantLowerBound());
+ }
}
if (loss_based_result_.state == LossBasedState::kDecreasing &&
@@ -874,12 +879,6 @@
std::vector<LossBasedBweV2::ChannelParameters> 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<DataRate> bandwidths;
for (double candidate_factor : config_->candidate_factors) {
bandwidths.push_back(candidate_factor *
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 d19464d..9b7ad03 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
@@ -1680,6 +1680,52 @@
DataRate::KilobitsPerSec(1000));
}
+TEST_F(LossBasedBweV2Test, EstimateNotLowerThanAckedRate) {
+ ExplicitKeyValueConfig key_value_config(
+ ShortObservationConfig("LowerBoundByAckedRateFactor:1.0"));
+ LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config);
+ loss_based_bandwidth_estimator.SetBandwidthEstimate(
+ DataRate::KilobitsPerSec(2500));
+ loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+ CreatePacketResultsWith100pLossRate(
+ /*first_packet_timestamp=*/Timestamp::Zero()),
+ /*delay_based_estimate=*/DataRate::PlusInfinity(),
+ /*in_alr=*/false);
+ ASSERT_LT(
+ loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
+ DataRate::KilobitsPerSec(1000));
+
+ loss_based_bandwidth_estimator.SetAcknowledgedBitrate(
+ DataRate::KilobitsPerSec(1000));
+ loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+ CreatePacketResultsWith100pLossRate(
+ /*first_packet_timestamp=*/Timestamp::Zero() +
+ kObservationDurationLowerBound),
+ /*delay_based_estimate=*/DataRate::PlusInfinity(),
+ /*in_alr=*/false);
+ EXPECT_EQ(
+ loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
+ DataRate::KilobitsPerSec(1000));
+
+ loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+ CreatePacketResultsWithReceivedPackets(
+ /*first_packet_timestamp=*/Timestamp::Zero() +
+ kObservationDurationLowerBound * 2),
+ /*delay_based_estimate=*/DataRate::PlusInfinity(),
+ /*in_alr=*/false);
+ loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+ CreatePacketResultsWithReceivedPackets(
+ /*first_packet_timestamp=*/Timestamp::Zero() +
+ kObservationDurationLowerBound * 3),
+ /*delay_based_estimate=*/DataRate::PlusInfinity(),
+ /*in_alr=*/false);
+
+ // Verify that the estimate recovers from the acked rate.
+ EXPECT_GT(
+ loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
+ DataRate::KilobitsPerSec(1000));
+}
+
TEST_F(LossBasedBweV2Test, EndHoldDurationIfDelayBasedEstimateWorks) {
ExplicitKeyValueConfig key_value_config(
ShortObservationConfig("HoldDurationFactor:3"));
