From 082696efd9c78be1e0c0ee6f94c0400f45523f17 Mon Sep 17 00:00:00 2001 From: Ilya Nikolaevskiy Date: Tue, 3 Sep 2019 07:52:52 +0000 Subject: [PATCH] Revert "Refactor FEC code to use COW buffers" This reverts commit eec5fff4df92b2330e5fec67ff08c7cbb4c4ab8d. Reason for revert: Some crashes found by the fuzzer Original change's description: > Refactor FEC code to use COW buffers > > This refactoring helps to reduce unnecessary memcpy calls on the receive > side. > > This CL replaces > |uint8 data[IP_PACKET_SIZE]| with |rtc::CopyOnWriteBuffer data| in Packet class, > removes |length| field there, and does necessary changes. > > This is a reland of these two CLs with fixes: > https://webrtc-review.googlesource.com/c/src/+/144942 > https://webrtc-review.googlesource.com/c/src/+/144881 > > Bug: webrtc:10750 > Change-Id: I76f6dee5a57ade59942ea2822ca4737edfe6438b > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145332 > Commit-Queue: Ilya Nikolaevskiy > Reviewed-by: Rasmus Brandt > Reviewed-by: Stefan Holmer > Cr-Commit-Position: refs/heads/master@{#29035} TBR=brandtr@webrtc.org,ilnik@webrtc.org,stefan@webrtc.org Change-Id: Id3d65fb1324b9f1b0446fe217012115ecacf2b40 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:10750 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/151130 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#29043} --- modules/rtp_rtcp/source/fec_test_helper.cc | 67 ++++---- .../source/flexfec_header_reader_writer.cc | 43 +++-- .../flexfec_header_reader_writer_unittest.cc | 69 ++++---- modules/rtp_rtcp/source/flexfec_receiver.cc | 21 ++- .../source/flexfec_receiver_unittest.cc | 58 +++---- modules/rtp_rtcp/source/flexfec_sender.cc | 7 +- .../source/flexfec_sender_unittest.cc | 6 +- .../source/forward_error_correction.cc | 150 ++++++++---------- .../source/forward_error_correction.h | 4 +- modules/rtp_rtcp/source/rtp_fec_unittest.cc | 10 +- modules/rtp_rtcp/source/rtp_packet.cc | 13 +- modules/rtp_rtcp/source/rtp_packet.h | 4 +- modules/rtp_rtcp/source/rtp_sender_video.cc | 3 +- modules/rtp_rtcp/source/ulpfec_generator.cc | 16 +- modules/rtp_rtcp/source/ulpfec_generator.h | 3 +- .../source/ulpfec_generator_unittest.cc | 15 +- .../source/ulpfec_header_reader_writer.cc | 30 ++-- .../ulpfec_header_reader_writer_unittest.cc | 33 ++-- .../rtp_rtcp/source/ulpfec_receiver_impl.cc | 44 +++-- .../source/ulpfec_receiver_unittest.cc | 14 +- modules/rtp_rtcp/test/testFec/test_fec.cc | 47 +++--- test/fuzzers/flexfec_header_reader_fuzzer.cc | 5 +- .../forward_error_correction_fuzzer.cc | 8 +- test/fuzzers/ulpfec_generator_fuzzer.cc | 10 +- test/fuzzers/ulpfec_header_reader_fuzzer.cc | 5 +- 25 files changed, 328 insertions(+), 357 deletions(-) diff --git a/modules/rtp_rtcp/source/fec_test_helper.cc b/modules/rtp_rtcp/source/fec_test_helper.cc index e94e9b075e..1da057ea1c 100644 --- a/modules/rtp_rtcp/source/fec_test_helper.cc +++ b/modules/rtp_rtcp/source/fec_test_helper.cc @@ -53,34 +53,34 @@ ForwardErrorCorrection::PacketList MediaPacketGenerator::ConstructMediaPackets( for (int i = 0; i < num_media_packets; ++i) { std::unique_ptr media_packet( new ForwardErrorCorrection::Packet()); - media_packet->data.SetSize( - random_->Rand(min_packet_size_, max_packet_size_)); + media_packet->length = random_->Rand(min_packet_size_, max_packet_size_); - uint8_t* data = media_packet->data.data(); // Generate random values for the first 2 bytes - data[0] = random_->Rand(); - data[1] = random_->Rand(); + media_packet->data[0] = random_->Rand(); + media_packet->data[1] = random_->Rand(); // The first two bits are assumed to be 10 by the FEC encoder. // In fact the FEC decoder will set the two first bits to 10 regardless of // what they actually were. Set the first two bits to 10 so that a memcmp // can be performed for the whole restored packet. - data[0] |= 0x80; - data[0] &= 0xbf; + media_packet->data[0] |= 0x80; + media_packet->data[0] &= 0xbf; // FEC is applied to a whole frame. // A frame is signaled by multiple packets without the marker bit set // followed by the last packet of the frame for which the marker bit is set. // Only push one (fake) frame to the FEC. - data[1] &= 0x7f; + media_packet->data[1] &= 0x7f; - webrtc::ByteWriter::WriteBigEndian(&data[2], seq_num); - webrtc::ByteWriter::WriteBigEndian(&data[4], time_stamp); - webrtc::ByteWriter::WriteBigEndian(&data[8], ssrc_); + webrtc::ByteWriter::WriteBigEndian(&media_packet->data[2], + seq_num); + webrtc::ByteWriter::WriteBigEndian(&media_packet->data[4], + time_stamp); + webrtc::ByteWriter::WriteBigEndian(&media_packet->data[8], ssrc_); // Generate random values for payload. - for (size_t j = 12; j < media_packet->data.size(); ++j) - data[j] = random_->Rand(); + for (size_t j = 12; j < media_packet->length; ++j) + media_packet->data[j] = random_->Rand(); seq_num++; media_packets.push_back(std::move(media_packet)); } @@ -120,18 +120,16 @@ std::unique_ptr AugmentedPacketGenerator::NextPacket( size_t length) { std::unique_ptr packet(new AugmentedPacket()); - packet->data.SetSize(length + kRtpHeaderSize); - uint8_t* data = packet->data.data(); for (size_t i = 0; i < length; ++i) - data[i + kRtpHeaderSize] = offset + i; - packet->data.SetSize(length + kRtpHeaderSize); + packet->data[i + kRtpHeaderSize] = offset + i; + packet->length = length + kRtpHeaderSize; packet->header.headerLength = kRtpHeaderSize; packet->header.markerBit = (num_packets_ == 1); packet->header.payloadType = kVp8PayloadType; packet->header.sequenceNumber = seq_num_; packet->header.timestamp = timestamp_; packet->header.ssrc = ssrc_; - WriteRtpHeader(packet->header, packet->data.data()); + WriteRtpHeader(packet->header, packet->data); ++seq_num_; --num_packets_; @@ -157,7 +155,7 @@ FlexfecPacketGenerator::FlexfecPacketGenerator(uint32_t media_ssrc, std::unique_ptr FlexfecPacketGenerator::BuildFlexfecPacket( const ForwardErrorCorrection::Packet& packet) { - RTC_DCHECK_LE(packet.data.size(), + RTC_DCHECK_LE(packet.length, static_cast(IP_PACKET_SIZE - kRtpHeaderSize)); RTPHeader header; @@ -169,10 +167,10 @@ std::unique_ptr FlexfecPacketGenerator::BuildFlexfecPacket( std::unique_ptr packet_with_rtp_header( new AugmentedPacket()); - packet_with_rtp_header->data.SetSize(kRtpHeaderSize + packet.data.size()); - WriteRtpHeader(header, packet_with_rtp_header->data.data()); - memcpy(packet_with_rtp_header->data.data() + kRtpHeaderSize, - packet.data.cdata(), packet.data.size()); + WriteRtpHeader(header, packet_with_rtp_header->data); + memcpy(packet_with_rtp_header->data + kRtpHeaderSize, packet.data, + packet.length); + packet_with_rtp_header->length = kRtpHeaderSize + packet.length; return packet_with_rtp_header; } @@ -186,13 +184,12 @@ std::unique_ptr UlpfecPacketGenerator::BuildMediaRedPacket( const size_t kHeaderLength = packet.header.headerLength; red_packet->header = packet.header; - red_packet->data.SetSize(packet.data.size() + 1); + red_packet->length = packet.length + 1; // 1 byte RED header. // Copy RTP header. - memcpy(red_packet->data.data(), packet.data.cdata(), kHeaderLength); + memcpy(red_packet->data, packet.data, kHeaderLength); SetRedHeader(red_packet->data[1] & 0x7f, kHeaderLength, red_packet.get()); - memcpy(red_packet->data.data() + kHeaderLength + 1, - packet.data.cdata() + kHeaderLength, - packet.data.size() - kHeaderLength); + memcpy(red_packet->data + kHeaderLength + 1, packet.data + kHeaderLength, + packet.length - kHeaderLength); return red_packet; } @@ -202,14 +199,13 @@ std::unique_ptr UlpfecPacketGenerator::BuildUlpfecRedPacket( // Create a fake media packet to get a correct header. 1 byte RED header. ++num_packets_; std::unique_ptr red_packet = - NextPacket(0, packet.data.size() + 1); + NextPacket(0, packet.length + 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()); + memcpy(red_packet->data + kHeaderLength + 1, packet.data, packet.length); + red_packet->length = kHeaderLength + 1 + packet.length; return red_packet; } @@ -217,13 +213,12 @@ std::unique_ptr UlpfecPacketGenerator::BuildUlpfecRedPacket( 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. + red_packet->data[1] &= 0x80; // Reset. + red_packet->data[1] += kRedPayloadType; // Replace. // Add RED header, f-bit always 0. - data[header_length] = payload_type; + red_packet->data[header_length] = payload_type; } } // namespace fec diff --git a/modules/rtp_rtcp/source/flexfec_header_reader_writer.cc b/modules/rtp_rtcp/source/flexfec_header_reader_writer.cc index ab0dcb68ae..e3cb0e9e87 100644 --- a/modules/rtp_rtcp/source/flexfec_header_reader_writer.cc +++ b/modules/rtp_rtcp/source/flexfec_header_reader_writer.cc @@ -80,35 +80,36 @@ FlexfecHeaderReader::~FlexfecHeaderReader() = default; // retransmissions, and/or several protected SSRCs. bool FlexfecHeaderReader::ReadFecHeader( ForwardErrorCorrection::ReceivedFecPacket* fec_packet) const { - if (fec_packet->pkt->data.size() <= - kBaseHeaderSize + kStreamSpecificHeaderSize) { + if (fec_packet->pkt->length <= kBaseHeaderSize + kStreamSpecificHeaderSize) { RTC_LOG(LS_WARNING) << "Discarding truncated FlexFEC packet."; return false; } - uint8_t* const data = fec_packet->pkt->data.data(); - bool r_bit = (data[0] & 0x80) != 0; + bool r_bit = (fec_packet->pkt->data[0] & 0x80) != 0; if (r_bit) { RTC_LOG(LS_INFO) << "FlexFEC packet with retransmission bit set. We do not yet " "support this, thus discarding the packet."; return false; } - bool f_bit = (data[0] & 0x40) != 0; + bool f_bit = (fec_packet->pkt->data[0] & 0x40) != 0; if (f_bit) { RTC_LOG(LS_INFO) << "FlexFEC packet with inflexible generator matrix. We do " "not yet support this, thus discarding packet."; return false; } - uint8_t ssrc_count = ByteReader::ReadBigEndian(&data[8]); + uint8_t ssrc_count = + ByteReader::ReadBigEndian(&fec_packet->pkt->data[8]); if (ssrc_count != 1) { RTC_LOG(LS_INFO) << "FlexFEC packet protecting multiple media SSRCs. We do not " "yet support this, thus discarding packet."; return false; } - uint32_t protected_ssrc = ByteReader::ReadBigEndian(&data[12]); - uint16_t seq_num_base = ByteReader::ReadBigEndian(&data[16]); + uint32_t protected_ssrc = + ByteReader::ReadBigEndian(&fec_packet->pkt->data[12]); + uint16_t seq_num_base = + ByteReader::ReadBigEndian(&fec_packet->pkt->data[16]); // Parse the FlexFEC packet mask and remove the interleaved K-bits. // (See FEC header schematic in flexfec_header_reader_writer.h.) @@ -120,11 +121,11 @@ bool FlexfecHeaderReader::ReadFecHeader( // // We treat the mask parts as unsigned integers with host order endianness // in order to simplify the bit shifting between bytes. - if (fec_packet->pkt->data.size() < kHeaderSizes[0]) { + if (fec_packet->pkt->length < kHeaderSizes[0]) { RTC_LOG(LS_WARNING) << "Discarding truncated FlexFEC packet."; return false; } - uint8_t* const packet_mask = data + kPacketMaskOffset; + uint8_t* const packet_mask = fec_packet->pkt->data + kPacketMaskOffset; bool k_bit0 = (packet_mask[0] & 0x80) != 0; uint16_t mask_part0 = ByteReader::ReadBigEndian(&packet_mask[0]); // Shift away K-bit 0, implicitly clearing the last bit. @@ -137,7 +138,7 @@ bool FlexfecHeaderReader::ReadFecHeader( // is payload. packet_mask_size = kFlexfecPacketMaskSizes[0]; } else { - if (fec_packet->pkt->data.size() < kHeaderSizes[1]) { + if (fec_packet->pkt->length < kHeaderSizes[1]) { return false; } bool k_bit1 = (packet_mask[2] & 0x80) != 0; @@ -157,7 +158,7 @@ bool FlexfecHeaderReader::ReadFecHeader( // and the rest of the packet is payload. packet_mask_size = kFlexfecPacketMaskSizes[1]; } else { - if (fec_packet->pkt->data.size() < kHeaderSizes[2]) { + if (fec_packet->pkt->length < kHeaderSizes[2]) { RTC_LOG(LS_WARNING) << "Discarding truncated FlexFEC packet."; return false; } @@ -197,7 +198,7 @@ bool FlexfecHeaderReader::ReadFecHeader( // In FlexFEC, all media packets are protected in their entirety. fec_packet->protection_length = - fec_packet->pkt->data.size() - fec_packet->fec_header_size; + fec_packet->pkt->length - fec_packet->fec_header_size; return true; } @@ -249,19 +250,17 @@ void FlexfecHeaderWriter::FinalizeFecHeader( const uint8_t* packet_mask, size_t packet_mask_size, ForwardErrorCorrection::Packet* fec_packet) const { - uint8_t* data = fec_packet->data.data(); - data[0] &= 0x7f; // Clear R bit. - data[0] &= 0xbf; // Clear F bit. - ByteWriter::WriteBigEndian(&data[8], kSsrcCount); - ByteWriter::WriteBigEndian(&data[9], kReservedBits); - ByteWriter::WriteBigEndian(&data[12], media_ssrc); - ByteWriter::WriteBigEndian(&data[16], seq_num_base); + fec_packet->data[0] &= 0x7f; // Clear R bit. + fec_packet->data[0] &= 0xbf; // Clear F bit. + ByteWriter::WriteBigEndian(&fec_packet->data[8], kSsrcCount); + ByteWriter::WriteBigEndian(&fec_packet->data[9], kReservedBits); + ByteWriter::WriteBigEndian(&fec_packet->data[12], media_ssrc); + ByteWriter::WriteBigEndian(&fec_packet->data[16], seq_num_base); // Adapt ULPFEC packet mask to FlexFEC header. // // We treat the mask parts as unsigned integers with host order endianness // in order to simplify the bit shifting between bytes. - uint8_t* const written_packet_mask = - fec_packet->data.data() + kPacketMaskOffset; + uint8_t* const written_packet_mask = fec_packet->data + kPacketMaskOffset; if (packet_mask_size == kUlpfecPacketMaskSizeLBitSet) { // The packet mask is 48 bits long. uint16_t tmp_mask_part0 = diff --git a/modules/rtp_rtcp/source/flexfec_header_reader_writer_unittest.cc b/modules/rtp_rtcp/source/flexfec_header_reader_writer_unittest.cc index 1d86dd0eb4..81d0cb311b 100644 --- a/modules/rtp_rtcp/source/flexfec_header_reader_writer_unittest.cc +++ b/modules/rtp_rtcp/source/flexfec_header_reader_writer_unittest.cc @@ -77,8 +77,8 @@ rtc::scoped_refptr WriteHeader(const uint8_t* packet_mask, size_t packet_mask_size) { FlexfecHeaderWriter writer; rtc::scoped_refptr written_packet(new Packet()); - written_packet->data.SetSize(kMediaPacketLength); - for (size_t i = 0; i < written_packet->data.size(); ++i) { + written_packet->length = kMediaPacketLength; + for (size_t i = 0; i < written_packet->length; ++i) { written_packet->data[i] = i; // Actual content doesn't matter. } writer.FinalizeFecHeader(kMediaSsrc, kMediaStartSeqNum, packet_mask, @@ -91,7 +91,8 @@ std::unique_ptr ReadHeader(const Packet& written_packet) { std::unique_ptr read_packet(new ReceivedFecPacket()); read_packet->ssrc = kFlexfecSsrc; read_packet->pkt = rtc::scoped_refptr(new Packet()); - read_packet->pkt->data = written_packet.data; + memcpy(read_packet->pkt->data, written_packet.data, written_packet.length); + read_packet->pkt->length = written_packet.length; EXPECT_TRUE(reader.ReadFecHeader(read_packet.get())); return read_packet; } @@ -108,20 +109,19 @@ void VerifyReadHeaders(size_t expected_fec_header_size, const size_t packet_mask_offset = read_packet.packet_mask_offset; EXPECT_EQ(kFlexfecPacketMaskOffset, packet_mask_offset); EXPECT_EQ(expected_packet_mask_size, read_packet.packet_mask_size); - EXPECT_EQ(read_packet.pkt->data.size() - expected_fec_header_size, + EXPECT_EQ(read_packet.pkt->length - expected_fec_header_size, read_packet.protection_length); // Ensure that the K-bits are removed and the packet mask has been packed. - EXPECT_THAT( - ::testing::make_tuple(read_packet.pkt->data.cdata() + packet_mask_offset, - read_packet.packet_mask_size), - ::testing::ElementsAreArray(expected_packet_mask, - expected_packet_mask_size)); + EXPECT_THAT(::testing::make_tuple(read_packet.pkt->data + packet_mask_offset, + read_packet.packet_mask_size), + ::testing::ElementsAreArray(expected_packet_mask, + expected_packet_mask_size)); } void VerifyFinalizedHeaders(const uint8_t* expected_packet_mask, size_t expected_packet_mask_size, const Packet& written_packet) { - const uint8_t* packet = written_packet.data.cdata(); + const uint8_t* packet = written_packet.data; EXPECT_EQ(0x00, packet[0] & 0x80); // F bit clear. EXPECT_EQ(0x00, packet[0] & 0x40); // R bit clear. EXPECT_EQ(0x01, packet[8]); // SSRCCount = 1. @@ -145,21 +145,21 @@ void VerifyWrittenAndReadHeaders(size_t expected_fec_header_size, EXPECT_EQ(kMediaStartSeqNum, read_packet.seq_num_base); EXPECT_EQ(kFlexfecPacketMaskOffset, read_packet.packet_mask_offset); ASSERT_EQ(expected_packet_mask_size, read_packet.packet_mask_size); - EXPECT_EQ(written_packet.data.size() - expected_fec_header_size, + EXPECT_EQ(written_packet.length - expected_fec_header_size, read_packet.protection_length); // Verify that the call to ReadFecHeader did normalize the packet masks. - EXPECT_THAT(::testing::make_tuple( - read_packet.pkt->data.cdata() + kFlexfecPacketMaskOffset, - read_packet.packet_mask_size), - ::testing::ElementsAreArray(expected_packet_mask, - expected_packet_mask_size)); + EXPECT_THAT( + ::testing::make_tuple(read_packet.pkt->data + kFlexfecPacketMaskOffset, + read_packet.packet_mask_size), + ::testing::ElementsAreArray(expected_packet_mask, + expected_packet_mask_size)); // Verify that the call to ReadFecHeader did not tamper with the payload. EXPECT_THAT(::testing::make_tuple( - read_packet.pkt->data.cdata() + read_packet.fec_header_size, - read_packet.pkt->data.size() - read_packet.fec_header_size), + read_packet.pkt->data + read_packet.fec_header_size, + read_packet.pkt->length - read_packet.fec_header_size), ::testing::ElementsAreArray( - written_packet.data.cdata() + expected_fec_header_size, - written_packet.data.size() - expected_fec_header_size)); + written_packet.data + expected_fec_header_size, + written_packet.length - expected_fec_header_size)); } } // namespace @@ -182,7 +182,8 @@ TEST(FlexfecHeaderReaderTest, ReadsHeaderWithKBit0Set) { const size_t packet_length = sizeof(kPacketData); ReceivedFecPacket read_packet; read_packet.pkt = rtc::scoped_refptr(new Packet()); - read_packet.pkt->data.SetData(kPacketData, packet_length); + memcpy(read_packet.pkt->data, kPacketData, packet_length); + read_packet.pkt->length = packet_length; FlexfecHeaderReader reader; EXPECT_TRUE(reader.ReadFecHeader(&read_packet)); @@ -213,7 +214,8 @@ TEST(FlexfecHeaderReaderTest, ReadsHeaderWithKBit1Set) { const size_t packet_length = sizeof(kPacketData); ReceivedFecPacket read_packet; read_packet.pkt = rtc::scoped_refptr(new Packet()); - read_packet.pkt->data.SetData(kPacketData, packet_length); + memcpy(read_packet.pkt->data, kPacketData, packet_length); + read_packet.pkt->length = packet_length; FlexfecHeaderReader reader; EXPECT_TRUE(reader.ReadFecHeader(&read_packet)); @@ -251,7 +253,8 @@ TEST(FlexfecHeaderReaderTest, ReadsHeaderWithKBit2Set) { const size_t packet_length = sizeof(kPacketData); ReceivedFecPacket read_packet; read_packet.pkt = rtc::scoped_refptr(new Packet()); - read_packet.pkt->data.SetData(kPacketData, packet_length); + memcpy(read_packet.pkt->data, kPacketData, packet_length); + read_packet.pkt->length = packet_length; FlexfecHeaderReader reader; EXPECT_TRUE(reader.ReadFecHeader(&read_packet)); @@ -269,7 +272,7 @@ TEST(FlexfecHeaderReaderTest, ReadPacketWithoutStreamSpecificHeaderShouldFail) { ReceivedFecPacket read_packet; read_packet.ssrc = kFlexfecSsrc; read_packet.pkt = std::move(written_packet); - read_packet.pkt->data.SetSize(12); + read_packet.pkt->length = 12; FlexfecHeaderReader reader; EXPECT_FALSE(reader.ReadFecHeader(&read_packet)); @@ -284,7 +287,7 @@ TEST(FlexfecHeaderReaderTest, ReadShortPacketWithKBit0SetShouldFail) { ReceivedFecPacket read_packet; read_packet.ssrc = kFlexfecSsrc; read_packet.pkt = std::move(written_packet); - read_packet.pkt->data.SetSize(18); + read_packet.pkt->length = 18; FlexfecHeaderReader reader; EXPECT_FALSE(reader.ReadFecHeader(&read_packet)); @@ -300,7 +303,7 @@ TEST(FlexfecHeaderReaderTest, ReadShortPacketWithKBit1SetShouldFail) { ReceivedFecPacket read_packet; read_packet.ssrc = kFlexfecSsrc; read_packet.pkt = std::move(written_packet); - read_packet.pkt->data.SetSize(20); + read_packet.pkt->length = 20; FlexfecHeaderReader reader; EXPECT_FALSE(reader.ReadFecHeader(&read_packet)); @@ -316,7 +319,7 @@ TEST(FlexfecHeaderReaderTest, ReadShortPacketWithKBit2SetShouldFail) { ReceivedFecPacket read_packet; read_packet.ssrc = kFlexfecSsrc; read_packet.pkt = std::move(written_packet); - read_packet.pkt->data.SetSize(24); + read_packet.pkt->length = 24; FlexfecHeaderReader reader; EXPECT_FALSE(reader.ReadFecHeader(&read_packet)); @@ -327,8 +330,8 @@ TEST(FlexfecHeaderWriterTest, FinalizesHeaderWithKBit0Set) { constexpr uint8_t kFlexfecPacketMask[] = {0x88, 0x81}; constexpr uint8_t kUlpfecPacketMask[] = {0x11, 0x02}; Packet written_packet; - written_packet.data.SetSize(kMediaPacketLength); - for (size_t i = 0; i < written_packet.data.size(); ++i) { + written_packet.length = kMediaPacketLength; + for (size_t i = 0; i < written_packet.length; ++i) { written_packet.data[i] = i; } @@ -345,8 +348,8 @@ TEST(FlexfecHeaderWriterTest, FinalizesHeaderWithKBit1Set) { constexpr uint8_t kFlexfecPacketMask[] = {0x48, 0x81, 0x82, 0x11, 0x00, 0x21}; constexpr uint8_t kUlpfecPacketMask[] = {0x91, 0x02, 0x08, 0x44, 0x00, 0x84}; Packet written_packet; - written_packet.data.SetSize(kMediaPacketLength); - for (size_t i = 0; i < written_packet.data.size(); ++i) { + written_packet.length = kMediaPacketLength; + for (size_t i = 0; i < written_packet.length; ++i) { written_packet.data[i] = i; } @@ -367,8 +370,8 @@ TEST(FlexfecHeaderWriterTest, FinalizesHeaderWithKBit2Set) { }; constexpr uint8_t kUlpfecPacketMask[] = {0x22, 0x22, 0x44, 0x44, 0x44, 0x41}; Packet written_packet; - written_packet.data.SetSize(kMediaPacketLength); - for (size_t i = 0; i < written_packet.data.size(); ++i) { + written_packet.length = kMediaPacketLength; + for (size_t i = 0; i < written_packet.length; ++i) { written_packet.data[i] = i; } diff --git a/modules/rtp_rtcp/source/flexfec_receiver.cc b/modules/rtp_rtcp/source/flexfec_receiver.cc index ba09db9e1c..4c788f4b22 100644 --- a/modules/rtp_rtcp/source/flexfec_receiver.cc +++ b/modules/rtp_rtcp/source/flexfec_receiver.cc @@ -107,12 +107,13 @@ FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) { ++packet_counter_.num_fec_packets; // Insert packet payload into erasure code. + // TODO(brandtr): Remove this memcpy when the FEC packet classes + // are using COW buffers internally. received_packet->pkt = rtc::scoped_refptr( new ForwardErrorCorrection::Packet()); - // TODO(ilnik): after slice capability is added to COW, use it here instead - // of initializing COW buffer with ArrayView. auto payload = packet.payload(); - received_packet->pkt->data.SetData(payload.data(), payload.size()); + memcpy(received_packet->pkt->data, payload.data(), payload.size()); + received_packet->pkt->length = payload.size(); } else { // This is a media packet, or a FlexFEC packet belonging to some // other FlexFEC stream. @@ -122,12 +123,11 @@ FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) { received_packet->is_fec = false; // Insert entire packet into erasure code. - // Create a copy and fill with zeros all mutable extensions. received_packet->pkt = rtc::scoped_refptr( new ForwardErrorCorrection::Packet()); - RtpPacketReceived packet_copy(packet); - packet_copy.ZeroMutableExtensions(); - received_packet->pkt->data = packet_copy.Buffer(); + // Create a copy and fill with zeros all mutable extensions. + packet.CopyAndZeroMutableExtensions(received_packet->pkt->data); + received_packet->pkt->length = packet.size(); } ++packet_counter_.num_packets; @@ -161,15 +161,14 @@ void FlexfecReceiver::ProcessReceivedPacket( // Set this flag first, since OnRecoveredPacket may end up here // again, with the same packet. recovered_packet->returned = true; - RTC_CHECK_GT(recovered_packet->pkt->data.size(), 0); + RTC_CHECK(recovered_packet->pkt); recovered_packet_receiver_->OnRecoveredPacket( - recovered_packet->pkt->data.cdata(), - recovered_packet->pkt->data.size()); + recovered_packet->pkt->data, recovered_packet->pkt->length); // Periodically log the incoming packets. int64_t now_ms = clock_->TimeInMilliseconds(); if (now_ms - last_recovered_packet_ms_ > kPacketLogIntervalMs) { uint32_t media_ssrc = - ForwardErrorCorrection::ParseSsrc(recovered_packet->pkt->data.data()); + ForwardErrorCorrection::ParseSsrc(recovered_packet->pkt->data); RTC_LOG(LS_VERBOSE) << "Recovered media packet with SSRC: " << media_ssrc << " from FlexFEC stream with SSRC: " << ssrc_ << "."; last_recovered_packet_ms_ = now_ms; diff --git a/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc b/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc index 224fee6aa0..3d77d7bf54 100644 --- a/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc @@ -39,7 +39,7 @@ constexpr uint32_t kMediaSsrc = 8353; RtpPacketReceived ParsePacket(const Packet& packet) { RtpPacketReceived parsed_packet; - EXPECT_TRUE(parsed_packet.Parse(packet.data)); + EXPECT_TRUE(parsed_packet.Parse(packet.data, packet.length)); return parsed_packet; } @@ -149,7 +149,7 @@ TEST_F(FlexfecReceiverTest, FailsOnTruncatedFecPacket) { std::list fec_packets = EncodeFec(media_packets, kNumFecPackets); const auto& media_packet = media_packets.front(); // Simulate truncated FlexFEC payload. - fec_packets.front()->data.SetSize(1); + fec_packets.front()->length = 1; auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front()); std::unique_ptr received_packet = @@ -240,9 +240,9 @@ TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) { packet_generator_.BuildFlexfecPacket(**fec_it); media_it++; EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_it)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), - (*media_it)->data.size()))); + OnRecoveredPacket(_, (*media_it)->length)) + .With( + Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); } @@ -262,9 +262,9 @@ TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) { packet_generator_.BuildFlexfecPacket(**fec_it); auto media_it = media_packets.begin(); EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_it)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), - (*media_it)->data.size()))); + OnRecoveredPacket(_, (*media_it)->length)) + .With( + Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); // Receive second FEC packet and recover second lost media packet. @@ -272,9 +272,9 @@ TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) { packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it); media_it++; EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_it)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), - (*media_it)->data.size()))); + OnRecoveredPacket(_, (*media_it)->length)) + .With( + Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); } @@ -311,9 +311,9 @@ TEST_F(FlexfecReceiverTest, DoesNotCallbackTwice) { packet_generator_.BuildFlexfecPacket(**fec_it); media_it++; EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_it)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), - (*media_it)->data.size()))); + OnRecoveredPacket(_, (*media_it)->length)) + .With( + Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); // Receive the FEC packet again, but do not call back. @@ -364,9 +364,9 @@ TEST_F(FlexfecReceiverTest, RecoversFrom50PercentLoss) { break; } EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_it)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), - (*media_it)->data.size()))); + OnRecoveredPacket(_, (*media_it)->length)) + .With(Args<0, 1>( + ElementsAreArray((*media_it)->data, (*media_it)->length))); receiver_.OnRtpPacket(ParsePacket(*fec_packet_with_rtp_header)); ++media_it; } @@ -404,9 +404,9 @@ TEST_F(FlexfecReceiverTest, DelayedFecPacketDoesHelp) { packet_generator_.BuildFlexfecPacket(**fec_it); media_it = media_packets.begin(); EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_it)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), - (*media_it)->data.size()))); + OnRecoveredPacket(_, (*media_it)->length)) + .With( + Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); } @@ -533,13 +533,13 @@ TEST_F(FlexfecReceiverTest, RecoversWithMediaPacketsOutOfOrder) { // Expect to recover lost media packets. EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_packet1)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_packet1)->data.cdata(), - (*media_packet1)->data.size()))); + OnRecoveredPacket(_, (*media_packet1)->length)) + .With(Args<0, 1>( + ElementsAreArray((*media_packet1)->data, (*media_packet1)->length))); EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_packet4)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_packet4)->data.cdata(), - (*media_packet4)->data.size()))); + OnRecoveredPacket(_, (*media_packet4)->length)) + .With(Args<0, 1>( + ElementsAreArray((*media_packet4)->data, (*media_packet4)->length))); // Add FEC packets. auto fec_it = fec_packets.begin(); @@ -635,9 +635,9 @@ TEST_F(FlexfecReceiverTest, CalculatesNumberOfPackets) { packet_generator_.BuildFlexfecPacket(**fec_it); media_it++; EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, (*media_it)->data.size())) - .With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), - (*media_it)->data.size()))); + OnRecoveredPacket(_, (*media_it)->length)) + .With( + Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); // Check stats calculations. diff --git a/modules/rtp_rtcp/source/flexfec_sender.cc b/modules/rtp_rtcp/source/flexfec_sender.cc index d35f4d6eed..038cef7b78 100644 --- a/modules/rtp_rtcp/source/flexfec_sender.cc +++ b/modules/rtp_rtcp/source/flexfec_sender.cc @@ -114,7 +114,7 @@ bool FlexfecSender::AddRtpPacketAndGenerateFec(const RtpPacketToSend& packet) { // protection. RTC_DCHECK_EQ(packet.Ssrc(), protected_media_ssrc_); return ulpfec_generator_.AddRtpPacketAndGenerateFec( - packet.Buffer(), packet.headers_size()) == 0; + packet.data(), packet.payload_size(), packet.headers_size()) == 0; } bool FlexfecSender::FecAvailable() const { @@ -153,9 +153,8 @@ std::vector> FlexfecSender::GetFecPackets() { } // RTP payload. - uint8_t* payload = - fec_packet_to_send->AllocatePayload(fec_packet->data.size()); - memcpy(payload, fec_packet->data.cdata(), fec_packet->data.size()); + uint8_t* payload = fec_packet_to_send->AllocatePayload(fec_packet->length); + memcpy(payload, fec_packet->data, fec_packet->length); fec_packets_to_send.push_back(std::move(fec_packet_to_send)); } diff --git a/modules/rtp_rtcp/source/flexfec_sender_unittest.cc b/modules/rtp_rtcp/source/flexfec_sender_unittest.cc index 10ec2e7495..c7291b02c4 100644 --- a/modules/rtp_rtcp/source/flexfec_sender_unittest.cc +++ b/modules/rtp_rtcp/source/flexfec_sender_unittest.cc @@ -62,7 +62,7 @@ std::unique_ptr GenerateSingleFlexfecPacket( std::unique_ptr packet = packet_generator.NextPacket(i, kPayloadLength); RtpPacketToSend rtp_packet(nullptr); // No header extensions. - rtp_packet.Parse(packet->data); + rtp_packet.Parse(packet->data, packet->length); EXPECT_TRUE(sender->AddRtpPacketAndGenerateFec(rtp_packet)); } EXPECT_TRUE(sender->FecAvailable()); @@ -133,7 +133,7 @@ TEST(FlexfecSenderTest, ProtectTwoFramesWithOneFecPacket) { std::unique_ptr packet = packet_generator.NextPacket(i, kPayloadLength); RtpPacketToSend rtp_packet(nullptr); - rtp_packet.Parse(packet->data); + rtp_packet.Parse(packet->data, packet->length); EXPECT_TRUE(sender.AddRtpPacketAndGenerateFec(rtp_packet)); } } @@ -173,7 +173,7 @@ TEST(FlexfecSenderTest, ProtectTwoFramesWithTwoFecPackets) { std::unique_ptr packet = packet_generator.NextPacket(i, kPayloadLength); RtpPacketToSend rtp_packet(nullptr); - rtp_packet.Parse(packet->data); + rtp_packet.Parse(packet->data, packet->length); EXPECT_TRUE(sender.AddRtpPacketAndGenerateFec(rtp_packet)); } EXPECT_TRUE(sender.FecAvailable()); diff --git a/modules/rtp_rtcp/source/forward_error_correction.cc b/modules/rtp_rtcp/source/forward_error_correction.cc index 1ac3bf73eb..413c7087cf 100644 --- a/modules/rtp_rtcp/source/forward_error_correction.cc +++ b/modules/rtp_rtcp/source/forward_error_correction.cc @@ -33,7 +33,7 @@ namespace { constexpr size_t kTransportOverhead = 28; } // namespace -ForwardErrorCorrection::Packet::Packet() : data(0), ref_count_(0) {} +ForwardErrorCorrection::Packet::Packet() : length(0), data(), ref_count_(0) {} ForwardErrorCorrection::Packet::~Packet() = default; int32_t ForwardErrorCorrection::Packet::AddRef() { @@ -128,16 +128,16 @@ int ForwardErrorCorrection::EncodeFec(const PacketList& media_packets, // Error check the media packets. for (const auto& media_packet : media_packets) { RTC_DCHECK(media_packet); - if (media_packet->data.size() < kRtpHeaderSize) { - RTC_LOG(LS_WARNING) << "Media packet " << media_packet->data.size() + if (media_packet->length < kRtpHeaderSize) { + RTC_LOG(LS_WARNING) << "Media packet " << media_packet->length << " bytes " << "is smaller than RTP header."; return -1; } // Ensure the FEC packets will fit in a typical MTU. - if (media_packet->data.size() + MaxPacketOverhead() + kTransportOverhead > + if (media_packet->length + MaxPacketOverhead() + kTransportOverhead > IP_PACKET_SIZE) { - RTC_LOG(LS_WARNING) << "Media packet " << media_packet->data.size() + RTC_LOG(LS_WARNING) << "Media packet " << media_packet->length << " bytes " << "with overhead is larger than " << IP_PACKET_SIZE << " bytes."; @@ -150,10 +150,9 @@ int ForwardErrorCorrection::EncodeFec(const PacketList& media_packets, return 0; } for (int i = 0; i < num_fec_packets; ++i) { - generated_fec_packets_[i].data.EnsureCapacity(IP_PACKET_SIZE); - memset(generated_fec_packets_[i].data.data(), 0, IP_PACKET_SIZE); + memset(generated_fec_packets_[i].data, 0, IP_PACKET_SIZE); // Use this as a marker for untouched packets. - generated_fec_packets_[i].data.SetSize(0); + generated_fec_packets_[i].length = 0; fec_packets->push_back(&generated_fec_packets_[i]); } @@ -178,9 +177,9 @@ int ForwardErrorCorrection::EncodeFec(const PacketList& media_packets, GenerateFecPayloads(media_packets, num_fec_packets); // TODO(brandtr): Generalize this when multistream protection support is // added. - const uint32_t media_ssrc = ParseSsrc(media_packets.front()->data.data()); + const uint32_t media_ssrc = ParseSsrc(media_packets.front()->data); const uint16_t seq_num_base = - ParseSequenceNumber(media_packets.front()->data.data()); + ParseSequenceNumber(media_packets.front()->data); FinalizeFecHeaders(num_fec_packets, media_ssrc, seq_num_base); return 0; @@ -212,39 +211,34 @@ void ForwardErrorCorrection::GenerateFecPayloads( size_t media_pkt_idx = 0; auto media_packets_it = media_packets.cbegin(); - uint16_t prev_seq_num = - ParseSequenceNumber((*media_packets_it)->data.data()); + uint16_t prev_seq_num = ParseSequenceNumber((*media_packets_it)->data); while (media_packets_it != media_packets.end()) { Packet* const media_packet = media_packets_it->get(); - const uint8_t* media_packet_data = media_packet->data.cdata(); // Should |media_packet| be protected by |fec_packet|? if (packet_masks_[pkt_mask_idx] & (1 << (7 - media_pkt_idx))) { - size_t media_payload_length = - media_packet->data.size() - kRtpHeaderSize; + size_t media_payload_length = media_packet->length - kRtpHeaderSize; - bool first_protected_packet = (fec_packet->data.size() == 0); + bool first_protected_packet = (fec_packet->length == 0); size_t fec_packet_length = fec_header_size + media_payload_length; - if (fec_packet_length > fec_packet->data.size()) { + if (fec_packet_length > fec_packet->length) { // Recall that XORing with zero (which the FEC packets are prefilled // with) is the identity operator, thus all prior XORs are // still correct even though we expand the packet length here. - fec_packet->data.SetSize(fec_packet_length); + fec_packet->length = fec_packet_length; } if (first_protected_packet) { - uint8_t* data = fec_packet->data.data(); // Write P, X, CC, M, and PT recovery fields. // Note that bits 0, 1, and 16 are overwritten in FinalizeFecHeaders. - memcpy(&data[0], &media_packet_data[0], 2); + memcpy(&fec_packet->data[0], &media_packet->data[0], 2); // Write length recovery field. (This is a temporary location for // ULPFEC.) - ByteWriter::WriteBigEndian(&data[2], media_payload_length); + ByteWriter::WriteBigEndian(&fec_packet->data[2], + media_payload_length); // Write timestamp recovery field. - memcpy(&data[4], &media_packet_data[4], 4); + memcpy(&fec_packet->data[4], &media_packet->data[4], 4); // Write payload. - if (media_payload_length > 0) { - memcpy(&data[fec_header_size], &media_packet_data[kRtpHeaderSize], - media_payload_length); - } + memcpy(&fec_packet->data[fec_header_size], + &media_packet->data[kRtpHeaderSize], media_payload_length); } else { XorHeaders(*media_packet, fec_packet); XorPayloads(*media_packet, media_payload_length, fec_header_size, @@ -253,15 +247,14 @@ void ForwardErrorCorrection::GenerateFecPayloads( } media_packets_it++; if (media_packets_it != media_packets.end()) { - uint16_t seq_num = - ParseSequenceNumber((*media_packets_it)->data.data()); + uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data); media_pkt_idx += static_cast(seq_num - prev_seq_num); prev_seq_num = seq_num; } pkt_mask_idx += media_pkt_idx / 8; media_pkt_idx %= 8; } - RTC_DCHECK_GT(fec_packet->data.size(), 0) + RTC_DCHECK_GT(fec_packet->length, 0) << "Packet mask is wrong or poorly designed."; } } @@ -273,10 +266,8 @@ int ForwardErrorCorrection::InsertZerosInPacketMasks( if (num_media_packets <= 1) { return num_media_packets; } - uint16_t last_seq_num = - ParseSequenceNumber(media_packets.back()->data.data()); - uint16_t first_seq_num = - ParseSequenceNumber(media_packets.front()->data.data()); + uint16_t last_seq_num = ParseSequenceNumber(media_packets.back()->data); + uint16_t first_seq_num = ParseSequenceNumber(media_packets.front()->data); size_t total_missing_seq_nums = static_cast(last_seq_num - first_seq_num) - num_media_packets + 1; @@ -309,7 +300,7 @@ int ForwardErrorCorrection::InsertZerosInPacketMasks( // We can only cover up to 48 packets. break; } - uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data.data()); + uint16_t seq_num = ParseSequenceNumber((*media_packets_it)->data); const int num_zeros_to_insert = static_cast(seq_num - prev_seq_num - 1); if (num_zeros_to_insert > 0) { @@ -378,6 +369,7 @@ void ForwardErrorCorrection::InsertMediaPacket( recovered_packet->ssrc = received_packet.ssrc; recovered_packet->seq_num = received_packet.seq_num; recovered_packet->pkt = received_packet.pkt; + recovered_packet->pkt->length = received_packet.pkt->length; // TODO(holmer): Consider replacing this with a binary search for the right // position, and then just insert the new packet. Would get rid of the sort. RecoveredPacket* recovered_packet_ptr = recovered_packet.get(); @@ -431,12 +423,6 @@ void ForwardErrorCorrection::InsertFecPacket( return; } - if (fec_packet->packet_mask_offset + fec_packet->packet_mask_size > - fec_packet->pkt->data.size()) { - RTC_LOG(LS_INFO) << "Received corrupted FEC packet; dropping."; - return; - } - // Parse packet mask from header and represent as protected packets. for (uint16_t byte_idx = 0; byte_idx < fec_packet->packet_mask_size; ++byte_idx) { @@ -542,84 +528,78 @@ void ForwardErrorCorrection::InsertPacket( bool ForwardErrorCorrection::StartPacketRecovery( const ReceivedFecPacket& fec_packet, RecoveredPacket* recovered_packet) { - // Ensure pkt is initialized. - recovered_packet->pkt = new Packet(); // Sanity check packet length. - if (fec_packet.pkt->data.size() < fec_packet.fec_header_size) { + if (fec_packet.pkt->length < fec_packet.fec_header_size) { RTC_LOG(LS_WARNING) << "The FEC packet is truncated: it does not contain enough room " << "for its own header."; return false; } - if (fec_packet.protection_length > - std::min(size_t{IP_PACKET_SIZE - kRtpHeaderSize}, - IP_PACKET_SIZE - fec_packet.fec_header_size)) { - RTC_LOG(LS_WARNING) << "Incorrect protection length, dropping FEC packet."; - return false; - } // Initialize recovered packet data. - recovered_packet->pkt->data.EnsureCapacity(IP_PACKET_SIZE); - recovered_packet->pkt->data.SetSize(fec_packet.protection_length + - kRtpHeaderSize); + recovered_packet->pkt = new Packet(); + memset(recovered_packet->pkt->data, 0, IP_PACKET_SIZE); recovered_packet->returned = false; recovered_packet->was_recovered = true; // Copy bytes corresponding to minimum RTP header size. // Note that the sequence number and SSRC fields will be overwritten // at the end of packet recovery. - memcpy(recovered_packet->pkt->data.data(), fec_packet.pkt->data.cdata(), - kRtpHeaderSize); + memcpy(&recovered_packet->pkt->data, fec_packet.pkt->data, kRtpHeaderSize); // Copy remaining FEC payload. - if (fec_packet.protection_length > 0) { - memcpy(recovered_packet->pkt->data.data() + kRtpHeaderSize, - fec_packet.pkt->data.cdata() + fec_packet.fec_header_size, - fec_packet.protection_length); + if (fec_packet.protection_length > + std::min(sizeof(recovered_packet->pkt->data) - kRtpHeaderSize, + sizeof(fec_packet.pkt->data) - fec_packet.fec_header_size)) { + RTC_LOG(LS_WARNING) << "Incorrect protection length, dropping FEC packet."; + return false; } + memcpy(&recovered_packet->pkt->data[kRtpHeaderSize], + &fec_packet.pkt->data[fec_packet.fec_header_size], + fec_packet.protection_length); return true; } bool ForwardErrorCorrection::FinishPacketRecovery( const ReceivedFecPacket& fec_packet, RecoveredPacket* recovered_packet) { - uint8_t* data = recovered_packet->pkt->data.data(); // Set the RTP version to 2. - data[0] |= 0x80; // Set the 1st bit. - data[0] &= 0xbf; // Clear the 2nd bit. + recovered_packet->pkt->data[0] |= 0x80; // Set the 1st bit. + recovered_packet->pkt->data[0] &= 0xbf; // Clear the 2nd bit. // Recover the packet length, from temporary location. - const size_t new_size = - ByteReader::ReadBigEndian(&data[2]) + kRtpHeaderSize; - if (new_size > size_t{IP_PACKET_SIZE - kRtpHeaderSize}) { + recovered_packet->pkt->length = + ByteReader::ReadBigEndian(&recovered_packet->pkt->data[2]) + + kRtpHeaderSize; + if (recovered_packet->pkt->length > + sizeof(recovered_packet->pkt->data) - kRtpHeaderSize) { RTC_LOG(LS_WARNING) << "The recovered packet had a length larger than a " << "typical IP packet, and is thus dropped."; return false; } - recovered_packet->pkt->data.SetSize(new_size); // Set the SN field. - ByteWriter::WriteBigEndian(&data[2], recovered_packet->seq_num); + ByteWriter::WriteBigEndian(&recovered_packet->pkt->data[2], + recovered_packet->seq_num); // Set the SSRC field. - ByteWriter::WriteBigEndian(&data[8], fec_packet.protected_ssrc); + ByteWriter::WriteBigEndian(&recovered_packet->pkt->data[8], + fec_packet.protected_ssrc); recovered_packet->ssrc = fec_packet.protected_ssrc; return true; } void ForwardErrorCorrection::XorHeaders(const Packet& src, Packet* dst) { - uint8_t* dst_data = dst->data.data(); - const uint8_t* src_data = src.data.cdata(); // XOR the first 2 bytes of the header: V, P, X, CC, M, PT fields. - dst_data[0] ^= src_data[0]; - dst_data[1] ^= src_data[1]; + dst->data[0] ^= src.data[0]; + dst->data[1] ^= src.data[1]; // XOR the length recovery field. uint8_t src_payload_length_network_order[2]; ByteWriter::WriteBigEndian(src_payload_length_network_order, - src.data.size() - kRtpHeaderSize); - dst_data[2] ^= src_payload_length_network_order[0]; - dst_data[3] ^= src_payload_length_network_order[1]; + src.length - kRtpHeaderSize); + dst->data[2] ^= src_payload_length_network_order[0]; + dst->data[3] ^= src_payload_length_network_order[1]; // XOR the 5th to 8th bytes of the header: the timestamp field. - dst_data[4] ^= src_data[4]; - dst_data[5] ^= src_data[5]; - dst_data[6] ^= src_data[6]; - dst_data[7] ^= src_data[7]; + dst->data[4] ^= src.data[4]; + dst->data[5] ^= src.data[5]; + dst->data[6] ^= src.data[6]; + dst->data[7] ^= src.data[7]; // Skip the 9th to 12th bytes of the header. } @@ -629,15 +609,10 @@ void ForwardErrorCorrection::XorPayloads(const Packet& src, size_t dst_offset, Packet* dst) { // XOR the payload. - RTC_DCHECK_LE(kRtpHeaderSize + payload_length, src.data.size()); - RTC_DCHECK_LE(dst_offset + payload_length, dst->data.capacity()); - if (dst_offset + payload_length > dst->data.size()) { - dst->data.SetSize(dst_offset + payload_length); - } - uint8_t* dst_data = dst->data.data(); - const uint8_t* src_data = src.data.cdata(); + RTC_DCHECK_LE(kRtpHeaderSize + payload_length, sizeof(src.data)); + RTC_DCHECK_LE(dst_offset + payload_length, sizeof(dst->data)); for (size_t i = 0; i < payload_length; ++i) { - dst_data[dst_offset + i] ^= src_data[kRtpHeaderSize + i]; + dst->data[dst_offset + i] ^= src.data[kRtpHeaderSize + i]; } } @@ -652,8 +627,7 @@ bool ForwardErrorCorrection::RecoverPacket(const ReceivedFecPacket& fec_packet, recovered_packet->seq_num = protected_packet->seq_num; } else { XorHeaders(*protected_packet->pkt, recovered_packet->pkt); - XorPayloads(*protected_packet->pkt, - protected_packet->pkt->data.size() - kRtpHeaderSize, + XorPayloads(*protected_packet->pkt, protected_packet->pkt->length, kRtpHeaderSize, recovered_packet->pkt); } } diff --git a/modules/rtp_rtcp/source/forward_error_correction.h b/modules/rtp_rtcp/source/forward_error_correction.h index 100f532389..ad2eef1b5a 100644 --- a/modules/rtp_rtcp/source/forward_error_correction.h +++ b/modules/rtp_rtcp/source/forward_error_correction.h @@ -22,7 +22,6 @@ #include "modules/include/module_fec_types.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/forward_error_correction_internal.h" -#include "rtc_base/copy_on_write_buffer.h" namespace webrtc { @@ -53,7 +52,8 @@ class ForwardErrorCorrection { // reaches zero. virtual int32_t Release(); - rtc::CopyOnWriteBuffer data; // Packet data. + size_t length; // Length of packet in bytes. + uint8_t data[IP_PACKET_SIZE]; // Packet data. private: int32_t ref_count_; // Counts the number of references to a packet. diff --git a/modules/rtp_rtcp/source/rtp_fec_unittest.cc b/modules/rtp_rtcp/source/rtp_fec_unittest.cc index eb559f2bd9..1c248c8c3f 100644 --- a/modules/rtp_rtcp/source/rtp_fec_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_fec_unittest.cc @@ -120,7 +120,8 @@ void RtpFecTest::ReceivedPackets( std::unique_ptr received_packet( new ForwardErrorCorrection::ReceivedPacket()); received_packet->pkt = new ForwardErrorCorrection::Packet(); - received_packet->pkt->data = packet->data; + received_packet->pkt->length = packet->length; + memcpy(received_packet->pkt->data, packet->data, packet->length); received_packet->is_fec = is_fec; if (!is_fec) { received_packet->ssrc = kMediaSsrc; @@ -154,12 +155,11 @@ bool RtpFecTest::IsRecoveryComplete() { [](const std::unique_ptr& media_packet, const std::unique_ptr& recovered_packet) { - if (media_packet->data.size() != recovered_packet->pkt->data.size()) { + if (media_packet->length != recovered_packet->pkt->length) { return false; } - if (memcmp(media_packet->data.cdata(), - recovered_packet->pkt->data.cdata(), - media_packet->data.size()) != 0) { + if (memcmp(media_packet->data, recovered_packet->pkt->data, + media_packet->length) != 0) { return false; } return true; diff --git a/modules/rtp_rtcp/source/rtp_packet.cc b/modules/rtp_rtcp/source/rtp_packet.cc index b9c7e54c26..5f919ff24e 100644 --- a/modules/rtp_rtcp/source/rtp_packet.cc +++ b/modules/rtp_rtcp/source/rtp_packet.cc @@ -157,7 +157,10 @@ void RtpPacket::SetSsrc(uint32_t ssrc) { ByteWriter::WriteBigEndian(WriteAt(8), ssrc); } -void RtpPacket::ZeroMutableExtensions() { +void RtpPacket::CopyAndZeroMutableExtensions( + rtc::ArrayView buffer) const { + RTC_CHECK_GE(buffer.size(), buffer_.size()); + memcpy(buffer.data(), buffer_.cdata(), buffer_.size()); for (const ExtensionInfo& extension : extension_entries_) { switch (extensions_.GetType(extension.id)) { case RTPExtensionType::kRtpExtensionNone: { @@ -167,9 +170,9 @@ void RtpPacket::ZeroMutableExtensions() { case RTPExtensionType::kRtpExtensionVideoTiming: { // Nullify 3 last entries: packetization delay and 2 network timestamps. // Each of them is 2 bytes. - memset( - WriteAt(extension.offset + VideoSendTiming::kPacerExitDeltaOffset), - 0, 6); + memset(buffer.data() + extension.offset + + VideoSendTiming::kPacerExitDeltaOffset, + 0, 6); break; } case RTPExtensionType::kRtpExtensionTransportSequenceNumber: @@ -177,7 +180,7 @@ void RtpPacket::ZeroMutableExtensions() { case RTPExtensionType::kRtpExtensionTransmissionTimeOffset: case RTPExtensionType::kRtpExtensionAbsoluteSendTime: { // Nullify whole extension, as it's filled in the pacer. - memset(WriteAt(extension.offset), 0, extension.length); + memset(buffer.data() + extension.offset, 0, extension.length); break; } case RTPExtensionType::kRtpExtensionAudioLevel: diff --git a/modules/rtp_rtcp/source/rtp_packet.h b/modules/rtp_rtcp/source/rtp_packet.h index 145f1d7bd8..c49e0709a3 100644 --- a/modules/rtp_rtcp/source/rtp_packet.h +++ b/modules/rtp_rtcp/source/rtp_packet.h @@ -89,9 +89,9 @@ class RtpPacket { void SetTimestamp(uint32_t timestamp); void SetSsrc(uint32_t ssrc); - // Fills with zeroes mutable extensions, + // Copies the buffer with zero-ed mutable extensions, // which are modified after FEC protection is generated. - void ZeroMutableExtensions(); + void CopyAndZeroMutableExtensions(rtc::ArrayView buffer) const; // Removes extension of given |type|, returns false is extension was not // registered in packet's extension map or not present in the packet. Only diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index 569ea8f090..d5cad467d1 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -313,7 +313,8 @@ void RTPSenderVideo::SendVideoPacketAsRedMaybeWithUlpfec( } ulpfec_generator_.AddRtpPacketAndGenerateFec( - media_packet->Buffer(), media_packet->headers_size()); + media_packet->data(), media_packet->payload_size(), + media_packet->headers_size()); } uint16_t num_fec_packets = ulpfec_generator_.NumAvailableFecPackets(); if (num_fec_packets > 0) { diff --git a/modules/rtp_rtcp/source/ulpfec_generator.cc b/modules/rtp_rtcp/source/ulpfec_generator.cc index 92e65df187..ec9088c027 100644 --- a/modules/rtp_rtcp/source/ulpfec_generator.cc +++ b/modules/rtp_rtcp/source/ulpfec_generator.cc @@ -133,9 +133,9 @@ void UlpfecGenerator::SetFecParameters(const FecProtectionParams& params) { } } -int UlpfecGenerator::AddRtpPacketAndGenerateFec( - const rtc::CopyOnWriteBuffer& data_buffer, - size_t rtp_header_length) { +int UlpfecGenerator::AddRtpPacketAndGenerateFec(const uint8_t* data_buffer, + size_t payload_length, + size_t rtp_header_length) { RTC_DCHECK(generated_fec_packets_.empty()); if (media_packets_.empty()) { params_ = new_params_; @@ -146,8 +146,8 @@ int UlpfecGenerator::AddRtpPacketAndGenerateFec( // Our packet masks can only protect up to |kUlpfecMaxMediaPackets| packets. std::unique_ptr packet( new ForwardErrorCorrection::Packet()); - RTC_DCHECK_GE(data_buffer.size(), rtp_header_length); - packet->data = 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. @@ -225,13 +225,13 @@ std::vector> UlpfecGenerator::GetUlpfecPacketsAsRed( 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->data.size())); - red_packet->CreateHeader(last_media_packet->data.data(), + 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(); - red_packet->AssignPayload(fec_packet->data.data(), fec_packet->data.size()); + red_packet->AssignPayload(fec_packet->data, fec_packet->length); red_packets.push_back(std::move(red_packet)); } diff --git a/modules/rtp_rtcp/source/ulpfec_generator.h b/modules/rtp_rtcp/source/ulpfec_generator.h index cdfa1ff67d..7b18c6e0bc 100644 --- a/modules/rtp_rtcp/source/ulpfec_generator.h +++ b/modules/rtp_rtcp/source/ulpfec_generator.h @@ -58,7 +58,8 @@ class UlpfecGenerator { // Adds a media packet to the internal buffer. When enough media packets // have been added, the FEC packets are generated and stored internally. // These FEC packets are then obtained by calling GetFecPacketsAsRed(). - int AddRtpPacketAndGenerateFec(const rtc::CopyOnWriteBuffer& data_buffer, + int AddRtpPacketAndGenerateFec(const uint8_t* data_buffer, + size_t payload_length, size_t rtp_header_length); // Returns true if there are generated FEC packets available. diff --git a/modules/rtp_rtcp/source/ulpfec_generator_unittest.cc b/modules/rtp_rtcp/source/ulpfec_generator_unittest.cc index 8c1c7ea396..6880f79cfb 100644 --- a/modules/rtp_rtcp/source/ulpfec_generator_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_generator_unittest.cc @@ -90,9 +90,8 @@ TEST_F(UlpfecGeneratorTest, NoEmptyFecWithSeqNumGaps) { packet[1] &= ~0x80; } ByteWriter::WriteBigEndian(&packet[2], p.seq_num); - ulpfec_generator_.AddRtpPacketAndGenerateFec( - rtc::CopyOnWriteBuffer(packet, p.payload_size + p.header_size), - p.header_size); + ulpfec_generator_.AddRtpPacketAndGenerateFec(packet, p.payload_size, + p.header_size); size_t num_fec_packets = ulpfec_generator_.NumAvailableFecPackets(); if (num_fec_packets > 0) { std::vector> fec_packets = @@ -118,8 +117,8 @@ TEST_F(UlpfecGeneratorTest, OneFrameFec) { for (size_t i = 0; i < kNumPackets; ++i) { std::unique_ptr packet = packet_generator_.NextPacket(i, 10); - EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec(packet->data, - kRtpHeaderSize)); + EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec( + packet->data, packet->length, kRtpHeaderSize)); last_timestamp = packet->header.timestamp; } EXPECT_TRUE(ulpfec_generator_.FecAvailable()); @@ -153,7 +152,7 @@ TEST_F(UlpfecGeneratorTest, TwoFrameFec) { std::unique_ptr packet = packet_generator_.NextPacket(i * kNumPackets + j, 10); EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec( - packet->data, kRtpHeaderSize)); + packet->data, packet->length, kRtpHeaderSize)); last_timestamp = packet->header.timestamp; } } @@ -182,7 +181,7 @@ TEST_F(UlpfecGeneratorTest, MixedMediaRtpHeaderLengths) { std::unique_ptr packet = packet_generator_.NextPacket(i, 10); EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec( - packet->data, kShortRtpHeaderLength)); + packet->data, packet->length, kShortRtpHeaderLength)); EXPECT_FALSE(ulpfec_generator_.FecAvailable()); } @@ -191,7 +190,7 @@ TEST_F(UlpfecGeneratorTest, MixedMediaRtpHeaderLengths) { std::unique_ptr packet = packet_generator_.NextPacket(kUlpfecMaxMediaPackets, 10); EXPECT_EQ(0, ulpfec_generator_.AddRtpPacketAndGenerateFec( - packet->data, kLongRtpHeaderLength)); + packet->data, packet->length, kLongRtpHeaderLength)); EXPECT_TRUE(ulpfec_generator_.FecAvailable()); // Ensure that the RED header is placed correctly, i.e. the correct diff --git a/modules/rtp_rtcp/source/ulpfec_header_reader_writer.cc b/modules/rtp_rtcp/source/ulpfec_header_reader_writer.cc index 261c8f739b..7086b13685 100644 --- a/modules/rtp_rtcp/source/ulpfec_header_reader_writer.cc +++ b/modules/rtp_rtcp/source/ulpfec_header_reader_writer.cc @@ -57,27 +57,24 @@ UlpfecHeaderReader::~UlpfecHeaderReader() = default; bool UlpfecHeaderReader::ReadFecHeader( ForwardErrorCorrection::ReceivedFecPacket* fec_packet) const { - uint8_t* data = fec_packet->pkt->data.data(); - if (fec_packet->pkt->data.size() < kPacketMaskOffset) { - return false; // Truncated packet. - } - bool l_bit = (data[0] & 0x40) != 0u; + bool l_bit = (fec_packet->pkt->data[0] & 0x40) != 0u; size_t packet_mask_size = l_bit ? kUlpfecPacketMaskSizeLBitSet : kUlpfecPacketMaskSizeLBitClear; fec_packet->fec_header_size = UlpfecHeaderSize(packet_mask_size); - uint16_t seq_num_base = ByteReader::ReadBigEndian(&data[2]); + uint16_t seq_num_base = + ByteReader::ReadBigEndian(&fec_packet->pkt->data[2]); fec_packet->protected_ssrc = fec_packet->ssrc; // Due to RED. fec_packet->seq_num_base = seq_num_base; fec_packet->packet_mask_offset = kPacketMaskOffset; fec_packet->packet_mask_size = packet_mask_size; fec_packet->protection_length = - ByteReader::ReadBigEndian(&data[10]); + ByteReader::ReadBigEndian(&fec_packet->pkt->data[10]); // Store length recovery field in temporary location in header. // This makes the header "compatible" with the corresponding // FlexFEC location of the length recovery field, thus simplifying // the XORing operations. - memcpy(&data[2], &data[8], 2); + memcpy(&fec_packet->pkt->data[2], &fec_packet->pkt->data[8], 2); return true; } @@ -108,29 +105,28 @@ void UlpfecHeaderWriter::FinalizeFecHeader( const uint8_t* packet_mask, size_t packet_mask_size, ForwardErrorCorrection::Packet* fec_packet) const { - uint8_t* data = fec_packet->data.data(); // Set E bit to zero. - data[0] &= 0x7f; + fec_packet->data[0] &= 0x7f; // Set L bit based on packet mask size. (Note that the packet mask // can only take on two discrete values.) bool l_bit = (packet_mask_size == kUlpfecPacketMaskSizeLBitSet); if (l_bit) { - data[0] |= 0x40; // Set the L bit. + fec_packet->data[0] |= 0x40; // Set the L bit. } else { RTC_DCHECK_EQ(packet_mask_size, kUlpfecPacketMaskSizeLBitClear); - data[0] &= 0xbf; // Clear the L bit. + fec_packet->data[0] &= 0xbf; // Clear the L bit. } // Copy length recovery field from temporary location. - memcpy(&data[8], &data[2], 2); + memcpy(&fec_packet->data[8], &fec_packet->data[2], 2); // Write sequence number base. - ByteWriter::WriteBigEndian(&data[2], seq_num_base); + ByteWriter::WriteBigEndian(&fec_packet->data[2], seq_num_base); // Protection length is set to entire packet. (This is not // required in general.) const size_t fec_header_size = FecHeaderSize(packet_mask_size); - ByteWriter::WriteBigEndian( - &data[10], fec_packet->data.size() - fec_header_size); + ByteWriter::WriteBigEndian(&fec_packet->data[10], + fec_packet->length - fec_header_size); // Copy the packet mask. - memcpy(&data[12], packet_mask, packet_mask_size); + memcpy(&fec_packet->data[12], packet_mask, packet_mask_size); } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/ulpfec_header_reader_writer_unittest.cc b/modules/rtp_rtcp/source/ulpfec_header_reader_writer_unittest.cc index 19da2c87c0..725f9a53ee 100644 --- a/modules/rtp_rtcp/source/ulpfec_header_reader_writer_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_header_reader_writer_unittest.cc @@ -52,8 +52,8 @@ std::unique_ptr WriteHeader(const uint8_t* packet_mask, size_t packet_mask_size) { UlpfecHeaderWriter writer; std::unique_ptr written_packet(new Packet()); - written_packet->data.SetSize(kMediaPacketLength); - for (size_t i = 0; i < written_packet->data.size(); ++i) { + written_packet->length = kMediaPacketLength; + for (size_t i = 0; i < written_packet->length; ++i) { written_packet->data[i] = i; // Actual content doesn't matter. } writer.FinalizeFecHeader(kMediaSsrc, kMediaStartSeqNum, packet_mask, @@ -66,7 +66,8 @@ std::unique_ptr ReadHeader(const Packet& written_packet) { std::unique_ptr read_packet(new ReceivedFecPacket()); read_packet->ssrc = kMediaSsrc; read_packet->pkt = rtc::scoped_refptr(new Packet()); - read_packet->pkt->data = written_packet.data; + memcpy(read_packet->pkt->data, written_packet.data, written_packet.length); + read_packet->pkt->length = written_packet.length; EXPECT_TRUE(reader.ReadFecHeader(read_packet.get())); return read_packet; } @@ -82,15 +83,15 @@ void VerifyHeaders(size_t expected_fec_header_size, EXPECT_EQ(kMediaStartSeqNum, read_packet.seq_num_base); EXPECT_EQ(kUlpfecPacketMaskOffset, read_packet.packet_mask_offset); ASSERT_EQ(expected_packet_mask_size, read_packet.packet_mask_size); - EXPECT_EQ(written_packet.data.size() - expected_fec_header_size, + EXPECT_EQ(written_packet.length - expected_fec_header_size, read_packet.protection_length); EXPECT_EQ(0, memcmp(expected_packet_mask, &read_packet.pkt->data[read_packet.packet_mask_offset], read_packet.packet_mask_size)); // Verify that the call to ReadFecHeader did not tamper with the payload. - EXPECT_EQ(0, memcmp(written_packet.data.data() + expected_fec_header_size, - read_packet.pkt->data.cdata() + expected_fec_header_size, - written_packet.data.size() - expected_fec_header_size)); + EXPECT_EQ(0, memcmp(&written_packet.data[expected_fec_header_size], + &read_packet.pkt->data[expected_fec_header_size], + written_packet.length - expected_fec_header_size)); } } // namespace @@ -106,7 +107,8 @@ TEST(UlpfecHeaderReaderTest, ReadsSmallHeader) { const size_t packet_length = sizeof(packet); ReceivedFecPacket read_packet; read_packet.pkt = rtc::scoped_refptr(new Packet()); - read_packet.pkt->data.SetData(packet, packet_length); + memcpy(read_packet.pkt->data, packet, packet_length); + read_packet.pkt->length = packet_length; UlpfecHeaderReader reader; EXPECT_TRUE(reader.ReadFecHeader(&read_packet)); @@ -130,7 +132,8 @@ TEST(UlpfecHeaderReaderTest, ReadsLargeHeader) { const size_t packet_length = sizeof(packet); ReceivedFecPacket read_packet; read_packet.pkt = rtc::scoped_refptr(new Packet()); - read_packet.pkt->data.SetData(packet, packet_length); + memcpy(read_packet.pkt->data, packet, packet_length); + read_packet.pkt->length = packet_length; UlpfecHeaderReader reader; EXPECT_TRUE(reader.ReadFecHeader(&read_packet)); @@ -146,8 +149,8 @@ TEST(UlpfecHeaderWriterTest, FinalizesSmallHeader) { const size_t packet_mask_size = kUlpfecPacketMaskSizeLBitClear; auto packet_mask = GeneratePacketMask(packet_mask_size, 0xabcd); Packet written_packet; - written_packet.data.SetSize(kMediaPacketLength); - for (size_t i = 0; i < written_packet.data.size(); ++i) { + written_packet.length = kMediaPacketLength; + for (size_t i = 0; i < written_packet.length; ++i) { written_packet.data[i] = i; } @@ -155,7 +158,7 @@ TEST(UlpfecHeaderWriterTest, FinalizesSmallHeader) { writer.FinalizeFecHeader(kMediaSsrc, kMediaStartSeqNum, packet_mask.get(), packet_mask_size, &written_packet); - const uint8_t* packet = written_packet.data.cdata(); + const uint8_t* packet = written_packet.data; EXPECT_EQ(0x00, packet[0] & 0x80); // E bit. EXPECT_EQ(0x00, packet[0] & 0x40); // L bit. EXPECT_EQ(kMediaStartSeqNum, ByteReader::ReadBigEndian(packet + 2)); @@ -170,8 +173,8 @@ TEST(UlpfecHeaderWriterTest, FinalizesLargeHeader) { const size_t packet_mask_size = kUlpfecPacketMaskSizeLBitSet; auto packet_mask = GeneratePacketMask(packet_mask_size, 0xabcd); Packet written_packet; - written_packet.data.SetSize(kMediaPacketLength); - for (size_t i = 0; i < written_packet.data.size(); ++i) { + written_packet.length = kMediaPacketLength; + for (size_t i = 0; i < written_packet.length; ++i) { written_packet.data[i] = i; } @@ -179,7 +182,7 @@ TEST(UlpfecHeaderWriterTest, FinalizesLargeHeader) { writer.FinalizeFecHeader(kMediaSsrc, kMediaStartSeqNum, packet_mask.get(), packet_mask_size, &written_packet); - const uint8_t* packet = written_packet.data.cdata(); + const uint8_t* packet = written_packet.data; EXPECT_EQ(0x00, packet[0] & 0x80); // E bit. EXPECT_EQ(0x40, packet[0] & 0x40); // L bit. EXPECT_EQ(kMediaStartSeqNum, ByteReader::ReadBigEndian(packet + 2)); diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc index 1974923ead..42d7af0109 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc @@ -131,30 +131,31 @@ int32_t UlpfecReceiverImpl::AddReceivedRedPacket( ++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); + memcpy(received_packet->pkt->data, + incoming_rtp_packet + header.headerLength + red_header_length, + payload_data_length - red_header_length); + received_packet->pkt->length = payload_data_length - red_header_length; received_packet->ssrc = ByteReader::ReadBigEndian(&incoming_rtp_packet[8]); } else { - received_packet->pkt->data.SetSize(header.headerLength + - payload_data_length - red_header_length); // Copy RTP header. - memcpy(received_packet->pkt->data.data(), incoming_rtp_packet, + memcpy(received_packet->pkt->data, incoming_rtp_packet, header.headerLength); + // 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); - } + memcpy(received_packet->pkt->data + header.headerLength, + incoming_rtp_packet + header.headerLength + red_header_length, + payload_data_length - red_header_length); + received_packet->pkt->length = + header.headerLength + payload_data_length - red_header_length; } - if (received_packet->pkt->data.size() == 0) { + if (received_packet->pkt->length == 0) { return 0; } @@ -182,18 +183,16 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() { if (!received_packet->is_fec) { ForwardErrorCorrection::Packet* packet = received_packet->pkt; crit_sect_.Leave(); - recovered_packet_callback_->OnRecoveredPacket(packet->data.data(), - packet->data.size()); + recovered_packet_callback_->OnRecoveredPacket(packet->data, + packet->length); crit_sect_.Enter(); - // Create a packet with the buffer to modify it. RtpPacketReceived rtp_packet; - rtp_packet.Parse(packet->data); + // TODO(ilnik): move extension nullifying out of RtpPacket, so there's no + // need to create one here, and avoid two memcpy calls below. + rtp_packet.Parse(packet->data, packet->length); // Does memcopy. rtp_packet.IdentifyExtensions(extensions_); - // Reset buffer reference, so zeroing would work on a buffer with a - // single reference. - packet->data = rtc::CopyOnWriteBuffer(0); - rtp_packet.ZeroMutableExtensions(); - packet->data = rtp_packet.Buffer(); + rtp_packet.CopyAndZeroMutableExtensions( // Does memcopy. + rtc::MakeArrayView(packet->data, packet->length)); } fec_->DecodeFec(*received_packet, &recovered_packets_); } @@ -210,8 +209,7 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() { // header, OnRecoveredPacket will recurse back here. recovered_packet->returned = true; crit_sect_.Leave(); - recovered_packet_callback_->OnRecoveredPacket(packet->data.data(), - packet->data.size()); + recovered_packet_callback_->OnRecoveredPacket(packet->data, packet->length); crit_sect_.Enter(); } diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc index e233037af8..32f3bbb2e2 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc @@ -126,16 +126,16 @@ 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)); + red_packet->header, red_packet->data, red_packet->length, + 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)); + red_packet->header, red_packet->data, red_packet->length, + kFecPayloadType)); } void UlpfecReceiverTest::VerifyReconstructedMediaPacket( @@ -144,10 +144,8 @@ void UlpfecReceiverTest::VerifyReconstructedMediaPacket( // Verify that the content of the reconstructed packet is equal to the // content of |packet|, and that the same content is received |times| number // of times in a row. - EXPECT_CALL(recovered_packet_receiver_, - OnRecoveredPacket(_, packet.data.size())) - .With( - Args<0, 1>(ElementsAreArray(packet.data.cdata(), packet.data.size()))) + EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, packet.length)) + .With(Args<0, 1>(ElementsAreArray(packet.data, packet.length))) .Times(times); } diff --git a/modules/rtp_rtcp/test/testFec/test_fec.cc b/modules/rtp_rtcp/test/testFec/test_fec.cc index 505084fa68..3a893b37ca 100644 --- a/modules/rtp_rtcp/test/testFec/test_fec.cc +++ b/modules/rtp_rtcp/test/testFec/test_fec.cc @@ -70,7 +70,9 @@ void ReceivePackets( new ForwardErrorCorrection::ReceivedPacket()); *duplicate_packet = *received_packet; duplicate_packet->pkt = new ForwardErrorCorrection::Packet(); - duplicate_packet->pkt->data = received_packet->pkt->data; + memcpy(duplicate_packet->pkt->data, received_packet->pkt->data, + received_packet->pkt->length); + duplicate_packet->pkt->length = received_packet->pkt->length; to_decode_list->push_back(std::move(duplicate_packet)); random_variable = random->Rand(); @@ -250,14 +252,12 @@ void RunTest(bool use_flexfec) { const uint32_t kMinPacketSize = 12; const uint32_t kMaxPacketSize = static_cast( IP_PACKET_SIZE - 12 - 28 - fec->MaxPacketOverhead()); - size_t packet_length = + media_packet->length = random.Rand(kMinPacketSize, kMaxPacketSize); - media_packet->data.SetSize(packet_length); - uint8_t* data = media_packet->data.data(); // Generate random values for the first 2 bytes. - data[0] = random.Rand(); - data[1] = random.Rand(); + media_packet->data[0] = random.Rand(); + media_packet->data[1] = random.Rand(); // The first two bits are assumed to be 10 by the // FEC encoder. In fact the FEC decoder will set the @@ -265,22 +265,25 @@ void RunTest(bool use_flexfec) { // actually were. Set the first two bits to 10 // so that a memcmp can be performed for the // whole restored packet. - data[0] |= 0x80; - data[0] &= 0xbf; + media_packet->data[0] |= 0x80; + media_packet->data[0] &= 0xbf; // FEC is applied to a whole frame. // A frame is signaled by multiple packets without // the marker bit set followed by the last packet of // the frame for which the marker bit is set. // Only push one (fake) frame to the FEC. - data[1] &= 0x7f; + media_packet->data[1] &= 0x7f; - ByteWriter::WriteBigEndian(&data[2], seq_num); - ByteWriter::WriteBigEndian(&data[4], timestamp); - ByteWriter::WriteBigEndian(&data[8], media_ssrc); + ByteWriter::WriteBigEndian(&media_packet->data[2], + seq_num); + ByteWriter::WriteBigEndian(&media_packet->data[4], + timestamp); + ByteWriter::WriteBigEndian(&media_packet->data[8], + media_ssrc); // Generate random values for payload - for (size_t j = 12; j < packet_length; ++j) { - data[j] = random.Rand(); + for (size_t j = 12; j < media_packet->length; ++j) { + media_packet->data[j] = random.Rand(); } media_packet_list.push_back(std::move(media_packet)); seq_num++; @@ -308,7 +311,9 @@ void RunTest(bool use_flexfec) { received_packet( new ForwardErrorCorrection::ReceivedPacket()); received_packet->pkt = new ForwardErrorCorrection::Packet(); - received_packet->pkt->data = media_packet->data; + received_packet->pkt->length = media_packet->length; + memcpy(received_packet->pkt->data, media_packet->data, + media_packet->length); received_packet->ssrc = media_ssrc; received_packet->seq_num = ByteReader::ReadBigEndian(&media_packet->data[2]); @@ -328,7 +333,9 @@ void RunTest(bool use_flexfec) { received_packet( new ForwardErrorCorrection::ReceivedPacket()); received_packet->pkt = new ForwardErrorCorrection::Packet(); - received_packet->pkt->data = fec_packet->data; + received_packet->pkt->length = fec_packet->length; + memcpy(received_packet->pkt->data, fec_packet->data, + fec_packet->length); received_packet->seq_num = fec_seq_num_offset + seq_num; received_packet->is_fec = true; received_packet->ssrc = fec_ssrc; @@ -416,13 +423,11 @@ void RunTest(bool use_flexfec) { ForwardErrorCorrection::RecoveredPacket* recovered_packet = recovered_packet_list_it->get(); - ASSERT_EQ(recovered_packet->pkt->data.size(), - media_packet->data.size()) + ASSERT_EQ(recovered_packet->pkt->length, media_packet->length) << "Recovered packet length not identical to original " << "media packet"; - ASSERT_EQ(0, memcmp(recovered_packet->pkt->data.cdata(), - media_packet->data.cdata(), - media_packet->data.size())) + ASSERT_EQ(0, memcmp(recovered_packet->pkt->data, + media_packet->data, media_packet->length)) << "Recovered packet payload not identical to original " << "media packet"; recovered_packet_list.pop_front(); diff --git a/test/fuzzers/flexfec_header_reader_fuzzer.cc b/test/fuzzers/flexfec_header_reader_fuzzer.cc index 7d710d972f..c887d2eb7d 100644 --- a/test/fuzzers/flexfec_header_reader_fuzzer.cc +++ b/test/fuzzers/flexfec_header_reader_fuzzer.cc @@ -25,9 +25,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) { packet.pkt = rtc::scoped_refptr(new Packet()); const size_t packet_size = std::min(size, static_cast(IP_PACKET_SIZE)); - packet.pkt->data.SetSize(packet_size); - packet.pkt->data.EnsureCapacity(IP_PACKET_SIZE); - memcpy(packet.pkt->data.data(), data, packet_size); + memcpy(packet.pkt->data, data, packet_size); + packet.pkt->length = packet_size; FlexfecHeaderReader flexfec_reader; flexfec_reader.ReadFecHeader(&packet); diff --git a/test/fuzzers/forward_error_correction_fuzzer.cc b/test/fuzzers/forward_error_correction_fuzzer.cc index 09009e1649..1c37889a53 100644 --- a/test/fuzzers/forward_error_correction_fuzzer.cc +++ b/test/fuzzers/forward_error_correction_fuzzer.cc @@ -56,8 +56,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { new ForwardErrorCorrection::RecoveredPacket(); recovered_packet->pkt = rtc::scoped_refptr( new ForwardErrorCorrection::Packet()); - recovered_packet->pkt->data.SetSize(kPacketSize); - memset(recovered_packet->pkt->data.data(), 0, kPacketSize); + recovered_packet->pkt->length = kPacketSize; recovered_packet->ssrc = kMediaSsrc; recovered_packet->seq_num = media_seqnum++; recovered_packets.emplace_back(recovered_packet); @@ -67,9 +66,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) { ForwardErrorCorrection::ReceivedPacket received_packet; received_packet.pkt = rtc::scoped_refptr( new ForwardErrorCorrection::Packet()); - received_packet.pkt->data.SetSize(kPacketSize); - received_packet.pkt->data.EnsureCapacity(IP_PACKET_SIZE); - uint8_t* packet_buffer = received_packet.pkt->data.data(); + received_packet.pkt->length = kPacketSize; + uint8_t* packet_buffer = received_packet.pkt->data; uint8_t reordering; uint16_t seq_num_diff; uint8_t packet_type; diff --git a/test/fuzzers/ulpfec_generator_fuzzer.cc b/test/fuzzers/ulpfec_generator_fuzzer.cc index 306f7a0da9..ce9d8fdbc8 100644 --- a/test/fuzzers/ulpfec_generator_fuzzer.cc +++ b/test/fuzzers/ulpfec_generator_fuzzer.cc @@ -15,7 +15,6 @@ #include "modules/rtp_rtcp/source/fec_test_helper.h" #include "modules/rtp_rtcp/source/ulpfec_generator.h" #include "rtc_base/checks.h" -#include "rtc_base/copy_on_write_buffer.h" namespace webrtc { @@ -39,8 +38,10 @@ void FuzzOneInput(const uint8_t* data, size_t size) { size_t payload_size = data[i++] % 10; if (i + payload_size + rtp_header_length + 2 > size) break; - rtc::CopyOnWriteBuffer packet(&data[i], payload_size + rtp_header_length); - packet.EnsureCapacity(IP_PACKET_SIZE); + std::unique_ptr packet( + new uint8_t[payload_size + rtp_header_length]); + memcpy(packet.get(), &data[i], payload_size + rtp_header_length); + // Make sure sequence numbers are increasing. ByteWriter::WriteBigEndian(&packet[2], seq_num++); i += payload_size + rtp_header_length; @@ -51,7 +52,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) { // number became out of order. if (protect && IsNewerSequenceNumber(seq_num, prev_seq_num) && seq_num < prev_seq_num + kUlpfecMaxMediaPackets) { - generator.AddRtpPacketAndGenerateFec(packet, rtp_header_length); + generator.AddRtpPacketAndGenerateFec(packet.get(), payload_size, + rtp_header_length); prev_seq_num = seq_num; } const size_t num_fec_packets = generator.NumAvailableFecPackets(); diff --git a/test/fuzzers/ulpfec_header_reader_fuzzer.cc b/test/fuzzers/ulpfec_header_reader_fuzzer.cc index 570fa321ac..46fe67b1d2 100644 --- a/test/fuzzers/ulpfec_header_reader_fuzzer.cc +++ b/test/fuzzers/ulpfec_header_reader_fuzzer.cc @@ -25,9 +25,8 @@ void FuzzOneInput(const uint8_t* data, size_t size) { packet.pkt = rtc::scoped_refptr(new Packet()); const size_t packet_size = std::min(size, static_cast(IP_PACKET_SIZE)); - packet.pkt->data.SetSize(packet_size); - packet.pkt->data.EnsureCapacity(IP_PACKET_SIZE); - memcpy(packet.pkt->data.data(), data, packet_size); + memcpy(packet.pkt->data, data, packet_size); + packet.pkt->length = packet_size; UlpfecHeaderReader ulpfec_reader; ulpfec_reader.ReadFecHeader(&packet);