Fix off-by-one bugs in video_coding::PacketBuffer when the buffer is filled with a single frame.
BUG=webrtc:8028 Review-Url: https://codereview.webrtc.org/2993513002 Cr-Commit-Position: refs/heads/master@{#19209}
This commit is contained in:
@ -93,7 +93,7 @@ RtpFrameObject::RtpFrameObject(PacketBuffer* packet_buffer,
|
||||
frame_type_ = first_packet->frameType;
|
||||
}
|
||||
|
||||
GetBitstream(_buffer);
|
||||
RTC_DCHECK(GetBitstream(_buffer));
|
||||
_encodedWidth = first_packet->width;
|
||||
_encodedHeight = first_packet->height;
|
||||
|
||||
|
@ -42,7 +42,7 @@ class FrameObject : public webrtc::VCMEncodedFrame {
|
||||
// been implemented.
|
||||
virtual bool delayed_by_retransmission() const { return 0; }
|
||||
|
||||
size_t size() { return _length; }
|
||||
size_t size() const { return _length; }
|
||||
|
||||
// The tuple (|picture_id|, |spatial_layer|) uniquely identifies a frame
|
||||
// object. For codec types that don't necessarily have picture ids they
|
||||
|
@ -251,14 +251,14 @@ std::vector<std::unique_ptr<RtpFrameObject>> PacketBuffer::FindFrames(
|
||||
// Find the start index by searching backward until the packet with
|
||||
// the |frame_begin| flag is set.
|
||||
int start_index = index;
|
||||
size_t tested_packets = 0;
|
||||
|
||||
bool is_h264 = data_buffer_[start_index].codec == kVideoCodecH264;
|
||||
bool is_h264_keyframe = false;
|
||||
int64_t frame_timestamp = data_buffer_[start_index].timestamp;
|
||||
|
||||
// Since packet at |data_buffer_[index]| is already part of the frame
|
||||
// we will have at most |size_ - 1| packets left to check.
|
||||
for (size_t j = 0; j < size_ - 1; ++j) {
|
||||
while (true) {
|
||||
++tested_packets;
|
||||
frame_size += data_buffer_[start_index].sizeBytes;
|
||||
max_nack_count =
|
||||
std::max(max_nack_count, data_buffer_[start_index].timesNacked);
|
||||
@ -278,6 +278,9 @@ std::vector<std::unique_ptr<RtpFrameObject>> PacketBuffer::FindFrames(
|
||||
}
|
||||
}
|
||||
|
||||
if (tested_packets == size_)
|
||||
break;
|
||||
|
||||
start_index = start_index > 0 ? start_index - 1 : size_ - 1;
|
||||
|
||||
// In the case of H264 we don't have a frame_begin bit (yes,
|
||||
@ -345,19 +348,30 @@ 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();
|
||||
while (index != end) {
|
||||
uint8_t* destination_end = destination + frame.size();
|
||||
|
||||
do {
|
||||
if (!sequence_buffer_[index].used ||
|
||||
sequence_buffer_[index].seq_num != seq_num) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const uint8_t* source = data_buffer_[index].dataPtr;
|
||||
RTC_DCHECK_EQ(data_buffer_[index].seqNum, sequence_buffer_[index].seq_num);
|
||||
size_t length = data_buffer_[index].sizeBytes;
|
||||
if (destination + length > destination_end) {
|
||||
LOG(LS_WARNING) << "Frame (" << frame.picture_id << ":"
|
||||
<< static_cast<int>(frame.spatial_layer) << ")"
|
||||
<< " bitstream buffer is not large enough.";
|
||||
return false;
|
||||
}
|
||||
|
||||
const uint8_t* source = data_buffer_[index].dataPtr;
|
||||
memcpy(destination, source, length);
|
||||
destination += length;
|
||||
index = (index + 1) % size_;
|
||||
++seq_num;
|
||||
}
|
||||
} while (index != end);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@ -98,8 +98,8 @@ class TestPacketBuffer : public ::testing::Test,
|
||||
<< ".";
|
||||
}
|
||||
|
||||
const int kStartSize = 16;
|
||||
const int kMaxSize = 64;
|
||||
static constexpr int kStartSize = 16;
|
||||
static constexpr int kMaxSize = 64;
|
||||
|
||||
Random rand_;
|
||||
std::unique_ptr<SimulatedClock> clock_;
|
||||
@ -374,10 +374,61 @@ TEST_F(TestPacketBuffer, GetBitstream) {
|
||||
|
||||
ASSERT_EQ(1UL, frames_from_callback_.size());
|
||||
CheckFrame(seq_num);
|
||||
EXPECT_EQ(frames_from_callback_[seq_num]->size(), sizeof(result));
|
||||
EXPECT_TRUE(frames_from_callback_[seq_num]->GetBitstream(result));
|
||||
EXPECT_EQ(memcmp(result, "many bitstream, such data", sizeof(result)), 0);
|
||||
}
|
||||
|
||||
TEST_F(TestPacketBuffer, GetBitstreamOneFrameFullBuffer) {
|
||||
uint8_t* data_arr[kStartSize];
|
||||
uint8_t expected[kStartSize];
|
||||
uint8_t result[kStartSize];
|
||||
|
||||
for (uint8_t i = 0; i < kStartSize; ++i) {
|
||||
data_arr[i] = new uint8_t[1];
|
||||
data_arr[i][0] = i;
|
||||
expected[i] = i;
|
||||
}
|
||||
|
||||
EXPECT_TRUE(Insert(0, kKeyFrame, kFirst, kNotLast, 1, data_arr[0]));
|
||||
for (uint8_t i = 1; i < kStartSize - 1; ++i)
|
||||
EXPECT_TRUE(Insert(i, kKeyFrame, kNotFirst, kNotLast, 1, data_arr[i]));
|
||||
EXPECT_TRUE(Insert(kStartSize - 1, kKeyFrame, kNotFirst, kLast, 1,
|
||||
data_arr[kStartSize - 1]));
|
||||
|
||||
ASSERT_EQ(1UL, frames_from_callback_.size());
|
||||
CheckFrame(0);
|
||||
EXPECT_EQ(frames_from_callback_[0]->size(), static_cast<size_t>(kStartSize));
|
||||
EXPECT_TRUE(frames_from_callback_[0]->GetBitstream(result));
|
||||
EXPECT_EQ(memcmp(result, expected, kStartSize), 0);
|
||||
}
|
||||
|
||||
TEST_F(TestPacketBuffer, GetBitstreamOneFrameFullBufferH264) {
|
||||
uint8_t* data_arr[kStartSize];
|
||||
uint8_t expected[kStartSize];
|
||||
uint8_t result[kStartSize];
|
||||
|
||||
for (uint8_t i = 0; i < kStartSize; ++i) {
|
||||
data_arr[i] = new uint8_t[1];
|
||||
data_arr[i][0] = i;
|
||||
expected[i] = i;
|
||||
}
|
||||
|
||||
EXPECT_TRUE(InsertH264(0, kKeyFrame, kFirst, kNotLast, 1, 1, data_arr[0]));
|
||||
for (uint8_t i = 1; i < kStartSize - 1; ++i) {
|
||||
EXPECT_TRUE(
|
||||
InsertH264(i, kKeyFrame, kNotFirst, kNotLast, 1, 1, data_arr[i]));
|
||||
}
|
||||
EXPECT_TRUE(InsertH264(kStartSize - 1, kKeyFrame, kNotFirst, kLast, 1, 1,
|
||||
data_arr[kStartSize - 1]));
|
||||
|
||||
ASSERT_EQ(1UL, frames_from_callback_.size());
|
||||
CheckFrame(0);
|
||||
EXPECT_EQ(frames_from_callback_[0]->size(), static_cast<size_t>(kStartSize));
|
||||
EXPECT_TRUE(frames_from_callback_[0]->GetBitstream(result));
|
||||
EXPECT_EQ(memcmp(result, expected, kStartSize), 0);
|
||||
}
|
||||
|
||||
TEST_F(TestPacketBuffer, GetBitstreamH264BufferPadding) {
|
||||
uint16_t seq_num = Rand();
|
||||
uint8_t data_data[] = "some plain old data";
|
||||
@ -548,6 +599,23 @@ TEST_F(TestPacketBuffer, OneFrameFillBufferH264) {
|
||||
CheckFrame(0);
|
||||
}
|
||||
|
||||
TEST_F(TestPacketBuffer, CreateFramesAfterFilledBufferH264) {
|
||||
InsertH264(kStartSize - 2, kKeyFrame, kFirst, kLast, 0);
|
||||
ASSERT_EQ(1UL, frames_from_callback_.size());
|
||||
frames_from_callback_.clear();
|
||||
|
||||
InsertH264(kStartSize, kDeltaFrame, kFirst, kNotLast, 2000);
|
||||
for (int i = 1; i < kStartSize; ++i)
|
||||
InsertH264(kStartSize + i, kDeltaFrame, kNotFirst, kNotLast, 2000);
|
||||
InsertH264(kStartSize + kStartSize, kDeltaFrame, kNotFirst, kLast, 2000);
|
||||
ASSERT_EQ(0UL, frames_from_callback_.size());
|
||||
|
||||
InsertH264(kStartSize - 1, kKeyFrame, kFirst, kLast, 1000);
|
||||
ASSERT_EQ(2UL, frames_from_callback_.size());
|
||||
CheckFrame(kStartSize - 1);
|
||||
CheckFrame(kStartSize);
|
||||
}
|
||||
|
||||
TEST_F(TestPacketBuffer, OneFrameMaxSeqNumH264) {
|
||||
InsertH264(65534, kKeyFrame, kFirst, kNotLast, 1000);
|
||||
InsertH264(65535, kKeyFrame, kNotFirst, kLast, 1000);
|
||||
|
Reference in New Issue
Block a user