admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove direct use of FieldTrials from modules/remote_bitrate_estimator

Browse Source 
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}
This commit is contained in:
Per Kjellander 2019-04-16 14:50:08 +02:00 committed by Commit Bot
parent bfb735b5a1
commit 494947bbcf
13 changed files with 73 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
modules/congestion_controller/goog_cc/delay_based_bwe.cc
View File

@ -90,6 +90,7 @@ DelayBasedBwe::DelayBasedBwe(const WebRtcKeyValueConfig* key_value_config,
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

3
modules/remote_bitrate_estimator/BUILD.gn
View File

@ -43,7 +43,9 @@ rtc_static_library("remote_bitrate_estimator") {
"../..: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 @@ if (rtc_include_tests) {
":remote_bitrate_estimator",
"..:module_api_public",
"../..:webrtc_common",
"../../api/transport:field_trial_based_config",
"../../rtc_base",
"../../rtc_base:checks",
"../../rtc_base:rtc_base_approved",

2
modules/remote_bitrate_estimator/DEPS
View File

@ -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",
]

28
modules/remote_bitrate_estimator/aimd_rate_control.cc
View File

@ -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 @@ double ReadBackoffFactor() {
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 @@ AimdRateControl::AimdRateControl()
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 @@ AimdRateControl::AimdRateControl()
// 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 @@ AimdRateControl::AimdRateControl()
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() {}

3
modules/remote_bitrate_estimator/aimd_rate_control.h
View File

@ -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 @@ namespace webrtc {
// multiplicatively.
class AimdRateControl {
public:
AimdRateControl();
explicit AimdRateControl(const WebRtcKeyValueConfig* key_value_config);
~AimdRateControl();
// Returns true if the target bitrate has been initialized. This happens

4
modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc
View File

@ -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 @@ constexpr double kFractionAfterOveruse = 0.85;
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;
}

25
modules/remote_bitrate_estimator/overuse_detector.cc
View File

@ -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 kMaxAdaptOffsetMs = 15.0;
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 @@ bool AdaptiveThresholdExperimentIsDisabled() {
// 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 @@ bool ReadExperimentConstants(double* k_up, double* k_down) {
"%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 @@ OveruseDetector::OveruseDetector()
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 @@ void OveruseDetector::UpdateThreshold(double modified_offset, int64_t now_ms) {
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;
}

8
modules/remote_bitrate_estimator/overuse_detector.h
View File

@ -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 @@ class OveruseDetector {
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_;

11
modules/remote_bitrate_estimator/overuse_detector_unittest.cc
View File

@ -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 @@ class OveruseDetectorTest : public ::testing::Test {
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 @@ class OveruseDetectorTest : public ::testing::Test {
}
}
const FieldTrialBasedConfig field_trials_;
int64_t now_ms_;
int64_t receive_time_ms_;
uint32_t rtp_timestamp_;
@ -671,9 +675,12 @@ class OveruseDetectorExperimentTest : public OveruseDetectorTest {
"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) {

6
modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc
View File

@ -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 @@ RemoteBitrateEstimatorAbsSendTime::RemoteBitrateEstimatorAbsSendTime(
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.";

2
modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h
View File

@ -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 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
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_;

14
modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc
View File

@ -43,13 +43,14 @@ static const double kTimestampToMs = 1.0 / 90.0;
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 @@ RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream(
: 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 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
// 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 @@ void RemoteBitrateEstimatorSingleStream::GetSsrcs(
AimdRateControl* RemoteBitrateEstimatorSingleStream::GetRemoteRate() {
if (!remote_rate_)
remote_rate_.reset(new AimdRateControl());
remote_rate_.reset(new AimdRateControl(&field_trials_));
return remote_rate_.get();
}

2
modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h
View File

@ -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 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator {
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_);