Only treat H.264 frames containing SPS, PPS, and IDR as key frames.
This is protected behind a field trial, for controlled rollout.
TESTED=MediaCodec (Qualcomm + Exynos) and VideoToolbox senders.
BUG=webrtc:8423
Change-Id: Ibccefb3d374e4a44461d33e77eff754d8d752666
Reviewed-on: https://webrtc-review.googlesource.com/13863
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20408}diff --git a/modules/video_coding/frame_object.cc b/modules/video_coding/frame_object.cc
index 7a724a8..7c9c41b 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 @@
_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<uint16_t>(last_seq_num + 1) &&
_frameType == kVideoFrameDelta;
@@ -82,13 +86,23 @@
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 4d6a52e..4c6f0b3 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 @@
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 @@
EXPECT_EQ(memcmp(result, expected, kStartSize), 0);
}
-TEST_F(TestPacketBuffer, GetBitstreamOneFrameFullBufferH264) {
+class TestPacketBufferH264 : public TestPacketBuffer,
+ public ::testing::WithParamInterface<bool> {
+ 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 873644f..96996ba 100644
--- a/video/end_to_end_tests.cc
+++ b/video/end_to_end_tests.cc
@@ -411,21 +411,32 @@
#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 @@
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) {
