diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc index fe93fb312d..552de4da89 100644 --- a/modules/video_coding/packet_buffer.cc +++ b/modules/video_coding/packet_buffer.cc @@ -398,8 +398,12 @@ void PacketBuffer::ReturnFrame(RtpFrameObject* frame) { size_t index = frame->first_seq_num() % size_; size_t end = (frame->last_seq_num() + 1) % size_; uint16_t seq_num = frame->first_seq_num(); + uint32_t timestamp = frame->Timestamp(); while (index != end) { - if (sequence_buffer_[index].seq_num == seq_num) { + // Check both seq_num and timestamp to handle the case when seq_num wraps + // around too quickly for high packet rates. + if (sequence_buffer_[index].seq_num == seq_num && + data_buffer_[index].timestamp == timestamp) { delete[] data_buffer_[index].dataPtr; data_buffer_[index].dataPtr = nullptr; sequence_buffer_[index].used = false; @@ -417,11 +421,15 @@ bool PacketBuffer::GetBitstream(const RtpFrameObject& frame, size_t index = frame.first_seq_num() % size_; size_t end = (frame.last_seq_num() + 1) % size_; uint16_t seq_num = frame.first_seq_num(); + uint32_t timestamp = frame.Timestamp(); uint8_t* destination_end = destination + frame.size(); do { + // Check both seq_num and timestamp to handle the case when seq_num wraps + // around too quickly for high packet rates. if (!sequence_buffer_[index].used || - sequence_buffer_[index].seq_num != seq_num) { + sequence_buffer_[index].seq_num != seq_num || + data_buffer_[index].timestamp != timestamp) { return false; } diff --git a/modules/video_coding/video_packet_buffer_unittest.cc b/modules/video_coding/video_packet_buffer_unittest.cc index 8866368c3b..425542c949 100644 --- a/modules/video_coding/video_packet_buffer_unittest.cc +++ b/modules/video_coding/video_packet_buffer_unittest.cc @@ -82,6 +82,14 @@ class TestPacketBuffer : public ::testing::Test, << "."; } + void DeleteFrame(uint16_t first_seq_num) { + auto frame_it = frames_from_callback_.find(first_seq_num); + ASSERT_FALSE(frame_it == frames_from_callback_.end()) + << "Could not find frame with first sequence number " << first_seq_num + << "."; + frames_from_callback_.erase(frame_it); + } + static constexpr int kStartSize = 16; static constexpr int kMaxSize = 64; @@ -494,6 +502,39 @@ TEST_F(TestPacketBuffer, GetBitstreamOneFrameFullBuffer) { EXPECT_EQ(memcmp(result, expected, kStartSize), 0); } +TEST_F(TestPacketBuffer, InsertPacketAfterOldFrameObjectIsRemoved) { + uint16_t kFirstSeqNum = 0; + uint32_t kTimestampDelta = 100; + uint32_t timestamp = 10000; + uint16_t seq_num = kFirstSeqNum; + + // Loop until seq_num wraps around. + SeqNumUnwrapper unwrapper(0); + while (unwrapper.Unwrap(seq_num) < std::numeric_limits::max()) { + Insert(seq_num++, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp); + for (int i = 0; i < 5; ++i) { + Insert(seq_num++, kKeyFrame, kNotFirst, kNotLast, 0, nullptr, timestamp); + } + Insert(seq_num++, kKeyFrame, kNotFirst, kLast, 0, nullptr, timestamp); + timestamp += kTimestampDelta; + } + + size_t number_of_frames = frames_from_callback_.size(); + // Delete old frame object while receiving frame with overlapping sequence + // numbers. + Insert(seq_num++, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp); + for (int i = 0; i < 5; ++i) { + Insert(seq_num++, kKeyFrame, kNotFirst, kNotLast, 0, nullptr, timestamp); + } + // Delete FrameObject connected to packets that have already been cleared. + DeleteFrame(kFirstSeqNum); + Insert(seq_num++, kKeyFrame, kNotFirst, kLast, 0, nullptr, timestamp); + + // Regardless of the initial size, the number of frames should be constant + // after removing and then adding a new frame object. + EXPECT_EQ(number_of_frames, frames_from_callback_.size()); +} + // If |sps_pps_idr_is_keyframe| is true, we require keyframes to contain // SPS/PPS/IDR and the keyframes we create as part of the test do contain // SPS/PPS/IDR. If |sps_pps_idr_is_keyframe| is false, we only require and