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

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

Issue 2990183002: Revert of Fix off-by-one bugs in video_coding::PacketBuffer when the buffer is filled with a single frame. (Closed)
Patch Set: 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;
255 254
256 bool is_h264 = data_buffer_[start_index].codec == kVideoCodecH264; 255 bool is_h264 = data_buffer_[start_index].codec == kVideoCodecH264;
257 bool is_h264_keyframe = false; 256 bool is_h264_keyframe = false;
258 int64_t frame_timestamp = data_buffer_[start_index].timestamp; 257 int64_t frame_timestamp = data_buffer_[start_index].timestamp;
259 258
260 while (true) { 259 // Since packet at |data_buffer_[index]| is already part of the frame
261 ++tested_packets; 260 // we will have at most |size_ - 1| packets left to check.
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
284 start_index = start_index > 0 ? start_index - 1 : size_ - 1; 281 start_index = start_index > 0 ? start_index - 1 : size_ - 1;
285 282
286 // In the case of H264 we don't have a frame_begin bit (yes, 283 // In the case of H264 we don't have a frame_begin bit (yes,
287 // |frame_begin| might be set to true but that is a lie). So instead 284 // |frame_begin| might be set to true but that is a lie). So instead
288 // we traverese backwards as long as we have a previous packet and 285 // we traverese backwards as long as we have a previous packet and
289 // the timestamp of that packet is the same as this one. This may cause 286 // the timestamp of that packet is the same as this one. This may cause
290 // the PacketBuffer to hand out incomplete frames. 287 // the PacketBuffer to hand out incomplete frames.
291 // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7106 288 // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7106
292 if (is_h264 && 289 if (is_h264 &&
293 (!sequence_buffer_[start_index].used || 290 (!sequence_buffer_[start_index].used ||
(...skipping 47 matching lines...) Expand 10 before | Expand all | Expand 10 after
341 } 338 }
342 } 339 }
343 340
344 bool PacketBuffer::GetBitstream(const RtpFrameObject& frame, 341 bool PacketBuffer::GetBitstream(const RtpFrameObject& frame,
345 uint8_t* destination) { 342 uint8_t* destination) {
346 rtc::CritScope lock(&crit_); 343 rtc::CritScope lock(&crit_);
347 344
348 size_t index = frame.first_seq_num() % size_; 345 size_t index = frame.first_seq_num() % size_;
349 size_t end = (frame.last_seq_num() + 1) % size_; 346 size_t end = (frame.last_seq_num() + 1) % size_;
350 uint16_t seq_num = frame.first_seq_num(); 347 uint16_t seq_num = frame.first_seq_num();
351 uint8_t* destination_end = destination + frame.size(); 348 while (index != end) {
352
353 do {
354 if (!sequence_buffer_[index].used || 349 if (!sequence_buffer_[index].used ||
355 sequence_buffer_[index].seq_num != seq_num) { 350 sequence_buffer_[index].seq_num != seq_num) {
356 return false; 351 return false;
357 } 352 }
358 353
359 RTC_DCHECK_EQ(data_buffer_[index].seqNum, sequence_buffer_[index].seq_num); 354 const uint8_t* source = data_buffer_[index].dataPtr;
360 size_t length = data_buffer_[index].sizeBytes; 355 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
368 const uint8_t* source = data_buffer_[index].dataPtr;
369 memcpy(destination, source, length); 356 memcpy(destination, source, length);
370 destination += length; 357 destination += length;
371 index = (index + 1) % size_; 358 index = (index + 1) % size_;
372 ++seq_num; 359 ++seq_num;
373 } while (index != end); 360 }
374
375 return true; 361 return true;
376 } 362 }
377 363
378 VCMPacket* PacketBuffer::GetPacket(uint16_t seq_num) { 364 VCMPacket* PacketBuffer::GetPacket(uint16_t seq_num) {
379 size_t index = seq_num % size_; 365 size_t index = seq_num % size_;
380 if (!sequence_buffer_[index].used || 366 if (!sequence_buffer_[index].used ||
381 seq_num != sequence_buffer_[index].seq_num) { 367 seq_num != sequence_buffer_[index].seq_num) {
382 return nullptr; 368 return nullptr;
383 } 369 }
384 return &data_buffer_[index]; 370 return &data_buffer_[index];
(...skipping 31 matching lines...) Expand 10 before | Expand all | Expand 10 after
416 missing_packets_.insert(*newest_inserted_seq_num_); 402 missing_packets_.insert(*newest_inserted_seq_num_);
417 ++*newest_inserted_seq_num_; 403 ++*newest_inserted_seq_num_;
418 } 404 }
419 } else { 405 } else {
420 missing_packets_.erase(seq_num); 406 missing_packets_.erase(seq_num);
421 } 407 }
422 } 408 }
423 409
424 } // namespace video_coding 410 } // namespace video_coding
425 } // namespace webrtc 411 } // 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