diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/modules/bitrate_controller/send_side_bandwidth_estimation.cc index a1672cbb3f..5b3591d304 100644 --- a/modules/bitrate_controller/send_side_bandwidth_estimation.cc +++ b/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; diff --git a/modules/bitrate_controller/send_side_bandwidth_estimation.h b/modules/bitrate_controller/send_side_bandwidth_estimation.h index b016faba2e..63b9cb820c 100644 --- a/modules/bitrate_controller/send_side_bandwidth_estimation.h +++ b/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 rtt_limit_; FieldTrialParameter drop_fraction_; FieldTrialParameter drop_interval_; FieldTrialFlag persist_on_route_change_; + FieldTrialParameter 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. 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 7f188e6f44..0f81962a88 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 @@ 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()); 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 0b1255180e..67ab99e4ba 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, 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 diff --git a/test/scenario/video_stream.cc b/test/scenario/video_stream.cc index 500a45d4e1..a3c6d3c052 100644 --- a/test/scenario/video_stream.cc +++ b/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 modifier) { rtc::CritScope cs(&crit_); diff --git a/test/scenario/video_stream.h b/test/scenario/video_stream.h index e38e30ec33..45180e52e1 100644 --- a/test/scenario/video_stream.h +++ b/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 modifier); private: