Make it possible to set the packet size needed to trigger a probe.
The value is today set to 200 which is too low for an audio packet to trigger sending probes.
For the initial probing, it would be good if audio packets, that may arrive before the first video frame can trigger sending a probe.
Also fix field trial parsing of required number of probes.
Bug: webrc:14392
Change-Id: I1f3cebcda38b71446e3602eef9cfa76de61a1ccf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275620
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38089}
diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc
index 00875d8..cd6334e 100644
--- a/modules/congestion_controller/goog_cc/probe_controller.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller.cc
@@ -110,7 +110,8 @@
&min_probe_duration, &network_state_estimate_probing_interval,
&network_state_estimate_fast_rampup_rate,
&network_state_estimate_drop_down_rate, &network_state_probe_scale,
- &network_state_probe_duration, &limit_probe_target_rate_to_loss_bwe},
+ &network_state_probe_duration, &min_probe_packets_sent,
+ &limit_probe_target_rate_to_loss_bwe},
key_value_config->Lookup("WebRTC-Bwe-ProbingConfiguration"));
// Specialized keys overriding subsets of WebRTC-Bwe-ProbingConfiguration
diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
index 63a3776..bf2235a 100644
--- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
@@ -470,7 +470,7 @@
ProbeControllerFixture fixture(
"WebRTC-Bwe-ProbingConfiguration/"
"p1:2,p2:5,step_size:3,further_probe_threshold:0.8,"
- "alloc_p1:2,alloc_p2/");
+ "alloc_p1:2,alloc_p2,min_probe_packets_sent:2/");
std::unique_ptr<ProbeController> probe_controller =
fixture.CreateController();
@@ -479,7 +479,9 @@
fixture.CurrentTime());
EXPECT_EQ(probes.size(), 2u);
EXPECT_EQ(probes[0].target_data_rate.bps(), 600);
+ EXPECT_EQ(probes[0].target_probe_count, 2);
EXPECT_EQ(probes[1].target_data_rate.bps(), 1500);
+ EXPECT_EQ(probes[1].target_probe_count, 2);
// Repeated probe should only be sent when estimated bitrate climbs above
// 0.8 * 5 * kStartBitrateBps = 1200.
diff --git a/modules/pacing/bitrate_prober.cc b/modules/pacing/bitrate_prober.cc
index 927e1c2..e01c3ae 100644
--- a/modules/pacing/bitrate_prober.cc
+++ b/modules/pacing/bitrate_prober.cc
@@ -23,11 +23,6 @@
namespace webrtc {
namespace {
-// 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.
-constexpr DataSize kMinProbePacketSize = DataSize::Bytes(200);
-
constexpr TimeDelta kProbeClusterTimeout = TimeDelta::Seconds(5);
} // namespace
@@ -35,8 +30,9 @@
BitrateProberConfig::BitrateProberConfig(
const FieldTrialsView* key_value_config)
: min_probe_delta("min_probe_delta", TimeDelta::Millis(2)),
- max_probe_delay("max_probe_delay", TimeDelta::Millis(10)) {
- ParseFieldTrial({&min_probe_delta, &max_probe_delay},
+ max_probe_delay("max_probe_delay", TimeDelta::Millis(10)),
+ min_packet_size("min_packet_size", DataSize::Bytes(200)) {
+ ParseFieldTrial({&min_probe_delta, &max_probe_delay, &min_packet_size},
key_value_config->Lookup("WebRTC-Bwe-ProbingBehavior"));
}
@@ -71,8 +67,11 @@
void BitrateProber::OnIncomingPacket(DataSize packet_size) {
// Don't initialize probing unless we have something large enough to start
// probing.
+ // Note that the pacer can send several packets at once when sending a probe,
+ // and thus, packets can be smaller than needed for a probe.
if (probing_state_ == ProbingState::kInactive && !clusters_.empty() &&
- packet_size >= std::min(RecommendedMinProbeSize(), kMinProbePacketSize)) {
+ packet_size >=
+ std::min(RecommendedMinProbeSize(), config_.min_packet_size.Get())) {
// Send next probe right away.
next_probe_time_ = Timestamp::MinusInfinity();
probing_state_ = ProbingState::kActive;
diff --git a/modules/pacing/bitrate_prober.h b/modules/pacing/bitrate_prober.h
index 222a28f..d2f1394 100644
--- a/modules/pacing/bitrate_prober.h
+++ b/modules/pacing/bitrate_prober.h
@@ -33,6 +33,11 @@
FieldTrialParameter<TimeDelta> min_probe_delta;
// Maximum amount of time each probe can be delayed.
FieldTrialParameter<TimeDelta> max_probe_delay;
+ // This is used to start sending a probe after a large enough packet.
+ // The min packet size is scaled with the bitrate we're probing at.
+ // This defines the max min packet size, meaning that on high bitrates
+ // a packet of at least this size is needed to trigger sending a probe.
+ FieldTrialParameter<DataSize> min_packet_size;
};
// Note that this class isn't thread-safe by itself and therefore relies
diff --git a/modules/pacing/bitrate_prober_unittest.cc b/modules/pacing/bitrate_prober_unittest.cc
index 1ebd337..00f84e6 100644
--- a/modules/pacing/bitrate_prober_unittest.cc
+++ b/modules/pacing/bitrate_prober_unittest.cc
@@ -146,14 +146,37 @@
TEST(BitrateProberTest, DoesntInitializeProbingForSmallPackets) {
const FieldTrialBasedConfig config;
BitrateProber prober(config);
-
prober.SetEnabled(true);
- EXPECT_FALSE(prober.is_probing());
+ ASSERT_FALSE(prober.is_probing());
+ prober.CreateProbeCluster({.at_time = Timestamp::Zero(),
+ .target_data_rate = DataRate::KilobitsPerSec(1000),
+ .target_duration = TimeDelta::Millis(15),
+ .target_probe_count = 5,
+ .id = 0});
prober.OnIncomingPacket(DataSize::Bytes(100));
+
EXPECT_FALSE(prober.is_probing());
}
+TEST(BitrateProberTest, DoesInitializeProbingForSmallPacketsIfConfigured) {
+ const test::ExplicitKeyValueConfig config(
+ "WebRTC-Bwe-ProbingBehavior/"
+ "min_packet_size:0bytes/");
+ BitrateProber prober(config);
+ prober.SetEnabled(true);
+ ASSERT_FALSE(prober.is_probing());
+
+ prober.CreateProbeCluster({.at_time = Timestamp::Zero(),
+ .target_data_rate = DataRate::KilobitsPerSec(1000),
+ .target_duration = TimeDelta::Millis(15),
+ .target_probe_count = 5,
+ .id = 0});
+ prober.OnIncomingPacket(DataSize::Bytes(10));
+
+ EXPECT_TRUE(prober.is_probing());
+}
+
TEST(BitrateProberTest, VerifyProbeSizeOnHighBitrate) {
const FieldTrialBasedConfig config;
BitrateProber prober(config);
