Do not probe if rtt is higher than the limit defined in RTTBasedBackoff

Bug: webrtc:14754
Change-Id: If7e0426fb8e568e3d51a767df12500f181fa86d3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/292841
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39308}

modules/congestion_controller/goog_cc/goog_cc_network_control.cc

@@ -30,6 +30,7 @@
 #include "modules/congestion_controller/goog_cc/alr_detector.h"
 #include "modules/congestion_controller/goog_cc/loss_based_bwe_v2.h"
 #include "modules/congestion_controller/goog_cc/probe_controller.h"
+#include "modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h"
 #include "modules/remote_bitrate_estimator/include/bwe_defines.h"
 #include "modules/remote_bitrate_estimator/test/bwe_test_logging.h"
 #include "rtc_base/checks.h"
@@ -64,12 +65,16 @@ bool IsNotDisabled(const FieldTrialsView* config, absl::string_view key) {
 
 BandwidthLimitedCause GetBandwidthLimitedCause(
     LossBasedState loss_based_state,
+    bool is_rtt_above_limit,
     BandwidthUsage bandwidth_usage,
     bool not_probe_if_delay_increased) {
-  if (not_probe_if_delay_increased &&
-      (bandwidth_usage == BandwidthUsage::kBwOverusing ||
-       bandwidth_usage == BandwidthUsage::kBwUnderusing)) {
-    return BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased;
+  if (not_probe_if_delay_increased) {
+    if (bandwidth_usage == BandwidthUsage::kBwOverusing ||
+        bandwidth_usage == BandwidthUsage::kBwUnderusing) {
+      return BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased;
+    } else if (is_rtt_above_limit) {
+      return BandwidthLimitedCause::kRttBasedBackOffHighRtt;
+    }
   }
   switch (loss_based_state) {
     case LossBasedState::kDecreasing:
@@ -686,6 +691,7 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged(
         loss_based_target_rate,
         GetBandwidthLimitedCause(
             bandwidth_estimation_->loss_based_state(),
+            bandwidth_estimation_->IsRttAboveLimit(),
             delay_based_bwe_->last_state(),
             probe_controller_->DontProbeIfDelayIncreased()),
         at_time);

modules/congestion_controller/goog_cc/probe_controller.cc

@@ -497,8 +497,10 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateProbing(
     }
   }
   if (config_.not_probe_if_delay_increased &&
-      bandwidth_limited_cause_ ==
-          BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased) {
+      (bandwidth_limited_cause_ ==
+           BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased ||
+       bandwidth_limited_cause_ ==
+           BandwidthLimitedCause::kRttBasedBackOffHighRtt)) {
     return {};
   }
 

modules/congestion_controller/goog_cc/probe_controller.h

@@ -78,7 +78,7 @@ struct ProbeControllerConfig {
   // Dont send a probe if min(estimate, network state estimate) is larger than
   // this fraction of the set max bitrate.
   FieldTrialParameter<double> skip_if_estimate_larger_than_fraction_of_max;
-  // Do not send probes if network is either overusing or underusing.
+  // Do not send probes if either overusing/underusing network or high rtt.
   FieldTrialParameter<bool> not_probe_if_delay_increased;
 };
 
@@ -90,6 +90,7 @@ enum class BandwidthLimitedCause {
   kLossLimitedBweDecreasing = 1,
   kDelayBasedLimited = 2,
   kDelayBasedLimitedDelayIncreased = 3,
+  kRttBasedBackOffHighRtt = 4
 };
 
 // This class controls initiation of probing to estimate initial channel

modules/congestion_controller/goog_cc/probe_controller_unittest.cc

@@ -1127,5 +1127,34 @@ TEST(ProbeControllerTest, DontProbeIfDelayIncreased) {
   probes = probe_controller->Process(fixture.CurrentTime());
   EXPECT_TRUE(probes.empty());
 }
+
+TEST(ProbeControllerTest, DontProbeIfHighRtt) {
+  ProbeControllerFixture fixture(
+      "WebRTC-Bwe-ProbingConfiguration/"
+      "network_state_interval:5s,not_probe_if_delay_increased:true/");
+  std::unique_ptr<ProbeController> probe_controller =
+      fixture.CreateController();
+
+  auto probes = probe_controller->SetBitrates(
+      kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime());
+  ASSERT_FALSE(probes.empty());
+
+  // Need to wait at least one second before process can trigger a new probe.
+  fixture.AdvanceTime(TimeDelta::Millis(1100));
+  probes = probe_controller->Process(fixture.CurrentTime());
+  ASSERT_TRUE(probes.empty());
+
+  NetworkStateEstimate state_estimate;
+  state_estimate.link_capacity_upper = 3 * kStartBitrate;
+  probe_controller->SetNetworkStateEstimate(state_estimate);
+  probes = probe_controller->SetEstimatedBitrate(
+      kStartBitrate, BandwidthLimitedCause::kRttBasedBackOffHighRtt,
+      fixture.CurrentTime());
+  ASSERT_TRUE(probes.empty());
+
+  fixture.AdvanceTime(TimeDelta::Seconds(5));
+  probes = probe_controller->Process(fixture.CurrentTime());
+  EXPECT_TRUE(probes.empty());
+}
 }  // namespace test
 }  // namespace webrtc

modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc

@@ -184,13 +184,14 @@ void RttBasedBackoff::UpdatePropagationRtt(Timestamp at_time,
   last_propagation_rtt_ = propagation_rtt;
 }
 
-TimeDelta RttBasedBackoff::CorrectedRtt(Timestamp at_time) const {
-  TimeDelta time_since_rtt = at_time - last_propagation_rtt_update_;
-  TimeDelta timeout_correction = time_since_rtt;
+bool RttBasedBackoff::IsRttAboveLimit() const {
+  return CorrectedRtt() > rtt_limit_;
+}
+
+TimeDelta RttBasedBackoff::CorrectedRtt() const {
   // Avoid timeout when no packets are being sent.
-  TimeDelta time_since_packet_sent = at_time - last_packet_sent_;
-  timeout_correction =
-      std::max(time_since_rtt - time_since_packet_sent, TimeDelta::Zero());
+  TimeDelta timeout_correction = std::max(
+      last_packet_sent_ - last_propagation_rtt_update_, TimeDelta::Zero());
   return timeout_correction + last_propagation_rtt_;
 }
 
@@ -328,6 +329,10 @@ LossBasedState SendSideBandwidthEstimation::loss_based_state() const {
   return loss_based_state_;
 }
 
+bool SendSideBandwidthEstimation::IsRttAboveLimit() const {
+  return rtt_backoff_.IsRttAboveLimit();
+}
+
 DataRate SendSideBandwidthEstimation::GetEstimatedLinkCapacity() const {
   return link_capacity_.estimate();
 }
@@ -464,7 +469,7 @@ void SendSideBandwidthEstimation::UpdateRtt(TimeDelta rtt, Timestamp at_time) {
 }
 
 void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) {
-  if (rtt_backoff_.CorrectedRtt(at_time) > rtt_backoff_.rtt_limit_) {
+  if (rtt_backoff_.IsRttAboveLimit()) {
     if (at_time - time_last_decrease_ >= rtt_backoff_.drop_interval_ &&
         current_target_ > rtt_backoff_.bandwidth_floor_) {
       time_last_decrease_ = at_time;

modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h

@@ -60,7 +60,7 @@ class RttBasedBackoff {
   explicit RttBasedBackoff(const FieldTrialsView* key_value_config);
   ~RttBasedBackoff();
   void UpdatePropagationRtt(Timestamp at_time, TimeDelta propagation_rtt);
-  TimeDelta CorrectedRtt(Timestamp at_time) const;
+  bool IsRttAboveLimit() const;
 
   FieldTrialFlag disabled_;
   FieldTrialParameter<TimeDelta> configured_limit_;
@@ -73,6 +73,9 @@ class RttBasedBackoff {
   Timestamp last_propagation_rtt_update_;
   TimeDelta last_propagation_rtt_;
   Timestamp last_packet_sent_;
+
+ private:
+  TimeDelta CorrectedRtt() const;
 };
 
 class SendSideBandwidthEstimation {
@@ -86,6 +89,9 @@ class SendSideBandwidthEstimation {
 
   DataRate target_rate() const;
   LossBasedState loss_based_state() const;
+  // Return whether the current rtt is higher than the rtt limited configured in
+  // RttBasedBackoff.
+  bool IsRttAboveLimit() const;
   uint8_t fraction_loss() const { return last_fraction_loss_; }
   TimeDelta round_trip_time() const { return last_round_trip_time_; }
 

modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc

@@ -203,4 +203,38 @@ TEST(SendSideBweTest, FractionLossIsNotOverflowed) {
   EXPECT_EQ(0, bwe.fraction_loss());
 }
 
+TEST(SendSideBweTest, RttIsAboveLimitIfRttGreaterThanLimit) {
+  ::testing::NiceMock<MockRtcEventLog> event_log;
+  test::ExplicitKeyValueConfig key_value_config("");
+  SendSideBandwidthEstimation bwe(&key_value_config, &event_log);
+  static const int kMinBitrateBps = 10000;
+  static const int kMaxBitrateBps = 10000000;
+  static const int kInitialBitrateBps = 300000;
+  int64_t now_ms = 0;
+  bwe.SetMinMaxBitrate(DataRate::BitsPerSec(kMinBitrateBps),
+                       DataRate::BitsPerSec(kMaxBitrateBps));
+  bwe.SetSendBitrate(DataRate::BitsPerSec(kInitialBitrateBps),
+                     Timestamp::Millis(now_ms));
+  bwe.UpdatePropagationRtt(/*at_time=*/Timestamp::Millis(now_ms),
+                           /*propagation_rtt=*/TimeDelta::Millis(5000));
+  EXPECT_TRUE(bwe.IsRttAboveLimit());
+}
+
+TEST(SendSideBweTest, RttIsBelowLimitIfRttLessThanLimit) {
+  ::testing::NiceMock<MockRtcEventLog> event_log;
+  test::ExplicitKeyValueConfig key_value_config("");
+  SendSideBandwidthEstimation bwe(&key_value_config, &event_log);
+  static const int kMinBitrateBps = 10000;
+  static const int kMaxBitrateBps = 10000000;
+  static const int kInitialBitrateBps = 300000;
+  int64_t now_ms = 0;
+  bwe.SetMinMaxBitrate(DataRate::BitsPerSec(kMinBitrateBps),
+                       DataRate::BitsPerSec(kMaxBitrateBps));
+  bwe.SetSendBitrate(DataRate::BitsPerSec(kInitialBitrateBps),
+                     Timestamp::Millis(now_ms));
+  bwe.UpdatePropagationRtt(/*at_time=*/Timestamp::Millis(now_ms),
+                           /*propagation_rtt=*/TimeDelta::Millis(1000));
+  EXPECT_FALSE(bwe.IsRttAboveLimit());
+}
+
 }  // namespace webrtc