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

Make new pacer padding more like old one

Browse Source 
The (currently unused) new pacer code path was implemented with what
was intended as a more careful padding strategy.
Unfortunately this doesn't work as well as expected due to the fact
that the padding budget cannot build up underuse.

I made another CL that could fix that, but I think it adds complexity
for dubious gains. It also will make it more difficult to find any
potential regression when switching to the new path, should one occur.
See https://webrtc-review.googlesource.com/c/src/+/144563

Therefore, this CL makes the new code path choose RTX payload in the
same way as is currently done.

Bug: webrtc:10633
Change-Id: If2115d4fa7463add959faa77c63101286c27e0f5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145202
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28537}
This commit is contained in:
Erik Språng 2019-07-10 10:10:37 +02:00 committed by Commit Bot
parent 741b96b175
commit bb7727211c
2 changed files with 8 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
modules/rtp_rtcp/source/rtp_sender.cc
View File

@ -1007,14 +1007,11 @@ std::vector<std::unique_ptr<RtpPacketToSend>> RTPSender::GeneratePadding(
std::vector<std::unique_ptr<RtpPacketToSend>> padding_packets;
size_t bytes_left = target_size_bytes;
if ((rtx_ & kRtxRedundantPayloads) != 0) {
while (bytes_left >= 0) {
while (bytes_left >= kMinPayloadPaddingBytes) {
std::unique_ptr<RtpPacketToSend> packet =
packet_history_.GetPayloadPaddingPacket(
[&](const RtpPacketToSend& packet)
-> std::unique_ptr<RtpPacketToSend> {
if (packet.payload_size() + kRtxHeaderSize > bytes_left) {
return nullptr;
}
return BuildRtxPacket(packet);
});
if (!packet) {

14
modules/rtp_rtcp/source/rtp_sender_unittest.cc
View File

@ -2549,6 +2549,9 @@ TEST_P(RtpSenderTest, TrySendPacketUpdatesStats) {
}
TEST_P(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) {
// Min requested size in order to use RTX payload.
const size_t kMinPaddingSize = 50;
rtp_sender_->SetRtxStatus(kRtxRetransmitted | kRtxRedundantPayloads);
rtp_sender_->SetRtxPayloadType(kRtxPayload, kPayload);
rtp_sender_->SetStorePacketsStatus(true, 1);
@ -2566,7 +2569,7 @@ TEST_P(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) {
// Generated padding has large enough budget that the video packet should be
// retransmitted as padding.
std::vector<std::unique_ptr<RtpPacketToSend>> generated_packets =
rtp_sender_->GeneratePadding(kPayloadPacketSize + kRtxHeaderSize);
rtp_sender_->GeneratePadding(kMinPaddingSize);
ASSERT_EQ(generated_packets.size(), 1u);
auto& padding_packet = generated_packets.front();
EXPECT_EQ(padding_packet->packet_type(), RtpPacketToSend::Type::kPadding);
@ -2575,13 +2578,11 @@ TEST_P(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) {
kPayloadPacketSize + kRtxHeaderSize);
// Not enough budged for payload padding, use plain padding instead.
const size_t kPaddingBytesRequested = kPayloadPacketSize + kRtxHeaderSize - 1;
const size_t kExpectedNumPaddingPackets =
(kPaddingBytesRequested + kMaxPaddingSize - 1) / kMaxPaddingSize;
const size_t kPaddingBytesRequested = kMinPaddingSize - 1;
size_t padding_bytes_generated = 0;
generated_packets = rtp_sender_->GeneratePadding(kPaddingBytesRequested);
EXPECT_EQ(generated_packets.size(), kExpectedNumPaddingPackets);
EXPECT_EQ(generated_packets.size(), 1u);
for (auto& packet : generated_packets) {
EXPECT_EQ(packet->packet_type(), RtpPacketToSend::Type::kPadding);
EXPECT_EQ(packet->Ssrc(), kRtxSsrc);
@ -2590,8 +2591,7 @@ TEST_P(RtpSenderTest, GeneratePaddingResendsOldPacketsWithRtx) {
padding_bytes_generated += packet->padding_size();
}
EXPECT_EQ(padding_bytes_generated,
kExpectedNumPaddingPackets * kMaxPaddingSize);
EXPECT_EQ(padding_bytes_generated, kMaxPaddingSize);
}
TEST_P(RtpSenderTest, GeneratePaddingCreatesPurePaddingWithoutRtx) {