From 9cb58d5d46d7ae1079ff57a7586336a77d300b8e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= <sprang@webrtc.org>
Date: Sat, 28 Mar 2020 17:15:54 +0100
Subject: [PATCH] Fixes issue where dynamic pacer could pace audio.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Specifically, if dynamic pacer (i.e. TaskQueuePacer) was enabled while
AccountForAudio was set to true, the pacer would pace audio packets.
This should only happen when the WebRTC-Pacer-BlockAudio field trial is
enabled.

Bug: webrtc:10809
Change-Id: If5edc77de88ca9866abeb3b47e171df50673299e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172082
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30938}
---
 modules/pacing/pacing_controller.cc          | 10 +++----
 modules/pacing/pacing_controller_unittest.cc | 28 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/modules/pacing/pacing_controller.cc b/modules/pacing/pacing_controller.cc
index 70f39f591c..4beb296c98 100644
--- a/modules/pacing/pacing_controller.cc
+++ b/modules/pacing/pacing_controller.cc
@@ -343,13 +343,11 @@ Timestamp PacingController::NextSendTime() const {
   // In dynamic mode, figure out when the next packet should be sent,
   // given the current conditions.
 
-  if (Congested() || packet_counter_ == 0) {
-    // If congested, we only send keep-alive or audio (if audio is
-    // configured in pass-through mode).
-    if (!pace_audio_ && packet_queue_.NextPacketIsAudio()) {
-      return now;
-    }
+  if (!pace_audio_ && packet_queue_.NextPacketIsAudio()) {
+    return now;
+  }
 
+  if (Congested() || packet_counter_ == 0) {
     // We need to at least send keep-alive packets with some interval.
     return last_send_time_ + kCongestedPacketInterval;
   }
diff --git a/modules/pacing/pacing_controller_unittest.cc b/modules/pacing/pacing_controller_unittest.cc
index 64ae00ec9f..df12def517 100644
--- a/modules/pacing/pacing_controller_unittest.cc
+++ b/modules/pacing/pacing_controller_unittest.cc
@@ -1767,6 +1767,34 @@ TEST_P(PacingControllerTest, NoProbingWhilePaused) {
             PacingController::kPausedProcessInterval);
 }
 
+TEST_P(PacingControllerTest, AudioNotPacedEvenWhenAccountedFor) {
+  const uint32_t kSsrc = 12345;
+  uint16_t sequence_number = 1234;
+  const size_t kPacketSize = 123;
+
+  // Account for audio - so that audio packets can cause pushback on other
+  // types such as video. Audio packet should still be immediated passed
+  // through though ("WebRTC-Pacer-BlockAudio" needs to be enabled in order
+  // to pace audio packets).
+  pacer_->SetAccountForAudioPackets(true);
+
+  // Set pacing rate to 1 packet/s, no padding.
+  pacer_->SetPacingRates(DataSize::Bytes(kPacketSize) / TimeDelta::Seconds(1),
+                         DataRate::Zero());
+
+  // Add and send an audio packet.
+  SendAndExpectPacket(RtpPacketMediaType::kAudio, kSsrc, sequence_number++,
+                      clock_.TimeInMilliseconds(), kPacketSize);
+  pacer_->ProcessPackets();
+
+  // Advance time, add another audio packet and process. It should be sent
+  // immediately.
+  clock_.AdvanceTimeMilliseconds(5);
+  SendAndExpectPacket(RtpPacketMediaType::kAudio, kSsrc, sequence_number++,
+                      clock_.TimeInMilliseconds(), kPacketSize);
+  pacer_->ProcessPackets();
+}
+
 INSTANTIATE_TEST_SUITE_P(
     WithAndWithoutIntervalBudget,
     PacingControllerTest,