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}diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/modules/bitrate_controller/send_side_bandwidth_estimation.cc
index a1672cb..5b3591d 100644
--- a/modules/bitrate_controller/send_side_bandwidth_estimation.cc
+++ b/modules/bitrate_controller/send_side_bandwidth_estimation.cc
@@ -151,12 +151,14 @@
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 @@
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::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 @@
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;
diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.h b/modules/bitrate_controller/send_side_bandwidth_estimation.h
index b016fab..63b9cb8 100644
--- a/modules/bitrate_controller/send_side_bandwidth_estimation.h
+++ b/modules/bitrate_controller/send_side_bandwidth_estimation.h
@@ -53,16 +53,18 @@
~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 @@
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.
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
index 7f188e6..0f81962 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@@ -277,6 +277,7 @@
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());
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc
index 0b125518..67ab99e 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_slowtest.cc
@@ -208,5 +208,28 @@
}
}
+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
diff --git a/test/scenario/video_stream.cc b/test/scenario/video_stream.cc
index 500a45d..a3c6d3c 100644
--- a/test/scenario/video_stream.cc
+++ b/test/scenario/video_stream.cc
@@ -271,6 +271,10 @@
sender_->call_->SignalChannelNetworkState(MediaType::VIDEO, kNetworkUp);
}
+void SendVideoStream::Stop() {
+ send_stream_->Stop();
+}
+
void SendVideoStream::UpdateConfig(
std::function<void(VideoStreamConfig*)> modifier) {
rtc::CritScope cs(&crit_);
diff --git a/test/scenario/video_stream.h b/test/scenario/video_stream.h
index e38e30e..45180e5 100644
--- a/test/scenario/video_stream.h
+++ b/test/scenario/video_stream.h
@@ -36,6 +36,7 @@
VideoSendStream::Stats GetStats() const;
ColumnPrinter StatsPrinter();
void Start();
+ void Stop();
void UpdateConfig(std::function<void(VideoStreamConfig*)> modifier);
private:
