Cleanup usage of FieldTrialsView by RateControlSettings and CongestionWindowPushbackController

Replace factory that takes optional FieldTrialView with a constructor that takes non-optional reference to the same interface - all callers already guarantee it is not nullptr
Replace several local IsEnabled/IsDisabled helpers with the same helpers in FieldTrialView
In CongestionWindowPushbackController tests pass field trials bypassing global field trial string

Bug: webrtc:42220378
Change-Id: Ic49ad78919d834a5e3b9b69545d3b39088023a75
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349900
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42270}

media/engine/simulcast_encoder_adapter.cc

@@ -247,8 +247,7 @@ SimulcastEncoderAdapter::SimulcastEncoderAdapter(
       bypass_mode_(false),
       encoded_complete_callback_(nullptr),
       boost_base_layer_quality_(
-          RateControlSettings::ParseFromKeyValueConfig(&env_.field_trials())
+          RateControlSettings(env_.field_trials()).Vp8BoostBaseLayerQuality()),
-              .Vp8BoostBaseLayerQuality()),
       prefer_temporal_support_on_base_layer_(env_.field_trials().IsEnabled(
           "WebRTC-Video-PreferTemporalSupportOnBaseLayer")),
       per_layer_pli_(SupportsPerLayerPictureLossIndication(format.parameters)),

modules/congestion_controller/goog_cc/congestion_window_pushback_controller.cc

@@ -13,7 +13,6 @@
 #include <algorithm>
 #include <cstdint>
 
-#include "absl/strings/match.h"
 #include "api/field_trials_view.h"
 #include "api/units/data_size.h"
 #include "rtc_base/experiments/rate_control_settings.h"
@@ -21,17 +20,14 @@
 namespace webrtc {
 
 CongestionWindowPushbackController::CongestionWindowPushbackController(
-    const FieldTrialsView* key_value_config)
+    const FieldTrialsView& key_value_config)
-    : add_pacing_(
+    : add_pacing_(key_value_config.IsEnabled(
-          absl::StartsWith(key_value_config->Lookup(
+          "WebRTC-AddPacingToCongestionWindowPushback")),
-                               "WebRTC-AddPacingToCongestionWindowPushback"),
-                           "Enabled")),
       min_pushback_target_bitrate_bps_(
-          RateControlSettings::ParseFromKeyValueConfig(key_value_config)
+          RateControlSettings(key_value_config)
               .CongestionWindowMinPushbackTargetBitrateBps()),
