From bc5a40870c6d83642523d27c660f75896ac5d6c3 Mon Sep 17 00:00:00 2001 From: philipel Date: Wed, 6 Dec 2017 10:41:08 +0100 Subject: [PATCH] Fix off-by-one error when removing information about missing packet in PacketBuffer. In the ClearTo function we risked removing one packet too many from the set of |missing_packets_|, and in the case of H264 this would cause us to create incomplete delta frames. This is turn disrupt the stream and a keyframe request has to be made. Bug: webrtc:8536 Change-Id: Ie7ccd5f1631a4cf3bd463878d5b0fe746744ec23 Reviewed-on: https://webrtc-review.googlesource.com/30141 Commit-Queue: Philip Eliasson Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#21119} --- modules/video_coding/packet_buffer.cc | 7 +++++-- modules/video_coding/video_packet_buffer_unittest.cc | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc index 9b3d93e658..741c11e2aa 100644 --- a/modules/video_coding/packet_buffer.cc +++ b/modules/video_coding/packet_buffer.cc @@ -162,8 +162,11 @@ void PacketBuffer::ClearTo(uint16_t seq_num) { first_seq_num_ = seq_num; is_cleared_to_first_seq_num_ = true; - missing_packets_.erase(missing_packets_.begin(), - missing_packets_.upper_bound(seq_num)); + auto clear_to_it = missing_packets_.upper_bound(seq_num); + if (clear_to_it != missing_packets_.begin()) { + --clear_to_it; + missing_packets_.erase(missing_packets_.begin(), clear_to_it); + } } void PacketBuffer::Clear() { diff --git a/modules/video_coding/video_packet_buffer_unittest.cc b/modules/video_coding/video_packet_buffer_unittest.cc index f8b3dc65dc..893649364a 100644 --- a/modules/video_coding/video_packet_buffer_unittest.cc +++ b/modules/video_coding/video_packet_buffer_unittest.cc @@ -489,6 +489,16 @@ INSTANTIATE_TEST_CASE_P(SpsPpsIdrIsKeyframe, TestPacketBufferH264Parameterized, ::testing::Values(false, true)); +TEST_P(TestPacketBufferH264Parameterized, DontRemoveMissingPacketOnClearTo) { + EXPECT_TRUE(InsertH264(0, kKeyFrame, kFirst, kLast, 0)); + EXPECT_TRUE(InsertH264(2, kDeltaFrame, kFirst, kNotLast, 2)); + packet_buffer_->ClearTo(0); + EXPECT_TRUE(InsertH264(3, kDeltaFrame, kNotFirst, kLast, 2)); + + ASSERT_EQ(1UL, frames_from_callback_.size()); + CheckFrame(0); +} + TEST_P(TestPacketBufferH264Parameterized, GetBitstreamOneFrameFullBuffer) { uint8_t* data_arr[kStartSize]; uint8_t expected[kStartSize];