Use timestamp instead of seq_num to distinguish between packets.
In the case a frame_object is kept for some time before it is deleted,
it may happend that a new frame is received with overlapping sequence
numbers. If the old frame_object is removed while receiving the new
frame there used to be a crash.
Bug: webrtc:9629
Change-Id: I270a8caa2b58b73c000542aa504c0ebe277d49c4
Reviewed-on: https://webrtc-review.googlesource.com/102683
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24914}diff --git a/modules/video_coding/video_packet_buffer_unittest.cc b/modules/video_coding/video_packet_buffer_unittest.cc
index 8866368..425542c 100644
--- a/modules/video_coding/video_packet_buffer_unittest.cc
+++ b/modules/video_coding/video_packet_buffer_unittest.cc
@@ -82,6 +82,14 @@
<< ".";
}
+ 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 @@
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<uint16_t> unwrapper(0);
+ while (unwrapper.Unwrap(seq_num) < std::numeric_limits<uint16_t>::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