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

Reland "Make new pacer padding more like old one"

Browse Source 
This reverts commit 11820502b840f34aa9fccb1c273cefbcc457c962.

Reason for revert: The culprit was https://webrtc-review.googlesource.com/c/src/+/133169.

Original change's description:
> Revert "Make new pacer padding more like old one"
> 
> This reverts commit bb7727211c535f8a9dce27891941b52b6ea8e750.
> 
> Reason for revert: Speculative revert (some perf test are failing)
> 
> Original change's description:
> > Make new pacer padding more like old one
> > 
> > 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}
> 
> TBR=sprang@webrtc.org,stefan@webrtc.org
> 
> Change-Id: I99b72858414e0a245da596d94694449da88fd626
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:10633
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145324
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28550}

TBR=mbonadei@webrtc.org,sprang@webrtc.org,stefan@webrtc.org

Change-Id: I1fff79d75dc888921c6fbfc7f3980395a67e1c1a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:10633
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145403
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28565}
This commit is contained in:
Mirko Bonadei 2019-07-12 17:35:56 +00:00 committed by Commit Bot
parent 3b67672af7
commit a7e3bcebae
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) {