Makes dynamic pacer select paddig target based on rate.

Today when the pacing debt is cleared, we blindly ask for 50 bytes of
padding, which is above a static magic number for RTX payload padding.

Instead, we should adjust the target size based on the current padding
rate. The old pacer sort-of does this, it allows the budget to grow up
to one process interval (usually 5ms).
This CL makes the dynamic pacer also use a duration as target, by
default 5ms to match old pacer but with a trial to allow tweaking it.

This will be important for good behavior due to
https://bugs.chromium.org/p/webrtc/issues/detail?id=11508

Bug: webrtc:10809
Change-Id: I9c14acc5730c6e2e0d7821adf5fb058b8d5487c6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173687
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31091}

modules/pacing/pacing_controller.cc

@@ -20,6 +20,7 @@
 #include "modules/pacing/interval_budget.h"
 #include "modules/utility/include/process_thread.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/experiments/field_trial_parser.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/time_utils.h"
 #include "system_wrappers/include/clock.h"
@@ -33,10 +34,9 @@ constexpr TimeDelta kCongestedPacketInterval = TimeDelta::Millis(500);
 // The maximum debt level, in terms of time, capped when sending packets.
 constexpr TimeDelta kMaxDebtInTime = TimeDelta::Millis(500);
 constexpr TimeDelta kMaxElapsedTime = TimeDelta::Seconds(2);
-constexpr DataSize kDefaultPaddingTarget = DataSize::Bytes(50);
 
 // Upper cap on process interval, in case process has not been called in a long
-// time.
+// time. Applies only to periodic mode.
 constexpr TimeDelta kMaxProcessingInterval = TimeDelta::Millis(30);
 
 constexpr int kFirstPriority = 0;
@@ -51,6 +51,14 @@ bool IsEnabled(const WebRtcKeyValueConfig& field_trials,
   return absl::StartsWith(field_trials.Lookup(key), "Enabled");
 }
 
+TimeDelta GetDynamicPaddingTarget(const WebRtcKeyValueConfig& field_trials) {
+  FieldTrialParameter<TimeDelta> padding_target("timedelta",
+                                                TimeDelta::Millis(5));
+  ParseFieldTrial({&padding_target},
+                  field_trials.Lookup("WebRTC-Pacer-DynamicPaddingTarget"));
+  return padding_target.Get();
+}
+
 int GetPriorityForType(RtpPacketMediaType type) {
   // Lower number takes priority over higher.
   switch (type) {
@@ -102,6 +110,7 @@ PacingController::PacingController(Clock* clock,
           IsEnabled(*field_trials_, "WebRTC-Pacer-SmallFirstProbePacket")),
       ignore_transport_overhead_(
           IsEnabled(*field_trials_, "WebRTC-Pacer-IgnoreTransportOverhead")),
+      padding_target_duration_(GetDynamicPaddingTarget(*field_trials_)),
       min_packet_limit_(kDefaultMinPacketLimit),
       transport_overhead_per_packet_(DataSize::Zero()),
       last_timestamp_(clock_->CurrentTime()),
@@ -605,7 +614,7 @@ DataSize PacingController::PaddingToAdd(
     return DataSize::Bytes(padding_budget_.bytes_remaining());
   } else if (padding_rate_ > DataRate::Zero() &&
              padding_debt_ == DataSize::Zero()) {
-    return kDefaultPaddingTarget;
+    return padding_target_duration_ * padding_rate_;
   }
   return DataSize::Zero();
 }

modules/pacing/pacing_controller.h

@@ -181,6 +181,9 @@ class PacingController {
   const bool pace_audio_;
   const bool small_first_probe_packet_;
   const bool ignore_transport_overhead_;
+  // In dynamic mode, indicates the target size when requesting padding,
+  // expressed as a duration in order to adjust for varying padding rate.
+  const TimeDelta padding_target_duration_;
 
   TimeDelta min_packet_limit_;
 

modules/pacing/pacing_controller_unittest.cc

@@ -184,7 +184,9 @@ class PacingControllerProbing : public PacingController::PacketSender {
 class PacingControllerTest
     : public ::testing::TestWithParam<PacingController::ProcessMode> {
  protected:
-  PacingControllerTest() : clock_(123456) {
+  PacingControllerTest() : clock_(123456) {}
+
+  void SetUp() override {
     srand(0);
     // Need to initialize PacingController after we initialize clock.
     pacer_ = std::make_unique<PacingController>(&clock_, &callback_, nullptr,
@@ -1983,6 +1985,46 @@ TEST_P(PacingControllerTest, NextSendTimeAccountsForPadding) {
   EXPECT_EQ(pacer_->NextSendTime() - clock_.CurrentTime(), kPacketPacingTime);
 }
 
+TEST_P(PacingControllerTest, PaddingTargetAccountsForPaddingRate) {
+  if (PeriodicProcess()) {
+    // This test applies only when NOT using interval budget.
+    return;
+  }
+
+  // Re-init pacer with an explicitly set padding target of 10ms;
+  const TimeDelta kPaddingTarget = TimeDelta::Millis(10);
+  ScopedFieldTrials field_trials(
+      "WebRTC-Pacer-DynamicPaddingTarget/timedelta:10ms/");
+  SetUp();
+
+  const uint32_t kSsrc = 12345;
+  const DataRate kPacingDataRate = DataRate::KilobitsPerSec(125);
+  const DataSize kPacketSize = DataSize::Bytes(130);
+
+  uint32_t sequnce_number = 1;
+
+  // Start with pacing and padding rate equal.
+  pacer_->SetPacingRates(kPacingDataRate, kPacingDataRate);
+
+  // Send a single packet.
+  SendAndExpectPacket(RtpPacketMediaType::kVideo, kSsrc, sequnce_number++,
+                      clock_.TimeInMilliseconds(), kPacketSize.bytes());
+  AdvanceTimeAndProcess();
+  ::testing::Mock::VerifyAndClearExpectations(&callback_);
+
+  size_t expected_padding_target_bytes =
+      (kPaddingTarget * kPacingDataRate).bytes();
+  EXPECT_CALL(callback_, SendPadding(expected_padding_target_bytes))
+      .WillOnce(Return(expected_padding_target_bytes));
+  AdvanceTimeAndProcess();
+
+  // Half the padding rate - expect half the padding target.
+  pacer_->SetPacingRates(kPacingDataRate, kPacingDataRate / 2);
+  EXPECT_CALL(callback_, SendPadding(expected_padding_target_bytes / 2))
+      .WillOnce(Return(expected_padding_target_bytes / 2));
+  AdvanceTimeAndProcess();
+}
+
 INSTANTIATE_TEST_SUITE_P(
     WithAndWithoutIntervalBudget,
     PacingControllerTest,