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

BUG=none

Review-Url: https://codereview.webrtc.org/2841913002
Cr-Commit-Position: refs/heads/master@{#17890}

webrtc/modules/video_coding/rtp_frame_reference_finder.cc

@@ -39,6 +39,48 @@ void RtpFrameReferenceFinder::ManageFrame(
     return;
   }
 
+  FrameDecision decision = ManageFrameInternal(frame.get());
+
+  switch (decision) {
+    case kStash:
+      if (stashed_frames_.size() > kMaxStashedFrames)
+        stashed_frames_.pop_back();
+      stashed_frames_.push_front(std::move(frame));
+      break;
+    case kHandOff:
+      frame_callback_->OnCompleteFrame(std::move(frame));
+      RetryStashedFrames();
+      break;
+    case kDrop:
+      break;
+  }
+}
+
+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 kHandOff:
+          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:
@@ -46,11 +88,9 @@ void RtpFrameReferenceFinder::ManageFrame(
       RTC_NOTREACHED();
       break;
     case kVideoCodecVP8:
-      ManageFrameVp8(std::move(frame));
+      return ManageFrameVp8(frame);
-      break;
     case kVideoCodecVP9:
-      ManageFrameVp9(std::move(frame));
+      return ManageFrameVp9(frame);
-      break;
     // 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
@@ -59,9 +99,12 @@ void RtpFrameReferenceFinder::ManageFrame(
     case kVideoCodecH264:
     case kVideoCodecI420:
     case kVideoCodecGeneric:
-      ManageFrameGeneric(std::move(frame), kNoPictureId);
+      return ManageFrameGeneric(frame, kNoPictureId);
-      break;
   }
+
+  // If not all code paths return a value it makes the win compiler sad.
+  RTC_NOTREACHED();
+  return kDrop;
 }
 
 void RtpFrameReferenceFinder::PaddingReceived(uint16_t seq_num) {
@@ -124,28 +167,9 @@ void RtpFrameReferenceFinder::UpdateLastPictureIdWithPadding(uint16_t seq_num) {
   }
 }
 
