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> 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 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(&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()); +} + } // namespace } // namespace webrtc