From ae68ea000865bf8233780ce32be25f76313ebaa7 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Tue, 9 Apr 2019 09:59:00 +0200 Subject: [PATCH] Reland "Add new field trial for controlling congestion window settings" This is a reland of dd33d8ec7113ae7bee1511dc9f3f2d6336a7f083 Original change's description: > Add new field trial for controlling congestion window settings > > Bug: None > Change-Id: Idb7425e394db74a9dfb4f3764a58710497adff56 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131127 > Reviewed-by: Magnus Flodman > Reviewed-by: Christoffer Rodbro > Commit-Queue: Evan Shrubsole > Cr-Commit-Position: refs/heads/master@{#27538} TBR=mflodman@webrtc.org,crodbro@webrtc.org Bug: None Change-Id: Icee2efb90e219ef2c3384ad84498fd6938a98e56 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132400 Reviewed-by: Mirko Bonadei Reviewed-by: Magnus Flodman Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#27550} --- .../goog_cc_network_control_unittest.cc | 10 ++-- rtc_base/experiments/rate_control_settings.cc | 51 +++---------------- .../rate_control_settings_unittest.cc | 5 +- .../transport_feedback_tests.cc | 2 +- 4 files changed, 14 insertions(+), 54 deletions(-) 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 a7bc235eff..baa278a0a1 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 @@ -239,8 +239,7 @@ TEST_F(GoogCcNetworkControllerTest, ReactsToChangedNetworkConditions) { // Test congestion window pushback on network delay happens. TEST_F(GoogCcNetworkControllerTest, CongestionWindowPushbackOnNetworkDelay) { ScopedFieldTrials trial( - "WebRTC-CongestionWindowPushback/Enabled/WebRTC-CwndExperiment/" - "Enabled-800/"); + "WebRTC-CongestionWindow/QueueSize:800,MinBitrate:30000/"); Scenario s("googcc_unit/cwnd_on_delay", false); auto send_net = s.CreateSimulationNode([=](NetworkNodeConfig* c) { c->simulation.bandwidth = DataRate::kbps(1000); @@ -328,8 +327,7 @@ TEST_F(GoogCcNetworkControllerTest, UpdatesDelayBasedEstimate) { TEST_F(GoogCcNetworkControllerTest, PaddingRateLimitedByCongestionWindowInTrial) { ScopedFieldTrials trial( - "WebRTC-CongestionWindowPushback/Enabled/WebRTC-CwndExperiment/" - "Enabled-200/"); + "WebRTC-CongestionWindow/QueueSize:200,MinBitrate:30000/"); Scenario s("googcc_unit/padding_limited", false); NetworkNodeConfig net_conf; @@ -633,8 +631,8 @@ TEST_F(GoogCcNetworkControllerTest, TargetRateReducedOnPacingBufferBuildupInTrial) { // Configure strict pacing to ensure build-up. ScopedFieldTrials trial( - "WebRTC-CongestionWindowPushback/Enabled/WebRTC-CwndExperiment/" - "Enabled-100/WebRTC-Video-Pacing/factor:1.0/" + "WebRTC-CongestionWindow/QueueSize:100,MinBitrate:30000/" + "WebRTC-Video-Pacing/factor:1.0/" "WebRTC-AddPacingToCongestionWindowPushback/Enabled/"); const DataRate kLinkCapacity = DataRate::kbps(1000); diff --git a/rtc_base/experiments/rate_control_settings.cc b/rtc_base/experiments/rate_control_settings.cc index 608996b8ac..7ef42dea94 100644 --- a/rtc_base/experiments/rate_control_settings.cc +++ b/rtc_base/experiments/rate_control_settings.cc @@ -16,6 +16,7 @@ #include #include "api/transport/field_trial_based_config.h" +#include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_conversions.h" @@ -23,11 +24,8 @@ namespace webrtc { namespace { -const char* kCongestionWindowFieldTrialName = "WebRTC-CwndExperiment"; const int kDefaultAcceptedQueueMs = 250; -const char* kCongestionWindowPushbackFieldTrialName = - "WebRTC-CongestionWindowPushback"; const int kDefaultMinPushbackTargetBitrateBps = 30000; const char kVp8TrustedRateControllerFieldTrialName[] = @@ -43,40 +41,6 @@ const char* kScreenshareHysteresisFieldTrialname = // Default to 35% hysteresis for simulcast screenshare. const double kDefaultScreenshareHysteresisFactor = 1.35; -absl::optional MaybeReadCwndExperimentParameter( - const WebRtcKeyValueConfig* const key_value_config) { - int64_t accepted_queue_ms; - std::string experiment_string = - key_value_config->Lookup(kCongestionWindowFieldTrialName); - int parsed_values = - sscanf(experiment_string.c_str(), "Enabled-%" PRId64, &accepted_queue_ms); - if (parsed_values == 1) { - RTC_CHECK_GE(accepted_queue_ms, 0) - << "Accepted must be greater than or equal to 0."; - return rtc::checked_cast(accepted_queue_ms); - } else if (experiment_string.find("Enabled") == 0) { - return kDefaultAcceptedQueueMs; - } - return absl::nullopt; -} - -absl::optional MaybeReadCongestionWindowPushbackExperimentParameter( - const WebRtcKeyValueConfig* const key_value_config) { - uint32_t min_pushback_target_bitrate_bps; - std::string experiment_string = - key_value_config->Lookup(kCongestionWindowPushbackFieldTrialName); - int parsed_values = sscanf(experiment_string.c_str(), "Enabled-%" PRIu32, - &min_pushback_target_bitrate_bps); - if (parsed_values == 1) { - RTC_CHECK_GE(min_pushback_target_bitrate_bps, 0) - << "Min pushback target bitrate must be greater than or equal to 0."; - return rtc::checked_cast(min_pushback_target_bitrate_bps); - } else if (experiment_string.find("Enabled") == 0) { - return kDefaultMinPushbackTargetBitrateBps; - } - return absl::nullopt; -} - bool IsEnabled(const WebRtcKeyValueConfig* const key_value_config, absl::string_view key) { return key_value_config->Lookup(key).find("Enabled") == 0; @@ -98,12 +62,8 @@ double ParseHysteresisFactor(const WebRtcKeyValueConfig* const key_value_config, RateControlSettings::RateControlSettings( const WebRtcKeyValueConfig* const key_value_config) - : congestion_window_("cwnd", - MaybeReadCwndExperimentParameter(key_value_config)), - congestion_window_pushback_( - "cwnd_pushback", - MaybeReadCongestionWindowPushbackExperimentParameter( - key_value_config)), + : congestion_window_("QueueSize"), + congestion_window_pushback_("MinBitrate"), pacing_factor_("pacing_factor"), alr_probing_("alr_probing", false), trust_vp8_( @@ -124,8 +84,9 @@ RateControlSettings::RateControlSettings( probe_max_allocation_("probe_max_allocation", true), bitrate_adjuster_("bitrate_adjuster", false), vp8_s0_boost_("vp8_s0_boost", true) { - ParseFieldTrial({&congestion_window_, &congestion_window_pushback_, - &pacing_factor_, &alr_probing_, &trust_vp8_, &trust_vp9_, + ParseFieldTrial({&congestion_window_, &congestion_window_pushback_}, + key_value_config->Lookup("WebRTC-CongestionWindow")); + ParseFieldTrial({&pacing_factor_, &alr_probing_, &trust_vp8_, &trust_vp9_, &video_hysteresis_, &screenshare_hysteresis_, &probe_max_allocation_, &bitrate_adjuster_, &vp8_s0_boost_}, key_value_config->Lookup("WebRTC-VideoRateControl")); diff --git a/rtc_base/experiments/rate_control_settings_unittest.cc b/rtc_base/experiments/rate_control_settings_unittest.cc index 0d8c3767cd..0f23df0726 100644 --- a/rtc_base/experiments/rate_control_settings_unittest.cc +++ b/rtc_base/experiments/rate_control_settings_unittest.cc @@ -23,7 +23,8 @@ TEST(RateControlSettingsTest, CongestionWindow) { EXPECT_FALSE( RateControlSettings::ParseFromFieldTrials().UseCongestionWindow()); - test::ScopedFieldTrials field_trials("WebRTC-VideoRateControl/cwnd:100/"); + test::ScopedFieldTrials field_trials( + "WebRTC-CongestionWindow/QueueSize:100/"); const RateControlSettings settings_after = RateControlSettings::ParseFromFieldTrials(); EXPECT_TRUE(settings_after.UseCongestionWindow()); @@ -35,7 +36,7 @@ TEST(RateControlSettingsTest, CongestionWindowPushback) { .UseCongestionWindowPushback()); test::ScopedFieldTrials field_trials( - "WebRTC-VideoRateControl/cwnd:100,cwnd_pushback:100000/"); + "WebRTC-CongestionWindow/QueueSize:100,MinBitrate:100000/"); const RateControlSettings settings_after = RateControlSettings::ParseFromFieldTrials(); EXPECT_TRUE(settings_after.UseCongestionWindowPushback()); diff --git a/video/end_to_end_tests/transport_feedback_tests.cc b/video/end_to_end_tests/transport_feedback_tests.cc index b607bf645d..95ab6ced30 100644 --- a/video/end_to_end_tests/transport_feedback_tests.cc +++ b/video/end_to_end_tests/transport_feedback_tests.cc @@ -345,7 +345,7 @@ TEST_F(TransportFeedbackEndToEndTest, AudioVideoReceivesTransportFeedback) { TEST_F(TransportFeedbackEndToEndTest, StopsAndResumesMediaWhenCongestionWindowFull) { test::ScopedFieldTrials override_field_trials( - "WebRTC-CwndExperiment/Enabled-250/"); + "WebRTC-CongestionWindow/QueueSize:250/"); class TransportFeedbackTester : public test::EndToEndTest { public: