diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc index a5512e139f..480e764206 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc @@ -219,7 +219,19 @@ int32_t UlpfecReceiverImpl::AddReceivedRedPacket( // TODO(nisse): Drop always-zero return value. int32_t UlpfecReceiverImpl::ProcessReceivedFec() { crit_sect_.Enter(); - for (const auto& received_packet : received_packets_) { + + // If we iterate over |received_packets_| and it contains a packet that cause + // us to recurse back to this function (for example a RED packet encapsulating + // a RED packet), then we will recurse forever. To avoid this we swap + // |received_packets_| with an empty vector so that the next recursive call + // wont iterate over the same packet again. This also solves the problem of + // not modifying the vector we are currently iterating over (packets are added + // in AddReceivedRedPacket). + std::vector> + received_packets; + received_packets.swap(received_packets_); + + for (const auto& received_packet : received_packets) { // Send received media packet to VCM. if (!received_packet->is_fec) { ForwardErrorCorrection::Packet* packet = received_packet->pkt; @@ -230,7 +242,6 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() { } fec_->DecodeFec(*received_packet, &recovered_packets_); } - received_packets_.clear(); // Send any recovered media packets to VCM. for (const auto& recovered_packet : recovered_packets_) { @@ -248,6 +259,7 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() { packet->length); crit_sect_.Enter(); } + crit_sect_.Leave(); return 0; } diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index 297218205f..cc9e61f870 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -218,6 +218,25 @@ TEST_F(RtpVideoStreamReceiverTest, GenericKeyFrame) { &rtp_header); } +TEST_F(RtpVideoStreamReceiverTest, NoInfiniteRecursionOnEncapsulatedRedPacket) { + const uint8_t kRedPayloadType = 125; + VideoCodec codec; + codec.plType = kRedPayloadType; + memcpy(codec.plName, "red", sizeof("red")); + rtp_video_stream_receiver_->AddReceiveCodec(codec, {}); + const std::vector data({0x80, // RTP version. + kRedPayloadType, // Payload type. + 0, 0, 0, 0, 0, 0, // Don't care. + 0, 0, 0x4, 0x57, // SSRC + kRedPayloadType, // RED header. + 0, 0, 0, 0, 0 // Don't care. + }); + RtpPacketReceived packet; + EXPECT_TRUE(packet.Parse(data.data(), data.size())); + rtp_video_stream_receiver_->StartReceive(); + rtp_video_stream_receiver_->OnRtpPacket(packet); +} + TEST_F(RtpVideoStreamReceiverTest, GenericKeyFrameBitstreamError) { WebRtcRTPHeader rtp_header; const std::vector data({1, 2, 3, 4});