Revert the deletion of WebRTC-VideoH26xPacketBuffer flag.
This unlaunches the experiment for H264 due to issues reported for H264
where the old packet buffer behaves better than the new one. A recently
introduced issue will be fixed separately from this CL and rollout
restarted.
This CL is not a clean revert though (see delta with PS#1):
we will continue to use the H26xPacketBuffer for the new codec H265
because it does not work with the old packet buffer anyway.
Bug: webrtc:41480904
Change-Id: Icac49fa70f1c78d1ed596a7838d1417f6588a8b4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/380861
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Jianlin Qiu <jianlin.qiu@intel.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44104}
diff --git a/experiments/field_trials.py b/experiments/field_trials.py
index d9e46f3..df107e0 100755
--- a/experiments/field_trials.py
+++ b/experiments/field_trials.py
@@ -182,6 +182,9 @@
FieldTrial('WebRTC-ZeroHertzQueueOverload',
42225879,
date(2024, 7, 1)),
+ FieldTrial('WebRTC-Video-H26xPacketBuffer',
+ 41480904,
+ date(2024, 6, 1)),
FieldTrial('WebRTC-Video-Vp9FlexibleMode',
329396373,
date(2025, 6, 26)),
diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc
index b23b04a..0474393 100644
--- a/video/rtp_video_stream_receiver2.cc
+++ b/video/rtp_video_stream_receiver2.cc
@@ -738,7 +738,7 @@
if (codec_payload.size() == 0) {
NotifyReceiverOfEmptyPacket(packet->seq_num(),
- IsH26xPayloadType(packet->payload_type));
+ GetCodecFromPayloadType(packet->payload_type));
rtcp_feedback_buffer_.SendBufferedRtcpFeedback();
return false;
}
@@ -753,7 +753,8 @@
}
}
- if (packet->codec() == kVideoCodecH264 && !h26x_packet_buffer_) {
+ if (packet->codec() == kVideoCodecH264 &&
+ !UseH26xPacketBuffer(packet->codec())) {
video_coding::H264SpsPpsTracker::FixedBitstream fixed =
tracker_.CopyAndFixBitstream(
rtc::MakeArrayView(codec_payload.cdata(), codec_payload.size()),
@@ -778,9 +779,7 @@
rtcp_feedback_buffer_.SendBufferedRtcpFeedback();
frame_counter_.Add(packet->timestamp);
- if ((packet->codec() == kVideoCodecH264 ||
- packet->codec() == kVideoCodecH265) &&
- h26x_packet_buffer_) {
+ if (h26x_packet_buffer_ && UseH26xPacketBuffer(packet->codec())) {
OnInsertedPacket(h26x_packet_buffer_->InsertPacket(std::move(packet)));
} else {
OnInsertedPacket(packet_buffer_.InsertPacket(std::move(packet)));
@@ -1170,13 +1169,26 @@
return rtp_rtcp_->GetSenderReportStats();
}
-bool RtpVideoStreamReceiver2::IsH26xPayloadType(uint8_t payload_type) const {
+std::optional<VideoCodecType> RtpVideoStreamReceiver2::GetCodecFromPayloadType(
+ uint8_t payload_type) const {
RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
auto it = pt_codec_.find(payload_type);
if (it == pt_codec_.end()) {
- return false;
+ return std::nullopt;
}
- return it->second == kVideoCodecH264 || it->second == kVideoCodecH265;
+ return it->second;
+}
+
+bool RtpVideoStreamReceiver2::UseH26xPacketBuffer(
+ std::optional<VideoCodecType> codec) const {
+ RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+ if (codec == kVideoCodecH265) {
+ return true;
+ }
+ if (codec == kVideoCodecH264) {
+ return env_.field_trials().IsEnabled("WebRTC-Video-H26xPacketBuffer");
+ }
+ return false;
}
void RtpVideoStreamReceiver2::ManageFrame(
@@ -1193,7 +1205,7 @@
// TODO(nisse): Could drop empty packets earlier, but need to figure out how
// they should be counted in stats.
NotifyReceiverOfEmptyPacket(packet.SequenceNumber(),
- IsH26xPayloadType(packet.PayloadType()));
+ GetCodecFromPayloadType(packet.PayloadType()));
return;
}
if (packet.PayloadType() == red_payload_type_) {
@@ -1264,7 +1276,7 @@
// Notify video_receiver about received FEC packets to avoid NACKing these
// packets.
NotifyReceiverOfEmptyPacket(packet.SequenceNumber(),
- IsH26xPayloadType(packet.PayloadType()));
+ GetCodecFromPayloadType(packet.PayloadType()));
}
if (ulpfec_receiver_->AddReceivedRedPacket(packet)) {
ulpfec_receiver_->ProcessReceivedFec();
@@ -1274,14 +1286,15 @@
// In the case of a video stream without picture ids and no rtx the
// RtpFrameReferenceFinder will need to know about padding to
// correctly calculate frame references.
-void RtpVideoStreamReceiver2::NotifyReceiverOfEmptyPacket(uint16_t seq_num,
- bool is_h26x) {
+void RtpVideoStreamReceiver2::NotifyReceiverOfEmptyPacket(
+ uint16_t seq_num,
+ std::optional<VideoCodecType> codec) {
RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
RTC_DCHECK_RUN_ON(&worker_task_checker_);
OnCompleteFrames(reference_finder_->PaddingReceived(seq_num));
- if (is_h26x && h26x_packet_buffer_) {
+ if (h26x_packet_buffer_ && UseH26xPacketBuffer(codec)) {
OnInsertedPacket(h26x_packet_buffer_->InsertPadding(seq_num));
} else {
OnInsertedPacket(packet_buffer_.InsertPadding(seq_num));
@@ -1428,7 +1441,8 @@
tracker_.InsertSpsPpsNalus(sprop_decoder.sps_nalu(),
sprop_decoder.pps_nalu());
- if (h26x_packet_buffer_) {
+ if (h26x_packet_buffer_ &&
+ UseH26xPacketBuffer(GetCodecFromPayloadType(payload_type))) {
h26x_packet_buffer_->SetSpropParameterSets(sprop_base64_it->second);
}
}
diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h
index b514d8f..8cfabff 100644
--- a/video/rtp_video_stream_receiver2.h
+++ b/video/rtp_video_stream_receiver2.h
@@ -300,7 +300,8 @@
// This function assumes that it's being called from only one thread.
void ParseAndHandleEncapsulatingHeader(const RtpPacketReceived& packet)
RTC_RUN_ON(packet_sequence_checker_);
- void NotifyReceiverOfEmptyPacket(uint16_t seq_num, bool is_h26x)
+ void NotifyReceiverOfEmptyPacket(uint16_t seq_num,
+ std::optional<VideoCodecType> codec)
RTC_RUN_ON(packet_sequence_checker_);
bool IsRedEnabled() const;
void InsertSpsPpsIntoTracker(uint8_t payload_type)
@@ -320,7 +321,9 @@
FrameInstrumentationData>& frame_instrumentation_data,
int spatial_idx);
- bool IsH26xPayloadType(uint8_t payload_type) const
+ std::optional<VideoCodecType> GetCodecFromPayloadType(
+ uint8_t payload_type) const RTC_RUN_ON(packet_sequence_checker_);
+ bool UseH26xPacketBuffer(std::optional<VideoCodecType> codec) const
RTC_RUN_ON(packet_sequence_checker_);
const Environment env_;
@@ -371,7 +374,9 @@
video_coding::PacketBuffer packet_buffer_
RTC_GUARDED_BY(packet_sequence_checker_);
- // `h26x_packet_buffer_` is null if codec list doens't contain H.264 or H.265.
+ // h26x_packet_buffer_ is applicable to H.264 and H.265. For H.265 it is
+ // always used but for H.264 it is only used if WebRTC-Video-H26xPacketBuffer
+ // is enabled, see condition inside UseH26xPacketBuffer().
std::unique_ptr<H26xPacketBuffer> h26x_packet_buffer_
RTC_GUARDED_BY(packet_sequence_checker_);
UniqueTimestampCounter frame_counter_
diff --git a/video/rtp_video_stream_receiver2_unittest.cc b/video/rtp_video_stream_receiver2_unittest.cc
index 4e635b0..74941da 100644
--- a/video/rtp_video_stream_receiver2_unittest.cc
+++ b/video/rtp_video_stream_receiver2_unittest.cc
@@ -765,7 +765,11 @@
INSTANTIATE_TEST_SUITE_P(SpsPpsIdrIsKeyframeAndH26xPacketBuffer,
RtpVideoStreamReceiver2TestH264,
- Values("", "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/"));
+ Values("",
+ "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/",
+ "WebRTC-Video-H26xPacketBuffer/Enabled/",
+ "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/"
+ "WebRTC-Video-H26xPacketBuffer/Enabled/"));
TEST_P(RtpVideoStreamReceiver2TestH264, InBandSpsPps) {
constexpr int kH264PayloadType = 98;
@@ -865,7 +869,8 @@
// IDR frames without SPS/PPS are not returned by
// |H26xPacketBuffer.InsertPacket| until SPS and PPS are received when
// WebRTC-SpsPpsIdrIsH264Keyframe is enabled.
- if (!env_.field_trials().IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe")) {
+ if (!env_.field_trials().IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe") ||
+ !env_.field_trials().IsEnabled("WebRTC-Video-H26xPacketBuffer")) {
EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame(_));
}
rtp_video_stream_receiver_->OnReceivedPayloadData(data, rtp_packet,
@@ -939,17 +944,30 @@
// IDR frames without SPS/PPS are not returned by
// |H26xPacketBuffer.InsertPacket| until SPS and PPS are received, while
// |PacketBuffer| returns it as a delta frame.
- EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame).Times(0);
+ if (env_.field_trials().IsEnabled("WebRTC-Video-H26xPacketBuffer")) {
+ EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame).Times(0);
+ } else {
+ EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame)
+ .WillOnce(
+ [&](EncodedFrame* frame) { EXPECT_FALSE(frame->is_keyframe()); });
+ }
rtp_video_stream_receiver_->OnReceivedPayloadData(idr_data, rtp_packet,
idr_video_header, 0);
}
-class RtpVideoStreamReceiver2TestPadding : public RtpVideoStreamReceiver2Test {
+class RtpVideoStreamReceiver2TestPadding
+ : public RtpVideoStreamReceiver2Test,
+ public ::testing::WithParamInterface<std::string> {
protected:
- RtpVideoStreamReceiver2TestPadding() : RtpVideoStreamReceiver2Test("") {}
+ RtpVideoStreamReceiver2TestPadding()
+ : RtpVideoStreamReceiver2Test(GetParam()) {}
};
-TEST_F(RtpVideoStreamReceiver2TestPadding, PaddingInMediaStream) {
+INSTANTIATE_TEST_SUITE_P(PaddingInMediaStreamAndH26xPacketBuffer,
+ RtpVideoStreamReceiver2TestPadding,
+ Values("", "WebRTC-Video-H26xPacketBuffer/Enabled/"));
+
+TEST_P(RtpVideoStreamReceiver2TestPadding, PaddingInMediaStream) {
RtpPacketReceived rtp_packet;
RTPVideoHeader video_header = GetDefaultH264VideoHeader();
rtc::CopyOnWriteBuffer data({'1', '2', '3'});
@@ -986,7 +1004,7 @@
video_header, 0);
}
-TEST_F(RtpVideoStreamReceiver2TestPadding, EmptyPaddingInMediaStream) {
+TEST_P(RtpVideoStreamReceiver2TestPadding, EmptyPaddingInMediaStream) {
constexpr int kH264PayloadType = 98;
RtpPacketReceived rtp_packet_idr, rtp_packet_padding, rtp_packet_slice;
// Example Stap-A packet with SPS, PPS, and IDR.
