From 5b11df789fac3234003bb81a9df59aa5570313a7 Mon Sep 17 00:00:00 2001
From: Diep Bui <diepbp@webrtc.org>
Date: Fri, 1 Dec 2023 15:23:26 +0000
Subject: [PATCH] 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}
---
 .../goog_cc/loss_based_bwe_v2.cc              | 11 ++---
 .../goog_cc/loss_based_bwe_v2_test.cc         | 46 +++++++++++++++++++
 2 files changed, 51 insertions(+), 6 deletions(-)

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 f50700cc5f..95f821352a 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 @@ void LossBasedBweV2::UpdateBandwidthEstimate(
     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 @@ DataRate LossBasedBweV2::GetCandidateBandwidthUpperBound() const {
 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 d19464d1ee..9b7ad03148 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 @@ TEST_F(LossBasedBweV2Test, HoldRateNotLowerThanAckedRate) {
       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"));