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}
This commit is contained in:
Per K 2023-10-11 10:32:39 +02:00 committed by WebRTC LUCI CQ
parent dde1cb6212
commit 0b554e7004
3 changed files with 46 additions and 4 deletions

View File

@ -17,6 +17,8 @@
#include "absl/cleanup/cleanup.h" #include "absl/cleanup/cleanup.h"
#include "absl/strings/match.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/bitrate_prober.h"
#include "modules/pacing/interval_budget.h" #include "modules/pacing/interval_budget.h"
#include "rtc_base/checks.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 // debt is allowed to grow up to one packet more than what can be sent
// during 'send_burst_period_'. // during 'send_burst_period_'.
TimeDelta drain_time = media_debt_ / adjusted_media_rate_; 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 = next_send_time =
last_process_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()) { } else if (padding_rate_ > DataRate::Zero() && packet_queue_.Empty()) {
// If we _don't_ have pending packets, check how long until we have // If we _don't_ have pending packets, check how long until we have
// bandwidth for padding packets. Both media and padding debts must // bandwidth for padding packets. Both media and padding debts must

View File

@ -24,6 +24,7 @@
#include "api/function_view.h" #include "api/function_view.h"
#include "api/transport/field_trial_based_config.h" #include "api/transport/field_trial_based_config.h"
#include "api/transport/network_types.h" #include "api/transport/network_types.h"
#include "api/units/data_size.h"
#include "modules/pacing/bitrate_prober.h" #include "modules/pacing/bitrate_prober.h"
#include "modules/pacing/interval_budget.h" #include "modules/pacing/interval_budget.h"
#include "modules/pacing/prioritized_packet_queue.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 // set to 1ms as this is intended to allow times be rounded down to the
// nearest millisecond. // nearest millisecond.
static const TimeDelta kMaxEarlyProbeProcessing; 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, PacingController(Clock* clock,
PacketSender* packet_sender, PacketSender* packet_sender,

View File

@ -11,17 +11,16 @@
#include "modules/pacing/pacing_controller.h" #include "modules/pacing/pacing_controller.h"
#include <algorithm> #include <algorithm>
#include <list> #include <cstddef>
#include <memory> #include <memory>
#include <string>
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "api/transport/network_types.h" #include "api/transport/network_types.h"
#include "api/units/data_rate.h" #include "api/units/data_rate.h"
#include "api/units/data_size.h"
#include "api/units/time_delta.h" #include "api/units/time_delta.h"
#include "api/units/timestamp.h" #include "api/units/timestamp.h"
#include "modules/pacing/packet_router.h"
#include "system_wrappers/include/clock.h" #include "system_wrappers/include/clock.h"
#include "test/explicit_key_value_config.h" #include "test/explicit_key_value_config.h"
#include "test/gmock.h" #include "test/gmock.h"
@ -30,6 +29,7 @@
using ::testing::_; using ::testing::_;
using ::testing::AnyNumber; using ::testing::AnyNumber;
using ::testing::Field; using ::testing::Field;
using ::testing::NiceMock;
using ::testing::Pointee; using ::testing::Pointee;
using ::testing::Property; using ::testing::Property;
using ::testing::Return; using ::testing::Return;
@ -2146,6 +2146,36 @@ TEST_F(PacingControllerTest, RespectsTargetRateWhenSendingPacketsInBursts) {
EXPECT_EQ(number_of_bursts, 4); 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) { TEST_F(PacingControllerTest, RespectsQueueTimeLimit) {
static constexpr DataSize kPacketSize = DataSize::Bytes(100); static constexpr DataSize kPacketSize = DataSize::Bytes(100);
static constexpr DataRate kNominalPacingRate = DataRate::KilobitsPerSec(200); static constexpr DataRate kNominalPacingRate = DataRate::KilobitsPerSec(200);