From 7db19e0b0208e6fded74901561ecdbaab18d32e6 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Wed, 24 Jul 2019 15:51:12 +0200 Subject: [PATCH] Report congestion window updates on GoogCC time updates In https://webrtc-review.googlesource.com/c/src/+/138275 the congestion window was recalculated during OnProcessInterval, as to consider the case when downlink is down. However, this update was not propagated to the congestion window pusback controller, nor returned in the update. This patch fixes that issue, as well as adding two tests to ensure the behaviour works as expected. Bug: None Change-Id: Ic126d929dc7a7a3393a2f34a4682eea1ee1f2240 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146704 Commit-Queue: Evan Shrubsole Reviewed-by: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#28667} --- .../goog_cc/goog_cc_network_control.cc | 6 ++ .../goog_cc_network_control_unittest.cc | 71 +++++++++++++++++++ 2 files changed, 77 insertions(+) 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 df75b04927..7dfff3d5f5 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -209,6 +209,12 @@ NetworkControlUpdate GoogCcNetworkController::OnProcessInterval( last_packet_received_time_.IsFinite() && !feedback_max_rtts_.empty()) { UpdateCongestionWindowSize(msg.at_time - last_packet_received_time_); } + if (congestion_window_pushback_controller_ && current_data_window_) { + congestion_window_pushback_controller_->SetDataWindow( + *current_data_window_); + } else { + update.congestion_window = current_data_window_; + } MaybeTriggerOnNetworkChanged(&update, msg.at_time); return update; } 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 34650da9e0..c32c26b2f8 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 @@ -375,6 +375,77 @@ TEST_F(GoogCcNetworkControllerTest, EXPECT_NEAR(client->padding_rate().kbps(), client->target_rate().kbps(), 1); } +TEST_F(GoogCcNetworkControllerTest, + NoCongestionWindowPushbackWithoutReceiveTraffic) { + ScopedFieldTrials trial( + "WebRTC-CongestionWindow/QueueSize:800,MinBitrate:30000/" + "WebRTC-Bwe-CongestionWindowDownlinkDelay/Enabled/"); + Scenario s("googcc_unit/cwnd_no_downlink", false); + NetworkSimulationConfig net_conf; + net_conf.bandwidth = DataRate::kbps(1000); + net_conf.delay = TimeDelta::ms(100); + auto send_net = s.CreateSimulationNode(net_conf); + auto ret_net = s.CreateMutableSimulationNode(net_conf); + + auto* client = s.CreateClient("sender", CallClientConfig()); + auto* route = s.CreateRoutes(client, {send_net}, + s.CreateClient("return", CallClientConfig()), + {ret_net->node()}); + + s.CreateVideoStream(route->forward(), VideoStreamConfig()); + // A return video stream ensures we get steady traffic stream, + // so we can better differentiate between send being down and return + // being down. + s.CreateVideoStream(route->reverse(), VideoStreamConfig()); + + // Wait to stabilize the bandwidth estimate. + s.RunFor(TimeDelta::seconds(10)); + // Disabling the return triggers the data window expansion logic + // which will stop the congestion window from activating. + ret_net->PauseTransmissionUntil(s.Now() + TimeDelta::seconds(10)); + s.RunFor(TimeDelta::seconds(5)); + + // Expect that we never lost send speed because we received no packets. + // 500kbps is enough to demonstrate that congestion window isn't activated. + EXPECT_GE(client->target_rate().kbps(), 500); +} + +TEST_F(GoogCcNetworkControllerTest, CongestionWindowPushBackOnSendDelaySpike) { + ScopedFieldTrials trial( + "WebRTC-CongestionWindow/QueueSize:800,MinBitrate:30000/" + "WebRTC-Bwe-CongestionWindowDownlinkDelay/Enabled/"); + Scenario s("googcc_unit/cwnd_actives_no_feedback", false); + NetworkSimulationConfig net_conf; + net_conf.bandwidth = DataRate::kbps(1000); + net_conf.delay = TimeDelta::ms(100); + auto send_net = s.CreateMutableSimulationNode(net_conf); + auto ret_net = s.CreateSimulationNode(net_conf); + + auto* client = s.CreateClient("sender", CallClientConfig()); + auto* route = + s.CreateRoutes(client, {send_net->node()}, + s.CreateClient("return", CallClientConfig()), {ret_net}); + + s.CreateVideoStream(route->forward(), VideoStreamConfig()); + // A return video stream ensures we get steady traffic stream, + // so we can better differentiate between send being down and return + // being down. + s.CreateVideoStream(route->reverse(), VideoStreamConfig()); + + // Wait to stabilize the bandwidth estimate. + s.RunFor(TimeDelta::seconds(10)); + // Send being down triggers congestion window pushback. + send_net->PauseTransmissionUntil(s.Now() + TimeDelta::seconds(10)); + s.RunFor(TimeDelta::seconds(3)); + + // Expect the target rate to be reduced rapidly due to congestion. + // We would expect things to be at 30kbps, the min bitrate. Note + // that the congestion window still gets activated since we are + // receiving packets upstream. + EXPECT_LT(client->target_rate().kbps(), 100); + EXPECT_GE(client->target_rate().kbps(), 30); +} + TEST_F(GoogCcNetworkControllerTest, LimitsToFloorIfRttIsHighInTrial) { // 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