diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc index b5f9fa7bf9..a046b549a9 100644 --- a/modules/video_coding/packet_buffer.cc +++ b/modules/video_coding/packet_buffer.cc @@ -22,7 +22,6 @@ #include "modules/rtp_rtcp/source/rtp_video_header.h" #include "modules/video_coding/codecs/h264/include/h264_globals.h" #include "modules/video_coding/frame_object.h" -#include "rtc_base/atomic_ops.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/numerics/mod_ops.h" @@ -32,15 +31,6 @@ namespace webrtc { namespace video_coding { -rtc::scoped_refptr PacketBuffer::Create( - Clock* clock, - size_t start_buffer_size, - size_t max_buffer_size, - OnAssembledFrameCallback* assembled_frame_callback) { - return rtc::scoped_refptr(new PacketBuffer( - clock, start_buffer_size, max_buffer_size, assembled_frame_callback)); -} - PacketBuffer::PacketBuffer(Clock* clock, size_t start_buffer_size, size_t max_buffer_size, @@ -483,18 +473,6 @@ VCMPacket* PacketBuffer::GetPacket(uint16_t seq_num) { return &data_buffer_[index]; } -int PacketBuffer::AddRef() const { - return rtc::AtomicOps::Increment(&ref_count_); -} - -int PacketBuffer::Release() const { - int count = rtc::AtomicOps::Decrement(&ref_count_); - if (!count) { - delete this; - } - return count; -} - void PacketBuffer::UpdateMissingPackets(uint16_t seq_num) { if (!newest_inserted_seq_num_) newest_inserted_seq_num_ = seq_num; diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h index 77fa6ad8be..ebbd974ff4 100644 --- a/modules/video_coding/packet_buffer.h +++ b/modules/video_coding/packet_buffer.h @@ -16,7 +16,6 @@ #include #include -#include "api/scoped_refptr.h" #include "api/video/encoded_image.h" #include "modules/include/module_common_types.h" #include "modules/video_coding/packet.h" @@ -41,12 +40,11 @@ class OnAssembledFrameCallback { class PacketBuffer { public: - static rtc::scoped_refptr Create( - Clock* clock, - size_t start_buffer_size, - size_t max_buffer_size, - OnAssembledFrameCallback* frame_callback); - + // Both |start_buffer_size| and |max_buffer_size| must be a power of 2. + PacketBuffer(Clock* clock, + size_t start_buffer_size, + size_t max_buffer_size, + OnAssembledFrameCallback* frame_callback); virtual ~PacketBuffer(); // Returns true unless the packet buffer is cleared, which means that a key @@ -65,16 +63,6 @@ class PacketBuffer { // Returns number of different frames seen in the packet buffer int GetUniqueFramesSeen() const; - int AddRef() const; - int Release() const; - - protected: - // Both |start_buffer_size| and |max_buffer_size| must be a power of 2. - PacketBuffer(Clock* clock, - size_t start_buffer_size, - size_t max_buffer_size, - OnAssembledFrameCallback* frame_callback); - private: friend RtpFrameObject; // Since we want the packet buffer to be as packet type agnostic @@ -181,8 +169,6 @@ class PacketBuffer { std::set rtp_timestamps_history_set_ RTC_GUARDED_BY(crit_); // Stores the same unique timestamps in the order of insertion. std::queue rtp_timestamps_history_queue_ RTC_GUARDED_BY(crit_); - - mutable volatile int ref_count_ = 0; }; } // namespace video_coding diff --git a/modules/video_coding/rtp_frame_reference_finder_unittest.cc b/modules/video_coding/rtp_frame_reference_finder_unittest.cc index 9d9ba0b73c..5fe5c09333 100644 --- a/modules/video_coding/rtp_frame_reference_finder_unittest.cc +++ b/modules/video_coding/rtp_frame_reference_finder_unittest.cc @@ -49,7 +49,6 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, protected: TestRtpFrameReferenceFinder() : rand_(0x8739211), - ref_packet_buffer_(new FakePacketBuffer()), reference_finder_(new RtpFrameReferenceFinder(this)), frames_from_callback_(FrameComp()) {} @@ -78,15 +77,15 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, packet.video_header.frame_type = keyframe ? VideoFrameType::kVideoFrameKey : VideoFrameType::kVideoFrameDelta; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); packet.seqNum = seq_num_end; packet.video_header.is_last_packet_in_frame = true; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); - std::unique_ptr frame( - new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, - 0, {}, EncodedImageBuffer::Create(/*size=*/0))); + auto frame = std::make_unique( + &packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, RtpPacketInfos(), + EncodedImageBuffer::Create(/*size=*/0)); reference_finder_->ManageFrame(std::move(frame)); } @@ -111,17 +110,17 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, vp8_header.temporalIdx = tid; vp8_header.tl0PicIdx = tl0; vp8_header.layerSync = sync; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); if (seq_num_start != seq_num_end) { packet.seqNum = seq_num_end; packet.video_header.is_last_packet_in_frame = true; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); } - std::unique_ptr frame( - new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, - 0, {}, EncodedImageBuffer::Create(/*size=*/0))); + auto frame = std::make_unique( + &packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, RtpPacketInfos(), + EncodedImageBuffer::Create(/*size=*/0)); reference_finder_->ManageFrame(std::move(frame)); } @@ -157,18 +156,18 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, vp9_header.ss_data_available = true; vp9_header.gof = *ss; } - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); if (seq_num_start != seq_num_end) { packet.video_header.is_last_packet_in_frame = true; vp9_header.ss_data_available = false; packet.seqNum = seq_num_end; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); } - std::unique_ptr frame( - new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, - 0, {}, EncodedImageBuffer::Create(/*size=*/0))); + auto frame = std::make_unique( + &packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, RtpPacketInfos(), + EncodedImageBuffer::Create(/*size=*/0)); reference_finder_->ManageFrame(std::move(frame)); } @@ -200,17 +199,17 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, vp9_header.num_ref_pics = refs.size(); for (size_t i = 0; i < refs.size(); ++i) vp9_header.pid_diff[i] = refs[i]; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); if (seq_num_start != seq_num_end) { packet.seqNum = seq_num_end; packet.video_header.is_last_packet_in_frame = true; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); } - std::unique_ptr frame( - new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, - 0, {}, EncodedImageBuffer::Create(/*size=*/0))); + auto frame = std::make_unique( + &packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, RtpPacketInfos(), + EncodedImageBuffer::Create(/*size=*/0)); reference_finder_->ManageFrame(std::move(frame)); } @@ -231,17 +230,17 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, packet.video_header.frame_marking.temporal_id = tid; packet.video_header.frame_marking.tl0_pic_idx = tl0; packet.video_header.frame_marking.base_layer_sync = sync; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); if (seq_num_start != seq_num_end) { packet.seqNum = seq_num_end; packet.video_header.is_last_packet_in_frame = true; - ref_packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); } - std::unique_ptr frame( - new RtpFrameObject(ref_packet_buffer_, seq_num_start, seq_num_end, 0, 0, - 0, {}, EncodedImageBuffer::Create(/*size=*/0))); + auto frame = std::make_unique( + &packet_buffer_, seq_num_start, seq_num_end, 0, 0, 0, RtpPacketInfos(), + EncodedImageBuffer::Create(/*size=*/0)); reference_finder_->ManageFrame(std::move(frame)); } @@ -299,7 +298,7 @@ class TestRtpFrameReferenceFinder : public ::testing::Test, void RefsToSet(std::set* m) const {} Random rand_; - rtc::scoped_refptr ref_packet_buffer_; + FakePacketBuffer packet_buffer_; std::unique_ptr reference_finder_; struct FrameComp { bool operator()(const std::pair f1, diff --git a/modules/video_coding/video_packet_buffer_unittest.cc b/modules/video_coding/video_packet_buffer_unittest.cc index ae411734b0..efe2eccd29 100644 --- a/modules/video_coding/video_packet_buffer_unittest.cc +++ b/modules/video_coding/video_packet_buffer_unittest.cc @@ -32,8 +32,7 @@ class TestPacketBuffer : public ::testing::Test, : scoped_field_trials_(field_trials), rand_(0x7732213), clock_(new SimulatedClock(0)), - packet_buffer_( - PacketBuffer::Create(clock_.get(), kStartSize, kMaxSize, this)) {} + packet_buffer_(clock_.get(), kStartSize, kMaxSize, this) {} uint16_t Rand() { return rand_.Rand(); } @@ -73,7 +72,7 @@ class TestPacketBuffer : public ::testing::Test, packet.sizeBytes = data_size; packet.dataPtr = data; - return packet_buffer_->InsertPacket(&packet); + return packet_buffer_.InsertPacket(&packet); } void CheckFrame(uint16_t first_seq_num) { @@ -98,7 +97,7 @@ class TestPacketBuffer : public ::testing::Test, Random rand_; std::unique_ptr clock_; - rtc::scoped_refptr packet_buffer_; + PacketBuffer packet_buffer_; std::map> frames_from_callback_; }; @@ -153,7 +152,7 @@ TEST_F(TestPacketBuffer, InsertOldPackets) { ASSERT_TRUE(Insert(seq_num, kKeyFrame, kFirst, kNotLast)); EXPECT_TRUE(Insert(seq_num + 2, kDeltaFrame, kFirst, kLast)); - packet_buffer_->ClearTo(seq_num + 2); + packet_buffer_.ClearTo(seq_num + 2); EXPECT_TRUE(Insert(seq_num + 2, kDeltaFrame, kFirst, kLast)); EXPECT_TRUE(Insert(seq_num + 3, kDeltaFrame, kFirst, kLast)); ASSERT_EQ(2UL, frames_from_callback_.size()); @@ -170,21 +169,21 @@ TEST_F(TestPacketBuffer, NackCount) { packet.video_header.is_last_packet_in_frame = false; packet.timesNacked = 0; - packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); packet.seqNum++; packet.video_header.is_first_packet_in_frame = false; packet.timesNacked = 1; - packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); packet.seqNum++; packet.timesNacked = 3; - packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); packet.seqNum++; packet.video_header.is_last_packet_in_frame = true; packet.timesNacked = 1; - packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); ASSERT_EQ(1UL, frames_from_callback_.size()); RtpFrameObject* frame = frames_from_callback_.begin()->second.get(); @@ -210,34 +209,34 @@ TEST_F(TestPacketBuffer, FrameSize) { TEST_F(TestPacketBuffer, CountsUniqueFrames) { const uint16_t seq_num = Rand(); - ASSERT_EQ(0, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(0, packet_buffer_.GetUniqueFramesSeen()); EXPECT_TRUE(Insert(seq_num, kKeyFrame, kFirst, kNotLast, 0, nullptr, 100)); - ASSERT_EQ(1, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(1, packet_buffer_.GetUniqueFramesSeen()); // Still the same frame. EXPECT_TRUE( Insert(seq_num + 1, kKeyFrame, kNotFirst, kLast, 0, nullptr, 100)); - ASSERT_EQ(1, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(1, packet_buffer_.GetUniqueFramesSeen()); // Second frame. EXPECT_TRUE( Insert(seq_num + 2, kKeyFrame, kFirst, kNotLast, 0, nullptr, 200)); - ASSERT_EQ(2, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(2, packet_buffer_.GetUniqueFramesSeen()); EXPECT_TRUE( Insert(seq_num + 3, kKeyFrame, kNotFirst, kLast, 0, nullptr, 200)); - ASSERT_EQ(2, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(2, packet_buffer_.GetUniqueFramesSeen()); // Old packet. EXPECT_TRUE( Insert(seq_num + 1, kKeyFrame, kNotFirst, kLast, 0, nullptr, 100)); - ASSERT_EQ(2, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(2, packet_buffer_.GetUniqueFramesSeen()); // Missing middle packet. EXPECT_TRUE( Insert(seq_num + 4, kKeyFrame, kFirst, kNotLast, 0, nullptr, 300)); EXPECT_TRUE( Insert(seq_num + 6, kKeyFrame, kNotFirst, kLast, 0, nullptr, 300)); - ASSERT_EQ(3, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(3, packet_buffer_.GetUniqueFramesSeen()); } TEST_F(TestPacketBuffer, HasHistoryOfUniqueFrames) { @@ -250,18 +249,18 @@ TEST_F(TestPacketBuffer, HasHistoryOfUniqueFrames) { Insert(seq_num + i, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp + 10 * i); } - ASSERT_EQ(kNumFrames, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(kNumFrames, packet_buffer_.GetUniqueFramesSeen()); // Old packets within history should not affect number of seen unique frames. for (int i = kNumFrames - kRequiredHistoryLength; i < kNumFrames; ++i) { Insert(seq_num + i, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp + 10 * i); } - ASSERT_EQ(kNumFrames, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(kNumFrames, packet_buffer_.GetUniqueFramesSeen()); // Very old packets should be treated as unique. Insert(seq_num, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp); - ASSERT_EQ(kNumFrames + 1, packet_buffer_->GetUniqueFramesSeen()); + ASSERT_EQ(kNumFrames + 1, packet_buffer_.GetUniqueFramesSeen()); } TEST_F(TestPacketBuffer, ExpandBuffer) { @@ -360,7 +359,7 @@ TEST_F(TestPacketBuffer, ClearSinglePacket) { for (int i = 0; i < kMaxSize; ++i) EXPECT_TRUE(Insert(seq_num + i, kDeltaFrame, kFirst, kLast)); - packet_buffer_->ClearTo(seq_num); + packet_buffer_.ClearTo(seq_num); EXPECT_TRUE(Insert(seq_num + kMaxSize, kDeltaFrame, kFirst, kLast)); } @@ -368,7 +367,7 @@ TEST_F(TestPacketBuffer, ClearFullBuffer) { for (int i = 0; i < kMaxSize; ++i) EXPECT_TRUE(Insert(i, kDeltaFrame, kFirst, kLast)); - packet_buffer_->ClearTo(kMaxSize - 1); + packet_buffer_.ClearTo(kMaxSize - 1); for (int i = kMaxSize; i < 2 * kMaxSize; ++i) EXPECT_TRUE(Insert(i, kDeltaFrame, kFirst, kLast)); @@ -376,10 +375,10 @@ TEST_F(TestPacketBuffer, ClearFullBuffer) { TEST_F(TestPacketBuffer, DontClearNewerPacket) { EXPECT_TRUE(Insert(0, kKeyFrame, kFirst, kLast)); - packet_buffer_->ClearTo(0); + packet_buffer_.ClearTo(0); EXPECT_TRUE(Insert(2 * kStartSize, kKeyFrame, kFirst, kLast)); EXPECT_TRUE(Insert(3 * kStartSize + 1, kKeyFrame, kFirst, kNotLast)); - packet_buffer_->ClearTo(2 * kStartSize); + packet_buffer_.ClearTo(2 * kStartSize); EXPECT_TRUE(Insert(3 * kStartSize + 2, kKeyFrame, kNotFirst, kLast)); ASSERT_EQ(3UL, frames_from_callback_.size()); @@ -577,7 +576,7 @@ class TestPacketBufferH264 : public TestPacketBuffer { packet.sizeBytes = data_size; packet.dataPtr = data; - return packet_buffer_->InsertPacket(&packet); + return packet_buffer_.InsertPacket(&packet); } const bool sps_pps_idr_is_keyframe_; @@ -599,7 +598,7 @@ INSTANTIATE_TEST_SUITE_P(SpsPpsIdrIsKeyframe, TEST_P(TestPacketBufferH264Parameterized, DontRemoveMissingPacketOnClearTo) { EXPECT_TRUE(InsertH264(0, kKeyFrame, kFirst, kLast, 0)); EXPECT_TRUE(InsertH264(2, kDeltaFrame, kFirst, kNotLast, 2)); - packet_buffer_->ClearTo(0); + packet_buffer_.ClearTo(0); EXPECT_TRUE(InsertH264(3, kDeltaFrame, kNotFirst, kLast, 2)); ASSERT_EQ(1UL, frames_from_callback_.size()); @@ -649,7 +648,7 @@ TEST_P(TestPacketBufferH264Parameterized, GetBitstreamBufferPadding) { packet.sizeBytes = sizeof(data_data); packet.video_header.is_first_packet_in_frame = true; packet.video_header.is_last_packet_in_frame = true; - packet_buffer_->InsertPacket(&packet); + packet_buffer_.InsertPacket(&packet); ASSERT_EQ(1UL, frames_from_callback_.size()); EXPECT_EQ(frames_from_callback_[seq_num]->EncodedImage().size(), @@ -690,7 +689,7 @@ TEST_F(TestPacketBuffer, Clear) { EXPECT_EQ(1UL, frames_from_callback_.size()); CheckFrame(seq_num); - packet_buffer_->Clear(); + packet_buffer_.Clear(); EXPECT_TRUE(Insert(seq_num + kStartSize, kKeyFrame, kFirst, kNotLast)); EXPECT_TRUE( @@ -703,7 +702,7 @@ TEST_F(TestPacketBuffer, Clear) { TEST_F(TestPacketBuffer, FramesAfterClear) { Insert(9025, kDeltaFrame, kFirst, kLast); Insert(9024, kKeyFrame, kFirst, kLast); - packet_buffer_->ClearTo(9025); + packet_buffer_.ClearTo(9025); Insert(9057, kDeltaFrame, kFirst, kLast); Insert(9026, kDeltaFrame, kFirst, kLast); @@ -735,7 +734,7 @@ TEST_F(TestPacketBuffer, DontLeakPayloadData) { EXPECT_TRUE(Insert(2, kKeyFrame, kFirst, kNotLast, 5, data2)); // Expect to free data3 upon insertion (old packet). - packet_buffer_->ClearTo(1); + packet_buffer_.ClearTo(1); EXPECT_TRUE(Insert(1, kKeyFrame, kFirst, kNotLast, 5, data3)); // Expect to free data4 upon insertion (packet buffer is full). @@ -755,15 +754,15 @@ TEST_F(TestPacketBuffer, PacketTimestamps) { absl::optional packet_ms; absl::optional packet_keyframe_ms; - packet_ms = packet_buffer_->LastReceivedPacketMs(); - packet_keyframe_ms = packet_buffer_->LastReceivedKeyframePacketMs(); + packet_ms = packet_buffer_.LastReceivedPacketMs(); + packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs(); EXPECT_FALSE(packet_ms); EXPECT_FALSE(packet_keyframe_ms); int64_t keyframe_ms = clock_->TimeInMilliseconds(); EXPECT_TRUE(Insert(100, kKeyFrame, kFirst, kLast)); - packet_ms = packet_buffer_->LastReceivedPacketMs(); - packet_keyframe_ms = packet_buffer_->LastReceivedKeyframePacketMs(); + packet_ms = packet_buffer_.LastReceivedPacketMs(); + packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs(); EXPECT_TRUE(packet_ms); EXPECT_TRUE(packet_keyframe_ms); EXPECT_EQ(keyframe_ms, *packet_ms); @@ -772,16 +771,16 @@ TEST_F(TestPacketBuffer, PacketTimestamps) { clock_->AdvanceTimeMilliseconds(100); int64_t delta_ms = clock_->TimeInMilliseconds(); EXPECT_TRUE(Insert(101, kDeltaFrame, kFirst, kLast)); - packet_ms = packet_buffer_->LastReceivedPacketMs(); - packet_keyframe_ms = packet_buffer_->LastReceivedKeyframePacketMs(); + packet_ms = packet_buffer_.LastReceivedPacketMs(); + packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs(); EXPECT_TRUE(packet_ms); EXPECT_TRUE(packet_keyframe_ms); EXPECT_EQ(delta_ms, *packet_ms); EXPECT_EQ(keyframe_ms, *packet_keyframe_ms); - packet_buffer_->Clear(); - packet_ms = packet_buffer_->LastReceivedPacketMs(); - packet_keyframe_ms = packet_buffer_->LastReceivedKeyframePacketMs(); + packet_buffer_.Clear(); + packet_ms = packet_buffer_.LastReceivedPacketMs(); + packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs(); EXPECT_FALSE(packet_ms); EXPECT_FALSE(packet_keyframe_ms); } @@ -798,7 +797,7 @@ TEST_F(TestPacketBuffer, IncomingCodecChange) { packet.timestamp = 1; packet.seqNum = 1; packet.video_header.frame_type = VideoFrameType::kVideoFrameKey; - EXPECT_TRUE(packet_buffer_->InsertPacket(&packet)); + EXPECT_TRUE(packet_buffer_.InsertPacket(&packet)); packet.video_header.codec = kVideoCodecH264; auto& h264_header = @@ -806,7 +805,7 @@ TEST_F(TestPacketBuffer, IncomingCodecChange) { h264_header.nalus_length = 1; packet.timestamp = 3; packet.seqNum = 3; - EXPECT_TRUE(packet_buffer_->InsertPacket(&packet)); + EXPECT_TRUE(packet_buffer_.InsertPacket(&packet)); packet.video_header.codec = kVideoCodecVP8; packet.video_header.video_type_header.emplace(); @@ -814,7 +813,7 @@ TEST_F(TestPacketBuffer, IncomingCodecChange) { packet.seqNum = 2; packet.video_header.frame_type = VideoFrameType::kVideoFrameDelta; - EXPECT_TRUE(packet_buffer_->InsertPacket(&packet)); + EXPECT_TRUE(packet_buffer_.InsertPacket(&packet)); EXPECT_EQ(3UL, frames_from_callback_.size()); } @@ -832,7 +831,7 @@ TEST_F(TestPacketBuffer, TooManyNalusInPacket) { h264_header.nalus_length = kMaxNalusPerPacket; packet.sizeBytes = 0; packet.dataPtr = nullptr; - EXPECT_TRUE(packet_buffer_->InsertPacket(&packet)); + EXPECT_TRUE(packet_buffer_.InsertPacket(&packet)); EXPECT_EQ(0UL, frames_from_callback_.size()); } @@ -894,7 +893,7 @@ TEST_P(TestPacketBufferH264Parameterized, FindFramesOnPadding) { InsertH264(2, kDeltaFrame, kFirst, kLast, 1000); ASSERT_EQ(1UL, frames_from_callback_.size()); - packet_buffer_->PaddingReceived(1); + packet_buffer_.PaddingReceived(1); ASSERT_EQ(2UL, frames_from_callback_.size()); CheckFrame(0); CheckFrame(2); @@ -928,7 +927,7 @@ TEST_F(TestPacketBufferH264IdrIsKeyframe, IdrIsKeyframe) { packet_.video_header.video_type_header.emplace(); h264_header.nalus[0].type = H264::NaluType::kIdr; h264_header.nalus_length = 1; - packet_buffer_->InsertPacket(&packet_); + packet_buffer_.InsertPacket(&packet_); ASSERT_EQ(1u, frames_from_callback_.size()); EXPECT_EQ(VideoFrameType::kVideoFrameKey, @@ -943,7 +942,7 @@ TEST_F(TestPacketBufferH264IdrIsKeyframe, SpsPpsIdrIsKeyframe) { h264_header.nalus[2].type = H264::NaluType::kIdr; h264_header.nalus_length = 3; - packet_buffer_->InsertPacket(&packet_); + packet_buffer_.InsertPacket(&packet_); ASSERT_EQ(1u, frames_from_callback_.size()); EXPECT_EQ(VideoFrameType::kVideoFrameKey, @@ -963,7 +962,7 @@ TEST_F(TestPacketBufferH264SpsPpsIdrIsKeyframe, IdrIsNotKeyframe) { h264_header.nalus[0].type = H264::NaluType::kIdr; h264_header.nalus_length = 1; - packet_buffer_->InsertPacket(&packet_); + packet_buffer_.InsertPacket(&packet_); ASSERT_EQ(1u, frames_from_callback_.size()); EXPECT_EQ(VideoFrameType::kVideoFrameDelta, @@ -977,7 +976,7 @@ TEST_F(TestPacketBufferH264SpsPpsIdrIsKeyframe, SpsPpsIsNotKeyframe) { h264_header.nalus[1].type = H264::NaluType::kPps; h264_header.nalus_length = 2; - packet_buffer_->InsertPacket(&packet_); + packet_buffer_.InsertPacket(&packet_); ASSERT_EQ(1u, frames_from_callback_.size()); EXPECT_EQ(VideoFrameType::kVideoFrameDelta, @@ -992,7 +991,7 @@ TEST_F(TestPacketBufferH264SpsPpsIdrIsKeyframe, SpsPpsIdrIsKeyframe) { h264_header.nalus[2].type = H264::NaluType::kIdr; h264_header.nalus_length = 3; - packet_buffer_->InsertPacket(&packet_); + packet_buffer_.InsertPacket(&packet_); ASSERT_EQ(1u, frames_from_callback_.size()); EXPECT_EQ(VideoFrameType::kVideoFrameKey, diff --git a/test/fuzzers/packet_buffer_fuzzer.cc b/test/fuzzers/packet_buffer_fuzzer.cc index f8067b78ea..9f0a6366d1 100644 --- a/test/fuzzers/packet_buffer_fuzzer.cc +++ b/test/fuzzers/packet_buffer_fuzzer.cc @@ -27,8 +27,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { VCMPacket packet; NullCallback callback; SimulatedClock clock(0); - rtc::scoped_refptr packet_buffer( - video_coding::PacketBuffer::Create(&clock, 8, 1024, &callback)); + video_coding::PacketBuffer packet_buffer(&clock, 8, 1024, &callback); test::FuzzDataHelper helper(rtc::ArrayView(data, size)); while (helper.BytesLeft()) { @@ -60,7 +59,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { packet.sizeBytes = payload_size; packet.dataPtr = new uint8_t[payload_size]; - packet_buffer->InsertPacket(&packet); + packet_buffer.InsertPacket(&packet); } } diff --git a/test/fuzzers/rtp_frame_reference_finder_fuzzer.cc b/test/fuzzers/rtp_frame_reference_finder_fuzzer.cc index 7083295495..f238138d76 100644 --- a/test/fuzzers/rtp_frame_reference_finder_fuzzer.cc +++ b/test/fuzzers/rtp_frame_reference_finder_fuzzer.cc @@ -112,7 +112,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { return; } DataReader reader(data, size); - rtc::scoped_refptr pb(new FuzzyPacketBuffer(&reader)); + FuzzyPacketBuffer packet_buffer(&reader); NullCallback cb; video_coding::RtpFrameReferenceFinder reference_finder(&cb); @@ -120,13 +120,13 @@ void FuzzOneInput(const uint8_t* data, size_t size) { // Make sure that these packets fulfill the contract of RtpFrameObject. uint16_t first_seq_num = reader.GetNum(); uint16_t last_seq_num = reader.GetNum(); - VCMPacket* first_packet = pb->GetPacket(first_seq_num); - VCMPacket* last_packet = pb->GetPacket(last_seq_num); + VCMPacket* first_packet = packet_buffer.GetPacket(first_seq_num); + VCMPacket* last_packet = packet_buffer.GetPacket(last_seq_num); first_packet->video_header.is_first_packet_in_frame = true; last_packet->video_header.is_last_packet_in_frame = true; auto frame = std::make_unique( - pb, first_seq_num, last_seq_num, 0, 0, 0, RtpPacketInfos(), + &packet_buffer, first_seq_num, last_seq_num, 0, 0, 0, RtpPacketInfos(), EncodedImageBuffer::Create(/*size=*/0)); reference_finder.ManageFrame(std::move(frame)); } diff --git a/video/buffered_frame_decryptor_unittest.cc b/video/buffered_frame_decryptor_unittest.cc index ad0c723c2d..7dcf712c9e 100644 --- a/video/buffered_frame_decryptor_unittest.cc +++ b/video/buffered_frame_decryptor_unittest.cc @@ -90,20 +90,18 @@ class BufferedFrameDecryptorTest ? VideoFrameType::kVideoFrameKey : VideoFrameType::kVideoFrameDelta; packet.generic_descriptor = RtpGenericFrameDescriptor(); - fake_packet_buffer_->InsertPacket(&packet); + fake_packet_buffer_.InsertPacket(&packet); packet.seqNum = seq_num_; packet.video_header.is_last_packet_in_frame = true; - fake_packet_buffer_->InsertPacket(&packet); + fake_packet_buffer_.InsertPacket(&packet); - return std::unique_ptr( - new video_coding::RtpFrameObject( - fake_packet_buffer_.get(), seq_num_, seq_num_, 0, 0, 0, {}, - EncodedImageBuffer::Create(/*size=*/0))); + return std::make_unique( + &fake_packet_buffer_, seq_num_, seq_num_, 0, 0, 0, RtpPacketInfos(), + EncodedImageBuffer::Create(/*size=*/0)); } protected: - BufferedFrameDecryptorTest() : fake_packet_buffer_(new FakePacketBuffer()) {} - void SetUp() override { + BufferedFrameDecryptorTest() { fake_packet_data_ = std::vector(100); decrypted_frame_call_count_ = 0; decryption_status_change_count_ = 0; @@ -117,7 +115,7 @@ class BufferedFrameDecryptorTest static const size_t kMaxStashedFrames; std::vector fake_packet_data_; - rtc::scoped_refptr fake_packet_buffer_; + FakePacketBuffer fake_packet_buffer_; rtc::scoped_refptr mock_frame_decryptor_; std::unique_ptr buffered_frame_decryptor_; size_t decrypted_frame_call_count_; diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 007a932673..ae9a3cad43 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -49,6 +49,24 @@ namespace { // crbug.com/752886 constexpr int kPacketBufferStartSize = 512; constexpr int kPacketBufferMaxSize = 2048; + +int PacketBufferMaxSize() { + // The group here must be a positive power of 2, in which case that is used as + // size. All other values shall result in the default value being used. + const std::string group_name = + webrtc::field_trial::FindFullName("WebRTC-PacketBufferMaxSize"); + int packet_buffer_max_size = kPacketBufferMaxSize; + if (!group_name.empty() && + (sscanf(group_name.c_str(), "%d", &packet_buffer_max_size) != 1 || + packet_buffer_max_size <= 0 || + // Verify that the number is a positive power of 2. + (packet_buffer_max_size & (packet_buffer_max_size - 1)) != 0)) { + RTC_LOG(LS_WARNING) << "Invalid packet buffer max size: " << group_name; + packet_buffer_max_size = kPacketBufferMaxSize; + } + return packet_buffer_max_size; +} + } // namespace std::unique_ptr CreateRtpRtcpModule( @@ -192,6 +210,10 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( // TODO(bugs.webrtc.org/10336): Let |rtcp_feedback_buffer_| communicate // directly with |rtp_rtcp_|. rtcp_feedback_buffer_(this, nack_sender, this), + packet_buffer_(clock_, + kPacketBufferStartSize, + PacketBufferMaxSize(), + this), has_received_frame_(false), frames_decryptable_(false) { constexpr bool remb_candidate = true; @@ -242,22 +264,6 @@ RtpVideoStreamReceiver::RtpVideoStreamReceiver( process_thread_->RegisterModule(nack_module_.get(), RTC_FROM_HERE); } - // The group here must be a positive power of 2, in which case that is used as - // size. All other values shall result in the default value being used. - const std::string group_name = - webrtc::field_trial::FindFullName("WebRTC-PacketBufferMaxSize"); - int packet_buffer_max_size = kPacketBufferMaxSize; - if (!group_name.empty() && - (sscanf(group_name.c_str(), "%d", &packet_buffer_max_size) != 1 || - packet_buffer_max_size <= 0 || - // Verify that the number is a positive power of 2. - (packet_buffer_max_size & (packet_buffer_max_size - 1)) != 0)) { - RTC_LOG(LS_WARNING) << "Invalid packet buffer max size: " << group_name; - packet_buffer_max_size = kPacketBufferMaxSize; - } - - packet_buffer_ = video_coding::PacketBuffer::Create( - clock_, kPacketBufferStartSize, packet_buffer_max_size, this); reference_finder_ = std::make_unique(this); @@ -387,7 +393,7 @@ int32_t RtpVideoStreamReceiver::OnReceivedPayloadData( } rtcp_feedback_buffer_.SendBufferedRtcpFeedback(); - if (!packet_buffer_->InsertPacket(&packet)) { + if (!packet_buffer_.InsertPacket(&packet)) { RequestKeyFrame(); } return 0; @@ -580,12 +586,12 @@ void RtpVideoStreamReceiver::UpdateRtt(int64_t max_rtt_ms) { } absl::optional RtpVideoStreamReceiver::LastReceivedPacketMs() const { - return packet_buffer_->LastReceivedPacketMs(); + return packet_buffer_.LastReceivedPacketMs(); } absl::optional RtpVideoStreamReceiver::LastReceivedKeyframePacketMs() const { - return packet_buffer_->LastReceivedKeyframePacketMs(); + return packet_buffer_.LastReceivedKeyframePacketMs(); } void RtpVideoStreamReceiver::AddSecondarySink(RtpPacketSinkInterface* sink) { @@ -745,7 +751,7 @@ void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader( // correctly calculate frame references. void RtpVideoStreamReceiver::NotifyReceiverOfEmptyPacket(uint16_t seq_num) { reference_finder_->PaddingReceived(seq_num); - packet_buffer_->PaddingReceived(seq_num); + packet_buffer_.PaddingReceived(seq_num); if (nack_module_) { nack_module_->OnReceivedPacket(seq_num, /* is_keyframe = */ false, /* is _recovered = */ false); @@ -821,7 +827,7 @@ void RtpVideoStreamReceiver::FrameDecoded(int64_t picture_id) { } } if (seq_num != -1) { - packet_buffer_->ClearTo(seq_num); + packet_buffer_.ClearTo(seq_num); reference_finder_->ClearTo(seq_num); } } @@ -832,7 +838,7 @@ void RtpVideoStreamReceiver::SignalNetworkState(NetworkState state) { } int RtpVideoStreamReceiver::GetUniqueFramesSeen() const { - return packet_buffer_->GetUniqueFramesSeen(); + return packet_buffer_.GetUniqueFramesSeen(); } void RtpVideoStreamReceiver::StartReceive() { diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 365be4aaff..392bf552a1 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -273,7 +273,7 @@ class RtpVideoStreamReceiver : public LossNotificationSender, std::unique_ptr nack_module_; std::unique_ptr loss_notification_controller_; - rtc::scoped_refptr packet_buffer_; + video_coding::PacketBuffer packet_buffer_; std::unique_ptr reference_finder_; rtc::CriticalSection last_seq_num_cs_; std::map last_seq_num_for_pic_id_