admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't account for audio in the pacer budget.

Browse Source 
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}
This commit is contained in:
Stefan Holmer 2015-12-16 16:55:03 +01:00
parent 5f026d03af
commit c482eb3c84
2 changed files with 47 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
webrtc/modules/pacing/paced_sender.cc
View File

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

42
webrtc/modules/pacing/paced_sender_unittest.cc
View File

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