h264: bail out early when failing to parse SPS/PPS ids
This currently gets caught later in the process by the H264 SPS/PPS
tracker but can be rejected explicitly here. The network observable
behavior should be similar and request a key frame after a 200ms delay, at least for entities that send such bad bitstreams
BUG=webrtc:337076010
Change-Id: I239c64efa7db631460ef9e9986d283335303df5f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349060
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#42211}
diff --git a/modules/rtp_rtcp/source/video_rtp_depacketizer_h264.cc b/modules/rtp_rtcp/source/video_rtp_depacketizer_h264.cc
index 9978e5f..4198c80 100644
--- a/modules/rtp_rtcp/source/video_rtp_depacketizer_h264.cc
+++ b/modules/rtp_rtcp/source/video_rtp_depacketizer_h264.cc
@@ -133,45 +133,47 @@
SpsVuiRewriter::ParseResult result = SpsVuiRewriter::ParseAndRewriteSps(
&payload_data[start_offset], end_offset - start_offset, &sps,
nullptr, &output_buffer, SpsVuiRewriter::Direction::kIncoming);
+ switch (result) {
+ case SpsVuiRewriter::ParseResult::kFailure:
+ RTC_LOG(LS_WARNING) << "Failed to parse SPS NAL unit.";
+ return absl::nullopt;
+ case SpsVuiRewriter::ParseResult::kVuiRewritten:
+ if (modified_buffer) {
+ RTC_LOG(LS_WARNING)
+ << "More than one H264 SPS NAL units needing "
+ "rewriting found within a single STAP-A packet. "
+ "Keeping the first and rewriting the last.";
+ }
- if (result == SpsVuiRewriter::ParseResult::kVuiRewritten) {
- if (modified_buffer) {
- RTC_LOG(LS_WARNING)
- << "More than one H264 SPS NAL units needing "
- "rewriting found within a single STAP-A packet. "
- "Keeping the first and rewriting the last.";
- }
+ // Rewrite length field to new SPS size.
+ if (h264_header.packetization_type == kH264StapA) {
+ size_t length_field_offset =
+ start_offset - (H264::kNaluTypeSize + kLengthFieldSize);
+ // Stap-A Length includes payload data and type header.
+ size_t rewritten_size =
+ output_buffer.size() - start_offset + H264::kNaluTypeSize;
+ ByteWriter<uint16_t>::WriteBigEndian(
+ &output_buffer[length_field_offset], rewritten_size);
+ }
- // Rewrite length field to new SPS size.
- if (h264_header.packetization_type == kH264StapA) {
- size_t length_field_offset =
- start_offset - (H264::kNaluTypeSize + kLengthFieldSize);
- // Stap-A Length includes payload data and type header.
- size_t rewritten_size =
- output_buffer.size() - start_offset + H264::kNaluTypeSize;
- ByteWriter<uint16_t>::WriteBigEndian(
- &output_buffer[length_field_offset], rewritten_size);
- }
+ parsed_payload->video_payload.SetData(output_buffer.data(),
+ output_buffer.size());
+ // Append rest of packet.
+ parsed_payload->video_payload.AppendData(
+ &payload_data[end_offset],
+ nalu_length + kNalHeaderSize - end_offset);
- parsed_payload->video_payload.SetData(output_buffer.data(),
- output_buffer.size());
- // Append rest of packet.
- parsed_payload->video_payload.AppendData(
- &payload_data[end_offset],
- nalu_length + kNalHeaderSize - end_offset);
-
- modified_buffer = true;
+ modified_buffer = true;
+ [[fallthrough]];
+ case SpsVuiRewriter::ParseResult::kVuiOk:
+ RTC_DCHECK(sps);
+ nalu.sps_id = sps->id;
+ parsed_payload->video_header.width = sps->width;
+ parsed_payload->video_header.height = sps->height;
+ parsed_payload->video_header.frame_type =
+ VideoFrameType::kVideoFrameKey;
+ break;
}
-
- if (sps) {
- parsed_payload->video_header.width = sps->width;
- parsed_payload->video_header.height = sps->height;
- nalu.sps_id = sps->id;
- } else {
- RTC_LOG(LS_WARNING) << "Failed to parse SPS id from SPS slice.";
- }
- parsed_payload->video_header.frame_type =
- VideoFrameType::kVideoFrameKey;
break;
}
case H264::NaluType::kPps: {
@@ -185,6 +187,7 @@
} else {
RTC_LOG(LS_WARNING)
<< "Failed to parse PPS id and SPS id from PPS slice.";
+ return absl::nullopt;
}
break;
}
@@ -200,6 +203,7 @@
} else {
RTC_LOG(LS_WARNING) << "Failed to parse PPS id from slice of type: "
<< static_cast<int>(nalu.type);
+ return absl::nullopt;
}
break;
}
diff --git a/modules/rtp_rtcp/source/video_rtp_depacketizer_h264_unittest.cc b/modules/rtp_rtcp/source/video_rtp_depacketizer_h264_unittest.cc
index f569c45..7cc4108 100644
--- a/modules/rtp_rtcp/source/video_rtp_depacketizer_h264_unittest.cc
+++ b/modules/rtp_rtcp/source/video_rtp_depacketizer_h264_unittest.cc
@@ -396,12 +396,6 @@
EXPECT_FALSE(depacketizer.Parse(rtc::CopyOnWriteBuffer(kPayload)));
}
-TEST(VideoRtpDepacketizerH264Test, ShortSpsPacket) {
- const uint8_t kPayload[] = {0x27, 0x80, 0x00};
- VideoRtpDepacketizerH264 depacketizer;
- EXPECT_TRUE(depacketizer.Parse(rtc::CopyOnWriteBuffer(kPayload)));
-}
-
TEST(VideoRtpDepacketizerH264Test, SeiPacket) {
const uint8_t kPayload[] = {
kSei, // F=0, NRI=0, Type=6.
@@ -421,5 +415,37 @@
EXPECT_EQ(h264.nalus[0].pps_id, -1);
}
+TEST(VideoRtpDepacketizerH264Test, ShortSpsPacket) {
+ const uint8_t kPayload[] = {0x27, 0x80, 0x00};
+ VideoRtpDepacketizerH264 depacketizer;
+ EXPECT_FALSE(depacketizer.Parse(rtc::CopyOnWriteBuffer(kPayload)));
+}
+
+TEST(VideoRtpDepacketizerH264Test, BadSps) {
+ const uint8_t kPayload[] = {
+ kSps, 0x42, 0x41, 0x2a, 0xd3, 0x93, 0xd3, 0x3b // Payload.
+ };
+ VideoRtpDepacketizerH264 depacketizer;
+ EXPECT_FALSE(depacketizer.Parse(rtc::CopyOnWriteBuffer(kPayload)));
+}
+
+TEST(VideoRtpDepacketizerH264Test, BadPps) {
+ const uint8_t kPayload[] = {
+ kPps,
+ 0x00 // Payload.
+ };
+ VideoRtpDepacketizerH264 depacketizer;
+ EXPECT_FALSE(depacketizer.Parse(rtc::CopyOnWriteBuffer(kPayload)));
+}
+
+TEST(VideoRtpDepacketizerH264Test, BadSlice) {
+ const uint8_t kPayload[] = {
+ kIdr,
+ 0xc0 // Payload.
+ };
+ VideoRtpDepacketizerH264 depacketizer;
+ EXPECT_FALSE(depacketizer.Parse(rtc::CopyOnWriteBuffer(kPayload)));
+}
+
} // namespace
} // namespace webrtc
