Make some constants in the bitrate prober configurable.
This lets us change how many bytes and packets goes into the probes, as
well as some other things.
Bug: webrtc:10394
Change-Id: I26bb26a644e6f00366e9275228760c8744d63735
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/128424
Commit-Queue: Jonas Olsson <jonasolsson@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27312}diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc
index dfcb7f6..e5a6d1f 100644
--- a/modules/congestion_controller/goog_cc/probe_controller.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller.cc
@@ -73,9 +73,6 @@
// Never probe higher than configured by OnMaxTotalAllocatedBitrate().
constexpr char kCappedProbingFieldTrialName[] = "WebRTC-BweCappedProbing";
-constexpr char kConfigurableProbingFieldTrialName[] =
- "WebRTC-Bwe-ConfigurableProbing";
-
void MaybeLogProbeClusterCreated(RtcEventLog* event_log,
const ProbeClusterConfig& probe) {
RTC_DCHECK(event_log);
@@ -104,7 +101,7 @@
{&first_exponential_probe_scale_, &second_exponential_probe_scale_,
&further_exponential_probe_scale_, &further_probe_threshold,
&alr_probing_interval_, &alr_probe_scale_},
- key_value_config->Lookup(kConfigurableProbingFieldTrialName));
+ key_value_config->Lookup("WebRTC-Bwe-ProbingConfiguration"));
}
ProbeControllerConfig::ProbeControllerConfig(const ProbeControllerConfig&) =
diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
index 04dc440..7aba8e1 100644
--- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
@@ -314,7 +314,7 @@
TEST_F(ProbeControllerTest, ConfigurableProbingFieldTrial) {
test::ScopedFieldTrials trials(
- "WebRTC-Bwe-ConfigurableProbing/"
+ "WebRTC-Bwe-ProbingConfiguration/"
"p1:2,p2:5,step_size:3,further_probe_threshold:0.8/");
probe_controller_.reset(
new ProbeController(&field_trial_config_, &mock_rtc_event_log));
diff --git a/modules/pacing/bitrate_prober.cc b/modules/pacing/bitrate_prober.cc
index c81a18c..52f0bc1 100644
--- a/modules/pacing/bitrate_prober.cc
+++ b/modules/pacing/bitrate_prober.cc
@@ -23,20 +23,6 @@
namespace webrtc {
namespace {
-
-// A minimum interval between probes to allow scheduling to be feasible.
-constexpr int kMinProbeDeltaMs = 1;
-
-// The minimum number probing packets used.
-constexpr int kMinProbePacketsSent = 5;
-
-// The minimum probing duration in ms.
-constexpr int kMinProbeDurationMs = 15;
-
-// Maximum amount of time each probe can be delayed. Probe cluster is reset and
-// retried from the start when this limit is reached.
-constexpr int kMaxProbeDelayMs = 3;
-
// The min probe packet size is scaled with the bitrate we're probing at.
// This defines the max min probe packet size, meaning that on high bitrates
// we have a min probe packet size of 200 bytes.
@@ -46,7 +32,16 @@
} // namespace
-BitrateProber::BitrateProber() : BitrateProber(nullptr) {}
+BitrateProberConfig::BitrateProberConfig(
+ const WebRtcKeyValueConfig* key_value_config)
+ : min_probe_packets_sent("min_probe_packets_sent", 5),
+ min_probe_delta("min_probe_delta", TimeDelta::ms(1)),
+ min_probe_duration("min_probe_duration", TimeDelta::ms(15)),
+ max_probe_delay("max_probe_delay", TimeDelta::ms(3)) {
+ ParseFieldTrial({&min_probe_packets_sent, &min_probe_delta,
+ &min_probe_duration, &max_probe_delay},
+ key_value_config->Lookup("WebRTC-Bwe-ProbingConfiguration"));
+}
BitrateProber::~BitrateProber() {
RTC_HISTOGRAM_COUNTS_1000("WebRTC.BWE.Probing.TotalProbeClustersRequested",
@@ -55,12 +50,12 @@
total_failed_probe_count_);
}
-// TODO(psla): Remove this constructor in a follow up change.
-BitrateProber::BitrateProber(RtcEventLog* event_log)
+BitrateProber::BitrateProber(const WebRtcKeyValueConfig& field_trials)
: probing_state_(ProbingState::kDisabled),
next_probe_time_ms_(-1),
total_probe_count_(0),
- total_failed_probe_count_(0) {
+ total_failed_probe_count_(0),
+ config_(&field_trials) {
SetEnabled(true);
}
@@ -107,9 +102,10 @@
ProbeCluster cluster;
cluster.time_created_ms = now_ms;
- cluster.pace_info.probe_cluster_min_probes = kMinProbePacketsSent;
- cluster.pace_info.probe_cluster_min_bytes = static_cast<int32_t>(
- static_cast<int64_t>(bitrate_bps) * kMinProbeDurationMs / 8000);
+ cluster.pace_info.probe_cluster_min_probes = config_.min_probe_packets_sent;
+ cluster.pace_info.probe_cluster_min_bytes =
+ static_cast<int32_t>(static_cast<int64_t>(bitrate_bps) *
+ config_.min_probe_duration->ms() / 8000);
RTC_DCHECK_GE(cluster.pace_info.probe_cluster_min_bytes, 0);
cluster.pace_info.send_bitrate_bps = bitrate_bps;
cluster.pace_info.probe_cluster_id = cluster_id;
@@ -133,7 +129,7 @@
int time_until_probe_ms = 0;
if (next_probe_time_ms_ >= 0) {
time_until_probe_ms = next_probe_time_ms_ - now_ms;
- if (time_until_probe_ms < -kMaxProbeDelayMs) {
+ if (time_until_probe_ms < -config_.max_probe_delay->ms()) {
RTC_DLOG(LS_WARNING) << "Probe delay too high"
<< " (next_ms:" << next_probe_time_ms_
<< ", now_ms: " << now_ms << ")";
@@ -155,8 +151,8 @@
// feasible.
size_t BitrateProber::RecommendedMinProbeSize() const {
RTC_DCHECK(!clusters_.empty());
- return clusters_.front().pace_info.send_bitrate_bps * 2 * kMinProbeDeltaMs /
- (8 * 1000);
+ return clusters_.front().pace_info.send_bitrate_bps * 2 *
+ config_.min_probe_delta->ms() / (8 * 1000);
}
void BitrateProber::ProbeSent(int64_t now_ms, size_t bytes) {
diff --git a/modules/pacing/bitrate_prober.h b/modules/pacing/bitrate_prober.h
index e970eee..84b2c12 100644
--- a/modules/pacing/bitrate_prober.h
+++ b/modules/pacing/bitrate_prober.h
@@ -15,17 +15,35 @@
#include <stdint.h>
#include <queue>
+#include "api/transport/field_trial_based_config.h"
#include "api/transport/network_types.h"
+#include "rtc_base/experiments/field_trial_parser.h"
namespace webrtc {
class RtcEventLog;
+struct BitrateProberConfig {
+ explicit BitrateProberConfig(const WebRtcKeyValueConfig* key_value_config);
+ BitrateProberConfig(const BitrateProberConfig&) = default;
+ BitrateProberConfig& operator=(const BitrateProberConfig&) = default;
+ ~BitrateProberConfig() = default;
+
+ // The minimum number probing packets used.
+ FieldTrialParameter<int> min_probe_packets_sent;
+ // A minimum interval between probes to allow scheduling to be feasible.
+ FieldTrialParameter<TimeDelta> min_probe_delta;
+ // The minimum probing duration.
+ FieldTrialParameter<TimeDelta> min_probe_duration;
+ // Maximum amount of time each probe can be delayed. Probe cluster is reset
+ // and retried from the start when this limit is reached.
+ FieldTrialParameter<TimeDelta> max_probe_delay;
+};
+
// Note that this class isn't thread-safe by itself and therefore relies
// on being protected by the caller.
class BitrateProber {
public:
- BitrateProber();
- explicit BitrateProber(RtcEventLog* event_log);
+ explicit BitrateProber(const WebRtcKeyValueConfig& field_trials);
~BitrateProber();
void SetEnabled(bool enable);
@@ -101,6 +119,8 @@
int total_probe_count_;
int total_failed_probe_count_;
+
+ BitrateProberConfig config_;
};
} // namespace webrtc
diff --git a/modules/pacing/bitrate_prober_unittest.cc b/modules/pacing/bitrate_prober_unittest.cc
index 1770ac5..380d345 100644
--- a/modules/pacing/bitrate_prober_unittest.cc
+++ b/modules/pacing/bitrate_prober_unittest.cc
@@ -14,8 +14,10 @@
namespace webrtc {
TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) {
- BitrateProber prober;
+ const FieldTrialBasedConfig config;
+ BitrateProber prober(config);
EXPECT_FALSE(prober.IsProbing());
+
int64_t now_ms = 0;
EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms));
@@ -71,8 +73,9 @@
}
TEST(BitrateProberTest, DoesntProbeWithoutRecentPackets) {
- BitrateProber prober;
- EXPECT_FALSE(prober.IsProbing());
+ const FieldTrialBasedConfig config;
+ BitrateProber prober(config);
+
int64_t now_ms = 0;
EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms));
@@ -93,7 +96,9 @@
}
TEST(BitrateProberTest, DoesntInitializeProbingForSmallPackets) {
- BitrateProber prober;
+ const FieldTrialBasedConfig config;
+ BitrateProber prober(config);
+
prober.SetEnabled(true);
EXPECT_FALSE(prober.IsProbing());
@@ -102,7 +107,9 @@
}
TEST(BitrateProberTest, VerifyProbeSizeOnHighBitrate) {
- BitrateProber prober;
+ const FieldTrialBasedConfig config;
+ BitrateProber prober(config);
+
constexpr unsigned kHighBitrateBps = 10000000; // 10 Mbps
prober.CreateProbeCluster(kHighBitrateBps, 0, /*cluster_id=*/0);
@@ -111,7 +118,8 @@
}
TEST(BitrateProberTest, MinumumNumberOfProbingPackets) {
- BitrateProber prober;
+ const FieldTrialBasedConfig config;
+ BitrateProber prober(config);
// Even when probing at a low bitrate we expect a minimum number
// of packets to be sent.
constexpr int kBitrateBps = 100000; // 100 kbps
@@ -128,7 +136,8 @@
}
TEST(BitrateProberTest, ScaleBytesUsedForProbing) {
- BitrateProber prober;
+ const FieldTrialBasedConfig config;
+ BitrateProber prober(config);
constexpr int kBitrateBps = 10000000; // 10 Mbps
constexpr int kPacketSizeBytes = 1000;
constexpr int kExpectedBytesSent = kBitrateBps * 15 / 8000;
@@ -146,7 +155,8 @@
}
TEST(BitrateProberTest, HighBitrateProbing) {
- BitrateProber prober;
+ const FieldTrialBasedConfig config;
+ BitrateProber prober(config);
constexpr int kBitrateBps = 1000000000; // 1 Gbps.
constexpr int kPacketSizeBytes = 1000;
constexpr int kExpectedBytesSent = (kBitrateBps / 8000) * 15;
@@ -164,7 +174,8 @@
}
TEST(BitrateProberTest, ProbeClusterTimeout) {
- BitrateProber prober;
+ const FieldTrialBasedConfig config;
+ BitrateProber prober(config);
constexpr int kBitrateBps = 300000; // 300 kbps
constexpr int kSmallPacketSize = 20;
// Expecting two probe clusters of 5 packets each.
diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc
index 83dd01f..e6331f2 100644
--- a/modules/pacing/paced_sender.cc
+++ b/modules/pacing/paced_sender.cc
@@ -78,7 +78,7 @@
paused_(false),
media_budget_(0),
padding_budget_(0),
- prober_(event_log),
+ prober_(field_trials),
probing_send_failure_(false),
estimated_bitrate_bps_(0),
min_send_bitrate_kbps_(0u),