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 <srte@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23635}diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc
index 883c33f..00440e4 100644
--- a/modules/pacing/paced_sender.cc
+++ b/modules/pacing/paced_sender.cc
@@ -65,6 +65,7 @@
alr_detector_(rtc::MakeUnique<AlrDetector>(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<IntervalBudget>(0)),
padding_budget_(rtc::MakeUnique<IntervalBudget>(0)),
@@ -360,9 +361,12 @@
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 @@
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 959f6d9..8f02e78 100644
--- a/modules/pacing/paced_sender.h
+++ b/modules/pacing/paced_sender.h
@@ -163,6 +163,7 @@
PacketSender* const packet_sender_;
const std::unique_ptr<AlrDetector> 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 a8b9bae..fa55e21 100644
--- a/modules/pacing/paced_sender_unittest.cc
+++ b/modules/pacing/paced_sender_unittest.cc
@@ -199,6 +199,81 @@
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;
