diff --git a/modules/video_coding/h264_sps_pps_tracker.cc b/modules/video_coding/h264_sps_pps_tracker.cc index 26a070530d..3965b28e8e 100644 --- a/modules/video_coding/h264_sps_pps_tracker.cc +++ b/modules/video_coding/h264_sps_pps_tracker.cc @@ -148,22 +148,16 @@ H264SpsPpsTracker::FixedBitstream H264SpsPpsTracker::CopyAndFixBitstream( // Then we copy to the new buffer. H264SpsPpsTracker::FixedBitstream fixed; - fixed.data = std::make_unique(required_size); - fixed.size = required_size; - uint8_t* insert_at = fixed.data.get(); + fixed.bitstream.EnsureCapacity(required_size); if (append_sps_pps) { // Insert SPS. - memcpy(insert_at, start_code_h264, sizeof(start_code_h264)); - insert_at += sizeof(start_code_h264); - memcpy(insert_at, sps->second.data.get(), sps->second.size); - insert_at += sps->second.size; + fixed.bitstream.AppendData(start_code_h264); + fixed.bitstream.AppendData(sps->second.data.get(), sps->second.size); // Insert PPS. - memcpy(insert_at, start_code_h264, sizeof(start_code_h264)); - insert_at += sizeof(start_code_h264); - memcpy(insert_at, pps->second.data.get(), pps->second.size); - insert_at += pps->second.size; + fixed.bitstream.AppendData(start_code_h264); + fixed.bitstream.AppendData(pps->second.data.get(), pps->second.size); // Update codec header to reflect the newly added SPS and PPS. NaluInfo sps_info; @@ -187,8 +181,7 @@ H264SpsPpsTracker::FixedBitstream H264SpsPpsTracker::CopyAndFixBitstream( if (h264_header.packetization_type == kH264StapA) { const uint8_t* nalu_ptr = bitstream.data() + 1; while (nalu_ptr < bitstream.data() + bitstream.size()) { - memcpy(insert_at, start_code_h264, sizeof(start_code_h264)); - insert_at += sizeof(start_code_h264); + fixed.bitstream.AppendData(start_code_h264); // The first two bytes describe the length of a segment. uint16_t segment_length = nalu_ptr[0] << 8 | nalu_ptr[1]; @@ -199,16 +192,14 @@ H264SpsPpsTracker::FixedBitstream H264SpsPpsTracker::CopyAndFixBitstream( return {kDrop}; } - memcpy(insert_at, nalu_ptr, segment_length); - insert_at += segment_length; + fixed.bitstream.AppendData(nalu_ptr, segment_length); nalu_ptr += segment_length; } } else { if (h264_header.nalus_length > 0) { - memcpy(insert_at, start_code_h264, sizeof(start_code_h264)); - insert_at += sizeof(start_code_h264); + fixed.bitstream.AppendData(start_code_h264); } - memcpy(insert_at, bitstream.data(), bitstream.size()); + fixed.bitstream.AppendData(bitstream.data(), bitstream.size()); } fixed.action = kInsert; diff --git a/modules/video_coding/h264_sps_pps_tracker.h b/modules/video_coding/h264_sps_pps_tracker.h index 0d1815b99f..30c4f256f8 100644 --- a/modules/video_coding/h264_sps_pps_tracker.h +++ b/modules/video_coding/h264_sps_pps_tracker.h @@ -19,6 +19,7 @@ #include "api/array_view.h" #include "modules/rtp_rtcp/source/rtp_video_header.h" +#include "rtc_base/copy_on_write_buffer.h" namespace webrtc { namespace video_coding { @@ -28,8 +29,7 @@ class H264SpsPpsTracker { enum PacketAction { kInsert, kDrop, kRequestKeyframe }; struct FixedBitstream { PacketAction action; - std::unique_ptr data; - size_t size; + rtc::CopyOnWriteBuffer bitstream; }; H264SpsPpsTracker(); diff --git a/modules/video_coding/h264_sps_pps_tracker_unittest.cc b/modules/video_coding/h264_sps_pps_tracker_unittest.cc index 00a95ec90d..04abb75e4e 100644 --- a/modules/video_coding/h264_sps_pps_tracker_unittest.cc +++ b/modules/video_coding/h264_sps_pps_tracker_unittest.cc @@ -32,7 +32,7 @@ const uint8_t start_code[] = {0, 0, 0, 1}; rtc::ArrayView Bitstream( const H264SpsPpsTracker::FixedBitstream& fixed) { - return rtc::MakeArrayView(fixed.data.get(), fixed.size); + return fixed.bitstream; } void ExpectSpsPpsIdr(const RTPVideoHeaderH264& codec_header, diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc index 30dfc21e41..b6fc521d04 100644 --- a/modules/video_coding/packet_buffer.cc +++ b/modules/video_coding/packet_buffer.cc @@ -93,8 +93,6 @@ PacketBuffer::InsertResult PacketBuffer::InsertPacket( // If we have explicitly cleared past this packet then it's old, // don't insert it, just silently ignore it. if (is_cleared_to_first_seq_num_) { - delete[] packet->data; - packet->data = nullptr; return result; } @@ -104,8 +102,6 @@ PacketBuffer::InsertResult PacketBuffer::InsertPacket( if (buffer_[index].used) { // Duplicate packet, just delete the payload. if (buffer_[index].seq_num() == packet->seq_num) { - delete[] packet->data; - packet->data = nullptr; return result; } @@ -120,8 +116,6 @@ PacketBuffer::InsertResult PacketBuffer::InsertPacket( // new keyframe is needed. RTC_LOG(LS_WARNING) << "Clear PacketBuffer and request key frame."; Clear(); - delete[] packet->data; - packet->data = nullptr; result.buffer_cleared = true; return result; } @@ -136,7 +130,6 @@ PacketBuffer::InsertResult PacketBuffer::InsertPacket( new_entry.continuous = false; new_entry.used = true; new_entry.data = std::move(*packet); - packet->data = nullptr; UpdateMissingPackets(seq_num); @@ -164,8 +157,7 @@ void PacketBuffer::ClearTo(uint16_t seq_num) { for (size_t i = 0; i < iterations; ++i) { size_t index = first_seq_num_ % buffer_.size(); if (AheadOf(seq_num, buffer_[index].seq_num())) { - delete[] buffer_[index].data.data; - buffer_[index].data.data = nullptr; + buffer_[index].data.video_payload = {}; buffer_[index].used = false; } ++first_seq_num_; @@ -191,8 +183,7 @@ void PacketBuffer::ClearInterval(uint16_t start_seq_num, for (size_t i = 0; i < iterations; ++i) { size_t index = seq_num % buffer_.size(); RTC_DCHECK_EQ(buffer_[index].seq_num(), seq_num); - delete[] buffer_[index].data.data; - buffer_[index].data.data = nullptr; + buffer_[index].data.video_payload = {}; buffer_[index].used = false; ++seq_num; @@ -202,8 +193,7 @@ void PacketBuffer::ClearInterval(uint16_t start_seq_num, void PacketBuffer::Clear() { rtc::CritScope lock(&crit_); for (StoredPacket& entry : buffer_) { - delete[] entry.data.data; - entry.data.data = nullptr; + entry.data.video_payload = {}; entry.used = false; } @@ -439,8 +429,8 @@ std::unique_ptr PacketBuffer::AssembleFrame( std::min(min_recv_time, packet.packet_info.receive_time_ms()); max_recv_time = std::max(max_recv_time, packet.packet_info.receive_time_ms()); - frame_size += packet.size_bytes; - payloads.emplace_back(packet.data, packet.size_bytes); + frame_size += packet.video_payload.size(); + payloads.emplace_back(packet.video_payload); packet_infos.push_back(packet.packet_info); } diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h index 3f420cbab0..8371a3737d 100644 --- a/modules/video_coding/packet_buffer.h +++ b/modules/video_coding/packet_buffer.h @@ -23,6 +23,7 @@ #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/rtp_video_header.h" #include "modules/video_coding/frame_object.h" +#include "rtc_base/copy_on_write_buffer.h" #include "rtc_base/critical_section.h" #include "rtc_base/numerics/sequence_number_util.h" #include "rtc_base/thread_annotations.h" @@ -64,9 +65,7 @@ class PacketBuffer { int64_t ntp_time_ms = -1; int times_nacked = -1; - const uint8_t* data = nullptr; - size_t size_bytes = 0; - + rtc::CopyOnWriteBuffer video_payload; RTPVideoHeader video_header; absl::optional generic_descriptor; diff --git a/modules/video_coding/packet_buffer_unittest.cc b/modules/video_coding/packet_buffer_unittest.cc index b1468537b3..980ac35ef7 100644 --- a/modules/video_coding/packet_buffer_unittest.cc +++ b/modules/video_coding/packet_buffer_unittest.cc @@ -96,8 +96,8 @@ class PacketBufferTest : public ::testing::Test { explicit PacketBufferTest(std::string field_trials = "") : scoped_field_trials_(field_trials), rand_(0x7732213), - clock_(new SimulatedClock(0)), - packet_buffer_(clock_.get(), kStartSize, kMaxSize) {} + clock_(0), + packet_buffer_(&clock_, kStartSize, kMaxSize) {} uint16_t Rand() { return rand_.Rand(); } @@ -109,8 +109,7 @@ class PacketBufferTest : public ::testing::Test { IsKeyFrame keyframe, // is keyframe IsFirst first, // is first packet of frame IsLast last, // is last packet of frame - int data_size = 0, // size of data - uint8_t* data = nullptr, // data pointer + rtc::ArrayView data = {}, uint32_t timestamp = 123u) { // rtp timestamp PacketBuffer::Packet packet; packet.video_header.codec = kVideoCodecGeneric; @@ -121,15 +120,14 @@ class PacketBufferTest : public ::testing::Test { : VideoFrameType::kVideoFrameDelta; packet.video_header.is_first_packet_in_frame = first == kFirst; packet.video_header.is_last_packet_in_frame = last == kLast; - packet.size_bytes = data_size; - packet.data = data; + packet.video_payload.SetData(data.data(), data.size()); return PacketBufferInsertResult(packet_buffer_.InsertPacket(&packet)); } const test::ScopedFieldTrials scoped_field_trials_; Random rand_; - std::unique_ptr clock_; + SimulatedClock clock_; PacketBuffer packet_buffer_; }; @@ -213,15 +211,15 @@ TEST_F(PacketBufferTest, NackCount) { TEST_F(PacketBufferTest, FrameSize) { const uint16_t seq_num = Rand(); - uint8_t* data1 = new uint8_t[5](); - uint8_t* data2 = new uint8_t[5](); - uint8_t* data3 = new uint8_t[5](); - uint8_t* data4 = new uint8_t[5](); + uint8_t data1[5] = {}; + uint8_t data2[5] = {}; + uint8_t data3[5] = {}; + uint8_t data4[5] = {}; - Insert(seq_num, kKeyFrame, kFirst, kNotLast, 5, data1); - Insert(seq_num + 1, kKeyFrame, kNotFirst, kNotLast, 5, data2); - Insert(seq_num + 2, kKeyFrame, kNotFirst, kNotLast, 5, data3); - EXPECT_THAT(Insert(seq_num + 3, kKeyFrame, kNotFirst, kLast, 5, data4).frames, + Insert(seq_num, kKeyFrame, kFirst, kNotLast, data1); + Insert(seq_num + 1, kKeyFrame, kNotFirst, kNotLast, data2); + Insert(seq_num + 2, kKeyFrame, kNotFirst, kNotLast, data3); + EXPECT_THAT(Insert(seq_num + 3, kKeyFrame, kNotFirst, kLast, data4).frames, ElementsAre(Pointee(SizeIs(20)))); } @@ -377,32 +375,18 @@ TEST_F(PacketBufferTest, FramesReordered) { TEST_F(PacketBufferTest, GetBitstream) { // "many bitstream, such data" with null termination. - uint8_t many_data[] = {0x6d, 0x61, 0x6e, 0x79, 0x20}; - uint8_t bitstream_data[] = {0x62, 0x69, 0x74, 0x73, 0x74, 0x72, - 0x65, 0x61, 0x6d, 0x2c, 0x20}; - uint8_t such_data[] = {0x73, 0x75, 0x63, 0x68, 0x20}; - uint8_t data_data[] = {0x64, 0x61, 0x74, 0x61, 0x0}; - - uint8_t* many = new uint8_t[sizeof(many_data)]; - uint8_t* bitstream = new uint8_t[sizeof(bitstream_data)]; - uint8_t* such = new uint8_t[sizeof(such_data)]; - uint8_t* data = new uint8_t[sizeof(data_data)]; - - memcpy(many, many_data, sizeof(many_data)); - memcpy(bitstream, bitstream_data, sizeof(bitstream_data)); - memcpy(such, such_data, sizeof(such_data)); - memcpy(data, data_data, sizeof(data_data)); + uint8_t many[] = {0x6d, 0x61, 0x6e, 0x79, 0x20}; + uint8_t bitstream[] = {0x62, 0x69, 0x74, 0x73, 0x74, 0x72, + 0x65, 0x61, 0x6d, 0x2c, 0x20}; + uint8_t such[] = {0x73, 0x75, 0x63, 0x68, 0x20}; + uint8_t data[] = {0x64, 0x61, 0x74, 0x61, 0x0}; const uint16_t seq_num = Rand(); - Insert(seq_num, kKeyFrame, kFirst, kNotLast, sizeof(many_data), many); - Insert(seq_num + 1, kDeltaFrame, kNotFirst, kNotLast, sizeof(bitstream_data), - bitstream); - Insert(seq_num + 2, kDeltaFrame, kNotFirst, kNotLast, sizeof(such_data), - such); - auto frames = Insert(seq_num + 3, kDeltaFrame, kNotFirst, kLast, - sizeof(data_data), data) - .frames; + Insert(seq_num, kKeyFrame, kFirst, kNotLast, many); + Insert(seq_num + 1, kDeltaFrame, kNotFirst, kNotLast, bitstream); + Insert(seq_num + 2, kDeltaFrame, kNotFirst, kNotLast, such); + auto frames = Insert(seq_num + 3, kDeltaFrame, kNotFirst, kLast, data).frames; ASSERT_THAT(frames, SizeIs(1)); EXPECT_EQ(frames[0]->first_seq_num(), seq_num); @@ -411,31 +395,27 @@ TEST_F(PacketBufferTest, GetBitstream) { } TEST_F(PacketBufferTest, GetBitstreamOneFrameOnePacket) { - uint8_t bitstream_data[] = "All the bitstream data for this frame!"; - uint8_t* data = new uint8_t[sizeof(bitstream_data)]; - memcpy(data, bitstream_data, sizeof(bitstream_data)); + uint8_t bitstream[] = "All the bitstream data for this frame!"; - auto frames = - Insert(0, kKeyFrame, kFirst, kLast, sizeof(bitstream_data), data).frames; + auto frames = Insert(0, kKeyFrame, kFirst, kLast, bitstream).frames; ASSERT_THAT(StartSeqNums(frames), ElementsAre(0)); EXPECT_THAT(rtc::MakeArrayView(frames[0]->data(), frames[0]->size()), - ElementsAreArray(bitstream_data)); + ElementsAreArray(bitstream)); } TEST_F(PacketBufferTest, GetBitstreamOneFrameFullBuffer) { - uint8_t* data_arr[kStartSize]; + uint8_t data_arr[kStartSize][1]; uint8_t expected[kStartSize]; for (uint8_t i = 0; i < kStartSize; ++i) { - data_arr[i] = new uint8_t[1]; data_arr[i][0] = i; expected[i] = i; } - Insert(0, kKeyFrame, kFirst, kNotLast, 1, data_arr[0]); + Insert(0, kKeyFrame, kFirst, kNotLast, data_arr[0]); for (uint8_t i = 1; i < kStartSize - 1; ++i) - Insert(i, kKeyFrame, kNotFirst, kNotLast, 1, data_arr[i]); - auto frames = Insert(kStartSize - 1, kKeyFrame, kNotFirst, kLast, 1, + Insert(i, kKeyFrame, kNotFirst, kNotLast, data_arr[i]); + auto frames = Insert(kStartSize - 1, kKeyFrame, kNotFirst, kLast, data_arr[kStartSize - 1]) .frames; @@ -448,18 +428,12 @@ TEST_F(PacketBufferTest, GetBitstreamAv1) { const uint8_t data1[] = {0b01'01'0000, 0b0'0100'000, 'm', 'a', 'n', 'y', ' '}; const uint8_t data2[] = {0b10'01'0000, 'b', 'i', 't', 's', 0}; - uint8_t* new_data1 = new uint8_t[sizeof(data1)]; - memcpy(new_data1, data1, sizeof(data1)); - uint8_t* new_data2 = new uint8_t[sizeof(data2)]; - memcpy(new_data2, data2, sizeof(data2)); - PacketBuffer::Packet packet1; packet1.video_header.codec = kVideoCodecAV1; packet1.seq_num = 13; packet1.video_header.is_first_packet_in_frame = true; packet1.video_header.is_last_packet_in_frame = false; - packet1.size_bytes = sizeof(data1); - packet1.data = new_data1; + packet1.video_payload = data1; auto frames = packet_buffer_.InsertPacket(&packet1).frames; EXPECT_THAT(frames, IsEmpty()); @@ -468,8 +442,7 @@ TEST_F(PacketBufferTest, GetBitstreamAv1) { packet2.seq_num = 14; packet2.video_header.is_first_packet_in_frame = false; packet2.video_header.is_last_packet_in_frame = true; - packet2.size_bytes = sizeof(data2); - packet2.data = new_data2; + packet2.video_payload = data2; frames = packet_buffer_.InsertPacket(&packet2).frames; ASSERT_THAT(frames, SizeIs(1)); @@ -485,18 +458,12 @@ TEST_F(PacketBufferTest, GetBitstreamInvalidAv1) { const uint8_t data1[] = {0b01'01'0000, 0b0'0100'000, 'm', 'a', 'n', 'y', ' '}; const uint8_t data2[] = {0b00'01'0000, 'b', 'i', 't', 's', 0}; - uint8_t* new_data1 = new uint8_t[sizeof(data1)]; - memcpy(new_data1, data1, sizeof(data1)); - uint8_t* new_data2 = new uint8_t[sizeof(data2)]; - memcpy(new_data2, data2, sizeof(data2)); - PacketBuffer::Packet packet1; packet1.video_header.codec = kVideoCodecAV1; packet1.seq_num = 13; packet1.video_header.is_first_packet_in_frame = true; packet1.video_header.is_last_packet_in_frame = false; - packet1.size_bytes = sizeof(data1); - packet1.data = new_data1; + packet1.video_payload = data1; auto frames = packet_buffer_.InsertPacket(&packet1).frames; EXPECT_THAT(frames, IsEmpty()); @@ -505,8 +472,7 @@ TEST_F(PacketBufferTest, GetBitstreamInvalidAv1) { packet2.seq_num = 14; packet2.video_header.is_first_packet_in_frame = false; packet2.video_header.is_last_packet_in_frame = true; - packet2.size_bytes = sizeof(data2); - packet2.data = new_data2; + packet2.video_payload = data2; frames = packet_buffer_.InsertPacket(&packet2).frames; EXPECT_THAT(frames, IsEmpty()); @@ -521,22 +487,21 @@ TEST_F(PacketBufferTest, InsertPacketAfterSequenceNumberWrapAround) { // Loop until seq_num wraps around. SeqNumUnwrapper unwrapper; while (unwrapper.Unwrap(seq_num) < std::numeric_limits::max()) { - Insert(seq_num++, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp); + Insert(seq_num++, kKeyFrame, kFirst, kNotLast, {}, timestamp); for (int i = 0; i < 5; ++i) { - Insert(seq_num++, kKeyFrame, kNotFirst, kNotLast, 0, nullptr, timestamp); + Insert(seq_num++, kKeyFrame, kNotFirst, kNotLast, {}, timestamp); } - Insert(seq_num++, kKeyFrame, kNotFirst, kLast, 0, nullptr, timestamp); + Insert(seq_num++, kKeyFrame, kNotFirst, kLast, {}, timestamp); timestamp += kTimestampDelta; } // Receive frame with overlapping sequence numbers. - Insert(seq_num++, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp); + Insert(seq_num++, kKeyFrame, kFirst, kNotLast, {}, timestamp); for (int i = 0; i < 5; ++i) { - Insert(seq_num++, kKeyFrame, kNotFirst, kNotLast, 0, nullptr, timestamp); + Insert(seq_num++, kKeyFrame, kNotFirst, kNotLast, {}, timestamp); } EXPECT_THAT( - Insert(seq_num++, kKeyFrame, kNotFirst, kLast, 0, nullptr, timestamp) - .frames, + Insert(seq_num++, kKeyFrame, kNotFirst, kLast, {}, timestamp).frames, SizeIs(1)); } @@ -553,15 +518,14 @@ class PacketBufferH264Test : public PacketBufferTest { sps_pps_idr_is_keyframe_(sps_pps_idr_is_keyframe) {} PacketBufferInsertResult InsertH264( - uint16_t seq_num, // packet sequence number - IsKeyFrame keyframe, // is keyframe - IsFirst first, // is first packet of frame - IsLast last, // is last packet of frame - uint32_t timestamp, // rtp timestamp - int data_size = 0, // size of data - uint8_t* data = nullptr, // data pointer - uint32_t width = 0, // width of frame (SPS/IDR) - uint32_t height = 0) { // height of frame (SPS/IDR) + uint16_t seq_num, // packet sequence number + IsKeyFrame keyframe, // is keyframe + IsFirst first, // is first packet of frame + IsLast last, // is last packet of frame + uint32_t timestamp, // rtp timestamp + rtc::ArrayView data = {}, + uint32_t width = 0, // width of frame (SPS/IDR) + uint32_t height = 0) { // height of frame (SPS/IDR) PacketBuffer::Packet packet; packet.video_header.codec = kVideoCodecH264; auto& h264_header = @@ -583,22 +547,20 @@ class PacketBufferH264Test : public PacketBufferTest { packet.video_header.height = height; packet.video_header.is_first_packet_in_frame = first == kFirst; packet.video_header.is_last_packet_in_frame = last == kLast; - packet.size_bytes = data_size; - packet.data = data; + packet.video_payload.SetData(data.data(), data.size()); return PacketBufferInsertResult(packet_buffer_.InsertPacket(&packet)); } PacketBufferInsertResult InsertH264KeyFrameWithAud( - uint16_t seq_num, // packet sequence number - IsKeyFrame keyframe, // is keyframe - IsFirst first, // is first packet of frame - IsLast last, // is last packet of frame - uint32_t timestamp, // rtp timestamp - int data_size = 0, // size of data - uint8_t* data = nullptr, // data pointer - uint32_t width = 0, // width of frame (SPS/IDR) - uint32_t height = 0) { // height of frame (SPS/IDR) + uint16_t seq_num, // packet sequence number + IsKeyFrame keyframe, // is keyframe + IsFirst first, // is first packet of frame + IsLast last, // is last packet of frame + uint32_t timestamp, // rtp timestamp + rtc::ArrayView data = {}, + uint32_t width = 0, // width of frame (SPS/IDR) + uint32_t height = 0) { // height of frame (SPS/IDR) PacketBuffer::Packet packet; packet.video_header.codec = kVideoCodecH264; auto& h264_header = @@ -614,12 +576,10 @@ class PacketBufferH264Test : public PacketBufferTest { h264_header.nalus_length = 1; packet.video_header.is_first_packet_in_frame = true; packet.video_header.is_last_packet_in_frame = false; - packet.size_bytes = 0; - packet.data = nullptr; IgnoreResult(packet_buffer_.InsertPacket(&packet)); // insert IDR - return InsertH264(seq_num + 1, keyframe, kNotFirst, last, timestamp, - data_size, data, width, height); + return InsertH264(seq_num + 1, keyframe, kNotFirst, last, timestamp, data, + width, height); } const bool sps_pps_idr_is_keyframe_; @@ -648,21 +608,20 @@ TEST_P(PacketBufferH264ParameterizedTest, DontRemoveMissingPacketOnClearTo) { } TEST_P(PacketBufferH264ParameterizedTest, GetBitstreamOneFrameFullBuffer) { - uint8_t* data_arr[kStartSize]; + uint8_t data_arr[kStartSize][1]; uint8_t expected[kStartSize]; for (uint8_t i = 0; i < kStartSize; ++i) { - data_arr[i] = new uint8_t[1]; data_arr[i][0] = i; expected[i] = i; } - InsertH264(0, kKeyFrame, kFirst, kNotLast, 1, 1, data_arr[0]); + InsertH264(0, kKeyFrame, kFirst, kNotLast, 1, data_arr[0]); for (uint8_t i = 1; i < kStartSize - 1; ++i) { - InsertH264(i, kKeyFrame, kNotFirst, kNotLast, 1, 1, data_arr[i]); + InsertH264(i, kKeyFrame, kNotFirst, kNotLast, 1, data_arr[i]); } - auto frames = InsertH264(kStartSize - 1, kKeyFrame, kNotFirst, kLast, 1, 1, + auto frames = InsertH264(kStartSize - 1, kKeyFrame, kNotFirst, kLast, 1, data_arr[kStartSize - 1]) .frames; ASSERT_THAT(StartSeqNums(frames), ElementsAre(0)); @@ -672,9 +631,7 @@ TEST_P(PacketBufferH264ParameterizedTest, GetBitstreamOneFrameFullBuffer) { TEST_P(PacketBufferH264ParameterizedTest, GetBitstreamBufferPadding) { uint16_t seq_num = Rand(); - uint8_t data_data[] = "some plain old data"; - uint8_t* data = new uint8_t[sizeof(data_data)]; - memcpy(data, data_data, sizeof(data_data)); + uint8_t data[] = "some plain old data"; PacketBuffer::Packet packet; auto& h264_header = @@ -684,62 +641,56 @@ TEST_P(PacketBufferH264ParameterizedTest, GetBitstreamBufferPadding) { h264_header.packetization_type = kH264SingleNalu; packet.seq_num = seq_num; packet.video_header.codec = kVideoCodecH264; - packet.data = data; - packet.size_bytes = sizeof(data_data); + packet.video_payload = data; packet.video_header.is_first_packet_in_frame = true; packet.video_header.is_last_packet_in_frame = true; auto frames = packet_buffer_.InsertPacket(&packet).frames; ASSERT_THAT(frames, SizeIs(1)); EXPECT_EQ(frames[0]->first_seq_num(), seq_num); - EXPECT_EQ(frames[0]->EncodedImage().size(), sizeof(data_data)); - EXPECT_EQ(frames[0]->EncodedImage().capacity(), sizeof(data_data)); + EXPECT_EQ(frames[0]->EncodedImage().size(), sizeof(data)); + EXPECT_EQ(frames[0]->EncodedImage().capacity(), sizeof(data)); EXPECT_THAT(rtc::MakeArrayView(frames[0]->data(), frames[0]->size()), - ElementsAreArray(data_data)); + ElementsAreArray(data)); } TEST_P(PacketBufferH264ParameterizedTest, FrameResolution) { uint16_t seq_num = 100; - uint8_t data_data[] = "some plain old data"; - uint8_t* data = new uint8_t[sizeof(data_data)]; - memcpy(data, data_data, sizeof(data_data)); + uint8_t data[] = "some plain old data"; uint32_t width = 640; uint32_t height = 360; uint32_t timestamp = 1000; - auto frames = InsertH264(seq_num, kKeyFrame, kFirst, kLast, timestamp, - sizeof(data_data), data, width, height) + auto frames = InsertH264(seq_num, kKeyFrame, kFirst, kLast, timestamp, data, + width, height) .frames; ASSERT_THAT(frames, SizeIs(1)); EXPECT_THAT(rtc::MakeArrayView(frames[0]->data(), frames[0]->size()), - ElementsAreArray(data_data)); + ElementsAreArray(data)); EXPECT_EQ(frames[0]->EncodedImage()._encodedWidth, width); EXPECT_EQ(frames[0]->EncodedImage()._encodedHeight, height); } TEST_P(PacketBufferH264ParameterizedTest, FrameResolutionNaluBeforeSPS) { uint16_t seq_num = 100; - uint8_t data_data[] = "some plain old data"; - uint8_t* data = new uint8_t[sizeof(data_data)]; - memcpy(data, data_data, sizeof(data_data)); + uint8_t data[] = "some plain old data"; uint32_t width = 640; uint32_t height = 360; uint32_t timestamp = 1000; - auto frames = - InsertH264KeyFrameWithAud(seq_num, kKeyFrame, kFirst, kLast, timestamp, - sizeof(data_data), data, width, height) - .frames; + auto frames = InsertH264KeyFrameWithAud(seq_num, kKeyFrame, kFirst, kLast, + timestamp, data, width, height) + .frames; ASSERT_THAT(StartSeqNums(frames), ElementsAre(seq_num)); - EXPECT_EQ(frames[0]->EncodedImage().size(), sizeof(data_data)); - EXPECT_EQ(frames[0]->EncodedImage().capacity(), sizeof(data_data)); + EXPECT_EQ(frames[0]->EncodedImage().size(), sizeof(data)); + EXPECT_EQ(frames[0]->EncodedImage().capacity(), sizeof(data)); EXPECT_EQ(frames[0]->EncodedImage()._encodedWidth, width); EXPECT_EQ(frames[0]->EncodedImage()._encodedHeight, height); EXPECT_THAT(rtc::MakeArrayView(frames[0]->data(), frames[0]->size()), - ElementsAreArray(data_data)); + ElementsAreArray(data)); } TEST_F(PacketBufferTest, FreeSlotsOnFrameCreation) { @@ -783,33 +734,11 @@ TEST_F(PacketBufferTest, FramesAfterClear) { } TEST_F(PacketBufferTest, SameFrameDifferentTimestamps) { - Insert(0, kKeyFrame, kFirst, kNotLast, 0, nullptr, 1000); - EXPECT_THAT(Insert(1, kKeyFrame, kNotFirst, kLast, 0, nullptr, 1001).frames, + Insert(0, kKeyFrame, kFirst, kNotLast, {}, 1000); + EXPECT_THAT(Insert(1, kKeyFrame, kNotFirst, kLast, {}, 1001).frames, IsEmpty()); } -TEST_F(PacketBufferTest, DontLeakPayloadData) { - // NOTE! Any eventual leak is suppose to be detected by valgrind - // or any other similar tool. - uint8_t* data1 = new uint8_t[5]; - uint8_t* data2 = new uint8_t[5]; - uint8_t* data3 = new uint8_t[5]; - uint8_t* data4 = new uint8_t[5]; - - // Expected to free data1 upon PacketBuffer destruction. - Insert(2, kKeyFrame, kFirst, kNotLast, 5, data1); - - // Expect to free data2 upon insertion. - Insert(2, kKeyFrame, kFirst, kNotLast, 5, data2); - - // Expect to free data3 upon insertion (old packet). - packet_buffer_.ClearTo(1); - Insert(1, kKeyFrame, kFirst, kNotLast, 5, data3); - - // Expect to free data4 upon insertion (packet buffer is full). - Insert(2 + kMaxSize, kKeyFrame, kFirst, kNotLast, 5, data4); -} - TEST_F(PacketBufferTest, ContinuousSeqNumDoubleMarkerBit) { Insert(2, kKeyFrame, kNotFirst, kNotLast); Insert(1, kKeyFrame, kFirst, kLast); @@ -825,7 +754,7 @@ TEST_F(PacketBufferTest, PacketTimestamps) { EXPECT_FALSE(packet_ms); EXPECT_FALSE(packet_keyframe_ms); - int64_t keyframe_ms = clock_->TimeInMilliseconds(); + int64_t keyframe_ms = clock_.TimeInMilliseconds(); Insert(100, kKeyFrame, kFirst, kLast); packet_ms = packet_buffer_.LastReceivedPacketMs(); packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs(); @@ -834,8 +763,8 @@ TEST_F(PacketBufferTest, PacketTimestamps) { EXPECT_EQ(keyframe_ms, *packet_ms); EXPECT_EQ(keyframe_ms, *packet_keyframe_ms); - clock_->AdvanceTimeMilliseconds(100); - int64_t delta_ms = clock_->TimeInMilliseconds(); + clock_.AdvanceTimeMilliseconds(100); + int64_t delta_ms = clock_.TimeInMilliseconds(); Insert(101, kDeltaFrame, kFirst, kLast); packet_ms = packet_buffer_.LastReceivedPacketMs(); packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs(); @@ -855,8 +784,6 @@ TEST_F(PacketBufferTest, IncomingCodecChange) { PacketBuffer::Packet packet; packet.video_header.is_first_packet_in_frame = true; packet.video_header.is_last_packet_in_frame = true; - packet.size_bytes = 0; - packet.data = nullptr; packet.video_header.codec = kVideoCodecVP8; packet.video_header.video_type_header.emplace(); @@ -892,8 +819,6 @@ TEST_F(PacketBufferTest, TooManyNalusInPacket) { auto& h264_header = packet.video_header.video_type_header.emplace(); h264_header.nalus_length = kMaxNalusPerPacket; - packet.size_bytes = 0; - packet.data = nullptr; EXPECT_THAT(packet_buffer_.InsertPacket(&packet).frames, IsEmpty()); } diff --git a/test/fuzzers/packet_buffer_fuzzer.cc b/test/fuzzers/packet_buffer_fuzzer.cc index a68cafe477..3c4badbb08 100644 --- a/test/fuzzers/packet_buffer_fuzzer.cc +++ b/test/fuzzers/packet_buffer_fuzzer.cc @@ -21,39 +21,23 @@ void FuzzOneInput(const uint8_t* data, size_t size) { if (size > 200000) { return; } - video_coding::PacketBuffer::Packet packet; SimulatedClock clock(0); video_coding::PacketBuffer packet_buffer(&clock, 8, 1024); test::FuzzDataHelper helper(rtc::ArrayView(data, size)); while (helper.BytesLeft()) { - // Complex types (e.g. non-POD-like types) can't be bit-wise fuzzed with - // random data or it will put them in an invalid state. We therefore backup - // their byte-patterns before the fuzzing and restore them after. - uint8_t video_header_backup[sizeof(packet.video_header)]; - memcpy(&video_header_backup, &packet.video_header, - sizeof(packet.video_header)); - uint8_t generic_descriptor_backup[sizeof(packet.generic_descriptor)]; - memcpy(&generic_descriptor_backup, &packet.generic_descriptor, - sizeof(packet.generic_descriptor)); - uint8_t packet_info_backup[sizeof(packet.packet_info)]; - memcpy(&packet_info_backup, &packet.packet_info, - sizeof(packet.packet_info)); + video_coding::PacketBuffer::Packet packet; + // Fuzz POD members of the packet. + helper.CopyTo(&packet.marker_bit); + helper.CopyTo(&packet.payload_type); + helper.CopyTo(&packet.seq_num); + helper.CopyTo(&packet.timestamp); + helper.CopyTo(&packet.ntp_time_ms); + helper.CopyTo(&packet.times_nacked); - helper.CopyTo(&packet); - - memcpy(&packet.video_header, &video_header_backup, - sizeof(packet.video_header)); - memcpy(&packet.generic_descriptor, &generic_descriptor_backup, - sizeof(packet.generic_descriptor)); - memcpy(&packet.packet_info, &packet_info_backup, - sizeof(packet.packet_info)); - - // The packet buffer owns the payload of the packet. - uint8_t payload_size; - helper.CopyTo(&payload_size); - packet.size_bytes = payload_size; - packet.data = new uint8_t[payload_size]; + // Fuzz non-POD member of the packet. + packet.video_payload.SetSize(helper.ReadOrDefaultValue(0)); + // TODO(danilchap): Fuzz other non-POD members of the |packet|. IgnoreResult(packet_buffer.InsertPacket(&packet)); } diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 563aca2841..3373024986 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -466,16 +466,12 @@ void RtpVideoStreamReceiver::OnReceivedPayloadData( case video_coding::H264SpsPpsTracker::kDrop: return; case video_coding::H264SpsPpsTracker::kInsert: - packet.data = fixed.data.release(); - packet.size_bytes = fixed.size; + packet.video_payload = std::move(fixed.bitstream); break; } } else { - packet.size_bytes = codec_payload.size(); - uint8_t* data = new uint8_t[packet.size_bytes]; - memcpy(data, codec_payload.data(), codec_payload.size()); - packet.data = data; + packet.video_payload.SetData(codec_payload.data(), codec_payload.size()); } rtcp_feedback_buffer_.SendBufferedRtcpFeedback();