Fix bug in dynamic pacer causing slightly inaccurate pacing rate.

When new packets are enqueued after a dead-period where media debt is
zero, that time slice should not be used to reduce the debt for the
new packet.

Bug: webrtc:10809
Change-Id: Ifb960548e6aa349b79f37743cbfed78a5c937a13
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/234081
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35143}
This commit is contained in:
Erik Språng 2021-10-05 10:17:39 +02:00 committed by WebRTC LUCI CQ
parent b8ffdc4bb3
commit 41bbc3df78
2 changed files with 48 additions and 5 deletions

View file

@ -297,10 +297,21 @@ void PacingController::EnqueuePacketInternal(
Timestamp now = CurrentTime();
if (mode_ == ProcessMode::kDynamic && packet_queue_.Empty() &&
NextSendTime() <= now) {
TimeDelta elapsed_time = UpdateTimeAndGetElapsed(now);
if (mode_ == ProcessMode::kDynamic && packet_queue_.Empty()) {
// If queue is empty, we need to "fast-forward" the last process time,
// so that we don't use passed time as budget for sending the first new
// packet.
Timestamp target_process_time = now;
Timestamp next_send_time = NextSendTime();
if (next_send_time.IsFinite()) {
// There was already a valid planned send time, such as a keep-alive.
// Use that as last process time only if it's prior to now.
target_process_time = std::min(now, next_send_time);
}
TimeDelta elapsed_time = UpdateTimeAndGetElapsed(target_process_time);
UpdateBudgetWithElapsedTime(elapsed_time);
last_process_time_ = target_process_time;
}
packet_queue_.Push(priority, now, packet_counter_++, std::move(packet));
}

View file

@ -252,8 +252,7 @@ class PacingControllerTest
Send(type, ssrc, sequence_number, capture_time_ms, size);
EXPECT_CALL(callback_,
SendPacket(ssrc, sequence_number, capture_time_ms,
type == RtpPacketMediaType::kRetransmission, false))
.Times(1);
type == RtpPacketMediaType::kRetransmission, false));
}
std::unique_ptr<RtpPacketToSend> BuildRtpPacket(RtpPacketMediaType type) {
@ -2135,6 +2134,39 @@ TEST_P(PacingControllerTest, SendsFecPackets) {
AdvanceTimeAndProcess();
}
TEST_P(PacingControllerTest, GapInPacingDoesntAccumulateBudget) {
if (PeriodicProcess()) {
// This test checks behavior when not using interval budget.
return;
}
const uint32_t kSsrc = 12345;
uint16_t sequence_number = 1234;
const DataSize kPackeSize = DataSize::Bytes(250);
const TimeDelta kPacketSendTime = TimeDelta::Millis(15);
pacer_->SetPacingRates(kPackeSize / kPacketSendTime,
/*padding_rate=*/DataRate::Zero());
// Send an initial packet.
SendAndExpectPacket(RtpPacketMediaType::kVideo, kSsrc, sequence_number++,
clock_.TimeInMilliseconds(), kPackeSize.bytes());
pacer_->ProcessPackets();
::testing::Mock::VerifyAndClearExpectations(&callback_);
// 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().
Send(RtpPacketMediaType::kVideo, kSsrc, sequence_number + 1,
clock_.TimeInMilliseconds(), kPackeSize.bytes());
Send(RtpPacketMediaType::kVideo, kSsrc, sequence_number + 2,
clock_.TimeInMilliseconds(), kPackeSize.bytes());
EXPECT_CALL(callback_, SendPacket(kSsrc, sequence_number + 1,
clock_.TimeInMilliseconds(), false, false));
pacer_->ProcessPackets();
}
INSTANTIATE_TEST_SUITE_P(
WithAndWithoutIntervalBudget,
PacingControllerTest,