From 869e9fb4f36cc2759d28c02a1fdcfff7eaf650da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85sa=20Persson?= Date: Tue, 28 Jul 2020 12:49:49 +0200 Subject: [PATCH] Use field trial list in CpuSpeedExperiment. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the need for specifying a fixed number of parameters. Bug: none Change-Id: I1324861807cb4929963aedccb6c2755b9c6ea3fc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/180421 Reviewed-by: Rasmus Brandt Commit-Queue: Åsa Persson Cr-Commit-Position: refs/heads/master@{#32055} --- .../codecs/vp8/libvpx_vp8_encoder.cc | 6 +- .../codecs/vp8/libvpx_vp8_encoder.h | 3 +- rtc_base/experiments/BUILD.gn | 1 + rtc_base/experiments/cpu_speed_experiment.cc | 59 +++++++------ rtc_base/experiments/cpu_speed_experiment.h | 27 +++--- .../cpu_speed_experiment_unittest.cc | 87 +++++++++---------- 6 files changed, 94 insertions(+), 89 deletions(-) diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index 32e27108da..847499c0e4 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -291,7 +291,6 @@ vpx_enc_frame_flags_t LibvpxVp8Encoder::EncodeFlags( LibvpxVp8Encoder::LibvpxVp8Encoder(std::unique_ptr interface, VP8Encoder::Settings settings) : libvpx_(std::move(interface)), - experimental_cpu_speed_config_arm_(CpuSpeedExperiment::GetConfigs()), rate_control_settings_(RateControlSettings::ParseFromFieldTrials()), requested_resolution_alignment_override_( GetRequestedResolutionAlignmentOverride()), @@ -733,9 +732,8 @@ int LibvpxVp8Encoder::GetCpuSpeed(int width, int height) { if (number_of_cores_ <= 3) return -12; - if (experimental_cpu_speed_config_arm_) { - return CpuSpeedExperiment::GetValue(width * height, - *experimental_cpu_speed_config_arm_); + if (experimental_cpu_speed_config_arm_.GetValue(width * height).has_value()) { + return experimental_cpu_speed_config_arm_.GetValue(width * height).value(); } if (width * height <= 352 * 288) diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h index f6cfd0ffe6..731a9a08df 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h @@ -95,8 +95,7 @@ class LibvpxVp8Encoder : public VideoEncoder { const std::unique_ptr libvpx_; - const absl::optional> - experimental_cpu_speed_config_arm_; + const CpuSpeedExperiment experimental_cpu_speed_config_arm_; const RateControlSettings rate_control_settings_; // EncoderInfo::requested_resolution_alignment override from field trial. diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn index 282b5b9270..a40c9e0d80 100644 --- a/rtc_base/experiments/BUILD.gn +++ b/rtc_base/experiments/BUILD.gn @@ -123,6 +123,7 @@ rtc_library("cpu_speed_experiment") { "cpu_speed_experiment.h", ] deps = [ + ":field_trial_parser", "../:rtc_base_approved", "../../system_wrappers:field_trial", ] diff --git a/rtc_base/experiments/cpu_speed_experiment.cc b/rtc_base/experiments/cpu_speed_experiment.cc index 6d5650acc8..3d68b8acca 100644 --- a/rtc_base/experiments/cpu_speed_experiment.cc +++ b/rtc_base/experiments/cpu_speed_experiment.cc @@ -12,8 +12,8 @@ #include -#include - +#include "rtc_base/experiments/field_trial_list.h" +#include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/logging.h" #include "system_wrappers/include/field_trial.h" @@ -22,30 +22,18 @@ namespace { constexpr char kFieldTrial[] = "WebRTC-VP8-CpuSpeed-Arm"; constexpr int kMinSetting = -16; constexpr int kMaxSetting = -1; -} // namespace -absl::optional> -CpuSpeedExperiment::GetConfigs() { - if (!webrtc::field_trial::IsEnabled(kFieldTrial)) - return absl::nullopt; - - const std::string group = webrtc::field_trial::FindFullName(kFieldTrial); - if (group.empty()) - return absl::nullopt; - - std::vector configs(3); - if (sscanf(group.c_str(), "Enabled-%d,%d,%d,%d,%d,%d", &(configs[0].pixels), - &(configs[0].cpu_speed), &(configs[1].pixels), - &(configs[1].cpu_speed), &(configs[2].pixels), - &(configs[2].cpu_speed)) != 6) { - RTC_LOG(LS_WARNING) << "Too few parameters provided."; - return absl::nullopt; +std::vector GetValidOrEmpty( + const std::vector& configs) { + if (configs.empty()) { + RTC_LOG(LS_WARNING) << "Unsupported size, value ignored."; + return {}; } for (const auto& config : configs) { if (config.cpu_speed < kMinSetting || config.cpu_speed > kMaxSetting) { RTC_LOG(LS_WARNING) << "Unsupported cpu speed setting, value ignored."; - return absl::nullopt; + return {}; } } @@ -53,20 +41,37 @@ CpuSpeedExperiment::GetConfigs() { if (configs[i].pixels < configs[i - 1].pixels || configs[i].cpu_speed > configs[i - 1].cpu_speed) { RTC_LOG(LS_WARNING) << "Invalid parameter value provided."; - return absl::nullopt; + return {}; } } - return absl::optional>(configs); + return configs; +} +} // namespace + +CpuSpeedExperiment::CpuSpeedExperiment() { + FieldTrialStructList configs( + {FieldTrialStructMember("pixels", [](Config* c) { return &c->pixels; }), + FieldTrialStructMember("cpu_speed", + [](Config* c) { return &c->cpu_speed; })}, + {}); + + ParseFieldTrial({&configs}, field_trial::FindFullName(kFieldTrial)); + + configs_ = GetValidOrEmpty(configs.Get()); } -int CpuSpeedExperiment::GetValue(int pixels, - const std::vector& configs) { - for (const auto& config : configs) { +CpuSpeedExperiment::~CpuSpeedExperiment() {} + +absl::optional CpuSpeedExperiment::GetValue(int pixels) const { + if (configs_.empty()) + return absl::nullopt; + + for (const auto& config : configs_) { if (pixels <= config.pixels) - return config.cpu_speed; + return absl::optional(config.cpu_speed); } - return kMinSetting; + return absl::optional(kMinSetting); } } // namespace webrtc diff --git a/rtc_base/experiments/cpu_speed_experiment.h b/rtc_base/experiments/cpu_speed_experiment.h index e6c8340943..a788508903 100644 --- a/rtc_base/experiments/cpu_speed_experiment.h +++ b/rtc_base/experiments/cpu_speed_experiment.h @@ -19,21 +19,26 @@ namespace webrtc { class CpuSpeedExperiment { public: - struct Config { - bool operator==(const Config& o) const { - return pixels == o.pixels && cpu_speed == o.cpu_speed; - } + CpuSpeedExperiment(); + ~CpuSpeedExperiment(); - int pixels; // The video frame size. - int cpu_speed; // The |cpu_speed| to be used if the frame size is less - // than or equal to |pixels|. + // Example: + // WebRTC-VP8-CpuSpeed-Arm/pixels:100|200|300,cpu_speed:-1|-2|-3/ + // pixels <= 100 -> cpu speed: -1 + // pixels <= 200 -> cpu speed: -2 + // pixels <= 300 -> cpu speed: -3 + + struct Config { + int pixels = 0; // The video frame size. + int cpu_speed = 0; // The |cpu_speed| to be used if the frame size is less + // than or equal to |pixels|. }; - // Returns the configurations from field trial on success. - static absl::optional> GetConfigs(); + // Gets the cpu speed based on |pixels|. + absl::optional GetValue(int pixels) const; - // Gets the cpu speed from the |configs| based on |pixels|. - static int GetValue(int pixels, const std::vector& configs); + private: + std::vector configs_; }; } // namespace webrtc diff --git a/rtc_base/experiments/cpu_speed_experiment_unittest.cc b/rtc_base/experiments/cpu_speed_experiment_unittest.cc index edc782c0ad..2a73238305 100644 --- a/rtc_base/experiments/cpu_speed_experiment_unittest.cc +++ b/rtc_base/experiments/cpu_speed_experiment_unittest.cc @@ -16,70 +16,67 @@ namespace webrtc { -TEST(CpuSpeedExperimentTest, GetConfigsFailsIfNotEnabled) { - EXPECT_FALSE(CpuSpeedExperiment::GetConfigs()); -} - -TEST(CpuSpeedExperimentTest, GetConfigsFailsForTooFewParameters) { - webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-VP8-CpuSpeed-Arm/Enabled-1000,-1,2000,-10,3000/"); - EXPECT_FALSE(CpuSpeedExperiment::GetConfigs()); -} - -TEST(CpuSpeedExperimentTest, GetConfigs) { - webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-VP8-CpuSpeed-Arm/Enabled-1000,-1,2000,-10,3000,-16/"); - - const absl::optional> kConfigs = - CpuSpeedExperiment::GetConfigs(); - ASSERT_TRUE(kConfigs); - EXPECT_THAT(*kConfigs, - ::testing::ElementsAre(CpuSpeedExperiment::Config{1000, -1}, - CpuSpeedExperiment::Config{2000, -10}, - CpuSpeedExperiment::Config{3000, -16})); +TEST(CpuSpeedExperimentTest, NoValueIfNotEnabled) { + CpuSpeedExperiment cpu_speed_config; + EXPECT_FALSE(cpu_speed_config.GetValue(1)); } TEST(CpuSpeedExperimentTest, GetValue) { webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-VP8-CpuSpeed-Arm/Enabled-1000,-5,2000,-10,3000,-12/"); + "WebRTC-VP8-CpuSpeed-Arm/pixels:1000,cpu_speed:-12/"); - const absl::optional> kConfigs = - CpuSpeedExperiment::GetConfigs(); - ASSERT_TRUE(kConfigs); - ASSERT_EQ(3u, (*kConfigs).size()); - EXPECT_EQ(-5, CpuSpeedExperiment::GetValue(1, *kConfigs)); - EXPECT_EQ(-5, CpuSpeedExperiment::GetValue(1000, *kConfigs)); - EXPECT_EQ(-10, CpuSpeedExperiment::GetValue(1000 + 1, *kConfigs)); - EXPECT_EQ(-10, CpuSpeedExperiment::GetValue(2000, *kConfigs)); - EXPECT_EQ(-12, CpuSpeedExperiment::GetValue(2000 + 1, *kConfigs)); - EXPECT_EQ(-12, CpuSpeedExperiment::GetValue(3000, *kConfigs)); - EXPECT_EQ(-16, CpuSpeedExperiment::GetValue(3000 + 1, *kConfigs)); + CpuSpeedExperiment cpu_speed_config; + EXPECT_EQ(-12, cpu_speed_config.GetValue(1)); + EXPECT_EQ(-12, cpu_speed_config.GetValue(1000)); + EXPECT_EQ(-16, cpu_speed_config.GetValue(1001)); } -TEST(CpuSpeedExperimentTest, GetConfigsFailsForTooSmallValue) { +TEST(CpuSpeedExperimentTest, GetValueWithList) { + webrtc::test::ScopedFieldTrials field_trials( + "WebRTC-VP8-CpuSpeed-Arm/pixels:1000|2000|3000,cpu_speed:-1|-10|-16/"); + + CpuSpeedExperiment cpu_speed_config; + EXPECT_EQ(-1, cpu_speed_config.GetValue(1)); + EXPECT_EQ(-1, cpu_speed_config.GetValue(1000)); + EXPECT_EQ(-10, cpu_speed_config.GetValue(1001)); + EXPECT_EQ(-10, cpu_speed_config.GetValue(2000)); + EXPECT_EQ(-16, cpu_speed_config.GetValue(2001)); + EXPECT_EQ(-16, cpu_speed_config.GetValue(3000)); + EXPECT_EQ(-16, cpu_speed_config.GetValue(3001)); +} + +TEST(CpuSpeedExperimentTest, GetValueFailsForTooSmallValue) { // Supported range: [-16, -1]. webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-VP8-CpuSpeed-Arm/Enabled-1000,-1,2000,-10,3000,-17/"); - EXPECT_FALSE(CpuSpeedExperiment::GetConfigs()); + "WebRTC-VP8-CpuSpeed-Arm/pixels:1000|2000|3000,cpu_speed:-1|-10|-17/"); + + CpuSpeedExperiment cpu_speed_config; + EXPECT_FALSE(cpu_speed_config.GetValue(1)); } -TEST(CpuSpeedExperimentTest, GetConfigsFailsForTooLargeValue) { +TEST(CpuSpeedExperimentTest, GetValueFailsForTooLargeValue) { // Supported range: [-16, -1]. webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-VP8-CpuSpeed-Arm/Enabled-1000,0,2000,-10,3000,-16/"); - EXPECT_FALSE(CpuSpeedExperiment::GetConfigs()); + "WebRTC-VP8-CpuSpeed-Arm/pixels:1000|2000|3000,cpu_speed:0|-10|-16/"); + + CpuSpeedExperiment cpu_speed_config; + EXPECT_FALSE(cpu_speed_config.GetValue(1)); } -TEST(CpuSpeedExperimentTest, GetConfigsFailsIfPixelsDecreasing) { +TEST(CpuSpeedExperimentTest, GetValueFailsIfPixelsDecreases) { webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-VP8-CpuSpeed-Arm/Enabled-1000,-5,999,-10,3000,-16/"); - EXPECT_FALSE(CpuSpeedExperiment::GetConfigs()); + "WebRTC-VP8-CpuSpeed-Arm/pixels:1000|999|3000,cpu_speed:-5|-10|-16/"); + + CpuSpeedExperiment cpu_speed_config; + EXPECT_FALSE(cpu_speed_config.GetValue(1)); } -TEST(CpuSpeedExperimentTest, GetConfigsFailsIfCpuSpeedIncreasing) { +TEST(CpuSpeedExperimentTest, GetValueFailsIfCpuSpeedIncreases) { webrtc::test::ScopedFieldTrials field_trials( - "WebRTC-VP8-CpuSpeed-Arm/Enabled-1000,-5,2000,-4,3000,-16/"); - EXPECT_FALSE(CpuSpeedExperiment::GetConfigs()); + "WebRTC-VP8-CpuSpeed-Arm/pixels:1000|2000|3000,cpu_speed:-5|-4|-16/"); + + CpuSpeedExperiment cpu_speed_config; + EXPECT_FALSE(cpu_speed_config.GetValue(1)); } } // namespace webrtc