Don't account for audio in the pacer budget.

We should only account for audio packets in the pacer budget if we also
are allocating bandwidth for the audio streams.

BUG=chromium:567659,webrtc:5263
R=mflodman@webrtc.org

Review URL: https://codereview.webrtc.org/1524763002 .

Cr-Commit-Position: refs/heads/master@{#11053}

2 files changed, 47 insertions, 9 deletions

diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index bfa6b53b6b..121f860c7d 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -389,10 +389,7 @@ int32_t PacedSender::Process() {
     // reinsert it if send fails.
     const paced_sender::Packet& packet = packets_->BeginPop();
 
-    // TODO(holmer): Because of this bug issue 5307 we have to send audio
-    // packets even when the pacer is paused. Here we assume audio packets are
-    // always high priority and that they are the only high priority packets.
-    if ((!paused_ || packet.priority == kHighPriority) && SendPacket(packet)) {
+    if (SendPacket(packet)) {
       // Send succeeded, remove it from the queue.
       packets_->FinalizePop(packet);
       if (prober_->IsProbing())
@@ -421,6 +418,11 @@ int32_t PacedSender::Process() {
 }
 
 bool PacedSender::SendPacket(const paced_sender::Packet& packet) {
+  // TODO(holmer): Because of this bug issue 5307 we have to send audio
+  // packets even when the pacer is paused. Here we assume audio packets are
+  // always high priority and that they are the only high priority packets.
+  if (paused_ && packet.priority != kHighPriority)
+    return false;
   critsect_->Leave();
   const bool success = callback_->TimeToSendPacket(packet.ssrc,
                                                    packet.sequence_number,
@@ -428,7 +430,9 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet) {
                                                    packet.retransmission);
   critsect_->Enter();
 
-  if (success) {
+  // TODO(holmer): High priority packets should only be accounted for if we are
+  // allocating bandwidth for audio.
+  if (success && packet.priority != kHighPriority) {
     // Update media bytes sent.
     prober_->PacketSent(clock_->TimeInMilliseconds(), packet.bytes);
     media_budget_->UseBudget(packet.bytes);
diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc
index bf00a05237..588bf3b669 100644
--- a/webrtc/modules/pacing/paced_sender_unittest.cc
+++ b/webrtc/modules/pacing/paced_sender_unittest.cc
@@ -471,13 +471,15 @@ TEST_F(PacedSenderTest, Priority) {
                              sequence_number++, capture_time_ms, 250, false);
   send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
                              sequence_number++, capture_time_ms, 250, false);
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number++, capture_time_ms, 250, false);
   send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc,
                              sequence_number++, capture_time_ms, 250, false);
 
   // Expect all high and normal priority to be sent out first.
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
   EXPECT_CALL(callback_, TimeToSendPacket(ssrc, _, capture_time_ms, false))
-      .Times(3)
+      .Times(4)
       .WillRepeatedly(Return(true));
 
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
@@ -497,6 +499,37 @@ TEST_F(PacedSenderTest, Priority) {
   EXPECT_EQ(0, send_bucket_->Process());
 }
 
+TEST_F(PacedSenderTest, HighPrioDoesntAffectBudget) {
+  uint32_t ssrc = 12346;
+  uint16_t sequence_number = 1234;
+  int64_t capture_time_ms = 56789;
+
+  // As high prio packets doesn't affect the budget, we should be able to send
+  // a high number of them at once.
+  for (int i = 0; i < 25; ++i) {
+    SendAndExpectPacket(PacedSender::kHighPriority, ssrc, sequence_number++,
+                        capture_time_ms, 250, false);
+  }
+  send_bucket_->Process();
+  // Low prio packets does affect the budget, so we should only be able to send
+  // 3 at once, the 4th should be queued.
+  for (int i = 0; i < 3; ++i) {
+    SendAndExpectPacket(PacedSender::kLowPriority, ssrc, sequence_number++,
+                        capture_time_ms, 250, false);
+  }
+  send_bucket_->InsertPacket(PacedSender::kLowPriority, ssrc, sequence_number,
+                             capture_time_ms, 250, false);
+  EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
+  clock_.AdvanceTimeMilliseconds(5);
+  send_bucket_->Process();
+  EXPECT_CALL(callback_,
+              TimeToSendPacket(ssrc, sequence_number++, capture_time_ms, false))
+      .Times(1);
+  EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
+  clock_.AdvanceTimeMilliseconds(5);
+  send_bucket_->Process();
+}
+
 TEST_F(PacedSenderTest, Pause) {
   uint32_t ssrc_low_priority = 12345;
   uint32_t ssrc = 12346;
@@ -837,10 +870,11 @@ TEST_F(PacedSenderTest, AverageQueueTime) {
   EXPECT_EQ(0, send_bucket_->AverageQueueTimeMs());
 
   int64_t first_capture_time = clock_.TimeInMilliseconds();
-  send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc, sequence_number,
-                             first_capture_time, kPacketSize, false);
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
+                             sequence_number, first_capture_time, kPacketSize,
+                             false);
   clock_.AdvanceTimeMilliseconds(10);
-  send_bucket_->InsertPacket(PacedSender::kHighPriority, ssrc,
+  send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
                              sequence_number + 1, clock_.TimeInMilliseconds(),
                              kPacketSize, false);
   clock_.AdvanceTimeMilliseconds(10);