Revert "Always unwrap VP9 TL0PicIdx forward if the frame is newer."

This reverts commit dbab1be. Reason for revert: Breaks VP9 media performance under heavy packet loss. Original change's description: > Always unwrap VP9 TL0PicIdx forward if the frame is newer. > > Bug: webrtc:12979 > Change-Id: Idcc14f8f61b04f9eb194b55ffa40fb95319a881c > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/226463 > Reviewed-by: Danil Chapovalov <[email protected]> > Reviewed-by: Ilya Nikolaevskiy <[email protected]> > Reviewed-by: Mirko Bonadei <[email protected]> > Commit-Queue: Philip Eliasson <[email protected]> > Cr-Commit-Position: refs/heads/master@{#34513} # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:12979 Change-Id: Id315db8d67143372724448b8801a86aee9a2f0aa Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/230422 Reviewed-by: Philip Eliasson <[email protected]> Reviewed-by: Danil Chapovalov <[email protected]> Reviewed-by: Ilya Nikolaevskiy <[email protected]> Commit-Queue: Philip Eliasson <[email protected]> Cr-Commit-Position: refs/heads/main@{#34863}
webrtc-sdk · Aug 30, 2021 · 0d17535 · 0d17535
1 parent e5b4e94
commit 0d17535
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 44 deletions.
diff --git a/modules/video_coding/rtp_vp9_ref_finder.cc b/modules/video_coding/rtp_vp9_ref_finder.cc
@@ -77,21 +77,8 @@ RtpVp9RefFinder::FrameDecision RtpVp9RefFinder::ManageFrameInternal(
   }
 
   GofInfo* info;
-
-  // The VP9 `tl0_pic_idx` is 8 bits and therefor wraps often. In the case of
-  // packet loss the next received frame could have a `tl0_pic_idx` that looks
-  // older than the previously received frame. Always wrap forward if `frame` is
-  // newer in RTP packet sequence number order.
-  int64_t unwrapped_tl0;
-  auto tl0_it = gof_info_.rbegin();
-  if (tl0_it != gof_info_.rend() &&
-      AheadOf(frame->last_seq_num(), tl0_it->second.last_seq_num)) {
-    unwrapped_tl0 =
-        tl0_unwrapper_.UnwrapForward(codec_header.tl0_pic_idx & 0xFF);
-  } else {
-    unwrapped_tl0 = tl0_unwrapper_.Unwrap(codec_header.tl0_pic_idx & 0xFF);
-  }
-
+  int64_t unwrapped_tl0 =
+      tl0_unwrapper_.Unwrap(codec_header.tl0_pic_idx & 0xFF);
   if (codec_header.ss_data_available) {
     if (codec_header.temporal_idx != 0) {
       RTC_LOG(LS_WARNING) << "Received scalability structure on a non base "
@@ -117,9 +104,9 @@ RtpVp9RefFinder::FrameDecision RtpVp9RefFinder::ManageFrameInternal(
       current_ss_idx_ = Add<kMaxGofSaved>(current_ss_idx_, 1);
       scalability_structures_[current_ss_idx_] = gof;
       scalability_structures_[current_ss_idx_].pid_start = frame->Id();
-      gof_info_.emplace(unwrapped_tl0,
-                        GofInfo(&scalability_structures_[current_ss_idx_],
-                                frame->Id(), frame->last_seq_num()));
+      gof_info_.emplace(
+          unwrapped_tl0,
+          GofInfo(&scalability_structures_[current_ss_idx_], frame->Id()));
     }
 
     const auto gof_info_it = gof_info_.find(unwrapped_tl0);
@@ -160,8 +147,7 @@ RtpVp9RefFinder::FrameDecision RtpVp9RefFinder::ManageFrameInternal(
     if (codec_header.temporal_idx == 0) {
       gof_info_it = gof_info_
                         .emplace(unwrapped_tl0,
-                                 GofInfo(gof_info_it->second.gof, frame->Id(),
-                                         frame->last_seq_num()))
+                                 GofInfo(gof_info_it->second.gof, frame->Id()))
                         .first;
     }
 

diff --git a/modules/video_coding/rtp_vp9_ref_finder.h b/modules/video_coding/rtp_vp9_ref_finder.h
@@ -42,13 +42,10 @@ class RtpVp9RefFinder {
   enum FrameDecision { kStash, kHandOff, kDrop };
 
   struct GofInfo {
-    GofInfo(GofInfoVP9* gof, uint16_t last_picture_id, uint16_t last_seq_num)
-        : gof(gof),
-          last_picture_id(last_picture_id),
-          last_seq_num(last_seq_num) {}
+    GofInfo(GofInfoVP9* gof, uint16_t last_picture_id)
+        : gof(gof), last_picture_id(last_picture_id) {}
     GofInfoVP9* gof;
     uint16_t last_picture_id;
-    uint16_t last_seq_num;
   };
 
   FrameDecision ManageFrameInternal(RtpFrameObject* frame);

diff --git a/modules/video_coding/rtp_vp9_ref_finder_unittest.cc b/modules/video_coding/rtp_vp9_ref_finder_unittest.cc
@@ -23,7 +23,6 @@ using ::testing::Matches;
 using ::testing::MatchResultListener;
 using ::testing::Pointee;
 using ::testing::Property;
-using ::testing::SizeIs;
 using ::testing::UnorderedElementsAreArray;
 
 namespace webrtc {
@@ -662,24 +661,6 @@ TEST_F(RtpVp9RefFinderTest, GofTl0Jump) {
   Insert(Frame().Pid(1).SidAndTid(0, 0).Tl0(0).Gof(&ss));
 }
 
-TEST_F(RtpVp9RefFinderTest, DontDiscardNewerFramesWithWrappedTl0) {
-  GofInfoVP9 ss;
-  ss.SetGofInfoVP9(kTemporalStructureMode1);
-
-  Insert(
-      Frame().Pid(0).SidAndTid(0, 0).Tl0(0).SeqNum(0, 0).AsKeyFrame().Gof(&ss));
-  // ... 254 frames are lost ...
-  Insert(Frame()
-             .Pid(255)
-             .SidAndTid(0, 0)
-             .Tl0(255)
-             .SeqNum(255, 255)
-             .AsKeyFrame()
-             .Gof(&ss));
-
-  EXPECT_THAT(frames_, SizeIs(2));
-}
-
 TEST_F(RtpVp9RefFinderTest, GofTidTooHigh) {
   const int kMaxTemporalLayers = 5;
   GofInfoVP9 ss;