From 085eceb9ec33ab42bac4fc5854d204ac21df338f Mon Sep 17 00:00:00 2001 From: Harsh Maniar Date: Wed, 21 Apr 2021 01:21:53 -0700 Subject: [PATCH] Increase FEC receiver's protected packet queue size. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The FEC receiver tracks maximum of 48 media packets at a time, and packet reordering can delay the FEC packet from its protected media packets by more than 48 sequences. - Such FEC packets do not get purged until much later when newer FEC packets with much higher sequence mark them as old. - Until that happens, they sit in the receiver queue, wasting CPU cycles. - If the receiver maintains a larger queue size for the media packets, it increases possibility of having all media packets in the queue, thereby organically purging the FEC packet. - More importantly, this also increases the efficacy of FEC decode for such packet, since media packets now remain relevant for longer and aid in lost packet recovery. Bug: webrtc:12656 Change-Id: Id0058df9a23ea31839decf2c37e0670a54c947fc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215882 Reviewed-by: Rasmus Brandt Reviewed-by: Åsa Persson Commit-Queue: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#33989} --- .../source/flexfec_header_reader_writer.cc | 7 ++++++- .../source/flexfec_receiver_unittest.cc | 17 ++++++++++------- .../source/ulpfec_header_reader_writer.cc | 7 ++++++- .../rtp_rtcp/source/ulpfec_receiver_unittest.cc | 4 ++-- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/modules/rtp_rtcp/source/flexfec_header_reader_writer.cc b/modules/rtp_rtcp/source/flexfec_header_reader_writer.cc index 8b4162fe2f..40426f16bf 100644 --- a/modules/rtp_rtcp/source/flexfec_header_reader_writer.cc +++ b/modules/rtp_rtcp/source/flexfec_header_reader_writer.cc @@ -25,6 +25,11 @@ namespace { // Maximum number of media packets that can be protected in one batch. constexpr size_t kMaxMediaPackets = 48; // Since we are reusing ULPFEC masks. +// Maximum number of media packets tracked by FEC decoder. +// Maintain a sufficiently larger tracking window than |kMaxMediaPackets| +// to account for packet reordering in pacer/ network. +constexpr size_t kMaxTrackedMediaPackets = 4 * kMaxMediaPackets; + // Maximum number of FEC packets stored inside ForwardErrorCorrection. constexpr size_t kMaxFecPackets = kMaxMediaPackets; @@ -72,7 +77,7 @@ size_t FlexfecHeaderSize(size_t packet_mask_size) { } // namespace FlexfecHeaderReader::FlexfecHeaderReader() - : FecHeaderReader(kMaxMediaPackets, kMaxFecPackets) {} + : FecHeaderReader(kMaxTrackedMediaPackets, kMaxFecPackets) {} FlexfecHeaderReader::~FlexfecHeaderReader() = default; diff --git a/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc b/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc index dee5da080c..7261280aef 100644 --- a/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc @@ -374,7 +374,8 @@ TEST_F(FlexfecReceiverTest, RecoversFrom50PercentLoss) { TEST_F(FlexfecReceiverTest, DelayedFecPacketDoesHelp) { // These values need to be updated if the underlying erasure code // implementation changes. - const size_t kNumFrames = 48; + // Delay FEC packet by maximum number of media packets tracked by receiver. + const size_t kNumFrames = 192; const size_t kNumMediaPacketsPerFrame = 1; const size_t kNumFecPackets = 1; @@ -412,14 +413,16 @@ TEST_F(FlexfecReceiverTest, DelayedFecPacketDoesHelp) { TEST_F(FlexfecReceiverTest, TooDelayedFecPacketDoesNotHelp) { // These values need to be updated if the underlying erasure code // implementation changes. - const size_t kNumFrames = 49; + // Delay FEC packet by one more than maximum number of media packets + // tracked by receiver. + const size_t kNumFrames = 193; const size_t kNumMediaPacketsPerFrame = 1; const size_t kNumFecPackets = 1; PacketList media_packets; PacketizeFrame(kNumMediaPacketsPerFrame, 0, &media_packets); PacketizeFrame(kNumMediaPacketsPerFrame, 1, &media_packets); - // Protect two first frames. + // Protect first two frames. std::list fec_packets = EncodeFec(media_packets, kNumFecPackets); for (size_t i = 2; i < kNumFrames; ++i) { PacketizeFrame(kNumMediaPacketsPerFrame, i, &media_packets); @@ -667,11 +670,11 @@ TEST_F(FlexfecReceiverTest, DoesNotDecodeWrappedMediaSequenceUsingOldFec) { PacketizeFrame(kNumMediaPacketsPerFrame, i, &media_packets); } - // Receive first (|kFirstFrameNumMediaPackets| + 48) media packets. + // Receive first (|kFirstFrameNumMediaPackets| + 192) media packets. // Simulate an old FEC packet by separating it from its encoded media - // packets by at least 48 packets. + // packets by at least 192 packets. auto media_it = media_packets.begin(); - for (size_t i = 0; i < (kFirstFrameNumMediaPackets + 48); i++) { + for (size_t i = 0; i < (kFirstFrameNumMediaPackets + 192); i++) { if (i == 1) { // Drop the second packet of the first frame. media_it++; @@ -682,7 +685,7 @@ TEST_F(FlexfecReceiverTest, DoesNotDecodeWrappedMediaSequenceUsingOldFec) { // Receive FEC packet. Although a protected packet was dropped, // expect no recovery callback since it is delayed from first frame - // by more than 48 packets. + // by more than 192 packets. auto fec_it = fec_packets.begin(); std::unique_ptr fec_packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it); diff --git a/modules/rtp_rtcp/source/ulpfec_header_reader_writer.cc b/modules/rtp_rtcp/source/ulpfec_header_reader_writer.cc index 2aebbead68..49f483dad6 100644 --- a/modules/rtp_rtcp/source/ulpfec_header_reader_writer.cc +++ b/modules/rtp_rtcp/source/ulpfec_header_reader_writer.cc @@ -24,6 +24,11 @@ namespace { // Maximum number of media packets that can be protected in one batch. constexpr size_t kMaxMediaPackets = 48; +// Maximum number of media packets tracked by FEC decoder. +// Maintain a sufficiently larger tracking window than |kMaxMediaPackets| +// to account for packet reordering in pacer/ network. +constexpr size_t kMaxTrackedMediaPackets = 4 * kMaxMediaPackets; + // Maximum number of FEC packets stored inside ForwardErrorCorrection. constexpr size_t kMaxFecPackets = kMaxMediaPackets; @@ -51,7 +56,7 @@ size_t UlpfecHeaderSize(size_t packet_mask_size) { } // namespace UlpfecHeaderReader::UlpfecHeaderReader() - : FecHeaderReader(kMaxMediaPackets, kMaxFecPackets) {} + : FecHeaderReader(kMaxTrackedMediaPackets, kMaxFecPackets) {} UlpfecHeaderReader::~UlpfecHeaderReader() = default; diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc index 016df6e834..53d363de67 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc @@ -392,7 +392,7 @@ TEST_F(UlpfecReceiverTest, PacketNotDroppedTooEarly) { delayed_fec = fec_packets.front(); // Fill the FEC decoder. No packets should be dropped. - const size_t kNumMediaPacketsBatch2 = 46; + const size_t kNumMediaPacketsBatch2 = 191; std::list augmented_media_packets_batch2; ForwardErrorCorrection::PacketList media_packets_batch2; for (size_t i = 0; i < kNumMediaPacketsBatch2; ++i) { @@ -431,7 +431,7 @@ TEST_F(UlpfecReceiverTest, PacketDroppedWhenTooOld) { delayed_fec = fec_packets.front(); // Fill the FEC decoder and force the last packet to be dropped. - const size_t kNumMediaPacketsBatch2 = 48; + const size_t kNumMediaPacketsBatch2 = 192; std::list augmented_media_packets_batch2; ForwardErrorCorrection::PacketList media_packets_batch2; for (size_t i = 0; i < kNumMediaPacketsBatch2; ++i) {