Remove direct use of FieldTrials from modules/remote_bitrate_estimator
Instead use WebRtcKeyValueConfig and FieldTrialBasedConfig
BUG=webrtc:10335
Change-Id: Ie148cb466f86d8fa1ded5c7f125fbcccf6e7dbe3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132714
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27642}diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc
index 450449f..665c11d 100644
--- a/modules/congestion_controller/goog_cc/delay_based_bwe.cc
+++ b/modules/congestion_controller/goog_cc/delay_based_bwe.cc
@@ -90,6 +90,7 @@
delay_detector_(),
last_seen_packet_(Timestamp::MinusInfinity()),
uma_recorded_(false),
+ rate_control_(key_value_config),
trendline_window_size_(
key_value_config->Lookup(kBweWindowSizeInPacketsExperiment)
.find("Enabled") == 0
diff --git a/modules/remote_bitrate_estimator/BUILD.gn b/modules/remote_bitrate_estimator/BUILD.gn
index 7ce7cba..20977b2 100644
--- a/modules/remote_bitrate_estimator/BUILD.gn
+++ b/modules/remote_bitrate_estimator/BUILD.gn
@@ -43,7 +43,9 @@
"../..:webrtc_common",
"../../api:network_state_predictor_api",
"../../api:rtp_headers",
+ "../../api/transport:field_trial_based_config",
"../../api/transport:network_control",
+ "../../api/transport:webrtc_key_value_config",
"../../api/units:data_rate",
"../../api/units:timestamp",
"../../modules:module_api",
@@ -208,6 +210,7 @@
":remote_bitrate_estimator",
"..:module_api_public",
"../..:webrtc_common",
+ "../../api/transport:field_trial_based_config",
"../../rtc_base",
"../../rtc_base:checks",
"../../rtc_base:rtc_base_approved",
diff --git a/modules/remote_bitrate_estimator/DEPS b/modules/remote_bitrate_estimator/DEPS
index 8499c25..66a3201 100644
--- a/modules/remote_bitrate_estimator/DEPS
+++ b/modules/remote_bitrate_estimator/DEPS
@@ -1,4 +1,6 @@
include_rules = [
"+logging/rtc_event_log",
"+system_wrappers",
+ # Avoid directly using field_trial. Instead use WebRtcKeyValueConfig.
+ "-system_wrappers/include/field_trial.h",
]
diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc
index 688b3b8..90cda33 100644
--- a/modules/remote_bitrate_estimator/aimd_rate_control.cc
+++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc
@@ -25,17 +25,23 @@
#include "rtc_base/experiments/field_trial_parser.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_minmax.h"
-#include "system_wrappers/include/field_trial.h"
namespace webrtc {
+namespace {
+
constexpr TimeDelta kDefaultRtt = TimeDelta::Millis<200>();
constexpr double kDefaultBackoffFactor = 0.85;
const char kBweBackOffFactorExperiment[] = "WebRTC-BweBackOffFactor";
-double ReadBackoffFactor() {
+bool IsEnabled(const WebRtcKeyValueConfig& field_trials,
+ absl::string_view key) {
+ return field_trials.Lookup(key).find("Enabled") == 0;
+}
+
+double ReadBackoffFactor(const WebRtcKeyValueConfig& key_value_config) {
std::string experiment_string =
- webrtc::field_trial::FindFullName(kBweBackOffFactorExperiment);
+ key_value_config.Lookup(kBweBackOffFactorExperiment);
double backoff_factor;
int parsed_values =
sscanf(experiment_string.c_str(), "Enabled-%lf", &backoff_factor);
@@ -53,7 +59,9 @@
return kDefaultBackoffFactor;
}
-AimdRateControl::AimdRateControl()
+} // namespace
+
+AimdRateControl::AimdRateControl(const WebRtcKeyValueConfig* key_value_config)
: min_configured_bitrate_(congestion_controller::GetMinBitrate()),
max_configured_bitrate_(DataRate::kbps(30000)),
current_bitrate_(max_configured_bitrate_),
@@ -64,13 +72,13 @@
time_last_bitrate_decrease_(Timestamp::MinusInfinity()),
time_first_throughput_estimate_(Timestamp::MinusInfinity()),
bitrate_is_initialized_(false),
- beta_(webrtc::field_trial::IsEnabled(kBweBackOffFactorExperiment)
- ? ReadBackoffFactor()
+ beta_(IsEnabled(*key_value_config, kBweBackOffFactorExperiment)
+ ? ReadBackoffFactor(*key_value_config)
: kDefaultBackoffFactor),
rtt_(kDefaultRtt),
- in_experiment_(!AdaptiveThresholdExperimentIsDisabled()),
+ in_experiment_(!AdaptiveThresholdExperimentIsDisabled(*key_value_config)),
smoothing_experiment_(
- webrtc::field_trial::IsEnabled("WebRTC-Audio-BandwidthSmoothing")),
+ IsEnabled(*key_value_config, "WebRTC-Audio-BandwidthSmoothing")),
initial_backoff_interval_("initial_backoff_interval"),
low_throughput_threshold_("low_throughput", DataRate::Zero()),
capacity_deviation_ratio_threshold_("cap_thr", 0.2),
@@ -80,7 +88,7 @@
// WebRTC-BweAimdRateControlConfig/initial_backoff_interval:100ms,
// low_throughput:50kbps/
ParseFieldTrial({&initial_backoff_interval_, &low_throughput_threshold_},
- field_trial::FindFullName("WebRTC-BweAimdRateControlConfig"));
+ key_value_config->Lookup("WebRTC-BweAimdRateControlConfig"));
if (initial_backoff_interval_) {
RTC_LOG(LS_INFO) << "Using aimd rate control with initial back-off interval"
<< " " << ToString(*initial_backoff_interval_) << ".";
@@ -89,7 +97,7 @@
ParseFieldTrial(
{&capacity_deviation_ratio_threshold_, &cross_traffic_factor_,
&capacity_limit_deviation_factor_},
- field_trial::FindFullName("WebRTC-Bwe-AimdRateControl-NetworkState"));
+ key_value_config->Lookup("WebRTC-Bwe-AimdRateControl-NetworkState"));
}
AimdRateControl::~AimdRateControl() {}
diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.h b/modules/remote_bitrate_estimator/aimd_rate_control.h
index dfefc48..4b51c94 100644
--- a/modules/remote_bitrate_estimator/aimd_rate_control.h
+++ b/modules/remote_bitrate_estimator/aimd_rate_control.h
@@ -15,6 +15,7 @@
#include "absl/types/optional.h"
#include "api/transport/network_types.h"
+#include "api/transport/webrtc_key_value_config.h"
#include "api/units/data_rate.h"
#include "api/units/timestamp.h"
#include "modules/congestion_controller/goog_cc/link_capacity_estimator.h"
@@ -29,7 +30,7 @@
// multiplicatively.
class AimdRateControl {
public:
- AimdRateControl();
+ explicit AimdRateControl(const WebRtcKeyValueConfig* key_value_config);
~AimdRateControl();
// Returns true if the target bitrate has been initialized. This happens
diff --git a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc
index 4bec9e8..0c93ca8 100644
--- a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc
+++ b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc
@@ -9,6 +9,7 @@
*/
#include <memory>
+#include "api/transport/field_trial_based_config.h"
#include "modules/remote_bitrate_estimator/aimd_rate_control.h"
#include "system_wrappers/include/clock.h"
#include "test/field_trial.h"
@@ -32,11 +33,12 @@
struct AimdRateControlStates {
std::unique_ptr<AimdRateControl> aimd_rate_control;
std::unique_ptr<SimulatedClock> simulated_clock;
+ FieldTrialBasedConfig field_trials;
};
AimdRateControlStates CreateAimdRateControlStates() {
AimdRateControlStates states;
- states.aimd_rate_control.reset(new AimdRateControl());
+ states.aimd_rate_control.reset(new AimdRateControl(&states.field_trials));
states.simulated_clock.reset(new SimulatedClock(kClockInitialTime));
return states;
}
diff --git a/modules/remote_bitrate_estimator/overuse_detector.cc b/modules/remote_bitrate_estimator/overuse_detector.cc
index ec4b772..6698c55 100644
--- a/modules/remote_bitrate_estimator/overuse_detector.cc
+++ b/modules/remote_bitrate_estimator/overuse_detector.cc
@@ -19,7 +19,6 @@
#include "modules/remote_bitrate_estimator/test/bwe_test_logging.h"
#include "rtc_base/checks.h"
#include "rtc_base/numerics/safe_minmax.h"
-#include "system_wrappers/include/field_trial.h"
namespace webrtc {
@@ -33,9 +32,10 @@
const double kOverUsingTimeThreshold = 10;
const int kMaxNumDeltas = 60;
-bool AdaptiveThresholdExperimentIsDisabled() {
+bool AdaptiveThresholdExperimentIsDisabled(
+ const WebRtcKeyValueConfig& key_value_config) {
std::string experiment_string =
- webrtc::field_trial::FindFullName(kAdaptiveThresholdExperiment);
+ key_value_config.Lookup(kAdaptiveThresholdExperiment);
const size_t kMinExperimentLength = kDisabledPrefixLength;
if (experiment_string.length() < kMinExperimentLength)
return false;
@@ -44,9 +44,11 @@
// Gets thresholds from the experiment name following the format
// "WebRTC-AdaptiveBweThreshold/Enabled-0.5,0.002/".
-bool ReadExperimentConstants(double* k_up, double* k_down) {
+bool ReadExperimentConstants(const WebRtcKeyValueConfig& key_value_config,
+ double* k_up,
+ double* k_down) {
std::string experiment_string =
- webrtc::field_trial::FindFullName(kAdaptiveThresholdExperiment);
+ key_value_config.Lookup(kAdaptiveThresholdExperiment);
const size_t kMinExperimentLength = kEnabledPrefixLength + 3;
if (experiment_string.length() < kMinExperimentLength ||
experiment_string.substr(0, kEnabledPrefixLength) != kEnabledPrefix)
@@ -55,10 +57,10 @@
"%lf,%lf", k_up, k_down) == 2;
}
-OveruseDetector::OveruseDetector()
+OveruseDetector::OveruseDetector(const WebRtcKeyValueConfig* key_value_config)
// Experiment is on by default, but can be disabled with finch by setting
// the field trial string to "WebRTC-AdaptiveBweThreshold/Disabled/".
- : in_experiment_(!AdaptiveThresholdExperimentIsDisabled()),
+ : in_experiment_(!AdaptiveThresholdExperimentIsDisabled(*key_value_config)),
k_up_(0.0087),
k_down_(0.039),
overusing_time_threshold_(100),
@@ -68,8 +70,8 @@
time_over_using_(-1),
overuse_counter_(0),
hypothesis_(BandwidthUsage::kBwNormal) {
- if (!AdaptiveThresholdExperimentIsDisabled())
- InitializeExperiment();
+ if (!AdaptiveThresholdExperimentIsDisabled(*key_value_config))
+ InitializeExperiment(*key_value_config);
}
OveruseDetector::~OveruseDetector() {}
@@ -144,12 +146,13 @@
last_update_ms_ = now_ms;
}
-void OveruseDetector::InitializeExperiment() {
+void OveruseDetector::InitializeExperiment(
+ const WebRtcKeyValueConfig& key_value_config) {
RTC_DCHECK(in_experiment_);
double k_up = 0.0;
double k_down = 0.0;
overusing_time_threshold_ = kOverUsingTimeThreshold;
- if (ReadExperimentConstants(&k_up, &k_down)) {
+ if (ReadExperimentConstants(key_value_config, &k_up, &k_down)) {
k_up_ = k_up;
k_down_ = k_down;
}
diff --git a/modules/remote_bitrate_estimator/overuse_detector.h b/modules/remote_bitrate_estimator/overuse_detector.h
index a72881b..1df6cab 100644
--- a/modules/remote_bitrate_estimator/overuse_detector.h
+++ b/modules/remote_bitrate_estimator/overuse_detector.h
@@ -12,16 +12,18 @@
#include <stdint.h>
+#include "api/transport/webrtc_key_value_config.h"
#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
#include "rtc_base/constructor_magic.h"
namespace webrtc {
-bool AdaptiveThresholdExperimentIsDisabled();
+bool AdaptiveThresholdExperimentIsDisabled(
+ const WebRtcKeyValueConfig& key_value_config);
class OveruseDetector {
public:
- OveruseDetector();
+ explicit OveruseDetector(const WebRtcKeyValueConfig* key_value_config);
virtual ~OveruseDetector();
// Update the detection state based on the estimated inter-arrival time delta
@@ -40,7 +42,7 @@
private:
void UpdateThreshold(double modified_offset, int64_t now_ms);
- void InitializeExperiment();
+ void InitializeExperiment(const WebRtcKeyValueConfig& key_value_config);
bool in_experiment_;
double k_up_;
diff --git a/modules/remote_bitrate_estimator/overuse_detector_unittest.cc b/modules/remote_bitrate_estimator/overuse_detector_unittest.cc
index 7fefa5c..91f9609 100644
--- a/modules/remote_bitrate_estimator/overuse_detector_unittest.cc
+++ b/modules/remote_bitrate_estimator/overuse_detector_unittest.cc
@@ -14,6 +14,7 @@
#include <cstdlib>
#include <memory>
+#include "api/transport/field_trial_based_config.h"
#include "modules/remote_bitrate_estimator/inter_arrival.h"
#include "modules/remote_bitrate_estimator/overuse_detector.h"
#include "modules/remote_bitrate_estimator/overuse_estimator.h"
@@ -38,7 +39,9 @@
random_(123456789) {}
protected:
- void SetUp() override { overuse_detector_.reset(new OveruseDetector()); }
+ void SetUp() override {
+ overuse_detector_.reset(new OveruseDetector(&field_trials_));
+ }
int Run100000Samples(int packets_per_frame,
size_t packet_size,
@@ -107,6 +110,7 @@
}
}
+ const FieldTrialBasedConfig field_trials_;
int64_t now_ms_;
int64_t receive_time_ms_;
uint32_t rtp_timestamp_;
@@ -671,9 +675,12 @@
"WebRTC-AdaptiveBweThreshold/Enabled-0.01,0.00018/") {}
protected:
- void SetUp() override { overuse_detector_.reset(new OveruseDetector()); }
+ void SetUp() override {
+ overuse_detector_.reset(new OveruseDetector(&field_trials_));
+ }
test::ScopedFieldTrials override_field_trials_;
+ const FieldTrialBasedConfig field_trials_;
};
TEST_F(OveruseDetectorExperimentTest, ThresholdAdapts) {
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
index cb3a01c..ed2d061 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
@@ -14,6 +14,7 @@
#include <algorithm>
+#include "api/transport/field_trial_based_config.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "rtc_base/checks.h"
#include "rtc_base/constructor_magic.h"
@@ -95,13 +96,14 @@
observer_(observer),
inter_arrival_(),
estimator_(),
- detector_(),
+ detector_(&field_trials_),
incoming_bitrate_(kBitrateWindowMs, 8000),
incoming_bitrate_initialized_(false),
total_probes_received_(0),
first_packet_time_ms_(-1),
last_update_ms_(-1),
- uma_recorded_(false) {
+ uma_recorded_(false),
+ remote_rate_(&field_trials_) {
RTC_DCHECK(clock_);
RTC_DCHECK(observer_);
RTC_LOG(LS_INFO) << "RemoteBitrateEstimatorAbsSendTime: Instantiating.";
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
index 5403fca..02225a5 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
@@ -19,6 +19,7 @@
#include <vector>
#include "api/rtp_headers.h"
+#include "api/transport/field_trial_based_config.h"
#include "modules/remote_bitrate_estimator/aimd_rate_control.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "modules/remote_bitrate_estimator/inter_arrival.h"
@@ -121,6 +122,7 @@
rtc::RaceChecker network_race_;
Clock* const clock_;
+ const FieldTrialBasedConfig field_trials_;
RemoteBitrateObserver* const observer_;
std::unique_ptr<InterArrival> inter_arrival_;
std::unique_ptr<OveruseEstimator> estimator_;
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
index e5e25c9..aabf122 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
@@ -43,13 +43,14 @@
struct RemoteBitrateEstimatorSingleStream::Detector {
explicit Detector(int64_t last_packet_time_ms,
const OverUseDetectorOptions& options,
- bool enable_burst_grouping)
+ bool enable_burst_grouping,
+ const WebRtcKeyValueConfig* key_value_config)
: last_packet_time_ms(last_packet_time_ms),
inter_arrival(90 * kTimestampGroupLengthMs,
kTimestampToMs,
enable_burst_grouping),
estimator(options),
- detector() {}
+ detector(key_value_config) {}
int64_t last_packet_time_ms;
InterArrival inter_arrival;
OveruseEstimator estimator;
@@ -62,7 +63,7 @@
: clock_(clock),
incoming_bitrate_(kBitrateWindowMs, 8000),
last_valid_incoming_bitrate_(0),
- remote_rate_(new AimdRateControl()),
+ remote_rate_(new AimdRateControl(&field_trials_)),
observer_(observer),
last_process_time_(-1),
process_interval_ms_(kProcessIntervalMs),
@@ -103,8 +104,9 @@
// automatically cleaned up when we have one RemoteBitrateEstimator per REMB
// group.
std::pair<SsrcOveruseEstimatorMap::iterator, bool> insert_result =
- overuse_detectors_.insert(std::make_pair(
- ssrc, new Detector(now_ms, OverUseDetectorOptions(), true)));
+ overuse_detectors_.insert(
+ std::make_pair(ssrc, new Detector(now_ms, OverUseDetectorOptions(),
+ true, &field_trials_)));
it = insert_result.first;
}
Detector* estimator = it->second;
@@ -255,7 +257,7 @@
AimdRateControl* RemoteBitrateEstimatorSingleStream::GetRemoteRate() {
if (!remote_rate_)
- remote_rate_.reset(new AimdRateControl());
+ remote_rate_.reset(new AimdRateControl(&field_trials_));
return remote_rate_.get();
}
diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
index 3f05c45..80129ce 100644
--- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
+++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
@@ -17,6 +17,7 @@
#include <memory>
#include <vector>
+#include "api/transport/field_trial_based_config.h"
#include "modules/remote_bitrate_estimator/aimd_rate_control.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "rtc_base/constructor_magic.h"
@@ -63,6 +64,7 @@
AimdRateControl* GetRemoteRate() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
Clock* const clock_;
+ const FieldTrialBasedConfig field_trials_;
SsrcOveruseEstimatorMap overuse_detectors_ RTC_GUARDED_BY(crit_sect_);
RateStatistics incoming_bitrate_ RTC_GUARDED_BY(crit_sect_);
uint32_t last_valid_incoming_bitrate_ RTC_GUARDED_BY(crit_sect_);