From ce4829a04ab16beb692348b449e703b905f2f93f Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Fri, 15 Jun 2018 14:47:35 +0200 Subject: [PATCH] Adds trial to ignore video pacing for audio packets. This CL adds a field trial to ensure that audio packets are only blocked if they are also accounted for. Without the field trial active, audio packets are blocked due to full congestion windows and media budget overuse caused by video packets, even it the audio is not accounted for. Bug: webrtc:8415 Change-Id: I64c3507fcc6e91e6b0759e5f97b34d7f99492658 Reviewed-on: https://webrtc-review.googlesource.com/81187 Commit-Queue: Sebastian Jansson Reviewed-by: Stefan Holmer Cr-Commit-Position: refs/heads/master@{#23635} --- modules/pacing/paced_sender.cc | 12 ++-- modules/pacing/paced_sender.h | 1 + modules/pacing/paced_sender_unittest.cc | 75 +++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 883c33fbbe..00440e416e 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -65,6 +65,7 @@ PacedSender::PacedSender(const Clock* clock, alr_detector_(rtc::MakeUnique(event_log)), send_padding_if_silent_( field_trial::IsEnabled("WebRTC-Pacer-PadInSilence")), + video_blocks_audio_(!field_trial::IsDisabled("WebRTC-Pacer-BlockAudio")), paused_(false), media_budget_(rtc::MakeUnique(0)), padding_budget_(rtc::MakeUnique(0)), @@ -360,9 +361,12 @@ void PacedSender::ProcessThreadAttached(ProcessThread* process_thread) { bool PacedSender::SendPacket(const PacketQueueInterface::Packet& packet, const PacedPacketInfo& pacing_info) { RTC_DCHECK(!paused_); - if (Congested() || - (media_budget_->bytes_remaining() == 0 && - pacing_info.probe_cluster_id == PacedPacketInfo::kNotAProbe)) { + bool audio_packet = packet.priority == kHighPriority; + bool apply_pacing = + !audio_packet || account_for_audio_ || video_blocks_audio_; + if (apply_pacing && (Congested() || (media_budget_->bytes_remaining() == 0 && + pacing_info.probe_cluster_id == + PacedPacketInfo::kNotAProbe))) { return false; } @@ -375,7 +379,7 @@ bool PacedSender::SendPacket(const PacketQueueInterface::Packet& packet, if (success) { if (first_sent_packet_ms_ == -1) first_sent_packet_ms_ = clock_->TimeInMilliseconds(); - if (packet.priority != kHighPriority || account_for_audio_) { + if (!audio_packet || account_for_audio_) { // Update media bytes sent. // TODO(eladalon): TimeToSendPacket() can also return |true| in some // situations where nothing actually ended up being sent to the network, diff --git a/modules/pacing/paced_sender.h b/modules/pacing/paced_sender.h index 959f6d9606..8f02e785fd 100644 --- a/modules/pacing/paced_sender.h +++ b/modules/pacing/paced_sender.h @@ -163,6 +163,7 @@ class PacedSender : public Pacer { PacketSender* const packet_sender_; const std::unique_ptr alr_detector_ RTC_PT_GUARDED_BY(critsect_); const bool send_padding_if_silent_; + const bool video_blocks_audio_; rtc::CriticalSection critsect_; bool paused_ RTC_GUARDED_BY(critsect_); // This is the media budget, keeping track of how many bits of media diff --git a/modules/pacing/paced_sender_unittest.cc b/modules/pacing/paced_sender_unittest.cc index a8b9baed2e..fa55e216bc 100644 --- a/modules/pacing/paced_sender_unittest.cc +++ b/modules/pacing/paced_sender_unittest.cc @@ -199,6 +199,81 @@ TEST_F(PacedSenderFieldTrialTest, PaddingInSilenceWithTrial) { pacer.Process(); } +TEST_F(PacedSenderFieldTrialTest, DefaultCongestionWindowAffectsAudio) { + EXPECT_CALL(callback_, TimeToSendPadding).Times(0); + PacedSender pacer(&clock_, &callback_, nullptr); + pacer.SetPacingRates(10000000, 0); + pacer.SetCongestionWindow(800); + pacer.UpdateOutstandingData(0); + // Video packet fills congestion window. + InsertPacket(&pacer, &video); + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + ProcessNext(&pacer); + // Audio packet blocked due to congestion. + InsertPacket(&pacer, &audio); + EXPECT_CALL(callback_, TimeToSendPacket).Times(0); + ProcessNext(&pacer); + ProcessNext(&pacer); + // Audio packet unblocked when congestion window clear. + testing::Mock::VerifyAndClearExpectations(&callback_); + pacer.UpdateOutstandingData(0); + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + ProcessNext(&pacer); +} + +TEST_F(PacedSenderFieldTrialTest, CongestionWindowDoesNotAffectAudioInTrial) { + ScopedFieldTrials trial("WebRTC-Pacer-BlockAudio/Disabled/"); + EXPECT_CALL(callback_, TimeToSendPadding).Times(0); + PacedSender pacer(&clock_, &callback_, nullptr); + pacer.SetPacingRates(10000000, 0); + pacer.SetCongestionWindow(800); + pacer.UpdateOutstandingData(0); + // Video packet fills congestion window. + InsertPacket(&pacer, &video); + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + ProcessNext(&pacer); + // Audio not blocked due to congestion. + InsertPacket(&pacer, &audio); + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + ProcessNext(&pacer); +} + +TEST_F(PacedSenderFieldTrialTest, DefaultBudgetAffectsAudio) { + PacedSender pacer(&clock_, &callback_, nullptr); + pacer.SetPacingRates(video.packet_size / 3 * 8 * kProcessIntervalsPerSecond, + 0); + // Video fills budget for following process periods. + InsertPacket(&pacer, &video); + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + ProcessNext(&pacer); + // Audio packet blocked due to budget limit. + EXPECT_CALL(callback_, TimeToSendPacket).Times(0); + InsertPacket(&pacer, &audio); + ProcessNext(&pacer); + ProcessNext(&pacer); + testing::Mock::VerifyAndClearExpectations(&callback_); + // Audio packet unblocked when the budget has recovered. + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + ProcessNext(&pacer); + ProcessNext(&pacer); +} + +TEST_F(PacedSenderFieldTrialTest, BudgetDoesNotAffectAudioInTrial) { + ScopedFieldTrials trial("WebRTC-Pacer-BlockAudio/Disabled/"); + EXPECT_CALL(callback_, TimeToSendPadding).Times(0); + PacedSender pacer(&clock_, &callback_, nullptr); + pacer.SetPacingRates(video.packet_size / 3 * 8 * kProcessIntervalsPerSecond, + 0); + // Video fills budget for following process periods. + InsertPacket(&pacer, &video); + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + ProcessNext(&pacer); + // Audio packet not blocked due to budget limit. + EXPECT_CALL(callback_, TimeToSendPacket).WillOnce(Return(true)); + InsertPacket(&pacer, &audio); + ProcessNext(&pacer); +} + TEST_F(PacedSenderTest, FirstSentPacketTimeIsSet) { uint16_t sequence_number = 1234; const uint32_t kSsrc = 12345;