Remove option to parse RateControlSettings from the global field trial string
Bug: webrtc:42220378
Change-Id: Iff016f0f53f427ff59df816d8d87dc4a8119db65
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350921
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42348}
diff --git a/modules/video_coding/utility/simulcast_rate_allocator.cc b/modules/video_coding/utility/simulcast_rate_allocator.cc
index 3ef4d43..89522e6 100644
--- a/modules/video_coding/utility/simulcast_rate_allocator.cc
+++ b/modules/video_coding/utility/simulcast_rate_allocator.cc
@@ -58,12 +58,6 @@
return kLayerRateAllocation[num_layers - 1][temporal_id];
}
-SimulcastRateAllocator::SimulcastRateAllocator(const VideoCodec& codec)
- : codec_(codec),
- stable_rate_settings_(StableTargetRateExperiment::ParseFromFieldTrials()),
- rate_control_settings_(RateControlSettings::ParseFromFieldTrials()),
- legacy_conference_mode_(false) {}
-
SimulcastRateAllocator::SimulcastRateAllocator(const Environment& env,
const VideoCodec& codec)
: codec_(codec),
diff --git a/modules/video_coding/utility/simulcast_rate_allocator.h b/modules/video_coding/utility/simulcast_rate_allocator.h
index 60b2018..0b10011 100644
--- a/modules/video_coding/utility/simulcast_rate_allocator.h
+++ b/modules/video_coding/utility/simulcast_rate_allocator.h
@@ -27,7 +27,6 @@
class SimulcastRateAllocator : public VideoBitrateAllocator {
public:
- [[deprecated]] explicit SimulcastRateAllocator(const VideoCodec& codec);
SimulcastRateAllocator(const Environment& env, const VideoCodec& codec);
~SimulcastRateAllocator() override;
diff --git a/rtc_base/experiments/BUILD.gn b/rtc_base/experiments/BUILD.gn
index 98d5062..dfce9ee 100644
--- a/rtc_base/experiments/BUILD.gn
+++ b/rtc_base/experiments/BUILD.gn
@@ -134,10 +134,8 @@
"..:logging",
"..:safe_conversions",
"../../api:field_trials_view",
- "../../api/transport:field_trial_based_config",
"../../api/units:data_size",
"../../api/video_codecs:video_codecs_api",
- "../../system_wrappers:field_trial",
"../../video/config:encoder_config",
]
absl_deps = [
diff --git a/rtc_base/experiments/rate_control_settings.cc b/rtc_base/experiments/rate_control_settings.cc
index b81432d8..597bc73 100644
--- a/rtc_base/experiments/rate_control_settings.cc
+++ b/rtc_base/experiments/rate_control_settings.cc
@@ -16,7 +16,6 @@
#include <string>
#include "absl/strings/match.h"
-#include "api/transport/field_trial_based_config.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
@@ -87,10 +86,6 @@
RateControlSettings::~RateControlSettings() = default;
RateControlSettings::RateControlSettings(RateControlSettings&&) = default;
-RateControlSettings RateControlSettings::ParseFromFieldTrials() {
- return RateControlSettings(FieldTrialBasedConfig());
-}
-
bool RateControlSettings::UseCongestionWindow() const {
return static_cast<bool>(congestion_window_config_.queue_size_ms);
}
diff --git a/rtc_base/experiments/rate_control_settings.h b/rtc_base/experiments/rate_control_settings.h
index ab9e1ed..0dd4a14 100644
--- a/rtc_base/experiments/rate_control_settings.h
+++ b/rtc_base/experiments/rate_control_settings.h
@@ -52,8 +52,6 @@
RateControlSettings(RateControlSettings&&);
~RateControlSettings();
- static RateControlSettings ParseFromFieldTrials();
-
// When CongestionWindowPushback is enabled, the pacer is oblivious to
// the congestion window. The relation between outstanding data and
// the congestion window affects encoder allocations directly.
diff --git a/rtc_base/experiments/rate_control_settings_unittest.cc b/rtc_base/experiments/rate_control_settings_unittest.cc
index 91ebf53..72470cb 100644
--- a/rtc_base/experiments/rate_control_settings_unittest.cc
+++ b/rtc_base/experiments/rate_control_settings_unittest.cc
@@ -11,7 +11,8 @@
#include "rtc_base/experiments/rate_control_settings.h"
#include "api/video_codecs/video_codec.h"
-#include "test/field_trial.h"
+#include "test/explicit_key_value_config.h"
+#include "test/gmock.h"
#include "test/gtest.h"
#include "video/config/video_encoder_config.h"
@@ -19,171 +20,130 @@
namespace {
-TEST(RateControlSettingsTest, CongestionWindow) {
- EXPECT_TRUE(
- RateControlSettings::ParseFromFieldTrials().UseCongestionWindow());
+using test::ExplicitKeyValueConfig;
+using ::testing::DoubleEq;
+using ::testing::Optional;
- test::ScopedFieldTrials field_trials(
- "WebRTC-CongestionWindow/QueueSize:100/");
- const RateControlSettings settings_after =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_TRUE(settings_after.UseCongestionWindow());
- EXPECT_EQ(settings_after.GetCongestionWindowAdditionalTimeMs(), 100);
+RateControlSettings ParseFrom(absl::string_view field_trials) {
+ return RateControlSettings(ExplicitKeyValueConfig(field_trials));
+}
+
+TEST(RateControlSettingsTest, CongestionWindow) {
+ EXPECT_TRUE(ParseFrom("").UseCongestionWindow());
+
+ const RateControlSettings settings =
+ ParseFrom("WebRTC-CongestionWindow/QueueSize:100/");
+ EXPECT_TRUE(settings.UseCongestionWindow());
+ EXPECT_EQ(settings.GetCongestionWindowAdditionalTimeMs(), 100);
}
TEST(RateControlSettingsTest, CongestionWindowPushback) {
- EXPECT_TRUE(RateControlSettings::ParseFromFieldTrials()
- .UseCongestionWindowPushback());
+ EXPECT_TRUE(ParseFrom("").UseCongestionWindowPushback());
- test::ScopedFieldTrials field_trials(
- "WebRTC-CongestionWindow/QueueSize:100,MinBitrate:100000/");
- const RateControlSettings settings_after =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_TRUE(settings_after.UseCongestionWindowPushback());
- EXPECT_EQ(settings_after.CongestionWindowMinPushbackTargetBitrateBps(),
- 100000u);
+ const RateControlSettings settings =
+ ParseFrom("WebRTC-CongestionWindow/QueueSize:100,MinBitrate:100000/");
+ EXPECT_TRUE(settings.UseCongestionWindowPushback());
+ EXPECT_EQ(settings.CongestionWindowMinPushbackTargetBitrateBps(), 100000u);
}
TEST(RateControlSettingsTest, CongestionWindowPushbackDropframe) {
- EXPECT_TRUE(RateControlSettings::ParseFromFieldTrials()
- .UseCongestionWindowPushback());
+ EXPECT_TRUE(ParseFrom("").UseCongestionWindowPushback());
- test::ScopedFieldTrials field_trials(
+ const RateControlSettings settings = ParseFrom(
"WebRTC-CongestionWindow/"
"QueueSize:100,MinBitrate:100000,DropFrame:true/");
- const RateControlSettings settings_after =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_TRUE(settings_after.UseCongestionWindowPushback());
- EXPECT_EQ(settings_after.CongestionWindowMinPushbackTargetBitrateBps(),
- 100000u);
- EXPECT_TRUE(settings_after.UseCongestionWindowDropFrameOnly());
+ EXPECT_TRUE(settings.UseCongestionWindowPushback());
+ EXPECT_EQ(settings.CongestionWindowMinPushbackTargetBitrateBps(), 100000u);
+ EXPECT_TRUE(settings.UseCongestionWindowDropFrameOnly());
}
TEST(RateControlSettingsTest, CongestionWindowPushbackDefaultConfig) {
- const RateControlSettings settings =
- RateControlSettings::ParseFromFieldTrials();
+ const RateControlSettings settings = ParseFrom("");
EXPECT_TRUE(settings.UseCongestionWindowPushback());
EXPECT_EQ(settings.CongestionWindowMinPushbackTargetBitrateBps(), 30000u);
EXPECT_TRUE(settings.UseCongestionWindowDropFrameOnly());
}
TEST(RateControlSettingsTest, PacingFactor) {
- EXPECT_FALSE(RateControlSettings::ParseFromFieldTrials().GetPacingFactor());
+ EXPECT_FALSE(ParseFrom("").GetPacingFactor());
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/pacing_factor:1.2/");
- const RateControlSettings settings_after =
- RateControlSettings::ParseFromFieldTrials();
- // Need to explicitly dereference the absl::optional
- // for the EXPECT_DOUBLE_EQ to compile.
- ASSERT_TRUE(settings_after.GetPacingFactor());
- EXPECT_DOUBLE_EQ(*settings_after.GetPacingFactor(), 1.2);
+ EXPECT_THAT(
+ ParseFrom("WebRTC-VideoRateControl/pacing_factor:1.2/").GetPacingFactor(),
+ Optional(DoubleEq(1.2)));
}
TEST(RateControlSettingsTest, AlrProbing) {
- EXPECT_FALSE(RateControlSettings::ParseFromFieldTrials().UseAlrProbing());
+ EXPECT_FALSE(ParseFrom("").UseAlrProbing());
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/alr_probing:1/");
- EXPECT_TRUE(RateControlSettings::ParseFromFieldTrials().UseAlrProbing());
+ EXPECT_TRUE(
+ ParseFrom("WebRTC-VideoRateControl/alr_probing:1/").UseAlrProbing());
}
TEST(RateControlSettingsTest, LibvpxVp8QpMax) {
- EXPECT_FALSE(RateControlSettings::ParseFromFieldTrials().LibvpxVp8QpMax());
+ EXPECT_FALSE(ParseFrom("").LibvpxVp8QpMax());
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/vp8_qp_max:50/");
- EXPECT_EQ(RateControlSettings::ParseFromFieldTrials().LibvpxVp8QpMax(), 50);
+ EXPECT_EQ(
+ ParseFrom("WebRTC-VideoRateControl/vp8_qp_max:50/").LibvpxVp8QpMax(), 50);
}
TEST(RateControlSettingsTest, DoesNotGetTooLargeLibvpxVp8QpMaxValue) {
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/vp8_qp_max:70/");
- EXPECT_FALSE(RateControlSettings::ParseFromFieldTrials().LibvpxVp8QpMax());
+ EXPECT_FALSE(
+ ParseFrom("WebRTC-VideoRateControl/vp8_qp_max:70/").LibvpxVp8QpMax());
}
TEST(RateControlSettingsTest, LibvpxVp8MinPixels) {
- EXPECT_FALSE(
- RateControlSettings::ParseFromFieldTrials().LibvpxVp8MinPixels());
+ EXPECT_FALSE(ParseFrom("").LibvpxVp8MinPixels());
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/vp8_min_pixels:50000/");
- EXPECT_EQ(RateControlSettings::ParseFromFieldTrials().LibvpxVp8MinPixels(),
+ EXPECT_EQ(ParseFrom("WebRTC-VideoRateControl/vp8_min_pixels:50000/")
+ .LibvpxVp8MinPixels(),
50000);
}
TEST(RateControlSettingsTest, DoesNotGetTooSmallLibvpxVp8MinPixelValue) {
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/vp8_min_pixels:0/");
- EXPECT_FALSE(
- RateControlSettings::ParseFromFieldTrials().LibvpxVp8MinPixels());
+ EXPECT_FALSE(ParseFrom("WebRTC-VideoRateControl/vp8_min_pixels:0/")
+ .LibvpxVp8MinPixels());
}
TEST(RateControlSettingsTest, LibvpxTrustedRateController) {
- const RateControlSettings settings_before =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_TRUE(settings_before.LibvpxVp8TrustedRateController());
- EXPECT_TRUE(settings_before.LibvpxVp9TrustedRateController());
+ const RateControlSettings default_settings = ParseFrom("");
+ EXPECT_TRUE(default_settings.LibvpxVp8TrustedRateController());
+ EXPECT_TRUE(default_settings.LibvpxVp9TrustedRateController());
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/trust_vp8:0,trust_vp9:0/");
- const RateControlSettings settings_after =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_FALSE(settings_after.LibvpxVp8TrustedRateController());
- EXPECT_FALSE(settings_after.LibvpxVp9TrustedRateController());
+ const RateControlSettings settings =
+ ParseFrom("WebRTC-VideoRateControl/trust_vp8:0,trust_vp9:0/");
+ EXPECT_FALSE(settings.LibvpxVp8TrustedRateController());
+ EXPECT_FALSE(settings.LibvpxVp9TrustedRateController());
}
TEST(RateControlSettingsTest, Vp8BaseHeavyTl3RateAllocationLegacyKey) {
- const RateControlSettings settings_before =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_FALSE(settings_before.Vp8BaseHeavyTl3RateAllocation());
+ EXPECT_FALSE(ParseFrom("").Vp8BaseHeavyTl3RateAllocation());
- test::ScopedFieldTrials field_trials(
- "WebRTC-UseBaseHeavyVP8TL3RateAllocation/Enabled/");
- const RateControlSettings settings_after =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_TRUE(settings_after.Vp8BaseHeavyTl3RateAllocation());
+ EXPECT_TRUE(ParseFrom("WebRTC-UseBaseHeavyVP8TL3RateAllocation/Enabled/")
+ .Vp8BaseHeavyTl3RateAllocation());
}
TEST(RateControlSettingsTest,
Vp8BaseHeavyTl3RateAllocationVideoRateControlKey) {
- const RateControlSettings settings_before =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_FALSE(settings_before.Vp8BaseHeavyTl3RateAllocation());
+ EXPECT_FALSE(ParseFrom("").Vp8BaseHeavyTl3RateAllocation());
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/vp8_base_heavy_tl3_alloc:1/");
- const RateControlSettings settings_after =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_TRUE(settings_after.Vp8BaseHeavyTl3RateAllocation());
+ EXPECT_TRUE(ParseFrom("WebRTC-VideoRateControl/vp8_base_heavy_tl3_alloc:1/")
+ .Vp8BaseHeavyTl3RateAllocation());
}
TEST(RateControlSettingsTest,
Vp8BaseHeavyTl3RateAllocationVideoRateControlKeyOverridesLegacyKey) {
- const RateControlSettings settings_before =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_FALSE(settings_before.Vp8BaseHeavyTl3RateAllocation());
+ EXPECT_FALSE(ParseFrom("").Vp8BaseHeavyTl3RateAllocation());
- test::ScopedFieldTrials field_trials(
- "WebRTC-UseBaseHeavyVP8TL3RateAllocation/Enabled/WebRTC-VideoRateControl/"
- "vp8_base_heavy_tl3_alloc:0/");
- const RateControlSettings settings_after =
- RateControlSettings::ParseFromFieldTrials();
- EXPECT_FALSE(settings_after.Vp8BaseHeavyTl3RateAllocation());
+ EXPECT_FALSE(ParseFrom("WebRTC-UseBaseHeavyVP8TL3RateAllocation/Enabled/"
+ "WebRTC-VideoRateControl/vp8_base_heavy_tl3_alloc:0/")
+ .Vp8BaseHeavyTl3RateAllocation());
}
TEST(RateControlSettingsTest, UseEncoderBitrateAdjuster) {
- // Should be on by default.
- EXPECT_TRUE(
- RateControlSettings::ParseFromFieldTrials().UseEncoderBitrateAdjuster());
+ EXPECT_TRUE(ParseFrom("").UseEncoderBitrateAdjuster());
- {
- // Can be turned off via field trial.
- test::ScopedFieldTrials field_trials(
- "WebRTC-VideoRateControl/bitrate_adjuster:false/");
- EXPECT_FALSE(RateControlSettings::ParseFromFieldTrials()
- .UseEncoderBitrateAdjuster());
- }
+ EXPECT_FALSE(ParseFrom("WebRTC-VideoRateControl/bitrate_adjuster:false/")
+ .UseEncoderBitrateAdjuster());
}
} // namespace
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 38fe5d4..10ad57a 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -7300,8 +7300,7 @@
// of video, verify number of drops. Rate needs to be slightly changed in
// order to force the rate to be reconfigured.
double overshoot_factor = 2.0;
- const RateControlSettings trials =
- RateControlSettings::ParseFromFieldTrials();
+ const RateControlSettings trials(env_.field_trials());
if (trials.UseEncoderBitrateAdjuster()) {
// With bitrate adjuster, when need to overshoot even more to trigger
// frame dropping since the adjuter will try to just lower the target
