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);