From 5d4740170a0a7cf4c9c83dc35ed355d011e9cafe Mon Sep 17 00:00:00 2001 From: Christoffer Rodbro Date: Mon, 10 Dec 2018 16:14:53 +0100 Subject: [PATCH] Reduce pacing buffer padding rate during pushback. Bug: webrtc:10112 Change-Id: I2cd2d07bd5bcbff5b3808ee63eea251a52e45b79 Reviewed-on: https://webrtc-review.googlesource.com/c/113808 Reviewed-by: Sebastian Jansson Commit-Queue: Christoffer Rodbro Cr-Commit-Position: refs/heads/master@{#25968} --- .../goog_cc/goog_cc_network_control.cc | 4 +- .../goog_cc/goog_cc_network_control.h | 1 + .../goog_cc_network_control_unittest.cc | 37 +++++++++++++++++++ test/scenario/scenario_config.h | 1 + test/scenario/simulated_time.cc | 6 +++ test/scenario/simulated_time.h | 1 + 6 files changed, 49 insertions(+), 1 deletion(-) 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 c99a6c97e5..6de143d1df 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -150,6 +150,7 @@ GoogCcNetworkController::GoogCcNetworkController(RtcEventLog* event_log, absl::make_unique()), initial_config_(config), last_target_rate_(*config.constraints.starting_rate), + pushback_target_rate_(last_target_rate_), pacing_factor_(config.stream_based_config.pacing_factor.value_or( kDefaultPaceMultiplier)), min_pacing_rate_(config.stream_based_config.min_pacing_rate.value_or( @@ -619,6 +620,7 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged( pushback_rate); target_rate = DataRate::bps(pushback_rate); } + pushback_target_rate_ = target_rate; TargetTransferRate target_rate_msg; target_rate_msg.at_time = at_time; @@ -642,7 +644,7 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged( PacerConfig GoogCcNetworkController::GetPacingRates(Timestamp at_time) const { DataRate pacing_rate = std::max(min_pacing_rate_, last_target_rate_) * pacing_factor_; - DataRate padding_rate = std::min(max_padding_rate_, last_target_rate_); + DataRate padding_rate = std::min(max_padding_rate_, pushback_target_rate_); PacerConfig msg; msg.at_time = at_time; msg.time_window = TimeDelta::seconds(1); diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.h b/modules/congestion_controller/goog_cc/goog_cc_network_control.h index 057ed121fa..c635ab483b 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h @@ -95,6 +95,7 @@ class GoogCcNetworkController : public NetworkControllerInterface { std::deque feedback_max_rtts_; DataRate last_target_rate_; + DataRate pushback_target_rate_; int32_t last_estimated_bitrate_bps_ = 0; uint8_t last_estimated_fraction_loss_ = 0; diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc index 79ded001bd..62be521425 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc @@ -357,6 +357,43 @@ TEST_F(GoogCcNetworkControllerTest, 20); } +TEST_F(GoogCcNetworkControllerTest, + PaddingRateLimitedByCongestionWindowInTrial) { + ScopedFieldTrials trial( + "WebRTC-CongestionWindowPushback/Enabled/WebRTC-CwndExperiment/" + "Enabled-200/"); + + Scenario s("googcc_unit/padding_limited", false); + NetworkNodeConfig net_conf; + auto send_net = s.CreateSimulationNode([=](NetworkNodeConfig* c) { + c->simulation.bandwidth = DataRate::kbps(1000); + c->simulation.delay = TimeDelta::ms(100); + c->update_frequency = TimeDelta::ms(5); + }); + auto ret_net = s.CreateSimulationNode([](NetworkNodeConfig* c) { + c->simulation.delay = TimeDelta::ms(100); + c->update_frequency = TimeDelta::ms(5); + }); + SimulatedTimeClientConfig config; + config.transport.cc = + TransportControllerConfig::CongestionController::kGoogCc; + // Start high so bandwidth drop has max effect. + config.transport.rates.start_rate = DataRate::kbps(1000); + config.transport.rates.max_rate = DataRate::kbps(2000); + config.transport.rates.max_padding_rate = config.transport.rates.max_rate; + SimulatedTimeClient* client = s.CreateSimulatedTimeClient( + "send", config, {PacketStreamConfig()}, {send_net}, {ret_net}); + // Run for a few seconds to allow the controller to stabilize. + s.RunFor(TimeDelta::seconds(10)); + + // Check that padding rate matches target rate. + EXPECT_NEAR(client->padding_rate().kbps(), client->target_rate_kbps(), 1); + + // Check this is also the case when congestion window pushback kicks in. + send_net->PauseTransmissionUntil(s.Now() + TimeDelta::seconds(1)); + EXPECT_NEAR(client->padding_rate().kbps(), client->target_rate_kbps(), 1); +} + TEST_F(GoogCcNetworkControllerTest, LimitsToMinRateIfRttIsHighInTrial) { // The field trial limits maximum RTT to 2 seconds, higher RTT means that the // controller backs off until it reaches the minimum configured bitrate. This diff --git a/test/scenario/scenario_config.h b/test/scenario/scenario_config.h index 7716b4e8ae..cb538a7bf5 100644 --- a/test/scenario/scenario_config.h +++ b/test/scenario/scenario_config.h @@ -44,6 +44,7 @@ struct TransportControllerConfig { DataRate min_rate = DataRate::kbps(30); DataRate max_rate = DataRate::kbps(3000); DataRate start_rate = DataRate::kbps(300); + DataRate max_padding_rate = DataRate::Zero(); } rates; enum CongestionController { kBbr, kGoogCc, kGoogCcFeedback } cc = kGoogCc; TimeDelta state_log_interval = TimeDelta::ms(100); diff --git a/test/scenario/simulated_time.cc b/test/scenario/simulated_time.cc index becd79367d..90ea5188e8 100644 --- a/test/scenario/simulated_time.cc +++ b/test/scenario/simulated_time.cc @@ -264,6 +264,8 @@ SimulatedTimeClient::SimulatedTimeClient( current_contraints_.max_data_rate = config.transport.rates.max_rate; NetworkControllerConfig initial_config; initial_config.constraints = current_contraints_; + initial_config.stream_based_config.max_padding_rate = + config.transport.rates.max_padding_rate; congestion_controller_ = network_controller_factory_.Create(initial_config); for (auto& stream_config : stream_configs) packet_streams_.emplace_back(new PacketStream(stream_config)); @@ -364,5 +366,9 @@ double SimulatedTimeClient::target_rate_kbps() const { return target_rate_.kbps(); } +DataRate SimulatedTimeClient::padding_rate() const { + return sender_.pacer_config_.pad_rate(); +} + } // namespace test } // namespace webrtc diff --git a/test/scenario/simulated_time.h b/test/scenario/simulated_time.h index 4d91c8ff21..da5e972c80 100644 --- a/test/scenario/simulated_time.h +++ b/test/scenario/simulated_time.h @@ -139,6 +139,7 @@ class SimulatedTimeClient : NetworkReceiverInterface { TimeDelta GetNetworkControllerProcessInterval() const; double target_rate_kbps() const; DataRate link_capacity() const; + DataRate padding_rate() const; bool TryDeliverPacket(rtc::CopyOnWriteBuffer packet, uint64_t receiver,