Add new fmtp parameter for H.264

Bug: webrtc:11769, webrtc:8423, webrtc:11376
Change-Id: Ia8f22ff90f817ba46ca03de1e43d3088c05023cd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178904
Commit-Queue: Eldar Rello <elrello@microsoft.com>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31878}

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";
 

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[];
 

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_(
+      sps_pps_idr_is_h264_keyframe_(false) {
-          field_trial::IsEnabled("WebRTC-SpsPpsIdrIsH264Keyframe")) {
   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<int64_t> 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;

modules/video_coding/packet_buffer.h

@@ -94,6 +94,7 @@ class PacketBuffer {
       RTC_LOCKS_EXCLUDED(mutex_);
   absl::optional<int64_t> 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

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 = "")
+  PacketBufferTest()
-      : scoped_field_trials_(field_trials),
+      : rand_(0x7732213),
-        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
+      : PacketBufferTest(), sps_pps_idr_is_keyframe_(sps_pps_idr_is_keyframe) {
-                             ? "WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/"
+    if (sps_pps_idr_is_keyframe) {
-                             : ""),
+      packet_buffer_.ForceSpsPpsIdrIsH264Keyframe();
-        sps_pps_idr_is_keyframe_(sps_pps_idr_is_keyframe) {}
+    }
+  }
 
   PacketBufferInsertResult InsertH264(
       uint16_t seq_num,     // packet sequence number

video/rtp_video_stream_receiver.cc

@@ -361,6 +361,10 @@ void RtpVideoStreamReceiver::AddReceiveCodec(
     const VideoCodec& video_codec,
     const std::map<std::string, std::string>& 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<VideoRtpDepacketizerRaw>()

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<uint8_t, 2>({H264::NaluType::kSps, sps_id});
     auto& h264 = absl::get<RTPVideoHeaderH264>(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<uint8_t, 2>({H264::NaluType::kPps, pps_id});
     auto& h264 = absl::get<RTPVideoHeaderH264>(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.
+TEST_P(RtpVideoStreamReceiverTestH264, InBandSpsPps) {
-#if defined(MEMORY_SANITIZER)
-#define MAYBE_InBandSpsPps DISABLED_InBandSpsPps
-#else
-#define MAYBE_InBandSpsPps InBandSpsPps
-#endif
-TEST_P(RtpVideoStreamReceiverTestH264, MAYBE_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<std::string, std::string> 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();