Remove direct use of FieldTrials from AlrDetector
Instead use WebRtcKeyValueConfig and FieldTrialBasedConfig. The purpose is to allow a user of GoogCC to use different settings on different instances. BUG=webrtc:10335 Change-Id: I2f837688c9fdd341eecb44484cc784b1c80da1a9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132791 Commit-Queue: Per Kjellander <perkj@webrtc.org> Reviewed-by: Sebastian Jansson <srte@webrtc.org> Cr-Commit-Position: refs/heads/master@{#27617}
This commit is contained in:

committed by
Commit Bot

parent
315de596b0
commit
5b69873cb5
@ -89,6 +89,7 @@ rtc_source_set("alr_detector") {
|
||||
"alr_detector.h",
|
||||
]
|
||||
deps = [
|
||||
"../../../api/transport:field_trial_based_config",
|
||||
"../../../api/transport:webrtc_key_value_config",
|
||||
"../../../logging:rtc_event_log_api",
|
||||
"../../../logging:rtc_event_pacing",
|
||||
|
@ -23,20 +23,25 @@
|
||||
#include "rtc_base/time_utils.h"
|
||||
|
||||
namespace webrtc {
|
||||
AlrDetector::AlrDetector() : AlrDetector(nullptr) {}
|
||||
|
||||
AlrDetector::AlrDetector(RtcEventLog* event_log)
|
||||
AlrDetector::AlrDetector(const WebRtcKeyValueConfig* key_value_config)
|
||||
: AlrDetector(key_value_config, nullptr) {}
|
||||
|
||||
AlrDetector::AlrDetector(const WebRtcKeyValueConfig* key_value_config,
|
||||
RtcEventLog* event_log)
|
||||
: bandwidth_usage_percent_(kDefaultAlrBandwidthUsagePercent),
|
||||
alr_start_budget_level_percent_(kDefaultAlrStartBudgetLevelPercent),
|
||||
alr_stop_budget_level_percent_(kDefaultAlrStopBudgetLevelPercent),
|
||||
alr_budget_(0, true),
|
||||
event_log_(event_log) {
|
||||
RTC_CHECK(AlrExperimentSettings::MaxOneFieldTrialEnabled());
|
||||
RTC_CHECK(AlrExperimentSettings::MaxOneFieldTrialEnabled(*key_value_config));
|
||||
absl::optional<AlrExperimentSettings> experiment_settings =
|
||||
AlrExperimentSettings::CreateFromFieldTrial(
|
||||
*key_value_config,
|
||||
AlrExperimentSettings::kScreenshareProbingBweExperimentName);
|
||||
if (!experiment_settings) {
|
||||
experiment_settings = AlrExperimentSettings::CreateFromFieldTrial(
|
||||
*key_value_config,
|
||||
AlrExperimentSettings::kStrictPacingAndProbingExperimentName);
|
||||
}
|
||||
if (experiment_settings) {
|
||||
|
@ -15,6 +15,7 @@
|
||||
#include <stdint.h>
|
||||
|
||||
#include "absl/types/optional.h"
|
||||
#include "api/transport/webrtc_key_value_config.h"
|
||||
#include "modules/pacing/interval_budget.h"
|
||||
|
||||
namespace webrtc {
|
||||
@ -30,8 +31,9 @@ class RtcEventLog;
|
||||
// Note: This class is not thread-safe.
|
||||
class AlrDetector {
|
||||
public:
|
||||
AlrDetector();
|
||||
explicit AlrDetector(RtcEventLog* event_log);
|
||||
explicit AlrDetector(const WebRtcKeyValueConfig* key_value_config);
|
||||
AlrDetector(const WebRtcKeyValueConfig* key_value_config,
|
||||
RtcEventLog* event_log);
|
||||
~AlrDetector();
|
||||
|
||||
void OnBytesSent(size_t bytes_sent, int64_t send_time_ms);
|
||||
|
@ -10,6 +10,7 @@
|
||||
|
||||
#include "modules/congestion_controller/goog_cc/alr_detector.h"
|
||||
|
||||
#include "api/transport/field_trial_based_config.h"
|
||||
#include "rtc_base/checks.h"
|
||||
#include "rtc_base/experiments/alr_experiment.h"
|
||||
#include "test/field_trial.h"
|
||||
@ -71,11 +72,13 @@ class SimulateOutgoingTrafficIn {
|
||||
|
||||
class AlrDetectorTest : public ::testing::Test {
|
||||
public:
|
||||
AlrDetectorTest() : alr_detector_(&field_trials_) {}
|
||||
void SetUp() override {
|
||||
alr_detector_.SetEstimatedBitrate(kEstimatedBitrateBps);
|
||||
}
|
||||
|
||||
protected:
|
||||
FieldTrialBasedConfig field_trials_;
|
||||
AlrDetector alr_detector_;
|
||||
int64_t timestamp_ms_ = 1000;
|
||||
};
|
||||
@ -153,7 +156,7 @@ TEST_F(AlrDetectorTest, ParseControlFieldTrial) {
|
||||
"WebRTC-ProbingScreenshareBwe/Control/");
|
||||
absl::optional<AlrExperimentSettings> parsed_params =
|
||||
AlrExperimentSettings::CreateFromFieldTrial(
|
||||
"WebRTC-ProbingScreenshareBwe");
|
||||
FieldTrialBasedConfig(), "WebRTC-ProbingScreenshareBwe");
|
||||
EXPECT_FALSE(static_cast<bool>(parsed_params));
|
||||
}
|
||||
|
||||
@ -162,7 +165,7 @@ TEST_F(AlrDetectorTest, ParseActiveFieldTrial) {
|
||||
"WebRTC-ProbingScreenshareBwe/1.1,2875,85,20,-20,1/");
|
||||
absl::optional<AlrExperimentSettings> parsed_params =
|
||||
AlrExperimentSettings::CreateFromFieldTrial(
|
||||
"WebRTC-ProbingScreenshareBwe");
|
||||
FieldTrialBasedConfig(), "WebRTC-ProbingScreenshareBwe");
|
||||
ASSERT_TRUE(static_cast<bool>(parsed_params));
|
||||
EXPECT_EQ(1.1f, parsed_params->pacing_factor);
|
||||
EXPECT_EQ(2875, parsed_params->max_paced_queue_time);
|
||||
|
@ -105,7 +105,7 @@ GoogCcNetworkController::GoogCcNetworkController(
|
||||
: nullptr),
|
||||
bandwidth_estimation_(
|
||||
absl::make_unique<SendSideBandwidthEstimation>(event_log_)),
|
||||
alr_detector_(absl::make_unique<AlrDetector>()),
|
||||
alr_detector_(absl::make_unique<AlrDetector>(key_value_config_)),
|
||||
probe_bitrate_estimator_(new ProbeBitrateEstimator(event_log)),
|
||||
delay_based_bwe_(new DelayBasedBwe(key_value_config_,
|
||||
event_log_,
|
||||
|
@ -46,7 +46,6 @@ bool IsEnabled(const WebRtcKeyValueConfig& field_trials,
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
const int64_t PacedSender::kMaxQueueLengthMs = 2000;
|
||||
const float PacedSender::kDefaultPaceMultiplier = 2.5f;
|
||||
|
||||
@ -54,31 +53,23 @@ PacedSender::PacedSender(Clock* clock,
|
||||
PacketSender* packet_sender,
|
||||
RtcEventLog* event_log,
|
||||
const WebRtcKeyValueConfig* field_trials)
|
||||
: PacedSender(clock,
|
||||
packet_sender,
|
||||
event_log,
|
||||
field_trials
|
||||
? *field_trials
|
||||
: static_cast<const webrtc::WebRtcKeyValueConfig&>(
|
||||
FieldTrialBasedConfig())) {}
|
||||
|
||||
PacedSender::PacedSender(Clock* clock,
|
||||
PacketSender* packet_sender,
|
||||
RtcEventLog* event_log,
|
||||
const WebRtcKeyValueConfig& field_trials)
|
||||
: clock_(clock),
|
||||
packet_sender_(packet_sender),
|
||||
fallback_field_trials_(
|
||||
!field_trials ? absl::make_unique<FieldTrialBasedConfig>() : nullptr),
|
||||
field_trials_(field_trials ? field_trials : fallback_field_trials_.get()),
|
||||
alr_detector_(),
|
||||
drain_large_queues_(!IsDisabled(field_trials, "WebRTC-Pacer-DrainQueue")),
|
||||
drain_large_queues_(
|
||||
!IsDisabled(*field_trials_, "WebRTC-Pacer-DrainQueue")),
|
||||
send_padding_if_silent_(
|
||||
IsEnabled(field_trials, "WebRTC-Pacer-PadInSilence")),
|
||||
pace_audio_(!IsDisabled(field_trials, "WebRTC-Pacer-BlockAudio")),
|
||||
IsEnabled(*field_trials_, "WebRTC-Pacer-PadInSilence")),
|
||||
pace_audio_(!IsDisabled(*field_trials_, "WebRTC-Pacer-BlockAudio")),
|
||||
min_packet_limit_ms_("", kDefaultMinPacketLimitMs),
|
||||
last_timestamp_ms_(clock_->TimeInMilliseconds()),
|
||||
paused_(false),
|
||||
media_budget_(0),
|
||||
padding_budget_(0),
|
||||
prober_(field_trials),
|
||||
prober_(*field_trials_),
|
||||
probing_send_failure_(false),
|
||||
estimated_bitrate_bps_(0),
|
||||
min_send_bitrate_kbps_(0u),
|
||||
@ -97,7 +88,7 @@ PacedSender::PacedSender(Clock* clock,
|
||||
"pushback experiment must be enabled.";
|
||||
}
|
||||
ParseFieldTrial({&min_packet_limit_ms_},
|
||||
field_trials.Lookup("WebRTC-Pacer-MinPacketLimitMs"));
|
||||
field_trials_->Lookup("WebRTC-Pacer-MinPacketLimitMs"));
|
||||
UpdateBudgetWithElapsedTime(min_packet_limit_ms_);
|
||||
}
|
||||
|
||||
@ -184,7 +175,7 @@ void PacedSender::SetEstimatedBitrate(uint32_t bitrate_bps) {
|
||||
std::max(min_send_bitrate_kbps_, estimated_bitrate_bps_ / 1000) *
|
||||
pacing_factor_;
|
||||
if (!alr_detector_)
|
||||
alr_detector_ = absl::make_unique<AlrDetector>(nullptr /*event_log*/);
|
||||
alr_detector_ = absl::make_unique<AlrDetector>(field_trials_);
|
||||
alr_detector_->SetEstimatedBitrate(bitrate_bps);
|
||||
}
|
||||
|
||||
@ -248,7 +239,7 @@ int64_t PacedSender::ExpectedQueueTimeMs() const {
|
||||
absl::optional<int64_t> PacedSender::GetApplicationLimitedRegionStartTime() {
|
||||
rtc::CritScope cs(&critsect_);
|
||||
if (!alr_detector_)
|
||||
alr_detector_ = absl::make_unique<AlrDetector>(nullptr /*event_log*/);
|
||||
alr_detector_ = absl::make_unique<AlrDetector>(field_trials_);
|
||||
return alr_detector_->GetApplicationLimitedRegionStartTime();
|
||||
}
|
||||
|
||||
|
@ -150,11 +150,6 @@ class PacedSender : public Pacer {
|
||||
void SetQueueTimeLimit(int limit_ms);
|
||||
|
||||
private:
|
||||
PacedSender(Clock* clock,
|
||||
PacketSender* packet_sender,
|
||||
RtcEventLog* event_log,
|
||||
const WebRtcKeyValueConfig& field_trials);
|
||||
|
||||
int64_t UpdateTimeAndGetElapsedMs(int64_t now_us)
|
||||
RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_);
|
||||
bool ShouldSendKeepalive(int64_t at_time_us) const
|
||||
@ -179,6 +174,8 @@ class PacedSender : public Pacer {
|
||||
|
||||
Clock* const clock_;
|
||||
PacketSender* const packet_sender_;
|
||||
const std::unique_ptr<FieldTrialBasedConfig> fallback_field_trials_;
|
||||
const WebRtcKeyValueConfig* field_trials_;
|
||||
std::unique_ptr<AlrDetector> alr_detector_ RTC_PT_GUARDED_BY(critsect_);
|
||||
|
||||
const bool drain_large_queues_;
|
||||
|
@ -15,7 +15,8 @@ rtc_static_library("alr_experiment") {
|
||||
]
|
||||
deps = [
|
||||
"../:rtc_base_approved",
|
||||
"../../system_wrappers:field_trial",
|
||||
"../../api/transport:field_trial_based_config",
|
||||
"../../api/transport:webrtc_key_value_config",
|
||||
"//third_party/abseil-cpp/absl/types:optional",
|
||||
]
|
||||
}
|
||||
|
@ -14,8 +14,8 @@
|
||||
#include <stdio.h>
|
||||
#include <string>
|
||||
|
||||
#include "api/transport/field_trial_based_config.h"
|
||||
#include "rtc_base/logging.h"
|
||||
#include "system_wrappers/include/field_trial.h"
|
||||
|
||||
namespace webrtc {
|
||||
|
||||
@ -26,16 +26,29 @@ const char AlrExperimentSettings::kStrictPacingAndProbingExperimentName[] =
|
||||
const char kDefaultProbingScreenshareBweSettings[] = "1.0,2875,80,40,-60,3";
|
||||
|
||||
bool AlrExperimentSettings::MaxOneFieldTrialEnabled() {
|
||||
return field_trial::FindFullName(kStrictPacingAndProbingExperimentName)
|
||||
return AlrExperimentSettings::MaxOneFieldTrialEnabled(
|
||||
FieldTrialBasedConfig());
|
||||
}
|
||||
|
||||
bool AlrExperimentSettings::MaxOneFieldTrialEnabled(
|
||||
const WebRtcKeyValueConfig& key_value_config) {
|
||||
return key_value_config.Lookup(kStrictPacingAndProbingExperimentName)
|
||||
.empty() ||
|
||||
field_trial::FindFullName(kScreenshareProbingBweExperimentName)
|
||||
.empty();
|
||||
key_value_config.Lookup(kScreenshareProbingBweExperimentName).empty();
|
||||
}
|
||||
|
||||
absl::optional<AlrExperimentSettings>
|
||||
AlrExperimentSettings::CreateFromFieldTrial(const char* experiment_name) {
|
||||
return AlrExperimentSettings::CreateFromFieldTrial(FieldTrialBasedConfig(),
|
||||
experiment_name);
|
||||
}
|
||||
|
||||
absl::optional<AlrExperimentSettings>
|
||||
AlrExperimentSettings::CreateFromFieldTrial(
|
||||
const WebRtcKeyValueConfig& key_value_config,
|
||||
const char* experiment_name) {
|
||||
absl::optional<AlrExperimentSettings> ret;
|
||||
std::string group_name = field_trial::FindFullName(experiment_name);
|
||||
std::string group_name = key_value_config.Lookup(experiment_name);
|
||||
|
||||
const std::string kIgnoredSuffix = "_Dogfood";
|
||||
std::string::size_type suffix_pos = group_name.rfind(kIgnoredSuffix);
|
||||
|
@ -14,6 +14,7 @@
|
||||
#include <stdint.h>
|
||||
|
||||
#include "absl/types/optional.h"
|
||||
#include "api/transport/webrtc_key_value_config.h"
|
||||
|
||||
namespace webrtc {
|
||||
struct AlrExperimentSettings {
|
||||
@ -32,7 +33,12 @@ struct AlrExperimentSettings {
|
||||
static const char kStrictPacingAndProbingExperimentName[];
|
||||
static absl::optional<AlrExperimentSettings> CreateFromFieldTrial(
|
||||
const char* experiment_name);
|
||||
static absl::optional<AlrExperimentSettings> CreateFromFieldTrial(
|
||||
const WebRtcKeyValueConfig& key_value_config,
|
||||
const char* experiment_name);
|
||||
static bool MaxOneFieldTrialEnabled();
|
||||
static bool MaxOneFieldTrialEnabled(
|
||||
const WebRtcKeyValueConfig& key_value_config);
|
||||
|
||||
private:
|
||||
AlrExperimentSettings() = default;
|
||||
|
Reference in New Issue
Block a user