diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 74def9c538..54a2f7a55e 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -17,6 +17,8 @@ #include "absl/cleanup/cleanup.h" #include "absl/strings/match.h" +#include "api/units/data_size.h" +#include "api/units/time_delta.h" #include "modules/pacing/bitrate_prober.h" #include "modules/pacing/interval_budget.h" #include "rtc_base/checks.h" @@ -343,9 +345,13 @@ Timestamp PacingController::NextSendTime() const { // debt is allowed to grow up to one packet more than what can be sent // during 'send_burst_period_'. TimeDelta drain_time = media_debt_ / adjusted_media_rate_; + // Ensure that a burst of sent packet is not larger than kMaxBurstSize in + // order to not risk overfilling socket buffers at high bitrate. + TimeDelta send_burst_interval = + std::min(send_burst_interval_, kMaxBurstSize / adjusted_media_rate_); next_send_time = last_process_time_ + - ((send_burst_interval_ > drain_time) ? TimeDelta::Zero() : drain_time); + ((send_burst_interval > drain_time) ? TimeDelta::Zero() : drain_time); } else if (padding_rate_ > DataRate::Zero() && packet_queue_.Empty()) { // If we _don't_ have pending packets, check how long until we have // bandwidth for padding packets. Both media and padding debts must diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h index 2145868a62..0a138c1a64 100644 --- a/modules/pacing/pacing_controller.h +++ b/modules/pacing/pacing_controller.h @@ -24,6 +24,7 @@ #include "api/function_view.h" #include "api/transport/field_trial_based_config.h" #include "api/transport/network_types.h" +#include "api/units/data_size.h" #include "modules/pacing/bitrate_prober.h" #include "modules/pacing/interval_budget.h" #include "modules/pacing/prioritized_packet_queue.h" @@ -86,6 +87,11 @@ class PacingController { // set to 1ms as this is intended to allow times be rounded down to the // nearest millisecond. static const TimeDelta kMaxEarlyProbeProcessing; + // Max total size of packets expected to be sent in a burst in order to not + // risk loosing packets due to too small send socket buffers. It upper limits + // the send burst interval. + // Ex: max send burst interval = 63Kb / 10Mbit/s = 50ms. + static constexpr DataSize kMaxBurstSize = DataSize::Bytes(63 * 1000); PacingController(Clock* clock, PacketSender* packet_sender, diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc index ade71cd5f5..ba93d05bb7 100644 --- a/modules/pacing/pacing_controller_unittest.cc +++ b/modules/pacing/pacing_controller_unittest.cc @@ -11,17 +11,16 @@ #include "modules/pacing/pacing_controller.h" #include -#include +#include #include -#include #include #include #include "api/transport/network_types.h" #include "api/units/data_rate.h" +#include "api/units/data_size.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" -#include "modules/pacing/packet_router.h" #include "system_wrappers/include/clock.h" #include "test/explicit_key_value_config.h" #include "test/gmock.h" @@ -30,6 +29,7 @@ using ::testing::_; using ::testing::AnyNumber; using ::testing::Field; +using ::testing::NiceMock; using ::testing::Pointee; using ::testing::Property; using ::testing::Return; @@ -2146,6 +2146,36 @@ TEST_F(PacingControllerTest, RespectsTargetRateWhenSendingPacketsInBursts) { EXPECT_EQ(number_of_bursts, 4); } +TEST_F(PacingControllerTest, + MaxBurstSizeLimitedAtHighPacingRateWhenSendingPacketsInBursts) { + NiceMock callback; + PacingController pacer(&clock_, &callback, trials_); + pacer.SetSendBurstInterval(TimeDelta::Millis(100)); + pacer.SetPacingRates(DataRate::KilobitsPerSec(10'000), DataRate::Zero()); + + size_t sent_size_in_burst = 0; + EXPECT_CALL(callback, SendPacket) + .WillRepeatedly([&](std::unique_ptr packet, + const PacedPacketInfo& cluster_info) { + sent_size_in_burst += packet->size(); + }); + + // Enqueue 200 packets from a 200Kb encoded frame. + for (int i = 0; i < 200; ++i) { + pacer.EnqueuePacket(video_.BuildNextPacket(1000)); + } + + while (pacer.QueueSizePackets() > 70) { + pacer.ProcessPackets(); + EXPECT_NEAR(sent_size_in_burst, PacingController::kMaxBurstSize.bytes(), + 1000); + sent_size_in_burst = 0; + TimeDelta time_to_next = pacer.NextSendTime() - clock_.CurrentTime(); + EXPECT_NEAR(time_to_next.ms(), 50, 2); + clock_.AdvanceTime(time_to_next); + } +} + TEST_F(PacingControllerTest, RespectsQueueTimeLimit) { static constexpr DataSize kPacketSize = DataSize::Bytes(100); static constexpr DataRate kNominalPacingRate = DataRate::KilobitsPerSec(200);