Avoid replaying excessive padding generation.

If the real-time clock makes a sudden and large shift forward while
there is at least one packet in the queue, the pacer would previously
try to "replay" the behavior it would have done while it was asleep.
This can lead to grand bursts of padding packets, which is
undesireable.
This CL mitigates this problem by forwarding the internal state clock
to (now - 50ms) if there is nothing to do but generate padding.

Bug: webrtc:11340, b/258509536
Change-Id: I5b8a130d938dd2566f72c1946c139f50e099e5a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285380
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38752}

3 files changed, 50 insertions, 5 deletions

diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index a6c56faea5..ca248748e6 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -33,7 +33,6 @@ 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 TimeDelta kTargetPaddingDuration = TimeDelta::Millis(5);
 
 bool IsDisabled(const FieldTrialsView& field_trials, absl::string_view key) {
   return absl::StartsWith(field_trials.Lookup(key), "Disabled");
@@ -50,6 +49,9 @@ const TimeDelta PacingController::kMaxExpectedQueueLength =
 const TimeDelta PacingController::kPausedProcessInterval =
     kCongestedPacketInterval;
 const TimeDelta PacingController::kMinSleepTime = TimeDelta::Millis(1);
+const TimeDelta PacingController::kTargetPaddingDuration = TimeDelta::Millis(5);
+const TimeDelta PacingController::kMaxPaddingReplayDuration =
+    TimeDelta::Millis(50);
 const TimeDelta PacingController::kMaxEarlyProbeProcessing =
     TimeDelta::Millis(1);
 
@@ -438,6 +440,14 @@ void PacingController::ProcessPackets() {
         GetPendingPacket(pacing_info, target_send_time, now);
     if (rtp_packet == nullptr) {
       // No packet available to send, check if we should send padding.
+      if (now - target_send_time > kMaxPaddingReplayDuration) {
+        // The target send time is more than `kMaxPaddingReplayDuration` behind
+        // the real-time clock. This can happen if the clock is adjusted forward
+        // without `ProcessPackets()` having been called at the expected times.
+        target_send_time = now - kMaxPaddingReplayDuration;
+        last_process_time_ = std::max(last_process_time_, target_send_time);
+      }
+
       DataSize padding_to_add = PaddingToAdd(recommended_probe_size, data_sent);
       if (padding_to_add > DataSize::Zero()) {
         std::vector<std::unique_ptr<RtpPacketToSend>> padding_packets =
diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h
index 40b22b166c..eb4fb4ec86 100644
--- a/modules/pacing/pacing_controller.h
+++ b/modules/pacing/pacing_controller.h
@@ -72,9 +72,14 @@ class PacingController {
   // order to send a keep-alive packet so we don't get stuck in a bad state due
   // to lack of feedback.
   static const TimeDelta kPausedProcessInterval;
-
+  // The default minimum time that should elapse calls to `ProcessPackets()`.
   static const TimeDelta kMinSleepTime;
-
+  // When padding should be generated, add packets to the buffer with a size
+  // corresponding to this duration times the current padding rate.
+  static const TimeDelta kTargetPaddingDuration;
+  // The maximum time that the pacer can use when "replaying" passed time where
+  // padding should have been generated.
+  static const TimeDelta kMaxPaddingReplayDuration;
   // Allow probes to be processed slightly ahead of inteded send time. Currently
   // set to 1ms as this is intended to allow times be rounded down to the
   // nearest millisecond.
diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc
index 6c26c79329..3b3c3eb761 100644
--- a/modules/pacing/pacing_controller_unittest.cc
+++ b/modules/pacing/pacing_controller_unittest.cc
@@ -27,6 +27,7 @@
 #include "test/gtest.h"
 
 using ::testing::_;
+using ::testing::AnyNumber;
 using ::testing::Field;
 using ::testing::Pointee;
 using ::testing::Property;
@@ -1520,7 +1521,7 @@ TEST_F(PacingControllerTest, SmallFirstProbePacket) {
   size_t packets_sent = 0;
   bool media_seen = false;
   EXPECT_CALL(callback, SendPacket)
-      .Times(::testing::AnyNumber())
+      .Times(AnyNumber())
       .WillRepeatedly([&](std::unique_ptr<RtpPacketToSend> packet,
                           const PacedPacketInfo& cluster_info) {
         if (packets_sent == 0) {
@@ -1674,7 +1675,7 @@ TEST_F(PacingControllerTest,
   for (bool account_for_audio : {false, true}) {
     uint16_t sequence_number = 1234;
     MockPacketSender callback;
-    EXPECT_CALL(callback, SendPacket).Times(::testing::AnyNumber());
+    EXPECT_CALL(callback, SendPacket).Times(AnyNumber());
     auto pacer =
         std::make_unique<PacingController>(&clock_, &callback, trials_);
     pacer->SetAccountForAudioPackets(account_for_audio);
@@ -2141,5 +2142,34 @@ TEST_F(PacingControllerTest, AbortsAfterReachingCircuitBreakLimit) {
   pacer.ProcessPackets();
 }
 
+TEST_F(PacingControllerTest, DoesNotPadIfProcessThreadIsBorked) {
+  PacingControllerPadding callback;
+  PacingController pacer(&clock_, &callback, trials_);
+
+  // Set both pacing and padding rate to be non-zero.
+  pacer.SetPacingRates(kTargetRate, /*padding_rate=*/kTargetRate);
+
+  // Add one packet to the queue, but do not send it yet.
+  pacer.EnqueuePacket(BuildPacket(RtpPacketMediaType::kVideo, kVideoSsrc,
+                                  /*sequence_number=*/1,
+                                  /*capture_time=*/1,
+                                  /*size=*/1000));
+
+  // Advance time to waaay after the packet should have been sent.
+  clock_.AdvanceTime(TimeDelta::Seconds(42));
+
+  // `ProcessPackets()` should send the delayed packet, followed by a small
+  // amount of missed padding.
+  pacer.ProcessPackets();
+
+  // The max padding window is the max replay duration + the target padding
+  // duration.
+  const DataSize kMaxPadding = (PacingController::kMaxPaddingReplayDuration +
+                                PacingController::kTargetPaddingDuration) *
+                               kTargetRate;
+
+  EXPECT_LE(callback.padding_sent(), kMaxPadding.bytes<size_t>());
+}
+
 }  // namespace
 }  // namespace webrtc