Revert "h264: bail out early when failing to parse SPS/PPS ids"
This reverts commit 4344eb713bb9a6d04d922d00fb492dfb31c9111f.
Reason for revert: Breaks downstream project.
Original change's description:
> 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}
Bug: webrtc:337076010
Change-Id: I15b815c69f1d25e41fb222d46359655242589fba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349661
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42217}
diff --git a/modules/rtp_rtcp/source/video_rtp_depacketizer_h264.cc b/modules/rtp_rtcp/source/video_rtp_depacketizer_h264.cc
index 4198c80..9978e5f 100644
--- a/modules/rtp_rtcp/source/video_rtp_depacketizer_h264.cc
+++ b/modules/rtp_rtcp/source/video_rtp_depacketizer_h264.cc
@@ -133,47 +133,45 @@
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.";
- }
- // 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);
- }
+ 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.";
+ }
- 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);
+ // 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);
+ }
- 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;
+ 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;
}
+
+ 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: {
@@ -187,7 +185,6 @@
} else {
RTC_LOG(LS_WARNING)
<< "Failed to parse PPS id and SPS id from PPS slice.";
- return absl::nullopt;
}
break;
}
@@ -203,7 +200,6 @@
} 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 7cc4108..f569c45 100644
--- a/modules/rtp_rtcp/source/video_rtp_depacketizer_h264_unittest.cc
+++ b/modules/rtp_rtcp/source/video_rtp_depacketizer_h264_unittest.cc
@@ -396,6 +396,12 @@
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.
@@ -415,37 +421,5 @@
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
