From d6604df27fd5f09f00b08e444d8a57a4bf14803c Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Mon, 1 Feb 2021 12:01:06 +0000 Subject: [PATCH] Revert "Enable Video-QualityScaling experiment by default" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 066b5b6ed7069d78e17b8ad6fb8c82546b31acea. Reason for revert: Regressions on iOS testbots. Original change's description: > Enable Video-QualityScaling experiment by default > > Bug: webrtc:12401 > Change-Id: Iebf3130e785892bb9fddf1012bc46027a21085a4 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/204000 > Commit-Queue: Ilya Nikolaevskiy > Reviewed-by: Åsa Persson > Cr-Commit-Position: refs/heads/master@{#33091} TBR=ilnik@webrtc.org,asapersson@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:12401 Change-Id: I489b805c7741b63c22c16cfce03347179a3e2602 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/205001 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#33123} --- .../video_coding/utility/quality_scaler_unittest.cc | 3 ++- rtc_base/experiments/quality_scaling_experiment.cc | 10 +++------- .../experiments/quality_scaling_experiment_unittest.cc | 9 ++++----- video/video_stream_encoder_unittest.cc | 2 -- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/modules/video_coding/utility/quality_scaler_unittest.cc b/modules/video_coding/utility/quality_scaler_unittest.cc index d92cffc5bf..d5b22a8a29 100644 --- a/modules/video_coding/utility/quality_scaler_unittest.cc +++ b/modules/video_coding/utility/quality_scaler_unittest.cc @@ -115,7 +115,8 @@ INSTANTIATE_TEST_SUITE_P( FieldTrials, QualityScalerTest, ::testing::Values( - "WebRTC-Video-QualityScaling/Enabled-1,2,3,4,5,6,7,8,0.9,0.99,1/")); + "WebRTC-Video-QualityScaling/Enabled-1,2,3,4,5,6,7,8,0.9,0.99,1/", + "")); TEST_P(QualityScalerTest, DownscalesAfterContinuousFramedrop) { task_queue_.SendTask([this] { TriggerScale(kScaleDown); }, RTC_FROM_HERE); diff --git a/rtc_base/experiments/quality_scaling_experiment.cc b/rtc_base/experiments/quality_scaling_experiment.cc index 64347fe9f5..ca58ba858a 100644 --- a/rtc_base/experiments/quality_scaling_experiment.cc +++ b/rtc_base/experiments/quality_scaling_experiment.cc @@ -19,8 +19,6 @@ namespace webrtc { namespace { constexpr char kFieldTrial[] = "WebRTC-Video-QualityScaling"; -constexpr char kDefaultQualityScalingSetttings[] = - "Enabled-29,95,149,205,24,37,26,36,0.9995,0.9999,1"; constexpr int kMinQp = 1; constexpr int kMaxVp8Qp = 127; constexpr int kMaxVp9Qp = 255; @@ -40,16 +38,14 @@ absl::optional GetThresholds(int low, } // namespace bool QualityScalingExperiment::Enabled() { - return !webrtc::field_trial::IsDisabled(kFieldTrial); + return webrtc::field_trial::IsEnabled(kFieldTrial); } absl::optional QualityScalingExperiment::ParseSettings() { - std::string group = webrtc::field_trial::FindFullName(kFieldTrial); - // TODO(http:crbug.org/webrtc/12401): Completely remove the experiment code - // after few releases. + const std::string group = webrtc::field_trial::FindFullName(kFieldTrial); if (group.empty()) - group = kDefaultQualityScalingSetttings; + return absl::nullopt; Settings s; if (sscanf(group.c_str(), "Enabled-%d,%d,%d,%d,%d,%d,%d,%d,%f,%f,%d", diff --git a/rtc_base/experiments/quality_scaling_experiment_unittest.cc b/rtc_base/experiments/quality_scaling_experiment_unittest.cc index 2ecdd7c49e..7a345b629f 100644 --- a/rtc_base/experiments/quality_scaling_experiment_unittest.cc +++ b/rtc_base/experiments/quality_scaling_experiment_unittest.cc @@ -38,9 +38,9 @@ void ExpectEqualConfig(QualityScalingExperiment::Config a, } } // namespace -TEST(QualityScalingExperimentTest, DefaultEnabledWithoutFieldTrial) { +TEST(QualityScalingExperimentTest, DisabledWithoutFieldTrial) { webrtc::test::ScopedFieldTrials field_trials(""); - EXPECT_TRUE(QualityScalingExperiment::Enabled()); + EXPECT_FALSE(QualityScalingExperiment::Enabled()); } TEST(QualityScalingExperimentTest, EnabledWithFieldTrial) { @@ -59,10 +59,9 @@ TEST(QualityScalingExperimentTest, ParseSettings) { ExpectEqualSettings(kExpected, *settings); } -TEST(QualityScalingExperimentTest, ParseSettingsUsesDefaultsWithoutFieldTrial) { +TEST(QualityScalingExperimentTest, ParseSettingsFailsWithoutFieldTrial) { webrtc::test::ScopedFieldTrials field_trials(""); - // Uses some default hard coded values. - EXPECT_TRUE(QualityScalingExperiment::ParseSettings()); + EXPECT_FALSE(QualityScalingExperiment::ParseSettings()); } TEST(QualityScalingExperimentTest, ParseSettingsFailsWithInvalidFieldTrial) { diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index b0cc6c021e..38e5111b8c 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -5634,8 +5634,6 @@ TEST_F(VideoStreamEncoderTest, RampsUpInQualityWhenBwIsHigh) { TEST_F(VideoStreamEncoderTest, QualityScalerAdaptationsRemovedWhenQualityScalingDisabled) { - webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-Video-QualityScaling/Disabled/"); AdaptingFrameForwarder source(&time_controller_); source.set_adaptation_enabled(true); video_stream_encoder_->SetSource(&source,