fix: h26x packet buffer video artifacts

This change resolves an issue that arises when there is a gap in the
sequence numbers of packets associated with a single frame.

Before this change, the H26x packet buffer could potentially assemble a
frame using only a subset of the packets in the buffer if a packet was
missing in the middle and a packet with a marker bit arrived.

To address this, the change introduces a check before assembling a
frame. This ensures that all packets belonging to a single frame are
correctly collected by iterating backward until the first packet in the
frame is identified.

Bug: webrtc:384391181
Change-Id: I4d09a3d6d569624ece204264cb32e5076ed090a2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374183
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Jianlin Qiu <jianlin.qiu@intel.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43793}

AUTHORS

@@ -74,6 +74,7 @@ Jie Mao <maojie0924@gmail.com>
 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>

modules/video_coding/h26x_packet_buffer.cc

@@ -207,6 +207,12 @@ H26xPacketBuffer::InsertResult H26xPacketBuffer::FindFrames(
         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;

modules/video_coding/h26x_packet_buffer.h

@@ -85,7 +85,7 @@ class H26xPacketBuffer {
   // received without SPS/PPS.
   void InsertSpsPpsNalus(const std::vector<uint8_t>& sps,
                          const std::vector<uint8_t>& pps);
-  // Insert start code and paramter sets for H.264 payload, also update header
+  // Insert start code and parameter 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);

modules/video_coding/h26x_packet_buffer_unittest.cc

@@ -547,6 +547,40 @@ TEST(H26xPacketBufferTest, IdrIsKeyframeFuaRequiresFirstFragmet) {
       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);
 
@@ -933,6 +967,7 @@ TEST(H26xPacketBufferTest, FrameBoundariesAreSet) {
                                             .Idr()
                                             .SeqNum(1)
                                             .Time(1)
+                                            .AsFirstPacket()
                                             .Marker()
                                             .Build());
 
@@ -941,7 +976,7 @@ TEST(H26xPacketBufferTest, FrameBoundariesAreSet) {
   EXPECT_TRUE(key.packets[0]->video_header.is_last_packet_in_frame);
 
   RTC_UNUSED(packet_buffer.InsertPacket(
-      H264Packet(kH264FuA).Slice().SeqNum(2).Time(2).Build()));
+      H264Packet(kH264FuA).Slice().SeqNum(2).Time(2).AsFirstPacket().Build()));
   RTC_UNUSED(packet_buffer.InsertPacket(
       H264Packet(kH264FuA).Slice().SeqNum(3).Time(2).Build()));
   auto delta = packet_buffer.InsertPacket(

video/rtp_video_stream_receiver2_unittest.cc

@@ -1726,11 +1726,13 @@ TEST_F(RtpVideoStreamReceiver2TestH265, H265Bitstream) {
   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);