-      current_data_window_(
+      current_data_window_(RateControlSettings(key_value_config)
-          RateControlSettings::ParseFromKeyValueConfig(key_value_config)
+                               .CongestionWindowInitialDataWindow()) {}
-              .CongestionWindowInitialDataWindow()) {}
 
 void CongestionWindowPushbackController::UpdateOutstandingData(
     int64_t outstanding_bytes) {

modules/congestion_controller/goog_cc/congestion_window_pushback_controller.h

@@ -27,7 +27,7 @@ namespace webrtc {
 class CongestionWindowPushbackController {
  public:
   explicit CongestionWindowPushbackController(
-      const FieldTrialsView* key_value_config);
+      const FieldTrialsView& key_value_config);
   void UpdateOutstandingData(int64_t outstanding_bytes);
   void UpdatePacingQueue(int64_t pacing_bytes);
   uint32_t UpdateTargetBitrate(uint32_t bitrate_bps);

modules/congestion_controller/goog_cc/congestion_window_pushback_controller_unittest.cc

@@ -13,93 +13,91 @@
 #include <cstdint>
 #include <memory>
 
-#include "api/transport/field_trial_based_config.h"
 #include "api/units/data_size.h"
-#include "test/field_trial.h"
+#include "test/explicit_key_value_config.h"
 #include "test/gmock.h"
 #include "test/gtest.h"
 
-using ::testing::_;
-
 namespace webrtc {
 namespace test {
 
-class CongestionWindowPushbackControllerTest : public ::testing::Test {
+using ::testing::_;
- public:
-  CongestionWindowPushbackControllerTest() {
-    cwnd_controller_.reset(
-        new CongestionWindowPushbackController(&field_trial_config_));
-  }
 
- protected:
+TEST(CongestionWindowPushbackControllerTest, FullCongestionWindow) {
-  FieldTrialBasedConfig field_trial_config_;
+  CongestionWindowPushbackController cwnd_controller(
+      ExplicitKeyValueConfig(""));
 
-  std::unique_ptr<CongestionWindowPushbackController> cwnd_controller_;
+  cwnd_controller.UpdateOutstandingData(100000);
-};
+  cwnd_controller.SetDataWindow(DataSize::Bytes(50000));
-
-TEST_F(CongestionWindowPushbackControllerTest, FullCongestionWindow) {
-  cwnd_controller_->UpdateOutstandingData(100000);
-  cwnd_controller_->SetDataWindow(DataSize::Bytes(50000));
 
   uint32_t bitrate_bps = 80000;
-  bitrate_bps = cwnd_controller_->UpdateTargetBitrate(bitrate_bps);
+  bitrate_bps = cwnd_controller.UpdateTargetBitrate(bitrate_bps);
   EXPECT_EQ(72000u, bitrate_bps);
 
-  cwnd_controller_->SetDataWindow(DataSize::Bytes(50000));
+  cwnd_controller.SetDataWindow(DataSize::Bytes(50000));
-  bitrate_bps = cwnd_controller_->UpdateTargetBitrate(bitrate_bps);
+  bitrate_bps = cwnd_controller.UpdateTargetBitrate(bitrate_bps);
   EXPECT_EQ(static_cast<uint32_t>(72000 * 0.9 * 0.9), bitrate_bps);
 }
 
-TEST_F(CongestionWindowPushbackControllerTest, NormalCongestionWindow) {
+TEST(CongestionWindowPushbackControllerTest, NormalCongestionWindow) {
-  cwnd_controller_->UpdateOutstandingData(199999);
+  CongestionWindowPushbackController cwnd_controller(
-  cwnd_controller_->SetDataWindow(DataSize::Bytes(200000));
+      ExplicitKeyValueConfig(""));
+
+  cwnd_controller.UpdateOutstandingData(199999);
+  cwnd_controller.SetDataWindow(DataSize::Bytes(200000));
 
   uint32_t bitrate_bps = 80000;
-  bitrate_bps = cwnd_controller_->UpdateTargetBitrate(bitrate_bps);
+  bitrate_bps = cwnd_controller.UpdateTargetBitrate(bitrate_bps);
   EXPECT_EQ(80000u, bitrate_bps);
 }
 
-TEST_F(CongestionWindowPushbackControllerTest, LowBitrate) {
+TEST(CongestionWindowPushbackControllerTest, LowBitrate) {
-  cwnd_controller_->UpdateOutstandingData(100000);
+  CongestionWindowPushbackController cwnd_controller(
-  cwnd_controller_->SetDataWindow(DataSize::Bytes(50000));
+      ExplicitKeyValueConfig(""));
+
+  cwnd_controller.UpdateOutstandingData(100000);
+  cwnd_controller.SetDataWindow(DataSize::Bytes(50000));
 
   uint32_t bitrate_bps = 35000;
-  bitrate_bps = cwnd_controller_->UpdateTargetBitrate(bitrate_bps);
+  bitrate_bps = cwnd_controller.UpdateTargetBitrate(bitrate_bps);
   EXPECT_EQ(static_cast<uint32_t>(35000 * 0.9), bitrate_bps);
 
-  cwnd_controller_->SetDataWindow(DataSize::Bytes(20000));
+  cwnd_controller.SetDataWindow(DataSize::Bytes(20000));
-  bitrate_bps = cwnd_controller_->UpdateTargetBitrate(bitrate_bps);
+  bitrate_bps = cwnd_controller.UpdateTargetBitrate(bitrate_bps);
   EXPECT_EQ(30000u, bitrate_bps);
 }
 
-TEST_F(CongestionWindowPushbackControllerTest, NoPushbackOnDataWindowUnset) {
+TEST(CongestionWindowPushbackControllerTest, NoPushbackOnDataWindowUnset) {
-  cwnd_controller_->UpdateOutstandingData(1e8);  // Large number
+  CongestionWindowPushbackController cwnd_controller(
+      ExplicitKeyValueConfig(""));
+
+  cwnd_controller.UpdateOutstandingData(1e8);  // Large number
 
   uint32_t bitrate_bps = 80000;
-  bitrate_bps = cwnd_controller_->UpdateTargetBitrate(bitrate_bps);
+  bitrate_bps = cwnd_controller.UpdateTargetBitrate(bitrate_bps);
   EXPECT_EQ(80000u, bitrate_bps);
 }
 
-TEST_F(CongestionWindowPushbackControllerTest, PushbackOnInititialDataWindow) {
+TEST(CongestionWindowPushbackControllerTest, PushbackOnInititialDataWindow) {
-  test::ScopedFieldTrials trials("WebRTC-CongestionWindow/InitWin:100000/");
+  CongestionWindowPushbackController cwnd_controller(
-  cwnd_controller_.reset(
+      ExplicitKeyValueConfig("WebRTC-CongestionWindow/InitWin:100000/"));
-      new CongestionWindowPushbackController(&field_trial_config_));
+
-  cwnd_controller_->UpdateOutstandingData(1e8);  // Large number
+  cwnd_controller.UpdateOutstandingData(1e8);  // Large number
 
   uint32_t bitrate_bps = 80000;
-  bitrate_bps = cwnd_controller_->UpdateTargetBitrate(bitrate_bps);
+  bitrate_bps = cwnd_controller.UpdateTargetBitrate(bitrate_bps);
   EXPECT_GT(80000u, bitrate_bps);
 }
 
-TEST_F(CongestionWindowPushbackControllerTest, PushbackDropFrame) {
+TEST(CongestionWindowPushbackControllerTest, PushbackDropFrame) {
-  test::ScopedFieldTrials trials("WebRTC-CongestionWindow/DropFrame:true/");
+  CongestionWindowPushbackController cwnd_controller(
-  cwnd_controller_.reset(
+      ExplicitKeyValueConfig("WebRTC-CongestionWindow/DropFrame:true/"));
-      new CongestionWindowPushbackController(&field_trial_config_));
+
-  cwnd_controller_->UpdateOutstandingData(1e8);  // Large number
+  cwnd_controller.UpdateOutstandingData(1e8);  // Large number
-  cwnd_controller_->SetDataWindow(DataSize::Bytes(50000));
+  cwnd_controller.SetDataWindow(DataSize::Bytes(50000));
 
   uint32_t bitrate_bps = 80000;
-  bitrate_bps = cwnd_controller_->UpdateTargetBitrate(bitrate_bps);
+  bitrate_bps = cwnd_controller.UpdateTargetBitrate(bitrate_bps);
   EXPECT_GT(80000u, bitrate_bps);
 }
 

modules/congestion_controller/goog_cc/goog_cc_network_control.cc

@@ -65,14 +65,6 @@ constexpr float kDefaultPaceMultiplier = 2.5f;
 // below the current throughput estimate to drain the network queues.
 constexpr double kProbeDropThroughputFraction = 0.85;
 
-bool IsEnabled(const FieldTrialsView* config, absl::string_view key) {
-  return absl::StartsWith(config->Lookup(key), "Enabled");
-}
-
-bool IsNotDisabled(const FieldTrialsView* config, absl::string_view key) {
-  return !absl::StartsWith(config->Lookup(key), "Disabled");
-}
-
 BandwidthLimitedCause GetBandwidthLimitedCause(LossBasedState loss_based_state,
                                                bool is_rtt_above_limit,
                                                BandwidthUsage bandwidth_usage) {
@@ -108,27 +100,24 @@ GoogCcNetworkController::GoogCcNetworkController(NetworkControllerConfig config,
       safe_reset_on_route_change_("Enabled"),
       safe_reset_acknowledged_rate_("ack"),
       use_min_allocatable_as_lower_bound_(
-          IsNotDisabled(key_value_config_, "WebRTC-Bwe-MinAllocAsLowerBound")),
+          !key_value_config_->IsDisabled("WebRTC-Bwe-MinAllocAsLowerBound")),
-      ignore_probes_lower_than_network_estimate_(IsNotDisabled(
+      ignore_probes_lower_than_network_estimate_(!key_value_config_->IsDisabled(
-          key_value_config_,
           "WebRTC-Bwe-IgnoreProbesLowerThanNetworkStateEstimate")),
       limit_probes_lower_than_throughput_estimate_(
-          IsNotDisabled(key_value_config_,
+          !key_value_config_->IsDisabled(
-                        "WebRTC-Bwe-LimitProbesLowerThanThroughputEstimate")),
+              "WebRTC-Bwe-LimitProbesLowerThanThroughputEstimate")),
-      rate_control_settings_(
+      rate_control_settings_(*key_value_config_),
-          RateControlSettings::ParseFromKeyValueConfig(key_value_config_)),
+      pace_at_max_of_bwe_and_lower_link_capacity_(key_value_config_->IsEnabled(
-      pace_at_max_of_bwe_and_lower_link_capacity_(
+          "WebRTC-Bwe-PaceAtMaxOfBweAndLowerLinkCapacity")),
-          IsEnabled(key_value_config_,
-                    "WebRTC-Bwe-PaceAtMaxOfBweAndLowerLinkCapacity")),
       limit_pacingfactor_by_upper_link_capacity_estimate_(
-          IsEnabled(key_value_config_,
+          key_value_config_->IsEnabled(
-                    "WebRTC-Bwe-LimitPacingFactorByUpperLinkCapacityEstimate")),
+              "WebRTC-Bwe-LimitPacingFactorByUpperLinkCapacityEstimate")),
       probe_controller_(
           new ProbeController(key_value_config_, config.event_log)),
       congestion_window_pushback_controller_(
           rate_control_settings_.UseCongestionWindowPushback()
               ? std::make_unique<CongestionWindowPushbackController>(
-                    key_value_config_)
+                    *key_value_config_)
               : nullptr),
       bandwidth_estimation_(
           std::make_unique<SendSideBandwidthEstimation>(key_value_config_,

modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc

@@ -309,8 +309,7 @@ LibvpxVp8Encoder::LibvpxVp8Encoder(const Environment& env,
                                    std::unique_ptr<LibvpxInterface> interface)
     : env_(env),
       libvpx_(std::move(interface)),
-      rate_control_settings_(
+      rate_control_settings_(env_.field_trials()),
-          RateControlSettings::ParseFromKeyValueConfig(&env_.field_trials())),
       resolution_bitrate_limits_(std::move(settings.resolution_bitrate_limits)),
       key_frame_request_(kMaxSimulcastStreams, false),
       last_encoder_output_time_(kMaxSimulcastStreams,

modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc

@@ -250,9 +250,8 @@ LibvpxVp9Encoder::LibvpxVp9Encoder(const Environment& env,
       is_svc_(false),
       inter_layer_pred_(InterLayerPredMode::kOn),
       external_ref_control_(false),  // Set in InitEncode because of tests.
-      trusted_rate_controller_(
+      trusted_rate_controller_(RateControlSettings(env.field_trials())
-          RateControlSettings::ParseFromKeyValueConfig(&env.field_trials())
+                                   .LibvpxVp9TrustedRateController()),
-              .LibvpxVp9TrustedRateController()),
       first_frame_in_picture_(true),
       ss_info_needed_(false),
       force_all_active_layers_(false),

rtc_base/experiments/rate_control_settings.cc

@@ -34,11 +34,6 @@ const char kCongestionWindowDefaultFieldTrialString[] =
 const char kUseBaseHeavyVp8Tl3RateAllocationFieldTrialName[] =
     "WebRTC-UseBaseHeavyVP8TL3RateAllocation";
 
-bool IsEnabled(const FieldTrialsView* const key_value_config,
-               absl::string_view key) {
-  return absl::StartsWith(key_value_config->Lookup(key), "Enabled");
-}
-
 }  // namespace
 
 constexpr char CongestionWindowConfig::kKey[];
@@ -75,32 +70,25 @@ std::unique_ptr<StructParametersParser> VideoRateControlConfig::Parser() {
 }
 
 RateControlSettings::RateControlSettings(
-    const FieldTrialsView* const key_value_config) {
+    const FieldTrialsView& key_value_config) {
   std::string congestion_window_config =
-      key_value_config->Lookup(CongestionWindowConfig::kKey).empty()
+      key_value_config.Lookup(CongestionWindowConfig::kKey);
-          ? kCongestionWindowDefaultFieldTrialString
+  if (congestion_window_config.empty()) {
-          : key_value_config->Lookup(CongestionWindowConfig::kKey);
+    congestion_window_config = kCongestionWindowDefaultFieldTrialString;
+  }
   congestion_window_config_ =
       CongestionWindowConfig::Parse(congestion_window_config);
-  video_config_.vp8_base_heavy_tl3_alloc = IsEnabled(
+  video_config_.vp8_base_heavy_tl3_alloc = key_value_config.IsEnabled(
-      key_value_config, kUseBaseHeavyVp8Tl3RateAllocationFieldTrialName);
+      kUseBaseHeavyVp8Tl3RateAllocationFieldTrialName);
   video_config_.Parser()->Parse(
-      key_value_config->Lookup(VideoRateControlConfig::kKey));
+      key_value_config.Lookup(VideoRateControlConfig::kKey));
 }
 
 RateControlSettings::~RateControlSettings() = default;
 RateControlSettings::RateControlSettings(RateControlSettings&&) = default;
 
 RateControlSettings RateControlSettings::ParseFromFieldTrials() {
-  FieldTrialBasedConfig field_trial_config;
+  return RateControlSettings(FieldTrialBasedConfig());
-  return RateControlSettings(&field_trial_config);
-}
-
-RateControlSettings RateControlSettings::ParseFromKeyValueConfig(
-    const FieldTrialsView* const key_value_config) {
-  FieldTrialBasedConfig field_trial_config;
-  return RateControlSettings(key_value_config ? key_value_config
-                                              : &field_trial_config);
 }
 
 bool RateControlSettings::UseCongestionWindow() const {

rtc_base/experiments/rate_control_settings.h

@@ -48,12 +48,11 @@ struct VideoRateControlConfig {
 
 class RateControlSettings final {
  public:
-  ~RateControlSettings();
+  explicit RateControlSettings(const FieldTrialsView& key_value_config);
   RateControlSettings(RateControlSettings&&);
+  ~RateControlSettings();
 
   static RateControlSettings ParseFromFieldTrials();
-  static RateControlSettings ParseFromKeyValueConfig(
-      const FieldTrialsView* const key_value_config);
 
   // When CongestionWindowPushback is enabled, the pacer is oblivious to
   // the congestion window. The relation between outstanding data and
@@ -82,8 +81,6 @@ class RateControlSettings final {
   bool BitrateAdjusterCanUseNetworkHeadroom() const;
 
  private:
-  explicit RateControlSettings(const FieldTrialsView* const key_value_config);
-
   CongestionWindowConfig congestion_window_config_;
   VideoRateControlConfig video_config_;
 };

video/config/simulcast.cc

@@ -381,8 +381,7 @@ std::vector<webrtc::VideoStream> GetSimulcastConfig(
   RTC_DCHECK(max_layers > 1 || is_screenshare_with_conference_mode);
 
   const bool base_heavy_tl3_rate_alloc =
-      webrtc::RateControlSettings::ParseFromKeyValueConfig(&trials)
+      webrtc::RateControlSettings(trials).Vp8BaseHeavyTl3RateAllocation();
-          .Vp8BaseHeavyTl3RateAllocation();
   if (is_screenshare_with_conference_mode) {
     return GetScreenshareLayers(max_layers, width, height, bitrate_priority,
                                 max_qp, temporal_layers_supported,

video/encoder_bitrate_adjuster.cc

@@ -47,9 +47,8 @@ constexpr double EncoderBitrateAdjuster::kDefaultUtilizationFactor;
 EncoderBitrateAdjuster::EncoderBitrateAdjuster(
     const VideoCodec& codec_settings,
     const FieldTrialsView& field_trials)
-    : utilize_bandwidth_headroom_(
+    : utilize_bandwidth_headroom_(RateControlSettings(field_trials)
-          RateControlSettings::ParseFromKeyValueConfig(&field_trials)
+                                      .BitrateAdjusterCanUseNetworkHeadroom()),
-              .BitrateAdjusterCanUseNetworkHeadroom()),
       frames_since_layout_change_(0),
       min_bitrates_bps_{},
       frame_size_pixels_{},

video/video_send_stream_impl.cc

@@ -221,10 +221,9 @@ absl::optional<float> GetConfiguredPacingFactor(
   if (alr_settings)
     return alr_settings->pacing_factor;
 
-  RateControlSettings rate_control_settings =
+  return RateControlSettings(field_trials)
-      RateControlSettings::ParseFromKeyValueConfig(&field_trials);
+      .GetPacingFactor()
-  return rate_control_settings.GetPacingFactor().value_or(
+      .value_or(default_pacing_config.pacing_factor);
-      default_pacing_config.pacing_factor);
 }
 
 int GetEncoderPriorityBitrate(std::string codec_name,
@@ -485,9 +484,8 @@ VideoSendStreamImpl::VideoSendStreamImpl(
       enable_alr_bw_probing = true;
       queue_time_limit_ms = alr_settings->max_paced_queue_time;
     } else {
-      RateControlSettings rate_control_settings =
+      enable_alr_bw_probing =
-          RateControlSettings::ParseFromKeyValueConfig(&env_.field_trials());
+          RateControlSettings(env_.field_trials()).UseAlrProbing();
-      enable_alr_bw_probing = rate_control_settings.UseAlrProbing();
       queue_time_limit_ms = pacing_config_.max_pacing_delay.Get().ms();
     }
 

video/video_stream_encoder.cc

@@ -660,8 +660,7 @@ VideoStreamEncoder::VideoStreamEncoder(
       number_of_cores_(number_of_cores),
       settings_(settings),
       allocation_cb_type_(allocation_cb_type),
-      rate_control_settings_(
+      rate_control_settings_(env_.field_trials()),
-          RateControlSettings::ParseFromKeyValueConfig(&env_.field_trials())),
       encoder_selector_from_constructor_(encoder_selector),
       encoder_selector_from_factory_(
           encoder_selector_from_constructor_