Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(433)

Side by Side Diff: webrtc/modules/video_coding/packet_buffer.cc

Issue 2993513002: Fix off-by-one bugs in video_coding::PacketBuffer when the buffer is filled with a single frame. (Closed)
Patch Set: new --> new[] Created 3 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 /* 1 /*
2 * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. 2 * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
3 * 3 *
4 * Use of this source code is governed by a BSD-style license 4 * Use of this source code is governed by a BSD-style license
5 * that can be found in the LICENSE file in the root of the source 5 * that can be found in the LICENSE file in the root of the source
6 * tree. An additional intellectual property rights grant can be found 6 * tree. An additional intellectual property rights grant can be found
7 * in the file PATENTS. All contributing project authors may 7 * in the file PATENTS. All contributing project authors may
8 * be found in the AUTHORS file in the root of the source tree. 8 * be found in the AUTHORS file in the root of the source tree.
9 */ 9 */
10 10
(...skipping 233 matching lines...) Expand 10 before | Expand all | Expand 10 after
244 // If all packets of the frame is continuous, find the first packet of the 244 // If all packets of the frame is continuous, find the first packet of the
245 // frame and create an RtpFrameObject. 245 // frame and create an RtpFrameObject.
246 if (sequence_buffer_[index].frame_end) { 246 if (sequence_buffer_[index].frame_end) {
247 size_t frame_size = 0; 247 size_t frame_size = 0;
248 int max_nack_count = -1; 248 int max_nack_count = -1;
249 uint16_t start_seq_num = seq_num; 249 uint16_t start_seq_num = seq_num;
250 250
251 // Find the start index by searching backward until the packet with 251 // Find the start index by searching backward until the packet with
252 // the |frame_begin| flag is set. 252 // the |frame_begin| flag is set.
253 int start_index = index; 253 int start_index = index;
254 size_t tested_packets = 0;
254 255
255 bool is_h264 = data_buffer_[start_index].codec == kVideoCodecH264; 256 bool is_h264 = data_buffer_[start_index].codec == kVideoCodecH264;
256 bool is_h264_keyframe = false; 257 bool is_h264_keyframe = false;
257 int64_t frame_timestamp = data_buffer_[start_index].timestamp; 258 int64_t frame_timestamp = data_buffer_[start_index].timestamp;
258 259
259 // Since packet at |data_buffer_[index]| is already part of the frame 260 while (true) {
260 // we will have at most |size_ - 1| packets left to check. 261 ++tested_packets;
261 for (size_t j = 0; j < size_ - 1; ++j) {
262 frame_size += data_buffer_[start_index].sizeBytes; 262 frame_size += data_buffer_[start_index].sizeBytes;
263 max_nack_count = 263 max_nack_count =
264 std::max(max_nack_count, data_buffer_[start_index].timesNacked); 264 std::max(max_nack_count, data_buffer_[start_index].timesNacked);
265 sequence_buffer_[start_index].frame_created = true; 265 sequence_buffer_[start_index].frame_created = true;
266 266
267 if (!is_h264 && sequence_buffer_[start_index].frame_begin) 267 if (!is_h264 && sequence_buffer_[start_index].frame_begin)
268 break; 268 break;
269 269
270 if (is_h264 && !is_h264_keyframe) { 270 if (is_h264 && !is_h264_keyframe) {
271 const RTPVideoHeaderH264& header = 271 const RTPVideoHeaderH264& header =
272 data_buffer_[start_index].video_header.codecHeader.H264; 272 data_buffer_[start_index].video_header.codecHeader.H264;
273 for (size_t i = 0; i < header.nalus_length; ++i) { 273 for (size_t i = 0; i < header.nalus_length; ++i) {
274 if (header.nalus[i].type == H264::NaluType::kIdr) { 274 if (header.nalus[i].type == H264::NaluType::kIdr) {
275 is_h264_keyframe = true; 275 is_h264_keyframe = true;
276 break; 276 break;
277 } 277 }
278 } 278 }
279 } 279 }
280 280
281 if (tested_packets == size_)
282 break;
283
281 start_index = start_index > 0 ? start_index - 1 : size_ - 1; 284 start_index = start_index > 0 ? start_index - 1 : size_ - 1;
282 285
283 // In the case of H264 we don't have a frame_begin bit (yes, 286 // In the case of H264 we don't have a frame_begin bit (yes,
284 // |frame_begin| might be set to true but that is a lie). So instead 287 // |frame_begin| might be set to true but that is a lie). So instead
285 // we traverese backwards as long as we have a previous packet and 288 // we traverese backwards as long as we have a previous packet and
286 // the timestamp of that packet is the same as this one. This may cause 289 // the timestamp of that packet is the same as this one. This may cause
287 // the PacketBuffer to hand out incomplete frames. 290 // the PacketBuffer to hand out incomplete frames.
288 // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7106 291 // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7106
289 if (is_h264 && 292 if (is_h264 &&
290 (!sequence_buffer_[start_index].used || 293 (!sequence_buffer_[start_index].used ||
(...skipping 47 matching lines...) Expand 10 before | Expand all | Expand 10 after
338 } 341 }
339 } 342 }
340 343
341 bool PacketBuffer::GetBitstream(const RtpFrameObject& frame, 344 bool PacketBuffer::GetBitstream(const RtpFrameObject& frame,
342 uint8_t* destination) { 345 uint8_t* destination) {
343 rtc::CritScope lock(&crit_); 346 rtc::CritScope lock(&crit_);
344 347
345 size_t index = frame.first_seq_num() % size_; 348 size_t index = frame.first_seq_num() % size_;
346 size_t end = (frame.last_seq_num() + 1) % size_; 349 size_t end = (frame.last_seq_num() + 1) % size_;
347 uint16_t seq_num = frame.first_seq_num(); 350 uint16_t seq_num = frame.first_seq_num();
348 while (index != end) { 351 uint8_t* destination_end = destination + frame.size();
352
353 do {
349 if (!sequence_buffer_[index].used || 354 if (!sequence_buffer_[index].used ||
350 sequence_buffer_[index].seq_num != seq_num) { 355 sequence_buffer_[index].seq_num != seq_num) {
351 return false; 356 return false;
352 } 357 }
353 358
359 RTC_DCHECK_EQ(data_buffer_[index].seqNum, sequence_buffer_[index].seq_num);
360 size_t length = data_buffer_[index].sizeBytes;
361 if (destination + length > destination_end) {
362 LOG(LS_WARNING) << "Frame (" << frame.picture_id << ":"
363 << static_cast<int>(frame.spatial_layer) << ")"
364 << " bitstream buffer is not large enough.";
365 return false;
366 }
367
354 const uint8_t* source = data_buffer_[index].dataPtr; 368 const uint8_t* source = data_buffer_[index].dataPtr;
355 size_t length = data_buffer_[index].sizeBytes;
356 memcpy(destination, source, length); 369 memcpy(destination, source, length);
357 destination += length; 370 destination += length;
358 index = (index + 1) % size_; 371 index = (index + 1) % size_;
359 ++seq_num; 372 ++seq_num;
360 } 373 } while (index != end);
374
361 return true; 375 return true;
362 } 376 }
363 377
364 VCMPacket* PacketBuffer::GetPacket(uint16_t seq_num) { 378 VCMPacket* PacketBuffer::GetPacket(uint16_t seq_num) {
365 size_t index = seq_num % size_; 379 size_t index = seq_num % size_;
366 if (!sequence_buffer_[index].used || 380 if (!sequence_buffer_[index].used ||
367 seq_num != sequence_buffer_[index].seq_num) { 381 seq_num != sequence_buffer_[index].seq_num) {
368 return nullptr; 382 return nullptr;
369 } 383 }
370 return &data_buffer_[index]; 384 return &data_buffer_[index];
(...skipping 31 matching lines...) Expand 10 before | Expand all | Expand 10 after
402 missing_packets_.insert(*newest_inserted_seq_num_); 416 missing_packets_.insert(*newest_inserted_seq_num_);
403 ++*newest_inserted_seq_num_; 417 ++*newest_inserted_seq_num_;
404 } 418 }
405 } else { 419 } else {
406 missing_packets_.erase(seq_num); 420 missing_packets_.erase(seq_num);
407 } 421 }
408 } 422 }
409 423
410 } // namespace video_coding 424 } // namespace video_coding
411 } // namespace webrtc 425 } // namespace webrtc
OLDNEW
« no previous file with comments | « webrtc/modules/video_coding/frame_object.cc ('k') | webrtc/modules/video_coding/video_packet_buffer_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698