Revert "fix: h26x packet buffer video artifacts"
In the case of H264 the fix to avoid incomplete keyframes being
assembled is to add `sps-pps-idr-is-keyframe` as an fmtp parameter
during negotiation.
This CL introduces a bug where SEI NALUs will block an actually complete
frame from being assembled as a SEI NALU is not a
`is_first_packet_in_frame`.
This reverts commit eafee5e3d68a7356dd415d2b3eee53017140d38b.
Bug: chromium:402547556, webrtc:384391181, chromium:402520966
Change-Id: Id3a3731b1d4171dff19ee4f467fe5c50e90bc7be
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/380860
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Jianlin Qiu <jianlin.qiu@intel.com>
Cr-Commit-Position: refs/heads/main@{#44108}
diff --git a/AUTHORS b/AUTHORS
index 4fb9455..7f2afd8 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -75,7 +75,6 @@
Jiwon Kim <jwkim0000@gmail.com>
Johnny Wong <hellojinqiang@gmail.com>
Jose Antonio Olivera Ortega <josea.olivera@gmail.com>
-Kacper Waśniowski <kacp.was@gmail.com>
Karim Hammache <karim@karhm.com>
Keiichi Enomoto <enm10k@gmail.com>
Kiran Thind <kiran.thind@gmail.com>
diff --git a/modules/video_coding/h26x_packet_buffer.cc b/modules/video_coding/h26x_packet_buffer.cc
index 8bc5d7e..2afe142 100644
--- a/modules/video_coding/h26x_packet_buffer.cc
+++ b/modules/video_coding/h26x_packet_buffer.cc
@@ -207,12 +207,6 @@
auto& prev_packet = GetPacket(seq_num_start - 1);
if (prev_packet == nullptr || prev_packet->timestamp != rtp_timestamp) {
- const auto& current_packet = GetPacket(seq_num_start);
- if (!current_packet->video_header.is_first_packet_in_frame) {
- // First packet of the frame is missing.
- return result;
- }
-
if (MaybeAssembleFrame(seq_num_start, seq_num, result)) {
// Frame was assembled, continue to look for more frames.
break;
diff --git a/modules/video_coding/h26x_packet_buffer.h b/modules/video_coding/h26x_packet_buffer.h
index d7f5286..c3db72b 100644
--- a/modules/video_coding/h26x_packet_buffer.h
+++ b/modules/video_coding/h26x_packet_buffer.h
@@ -85,7 +85,7 @@
// received without SPS/PPS.
void InsertSpsPpsNalus(const std::vector<uint8_t>& sps,
const std::vector<uint8_t>& pps);
- // Insert start code and parameter sets for H.264 payload, also update header
+ // Insert start code and paramter sets for H.264 payload, also update header
// if parameter sets are inserted. Return false if required SPS or PPS is not
// found.
bool FixH264Packet(Packet& packet);
diff --git a/modules/video_coding/h26x_packet_buffer_unittest.cc b/modules/video_coding/h26x_packet_buffer_unittest.cc
index dc5f3b5..a3ea3dc 100644
--- a/modules/video_coding/h26x_packet_buffer_unittest.cc
+++ b/modules/video_coding/h26x_packet_buffer_unittest.cc
@@ -547,40 +547,6 @@
SizeIs(2));
}
-TEST(H26xPacketBufferTest, ReorderedRtpPackets) {
- H26xPacketBuffer packet_buffer(/*h264_allow_idr_only_keyframes=*/true);
-
- RTC_UNUSED(packet_buffer.InsertPacket(H264Packet(kH264StapA)
- .Sps()
- .Pps()
- .SeqNum(1)
- .Time(0)
- .AsFirstPacket()
- .AsFirstFragment()
- .Build()));
-
- RTC_UNUSED(packet_buffer.InsertPacket(
- H264Packet(kH264FuA).Idr().SeqNum(2).Time(0).Build()));
-
- RTC_UNUSED(packet_buffer.InsertPacket(
- H264Packet(kH264FuA).Idr().SeqNum(3).Time(0).Build()));
-
- RTC_UNUSED(packet_buffer.InsertPacket(
- H264Packet(kH264FuA).Idr().SeqNum(5).Time(0).Build()));
-
- RTC_UNUSED(packet_buffer.InsertPacket(
- H264Packet(kH264FuA).Idr().AsFirstFragment().SeqNum(6).Time(0).Build()));
-
- RTC_UNUSED(packet_buffer.InsertPacket(
- H264Packet(kH264FuA).Idr().SeqNum(7).Time(0).Marker().Build()));
-
- EXPECT_THAT(
- packet_buffer
- .InsertPacket(H264Packet(kH264FuA).Idr().SeqNum(4).Time(0).Build())
- .packets,
- SizeIs(7));
-}
-
TEST(H26xPacketBufferTest, SpsPpsIdrIsKeyframeSingleNalus) {
H26xPacketBuffer packet_buffer(/*h264_allow_idr_only_keyframes=*/false);
@@ -967,7 +933,6 @@
.Idr()
.SeqNum(1)
.Time(1)
- .AsFirstPacket()
.Marker()
.Build());
@@ -976,7 +941,7 @@
EXPECT_TRUE(key.packets[0]->video_header.is_last_packet_in_frame);
RTC_UNUSED(packet_buffer.InsertPacket(
- H264Packet(kH264FuA).Slice().SeqNum(2).Time(2).AsFirstPacket().Build()));
+ H264Packet(kH264FuA).Slice().SeqNum(2).Time(2).Build()));
RTC_UNUSED(packet_buffer.InsertPacket(
H264Packet(kH264FuA).Slice().SeqNum(3).Time(2).Build()));
auto delta = packet_buffer.InsertPacket(
diff --git a/video/rtp_video_stream_receiver2_unittest.cc b/video/rtp_video_stream_receiver2_unittest.cc
index 74941da..6218bc9 100644
--- a/video/rtp_video_stream_receiver2_unittest.cc
+++ b/video/rtp_video_stream_receiver2_unittest.cc
@@ -1777,13 +1777,11 @@
rtp_packet.SetSequenceNumber(0);
rtp_packet.SetPayloadType(kH265PayloadType);
RTPVideoHeader video_header = GetDefaultH265VideoHeader();
- video_header.is_first_packet_in_frame = true;
mock_on_complete_frame_callback_.AppendExpectedBitstream(vps, sizeof(vps));
rtp_video_stream_receiver_->OnReceivedPayloadData(
rtc::CopyOnWriteBuffer(vps, sizeof(vps)), rtp_packet, video_header, 0);
rtp_packet.SetSequenceNumber(1);
- video_header.is_first_packet_in_frame = false;
mock_on_complete_frame_callback_.AppendExpectedBitstream(sps, sizeof(sps));
rtp_video_stream_receiver_->OnReceivedPayloadData(
rtc::CopyOnWriteBuffer(sps, sizeof(sps)), rtp_packet, video_header, 0);