From c5d9e63c2bed77422c365c46086f215680c0aa73 Mon Sep 17 00:00:00 2001 From: stefan Date: Mon, 14 Aug 2017 05:54:58 -0700 Subject: [PATCH] Revert of Make the acceptable queue in the cwnd experiment configurable. (patchset #1 id:1 of https://codereview.webrtc.org/2998753002/ ) Reason for revert: Speculative revert to see if this caused regressions in android perf tests. Original issue's description: > Make the acceptable queue in the cwnd experiment configurable. > > BUG=webrtc:7926 > > Review-Url: https://codereview.webrtc.org/2998753002 > Cr-Commit-Position: refs/heads/master@{#19320} > Committed: https://chromium.googlesource.com/external/webrtc/+/7c83c56b6dc521d82285798c2d0e7eec205ed3f0 TBR=philipel@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:7926 Review-Url: https://codereview.webrtc.org/2999893002 Cr-Commit-Position: refs/heads/master@{#19337} --- .../include/send_side_congestion_controller.h | 3 +-- .../send_side_congestion_controller.cc | 26 ++----------------- webrtc/video/end_to_end_tests.cc | 2 +- 3 files changed, 4 insertions(+), 27 deletions(-) diff --git a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h index 1d576aaf00..68213507a6 100644 --- a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h @@ -160,8 +160,7 @@ class SendSideCongestionController : public CallStatsObserver, rtc::CriticalSection bwe_lock_; int min_bitrate_bps_ GUARDED_BY(bwe_lock_); std::unique_ptr delay_based_bwe_ GUARDED_BY(bwe_lock_); - bool in_cwnd_experiment_; - int64_t accepted_queue_ms_; + const bool in_cwnd_experiment_; bool was_in_alr_; rtc::RaceChecker worker_race_; diff --git a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc index 77203d6a8b..751b541517 100644 --- a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc +++ b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc @@ -20,7 +20,6 @@ #include "webrtc/modules/pacing/alr_detector.h" #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h" #include "webrtc/rtc_base/checks.h" -#include "webrtc/rtc_base/format_macros.h" #include "webrtc/rtc_base/logging.h" #include "webrtc/rtc_base/ptr_util.h" #include "webrtc/rtc_base/rate_limiter.h" @@ -32,7 +31,6 @@ namespace webrtc { namespace { const char kCwndExperiment[] = "WebRTC-CwndExperiment"; -const int64_t kDefaultAcceptedQueueMs = 250; bool CwndExperimentEnabled() { std::string experiment_string = @@ -41,20 +39,6 @@ bool CwndExperimentEnabled() { return experiment_string.find("Enabled") == 0; } -bool ReadCwndExperimentParameter(int64_t* accepted_queue_ms) { - RTC_DCHECK(accepted_queue_ms); - std::string experiment_string = - webrtc::field_trial::FindFullName(kCwndExperiment); - 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 true; - } - return false; -} - static const int64_t kRetransmitWindowSizeMs = 500; // Makes sure that the bitrate and the min, max values are in valid range. @@ -131,15 +115,8 @@ SendSideCongestionController::SendSideCongestionController( min_bitrate_bps_(congestion_controller::GetMinBitrateBps()), delay_based_bwe_(new DelayBasedBwe(event_log_, clock_)), in_cwnd_experiment_(CwndExperimentEnabled()), - accepted_queue_ms_(kDefaultAcceptedQueueMs), was_in_alr_(0) { delay_based_bwe_->SetMinBitrate(min_bitrate_bps_); - if (in_cwnd_experiment_ && - !ReadCwndExperimentParameter(&accepted_queue_ms_)) { - LOG(LS_WARNING) << "Failed to parse parameters for CwndExperiment " - "from field trial string. Experiment disabled."; - in_cwnd_experiment_ = false; - } } SendSideCongestionController::~SendSideCongestionController() {} @@ -367,9 +344,10 @@ void SendSideCongestionController::LimitOutstandingBytes( // we don't try to limit the outstanding packets. if (!min_rtt_ms) return; + const int64_t kAcceptedQueueMs = 250; const size_t kMinCwndBytes = 2 * 1500; size_t max_outstanding_bytes = - std::max((*min_rtt_ms + accepted_queue_ms_) * + std::max((*min_rtt_ms + kAcceptedQueueMs) * last_reported_bitrate_bps_ / 1000 / 8, kMinCwndBytes); LOG(LS_INFO) << clock_->TimeInMilliseconds() diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 01d34a76a4..15db046b80 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -1930,7 +1930,7 @@ TEST_F(EndToEndTest, AudioVideoReceivesTransportFeedback) { TEST_F(EndToEndTest, StopsSendingMediaWithoutFeedback) { test::ScopedFieldTrials override_field_trials( - "WebRTC-CwndExperiment/Enabled-250/"); + "WebRTC-CwndExperiment/Enabled/"); class TransportFeedbackTester : public test::EndToEndTest { public: