Upper limit pacer send bursts to about 63Kbyte
The purpose is to ensure send socket buffers are not overfilled at high
pacing rates.
Bug: chromium:1354491
Change-Id: Ic6f473080292f84a2a099b85fb5817f7e14e7355
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323000
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40911}
diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index 74def9c..54a2f7a 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 @@
// 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 2145868..0a138c1 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 @@
// 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 ade71cd..ba93d05 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 <algorithm>
-#include <list>
+#include <cstddef>
#include <memory>
-#include <string>
#include <utility>
#include <vector>
#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 @@
EXPECT_EQ(number_of_bursts, 4);
}
+TEST_F(PacingControllerTest,
+ MaxBurstSizeLimitedAtHighPacingRateWhenSendingPacketsInBursts) {
+ NiceMock<MockPacketSender> 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<RtpPacketToSend> 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);
