diff --git a/modules/video_coding/frame_object.cc b/modules/video_coding/frame_object.cc index 7a724a8f34..7c9c41b72f 100644 --- a/modules/video_coding/frame_object.cc +++ b/modules/video_coding/frame_object.cc @@ -12,6 +12,7 @@ #include "common_video/h264/h264_common.h" #include "modules/video_coding/packet_buffer.h" #include "rtc_base/checks.h" +#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace video_coding { @@ -69,11 +70,14 @@ RtpFrameObject::RtpFrameObject(PacketBuffer* packet_buffer, _length = frame_size; // For H264 frames we can't determine the frame type by just looking at the - // first packet. Instead we consider the frame to be a keyframe if it - // contains an IDR NALU. + // first packet. Instead we consider the frame to be a keyframe if it contains + // an IDR, and SPS/PPS if the field trial is set. if (codec_type_ == kVideoCodecH264) { _frameType = kVideoFrameDelta; frame_type_ = kVideoFrameDelta; + bool contains_sps = false; + bool contains_pps = false; + bool contains_idr = false; for (uint16_t seq_num = first_seq_num; seq_num != static_cast(last_seq_num + 1) && _frameType == kVideoFrameDelta; @@ -82,13 +86,23 @@ RtpFrameObject::RtpFrameObject(PacketBuffer* packet_buffer, RTC_CHECK(packet); const RTPVideoHeaderH264& header = packet->video_header.codecHeader.H264; for (size_t i = 0; i < header.nalus_length; ++i) { - if (header.nalus[i].type == H264::NaluType::kIdr) { - _frameType = kVideoFrameKey; - frame_type_ = kVideoFrameKey; - break; + if (header.nalus[i].type == H264::NaluType::kSps) { + contains_sps = true; + } else if (header.nalus[i].type == H264::NaluType::kPps) { + contains_pps = true; + } else if (header.nalus[i].type == H264::NaluType::kIdr) { + contains_idr = true; } } } + const bool sps_pps_idr_is_keyframe = + field_trial::IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe"); + if ((sps_pps_idr_is_keyframe && contains_idr && contains_sps && + contains_pps) || + (!sps_pps_idr_is_keyframe && contains_idr)) { + _frameType = kVideoFrameKey; + frame_type_ = kVideoFrameKey; + } } else { _frameType = first_packet->frameType; frame_type_ = first_packet->frameType; diff --git a/modules/video_coding/video_packet_buffer_unittest.cc b/modules/video_coding/video_packet_buffer_unittest.cc index 4d6a52e9c2..4c6f0b3f52 100644 --- a/modules/video_coding/video_packet_buffer_unittest.cc +++ b/modules/video_coding/video_packet_buffer_unittest.cc @@ -18,6 +18,7 @@ #include "modules/video_coding/packet_buffer.h" #include "rtc_base/random.h" #include "system_wrappers/include/clock.h" +#include "test/field_trial.h" #include "test/gtest.h" namespace webrtc { @@ -70,27 +71,6 @@ class TestPacketBuffer : public ::testing::Test, return packet_buffer_->InsertPacket(&packet); } - bool InsertH264(uint16_t seq_num, // packet sequence number - IsKeyFrame keyframe, // is keyframe - IsFirst first, // is first packet of frame - IsLast last, // is last packet of frame - uint32_t timestamp, // rtp timestamp - int data_size = 0, // size of data - uint8_t* data = nullptr) { // data pointer - VCMPacket packet; - packet.codec = kVideoCodecH264; - packet.seqNum = seq_num; - packet.timestamp = timestamp; - packet.video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kIdr; - packet.video_header.codecHeader.H264.nalus_length = keyframe == kKeyFrame; - packet.is_first_packet_in_frame = first == kFirst; - packet.markerBit = last == kLast; - packet.sizeBytes = data_size; - packet.dataPtr = data; - - return packet_buffer_->InsertPacket(&packet); - } - void CheckFrame(uint16_t first_seq_num) { auto frame_it = frames_from_callback_.find(first_seq_num); ASSERT_FALSE(frame_it == frames_from_callback_.end()) @@ -443,7 +423,59 @@ TEST_F(TestPacketBuffer, GetBitstreamOneFrameFullBuffer) { EXPECT_EQ(memcmp(result, expected, kStartSize), 0); } -TEST_F(TestPacketBuffer, GetBitstreamOneFrameFullBufferH264) { +class TestPacketBufferH264 : public TestPacketBuffer, + public ::testing::WithParamInterface { + protected: + TestPacketBufferH264() : TestPacketBufferH264(GetParam()) {} + explicit TestPacketBufferH264(bool sps_pps_idr_is_keyframe) + : sps_pps_idr_is_keyframe_(sps_pps_idr_is_keyframe), + scoped_field_trials_(sps_pps_idr_is_keyframe_ + ? "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/" + : "") {} + + bool InsertH264(uint16_t seq_num, // packet sequence number + IsKeyFrame keyframe, // is keyframe + IsFirst first, // is first packet of frame + IsLast last, // is last packet of frame + uint32_t timestamp, // rtp timestamp + int data_size = 0, // size of data + uint8_t* data = nullptr) { // data pointer + VCMPacket packet; + packet.codec = kVideoCodecH264; + packet.seqNum = seq_num; + packet.timestamp = timestamp; + if (keyframe == kKeyFrame) { + if (sps_pps_idr_is_keyframe_) { + packet.video_header.codecHeader.H264.nalus[0].type = + H264::NaluType::kSps; + packet.video_header.codecHeader.H264.nalus[1].type = + H264::NaluType::kPps; + packet.video_header.codecHeader.H264.nalus[2].type = + H264::NaluType::kIdr; + packet.video_header.codecHeader.H264.nalus_length = 3; + } else { + packet.video_header.codecHeader.H264.nalus[0].type = + H264::NaluType::kIdr; + packet.video_header.codecHeader.H264.nalus_length = 1; + } + } + packet.is_first_packet_in_frame = first == kFirst; + packet.markerBit = last == kLast; + packet.sizeBytes = data_size; + packet.dataPtr = data; + + return packet_buffer_->InsertPacket(&packet); + } + + const bool sps_pps_idr_is_keyframe_; + const test::ScopedFieldTrials scoped_field_trials_; +}; + +INSTANTIATE_TEST_CASE_P(SpsPpsIdrIsKeyframe, + TestPacketBufferH264, + ::testing::Values(false, true)); + +TEST_P(TestPacketBufferH264, GetBitstreamOneFrameFullBuffer) { uint8_t* data_arr[kStartSize]; uint8_t expected[kStartSize]; uint8_t result[kStartSize]; @@ -469,7 +501,7 @@ TEST_F(TestPacketBuffer, GetBitstreamOneFrameFullBufferH264) { EXPECT_EQ(memcmp(result, expected, kStartSize), 0); } -TEST_F(TestPacketBuffer, GetBitstreamH264BufferPadding) { +TEST_P(TestPacketBufferH264, GetBitstreamBufferPadding) { uint16_t seq_num = Rand(); uint8_t data_data[] = "some plain old data"; uint8_t* data = new uint8_t[sizeof(data_data)]; @@ -629,7 +661,7 @@ TEST_F(TestPacketBuffer, PacketTimestamps) { EXPECT_FALSE(packet_keyframe_ms); } -TEST_F(TestPacketBuffer, OneFrameFillBufferH264) { +TEST_P(TestPacketBufferH264, OneFrameFillBuffer) { InsertH264(0, kKeyFrame, kFirst, kNotLast, 1000); for (int i = 1; i < kStartSize - 1; ++i) InsertH264(i, kKeyFrame, kNotFirst, kNotLast, 1000); @@ -639,7 +671,7 @@ TEST_F(TestPacketBuffer, OneFrameFillBufferH264) { CheckFrame(0); } -TEST_F(TestPacketBuffer, CreateFramesAfterFilledBufferH264) { +TEST_P(TestPacketBufferH264, CreateFramesAfterFilledBuffer) { InsertH264(kStartSize - 2, kKeyFrame, kFirst, kLast, 0); ASSERT_EQ(1UL, frames_from_callback_.size()); frames_from_callback_.clear(); @@ -656,7 +688,7 @@ TEST_F(TestPacketBuffer, CreateFramesAfterFilledBufferH264) { CheckFrame(kStartSize); } -TEST_F(TestPacketBuffer, OneFrameMaxSeqNumH264) { +TEST_P(TestPacketBufferH264, OneFrameMaxSeqNum) { InsertH264(65534, kKeyFrame, kFirst, kNotLast, 1000); InsertH264(65535, kKeyFrame, kNotFirst, kLast, 1000); @@ -664,7 +696,7 @@ TEST_F(TestPacketBuffer, OneFrameMaxSeqNumH264) { CheckFrame(65534); } -TEST_F(TestPacketBuffer, ClearMissingPacketsOnKeyframeH264) { +TEST_P(TestPacketBufferH264, ClearMissingPacketsOnKeyframe) { InsertH264(0, kKeyFrame, kFirst, kLast, 1000); InsertH264(2, kKeyFrame, kFirst, kLast, 3000); InsertH264(3, kDeltaFrame, kFirst, kNotLast, 4000); @@ -681,7 +713,7 @@ TEST_F(TestPacketBuffer, ClearMissingPacketsOnKeyframeH264) { CheckFrame(kStartSize + 1); } -TEST_F(TestPacketBuffer, FindFramesOnPaddingH264) { +TEST_P(TestPacketBufferH264, FindFramesOnPadding) { InsertH264(0, kKeyFrame, kFirst, kLast, 1000); InsertH264(2, kDeltaFrame, kFirst, kLast, 1000); @@ -692,5 +724,90 @@ TEST_F(TestPacketBuffer, FindFramesOnPaddingH264) { CheckFrame(2); } +class TestPacketBufferH264XIsKeyframe : public TestPacketBufferH264 { + protected: + const uint16_t kSeqNum = 5; + + explicit TestPacketBufferH264XIsKeyframe(bool sps_pps_idr_is_keyframe) + : TestPacketBufferH264(sps_pps_idr_is_keyframe) { + packet_.codec = kVideoCodecH264; + packet_.seqNum = kSeqNum; + + packet_.is_first_packet_in_frame = true; + packet_.markerBit = true; + } + + VCMPacket packet_; +}; + +class TestPacketBufferH264IdrIsKeyframe + : public TestPacketBufferH264XIsKeyframe { + protected: + TestPacketBufferH264IdrIsKeyframe() + : TestPacketBufferH264XIsKeyframe(false) {} +}; + +TEST_F(TestPacketBufferH264IdrIsKeyframe, IdrIsKeyframe) { + packet_.video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kIdr; + packet_.video_header.codecHeader.H264.nalus_length = 1; + + packet_buffer_->InsertPacket(&packet_); + + ASSERT_EQ(1u, frames_from_callback_.size()); + EXPECT_EQ(kVideoFrameKey, frames_from_callback_[kSeqNum]->frame_type()); +} + +TEST_F(TestPacketBufferH264IdrIsKeyframe, SpsPpsIdrIsKeyframe) { + packet_.video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kSps; + packet_.video_header.codecHeader.H264.nalus[1].type = H264::NaluType::kPps; + packet_.video_header.codecHeader.H264.nalus[2].type = H264::NaluType::kIdr; + packet_.video_header.codecHeader.H264.nalus_length = 3; + + packet_buffer_->InsertPacket(&packet_); + + ASSERT_EQ(1u, frames_from_callback_.size()); + EXPECT_EQ(kVideoFrameKey, frames_from_callback_[kSeqNum]->frame_type()); +} + +class TestPacketBufferH264SpsPpsIdrIsKeyframe + : public TestPacketBufferH264XIsKeyframe { + protected: + TestPacketBufferH264SpsPpsIdrIsKeyframe() + : TestPacketBufferH264XIsKeyframe(true) {} +}; + +TEST_F(TestPacketBufferH264SpsPpsIdrIsKeyframe, IdrIsNotKeyframe) { + packet_.video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kIdr; + packet_.video_header.codecHeader.H264.nalus_length = 1; + + packet_buffer_->InsertPacket(&packet_); + + ASSERT_EQ(1u, frames_from_callback_.size()); + EXPECT_EQ(kVideoFrameDelta, frames_from_callback_[5]->frame_type()); +} + +TEST_F(TestPacketBufferH264SpsPpsIdrIsKeyframe, SpsPpsIsNotKeyframe) { + packet_.video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kSps; + packet_.video_header.codecHeader.H264.nalus[1].type = H264::NaluType::kPps; + packet_.video_header.codecHeader.H264.nalus_length = 2; + + packet_buffer_->InsertPacket(&packet_); + + ASSERT_EQ(1u, frames_from_callback_.size()); + EXPECT_EQ(kVideoFrameDelta, frames_from_callback_[kSeqNum]->frame_type()); +} + +TEST_F(TestPacketBufferH264SpsPpsIdrIsKeyframe, SpsPpsIdrIsKeyframe) { + packet_.video_header.codecHeader.H264.nalus[0].type = H264::NaluType::kSps; + packet_.video_header.codecHeader.H264.nalus[1].type = H264::NaluType::kPps; + packet_.video_header.codecHeader.H264.nalus[2].type = H264::NaluType::kIdr; + packet_.video_header.codecHeader.H264.nalus_length = 3; + + packet_buffer_->InsertPacket(&packet_); + + ASSERT_EQ(1u, frames_from_callback_.size()); + EXPECT_EQ(kVideoFrameKey, frames_from_callback_[kSeqNum]->frame_type()); +} + } // namespace video_coding } // namespace webrtc diff --git a/video/end_to_end_tests.cc b/video/end_to_end_tests.cc index 873644fae5..96996ba932 100644 --- a/video/end_to_end_tests.cc +++ b/video/end_to_end_tests.cc @@ -411,21 +411,32 @@ TEST_P(EndToEndTest, SendsAndReceivesVP9VideoRotation90) { #endif // !defined(RTC_DISABLE_VP9) #if defined(WEBRTC_USE_H264) -TEST_P(EndToEndTest, SendsAndReceivesH264) { +class EndToEndTestH264 : public EndToEndTest {}; + +const auto h264_field_trial_combinations = ::testing::Values( + "WebRTC-SpsPpsIdrIsH264Keyframe/Disabled/WebRTC-RoundRobinPacing/Disabled/", + "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/WebRTC-RoundRobinPacing/Disabled/", + "WebRTC-SpsPpsIdrIsH264Keyframe/Disabled/WebRTC-RoundRobinPacing/Enabled/", + "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/WebRTC-RoundRobinPacing/Enabled/"); +INSTANTIATE_TEST_CASE_P(SpsPpsIdrIsKeyframe, + EndToEndTestH264, + h264_field_trial_combinations); + +TEST_P(EndToEndTestH264, SendsAndReceivesH264) { CodecObserver test(500, kVideoRotation_0, "H264", H264Encoder::Create(cricket::VideoCodec("H264")), H264Decoder::Create()); RunBaseTest(&test); } -TEST_P(EndToEndTest, SendsAndReceivesH264VideoRotation90) { +TEST_P(EndToEndTestH264, SendsAndReceivesH264VideoRotation90) { CodecObserver test(5, kVideoRotation_90, "H264", H264Encoder::Create(cricket::VideoCodec("H264")), H264Decoder::Create()); RunBaseTest(&test); } -TEST_P(EndToEndTest, SendsAndReceivesH264PacketizationMode0) { +TEST_P(EndToEndTestH264, SendsAndReceivesH264PacketizationMode0) { cricket::VideoCodec codec = cricket::VideoCodec("H264"); codec.SetParam(cricket::kH264FmtpPacketizationMode, "0"); CodecObserver test(500, kVideoRotation_0, "H264", H264Encoder::Create(codec), @@ -433,14 +444,13 @@ TEST_P(EndToEndTest, SendsAndReceivesH264PacketizationMode0) { RunBaseTest(&test); } -TEST_P(EndToEndTest, SendsAndReceivesH264PacketizationMode1) { +TEST_P(EndToEndTestH264, SendsAndReceivesH264PacketizationMode1) { cricket::VideoCodec codec = cricket::VideoCodec("H264"); codec.SetParam(cricket::kH264FmtpPacketizationMode, "1"); CodecObserver test(500, kVideoRotation_0, "H264", H264Encoder::Create(codec), H264Decoder::Create()); RunBaseTest(&test); } - #endif // defined(WEBRTC_USE_H264) TEST_P(EndToEndTest, ReceiverUsesLocalSsrc) {