Skip to content

Commit b83487f

Browse files
Philipel-WebRTCWebRTC LUCI CQ
authored andcommitted
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, chromium:1245564 > 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. (cherry picked from commit 0d17535) 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-Original-Commit-Position: refs/heads/main@{#34863} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231133 Cr-Commit-Position: refs/branch-heads/4606@{#3} Cr-Branched-From: 8b18304-refs/heads/master@{#34737}
1 parent 34837ac commit b83487f

File tree

3 files changed

+8
-44
lines changed

3 files changed

+8
-44
lines changed

modules/video_coding/rtp_vp9_ref_finder.cc

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -77,21 +77,8 @@ RtpVp9RefFinder::FrameDecision RtpVp9RefFinder::ManageFrameInternal(
7777
}
7878

7979
GofInfo* info;
80-
81-
// The VP9 `tl0_pic_idx` is 8 bits and therefor wraps often. In the case of
82-
// packet loss the next received frame could have a `tl0_pic_idx` that looks
83-
// older than the previously received frame. Always wrap forward if `frame` is
84-
// newer in RTP packet sequence number order.
85-
int64_t unwrapped_tl0;
86-
auto tl0_it = gof_info_.rbegin();
87-
if (tl0_it != gof_info_.rend() &&
88-
AheadOf(frame->last_seq_num(), tl0_it->second.last_seq_num)) {
89-
unwrapped_tl0 =
90-
tl0_unwrapper_.UnwrapForward(codec_header.tl0_pic_idx & 0xFF);
91-
} else {
92-
unwrapped_tl0 = tl0_unwrapper_.Unwrap(codec_header.tl0_pic_idx & 0xFF);
93-
}
94-
80+
int64_t unwrapped_tl0 =
81+
tl0_unwrapper_.Unwrap(codec_header.tl0_pic_idx & 0xFF);
9582
if (codec_header.ss_data_available) {
9683
if (codec_header.temporal_idx != 0) {
9784
RTC_LOG(LS_WARNING) << "Received scalability structure on a non base "
@@ -117,9 +104,9 @@ RtpVp9RefFinder::FrameDecision RtpVp9RefFinder::ManageFrameInternal(
117104
current_ss_idx_ = Add<kMaxGofSaved>(current_ss_idx_, 1);
118105
scalability_structures_[current_ss_idx_] = gof;
119106
scalability_structures_[current_ss_idx_].pid_start = frame->Id();
120-
gof_info_.emplace(unwrapped_tl0,
121-
GofInfo(&scalability_structures_[current_ss_idx_],
122-
frame->Id(), frame->last_seq_num()));
107+
gof_info_.emplace(
108+
unwrapped_tl0,
109+
GofInfo(&scalability_structures_[current_ss_idx_], frame->Id()));
123110
}
124111

125112
const auto gof_info_it = gof_info_.find(unwrapped_tl0);
@@ -160,8 +147,7 @@ RtpVp9RefFinder::FrameDecision RtpVp9RefFinder::ManageFrameInternal(
160147
if (codec_header.temporal_idx == 0) {
161148
gof_info_it = gof_info_
162149
.emplace(unwrapped_tl0,
163-
GofInfo(gof_info_it->second.gof, frame->Id(),
164-
frame->last_seq_num()))
150+
GofInfo(gof_info_it->second.gof, frame->Id()))
165151
.first;
166152
}
167153

modules/video_coding/rtp_vp9_ref_finder.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,10 @@ class RtpVp9RefFinder {
4242
enum FrameDecision { kStash, kHandOff, kDrop };
4343

4444
struct GofInfo {
45-
GofInfo(GofInfoVP9* gof, uint16_t last_picture_id, uint16_t last_seq_num)
46-
: gof(gof),
47-
last_picture_id(last_picture_id),
48-
last_seq_num(last_seq_num) {}
45+
GofInfo(GofInfoVP9* gof, uint16_t last_picture_id)
46+
: gof(gof), last_picture_id(last_picture_id) {}
4947
GofInfoVP9* gof;
5048
uint16_t last_picture_id;
51-
uint16_t last_seq_num;
5249
};
5350

5451
FrameDecision ManageFrameInternal(RtpFrameObject* frame);

modules/video_coding/rtp_vp9_ref_finder_unittest.cc

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ using ::testing::Matches;
2323
using ::testing::MatchResultListener;
2424
using ::testing::Pointee;
2525
using ::testing::Property;
26-
using ::testing::SizeIs;
2726
using ::testing::UnorderedElementsAreArray;
2827

2928
namespace webrtc {
@@ -662,24 +661,6 @@ TEST_F(RtpVp9RefFinderTest, GofTl0Jump) {
662661
Insert(Frame().Pid(1).SidAndTid(0, 0).Tl0(0).Gof(&ss));
663662
}
664663

665-
TEST_F(RtpVp9RefFinderTest, DontDiscardNewerFramesWithWrappedTl0) {
666-
GofInfoVP9 ss;
667-
ss.SetGofInfoVP9(kTemporalStructureMode1);
668-
669-
Insert(
670-
Frame().Pid(0).SidAndTid(0, 0).Tl0(0).SeqNum(0, 0).AsKeyFrame().Gof(&ss));
671-
// ... 254 frames are lost ...
672-
Insert(Frame()
673-
.Pid(255)
674-
.SidAndTid(0, 0)
675-
.Tl0(255)
676-
.SeqNum(255, 255)
677-
.AsKeyFrame()
678-
.Gof(&ss));
679-
680-
EXPECT_THAT(frames_, SizeIs(2));
681-
}
682-
683664
TEST_F(RtpVp9RefFinderTest, GofTidTooHigh) {
684665
const int kMaxTemporalLayers = 5;
685666
GofInfoVP9 ss;

0 commit comments

Comments
 (0)