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) {