Fix for RttBackoff when sending of packets with TWCC stops.

Bug: webrtc:10290
Change-Id: Ia825cbde070214e5ec9f5439246ea43f58c3c2b7
Reviewed-on: https://webrtc-review.googlesource.com/c/121561
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Christoffer Rodbro <crodbro@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26605}

modules/bitrate_controller/send_side_bandwidth_estimation.cc

@@ -151,12 +151,14 @@ RttBasedBackoff::RttBasedBackoff()
       drop_fraction_("fraction", 0.5),
       drop_interval_("interval", TimeDelta::ms(300)),
       persist_on_route_change_("persist"),
+      safe_timeout_("safe_timeout", true),
       // By initializing this to plus infinity, we make sure that we never
       // trigger rtt backoff unless packet feedback is enabled.
       last_propagation_rtt_update_(Timestamp::PlusInfinity()),
-      last_propagation_rtt_(TimeDelta::Zero()) {
+      last_propagation_rtt_(TimeDelta::Zero()),
+      last_packet_sent_(Timestamp::MinusInfinity()) {
   ParseFieldTrial({&rtt_limit_, &drop_fraction_, &drop_interval_,
-                   &persist_on_route_change_},
+                   &persist_on_route_change_, &safe_timeout_},
                   field_trial::FindFullName("WebRTC-Bwe-MaxRttLimit"));
 }
 
@@ -173,10 +175,16 @@ void RttBasedBackoff::UpdatePropagationRtt(Timestamp at_time,
   last_propagation_rtt_ = propagation_rtt;
 }
 
