Stashed frames are now retried in a loop rather than recursively.

webrtc/modules/video_coding/rtp_frame_reference_finder.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 21 matching lines @@
 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;
   }
 
+  if (stashed_frames_.size() > kMaxStashedFrames)
+    stashed_frames_.pop_back();

[stefan-webrtc, 2017/04/25 15:27:48]

Here we pop_back(), before we did pop_front()?

[philipel, 2017/04/26 12:22:27]

The order has changed so new frames are at the front and old frames are at the
back. It makes it easier to iterate over the frames in the order we want since
we don't have to use rbegin() and rend().

+
+  stashed_frames_.push_front(std::move(frame));
+  RetryStashedFrames();
+}
+
+void RtpFrameReferenceFinder::RetryStashedFrames() {
+  bool complete_frame = false;
+  do {
+    complete_frame = false;
+    for (auto frame_it = stashed_frames_.begin();
+         frame_it != stashed_frames_.end();) {
+      FrameDecision decision = ManageFrameInternal(frame_it->get());
+
+      switch (decision) {
+        case kStash:
+          ++frame_it;
+          break;
+        case kSend:
+          complete_frame = true;
+          frame_callback_->OnCompleteFrame(std::move(*frame_it));
+          FALLTHROUGH();
+        case kDrop:
+          frame_it = stashed_frames_.erase(frame_it);
+      }
+    }
+  } while (complete_frame);
+}
+
+RtpFrameReferenceFinder::FrameDecision
+RtpFrameReferenceFinder::ManageFrameInternal(RtpFrameObject* frame) {
   switch (frame->codec_type()) {
     case kVideoCodecFlexfec:
     case kVideoCodecULPFEC:
     case kVideoCodecRED:
       RTC_NOTREACHED();
       break;
     case kVideoCodecVP8:
-      ManageFrameVp8(std::move(frame));
-      break;
+      return ManageFrameVp8(frame);
     case kVideoCodecVP9:
-      ManageFrameVp9(std::move(frame));
-      break;
+      return ManageFrameVp9(frame);
     // Since the EndToEndTests use kVicdeoCodecUnknow we treat it the same as
     // kVideoCodecGeneric.
     // TODO(philipel): Take a look at the EndToEndTests and see if maybe they
     //                 should be changed to use kVideoCodecGeneric instead.
     case kVideoCodecUnknown:
     case kVideoCodecH264:
     case kVideoCodecI420:
     case kVideoCodecGeneric:
-      ManageFrameGeneric(std::move(frame), kNoPictureId);
-      break;
+      return ManageFrameGeneric(frame, kNoPictureId);
   }
 }
 
 void RtpFrameReferenceFinder::PaddingReceived(uint16_t seq_num) {
   rtc::CritScope lock(&crit_);
   auto clean_padding_to =
       stashed_padding_.lower_bound(seq_num - kMaxPaddingAge);
   stashed_padding_.erase(stashed_padding_.begin(), clean_padding_to);
   stashed_padding_.insert(seq_num);
   UpdateLastPictureIdWithPadding(seq_num);
@@ skipping 43 matching lines @@
   // for a while there is a risk that new frames will appear to be older than
   // the keyframe they belong to due to wrapping sequence number. In order
   // to prevent this we advance the picture id of the keyframe every so often.
   if (ForwardDiff(gop_seq_num_it->first, seq_num) > 10000) {
     RTC_DCHECK_EQ(1ul, last_seq_num_gop_.size());
     last_seq_num_gop_[seq_num] = gop_seq_num_it->second;
     last_seq_num_gop_.erase(gop_seq_num_it);
   }
 }
 
