From f288f5b2d41d6810428f6ec9bc646ace8a9edeca Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Fri, 18 Sep 2020 22:35:59 -0700 Subject: [PATCH] Fix bug with the sps-pps-idr-in-keyframe fmtp parameter. RtpVideoStreamReceiver was forked to RtpVideoStreamReceiver2 recently, so the code that checks for this parameter needs to be present in the forked location, but it wasn't. This also enables RtpVideoStreamReceiver2TestH264.InBandSpsPps test on MSAN, which was another already fixed bug that wasn't ported over to the recently forked RtpVideoStreamReceiver2. See webrtc:11595 for information about the fork. See webrtc:11769 for information about this fmtp parameter. See webrtc:11376 for the original MSAN issue. Bug: webrtc:11957, webrtc:11595, webrtc:11769, webrtc:8423 Change-Id: I3734d077b2883c2f747ad35a0189b83c1915c3ef Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/184524 Commit-Queue: Ilya Nikolaevskiy Reviewed-by: Artem Titov Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#32144} --- AUTHORS | 1 + video/rtp_video_stream_receiver2.cc | 4 + video/rtp_video_stream_receiver2_unittest.cc | 83 +++++++++++++++++--- 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/AUTHORS b/AUTHORS index 5be2ff647a..c37a03a307 100644 --- a/AUTHORS +++ b/AUTHORS @@ -18,6 +18,7 @@ Alexandre Gouaillard Alex Henrie Andrew MacDonald Andrey Efremov +Andrew Johnson Anil Kumar Ben Strong Bob Withers diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index 1a5316517d..f3345597ea 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -331,6 +331,10 @@ void RtpVideoStreamReceiver2::AddReceiveCodec( const std::map& codec_params, bool raw_payload) { RTC_DCHECK_RUN_ON(&worker_task_checker_); + if (codec_params.count(cricket::kH264FmtpSpsPpsIdrInKeyframe) || + field_trial::IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe")) { + packet_buffer_.ForceSpsPpsIdrIsH264Keyframe(); + } payload_type_map_.emplace( payload_type, raw_payload ? std::make_unique() diff --git a/video/rtp_video_stream_receiver2_unittest.cc b/video/rtp_video_stream_receiver2_unittest.cc index 68aafa5915..7d690636d9 100644 --- a/video/rtp_video_stream_receiver2_unittest.cc +++ b/video/rtp_video_stream_receiver2_unittest.cc @@ -201,7 +201,7 @@ class RtpVideoStreamReceiver2Test : public ::testing::Test { info.type = H264::NaluType::kSps; info.sps_id = sps_id; info.pps_id = -1; - data->AppendData({H264::NaluType::kSps, sps_id}); + data->AppendData({H264::NaluType::kSps, sps_id}); auto& h264 = absl::get(video_header->video_type_header); h264.nalus[h264.nalus_length++] = info; } @@ -214,7 +214,7 @@ class RtpVideoStreamReceiver2Test : public ::testing::Test { info.type = H264::NaluType::kPps; info.sps_id = sps_id; info.pps_id = pps_id; - data->AppendData({H264::NaluType::kPps, pps_id}); + data->AppendData({H264::NaluType::kPps, pps_id}); auto& h264 = absl::get(video_header->video_type_header); h264.nalus[h264.nalus_length++] = info; } @@ -523,13 +523,7 @@ INSTANTIATE_TEST_SUITE_P(SpsPpsIdrIsKeyframe, RtpVideoStreamReceiver2TestH264, Values("", "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/")); -// Fails on MSAN: https://bugs.chromium.org/p/webrtc/issues/detail?id=11376. -#if defined(MEMORY_SANITIZER) -#define MAYBE_InBandSpsPps DISABLED_InBandSpsPps -#else -#define MAYBE_InBandSpsPps InBandSpsPps -#endif -TEST_P(RtpVideoStreamReceiver2TestH264, MAYBE_InBandSpsPps) { +TEST_P(RtpVideoStreamReceiver2TestH264, InBandSpsPps) { rtc::CopyOnWriteBuffer sps_data; RtpPacketReceived rtp_packet; RTPVideoHeader sps_video_header = GetDefaultH264VideoHeader(); @@ -617,6 +611,77 @@ TEST_P(RtpVideoStreamReceiver2TestH264, OutOfBandFmtpSpsPps) { video_header); } +TEST_P(RtpVideoStreamReceiver2TestH264, ForceSpsPpsIdrIsKeyframe) { + constexpr int kPayloadType = 99; + VideoCodec codec; + std::map codec_params; + if (GetParam() == + "") { // Forcing can be done either with field trial or codec_params. + codec_params.insert({cricket::kH264FmtpSpsPpsIdrInKeyframe, ""}); + } + rtp_video_stream_receiver_->AddReceiveCodec(kPayloadType, codec, codec_params, + /*raw_payload=*/false); + rtc::CopyOnWriteBuffer sps_data; + RtpPacketReceived rtp_packet; + RTPVideoHeader sps_video_header = GetDefaultH264VideoHeader(); + AddSps(&sps_video_header, 0, &sps_data); + rtp_packet.SetSequenceNumber(0); + rtp_packet.SetPayloadType(kPayloadType); + sps_video_header.is_first_packet_in_frame = true; + sps_video_header.frame_type = VideoFrameType::kEmptyFrame; + mock_on_complete_frame_callback_.AppendExpectedBitstream( + kH264StartCode, sizeof(kH264StartCode)); + mock_on_complete_frame_callback_.AppendExpectedBitstream(sps_data.data(), + sps_data.size()); + rtp_video_stream_receiver_->OnReceivedPayloadData(sps_data, rtp_packet, + sps_video_header); + + rtc::CopyOnWriteBuffer pps_data; + RTPVideoHeader pps_video_header = GetDefaultH264VideoHeader(); + AddPps(&pps_video_header, 0, 1, &pps_data); + rtp_packet.SetSequenceNumber(1); + pps_video_header.is_first_packet_in_frame = true; + pps_video_header.frame_type = VideoFrameType::kEmptyFrame; + mock_on_complete_frame_callback_.AppendExpectedBitstream( + kH264StartCode, sizeof(kH264StartCode)); + mock_on_complete_frame_callback_.AppendExpectedBitstream(pps_data.data(), + pps_data.size()); + rtp_video_stream_receiver_->OnReceivedPayloadData(pps_data, rtp_packet, + pps_video_header); + + rtc::CopyOnWriteBuffer idr_data; + RTPVideoHeader idr_video_header = GetDefaultH264VideoHeader(); + AddIdr(&idr_video_header, 1); + rtp_packet.SetSequenceNumber(2); + idr_video_header.is_first_packet_in_frame = true; + idr_video_header.is_last_packet_in_frame = true; + idr_video_header.frame_type = VideoFrameType::kVideoFrameKey; + const uint8_t idr[] = {0x65, 1, 2, 3}; + idr_data.AppendData(idr); + mock_on_complete_frame_callback_.AppendExpectedBitstream( + kH264StartCode, sizeof(kH264StartCode)); + mock_on_complete_frame_callback_.AppendExpectedBitstream(idr_data.data(), + idr_data.size()); + EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame) + .WillOnce([&](video_coding::EncodedFrame* frame) { + EXPECT_TRUE(frame->is_keyframe()); + }); + rtp_video_stream_receiver_->OnReceivedPayloadData(idr_data, rtp_packet, + idr_video_header); + mock_on_complete_frame_callback_.ClearExpectedBitstream(); + mock_on_complete_frame_callback_.AppendExpectedBitstream( + kH264StartCode, sizeof(kH264StartCode)); + mock_on_complete_frame_callback_.AppendExpectedBitstream(idr_data.data(), + idr_data.size()); + rtp_packet.SetSequenceNumber(3); + EXPECT_CALL(mock_on_complete_frame_callback_, DoOnCompleteFrame) + .WillOnce([&](video_coding::EncodedFrame* frame) { + EXPECT_FALSE(frame->is_keyframe()); + }); + rtp_video_stream_receiver_->OnReceivedPayloadData(idr_data, rtp_packet, + idr_video_header); +} + TEST_F(RtpVideoStreamReceiver2Test, PaddingInMediaStream) { RtpPacketReceived rtp_packet; RTPVideoHeader video_header = GetDefaultH264VideoHeader();