From 0920d5d344ee2024c6e8083f01f9d6334b189a38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 30 Mar 2020 17:14:08 +0200 Subject: [PATCH] Fixes TaskQueuePacedSender padding while only sending non-paced audio. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EnqueuePackets() would reset the last process time if the queue and media budgets were empty. This was done without reducing the padding debt. The result of this was that, given an existing debt, and an interval between audio packets that is less than the drain time for the padding debt, padding would not be sent at all. Now, before adding a new packet, we reduce the padding debt if the packet queue is empty. Bug: webrtc:10809 Change-Id: I116169522c215257febd32e17abab45f1a7d609f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171808 Reviewed-by: Sebastian Jansson Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#30949} --- modules/pacing/pacing_controller.cc | 5 +- modules/pacing/pacing_controller_unittest.cc | 89 ++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc index 4beb296c98..7b84877c04 100644 --- a/modules/pacing/pacing_controller.cc +++ b/modules/pacing/pacing_controller.cc @@ -284,8 +284,9 @@ void PacingController::EnqueuePacketInternal( } if (mode_ == ProcessMode::kDynamic && packet_queue_.Empty() && - media_debt_ == DataSize::Zero()) { - last_process_time_ = CurrentTime(); + media_debt_.IsZero()) { + TimeDelta elapsed_time = UpdateTimeAndGetElapsed(now); + UpdateBudgetWithElapsedTime(elapsed_time); } packet_queue_.Push(priority, now, packet_counter_++, std::move(packet)); } diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc index df12def517..811e697bca 100644 --- a/modules/pacing/pacing_controller_unittest.cc +++ b/modules/pacing/pacing_controller_unittest.cc @@ -1795,6 +1795,95 @@ TEST_P(PacingControllerTest, AudioNotPacedEvenWhenAccountedFor) { pacer_->ProcessPackets(); } +TEST_P(PacingControllerTest, PaddingAndAudioAfterVideoDisabled) { + const uint32_t kSsrc = 12345; + const DataRate kPacingDataRate = DataRate::KilobitsPerSec(125); + const DataRate kPaddingDataRate = DataRate::KilobitsPerSec(100); + const TimeDelta kMaxBufferInTime = TimeDelta::Millis(500); + const DataSize kPacketSize = DataSize::Bytes(130); + const TimeDelta kAudioPacketInterval = TimeDelta::Millis(20); + + // In this test, we fist send a burst of video in order to saturate the + // padding debt level. + // We then proceed to send audio at a bitrate that is slightly lower than + // the padding rate, meaning there will be a period with audio but no + // padding sent while the debt is draining, then audio and padding will + // be interlieved. + + // Verify both with and without accounting for audio. + for (bool account_for_audio : {false, true}) { + uint16_t sequence_number = 1234; + MockPacketSender callback; + EXPECT_CALL(callback, SendRtpPacket).Times(::testing::AnyNumber()); + pacer_ = std::make_unique(&clock_, &callback, nullptr, + nullptr, GetParam()); + pacer_->SetAccountForAudioPackets(account_for_audio); + + // First, saturate the padding budget. + pacer_->SetPacingRates(kPacingDataRate, kPaddingDataRate); + + const TimeDelta kPaddingSaturationTime = + kMaxBufferInTime * kPaddingDataRate / + (kPacingDataRate - kPaddingDataRate); + const DataSize kVideoToSend = kPaddingSaturationTime * kPacingDataRate; + const DataSize kVideoPacketSize = DataSize::Bytes(1200); + DataSize video_sent = DataSize::Zero(); + while (video_sent < kVideoToSend) { + pacer_->EnqueuePacket( + BuildPacket(RtpPacketMediaType::kVideo, kSsrc, sequence_number++, + clock_.TimeInMilliseconds(), kVideoPacketSize.bytes())); + video_sent += kVideoPacketSize; + } + while (pacer_->QueueSizePackets() > 0) { + AdvanceTimeAndProcess(); + } + + // Add a stream of audio packets at a rate slightly lower than the padding + // rate, once the padding debt is paid off we expect padding to be + // generated. + pacer_->SetPacingRates(kPacingDataRate, kPaddingDataRate); + bool padding_seen = false; + EXPECT_CALL(callback, GeneratePadding).WillOnce([&](DataSize padding_size) { + padding_seen = true; + std::vector> padding_packets; + padding_packets.emplace_back( + BuildPacket(RtpPacketMediaType::kPadding, kSsrc, sequence_number++, + clock_.TimeInMilliseconds(), padding_size.bytes())); + return padding_packets; + }); + + Timestamp start_time = clock_.CurrentTime(); + Timestamp last_audio_time = start_time; + while (!padding_seen) { + Timestamp now = clock_.CurrentTime(); + Timestamp next_send_time = pacer_->NextSendTime(); + TimeDelta sleep_time = + std::min(next_send_time, last_audio_time + kAudioPacketInterval) - + now; + clock_.AdvanceTime(sleep_time); + while (clock_.CurrentTime() >= last_audio_time + kAudioPacketInterval) { + pacer_->EnqueuePacket( + BuildPacket(RtpPacketMediaType::kAudio, kSsrc, sequence_number++, + clock_.TimeInMilliseconds(), kPacketSize.bytes())); + last_audio_time += kAudioPacketInterval; + } + pacer_->ProcessPackets(); + } + + // Verify how long it took to drain the padding debt. Allow 2% error margin. + const DataRate kAudioDataRate = kPacketSize / kAudioPacketInterval; + const TimeDelta expected_drain_time = + account_for_audio ? (kMaxBufferInTime * kPaddingDataRate / + (kPaddingDataRate - kAudioDataRate)) + : kMaxBufferInTime; + const TimeDelta actual_drain_time = clock_.CurrentTime() - start_time; + EXPECT_NEAR(actual_drain_time.ms(), expected_drain_time.ms(), + expected_drain_time.ms() * 0.02) + << " where account_for_audio = " + << (account_for_audio ? "true" : "false"); + } +} + INSTANTIATE_TEST_SUITE_P( WithAndWithoutIntervalBudget, PacingControllerTest,