-void RtpFrameReferenceFinder::RetryStashedFrames() {
+RtpFrameReferenceFinder::FrameDecision
-  size_t num_stashed_frames = stashed_frames_.size();
+RtpFrameReferenceFinder::ManageFrameGeneric(RtpFrameObject* frame,
-
+                                            int picture_id) {
-  // 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) {
   // If |picture_id| is specified then we use that to set the frame references,
   // otherwise we use sequence number.
   if (picture_id != kNoPictureId) {
@@ -155,8 +179,7 @@ void RtpFrameReferenceFinder::ManageFrameGeneric(
     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 kHandOff;
-    return;
   }
 
   if (frame->frame_type() == kVideoFrameKey) {
@@ -166,10 +189,8 @@ void RtpFrameReferenceFinder::ManageFrameGeneric(
   }
 
   // We have received a frame but not yet a keyframe, stash this frame.
-  if (last_seq_num_gop_.empty()) {
+  if (last_seq_num_gop_.empty())
-    stashed_frames_.push_back(std::move(frame));
+    return kStash;
-    return;
-  }
 
   // Clean up info for old keyframes but make sure to keep info
   // for the last keyframe.
@@ -186,7 +207,7 @@ void RtpFrameReferenceFinder::ManageFrameGeneric(
     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--;
 
@@ -196,10 +217,9 @@ void RtpFrameReferenceFinder::ManageFrameGeneric(
   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));
+    if (prev_seq_num != last_picture_id_with_padding_gop)
-      return;
+      return kStash;
-    }
   }
 
   RTC_DCHECK(AheadOrAt(frame->last_seq_num(), seq_num_it->first));
@@ -216,23 +236,21 @@ void RtpFrameReferenceFinder::ManageFrameGeneric(
 
   last_picture_id_ = frame->picture_id;
   UpdateLastPictureIdWithPadding(frame->picture_id);
-  frame_callback_->OnCompleteFrame(std::move(frame));
+  return kHandOff;
-  RetryStashedFrames();
 }
 
-void RtpFrameReferenceFinder::ManageFrameVp8(
+RtpFrameReferenceFinder::FrameDecision RtpFrameReferenceFinder::ManageFrameVp8(
-    std::unique_ptr<RtpFrameObject> frame) {
+    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 ManageFrameGeneric(std::move(frame), codec_header.pictureId);
-    return;
   }
 
   frame->picture_id = codec_header.pictureId % kPicIdLength;
@@ -268,8 +286,8 @@ void RtpFrameReferenceFinder::ManageFrameVp8(
   if (frame->frame_type() == kVideoFrameKey) {
     frame->num_references = 0;
     layer_info_[codec_header.tl0PicIdx].fill(-1);
-    CompletedFrameVp8(std::move(frame));
+    UpdateLayerInfoVp8(frame);
-    return;
+    return kHandOff;
   }
 
   auto layer_info_it = layer_info_.find(codec_header.temporalIdx == 0
@@ -277,10 +295,8 @@ void RtpFrameReferenceFinder::ManageFrameVp8(
                                             : codec_header.tl0PicIdx);
 
   // If we don't have the base layer frame yet, stash this frame.
-  if (layer_info_it == layer_info_.end()) {
+  if (layer_info_it == layer_info_.end())
-    stashed_frames_.push_back(std::move(frame));
+    return kStash;
-    return;
-  }
 
   // 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
@@ -292,8 +308,8 @@ void RtpFrameReferenceFinder::ManageFrameVp8(
             .first;
     frame->num_references = 1;
     frame->references[0] = layer_info_it->second[0];
-    CompletedFrameVp8(std::move(frame));
+    UpdateLayerInfoVp8(frame);
-    return;
+    return kHandOff;
   }
 
   // Layer sync frame, this frame only references its base layer frame.
@@ -301,8 +317,8 @@ void RtpFrameReferenceFinder::ManageFrameVp8(
     frame->num_references = 1;
     frame->references[0] = layer_info_it->second[0];
 
-    CompletedFrameVp8(std::move(frame));
+    UpdateLayerInfoVp8(frame);
-    return;
+    return kHandOff;
   }
 
   // Find all references for this frame.
@@ -310,17 +326,15 @@ void RtpFrameReferenceFinder::ManageFrameVp8(
   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) {
+    if (layer_info_it->second[layer] == -1)
-      stashed_frames_.push_back(std::move(frame));
+      return kStash;
-      return;
-    }
 
     // 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
@@ -330,8 +344,7 @@ void RtpFrameReferenceFinder::ManageFrameVp8(
     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 kStash;
-      return;
     }
 
     if (!(AheadOf<uint16_t, kPicIdLength>(frame->picture_id,
@@ -340,22 +353,20 @@ void RtpFrameReferenceFinder::ManageFrameVp8(
                       << " 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));
+  UpdateLayerInfoVp8(frame);
+  return kHandOff;
 }
 
-void RtpFrameReferenceFinder::CompletedFrameVp8(
+void RtpFrameReferenceFinder::UpdateLayerInfoVp8(RtpFrameObject* frame) {
-    std::unique_ptr<RtpFrameObject> frame) {
   rtc::Optional<RTPVideoTypeHeader> rtp_codec_header = frame->GetCodecHeader();
-  if (!rtp_codec_header)
+  RTC_DCHECK(rtp_codec_header);
-    return;
-
   const RTPVideoHeaderVP8& codec_header = rtp_codec_header->VP8;
 
   uint8_t tl0_pic_idx = codec_header.tl0PicIdx;
@@ -378,31 +389,23 @@ void RtpFrameReferenceFinder::CompletedFrameVp8(
   }
   not_yet_received_frames_.erase(frame->picture_id);
 
-  for (size_t i = 0; i < frame->num_references; ++i)
+  UnwrapPictureIds(frame);
-    frame->references[i] = UnwrapPictureId(frame->references[i]);
-  frame->picture_id = UnwrapPictureId(frame->picture_id);
-
-  frame_callback_->OnCompleteFrame(std::move(frame));
-  RetryStashedFrames();
 }
 
-void RtpFrameReferenceFinder::ManageFrameVp9(
+RtpFrameReferenceFinder::FrameDecision RtpFrameReferenceFinder::ManageFrameVp9(
-    std::unique_ptr<RtpFrameObject> frame) {
+    RtpFrameObject* frame) {
   rtc::Optional<RTPVideoTypeHeader> rtp_codec_header = frame->GetCodecHeader();
-  if (!rtp_codec_header)
+  RTC_DCHECK(rtp_codec_header);
-    return;
-
   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 ManageFrameGeneric(std::move(frame), codec_header.picture_id);
-    return;
   }
 
   frame->spatial_layer = codec_header.spatial_idx;
@@ -422,8 +425,8 @@ void RtpFrameReferenceFinder::ManageFrameVp9(
           Subtract<1 << 16>(frame->picture_id, codec_header.pid_diff[i]);
     }
 
-    CompletedFrameVp9(std::move(frame));
+    UnwrapPictureIds(frame);
-    return;
+    return kHandOff;
   }
 
   if (codec_header.ss_data_available) {
@@ -455,8 +458,8 @@ void RtpFrameReferenceFinder::ManageFrameVp9(
     frame->num_references = 0;
     GofInfo info = gof_info_.find(codec_header.tl0_pic_idx)->second;
     FrameReceivedVp9(frame->picture_id, &info);
-    CompletedFrameVp9(std::move(frame));
+    UnwrapPictureIds(frame);
-    return;
+    return kHandOff;
   }
 
   auto gof_info_it = gof_info_.find(
@@ -465,20 +468,16 @@ void RtpFrameReferenceFinder::ManageFrameVp9(
           : codec_header.tl0_pic_idx);
 
   // Gof info for this frame is not available yet, stash this frame.
-  if (gof_info_it == gof_info_.end()) {
+  if (gof_info_it == gof_info_.end())
-    stashed_frames_.push_back(std::move(frame));
+    return kStash;
-    return;
-  }
 
   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)) {
+  if (MissingRequiredFrameVp9(frame->picture_id, *info))
-    stashed_frames_.push_back(std::move(frame));
+    return kStash;
-    return;
-  }
 
   if (codec_header.temporal_up_switch) {
     auto pid_tidx =
@@ -517,7 +516,8 @@ void RtpFrameReferenceFinder::ManageFrameVp9(
     }
   }
 
-  CompletedFrameVp9(std::move(frame));
+  UnwrapPictureIds(frame);
+  return kHandOff;
 }
 
 bool RtpFrameReferenceFinder::MissingRequiredFrameVp9(uint16_t picture_id,
@@ -589,14 +589,10 @@ bool RtpFrameReferenceFinder::UpSwitchInIntervalVp9(uint16_t picture_id,
   return false;
 }
 
-void RtpFrameReferenceFinder::CompletedFrameVp9(
+void RtpFrameReferenceFinder::UnwrapPictureIds(RtpFrameObject* frame) {
-    std::unique_ptr<RtpFrameObject> frame) {
   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) {

webrtc/modules/video_coding/rtp_frame_reference_finder.h

@@ -44,7 +44,7 @@ class RtpFrameReferenceFinder {
   // Manage this frame until:
   //  - We have all information needed to determine its references, after
   //    which |frame_callback_| is called with the completed frame, or
-  //  - We have too many stashed frames (determined by |kMaxStashedFrames)
+  //  - We have too many stashed frames (determined by |kMaxStashedFrames|)
   //    so we drop this frame, or
   //  - It gets cleared by ClearTo, which also means we drop it.
   void ManageFrame(std::unique_ptr<RtpFrameObject> frame);
@@ -65,6 +65,7 @@ class RtpFrameReferenceFinder {
   static const int kMaxGofSaved = 50;
   static const int kMaxPaddingAge = 100;
 
+  enum FrameDecision { kStash, kHandOff, kDrop };
 
   struct GofInfo {
     GofInfo(GofInfoVP9* gof, uint16_t last_picture_id)
@@ -80,33 +81,29 @@ class RtpFrameReferenceFinder {
   void UpdateLastPictureIdWithPadding(uint16_t seq_num)
       EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
-  // Retry finding references for all frames that previously didn't have
+  // Retry stashed frames until no more complete frames are found.
-  // all information needed.
   void RetryStashedFrames() EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
+  FrameDecision ManageFrameInternal(RtpFrameObject* frame)
+      EXCLUSIVE_LOCKS_REQUIRED(crit_);
+
   // Find references for generic frames. If |picture_id| is unspecified
   // then packet sequence numbers will be used to determine the references
   // of the frames.
-  void ManageFrameGeneric(std::unique_ptr<RtpFrameObject> frame,
+  FrameDecision ManageFrameGeneric(RtpFrameObject* frame, int picture_id)
-                          int picture_id) EXCLUSIVE_LOCKS_REQUIRED(crit_);
-
-  // Find references for Vp8 frames
-  void ManageFrameVp8(std::unique_ptr<RtpFrameObject> frame)
       EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
-  // Updates all necessary state used to determine frame references
+  // Find references for Vp8 frames
-  // for Vp8 and then calls the |frame_callback| callback with the
+  FrameDecision ManageFrameVp8(RtpFrameObject* frame)
-  // completed frame.
+      EXCLUSIVE_LOCKS_REQUIRED(crit_);
-  void CompletedFrameVp8(std::unique_ptr<RtpFrameObject> frame)
+
+  // Updates necessary layer info state used to determine frame references for
+  // Vp8.
+  void UpdateLayerInfoVp8(RtpFrameObject* frame)
       EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
   // Find references for Vp9 frames
-  void ManageFrameVp9(std::unique_ptr<RtpFrameObject> frame)
+  FrameDecision ManageFrameVp9(RtpFrameObject* frame)
-      EXCLUSIVE_LOCKS_REQUIRED(crit_);
-
-  // Unwrap the picture id and the frame references  and then call the
-  // |frame_callback| callback with the completed frame.
-  void CompletedFrameVp9(std::unique_ptr<RtpFrameObject> frame)
       EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
   // Check if we are missing a frame necessary to determine the references
@@ -126,6 +123,8 @@ class RtpFrameReferenceFinder {
                              uint8_t temporal_idx,
                              uint16_t pid_ref) EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
+  // Unwrap |frame|s picture id and its references to 16 bits.
+  void UnwrapPictureIds(RtpFrameObject* frame) EXCLUSIVE_LOCKS_REQUIRED(crit_);
   // All picture ids are unwrapped to 16 bits.
   uint16_t UnwrapPictureId(uint16_t picture_id) EXCLUSIVE_LOCKS_REQUIRED(crit_);