Limit BWE drops from probes to 85% of the throughput estimate.

Bug: webrtc:11498
Change-Id: Ia4bb1a3cbde951d7fce5f4408da481ee506f8d21
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173180
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31088}

modules/congestion_controller/goog_cc/goog_cc_network_control.cc

@@ -42,7 +42,13 @@ constexpr TimeDelta kLossUpdateInterval = TimeDelta::Millis(1000);
 // the number of bytes that can be transmitted per interval.
 // Increasing this factor will result in lower delays in cases of bitrate
 // overshoots from the encoder.
-const float kDefaultPaceMultiplier = 2.5f;
+constexpr float kDefaultPaceMultiplier = 2.5f;
+
+// If the probe result is far below the current throughput estimate
+// it's unlikely that the probe is accurate, so we don't want to drop too far.
+// However, if we actually are overusing, we want to drop to something slightly
+// below the current throughput estimate to drain the network queues.
+constexpr double kProbeDropThroughputFraction = 0.85;
 
 int64_t GetBpsOrDefault(const absl::optional<DataRate>& rate,
                         int64_t fallback_bps) {
@@ -75,6 +81,9 @@ GoogCcNetworkController::GoogCcNetworkController(NetworkControllerConfig config,
       ignore_probes_lower_than_network_estimate_(IsNotDisabled(
           key_value_config_,
           "WebRTC-Bwe-IgnoreProbesLowerThanNetworkStateEstimate")),
+      limit_probes_lower_than_throughput_estimate_(
+          IsEnabled(key_value_config_,
+                    "WebRTC-Bwe-LimitProbesLowerThanThroughputEstimate")),
       rate_control_settings_(
           RateControlSettings::ParseFromKeyValueConfig(key_value_config_)),
       loss_based_stable_rate_(
@@ -493,7 +502,7 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
     network_estimator_->OnTransportPacketsFeedback(report);
     auto prev_estimate = estimate_;
     estimate_ = network_estimator_->GetCurrentEstimate();
-    // TODO(srte): Make OnTransportPacketsFeedback signal wether the state
+    // TODO(srte): Make OnTransportPacketsFeedback signal whether the state
     // changed to avoid the need for this check.
     if (estimate_ && (!prev_estimate || estimate_->last_feed_time !=
                                             prev_estimate->last_feed_time)) {
@@ -508,6 +517,20 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
       *probe_bitrate < estimate_->link_capacity_lower) {
     probe_bitrate.reset();
   }
+  if (limit_probes_lower_than_throughput_estimate_ && probe_bitrate &&
+      acknowledged_bitrate) {
+    // Limit the backoff to something slightly below the acknowledged
+    // bitrate. ("Slightly below" because we want to drain the queues
+    // if we are actually overusing.)
+    // The acknowledged bitrate shouldn't normally be higher than the delay
+    // based estimate, but it could happen e.g. due to packet bursts or
+    // encoder overshoot. We use std::min to ensure that a probe result
+    // below the current BWE never causes an increase.
+    DataRate limit =
+        std::min(delay_based_bwe_->last_estimate(),
+                 *acknowledged_bitrate * kProbeDropThroughputFraction);
+    probe_bitrate = std::max(*probe_bitrate, limit);
+  }
 
   NetworkControlUpdate update;
   bool recovered_from_overuse = false;

modules/congestion_controller/goog_cc/goog_cc_network_control.h

@@ -87,6 +87,7 @@ class GoogCcNetworkController : public NetworkControllerInterface {
   FieldTrialFlag safe_reset_acknowledged_rate_;
   const bool use_min_allocatable_as_lower_bound_;
   const bool ignore_probes_lower_than_network_estimate_;
+  const bool limit_probes_lower_than_throughput_estimate_;
   const RateControlSettings rate_control_settings_;
   const bool loss_based_stable_rate_;
 

modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc

@@ -179,7 +179,7 @@ absl::optional<DataRate> ProbeBitrateEstimator::HandleProbeAndEstimateBitrate(
         std::make_unique<RtcEventProbeResultSuccess>(cluster_id, res.bps()));
   }
   estimated_data_rate_ = res;
-  return *estimated_data_rate_;
+  return estimated_data_rate_;
 }
 
 absl::optional<DataRate>