diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index ad618a5bb3..da756889c0 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -145,8 +145,7 @@ void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec( uint16_t first_fec_sequence_number = rtp_sender_->AllocateSequenceNumber(num_fec_packets); fec_packets = ulpfec_generator_.GetUlpfecPacketsAsRed( - red_payload_type_, ulpfec_payload_type_, first_fec_sequence_number, - media_packet->headers_size()); + red_payload_type_, ulpfec_payload_type_, first_fec_sequence_number); RTC_DCHECK_EQ(num_fec_packets, fec_packets.size()); if (retransmission_settings_ & kRetransmitFECPackets) fec_storage = kAllowRetransmission; diff --git a/modules/rtp_rtcp/source/ulpfec_generator.cc b/modules/rtp_rtcp/source/ulpfec_generator.cc index 8dc9752863..3432bdd11d 100644 --- a/modules/rtp_rtcp/source/ulpfec_generator.cc +++ b/modules/rtp_rtcp/source/ulpfec_generator.cc @@ -13,6 +13,7 @@ #include #include +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/byte_io.h" #include "modules/rtp_rtcp/source/forward_error_correction.h" #include "modules/rtp_rtcp/source/rtp_utility.h" @@ -105,6 +106,7 @@ UlpfecGenerator::UlpfecGenerator() UlpfecGenerator::UlpfecGenerator(std::unique_ptr fec) : fec_(std::move(fec)), + last_media_packet_rtp_header_length_(0), num_protected_frames_(0), min_num_media_packets_(1) { memset(¶ms_, 0, sizeof(params_)); @@ -142,6 +144,10 @@ int UlpfecGenerator::AddRtpPacketAndGenerateFec(const uint8_t* data_buffer, packet->length = payload_length + rtp_header_length; memcpy(packet->data, data_buffer, packet->length); media_packets_.push_back(std::move(packet)); + // Keep track of the RTP header length, so we can copy the RTP header + // from |packet| to newly generated ULPFEC+RED packets. + RTC_DCHECK_GE(rtp_header_length, kRtpHeaderSize); + last_media_packet_rtp_header_length_ = rtp_header_length; } if (marker_bit) { ++num_protected_frames_; @@ -200,8 +206,7 @@ size_t UlpfecGenerator::MaxPacketOverhead() const { std::vector> UlpfecGenerator::GetUlpfecPacketsAsRed( int red_payload_type, int ulpfec_payload_type, - uint16_t first_seq_num, - size_t rtp_header_length) { + uint16_t first_seq_num) { std::vector> red_packets; red_packets.reserve(generated_fec_packets_.size()); RTC_DCHECK(!media_packets_.empty()); @@ -212,9 +217,12 @@ std::vector> UlpfecGenerator::GetUlpfecPacketsAsRed( // Wrap FEC packet (including FEC headers) in a RED packet. Since the // FEC packets in |generated_fec_packets_| don't have RTP headers, we // reuse the header from the last media packet. - std::unique_ptr red_packet(new RedPacket( - fec_packet->length + kRedForFecHeaderLength + rtp_header_length)); - red_packet->CreateHeader(last_media_packet->data, rtp_header_length, + RTC_DCHECK_GT(last_media_packet_rtp_header_length_, 0); + std::unique_ptr red_packet( + new RedPacket(last_media_packet_rtp_header_length_ + + kRedForFecHeaderLength + fec_packet->length)); + red_packet->CreateHeader(last_media_packet->data, + last_media_packet_rtp_header_length_, red_payload_type, ulpfec_payload_type); red_packet->SetSeqNum(seq_num++); red_packet->ClearMarkerBit(); @@ -237,6 +245,7 @@ int UlpfecGenerator::Overhead() const { void UlpfecGenerator::ResetState() { media_packets_.clear(); + last_media_packet_rtp_header_length_ = 0; generated_fec_packets_.clear(); num_protected_frames_ = 0; } diff --git a/modules/rtp_rtcp/source/ulpfec_generator.h b/modules/rtp_rtcp/source/ulpfec_generator.h index 3de7ae2986..639c15be25 100644 --- a/modules/rtp_rtcp/source/ulpfec_generator.h +++ b/modules/rtp_rtcp/source/ulpfec_generator.h @@ -69,8 +69,7 @@ class UlpfecGenerator { std::vector> GetUlpfecPacketsAsRed( int red_payload_type, int ulpfec_payload_type, - uint16_t first_seq_num, - size_t rtp_header_length); + uint16_t first_seq_num); private: explicit UlpfecGenerator(std::unique_ptr fec); @@ -98,6 +97,7 @@ class UlpfecGenerator { std::unique_ptr fec_; ForwardErrorCorrection::PacketList media_packets_; + size_t last_media_packet_rtp_header_length_; std::list generated_fec_packets_; int num_protected_frames_; int min_num_media_packets_; diff --git a/modules/rtp_rtcp/source/ulpfec_generator_unittest.cc b/modules/rtp_rtcp/source/ulpfec_generator_unittest.cc index 6586d0dc88..b05775443c 100644 --- a/modules/rtp_rtcp/source/ulpfec_generator_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_generator_unittest.cc @@ -95,8 +95,8 @@ TEST_F(UlpfecGeneratorTest, NoEmptyFecWithSeqNumGaps) { size_t num_fec_packets = ulpfec_generator_.NumAvailableFecPackets(); if (num_fec_packets > 0) { std::vector> fec_packets = - ulpfec_generator_.GetUlpfecPacketsAsRed( - kRedPayloadType, kFecPayloadType, 100, p.header_size); + ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, + kFecPayloadType, 100); EXPECT_EQ(num_fec_packets, fec_packets.size()); } } @@ -122,10 +122,10 @@ TEST_F(UlpfecGeneratorTest, OneFrameFec) { last_timestamp = packet->header.header.timestamp; } EXPECT_TRUE(ulpfec_generator_.FecAvailable()); - uint16_t seq_num = packet_generator_.NextPacketSeqNum(); + const uint16_t seq_num = packet_generator_.NextPacketSeqNum(); std::vector> red_packets = ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, - seq_num, kRtpHeaderSize); + seq_num); EXPECT_FALSE(ulpfec_generator_.FecAvailable()); ASSERT_EQ(1u, red_packets.size()); VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType, @@ -157,14 +157,51 @@ TEST_F(UlpfecGeneratorTest, TwoFrameFec) { } } EXPECT_TRUE(ulpfec_generator_.FecAvailable()); - uint16_t seq_num = packet_generator_.NextPacketSeqNum(); + const uint16_t seq_num = packet_generator_.NextPacketSeqNum(); std::vector> red_packets = ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, - seq_num, kRtpHeaderSize); + seq_num); EXPECT_FALSE(ulpfec_generator_.FecAvailable()); ASSERT_EQ(1u, red_packets.size()); VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType, red_packets.front().get(), false); } +TEST_F(UlpfecGeneratorTest, MixedMediaRtpHeaderLengths) { + constexpr size_t kShortRtpHeaderLength = 12; + constexpr size_t kLongRtpHeaderLength = 16; + + // Only one frame required to generate FEC. + FecProtectionParams params = {127, 1, kFecMaskRandom}; + ulpfec_generator_.SetFecParameters(params); + + // Fill up internal buffer with media packets with short RTP header length. + packet_generator_.NewFrame(kUlpfecMaxMediaPackets + 1); + for (size_t i = 0; i < kUlpfecMaxMediaPackets; ++i) { + std::unique_ptr packet = + packet_generator_.NextPacket(i, 10); + EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec( + packet->data, packet->length, kShortRtpHeaderLength)); + EXPECT_FALSE(ulpfec_generator_.FecAvailable()); + } + + // Kick off FEC generation with media packet with long RTP header length. + // Since the internal buffer is full, this packet will not be protected. + std::unique_ptr packet = + packet_generator_.NextPacket(kUlpfecMaxMediaPackets, 10); + EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec( + packet->data, packet->length, kLongRtpHeaderLength)); + EXPECT_TRUE(ulpfec_generator_.FecAvailable()); + + // Ensure that the RED header is placed correctly, i.e. the correct + // RTP header length was used in the RED packet creation. + const uint16_t seq_num = packet_generator_.NextPacketSeqNum(); + std::vector> red_packets = + ulpfec_generator_.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, + seq_num); + for (const auto& red_packet : red_packets) { + EXPECT_EQ(kFecPayloadType, red_packet->data()[kShortRtpHeaderLength]); + } +} + } // namespace webrtc diff --git a/test/fuzzers/ulpfec_generator_fuzzer.cc b/test/fuzzers/ulpfec_generator_fuzzer.cc index 03728f6bac..995bf290b3 100644 --- a/test/fuzzers/ulpfec_generator_fuzzer.cc +++ b/test/fuzzers/ulpfec_generator_fuzzer.cc @@ -51,8 +51,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { const size_t num_fec_packets = generator.NumAvailableFecPackets(); if (num_fec_packets > 0) { std::vector> fec_packets = - generator.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100, - rtp_header_length); + generator.GetUlpfecPacketsAsRed(kRedPayloadType, kFecPayloadType, 100); RTC_CHECK_EQ(num_fec_packets, fec_packets.size()); } }