diff --git a/media/base/media_constants.cc b/media/base/media_constants.cc index bb3403f892..864855ef91 100644 --- a/media/base/media_constants.cc +++ b/media/base/media_constants.cc @@ -111,6 +111,7 @@ const char kH264FmtpProfileLevelId[] = "profile-level-id"; const char kH264FmtpLevelAsymmetryAllowed[] = "level-asymmetry-allowed"; const char kH264FmtpPacketizationMode[] = "packetization-mode"; const char kH264FmtpSpropParameterSets[] = "sprop-parameter-sets"; +const char kH264FmtpSpsPpsIdrInKeyframe[] = "sps-pps-idr-in-keyframe"; const char kH264ProfileLevelConstrainedBaseline[] = "42e01f"; const char kH264ProfileLevelConstrainedHigh[] = "640c1f"; diff --git a/media/base/media_constants.h b/media/base/media_constants.h index 4528167de1..ecc40d6aca 100644 --- a/media/base/media_constants.h +++ b/media/base/media_constants.h @@ -137,6 +137,7 @@ RTC_EXPORT extern const char kH264FmtpProfileLevelId[]; RTC_EXPORT extern const char kH264FmtpLevelAsymmetryAllowed[]; RTC_EXPORT extern const char kH264FmtpPacketizationMode[]; extern const char kH264FmtpSpropParameterSets[]; +extern const char kH264FmtpSpsPpsIdrInKeyframe[]; extern const char kH264ProfileLevelConstrainedBaseline[]; extern const char kH264ProfileLevelConstrainedHigh[]; diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc index 7da8a1c301..d2a2bcfb47 100644 --- a/modules/video_coding/packet_buffer.cc +++ b/modules/video_coding/packet_buffer.cc @@ -31,7 +31,6 @@ #include "rtc_base/logging.h" #include "rtc_base/numerics/mod_ops.h" #include "system_wrappers/include/clock.h" -#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace video_coding { @@ -63,8 +62,7 @@ PacketBuffer::PacketBuffer(Clock* clock, first_packet_received_(false), is_cleared_to_first_seq_num_(false), buffer_(start_buffer_size), - sps_pps_idr_is_h264_keyframe_( - field_trial::IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe")) { + sps_pps_idr_is_h264_keyframe_(false) { RTC_DCHECK_LE(start_buffer_size, max_buffer_size); // Buffer size must always be a power of 2. RTC_DCHECK((start_buffer_size & (start_buffer_size - 1)) == 0); @@ -194,7 +192,9 @@ absl::optional PacketBuffer::LastReceivedKeyframePacketMs() const { MutexLock lock(&mutex_); return last_received_keyframe_packet_ms_; } - +void PacketBuffer::ForceSpsPpsIdrIsH264Keyframe() { + sps_pps_idr_is_h264_keyframe_ = true; +} void PacketBuffer::ClearInternal() { for (auto& entry : buffer_) { entry = nullptr; diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h index 508fa8395f..160d8c7c3f 100644 --- a/modules/video_coding/packet_buffer.h +++ b/modules/video_coding/packet_buffer.h @@ -94,6 +94,7 @@ class PacketBuffer { RTC_LOCKS_EXCLUDED(mutex_); absl::optional LastReceivedKeyframePacketMs() const RTC_LOCKS_EXCLUDED(mutex_); + void ForceSpsPpsIdrIsH264Keyframe(); private: Clock* const clock_; @@ -147,7 +148,7 @@ class PacketBuffer { // Indicates if we should require SPS, PPS, and IDR for a particular // RTP timestamp to treat the corresponding frame as a keyframe. - const bool sps_pps_idr_is_h264_keyframe_; + bool sps_pps_idr_is_h264_keyframe_; }; } // namespace video_coding diff --git a/modules/video_coding/packet_buffer_unittest.cc b/modules/video_coding/packet_buffer_unittest.cc index 242fff2526..a01b480398 100644 --- a/modules/video_coding/packet_buffer_unittest.cc +++ b/modules/video_coding/packet_buffer_unittest.cc @@ -100,9 +100,8 @@ void PrintTo(const PacketBufferInsertResult& result, std::ostream* os) { class PacketBufferTest : public ::testing::Test { protected: - explicit PacketBufferTest(std::string field_trials = "") - : scoped_field_trials_(field_trials), - rand_(0x7732213), + PacketBufferTest() + : rand_(0x7732213), clock_(0), packet_buffer_(&clock_, kStartSize, kMaxSize) {} @@ -133,7 +132,6 @@ class PacketBufferTest : public ::testing::Test { packet_buffer_.InsertPacket(std::move(packet))); } - const test::ScopedFieldTrials scoped_field_trials_; Random rand_; SimulatedClock clock_; PacketBuffer packet_buffer_; @@ -391,10 +389,11 @@ TEST_F(PacketBufferTest, InsertPacketAfterSequenceNumberWrapAround) { class PacketBufferH264Test : public PacketBufferTest { protected: explicit PacketBufferH264Test(bool sps_pps_idr_is_keyframe) - : PacketBufferTest(sps_pps_idr_is_keyframe - ? "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/" - : ""), - sps_pps_idr_is_keyframe_(sps_pps_idr_is_keyframe) {} + : PacketBufferTest(), sps_pps_idr_is_keyframe_(sps_pps_idr_is_keyframe) { + if (sps_pps_idr_is_keyframe) { + packet_buffer_.ForceSpsPpsIdrIsH264Keyframe(); + } + } PacketBufferInsertResult InsertH264( uint16_t seq_num, // packet sequence number diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 05b419b8c9..afe2d86d54 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -361,6 +361,10 @@ void RtpVideoStreamReceiver::AddReceiveCodec( const VideoCodec& video_codec, const std::map& codec_params, bool raw_payload) { + if (codec_params.count(cricket::kH264FmtpSpsPpsIdrInKeyframe) || + field_trial::IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe")) { + packet_buffer_.ForceSpsPpsIdrIsH264Keyframe(); + } payload_type_map_.emplace( video_codec.plType, raw_payload ? std::make_unique() diff --git a/video/rtp_video_stream_receiver_unittest.cc b/video/rtp_video_stream_receiver_unittest.cc index 20d6ae88ad..e0c5a4f500 100644 --- a/video/rtp_video_stream_receiver_unittest.cc +++ b/video/rtp_video_stream_receiver_unittest.cc @@ -200,7 +200,7 @@ class RtpVideoStreamReceiverTest : 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; } @@ -213,7 +213,7 @@ class RtpVideoStreamReceiverTest : 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; } @@ -518,13 +518,7 @@ INSTANTIATE_TEST_SUITE_P(SpsPpsIdrIsKeyframe, RtpVideoStreamReceiverTestH264, 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(RtpVideoStreamReceiverTestH264, MAYBE_InBandSpsPps) { +TEST_P(RtpVideoStreamReceiverTestH264, InBandSpsPps) { rtc::CopyOnWriteBuffer sps_data; RtpPacketReceived rtp_packet; RTPVideoHeader sps_video_header = GetDefaultH264VideoHeader(); @@ -613,6 +607,78 @@ TEST_P(RtpVideoStreamReceiverTestH264, OutOfBandFmtpSpsPps) { video_header); } +TEST_P(RtpVideoStreamReceiverTestH264, ForceSpsPpsIdrIsKeyframe) { + constexpr int kPayloadType = 99; + VideoCodec codec; + codec.plType = kPayloadType; + 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(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(RtpVideoStreamReceiverTest, PaddingInMediaStream) { RtpPacketReceived rtp_packet; RTPVideoHeader video_header = GetDefaultH264VideoHeader();