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

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

Issue 2868723003: Break backwards traversal loop if we have looped around all packet in the PacketBuffer for H264 fra… (Closed)
Patch Set: Added comment. Created 3 years, 7 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
« no previous file with comments | « no previous file | webrtc/modules/video_coding/video_packet_buffer_unittest.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 179 matching lines...) Expand 10 before | Expand all | Expand 10 after
190 } 190 }
191 if (sequence_buffer_[prev_index].continuous) 191 if (sequence_buffer_[prev_index].continuous)
192 return true; 192 return true;
193 193
194 return false; 194 return false;
195 } 195 }
196 196
197 std::vector<std::unique_ptr<RtpFrameObject>> PacketBuffer::FindFrames( 197 std::vector<std::unique_ptr<RtpFrameObject>> PacketBuffer::FindFrames(
198 uint16_t seq_num) { 198 uint16_t seq_num) {
199 std::vector<std::unique_ptr<RtpFrameObject>> found_frames; 199 std::vector<std::unique_ptr<RtpFrameObject>> found_frames;
200 size_t packets_tested = 0; 200 for (size_t i = 0; i < size_ && PotentialNewFrame(seq_num); ++i) {
201 while (packets_tested < size_ && PotentialNewFrame(seq_num)) {
202 size_t index = seq_num % size_; 201 size_t index = seq_num % size_;
203 sequence_buffer_[index].continuous = true; 202 sequence_buffer_[index].continuous = true;
204 203
205 // If all packets of the frame is continuous, find the first packet of the 204 // If all packets of the frame is continuous, find the first packet of the
206 // frame and create an RtpFrameObject. 205 // frame and create an RtpFrameObject.
207 if (sequence_buffer_[index].frame_end) { 206 if (sequence_buffer_[index].frame_end) {
208 size_t frame_size = 0; 207 size_t frame_size = 0;
209 int max_nack_count = -1; 208 int max_nack_count = -1;
210 uint16_t start_seq_num = seq_num; 209 uint16_t start_seq_num = seq_num;
211 210
212 // Find the start index by searching backward until the packet with 211 // Find the start index by searching backward until the packet with
213 // the |frame_begin| flag is set. 212 // the |frame_begin| flag is set.
214 int start_index = index; 213 int start_index = index;
215 214
216 bool is_h264 = data_buffer_[start_index].codec == kVideoCodecH264; 215 bool is_h264 = data_buffer_[start_index].codec == kVideoCodecH264;
217 int64_t frame_timestamp = data_buffer_[start_index].timestamp; 216 int64_t frame_timestamp = data_buffer_[start_index].timestamp;
218 while (true) { 217
218 // Since packet at |data_buffer_[index]| is already part of the frame
219 // we will have at most |size_ - 1| packets left to check.
220 for (size_t j = 0; j < size_ - 1; ++j) {
219 frame_size += data_buffer_[start_index].sizeBytes; 221 frame_size += data_buffer_[start_index].sizeBytes;
220 max_nack_count = 222 max_nack_count =
221 std::max(max_nack_count, data_buffer_[start_index].timesNacked); 223 std::max(max_nack_count, data_buffer_[start_index].timesNacked);
222 sequence_buffer_[start_index].frame_created = true; 224 sequence_buffer_[start_index].frame_created = true;
223 225
224 if (!is_h264 && sequence_buffer_[start_index].frame_begin) 226 if (!is_h264 && sequence_buffer_[start_index].frame_begin)
225 break; 227 break;
226 228
227 start_index = start_index > 0 ? start_index - 1 : size_ - 1; 229 start_index = start_index > 0 ? start_index - 1 : size_ - 1;
228 230
229 // In the case of H264 we don't have a frame_begin bit (yes, 231 // In the case of H264 we don't have a frame_begin bit (yes,
230 // |frame_begin| might be set to true but that is a lie). So instead 232 // |frame_begin| might be set to true but that is a lie). So instead
231 // we traverese backwards as long as we have a previous packet and 233 // we traverese backwards as long as we have a previous packet and
232 // the timestamp of that packet is the same as this one. This may cause 234 // the timestamp of that packet is the same as this one. This may cause
233 // the PacketBuffer to hand out incomplete frames. 235 // the PacketBuffer to hand out incomplete frames.
234 // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7106 236 // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7106
235 // 237 if (is_h264 &&
236 // Since we ignore the |frame_begin| flag of the inserted packets
237 // we check that |start_index != static_cast<int>(index)| to make sure
238 // that we don't get stuck in a loop if the packet buffer is filled
239 // with packets of the same timestamp.
240 if (is_h264 && start_index != static_cast<int>(index) &&
241 (!sequence_buffer_[start_index].used || 238 (!sequence_buffer_[start_index].used ||
242 data_buffer_[start_index].timestamp != frame_timestamp)) { 239 data_buffer_[start_index].timestamp != frame_timestamp)) {
243 break; 240 break;
244 } 241 }
245 242
246 --start_seq_num; 243 --start_seq_num;
247 } 244 }
248 245
249 found_frames.emplace_back( 246 found_frames.emplace_back(
250 new RtpFrameObject(this, start_seq_num, seq_num, frame_size, 247 new RtpFrameObject(this, start_seq_num, seq_num, frame_size,
251 max_nack_count, clock_->TimeInMilliseconds())); 248 max_nack_count, clock_->TimeInMilliseconds()));
252 } 249 }
253 ++seq_num; 250 ++seq_num;
254 ++packets_tested;
255 } 251 }
256 return found_frames; 252 return found_frames;
257 } 253 }
258 254
259 void PacketBuffer::ReturnFrame(RtpFrameObject* frame) { 255 void PacketBuffer::ReturnFrame(RtpFrameObject* frame) {
260 rtc::CritScope lock(&crit_); 256 rtc::CritScope lock(&crit_);
261 size_t index = frame->first_seq_num() % size_; 257 size_t index = frame->first_seq_num() % size_;
262 size_t end = (frame->last_seq_num() + 1) % size_; 258 size_t end = (frame->last_seq_num() + 1) % size_;
263 uint16_t seq_num = frame->first_seq_num(); 259 uint16_t seq_num = frame->first_seq_num();
264 while (index != end) { 260 while (index != end) {
(...skipping 47 matching lines...) Expand 10 before | Expand all | Expand 10 after
312 int PacketBuffer::Release() const { 308 int PacketBuffer::Release() const {
313 int count = rtc::AtomicOps::Decrement(&ref_count_); 309 int count = rtc::AtomicOps::Decrement(&ref_count_);
314 if (!count) { 310 if (!count) {
315 delete this; 311 delete this;
316 } 312 }
317 return count; 313 return count;
318 } 314 }
319 315
320 } // namespace video_coding 316 } // namespace video_coding
321 } // namespace webrtc 317 } // namespace webrtc
OLDNEW
« no previous file with comments | « no previous file | webrtc/modules/video_coding/video_packet_buffer_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698