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

Deferred FEC: Prevents duplicate FEC addition of non-RTX retransmission.

Browse Source 
This CL fixes a bug where the RtpPackeToSend::fec_protect_packet flag
was not cleared when a packet copy was fetched from the packet history
in order to be retransmitted. This caused the packet to be added to the
FEC generator a second time when the retransmission passed through
RtpSenderEgress.

The bug did not affect RTX retransmission and only manifests when using
deferred FEC generation.

Bug: webrtc:11340
Change-Id: Ic7ce2800cce9a99e74bd3dd697bc0779d2a02fda
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185817
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32227}
This commit is contained in:
Erik Språng 2020-09-29 11:53:39 +02:00 committed by Commit Bot
parent 56f63c3e7e
commit f360506c7a
2 changed files with 41 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -338,6 +338,7 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id) {
return -1; return -1;
} }
packet->set_packet_type(RtpPacketMediaType::kRetransmission); packet->set_packet_type(RtpPacketMediaType::kRetransmission);
packet->set_fec_protect_packet(false);
std::vector<std::unique_ptr<RtpPacketToSend>> packets; std::vector<std::unique_ptr<RtpPacketToSend>> packets;
packets.emplace_back(std::move(packet)); packets.emplace_back(std::move(packet));
paced_sender_->EnqueuePackets(std::move(packets)); paced_sender_->EnqueuePackets(std::move(packets));

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

@ -2822,6 +2822,46 @@ TEST_P(RtpSenderTest, IgnoresNackAfterDisablingMedia) {
EXPECT_LT(rtp_sender()->ReSendPacket(kSeqNum), 0); EXPECT_LT(rtp_sender()->ReSendPacket(kSeqNum), 0);
} }
TEST_P(RtpSenderTest, DoesntFecProtectRetransmissions) {
if (!GetParam().deferred_fec) {
// This test make sense only for deferred fec generation.
return;
}
// Set up retranmission without RTX, so that a plain copy of the old packet is
// re-sent instead.
const int64_t kRtt = 10;
rtp_sender()->SetSendingMediaStatus(true);
rtp_sender()->SetRtxStatus(kRtxOff);
rtp_sender_context_->packet_history_.SetStorePacketsStatus(
RtpPacketHistory::StorageMode::kStoreAndCull, 10);
rtp_sender_context_->packet_history_.SetRtt(kRtt);
// Send a packet so it is in the packet history, make sure to mark it for
// FEC protection.
std::unique_ptr<RtpPacketToSend> packet_to_pace;
EXPECT_CALL(mock_paced_sender_, EnqueuePackets)
.WillOnce([&](std::vector<std::unique_ptr<RtpPacketToSend>> packets) {
packet_to_pace = std::move(packets[0]);
});
SendGenericPacket();
packet_to_pace->set_fec_protect_packet(true);
rtp_sender_context_->InjectPacket(std::move(packet_to_pace),
PacedPacketInfo());
ASSERT_EQ(1u, transport_.sent_packets_.size());
// Re-send packet, the retransmitted packet should not have the FEC protection
// flag set.
EXPECT_CALL(mock_paced_sender_,
EnqueuePackets(Each(Pointee(
Property(&RtpPacketToSend::fec_protect_packet, false)))));
time_controller_.AdvanceTime(TimeDelta::Millis(kRtt));
EXPECT_GT(rtp_sender()->ReSendPacket(kSeqNum), 0);
}
INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead,
RtpSenderTest, RtpSenderTest,
::testing::Values(TestConfig{false, false}, ::testing::Values(TestConfig{false, false},