Reland of Fix off-by-one bugs in video_coding::PacketBuffer when the buffer is filled with a single frame. (patchset #1 id:1 of https://codereview.chromium.org/2990183002/ )
Reason for revert:
Revert to create fix CL.
Original issue's description:
> Revert of Fix off-by-one bugs in video_coding::PacketBuffer when the buffer is filled with a single frame. (patchset #5 id:80001 of https://codereview.chromium.org/2993513002/ )
>
> Reason for revert:
> Break performance bots.
>
> Original issue's description:
> > 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}
> > Committed: https://chromium.googlesource.com/external/webrtc/+/ee13e8919c20de5860a510e91fac71fd5a7e9b8d
>
> TBR=stefan@webrtc.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=webrtc:8028
>
> Review-Url: https://codereview.webrtc.org/2990183002
> Cr-Commit-Position: refs/heads/master@{#19211}
> Committed: https://chromium.googlesource.com/external/webrtc/+/c18f1d7c9432a2bf7d112820eb2b1dd2dbe2ba4b
TBR=stefan@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:8028
TBR=stefan@webrtc.org
Review-Url: https://codereview.webrtc.org/2989313003
Cr-Commit-Position: refs/heads/master@{#19249}
diff --git a/webrtc/modules/video_coding/packet_buffer.cc b/webrtc/modules/video_coding/packet_buffer.cc
index 51a3e0a..4859a85 100644
--- a/webrtc/modules/video_coding/packet_buffer.cc
+++ b/webrtc/modules/video_coding/packet_buffer.cc
@@ -268,14 +268,14 @@
// 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);
@@ -295,6 +295,9 @@
}
}
+ 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,
@@ -362,19 +365,30 @@
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;
}