Save unwrapped tl0_pic_idx for inserted VP8 frames.

As stashed frames are retried their `tl0_pic_idx` are again unwrapped which can lead to the `tl0_unwrapper_` to unwrap the `tl0_pic_idx` of newer frames backwards. Instead unwrap the `tl0_pid_idx` only once and save it with the frame if necessary.

Related VP9 CL: https://webrtc-review.googlesource.com/c/src/+/253844

Bug: none
Change-Id: I8265dc5f36ee257db92d79cec719f56b165d3855
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256966
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36356}

modules/video_coding/rtp_vp8_ref_finder.cc

@@ -18,14 +18,20 @@ namespace webrtc {
 
 RtpFrameReferenceFinder::ReturnVector RtpVp8RefFinder::ManageFrame(
     std::unique_ptr<RtpFrameObject> frame) {
-  FrameDecision decision = ManageFrameInternal(frame.get());
+  const RTPVideoHeaderVP8& codec_header = absl::get<RTPVideoHeaderVP8>(
+      frame->GetRtpVideoHeader().video_type_header);
+  int64_t unwrapped_tl0 = tl0_unwrapper_.Unwrap(codec_header.tl0PicIdx & 0xFF);
+  FrameDecision decision =
+      ManageFrameInternal(frame.get(), codec_header, unwrapped_tl0);
 
   RtpFrameReferenceFinder::ReturnVector res;
   switch (decision) {
     case kStash:
-      if (stashed_frames_.size() > kMaxStashedFrames)
+      if (stashed_frames_.size() > kMaxStashedFrames) {
         stashed_frames_.pop_back();
-      stashed_frames_.push_front(std::move(frame));
+      }
+      stashed_frames_.push_front(
+          {.unwrapped_tl0 = unwrapped_tl0, .frame = std::move(frame)});
       return res;
     case kHandOff:
       res.push_back(std::move(frame));
@@ -39,11 +45,9 @@ RtpFrameReferenceFinder::ReturnVector RtpVp8RefFinder::ManageFrame(
 }
 
 RtpVp8RefFinder::FrameDecision RtpVp8RefFinder::ManageFrameInternal(
-    RtpFrameObject* frame) {
-  const RTPVideoHeader& video_header = frame->GetRtpVideoHeader();
-  const RTPVideoHeaderVP8& codec_header =
-      absl::get<RTPVideoHeaderVP8>(video_header.video_type_header);
-
+    RtpFrameObject* frame,
+    const RTPVideoHeaderVP8& codec_header,
+    int64_t unwrapped_tl0) {
   // Protect against corrupted packets with arbitrary large temporal idx.
   if (codec_header.temporalIdx >= kMaxTemporalLayers)
     return kDrop;
@@ -73,8 +77,6 @@ RtpVp8RefFinder::FrameDecision RtpVp8RefFinder::ManageFrameInternal(
     } while (last_picture_id_ != frame->Id());
   }
 
-  int64_t unwrapped_tl0 = tl0_unwrapper_.Unwrap(codec_header.tl0PicIdx & 0xFF);
-
   // Clean up info for base layers that are too old.
   int64_t old_tl0_pic_idx = unwrapped_tl0 - kMaxLayerInfo;
   auto clean_layer_info_to = layer_info_.lower_bound(old_tl0_pic_idx);
@@ -207,20 +209,22 @@ void RtpVp8RefFinder::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());
+    for (auto it = stashed_frames_.begin(); it != stashed_frames_.end();) {
+      const RTPVideoHeaderVP8& codec_header = absl::get<RTPVideoHeaderVP8>(
+          it->frame->GetRtpVideoHeader().video_type_header);
+      FrameDecision decision =
+          ManageFrameInternal(it->frame.get(), codec_header, it->unwrapped_tl0);
 
       switch (decision) {
         case kStash:
-          ++frame_it;
+          ++it;
           break;
         case kHandOff:
           complete_frame = true;
-          res.push_back(std::move(*frame_it));
+          res.push_back(std::move(it->frame));
           [[fallthrough]];
         case kDrop:
-          frame_it = stashed_frames_.erase(frame_it);
+          it = stashed_frames_.erase(it);
       }
     }
   } while (complete_frame);
@@ -235,7 +239,7 @@ void RtpVp8RefFinder::UnwrapPictureIds(RtpFrameObject* frame) {
 void RtpVp8RefFinder::ClearTo(uint16_t seq_num) {
   auto it = stashed_frames_.begin();
   while (it != stashed_frames_.end()) {
-    if (AheadOf<uint16_t>(seq_num, (*it)->first_seq_num())) {
+    if (AheadOf<uint16_t>(seq_num, it->frame->first_seq_num())) {
       it = stashed_frames_.erase(it);
     } else {
       ++it;

modules/video_coding/rtp_vp8_ref_finder.h

@@ -38,9 +38,16 @@ class RtpVp8RefFinder {
   static constexpr int kMaxStashedFrames = 100;
   static constexpr int kMaxTemporalLayers = 5;
 
+  struct UnwrappedTl0Frame {
+    int64_t unwrapped_tl0;
+    std::unique_ptr<RtpFrameObject> frame;
+  };
+
   enum FrameDecision { kStash, kHandOff, kDrop };
 
-  FrameDecision ManageFrameInternal(RtpFrameObject* frame);
+  FrameDecision ManageFrameInternal(RtpFrameObject* frame,
+                                    const RTPVideoHeaderVP8& codec_header,
+                                    int64_t unwrapped_tl0);
   void RetryStashedFrames(RtpFrameReferenceFinder::ReturnVector& res);
   void UpdateLayerInfoVp8(RtpFrameObject* frame,
                           int64_t unwrapped_tl0,
@@ -58,7 +65,7 @@ class RtpVp8RefFinder {
 
   // Frames that have been fully received but didn't have all the information
   // needed to determine their references.
-  std::deque<std::unique_ptr<RtpFrameObject>> stashed_frames_;
+  std::deque<UnwrappedTl0Frame> stashed_frames_;
 
   // Holds the information about the last completed frame for a given temporal
   // layer given an unwrapped Tl0 picture index.

modules/video_coding/rtp_vp8_ref_finder_unittest.cc

@@ -357,4 +357,14 @@ TEST_F(RtpVp8RefFinderTest, Vp8DetectMissingFrame_0212) {
   EXPECT_THAT(frames_, HasFrameWithIdAndRefs(8, {5, 6, 7}));
 }
 
+TEST_F(RtpVp8RefFinderTest, StashedFramesDoNotWrapTl0Backwards) {
+  Insert(Frame().Pid(0).Tid(0).Tl0(0));
+  EXPECT_THAT(frames_, SizeIs(0));
+
+  Insert(Frame().Pid(128).Tid(0).Tl0(128).AsKeyFrame());
+  EXPECT_THAT(frames_, SizeIs(1));
+  Insert(Frame().Pid(129).Tid(0).Tl0(129));
+  EXPECT_THAT(frames_, SizeIs(2));
+}
+
 }  // namespace webrtc