Added ClearTo(seq_num) to RtpFrameReferenceFinder.

webrtc/modules/video_coding/rtp_frame_reference_finder.cc

Index: webrtc/modules/video_coding/rtp_frame_reference_finder.cc
diff --git a/webrtc/modules/video_coding/rtp_frame_reference_finder.cc b/webrtc/modules/video_coding/rtp_frame_reference_finder.cc
index 998d757a7bc47089a9d52a32b4d38cbf474993ab..6e378fc6ad68293a51d44cd34a723917a39eab18 100644
--- a/webrtc/modules/video_coding/rtp_frame_reference_finder.cc
+++ b/webrtc/modules/video_coding/rtp_frame_reference_finder.cc
@@ -26,11 +26,19 @@ RtpFrameReferenceFinder::RtpFrameReferenceFinder(
     : last_picture_id_(-1),
       last_unwrap_(-1),
       current_ss_idx_(0),
+      cleared_to_seq_num_(-1),
       frame_callback_(frame_callback) {}
 
 void RtpFrameReferenceFinder::ManageFrame(
     std::unique_ptr<RtpFrameObject> frame) {
   rtc::CritScope lock(&crit_);
+
+  // If we have cleared past this frame, drop it.
+  if (cleared_to_seq_num_ != -1 &&
+      AheadOf<uint16_t>(cleared_to_seq_num_, frame->first_seq_num())) {
+    return;
+  }
+
   switch (frame->codec_type()) {
     case kVideoCodecULPFEC:
     case kVideoCodecRED:
@@ -61,6 +69,22 @@ void RtpFrameReferenceFinder::PaddingReceived(uint16_t seq_num) {
   RetryStashedFrames();
 }
 
+void RtpFrameReferenceFinder::ClearTo(uint16_t seq_num) {
+  rtc::CritScope lock(&crit_);
+  cleared_to_seq_num_ = seq_num;
+  size_t num_stashed_frames = stashed_frames_.size();
+
+  for (size_t i = 0; i < num_stashed_frames; ++i) {

[stefan-webrtc, 2016/09/07 13:29:57]

Isn't it simpler to write this:

while (AheadOf<uint16_t>(cleared_to_seq_num_,
stashed_frames_.front()->first_seq_num()) {
  stashed_frames_.pop();
}

[philipel, 2016/09/08 11:27:19]

Frames might be inserted out of order, so we don't know if the oldest frames are
first in the queue, and even if they were inserted in order I'm not 100% sure
that order would be maintained.

[stefan-webrtc, 2016/09/08 11:51:30]

Ok, but what about:

for (it = stashed_frames_.begin(); ... ; ++it) {
  if (AheadOf<uint16_t>(cleared_to_seq_num_, it->first_seq_num()))
    stashed_frames_.erase(it++);

}

My main point is that it seems unnecessarily complicated to first pop the frame
and then insert it back if it wasn't too old. I guess that'd mean you have to a
list instead, but maybe that makes sense?

[philipel, 2016/09/08 12:16:03]

Changed to deque

+    std::unique_ptr<RtpFrameObject> frame = std::move(stashed_frames_.front());
+    stashed_frames_.pop();
+
+    if (AheadOf<uint16_t>(cleared_to_seq_num_, frame->first_seq_num()))
+      continue;
+
+    stashed_frames_.emplace(std::move(frame));
+  }
+}
+
 void RtpFrameReferenceFinder::UpdateLastPictureIdWithPadding(uint16_t seq_num) {
   auto gop_seq_num_it = last_seq_num_gop_.upper_bound(seq_num);
 
@@ -97,6 +121,8 @@ void RtpFrameReferenceFinder::RetryStashedFrames() {
   // Since frames are stashed if there is not enough data to determine their
   // frame references we should at most check |stashed_frames_.size()| in
   // order to not pop and push frames in and endless loop.
+  // NOTE! This function may be called recursively, hence the
+  //       "!stashed_frames_.empty()" condition.
   for (size_t i = 0; i < num_stashed_frames && !stashed_frames_.empty(); ++i) {
     std::unique_ptr<RtpFrameObject> frame = std::move(stashed_frames_.front());
     stashed_frames_.pop();