diff --git a/modules/rtp_rtcp/include/ulpfec_receiver.h b/modules/rtp_rtcp/include/ulpfec_receiver.h index 5e0a156273..eb55deca23 100644 --- a/modules/rtp_rtcp/include/ulpfec_receiver.h +++ b/modules/rtp_rtcp/include/ulpfec_receiver.h @@ -16,6 +16,7 @@ #include "api/array_view.h" #include "api/rtp_parameters.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "modules/rtp_rtcp/source/rtp_packet.h" namespace webrtc { @@ -43,10 +44,8 @@ class UlpfecReceiver { // // TODO(brandtr): Set |ulpfec_payload_type| during constructor call, // rather than as a parameter here. - virtual int32_t AddReceivedRedPacket(const RTPHeader& rtp_header, - const uint8_t* incoming_rtp_packet, - size_t packet_length, - uint8_t ulpfec_payload_type) = 0; + virtual bool AddReceivedRedPacket(const RtpPacket& rtp_packet, + uint8_t ulpfec_payload_type) = 0; // Sends the received packets to the FEC and returns all packets // (both original media and recovered) through the callback. diff --git a/modules/rtp_rtcp/source/fec_test_helper.cc b/modules/rtp_rtcp/source/fec_test_helper.cc index e94e9b075e..1941e213ab 100644 --- a/modules/rtp_rtcp/source/fec_test_helper.cc +++ b/modules/rtp_rtcp/source/fec_test_helper.cc @@ -15,6 +15,7 @@ #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/byte_io.h" +#include "modules/rtp_rtcp/source/rtp_packet.h" #include "modules/rtp_rtcp/source/rtp_utility.h" #include "rtc_base/checks.h" @@ -180,52 +181,44 @@ std::unique_ptr FlexfecPacketGenerator::BuildFlexfecPacket( UlpfecPacketGenerator::UlpfecPacketGenerator(uint32_t ssrc) : AugmentedPacketGenerator(ssrc) {} -std::unique_ptr UlpfecPacketGenerator::BuildMediaRedPacket( +RtpPacket UlpfecPacketGenerator::BuildMediaRedPacket( const AugmentedPacket& packet) { - std::unique_ptr red_packet(new AugmentedPacket()); - - const size_t kHeaderLength = packet.header.headerLength; - red_packet->header = packet.header; - red_packet->data.SetSize(packet.data.size() + 1); + RtpPacket red_packet; // Copy RTP header. - memcpy(red_packet->data.data(), packet.data.cdata(), kHeaderLength); - SetRedHeader(red_packet->data[1] & 0x7f, kHeaderLength, red_packet.get()); - memcpy(red_packet->data.data() + kHeaderLength + 1, - packet.data.cdata() + kHeaderLength, + const size_t kHeaderLength = packet.header.headerLength; + red_packet.Parse(packet.data.cdata(), kHeaderLength); + RTC_DCHECK_EQ(red_packet.headers_size(), kHeaderLength); + uint8_t* rtp_payload = + red_packet.AllocatePayload(packet.data.size() + 1 - kHeaderLength); + // Move payload type into rtp payload. + rtp_payload[0] = red_packet.PayloadType(); + red_packet.SetPayloadType(kRedPayloadType); + // Copy the payload. + memcpy(rtp_payload + 1, packet.data.cdata() + kHeaderLength, packet.data.size() - kHeaderLength); return red_packet; } -std::unique_ptr UlpfecPacketGenerator::BuildUlpfecRedPacket( +RtpPacket UlpfecPacketGenerator::BuildUlpfecRedPacket( const ForwardErrorCorrection::Packet& packet) { // Create a fake media packet to get a correct header. 1 byte RED header. ++num_packets_; - std::unique_ptr red_packet = + std::unique_ptr fake_packet = NextPacket(0, packet.data.size() + 1); - red_packet->data[1] &= ~0x80; // Clear marker bit. - const size_t kHeaderLength = red_packet->header.headerLength; - red_packet->data.SetSize(kHeaderLength + 1 + packet.data.size()); - SetRedHeader(kFecPayloadType, kHeaderLength, red_packet.get()); - memcpy(red_packet->data.data() + kHeaderLength + 1, packet.data.cdata(), - packet.data.size()); + RtpPacket red_packet; + red_packet.Parse(fake_packet->data); + red_packet.SetMarker(false); + uint8_t* rtp_payload = red_packet.AllocatePayload(packet.data.size() + 1); + rtp_payload[0] = kFecPayloadType; + red_packet.SetPayloadType(kRedPayloadType); + + memcpy(rtp_payload + 1, packet.data.cdata(), packet.data.size()); return red_packet; } -void UlpfecPacketGenerator::SetRedHeader(uint8_t payload_type, - size_t header_length, - AugmentedPacket* red_packet) { - uint8_t* data = red_packet->data.data(); - // Replace payload type. - data[1] &= 0x80; // Reset. - data[1] += kRedPayloadType; // Replace. - - // Add RED header, f-bit always 0. - data[header_length] = payload_type; -} - } // namespace fec } // namespace test } // namespace webrtc diff --git a/modules/rtp_rtcp/source/fec_test_helper.h b/modules/rtp_rtcp/source/fec_test_helper.h index 635062a5b6..e66e6ca0dc 100644 --- a/modules/rtp_rtcp/source/fec_test_helper.h +++ b/modules/rtp_rtcp/source/fec_test_helper.h @@ -105,21 +105,14 @@ class UlpfecPacketGenerator : public AugmentedPacketGenerator { public: explicit UlpfecPacketGenerator(uint32_t ssrc); - // Creates a new AugmentedPacket with the RED header added to the packet. - static std::unique_ptr BuildMediaRedPacket( - const AugmentedPacket& packet); + // Creates a new RtpPacket with the RED header added to the packet. + static RtpPacket BuildMediaRedPacket(const AugmentedPacket& packet); - // Creates a new AugmentedPacket with FEC payload and RED header. Does this by + // Creates a new RtpPacket with FEC payload and RED header. Does this by // creating a new fake media AugmentedPacket, clears the marker bit and adds a // RED header. Finally replaces the payload with the content of // |packet->data|. - std::unique_ptr BuildUlpfecRedPacket( - const ForwardErrorCorrection::Packet& packet); - - private: - static void SetRedHeader(uint8_t payload_type, - size_t header_length, - AugmentedPacket* red_packet); + RtpPacket BuildUlpfecRedPacket(const ForwardErrorCorrection::Packet& packet); }; } // namespace fec diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc index 80e0302d01..43ce2b0245 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc @@ -10,13 +10,10 @@ #include "modules/rtp_rtcp/source/ulpfec_receiver_impl.h" -#include - #include #include #include "api/scoped_refptr.h" -#include "modules/rtp_rtcp/source/byte_io.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/logging.h" #include "rtc_base/time_utils.h" @@ -77,88 +74,75 @@ FecPacketCounter UlpfecReceiverImpl::GetPacketCounter() const { // block length: 10 bits Length in bytes of the corresponding data // block excluding header. -int32_t UlpfecReceiverImpl::AddReceivedRedPacket( - const RTPHeader& header, - const uint8_t* incoming_rtp_packet, - size_t packet_length, - uint8_t ulpfec_payload_type) { - if (header.ssrc != ssrc_) { +bool UlpfecReceiverImpl::AddReceivedRedPacket(const RtpPacket& rtp_packet, + uint8_t ulpfec_payload_type) { + if (rtp_packet.Ssrc() != ssrc_) { RTC_LOG(LS_WARNING) << "Received RED packet with different SSRC than expected; dropping."; - return -1; + return false; } - if (packet_length > IP_PACKET_SIZE) { + if (rtp_packet.size() > IP_PACKET_SIZE) { RTC_LOG(LS_WARNING) << "Received RED packet with length exceeds maximum IP " "packet size; dropping."; - return -1; + return false; } rtc::CritScope cs(&crit_sect_); - uint8_t red_header_length = 1; - size_t payload_data_length = packet_length - header.headerLength; + static constexpr uint8_t kRedHeaderLength = 1; - if (payload_data_length == 0) { + if (rtp_packet.payload_size() == 0) { RTC_LOG(LS_WARNING) << "Corrupt/truncated FEC packet."; - return -1; + return false; } // Remove RED header of incoming packet and store as a virtual RTP packet. - std::unique_ptr received_packet( - new ForwardErrorCorrection::ReceivedPacket()); + auto received_packet = + std::make_unique(); received_packet->pkt = new ForwardErrorCorrection::Packet(); // Get payload type from RED header and sequence number from RTP header. - uint8_t payload_type = incoming_rtp_packet[header.headerLength] & 0x7f; + uint8_t payload_type = rtp_packet.payload()[0] & 0x7f; received_packet->is_fec = payload_type == ulpfec_payload_type; - received_packet->ssrc = header.ssrc; - received_packet->seq_num = header.sequenceNumber; + received_packet->ssrc = rtp_packet.Ssrc(); + received_packet->seq_num = rtp_packet.SequenceNumber(); - if (incoming_rtp_packet[header.headerLength] & 0x80) { + if (rtp_packet.payload()[0] & 0x80) { // f bit set in RED header, i.e. there are more than one RED header blocks. // WebRTC never generates multiple blocks in a RED packet for FEC. RTC_LOG(LS_WARNING) << "More than 1 block in RED packet is not supported."; - return -1; + return false; } ++packet_counter_.num_packets; - packet_counter_.num_bytes += packet_length; + packet_counter_.num_bytes += rtp_packet.size(); if (packet_counter_.first_packet_time_ms == -1) { packet_counter_.first_packet_time_ms = rtc::TimeMillis(); } + auto red_payload = rtp_packet.payload().subview(kRedHeaderLength); if (received_packet->is_fec) { ++packet_counter_.num_fec_packets; // everything behind the RED header - received_packet->pkt->data.SetData( - incoming_rtp_packet + header.headerLength + red_header_length, - payload_data_length - red_header_length); - received_packet->ssrc = - ByteReader::ReadBigEndian(&incoming_rtp_packet[8]); - + received_packet->pkt->data.SetData(red_payload.data(), red_payload.size()); } else { - received_packet->pkt->data.SetSize(header.headerLength + - payload_data_length - red_header_length); + received_packet->pkt->data.EnsureCapacity(rtp_packet.headers_size() + + red_payload.size()); // Copy RTP header. - memcpy(received_packet->pkt->data.data(), incoming_rtp_packet, - header.headerLength); + received_packet->pkt->data.SetData(rtp_packet.data(), + rtp_packet.headers_size()); // Set payload type. received_packet->pkt->data[1] &= 0x80; // Reset RED payload type. received_packet->pkt->data[1] += payload_type; // Set media payload type. // Copy payload data. - if (payload_data_length > red_header_length) { - memcpy(received_packet->pkt->data.data() + header.headerLength, - incoming_rtp_packet + header.headerLength + red_header_length, - payload_data_length - red_header_length); - } + received_packet->pkt->data.AppendData(red_payload.data(), + red_payload.size()); } - if (received_packet->pkt->data.size() == 0) { - return 0; + if (received_packet->pkt->data.size() > 0) { + received_packets_.push_back(std::move(received_packet)); } - - received_packets_.push_back(std::move(received_packet)); - return 0; + return true; } // TODO(nisse): Drop always-zero return value. diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h index 51502cd9fe..7223696650 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h +++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h @@ -17,11 +17,11 @@ #include #include -#include "api/rtp_headers.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/include/ulpfec_receiver.h" #include "modules/rtp_rtcp/source/forward_error_correction.h" +#include "modules/rtp_rtcp/source/rtp_packet.h" #include "rtc_base/critical_section.h" namespace webrtc { @@ -33,10 +33,8 @@ class UlpfecReceiverImpl : public UlpfecReceiver { rtc::ArrayView extensions); ~UlpfecReceiverImpl() override; - int32_t AddReceivedRedPacket(const RTPHeader& rtp_header, - const uint8_t* incoming_rtp_packet, - size_t packet_length, - uint8_t ulpfec_payload_type) override; + bool AddReceivedRedPacket(const RtpPacket& rtp_packet, + uint8_t ulpfec_payload_type) override; int32_t ProcessReceivedFec() override; diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc index 4663c917b1..0ef8085b63 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc @@ -22,7 +22,6 @@ #include "modules/rtp_rtcp/source/forward_error_correction.h" #include "test/gmock.h" #include "test/gtest.h" -#include "test/rtp_header_parser.h" namespace webrtc { @@ -122,19 +121,13 @@ void UlpfecReceiverTest::PacketizeFrame( } void UlpfecReceiverTest::BuildAndAddRedMediaPacket(AugmentedPacket* packet) { - std::unique_ptr red_packet( - packet_generator_.BuildMediaRedPacket(*packet)); - EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket( - red_packet->header, red_packet->data.cdata(), - red_packet->data.size(), kFecPayloadType)); + RtpPacket red_packet = packet_generator_.BuildMediaRedPacket(*packet); + EXPECT_TRUE(receiver_fec_->AddReceivedRedPacket(red_packet, kFecPayloadType)); } void UlpfecReceiverTest::BuildAndAddRedFecPacket(Packet* packet) { - std::unique_ptr red_packet( - packet_generator_.BuildUlpfecRedPacket(*packet)); - EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket( - red_packet->header, red_packet->data.cdata(), - red_packet->data.size(), kFecPayloadType)); + RtpPacket red_packet = packet_generator_.BuildUlpfecRedPacket(*packet); + EXPECT_TRUE(receiver_fec_->AddReceivedRedPacket(red_packet, kFecPayloadType)); } void UlpfecReceiverTest::VerifyReconstructedMediaPacket( @@ -177,15 +170,13 @@ void UlpfecReceiverTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { void UlpfecReceiverTest::SurvivesMaliciousPacket(const uint8_t* data, size_t length, uint8_t ulpfec_payload_type) { - RTPHeader header; - std::unique_ptr parser(RtpHeaderParser::CreateForTest()); - ASSERT_TRUE(parser->Parse(data, length, &header)); - NullRecoveredPacketReceiver null_callback; std::unique_ptr receiver_fec( UlpfecReceiver::Create(kMediaSsrc, &null_callback, {})); - receiver_fec->AddReceivedRedPacket(header, data, length, ulpfec_payload_type); + RtpPacket rtp_packet; + ASSERT_TRUE(rtp_packet.Parse(data, length)); + receiver_fec->AddReceivedRedPacket(rtp_packet, ulpfec_payload_type); } TEST_F(UlpfecReceiverTest, TwoMediaOneFec) { diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn index 7d32c97c1a..9c1c028156 100644 --- a/test/fuzzers/BUILD.gn +++ b/test/fuzzers/BUILD.gn @@ -187,6 +187,7 @@ webrtc_fuzzer_test("ulpfec_receiver_fuzzer") { "ulpfec_receiver_fuzzer.cc", ] deps = [ + ":fuzz_data_helper", "../../modules/rtp_rtcp", "../../modules/rtp_rtcp:rtp_rtcp_format", "../../rtc_base:rtc_base_approved", diff --git a/test/fuzzers/ulpfec_receiver_fuzzer.cc b/test/fuzzers/ulpfec_receiver_fuzzer.cc index 1124e01112..9c76976290 100644 --- a/test/fuzzers/ulpfec_receiver_fuzzer.cc +++ b/test/fuzzers/ulpfec_receiver_fuzzer.cc @@ -14,6 +14,7 @@ #include "modules/rtp_rtcp/include/ulpfec_receiver.h" #include "modules/rtp_rtcp/source/byte_io.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" +#include "test/fuzzers/fuzz_data_helper.h" namespace webrtc { @@ -38,38 +39,31 @@ void FuzzOneInput(const uint8_t* data, size_t size) { std::unique_ptr receiver( UlpfecReceiver::Create(ulpfec_ssrc, &callback, {})); - std::unique_ptr packet; - size_t packet_length; - size_t i = kMinDataNeeded; - while (i < size) { - packet_length = kRtpHeaderSize + data[i++]; - packet = std::unique_ptr(new uint8_t[packet_length]); - if (i + packet_length >= size) { - break; - } - memcpy(packet.get(), data + i, packet_length); - i += packet_length; - // Overwrite the RTPHeader fields for the sequence number and SSRC with + test::FuzzDataHelper fuzz_data(rtc::MakeArrayView(data, size)); + while (fuzz_data.CanReadBytes(kMinDataNeeded)) { + size_t packet_length = kRtpHeaderSize + fuzz_data.Read(); + auto raw_packet = fuzz_data.ReadByteArray(packet_length); + + RtpPacket parsed_packet; + if (!parsed_packet.Parse(raw_packet)) + continue; + + // Overwrite the fields for the sequence number and SSRC with // consistent values for either a received UlpFEC packet or received media // packet. (We're still relying on libfuzzer to manage to generate packet // headers that interact together; this just ensures that we have two // consistent streams). - if (i < size && data[i++] % 2 == 0) { + if (fuzz_data.ReadOrDefaultValue(0) % 2 == 0) { // Simulate UlpFEC packet. - ByteWriter::WriteBigEndian(packet.get() + 2, ulpfec_seq_num++); - ByteWriter::WriteBigEndian(packet.get() + 8, ulpfec_ssrc); + parsed_packet.SetSequenceNumber(ulpfec_seq_num++); + parsed_packet.SetSsrc(ulpfec_ssrc); } else { // Simulate media packet. - ByteWriter::WriteBigEndian(packet.get() + 2, media_seq_num++); - ByteWriter::WriteBigEndian(packet.get() + 8, media_ssrc); - } - RtpPacketReceived parsed_packet; - RTPHeader parsed_header; - if (parsed_packet.Parse(packet.get(), packet_length)) { - parsed_packet.GetHeader(&parsed_header); - receiver->AddReceivedRedPacket(parsed_header, packet.get(), packet_length, - 0); + parsed_packet.SetSequenceNumber(media_seq_num++); + parsed_packet.SetSsrc(media_ssrc); } + + receiver->AddReceivedRedPacket(parsed_packet, 0); } receiver->ProcessReceivedFec(); diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 9ff089b287..007a932673 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -732,11 +732,8 @@ void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader( // packets. NotifyReceiverOfEmptyPacket(packet.SequenceNumber()); } - RTPHeader header; - packet.GetHeader(&header); - if (ulpfec_receiver_->AddReceivedRedPacket( - header, packet.data(), packet.size(), - config_.rtp.ulpfec_payload_type) != 0) { + if (!ulpfec_receiver_->AddReceivedRedPacket( + packet, config_.rtp.ulpfec_payload_type)) { return; } ulpfec_receiver_->ProcessReceivedFec();