Revert "Always unwrap VP9 TL0PicIdx forward if the frame is newer."
This reverts commit dbab1be1d13060666b303209eded45c55cb46856.
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 <danilchap@webrtc.org>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Philip Eliasson <philipel@webrtc.org>
> 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 <philipel@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#34863}
diff --git a/modules/video_coding/rtp_vp9_ref_finder.cc b/modules/video_coding/rtp_vp9_ref_finder.cc
index 01e6b45..cab5792 100644
--- a/modules/video_coding/rtp_vp9_ref_finder.cc
+++ b/modules/video_coding/rtp_vp9_ref_finder.cc
@@ -77,21 +77,8 @@
}
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 @@
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 @@
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
index 145cbce..436cb1c 100644
--- a/modules/video_coding/rtp_vp9_ref_finder.h
+++ b/modules/video_coding/rtp_vp9_ref_finder.h
@@ -42,13 +42,10 @@
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
index 9fe24c4..6de7ce1 100644
--- 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::MatchResultListener;
using ::testing::Pointee;
using ::testing::Property;
-using ::testing::SizeIs;
using ::testing::UnorderedElementsAreArray;
namespace webrtc {
@@ -662,24 +661,6 @@
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;
