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

Always take retransmissions into account when deciding pacing order

Browse Source 
Retransmissions are supposed to be sent before normal packets by the pacer, but the current implementation will only use it if the second packet is a retransmission and the first packet is not. It misses the case where the first packet is retransmission and the second packet is not.

This CL fixes the comparator and adds a unit test.

Also changed the SendAndExpectPacket function to propagate the retransmission flag to the expectations. Previously, all packets were expected to be normal packets.

BUG=webrtc:6124

Review-Url: https://codereview.webrtc.org/2156063004
Cr-Commit-Position: refs/heads/master@{#13502}
This commit is contained in:
terelius 2016-07-20 03:36:19 -07:00 committed by Commit bot
parent 63dcecd5c1
commit f39f7d931c
2 changed files with 57 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -76,8 +76,8 @@ struct Comparator {
return first->priority > second->priority;
// Retransmissions go first.
if (second->retransmission && !first->retransmission)
return true;
if (second->retransmission != first->retransmission)
return second->retransmission;
// Older frames have higher prio.
if (first->capture_time_ms != second->capture_time_ms)

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

@ -128,7 +128,7 @@ class PacedSenderTest : public ::testing::Test {
send_bucket_->InsertPacket(priority, ssrc, sequence_number, capture_time_ms,
size, retransmission);
EXPECT_CALL(callback_, TimeToSendPacket(ssrc, sequence_number,
capture_time_ms, false, _))
capture_time_ms, retransmission, _))
.Times(1)
.WillRepeatedly(Return(true));
}
@ -493,6 +493,60 @@ TEST_F(PacedSenderTest, Priority) {
send_bucket_->Process();
}
TEST_F(PacedSenderTest, RetransmissionPriority) {
uint32_t ssrc = 12345;
uint16_t sequence_number = 1234;
int64_t capture_time_ms = 45678;
int64_t capture_time_ms_retransmission = 56789;
// Due to the multiplicative factor we can send 5 packets during a send
// interval. (network capacity * multiplier / (8 bits per byte *
// (packet size * #send intervals per second)
const size_t packets_to_send_per_interval =
kTargetBitrateBps * PacedSender::kDefaultPaceMultiplier / (8 * 250 * 200);
send_bucket_->Process();
EXPECT_EQ(0u, send_bucket_->QueueSizePackets());
// Alternate retransmissions and normal packets.
for (size_t i = 0; i < packets_to_send_per_interval; ++i) {
send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
sequence_number++,
capture_time_ms_retransmission, 250, true);
send_bucket_->InsertPacket(PacedSender::kNormalPriority, ssrc,
sequence_number++, capture_time_ms, 250, false);
}
EXPECT_EQ(2 * packets_to_send_per_interval, send_bucket_->QueueSizePackets());
// Expect all retransmissions to be sent out first despite having a later
// capture time.
EXPECT_CALL(callback_, TimeToSendPadding(_, _)).Times(0);
EXPECT_CALL(callback_, TimeToSendPacket(_, _, _, false, _)).Times(0);
EXPECT_CALL(callback_, TimeToSendPacket(
ssrc, _, capture_time_ms_retransmission, true, _))
.Times(packets_to_send_per_interval)
.WillRepeatedly(Return(true));
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
send_bucket_->Process();
EXPECT_EQ(packets_to_send_per_interval, send_bucket_->QueueSizePackets());
// Expect the remaining (non-retransmission) packets to be sent.
EXPECT_CALL(callback_, TimeToSendPadding(_, _)).Times(0);
EXPECT_CALL(callback_, TimeToSendPacket(_, _, _, true, _)).Times(0);
EXPECT_CALL(callback_, TimeToSendPacket(ssrc, _, capture_time_ms, false, _))
.Times(packets_to_send_per_interval)
.WillRepeatedly(Return(true));
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
send_bucket_->Process();
EXPECT_EQ(0u, send_bucket_->QueueSizePackets());
}
TEST_F(PacedSenderTest, HighPrioDoesntAffectBudget) {
uint32_t ssrc = 12346;
uint16_t sequence_number = 1234;