Per default set PacingController burst interval to 40ms
PacingController per default use a burst interval of 40ms. The behaviour can still be overriden by using the method SetSendBurstInterval.
Bug: chromium:1354491
Change-Id: Ie3513109e88e9832dff47380c482ed6d943a2f2b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/311102
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41254}
diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h
index 8977137..920e291 100644
--- a/api/peer_connection_interface.h
+++ b/api/peer_connection_interface.h
@@ -688,6 +688,7 @@
PortAllocatorConfig port_allocator_config;
// The burst interval of the pacer, see TaskQueuePacedSender constructor.
+ // TODO(hbos): Deprecated, Remove once Chromium is not setting it.
absl::optional<TimeDelta> pacer_burst_interval;
//
diff --git a/call/call.cc b/call/call.cc
index 6b975ed..071b34e 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -453,7 +453,7 @@
bool is_started_ RTC_GUARDED_BY(worker_thread_) = false;
// Sequence checker for outgoing network traffic. Could be the network thread.
- // Could also be a pacer owned thread or TQ such as the TaskQueuePacedSender.
+ // Could also be a pacer owned thread or TQ such as the TaskQueueSender.
RTC_NO_UNIQUE_ADDRESS SequenceChecker sent_packet_sequence_checker_;
absl::optional<rtc::SentPacket> last_sent_packet_
RTC_GUARDED_BY(sent_packet_sequence_checker_);
diff --git a/call/call_config.cc b/call/call_config.cc
index 93f6b1a..23b60ce 100644
--- a/call/call_config.cc
+++ b/call/call_config.cc
@@ -31,7 +31,6 @@
network_state_predictor_factory;
transportConfig.task_queue_factory = task_queue_factory;
transportConfig.trials = trials;
- transportConfig.pacer_burst_interval = pacer_burst_interval;
return transportConfig;
}
diff --git a/call/call_config.h b/call/call_config.h
index 918c077..6b1c192 100644
--- a/call/call_config.h
+++ b/call/call_config.h
@@ -79,9 +79,6 @@
Metronome* metronome = nullptr;
- // The burst interval of the pacer, see TaskQueuePacedSender constructor.
- absl::optional<TimeDelta> pacer_burst_interval;
-
// Enables send packet batching from the egress RTP sender.
bool enable_send_packet_batching = false;
};
diff --git a/call/rtp_transport_config.h b/call/rtp_transport_config.h
index 6c94f7d..f2030b3 100644
--- a/call/rtp_transport_config.h
+++ b/call/rtp_transport_config.h
@@ -44,9 +44,6 @@
// Key-value mapping of internal configurations to apply,
// e.g. field trials.
const FieldTrialsView* trials = nullptr;
-
- // The burst interval of the pacer, see TaskQueuePacedSender constructor.
- absl::optional<TimeDelta> pacer_burst_interval;
};
} // namespace webrtc
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index 556a4dd..8d24f75 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -80,12 +80,7 @@
task_queue_(TaskQueueBase::Current()),
bitrate_configurator_(config.bitrate_config),
pacer_started_(false),
- pacer_(clock,
- &packet_router_,
- *config.trials,
- TimeDelta::Millis(5),
- 3,
- config.pacer_burst_interval),
+ pacer_(clock, &packet_router_, *config.trials, TimeDelta::Millis(5), 3),
observer_(nullptr),
controller_factory_override_(config.network_controller_factory),
controller_factory_fallback_(
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index cd2f1ef..9646a81 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -1092,7 +1092,7 @@
// Set a very low bitrate.
test.router()->OnBitrateUpdated(
- CreateBitrateAllocationUpdate(/*rate_bps=*/30'000),
+ CreateBitrateAllocationUpdate(/*rate_bps=*/10'000),
/*framerate=*/30);
// Create and send a large keyframe.
@@ -1119,7 +1119,7 @@
EXPECT_FALSE(packet.Marker());
}
EXPECT_GT(transmittedPayload, DataSize::Zero());
- EXPECT_LT(transmittedPayload, DataSize::Bytes(kImageSizeBytes / 4));
+ EXPECT_LT(transmittedPayload, DataSize::Bytes(kImageSizeBytes / 3));
// Record the RTP timestamp of the first frame.
const uint32_t first_frame_timestamp = sent_packets[0].Timestamp();
diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index 13ff9a2..5b81207 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -73,7 +73,7 @@
keyframe_flushing_(
IsEnabled(field_trials_, "WebRTC-Pacer-KeyframeFlushing")),
transport_overhead_per_packet_(DataSize::Zero()),
- send_burst_interval_(TimeDelta::Zero()),
+ send_burst_interval_(kDefaultBurstInterval),
last_timestamp_(clock_->CurrentTime()),
paused_(false),
media_debt_(DataSize::Zero()),
diff --git a/modules/pacing/pacing_controller.h b/modules/pacing/pacing_controller.h
index dd5636c..04e0a82 100644
--- a/modules/pacing/pacing_controller.h
+++ b/modules/pacing/pacing_controller.h
@@ -25,6 +25,7 @@
#include "api/transport/field_trial_based_config.h"
#include "api/transport/network_types.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 "modules/pacing/prioritized_packet_queue.h"
@@ -92,6 +93,10 @@
// the send burst interval.
// Ex: max send burst interval = 63Kb / 10Mbit/s = 50ms.
static constexpr DataSize kMaxBurstSize = DataSize::Bytes(63 * 1000);
+ // The pacer is allowed to send enqued packets in bursts and can build up a
+ // packet "debt" that correspond to approximately the send rate during
+ // the burst interval.
+ static constexpr TimeDelta kDefaultBurstInterval = TimeDelta::Millis(40);
PacingController(Clock* clock,
PacketSender* packet_sender,
diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc
index ba93d05..9e6ede6 100644
--- a/modules/pacing/pacing_controller_unittest.cc
+++ b/modules/pacing/pacing_controller_unittest.cc
@@ -427,6 +427,7 @@
DataRate pacing_rate =
DataRate::BitsPerSec(kPacketSize / 3 * 8 * kProcessIntervalsPerSecond);
pacer.SetPacingRates(pacing_rate, DataRate::Zero());
+ pacer.SetSendBurstInterval(TimeDelta::Zero());
// Video fills budget for following process periods.
pacer.EnqueuePacket(video_.BuildNextPacket(kPacketSize));
EXPECT_CALL(callback_, SendPacket).Times(1);
@@ -484,7 +485,7 @@
EXPECT_EQ(kStartTime, pacer->FirstSentPacketTime());
}
-TEST_F(PacingControllerTest, QueueAndPacePackets) {
+TEST_F(PacingControllerTest, QueueAndPacePacketsWithZeroBurstPeriod) {
const uint32_t kSsrc = 12345;
uint16_t sequence_number = 1234;
const DataSize kPackeSize = DataSize::Bytes(250);
@@ -495,6 +496,7 @@
const size_t kPacketsToSend = (kSendInterval * kTargetRate).bytes() *
kPaceMultiplier / kPackeSize.bytes();
auto pacer = std::make_unique<PacingController>(&clock_, &callback_, trials_);
+ pacer->SetSendBurstInterval(TimeDelta::Zero());
pacer->SetPacingRates(kTargetRate * kPaceMultiplier, DataRate::Zero());
for (size_t i = 0; i < kPacketsToSend; ++i) {
@@ -536,30 +538,30 @@
auto pacer = std::make_unique<PacingController>(&clock_, &callback_, trials_);
pacer->SetPacingRates(kTargetRate * kPaceMultiplier, DataRate::Zero());
- // Due to the multiplicative factor we can send 5 packets during a send
- // interval. (network capacity * multiplier / (8 bits per byte *
- // (packet size * #send intervals per second)
- const size_t packets_to_send_per_interval =
- kTargetRate.bps() * kPaceMultiplier / (8 * kPacketSize * 200);
- for (size_t i = 0; i < packets_to_send_per_interval; ++i) {
+ const size_t packets_to_send_per_burst_interval =
+ (kTargetRate * kPaceMultiplier * PacingController::kDefaultBurstInterval)
+ .bytes() /
+ kPacketSize;
+ for (size_t i = 0; i < packets_to_send_per_burst_interval; ++i) {
SendAndExpectPacket(pacer.get(), RtpPacketMediaType::kVideo, ssrc,
sequence_number++, clock_.TimeInMilliseconds(),
kPacketSize);
}
- for (size_t j = 0; j < packets_to_send_per_interval * 10; ++j) {
+ for (size_t j = 0; j < packets_to_send_per_burst_interval * 10; ++j) {
pacer->EnqueuePacket(BuildPacket(RtpPacketMediaType::kVideo, ssrc,
sequence_number++,
clock_.TimeInMilliseconds(), kPacketSize));
}
- EXPECT_EQ(packets_to_send_per_interval + packets_to_send_per_interval * 10,
+ EXPECT_EQ(packets_to_send_per_burst_interval +
+ packets_to_send_per_burst_interval * 10,
pacer->QueueSizePackets());
- while (pacer->QueueSizePackets() > packets_to_send_per_interval * 10) {
+ while (pacer->QueueSizePackets() > packets_to_send_per_burst_interval * 10) {
AdvanceTimeUntil(pacer->NextSendTime());
pacer->ProcessPackets();
}
- EXPECT_EQ(pacer->QueueSizePackets(), packets_to_send_per_interval * 10);
+ EXPECT_EQ(pacer->QueueSizePackets(), packets_to_send_per_burst_interval * 10);
EXPECT_CALL(callback_, SendPadding).Times(0);
EXPECT_CALL(callback_, SendPacket(ssrc, _, _, false, false))
@@ -582,12 +584,12 @@
pacer->ProcessPackets();
// Send some more packet, just show that we can..?
- for (size_t i = 0; i < packets_to_send_per_interval; ++i) {
+ for (size_t i = 0; i < packets_to_send_per_burst_interval; ++i) {
SendAndExpectPacket(pacer.get(), RtpPacketMediaType::kVideo, ssrc,
sequence_number++, clock_.TimeInMilliseconds(), 250);
}
- EXPECT_EQ(packets_to_send_per_interval, pacer->QueueSizePackets());
- for (size_t i = 0; i < packets_to_send_per_interval; ++i) {
+ EXPECT_EQ(packets_to_send_per_burst_interval, pacer->QueueSizePackets());
+ for (size_t i = 0; i < packets_to_send_per_burst_interval; ++i) {
AdvanceTimeUntil(pacer->NextSendTime());
pacer->ProcessPackets();
}
@@ -641,19 +643,23 @@
TEST_F(PacingControllerTest, Padding) {
uint32_t ssrc = 12345;
uint16_t sequence_number = 1234;
- const size_t kPacketSize = 250;
+ const size_t kPacketSize = 1000;
auto pacer = std::make_unique<PacingController>(&clock_, &callback_, trials_);
pacer->SetPacingRates(kTargetRate * kPaceMultiplier, kTargetRate);
- const size_t kPacketsToSend = 20;
+ const size_t kPacketsToSend = 30;
for (size_t i = 0; i < kPacketsToSend; ++i) {
SendAndExpectPacket(pacer.get(), RtpPacketMediaType::kVideo, ssrc,
sequence_number++, clock_.TimeInMilliseconds(),
kPacketSize);
}
+
+ int expected_bursts =
+ floor(DataSize::Bytes(pacer->QueueSizePackets() * kPacketSize) /
+ (kPaceMultiplier * kTargetRate) /
+ PacingController::kDefaultBurstInterval);
const TimeDelta expected_pace_time =
- DataSize::Bytes(pacer->QueueSizePackets() * kPacketSize) /
- (kPaceMultiplier * kTargetRate);
+ (expected_bursts - 1) * PacingController::kDefaultBurstInterval;
EXPECT_CALL(callback_, SendPadding).Times(0);
// Only the media packets should be sent.
Timestamp start_time = clock_.CurrentTime();
@@ -663,7 +669,7 @@
}
const TimeDelta actual_pace_time = clock_.CurrentTime() - start_time;
EXPECT_LE((actual_pace_time - expected_pace_time).Abs(),
- PacingController::kMinSleepTime);
+ PacingController::kDefaultBurstInterval);
// Pacing media happens at 2.5x, but padding was configured with 1.0x
// factor. We have to wait until the padding debt is gone before we start
@@ -766,8 +772,8 @@
media_payload));
media_bytes += media_payload;
}
-
- AdvanceTimeUntil(pacer->NextSendTime());
+ AdvanceTimeUntil(std::min(clock_.CurrentTime() + TimeDelta::Millis(20),
+ pacer->NextSendTime()));
pacer->ProcessPackets();
}
@@ -805,20 +811,18 @@
// Expect all high and normal priority to be sent out first.
EXPECT_CALL(callback_, SendPadding).Times(0);
+ testing::Sequence s;
EXPECT_CALL(callback_, SendPacket(ssrc, _, capture_time_ms, _, _))
- .Times(packets_to_send_per_interval + 1);
+ .Times(packets_to_send_per_interval + 1)
+ .InSequence(s);
+ EXPECT_CALL(callback_, SendPacket(ssrc_low_priority, _,
+ capture_time_ms_low_priority, _, _))
+ .InSequence(s);
- while (pacer->QueueSizePackets() > 1) {
+ while (pacer->QueueSizePackets() > 0) {
AdvanceTimeUntil(pacer->NextSendTime());
pacer->ProcessPackets();
}
-
- EXPECT_EQ(1u, pacer->QueueSizePackets());
-
- EXPECT_CALL(callback_, SendPacket(ssrc_low_priority, _,
- capture_time_ms_low_priority, _, _));
- AdvanceTimeUntil(pacer->NextSendTime());
- pacer->ProcessPackets();
}
TEST_F(PacingControllerTest, RetransmissionPriority) {
@@ -829,23 +833,22 @@
auto pacer = std::make_unique<PacingController>(&clock_, &callback_, trials_);
pacer->SetPacingRates(kTargetRate * kPaceMultiplier, DataRate::Zero());
- // Due to the multiplicative factor we can send 5 packets during a send
- // interval. (network capacity * multiplier / (8 bits per byte *
- // (packet size * #send intervals per second)
- const size_t packets_to_send_per_interval =
- kTargetRate.bps() * kPaceMultiplier / (8 * 250 * 200);
+ const size_t packets_to_send_per_burst_interval =
+ (kTargetRate * kPaceMultiplier * PacingController::kDefaultBurstInterval)
+ .bytes() /
+ 250;
pacer->ProcessPackets();
EXPECT_EQ(0u, pacer->QueueSizePackets());
// Alternate retransmissions and normal packets.
- for (size_t i = 0; i < packets_to_send_per_interval; ++i) {
+ for (size_t i = 0; i < packets_to_send_per_burst_interval; ++i) {
pacer->EnqueuePacket(BuildPacket(RtpPacketMediaType::kVideo, ssrc,
sequence_number++, capture_time_ms, 250));
pacer->EnqueuePacket(BuildPacket(RtpPacketMediaType::kRetransmission, ssrc,
sequence_number++,
capture_time_ms_retransmission, 250));
}
- EXPECT_EQ(2 * packets_to_send_per_interval, pacer->QueueSizePackets());
+ EXPECT_EQ(2 * packets_to_send_per_burst_interval, pacer->QueueSizePackets());
// Expect all retransmissions to be sent out first despite having a later
// capture time.
@@ -853,19 +856,19 @@
EXPECT_CALL(callback_, SendPacket(_, _, _, false, _)).Times(0);
EXPECT_CALL(callback_,
SendPacket(ssrc, _, capture_time_ms_retransmission, true, _))
- .Times(packets_to_send_per_interval);
+ .Times(packets_to_send_per_burst_interval);
- while (pacer->QueueSizePackets() > packets_to_send_per_interval) {
+ while (pacer->QueueSizePackets() > packets_to_send_per_burst_interval) {
AdvanceTimeUntil(pacer->NextSendTime());
pacer->ProcessPackets();
}
- EXPECT_EQ(packets_to_send_per_interval, pacer->QueueSizePackets());
+ EXPECT_EQ(packets_to_send_per_burst_interval, pacer->QueueSizePackets());
// Expect the remaining (non-retransmission) packets to be sent.
EXPECT_CALL(callback_, SendPadding).Times(0);
EXPECT_CALL(callback_, SendPacket(_, _, _, true, _)).Times(0);
EXPECT_CALL(callback_, SendPacket(ssrc, _, capture_time_ms, false, _))
- .Times(packets_to_send_per_interval);
+ .Times(packets_to_send_per_burst_interval);
while (pacer->QueueSizePackets() > 0) {
AdvanceTimeUntil(pacer->NextSendTime());
@@ -890,13 +893,13 @@
sequence_number++, capture_time_ms, kPacketSize);
}
pacer->ProcessPackets();
+ EXPECT_EQ(pacer->QueueSizePackets(), 0u);
// Low prio packets does affect the budget.
- // Due to the multiplicative factor we can send 5 packets during a send
- // interval. (network capacity * multiplier / (8 bits per byte *
- // (packet size * #send intervals per second)
- const size_t kPacketsToSendPerInterval =
- kTargetRate.bps() * kPaceMultiplier / (8 * kPacketSize * 200);
- for (size_t i = 0; i < kPacketsToSendPerInterval; ++i) {
+ const size_t kPacketsToSendPerBurstInterval =
+ (kTargetRate * kPaceMultiplier * PacingController::kDefaultBurstInterval)
+ .bytes() /
+ kPacketSize;
+ for (size_t i = 0; i < kPacketsToSendPerBurstInterval; ++i) {
SendAndExpectPacket(pacer.get(), RtpPacketMediaType::kVideo, ssrc,
sequence_number++, clock_.TimeInMilliseconds(),
kPacketSize);
@@ -904,16 +907,16 @@
// Send all packets and measure pace time.
Timestamp start_time = clock_.CurrentTime();
+ EXPECT_EQ(pacer->NextSendTime(), clock_.CurrentTime());
while (pacer->QueueSizePackets() > 0) {
AdvanceTimeUntil(pacer->NextSendTime());
pacer->ProcessPackets();
}
- // Measure pacing time. Expect only low-prio packets to affect this.
+ // Measure pacing time.
TimeDelta pacing_time = clock_.CurrentTime() - start_time;
- TimeDelta expected_pacing_time =
- DataSize::Bytes(kPacketsToSendPerInterval * kPacketSize) /
- (kTargetRate * kPaceMultiplier);
+ // All packets sent in one burst since audio packets are not accounted for.
+ TimeDelta expected_pacing_time = TimeDelta::Zero();
EXPECT_NEAR(pacing_time.us<double>(), expected_pacing_time.us<double>(),
PacingController::kMinSleepTime.us<double>());
}
@@ -965,6 +968,7 @@
auto now_ms = [this] { return clock_.TimeInMilliseconds(); };
auto pacer = std::make_unique<PacingController>(&clock_, &callback_, trials_);
pacer->SetPacingRates(kTargetRate * kPaceMultiplier, DataRate::Zero());
+ pacer->SetSendBurstInterval(TimeDelta::Zero());
EXPECT_CALL(callback_, SendPadding).Times(0);
// The pacing rate is low enough that the budget should not allow two packets
// to be sent in a row.
@@ -1853,6 +1857,7 @@
// Audio not paced, but still accounted for in budget.
pacer->SetAccountForAudioPackets(true);
pacer->SetPacingRates(kPacingDataRate, kPaddingDataRate);
+ pacer->SetSendBurstInterval(TimeDelta::Zero());
// Enqueue two audio packets, advance clock to where one packet
// should have drained the buffer already, has they been sent
@@ -1898,13 +1903,12 @@
EXPECT_EQ(pacer->NextSendTime() - clock_.CurrentTime(),
PacingController::kPausedProcessInterval);
- // Enqueue a new packet, that can't be sent until previous buffer has
- // drained.
+ // Enqueue a new packet, that can be sent immediately due to default burst
+ // rate is 40ms.
SendAndExpectPacket(pacer.get(), RtpPacketMediaType::kVideo, kSsrc,
sequnce_number++, clock_.TimeInMilliseconds(),
kPacketSize.bytes());
- EXPECT_EQ(pacer->NextSendTime() - clock_.CurrentTime(), kPacketPacingTime);
- clock_.AdvanceTime(kPacketPacingTime);
+ EXPECT_EQ(pacer->NextSendTime() - clock_.CurrentTime(), TimeDelta::Zero());
pacer->ProcessPackets();
::testing::Mock::VerifyAndClearExpectations(&callback_);
@@ -1916,11 +1920,13 @@
// previous debt has cleared. Since padding was disabled before, there
// currently is no padding debt.
pacer->SetPacingRates(kPacingDataRate, kPacingDataRate / 2);
- EXPECT_EQ(pacer->NextSendTime() - clock_.CurrentTime(), kPacketPacingTime);
+ EXPECT_EQ(pacer->QueueSizePackets(), 0u);
+ EXPECT_LT(pacer->NextSendTime() - clock_.CurrentTime(),
+ PacingController::kDefaultBurstInterval);
// Advance time, expect padding.
EXPECT_CALL(callback_, SendPadding).WillOnce(Return(kPacketSize.bytes()));
- clock_.AdvanceTime(kPacketPacingTime);
+ clock_.AdvanceTime(pacer->NextSendTime() - clock_.CurrentTime());
pacer->ProcessPackets();
::testing::Mock::VerifyAndClearExpectations(&callback_);
@@ -1933,7 +1939,7 @@
pacer->EnqueuePacket(
BuildPacket(RtpPacketMediaType::kVideo, kSsrc, sequnce_number++,
clock_.TimeInMilliseconds(), kPacketSize.bytes()));
- EXPECT_EQ(pacer->NextSendTime() - clock_.CurrentTime(), kPacketPacingTime);
+ EXPECT_EQ(pacer->NextSendTime(), clock_.CurrentTime());
}
TEST_F(PacingControllerTest, PaddingTargetAccountsForPaddingRate) {
@@ -2011,8 +2017,8 @@
TEST_F(PacingControllerTest, GapInPacingDoesntAccumulateBudget) {
const uint32_t kSsrc = 12345;
uint16_t sequence_number = 1234;
- const DataSize kPackeSize = DataSize::Bytes(250);
- const TimeDelta kPacketSendTime = TimeDelta::Millis(15);
+ const DataSize kPackeSize = DataSize::Bytes(1000);
+ const TimeDelta kPacketSendTime = TimeDelta::Millis(25);
auto pacer = std::make_unique<PacingController>(&clock_, &callback_, trials_);
pacer->SetPacingRates(kPackeSize / kPacketSendTime,
@@ -2028,15 +2034,20 @@
// Advance time kPacketSendTime past where the media debt should be 0.
clock_.AdvanceTime(2 * kPacketSendTime);
- // Enqueue two new packets. Expect only one to be sent one ProcessPackets().
+ // Enqueue three new packets. Expect only two to be sent one ProcessPackets()
+ // since the default burst interval is 40ms.
+ SendAndExpectPacket(pacer.get(), RtpPacketMediaType::kVideo, kSsrc,
+ sequence_number++, clock_.TimeInMilliseconds(),
+ kPackeSize.bytes());
+ SendAndExpectPacket(pacer.get(), RtpPacketMediaType::kVideo, kSsrc,
+ sequence_number++, clock_.TimeInMilliseconds(),
+ kPackeSize.bytes());
+ EXPECT_CALL(callback_, SendPacket(kSsrc, sequence_number + 1, _, _, _))
+ .Times(0);
pacer->EnqueuePacket(
BuildPacket(RtpPacketMediaType::kVideo, kSsrc, sequence_number + 1,
clock_.TimeInMilliseconds(), kPackeSize.bytes()));
- pacer->EnqueuePacket(
- BuildPacket(RtpPacketMediaType::kVideo, kSsrc, sequence_number + 2,
- clock_.TimeInMilliseconds(), kPackeSize.bytes()));
- EXPECT_CALL(callback_, SendPacket(kSsrc, sequence_number + 1,
- clock_.TimeInMilliseconds(), false, false));
+
pacer->ProcessPackets();
}
@@ -2044,6 +2055,7 @@
static constexpr DataSize kPacketSize = DataSize::Bytes(1);
static constexpr TimeDelta kPacketSendTime = TimeDelta::Micros(1);
auto pacer = std::make_unique<PacingController>(&clock_, &callback_, trials_);
+ pacer->SetSendBurstInterval(TimeDelta::Zero());
// Set pacing rate such that a packet is sent in 0.5us.
pacer->SetPacingRates(/*pacing_rate=*/2 * kPacketSize / kPacketSendTime,
diff --git a/modules/pacing/task_queue_paced_sender.cc b/modules/pacing/task_queue_paced_sender.cc
index afa36ea..f7218e4 100644
--- a/modules/pacing/task_queue_paced_sender.cc
+++ b/modules/pacing/task_queue_paced_sender.cc
@@ -17,35 +17,19 @@
#include "api/task_queue/pending_task_safety_flag.h"
#include "api/transport/network_types.h"
#include "rtc_base/checks.h"
-#include "rtc_base/experiments/field_trial_parser.h"
-#include "rtc_base/experiments/field_trial_units.h"
#include "rtc_base/trace_event.h"
namespace webrtc {
-namespace {
-
-constexpr const char* kBurstyPacerFieldTrial = "WebRTC-BurstyPacer";
-
-} // namespace
-
const int TaskQueuePacedSender::kNoPacketHoldback = -1;
-TaskQueuePacedSender::BurstyPacerFlags::BurstyPacerFlags(
- const FieldTrialsView& field_trials)
- : burst("burst") {
- ParseFieldTrial({&burst}, field_trials.Lookup(kBurstyPacerFieldTrial));
-}
-
TaskQueuePacedSender::TaskQueuePacedSender(
Clock* clock,
PacingController::PacketSender* packet_sender,
const FieldTrialsView& field_trials,
TimeDelta max_hold_back_window,
- int max_hold_back_window_in_packets,
- absl::optional<TimeDelta> burst_interval)
+ int max_hold_back_window_in_packets)
: clock_(clock),
- bursty_pacer_flags_(field_trials),
max_hold_back_window_(max_hold_back_window),
max_hold_back_window_in_packets_(max_hold_back_window_in_packets),
pacing_controller_(clock, packet_sender, field_trials),
@@ -56,17 +40,6 @@
include_overhead_(false),
task_queue_(TaskQueueBase::Current()) {
RTC_DCHECK_GE(max_hold_back_window_, PacingController::kMinSleepTime);
- // There are multiple field trials that can affect burst. If multiple bursts
- // are specified we pick the largest of the values.
- absl::optional<TimeDelta> burst = bursty_pacer_flags_.burst.GetOptional();
- // If not overriden by an experiment, the burst is specified by the
- // `burst_interval` argument.
- if (!burst.has_value()) {
- burst = burst_interval;
- }
- if (burst.has_value()) {
- pacing_controller_.SetSendBurstInterval(burst.value());
- }
}
TaskQueuePacedSender::~TaskQueuePacedSender() {
@@ -74,6 +47,11 @@
is_shutdown_ = true;
}
+void TaskQueuePacedSender::SetSendBurstInterval(TimeDelta burst_interval) {
+ RTC_DCHECK_RUN_ON(task_queue_);
+ pacing_controller_.SetSendBurstInterval(burst_interval);
+}
+
void TaskQueuePacedSender::EnsureStarted() {
RTC_DCHECK_RUN_ON(task_queue_);
is_started_ = true;
diff --git a/modules/pacing/task_queue_paced_sender.h b/modules/pacing/task_queue_paced_sender.h
index fd71be1..e29acdf 100644
--- a/modules/pacing/task_queue_paced_sender.h
+++ b/modules/pacing/task_queue_paced_sender.h
@@ -45,23 +45,21 @@
// processed. Increasing this reduces thread wakeups at the expense of higher
// latency.
//
- // If the `burst_interval` parameter is set, the pacer is allowed to build up
- // a packet "debt" that correspond to approximately the send rate during the
- // specified interval. This greatly reduced wake ups by not pacing packets
- // within the allowed burst budget.
- //
// The taskqueue used when constructing a TaskQueuePacedSender will also be
// used for pacing.
- TaskQueuePacedSender(
- Clock* clock,
- PacingController::PacketSender* packet_sender,
- const FieldTrialsView& field_trials,
- TimeDelta max_hold_back_window,
- int max_hold_back_window_in_packets,
- absl::optional<TimeDelta> burst_interval = absl::nullopt);
+ TaskQueuePacedSender(Clock* clock,
+ PacingController::PacketSender* packet_sender,
+ const FieldTrialsView& field_trials,
+ TimeDelta max_hold_back_window,
+ int max_hold_back_window_in_packets);
~TaskQueuePacedSender() override;
+ // The pacer is allowed to send enqued packets in bursts and can build up a
+ // packet "debt" that correspond to approximately the send rate during
+ // 'burst_interval'.
+ void SetSendBurstInterval(TimeDelta burst_interval);
+
// Ensure that necessary delayed tasks are scheduled.
void EnsureStarted();
@@ -145,15 +143,6 @@
Stats GetStats() const;
Clock* const clock_;
- struct BurstyPacerFlags {
- // Parses `kBurstyPacerFieldTrial`. Example:
- // --force-fieldtrials=WebRTC-BurstyPacer/burst:20ms/
- explicit BurstyPacerFlags(const FieldTrialsView& field_trials);
- // If set, the pacer is allowed to build up a packet "debt" that correspond
- // to approximately the send rate during the specified interval.
- FieldTrialOptional<TimeDelta> burst;
- };
- const BurstyPacerFlags bursty_pacer_flags_;
// The holdback window prevents too frequent delayed MaybeProcessPackets()
// calls. These are only applicable if `allow_low_precision` is false.
diff --git a/modules/pacing/task_queue_paced_sender_unittest.cc b/modules/pacing/task_queue_paced_sender_unittest.cc
index 5434749..f0a9ad7 100644
--- a/modules/pacing/task_queue_paced_sender_unittest.cc
+++ b/modules/pacing/task_queue_paced_sender_unittest.cc
@@ -11,6 +11,7 @@
#include "modules/pacing/task_queue_paced_sender.h"
#include <algorithm>
+#include <any>
#include <atomic>
#include <list>
#include <memory>
@@ -24,6 +25,7 @@
#include "api/units/data_rate.h"
#include "api/units/data_size.h"
#include "api/units/time_delta.h"
+#include "modules/pacing/pacing_controller.h"
#include "modules/pacing/packet_router.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "test/gmock.h"
@@ -33,6 +35,9 @@
using ::testing::_;
using ::testing::AtLeast;
+using ::testing::AtMost;
+using ::testing::Lt;
+using ::testing::NiceMock;
using ::testing::Return;
using ::testing::SaveArg;
@@ -167,9 +172,10 @@
TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
PacingController::kMinSleepTime,
- TaskQueuePacedSender::kNoPacketHoldback,
- // Half a second of bursting.
- TimeDelta::Seconds(0.5));
+ TaskQueuePacedSender::kNoPacketHoldback);
+ pacer.SetSendBurstInterval(
+ // Half a second of bursting.
+ TimeDelta::Seconds(0.5));
// Insert a number of packets, covering one second.
static constexpr size_t kPacketsToSend = 42;
@@ -262,7 +268,7 @@
TEST(TaskQueuePacedSenderTest, SendsAudioImmediately) {
GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234));
- MockPacketRouter packet_router;
+ NiceMock<MockPacketRouter> packet_router;
ScopedKeyValueConfig trials;
TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
@@ -270,21 +276,16 @@
TaskQueuePacedSender::kNoPacketHoldback);
const DataRate kPacingDataRate = DataRate::KilobitsPerSec(125);
- const DataSize kPacketSize = DataSize::Bytes(kDefaultPacketSize);
- const TimeDelta kPacketPacingTime = kPacketSize / kPacingDataRate;
pacer.SetPacingRates(kPacingDataRate, DataRate::Zero());
pacer.EnsureStarted();
- // Add some initial video packets, only one should be sent.
- EXPECT_CALL(packet_router, SendPacket);
+ // Add some initial video packets. Not all should be sent immediately.
+ EXPECT_CALL(packet_router, SendPacket).Times(AtMost(9));
pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 10));
time_controller.AdvanceTime(TimeDelta::Zero());
::testing::Mock::VerifyAndClearExpectations(&packet_router);
- // Advance time, but still before next packet should be sent.
- time_controller.AdvanceTime(kPacketPacingTime / 2);
-
// Insert an audio packet, it should be sent immediately.
EXPECT_CALL(packet_router, SendPacket);
pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kAudio, 1));
@@ -295,12 +296,13 @@
TEST(TaskQueuePacedSenderTest, SleepsDuringCoalscingWindow) {
const TimeDelta kCoalescingWindow = TimeDelta::Millis(5);
GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234));
- MockPacketRouter packet_router;
+ NiceMock<MockPacketRouter> packet_router;
ScopedKeyValueConfig trials;
TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
kCoalescingWindow,
TaskQueuePacedSender::kNoPacketHoldback);
+ pacer.SetSendBurstInterval(TimeDelta::Zero());
// Set rates so one packet adds one ms of buffer level.
const DataSize kPacketSize = DataSize::Bytes(kDefaultPacketSize);
@@ -310,9 +312,9 @@
pacer.SetPacingRates(kPacingDataRate, DataRate::Zero());
pacer.EnsureStarted();
- // Add 10 packets. The first should be sent immediately since the buffers
- // are clear.
- EXPECT_CALL(packet_router, SendPacket);
+ // Add 10 packets. The first burst should be sent immediately since the
+ // buffers are clear.
+ EXPECT_CALL(packet_router, SendPacket).Times(AtMost(9));
pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 10));
time_controller.AdvanceTime(TimeDelta::Zero());
::testing::Mock::VerifyAndClearExpectations(&packet_router);
@@ -370,11 +372,12 @@
ScopedKeyValueConfig trials(
"WebRTC-Bwe-ProbingBehavior/min_probe_delta:1ms/");
GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234));
- MockPacketRouter packet_router;
+ NiceMock<MockPacketRouter> packet_router;
TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
PacingController::kMinSleepTime,
TaskQueuePacedSender::kNoPacketHoldback);
+ pacer.SetSendBurstInterval(TimeDelta::Zero());
// Set rates so one packet adds 4ms of buffer level.
const DataSize kPacketSize = DataSize::Bytes(kDefaultPacketSize);
@@ -504,11 +507,12 @@
const int kPacketBasedHoldback = 5;
GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234));
- MockPacketRouter packet_router;
+ NiceMock<MockPacketRouter> packet_router;
ScopedKeyValueConfig trials;
TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
kFixedCoalescingWindow, kPacketBasedHoldback);
+ pacer.SetSendBurstInterval(TimeDelta::Zero());
// Set rates so one packet adds one ms of buffer level.
const DataSize kPacketSize = DataSize::Bytes(kDefaultPacketSize);
@@ -559,6 +563,7 @@
TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
kFixedCoalescingWindow, kPacketBasedHoldback);
+ pacer.SetSendBurstInterval(TimeDelta::Zero());
// Set rates so one packet adds one ms of buffer level.
const DataSize kPacketSize = DataSize::Bytes(kDefaultPacketSize);
@@ -691,7 +696,7 @@
TEST(TaskQueuePacedSenderTest, Stats) {
static constexpr Timestamp kStartTime = Timestamp::Millis(1234);
GlobalSimulatedTimeController time_controller(kStartTime);
- MockPacketRouter packet_router;
+ NiceMock<MockPacketRouter> packet_router;
ScopedKeyValueConfig trials;
TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
@@ -708,7 +713,8 @@
// Allowed `QueueSizeData` and `ExpectedQueueTime` deviation.
static constexpr size_t kAllowedPacketsDeviation = 1;
static constexpr DataSize kAllowedQueueSizeDeviation =
- DataSize::Bytes(kDefaultPacketSize * kAllowedPacketsDeviation);
+ DataSize::Bytes(kDefaultPacketSize * kAllowedPacketsDeviation) +
+ kPacingRate * PacingController::kDefaultBurstInterval;
static constexpr TimeDelta kAllowedQueueTimeDeviation =
kAllowedQueueSizeDeviation / kPacingRate;
diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc
index c656a4f..441c809 100644
--- a/pc/peer_connection_factory.cc
+++ b/pc/peer_connection_factory.cc
@@ -356,7 +356,6 @@
call_config.rtp_transport_controller_send_factory =
transport_controller_send_factory_.get();
call_config.metronome = metronome_.get();
- call_config.pacer_burst_interval = configuration.pacer_burst_interval;
return context_->call_factory()->CreateCall(call_config);
}
diff --git a/test/scenario/call_client.cc b/test/scenario/call_client.cc
index fdf36de..53dc5473 100644
--- a/test/scenario/call_client.cc
+++ b/test/scenario/call_client.cc
@@ -75,7 +75,6 @@
call_config.task_queue_factory = time_controller->GetTaskQueueFactory();
call_config.network_controller_factory = network_controller_factory;
call_config.audio_state = audio_state;
- call_config.pacer_burst_interval = config.pacer_burst_interval;
call_config.trials = config.field_trials;
Clock* clock = time_controller->GetClock();
return Call::Create(call_config, clock,