-TimeDelta RttBasedBackoff::RttLowerBound(Timestamp at_time) const {
-  // TODO(srte): Use time since last unacknowledged packet for this.
+TimeDelta RttBasedBackoff::CorrectedRtt(Timestamp at_time) const {
   TimeDelta time_since_rtt = at_time - last_propagation_rtt_update_;
-  return time_since_rtt + last_propagation_rtt_;
+  TimeDelta timeout_correction = time_since_rtt;
+  if (safe_timeout_) {
+    // 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());
+  }
+  return timeout_correction + last_propagation_rtt_;
 }
 
 RttBasedBackoff::~RttBasedBackoff() = default;
@@ -432,7 +440,7 @@ void SendSideBandwidthEstimation::UpdateRtt(TimeDelta rtt, Timestamp at_time) {
 
 void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) {
   DataRate new_bitrate = current_bitrate_;
-  if (rtt_backoff_.RttLowerBound(at_time) > rtt_backoff_.rtt_limit_) {
+  if (rtt_backoff_.CorrectedRtt(at_time) > rtt_backoff_.rtt_limit_) {
     if (at_time - time_last_decrease_ >= rtt_backoff_.drop_interval_) {
       time_last_decrease_ = at_time;
       new_bitrate = current_bitrate_ * rtt_backoff_.drop_fraction_;
@@ -552,6 +560,11 @@ void SendSideBandwidthEstimation::UpdatePropagationRtt(
   rtt_backoff_.UpdatePropagationRtt(at_time, propagation_rtt);
 }
 
+void SendSideBandwidthEstimation::OnSentPacket(const SentPacket& sent_packet) {
+  // Only feedback-triggering packets will be reported here.
+  rtt_backoff_.last_packet_sent_ = sent_packet.send_time;
+}
+
 bool SendSideBandwidthEstimation::IsInStartPhase(Timestamp at_time) const {
   return first_report_time_.IsInfinite() ||
          at_time - first_report_time_ < kStartPhase;

modules/bitrate_controller/send_side_bandwidth_estimation.h

@@ -53,16 +53,18 @@ class RttBasedBackoff {
   ~RttBasedBackoff();
   void OnRouteChange();
   void UpdatePropagationRtt(Timestamp at_time, TimeDelta propagation_rtt);
-  TimeDelta RttLowerBound(Timestamp at_time) const;
+  TimeDelta CorrectedRtt(Timestamp at_time) const;
 
   FieldTrialParameter<TimeDelta> rtt_limit_;
   FieldTrialParameter<double> drop_fraction_;
   FieldTrialParameter<TimeDelta> drop_interval_;
   FieldTrialFlag persist_on_route_change_;
+  FieldTrialParameter<bool> safe_timeout_;
 
  public:
   Timestamp last_propagation_rtt_update_;
   TimeDelta last_propagation_rtt_;
+  Timestamp last_packet_sent_;
 };
 
 class SendSideBandwidthEstimation {
@@ -76,7 +78,7 @@ class SendSideBandwidthEstimation {
   DataRate GetEstimatedLinkCapacity() const;
   // Call periodically to update estimate.
   void UpdateEstimate(Timestamp at_time);
-  void OnSentPacket(SentPacket sent_packet);
+  void OnSentPacket(const SentPacket& sent_packet);
   void UpdatePropagationRtt(Timestamp at_time, TimeDelta propagation_rtt);
 
   // Call when we receive a RTCP message with TMMBR or REMB.

modules/congestion_controller/goog_cc/goog_cc_network_control.cc

@@ -277,6 +277,7 @@ NetworkControlUpdate GoogCcNetworkController::OnSentPacket(
     bandwidth_estimation_->UpdatePropagationRtt(sent_packet.send_time,
                                                 TimeDelta::Zero());
   }
+  bandwidth_estimation_->OnSentPacket(sent_packet);
   if (congestion_window_pushback_controller_) {
     congestion_window_pushback_controller_->UpdateOutstandingData(
         sent_packet.data_in_flight.bytes());

modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc

@@ -208,5 +208,28 @@ TEST(GoogCcNetworkControllerTest, NoBandwidthTogglingInLossControlTrial) {
   }
 }
 
+TEST(GoogCcNetworkControllerTest, NoRttBackoffCollapseWhenVideoStops) {
+  ScopedFieldTrials trial("WebRTC-Bwe-MaxRttLimit/limit:2s/");
+  Scenario s("googcc_unit/rttbackoff_video_stop", true);
+  auto* send_net = s.CreateSimulationNode([&](NetworkNodeConfig* c) {
+    c->simulation.bandwidth = DataRate::kbps(2000);
+    c->simulation.delay = TimeDelta::ms(100);
+  });
+
+  auto* client = s.CreateClient("send", [&](CallClientConfig* c) {
+    c->transport.cc = TransportControllerConfig::CongestionController::kGoogCc;
+    c->transport.rates.start_rate = DataRate::kbps(1000);
+  });
+  auto* route = s.CreateRoutes(client, {send_net},
+                               s.CreateClient("return", CallClientConfig()),
+                               {s.CreateSimulationNode(NetworkNodeConfig())});
+  auto* video = s.CreateVideoStream(route->forward(), VideoStreamConfig());
+  // Allow the controller to initialize, then stop video.
+  s.RunFor(TimeDelta::seconds(1));
+  video->send()->Stop();
+  s.RunFor(TimeDelta::seconds(4));
+  EXPECT_GT(client->send_bandwidth().kbps(), 1000);
+}
+
 }  // namespace test
 }  // namespace webrtc

test/scenario/video_stream.cc

@@ -271,6 +271,10 @@ void SendVideoStream::Start() {
   sender_->call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
 }
 
+void SendVideoStream::Stop() {
+  send_stream_->Stop();
+}
+
 void SendVideoStream::UpdateConfig(
     std::function<void(VideoStreamConfig*)> modifier) {
   rtc::CritScope cs(&crit_);

test/scenario/video_stream.h

@@ -36,6 +36,7 @@ class SendVideoStream {
   VideoSendStream::Stats GetStats() const;
   ColumnPrinter StatsPrinter();
   void Start();
+  void Stop();
   void UpdateConfig(std::function<void(VideoStreamConfig*)> modifier);
 
  private: