Adds trial to fall back to probe rate if ack rate is missing.

Bug: webrtc:9718
Change-Id: I7b6e1d3c051e67b97f6de1ec95e84631af9c5b0d
Reviewed-on: https://webrtc-review.googlesource.com/c/113600
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25953}

modules/congestion_controller/goog_cc/goog_cc_network_control.cc

@@ -136,6 +136,8 @@ GoogCcNetworkController::GoogCcNetworkController(RtcEventLog* event_log,
       safe_reset_acknowledged_rate_("ack"),
       use_stable_bandwidth_estimate_(
           field_trial::IsEnabled("WebRTC-Bwe-StableBandwidthEstimate")),
+      fall_back_to_probe_rate_(
+          field_trial::IsEnabled("WebRTC-Bwe-ProbeRateFallback")),
       probe_controller_(new ProbeController()),
       congestion_window_pushback_controller_(
           MaybeInitalizeCongestionWindowPushbackController()),
@@ -486,9 +488,6 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
   acknowledged_bitrate_estimator_->IncomingPacketFeedbackVector(
       received_feedback_vector);
   auto acknowledged_bitrate = acknowledged_bitrate_estimator_->bitrate();
-  bandwidth_estimation_->SetAcknowledgedRate(acknowledged_bitrate,
-                                             report.feedback_time);
-  bandwidth_estimation_->IncomingPacketFeedbackVector(report);
   for (const auto& feedback : received_feedback_vector) {
     if (feedback.pacing_info.probe_cluster_id != PacedPacketInfo::kNotAProbe) {
       probe_bitrate_estimator_->HandleProbeAndEstimateBitrate(feedback);
@@ -496,7 +495,11 @@ NetworkControlUpdate GoogCcNetworkController::OnTransportPacketsFeedback(
   }
   absl::optional<DataRate> probe_bitrate =
       probe_bitrate_estimator_->FetchAndResetLastEstimatedBitrate();
-
+  if (fall_back_to_probe_rate_ && !acknowledged_bitrate)
+    acknowledged_bitrate = probe_bitrate_estimator_->last_estimate();
+  bandwidth_estimation_->SetAcknowledgedRate(acknowledged_bitrate,
+                                             report.feedback_time);
+  bandwidth_estimation_->IncomingPacketFeedbackVector(report);
   DelayBasedBwe::Result result;
   result = delay_based_bwe_->IncomingPacketFeedbackVector(
       received_feedback_vector, acknowledged_bitrate, probe_bitrate,

modules/congestion_controller/goog_cc/goog_cc_network_control.h

@@ -72,6 +72,7 @@ class GoogCcNetworkController : public NetworkControllerInterface {
   FieldTrialFlag safe_reset_on_route_change_;
   FieldTrialFlag safe_reset_acknowledged_rate_;
   const bool use_stable_bandwidth_estimate_;
+  const bool fall_back_to_probe_rate_;
 
   const std::unique_ptr<ProbeController> probe_controller_;
   const std::unique_ptr<CongestionWindowPushbackController>

modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc

@@ -74,12 +74,10 @@ TEST(GoogCcNetworkControllerTest, CutsHighRateInSafeResetTrial) {
   EXPECT_NEAR(client->send_bandwidth().kbps(), kStartRate.kbps(), 30);
 }
 
-// This test is flaky because probing on route change can trigger overuse
-// without having any acknowledged rate, causing a 50% backoff from the probe
-// rate.
-// TODO(srte): Add a fix for the above problem and enable this test.
-TEST(GoogCcNetworkControllerTest, DISABLED_DetectsHighRateInSafeResetTrial) {
-  ScopedFieldTrials trial("WebRTC-Bwe-SafeResetOnRouteChange/Enabled,ack/");
+TEST(GoogCcNetworkControllerTest, DetectsHighRateInSafeResetTrial) {
+  ScopedFieldTrials trial(
+      "WebRTC-Bwe-SafeResetOnRouteChange/Enabled,ack/"
+      "WebRTC-Bwe-ProbeRateFallback/Enabled/");
   const DataRate kInitialLinkCapacity = DataRate::kbps(200);
   const DataRate kNewLinkCapacity = DataRate::kbps(800);
   const DataRate kStartRate = DataRate::kbps(300);
@@ -113,7 +111,7 @@ TEST(GoogCcNetworkControllerTest, DISABLED_DetectsHighRateInSafeResetTrial) {
   EXPECT_NEAR(client->send_bandwidth().kbps(), kInitialLinkCapacity.kbps(), 50);
   // However, probing should have made us detect the higher rate.
   s.RunFor(TimeDelta::ms(2000));
-  EXPECT_NEAR(client->send_bandwidth().kbps(), kNewLinkCapacity.kbps(), 200);
+  EXPECT_GT(client->send_bandwidth().kbps(), kNewLinkCapacity.kbps() - 300);
 }
 
 }  // namespace test

modules/congestion_controller/goog_cc/probe_bitrate_estimator.cc

@@ -166,6 +166,7 @@ int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate(
     event_log_->Log(
         absl::make_unique<RtcEventProbeResultSuccess>(cluster_id, res));
   }
+  last_estimate_ = DataRate::bps(res);
   estimated_bitrate_bps_ = res;
   return *estimated_bitrate_bps_;
 }
@@ -179,6 +180,10 @@ ProbeBitrateEstimator::FetchAndResetLastEstimatedBitrate() {
   return absl::nullopt;
 }
 
+absl::optional<DataRate> ProbeBitrateEstimator::last_estimate() const {
+  return last_estimate_;
+}
+
 void ProbeBitrateEstimator::EraseOldClusters(int64_t timestamp_ms) {
   for (auto it = clusters_.begin(); it != clusters_.end();) {
     if (it->second.last_receive_ms < timestamp_ms) {

modules/congestion_controller/goog_cc/probe_bitrate_estimator.h

@@ -14,6 +14,8 @@
 #include <limits>
 #include <map>
 
+#include "absl/types/optional.h"
+#include "api/units/data_rate.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 
 namespace webrtc {
@@ -30,6 +32,8 @@ class ProbeBitrateEstimator {
 
   absl::optional<DataRate> FetchAndResetLastEstimatedBitrate();
 
+  absl::optional<DataRate> last_estimate() const;
+
  private:
   struct AggregatedCluster {
     int num_probes = 0;
@@ -48,6 +52,7 @@ class ProbeBitrateEstimator {
   std::map<int, AggregatedCluster> clusters_;
   RtcEventLog* const event_log_;
   absl::optional<int> estimated_bitrate_bps_;
+  absl::optional<DataRate> last_estimate_;
 };
 
 }  // namespace webrtc