-void RtpFrameReferenceFinder::RetryStashedFrames() {
-  size_t num_stashed_frames = stashed_frames_.size();
-
-  // Clean up stashed frames if there are too many.
-  while (stashed_frames_.size() > kMaxStashedFrames)
-    stashed_frames_.pop_front();
-
-  // 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_front();
-    ManageFrame(std::move(frame));
-  }
-}
-
-void RtpFrameReferenceFinder::ManageFrameGeneric(
-    std::unique_ptr<RtpFrameObject> frame,
-    int picture_id) {
+RtpFrameReferenceFinder::FrameDecision
+RtpFrameReferenceFinder::ManageFrameGeneric(RtpFrameObject* frame,
+                                            int picture_id) {
   // If |picture_id| is specified then we use that to set the frame references,
   // otherwise we use sequence number.
   if (picture_id != kNoPictureId) {
     if (last_unwrap_ == -1)
       last_unwrap_ = picture_id;
 
     frame->picture_id = UnwrapPictureId(picture_id % kPicIdLength);
     frame->num_references = frame->frame_type() == kVideoFrameKey ? 0 : 1;
     frame->references[0] = frame->picture_id - 1;
-    frame_callback_->OnCompleteFrame(std::move(frame));
-    return;
+    return kSend;
   }
 
   if (frame->frame_type() == kVideoFrameKey) {
     last_seq_num_gop_.insert(std::make_pair(
         frame->last_seq_num(),
         std::make_pair(frame->last_seq_num(), frame->last_seq_num())));
   }
 
   // We have received a frame but not yet a keyframe, stash this frame.
-  if (last_seq_num_gop_.empty()) {
-    stashed_frames_.push_back(std::move(frame));
-    return;
-  }
+  if (last_seq_num_gop_.empty())
+    return kStash;
 
   // Clean up info for old keyframes but make sure to keep info
   // for the last keyframe.
   auto clean_to = last_seq_num_gop_.lower_bound(frame->last_seq_num() - 100);
   for (auto it = last_seq_num_gop_.begin();
        it != clean_to && last_seq_num_gop_.size() > 1;) {
     it = last_seq_num_gop_.erase(it);
   }
 
   // Find the last sequence number of the last frame for the keyframe
   // that this frame indirectly references.
   auto seq_num_it = last_seq_num_gop_.upper_bound(frame->last_seq_num());
   if (seq_num_it == last_seq_num_gop_.begin()) {
     LOG(LS_WARNING) << "Generic frame with packet range ["
                     << frame->first_seq_num() << ", " << frame->last_seq_num()
                     << "] has no GoP, dropping frame.";
-    return;
+    return kDrop;
   }
   seq_num_it--;
 
   // Make sure the packet sequence numbers are continuous, otherwise stash
   // this frame.
   uint16_t last_picture_id_gop = seq_num_it->second.first;
   uint16_t last_picture_id_with_padding_gop = seq_num_it->second.second;
   if (frame->frame_type() == kVideoFrameDelta) {
     uint16_t prev_seq_num = frame->first_seq_num() - 1;
-    if (prev_seq_num != last_picture_id_with_padding_gop) {
-      stashed_frames_.push_back(std::move(frame));
-      return;
-    }
+
+    if (prev_seq_num != last_picture_id_with_padding_gop)
+      return kStash;
   }
 
   RTC_DCHECK(AheadOrAt(frame->last_seq_num(), seq_num_it->first));
 
   // Since keyframes can cause reordering we can't simply assign the
   // picture id according to some incrementing counter.
   frame->picture_id = frame->last_seq_num();
   frame->num_references = frame->frame_type() == kVideoFrameDelta;
   frame->references[0] = last_picture_id_gop;
   if (AheadOf(frame->picture_id, last_picture_id_gop)) {
     seq_num_it->second.first = frame->picture_id;
     seq_num_it->second.second = frame->picture_id;
   }
 
   last_picture_id_ = frame->picture_id;
   UpdateLastPictureIdWithPadding(frame->picture_id);
-  frame_callback_->OnCompleteFrame(std::move(frame));
-  RetryStashedFrames();
+  return kSend;
 }
 
-void RtpFrameReferenceFinder::ManageFrameVp8(
-    std::unique_ptr<RtpFrameObject> frame) {
+RtpFrameReferenceFinder::FrameDecision RtpFrameReferenceFinder::ManageFrameVp8(
+    RtpFrameObject* frame) {
   rtc::Optional<RTPVideoTypeHeader> rtp_codec_header = frame->GetCodecHeader();
   if (!rtp_codec_header)
-    return;
+    return kDrop;
 
   const RTPVideoHeaderVP8& codec_header = rtp_codec_header->VP8;
 
   if (codec_header.pictureId == kNoPictureId ||
       codec_header.temporalIdx == kNoTemporalIdx ||
       codec_header.tl0PicIdx == kNoTl0PicIdx) {
-    ManageFrameGeneric(std::move(frame), codec_header.pictureId);
-    return;
+    return ManageFrameGeneric(std::move(frame), codec_header.pictureId);
   }
 
   frame->picture_id = codec_header.pictureId % kPicIdLength;
 
   if (last_unwrap_ == -1)
     last_unwrap_ = codec_header.pictureId;
 
   if (last_picture_id_ == -1)
     last_picture_id_ = frame->picture_id;
 
@@ skipping 15 matching lines @@
   // Clean up info about not yet received frames that are too old.
   uint16_t old_picture_id =
       Subtract<kPicIdLength>(frame->picture_id, kMaxNotYetReceivedFrames);
   auto clean_frames_to = not_yet_received_frames_.lower_bound(old_picture_id);
   not_yet_received_frames_.erase(not_yet_received_frames_.begin(),
                                  clean_frames_to);
 
   if (frame->frame_type() == kVideoFrameKey) {
     frame->num_references = 0;
     layer_info_[codec_header.tl0PicIdx].fill(-1);
-    CompletedFrameVp8(std::move(frame));
-    return;
+    CompletedFrameVp8(frame);
+    return kSend;
   }
 
   auto layer_info_it = layer_info_.find(codec_header.temporalIdx == 0
                                             ? codec_header.tl0PicIdx - 1
                                             : codec_header.tl0PicIdx);
 
   // If we don't have the base layer frame yet, stash this frame.
-  if (layer_info_it == layer_info_.end()) {
-    stashed_frames_.push_back(std::move(frame));
-    return;
-  }
+  if (layer_info_it == layer_info_.end())
+    return kStash;
 
   // A non keyframe base layer frame has been received, copy the layer info
   // from the previous base layer frame and set a reference to the previous
   // base layer frame.
   if (codec_header.temporalIdx == 0) {
     layer_info_it =
         layer_info_
             .insert(make_pair(codec_header.tl0PicIdx, layer_info_it->second))
             .first;
     frame->num_references = 1;
     frame->references[0] = layer_info_it->second[0];
-    CompletedFrameVp8(std::move(frame));
-    return;
+    CompletedFrameVp8(frame);
+    return kSend;
   }
 
   // Layer sync frame, this frame only references its base layer frame.
   if (codec_header.layerSync) {
     frame->num_references = 1;
     frame->references[0] = layer_info_it->second[0];
 
-    CompletedFrameVp8(std::move(frame));
-    return;
+    CompletedFrameVp8(frame);
+    return kSend;
   }
 
   // Find all references for this frame.
   frame->num_references = 0;
   for (uint8_t layer = 0; layer <= codec_header.temporalIdx; ++layer) {
     // If we have not yet received a previous frame on this temporal layer,
     // stash this frame.
-    if (layer_info_it->second[layer] == -1) {
-      stashed_frames_.push_back(std::move(frame));
-      return;
-    }
+    if (layer_info_it->second[layer] == -1)
+      return kStash;
 
     // If the last frame on this layer is ahead of this frame it means that
     // a layer sync frame has been received after this frame for the same
     // base layer frame, drop this frame.
     if (AheadOf<uint16_t, kPicIdLength>(layer_info_it->second[layer],
                                         frame->picture_id)) {
-      return;
+      return kDrop;
     }
 
     // If we have not yet received a frame between this frame and the referenced
     // frame then we have to wait for that frame to be completed first.
     auto not_received_frame_it =
         not_yet_received_frames_.upper_bound(layer_info_it->second[layer]);
     if (not_received_frame_it != not_yet_received_frames_.end() &&
         AheadOf<uint16_t, kPicIdLength>(frame->picture_id,
                                         *not_received_frame_it)) {
-      stashed_frames_.push_back(std::move(frame));
-      return;
+      return kStash;
     }
 
     if (!(AheadOf<uint16_t, kPicIdLength>(frame->picture_id,
                                           layer_info_it->second[layer]))) {
       LOG(LS_WARNING) << "Frame with picture id " << frame->picture_id
                       << " and packet range [" << frame->first_seq_num() << ", "
                       << frame->last_seq_num() << "] already received, "
                       << " dropping frame.";
-      return;
+      return kDrop;
     }
 
     ++frame->num_references;
     frame->references[layer] = layer_info_it->second[layer];
   }
 
-  CompletedFrameVp8(std::move(frame));
+  CompletedFrameVp8(frame);
+  return kSend;
 }
 
-void RtpFrameReferenceFinder::CompletedFrameVp8(
-    std::unique_ptr<RtpFrameObject> frame) {
+void RtpFrameReferenceFinder::CompletedFrameVp8(RtpFrameObject* frame) {
   rtc::Optional<RTPVideoTypeHeader> rtp_codec_header = frame->GetCodecHeader();
-  if (!rtp_codec_header)
-    return;
-
+  RTC_DCHECK(rtp_codec_header);
   const RTPVideoHeaderVP8& codec_header = rtp_codec_header->VP8;
 
   uint8_t tl0_pic_idx = codec_header.tl0PicIdx;
   uint8_t temporal_index = codec_header.temporalIdx;
   auto layer_info_it = layer_info_.find(tl0_pic_idx);
 
   // Update this layer info and newer.
   while (layer_info_it != layer_info_.end()) {
     if (layer_info_it->second[temporal_index] != -1 &&
         AheadOf<uint16_t, kPicIdLength>(layer_info_it->second[temporal_index],
                                         frame->picture_id)) {
       // The frame was not newer, then no subsequent layer info have to be
       // update.
       break;
     }
 
     layer_info_it->second[codec_header.temporalIdx] = frame->picture_id;
     ++tl0_pic_idx;
     layer_info_it = layer_info_.find(tl0_pic_idx);
   }
   not_yet_received_frames_.erase(frame->picture_id);
 
   for (size_t i = 0; i < frame->num_references; ++i)
     frame->references[i] = UnwrapPictureId(frame->references[i]);
   frame->picture_id = UnwrapPictureId(frame->picture_id);
-
-  frame_callback_->OnCompleteFrame(std::move(frame));
-  RetryStashedFrames();
 }
 
-void RtpFrameReferenceFinder::ManageFrameVp9(
-    std::unique_ptr<RtpFrameObject> frame) {
+RtpFrameReferenceFinder::FrameDecision RtpFrameReferenceFinder::ManageFrameVp9(
+    RtpFrameObject* frame) {
   rtc::Optional<RTPVideoTypeHeader> rtp_codec_header = frame->GetCodecHeader();
-  if (!rtp_codec_header)
-    return;
-
+  RTC_DCHECK(rtp_codec_header);
   const RTPVideoHeaderVP9& codec_header = rtp_codec_header->VP9;
 
   bool old_frame = Vp9PidTl0Fix(*frame, &rtp_codec_header->VP9.picture_id,
                                 &rtp_codec_header->VP9.tl0_pic_idx);
   if (old_frame)
-    return;
+    return kDrop;
 
   if (codec_header.picture_id == kNoPictureId ||
       codec_header.temporal_idx == kNoTemporalIdx) {
-    ManageFrameGeneric(std::move(frame), codec_header.picture_id);
-    return;
+    return ManageFrameGeneric(std::move(frame), codec_header.picture_id);
   }
 
   frame->spatial_layer = codec_header.spatial_idx;
   frame->inter_layer_predicted = codec_header.inter_layer_predicted;
   frame->picture_id = codec_header.picture_id % kPicIdLength;
 
   if (last_unwrap_ == -1)
     last_unwrap_ = codec_header.picture_id;
 
   if (last_picture_id_ == -1)
     last_picture_id_ = frame->picture_id;
 
   if (codec_header.flexible_mode) {
     frame->num_references = codec_header.num_ref_pics;
     for (size_t i = 0; i < frame->num_references; ++i) {
       frame->references[i] =
           Subtract<1 << 16>(frame->picture_id, codec_header.pid_diff[i]);
     }
 
-    CompletedFrameVp9(std::move(frame));
-    return;
+    CompletedFrameVp9(frame);
+    return kSend;
   }
 
   if (codec_header.ss_data_available) {
     // Scalability structures can only be sent with tl0 frames.
     if (codec_header.temporal_idx != 0) {
       LOG(LS_WARNING) << "Received scalability structure on a non base layer"
                          " frame. Scalability structure ignored.";
     } else {
       current_ss_idx_ = Add<kMaxGofSaved>(current_ss_idx_, 1);
       scalability_structures_[current_ss_idx_] = codec_header.gof;
@@ skipping 11 matching lines @@
   gof_info_.erase(gof_info_.begin(), clean_gof_info_to);
 
   if (frame->frame_type() == kVideoFrameKey) {
     // When using GOF all keyframes must include the scalability structure.
     if (!codec_header.ss_data_available)
       LOG(LS_WARNING) << "Received keyframe without scalability structure";
 
     frame->num_references = 0;
     GofInfo info = gof_info_.find(codec_header.tl0_pic_idx)->second;
     FrameReceivedVp9(frame->picture_id, &info);
-    CompletedFrameVp9(std::move(frame));
-    return;
+    CompletedFrameVp9(frame);
+    return kSend;
   }
 
   auto gof_info_it = gof_info_.find(
       (codec_header.temporal_idx == 0 && !codec_header.ss_data_available)
           ? codec_header.tl0_pic_idx - 1
           : codec_header.tl0_pic_idx);
 
   // Gof info for this frame is not available yet, stash this frame.
-  if (gof_info_it == gof_info_.end()) {
-    stashed_frames_.push_back(std::move(frame));
-    return;
-  }
+  if (gof_info_it == gof_info_.end())
+    return kStash;
 
   GofInfo* info = &gof_info_it->second;
   FrameReceivedVp9(frame->picture_id, info);
 
   // Make sure we don't miss any frame that could potentially have the
   // up switch flag set.
-  if (MissingRequiredFrameVp9(frame->picture_id, *info)) {
-    stashed_frames_.push_back(std::move(frame));
-    return;
-  }
+  if (MissingRequiredFrameVp9(frame->picture_id, *info))
+    return kStash;
 
   if (codec_header.temporal_up_switch) {
     auto pid_tidx =
         std::make_pair(frame->picture_id, codec_header.temporal_idx);
     up_switch_.insert(pid_tidx);
   }
 
   // If this is a base layer frame that contains a scalability structure
   // then gof info has already been inserted earlier, so we only want to
   // insert if we haven't done so already.
@@ skipping 18 matching lines @@
         frame->picture_id, info->gof->pid_diff[gof_idx][i]);
 
     // If this is a reference to a frame earlier than the last up switch point,
     // then ignore this reference.
     if (UpSwitchInIntervalVp9(frame->picture_id, codec_header.temporal_idx,
                               frame->references[i])) {
       --frame->num_references;
     }
   }
 
-  CompletedFrameVp9(std::move(frame));
+  CompletedFrameVp9(frame);
+  return kSend;
 }
 
 bool RtpFrameReferenceFinder::MissingRequiredFrameVp9(uint16_t picture_id,
                                                       const GofInfo& info) {
   size_t diff =
       ForwardDiff<uint16_t, kPicIdLength>(info.gof->pid_start, picture_id);
   size_t gof_idx = diff % info.gof->num_frames_in_gof;
   size_t temporal_idx = info.gof->temporal_idx[gof_idx];
 
   // For every reference this frame has, check if there is a frame missing in
@@ skipping 51 matching lines @@
        up_switch_it != up_switch_.end() &&
        AheadOf<uint16_t, kPicIdLength>(picture_id, up_switch_it->first);
        ++up_switch_it) {
     if (up_switch_it->second < temporal_idx)
       return true;
   }
 
   return false;
 }
 
-void RtpFrameReferenceFinder::CompletedFrameVp9(
-    std::unique_ptr<RtpFrameObject> frame) {
+void RtpFrameReferenceFinder::CompletedFrameVp9(RtpFrameObject* frame) {

[stefan-webrtc, 2017/04/25 15:27:48]

CompletedFrameVp9 and CompletedFrameVp8 don't really say what these methods do.
I think we can pick a better name for them...

[philipel, 2017/04/26 12:22:27]

Renamed CompletedFrameVp9 to UnwrapPictureIds since that is all this function
does now.

Renamed CompletedFrameVp8 to UpdateLayerInfoVp8, which I think is a fairly
accurate/descriptive name.

   for (size_t i = 0; i < frame->num_references; ++i)
     frame->references[i] = UnwrapPictureId(frame->references[i]);
   frame->picture_id = UnwrapPictureId(frame->picture_id);
-
-  frame_callback_->OnCompleteFrame(std::move(frame));
-  RetryStashedFrames();
 }
 
 uint16_t RtpFrameReferenceFinder::UnwrapPictureId(uint16_t picture_id) {
   RTC_DCHECK_NE(-1, last_unwrap_);
 
   uint16_t unwrap_truncated = last_unwrap_ % kPicIdLength;
   uint16_t diff = MinDiff<uint16_t, kPicIdLength>(unwrap_truncated, picture_id);
 
   if (AheadOf<uint16_t, kPicIdLength>(picture_id, unwrap_truncated))
     last_unwrap_ = Add<1 << 16>(last_unwrap_, diff);
@@ skipping 141 matching lines @@
     if (!gof_info_.empty() &&
         AheadOf<uint8_t>(gof_info_.begin()->first, fixed_tl0)) {
       return true;
     }
   }
   return false;
 }
 
 }  // namespace video_coding
 }  // namespace webrtc