Set default for field value "WebRTC-Bwe-ProbingBehavior/min_probe_delta:2ms"

And add a unit test that verifies that next probe time is set at  is
the expected if the recommended probe size is used.

Bug: webrtc:14392
Change-Id: I239bb3a1c8eefc85509aacc82037c64e3ce49ed7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272648
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37901}

modules/pacing/bitrate_prober.cc

@@ -34,7 +34,7 @@ constexpr TimeDelta kProbeClusterTimeout = TimeDelta::Seconds(5);
 
 BitrateProberConfig::BitrateProberConfig(
     const FieldTrialsView* key_value_config)
-    : min_probe_delta("min_probe_delta", TimeDelta::Millis(1)),
+    : 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},
                   key_value_config->Lookup("WebRTC-Bwe-ProbingBehavior"));
@@ -107,6 +107,7 @@ void BitrateProber::CreateProbeCluster(
                    << cluster.pace_info.send_bitrate_bps << ":"
                    << cluster.pace_info.probe_cluster_min_bytes << ":"
                    << cluster.pace_info.probe_cluster_min_probes << ")";
+
   // If we are already probing, continue to do so. Otherwise set it to
   // kInactive and wait for OnIncomingPacket to start the probing.
   if (probing_state_ != ProbingState::kActive)
@@ -145,16 +146,13 @@ absl::optional<PacedPacketInfo> BitrateProber::CurrentCluster(Timestamp now) {
   return info;
 }
 
-// Probe size is recommended based on the probe bitrate required. We choose
-// a minimum of twice `kMinProbeDeltaMs` interval to allow scheduling to be
-// feasible.
 DataSize BitrateProber::RecommendedMinProbeSize() const {
   if (clusters_.empty()) {
     return DataSize::Zero();
   }
   DataRate send_rate =
       DataRate::BitsPerSec(clusters_.front().pace_info.send_bitrate_bps);
-  return 2 * send_rate * config_.min_probe_delta;
+  return send_rate * config_.min_probe_delta;
 }
 
 void BitrateProber::ProbeSent(Timestamp now, DataSize size) {
@@ -197,6 +195,7 @@ Timestamp BitrateProber::CalculateNextProbeTime(
   DataSize sent_bytes = DataSize::Bytes(cluster.sent_bytes);
   DataRate send_bitrate =
       DataRate::BitsPerSec(cluster.pace_info.send_bitrate_bps);
+
   TimeDelta delta = sent_bytes / send_bitrate;
   return cluster.started_at + delta;
 }

modules/pacing/bitrate_prober.h

@@ -66,7 +66,8 @@ class BitrateProber {
   absl::optional<PacedPacketInfo> CurrentCluster(Timestamp now);
 
   // Returns the minimum number of bytes that the prober recommends for
-  // the next probe, or zero if not probing.
+  // the next probe, or zero if not probing. A probe can consist of multiple
+  // packets that are sent back to back.
   DataSize RecommendedMinProbeSize() const;
 
   // Called to report to the prober that a probe has been sent. In case of

modules/pacing/bitrate_prober_unittest.cc

@@ -170,6 +170,30 @@ TEST(BitrateProberTest, VerifyProbeSizeOnHighBitrate) {
             kHighBitrate * TimeDelta::Millis(1));
 }
 
+TEST(BitrateProberTest, ProbeSizeCanBeSetWithFieldTrial) {
+  const test::ExplicitKeyValueConfig trials(
+      "WebRTC-Bwe-ProbingBehavior/min_probe_delta:20ms/");
+  BitrateProber prober(trials);
+  prober.SetEnabled(true);
+
+  const DataRate kHighBitrate = DataRate::KilobitsPerSec(10000);  // 10 Mbps
+
+  prober.CreateProbeCluster({.at_time = Timestamp::Zero(),
+                             .target_data_rate = kHighBitrate,
+                             .target_duration = TimeDelta::Millis(15),
+                             .target_probe_count = 5,
+                             .id = 0});
+  EXPECT_EQ(prober.RecommendedMinProbeSize(),
+            kHighBitrate * TimeDelta::Millis(20));
+
+  prober.OnIncomingPacket(DataSize::Bytes(1000));
+  // Next time to send probe should be "min_probe_delta" if the recommended
+  // number of bytes has been sent.
+  prober.ProbeSent(Timestamp::Zero(), prober.RecommendedMinProbeSize());
+  EXPECT_EQ(prober.NextProbeTime(Timestamp::Zero()),
+            Timestamp::Zero() + TimeDelta::Millis(20));
+}
+
 TEST(BitrateProberTest, MinumumNumberOfProbingPackets) {
   const FieldTrialBasedConfig config;
   BitrateProber prober(config);

modules/pacing/task_queue_paced_sender_unittest.cc

@@ -459,9 +459,9 @@ TEST(TaskQueuePacedSenderTest, SchedulesProbeAtSentTime) {
 
 TEST(TaskQueuePacedSenderTest, NoMinSleepTimeWhenProbing) {
   // Set min_probe_delta to be less than kMinSleepTime (1ms).
-  const TimeDelta kMinProbeDelta = TimeDelta::Micros(100);
+  const TimeDelta kMinProbeDelta = TimeDelta::Micros(200);
   ScopedKeyValueConfig trials(
-      "WebRTC-Bwe-ProbingBehavior/min_probe_delta:100us/");
+      "WebRTC-Bwe-ProbingBehavior/min_probe_delta:200us/");
   GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234));
   MockPacketRouter packet_router;
   TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
@@ -495,7 +495,7 @@ TEST(TaskQueuePacedSenderTest, NoMinSleepTimeWhenProbing) {
 
   // Advance time less than PacingController::kMinSleepTime, probing packets
   // for the first millisecond should be sent immediately. Min delta between
-  // probes is 2x 100us, meaning 4 times per ms we will get least one call to
+  // probes is 200us, meaning 4 times per ms we will get least one call to
   // SendPacket().
   DataSize data_sent = DataSize::Zero();
   EXPECT_CALL(packet_router,
@@ -515,7 +515,7 @@ TEST(TaskQueuePacedSenderTest, NoMinSleepTimeWhenProbing) {
   // Verify the amount of probing data sent.
   // Probe always starts with a small (1 byte) padding packet that's not
   // counted into the probe rate here.
-  const DataSize kMinProbeSize = 2 * kMinProbeDelta * kProbingRate;
+  const DataSize kMinProbeSize = kMinProbeDelta * kProbingRate;
   EXPECT_EQ(data_sent, DataSize::Bytes(1) + kPacketSize + 4 * kMinProbeSize);
 }