Check H264 NALUs for frametype and insert SPS/PPS packets into the PacketBuffer.

BUG=chromium:719095

Review-Url: https://codereview.webrtc.org/2889163003
Cr-Commit-Position: refs/heads/master@{#18214}

webrtc/modules/video_coding/frame_object.cc

@@ -9,6 +9,7 @@
  */
 
 #include "webrtc/base/checks.h"
+#include "webrtc/common_video/h264/h264_common.h"
 #include "webrtc/modules/video_coding/frame_object.h"
 #include "webrtc/modules/video_coding/packet_buffer.h"
 
@@ -65,7 +66,32 @@ RtpFrameObject::RtpFrameObject(PacketBuffer* packet_buffer,
 
   _buffer = new uint8_t[_size];
   _length = frame_size;
-  _frameType = first_packet->frameType;
+
+  // 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.
+  if (codec_type_ == kVideoCodecH264) {
+    _frameType = kVideoFrameDelta;
+    frame_type_ = kVideoFrameDelta;
+    for (uint16_t seq_num = first_seq_num;
+         seq_num != last_seq_num + 1 && _frameType == kVideoFrameDelta;
+         ++seq_num) {
+      VCMPacket* packet = packet_buffer_->GetPacket(seq_num);
+      RTC_DCHECK(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;
+        }
+      }
+    }
+  } else {
+    _frameType = first_packet->frameType;
+    frame_type_ = first_packet->frameType;
+  }
+
   GetBitstream(_buffer);
   _encodedWidth = first_packet->width;
   _encodedHeight = first_packet->height;

webrtc/modules/video_coding/h264_sps_pps_tracker.cc

@@ -37,15 +37,9 @@ H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream(
   const RTPVideoHeader& video_header = packet->video_header;
   const RTPVideoHeaderH264& codec_header = video_header.codecHeader.H264;
 
-  // Packets that only contains SPS/PPS are not decodable by themselves, and
-  // to avoid frames being created containing only these two nalus we don't
-  // insert them into the PacketBuffer. Instead we save the SPS/PPS and
-  // prepend the bitstream of first packet of an IDR referring to the
-  // corresponding SPS/PPS id.
-  bool insert_packet = codec_header.nalus_length == 0 ? true : false;
-
   int pps_id = -1;
   int sps_id = -1;
+  bool append_sps_pps = codec_header.nalus_length == 0;
   size_t required_size = 0;
   for (size_t i = 0; i < codec_header.nalus_length; ++i) {
     const NaluInfo& nalu = codec_header.nalus[i];
@@ -101,15 +95,28 @@ H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream(
         FALLTHROUGH();
       }
       default: {
-        // Something other than an SPS/PPS nalu in this packet, then it should
-        // be inserted into the PacketBuffer.
-        insert_packet = true;
+        // Something other than an SPS/PPS nalu in this packet, then the SPS/PPS
+        // should be appended.
+        append_sps_pps = true;
       }
     }
   }
 
-  if (!insert_packet)
-    return kDrop;
+  if (!append_sps_pps) {
+    // Two things: Firstly, when we receive a packet the data pointed at by
+    // |dataPtr| is volatile, meaning we have to copy the data into our own
+    // buffer if we want to use it at a later stage. Secondly, when a packet is
+    // inserted into the PacketBuffer it expects the packet to own its own
+    // buffer, and this function copies (and fix) the bitstream of the packet
+    // into its own buffer.
+    //
+    // SPS/PPS packets is a special case. Since we save the SPS/PPS NALU and
+    // append it to the first packet of every IDR frame the SPS/PPS packet
+    // doesn't actually need to contain any bitstream data.
+    packet->dataPtr = nullptr;
+    packet->sizeBytes = 0;
+    return kInsert;
+  }
 
   // Calculate how much space we need for the rest of the bitstream.
   if (codec_header.packetization_type == kH264StapA) {

webrtc/modules/video_coding/h264_sps_pps_tracker_unittest.cc

@@ -198,7 +198,7 @@ TEST_F(TestH264SpsPpsTracker, SpsPpsPacketThenIdrFirstPacket) {
   AddPps(&sps_pps_packet, 0, 1, &data);
   sps_pps_packet.dataPtr = data.data();
   sps_pps_packet.sizeBytes = data.size();
-  EXPECT_EQ(H264SpsPpsTracker::kDrop,
+  EXPECT_EQ(H264SpsPpsTracker::kInsert,
             tracker_.CopyAndFixBitstream(&sps_pps_packet));
   data.clear();
 
@@ -335,28 +335,28 @@ TEST_F(TestH264SpsPpsTracker, SaveRestoreWidthHeight) {
 
   // Insert an SPS/PPS packet with width/height and make sure
   // that information is set on the first IDR packet.
-  VCMPacket sps_pps_packet1 = GetDefaultPacket();
-  AddSps(&sps_pps_packet1, 0, &data);
-  AddPps(&sps_pps_packet1, 0, 1, &data);
-  sps_pps_packet1.dataPtr = data.data();
-  sps_pps_packet1.sizeBytes = data.size();
-  sps_pps_packet1.width = 320;
-  sps_pps_packet1.height = 240;
-  EXPECT_EQ(H264SpsPpsTracker::kDrop,
-            tracker_.CopyAndFixBitstream(&sps_pps_packet1));
-
-  VCMPacket idr_packet1 = GetDefaultPacket();
-  idr_packet1.video_header.is_first_packet_in_frame = true;
-  AddIdr(&idr_packet1, 1);
-  data.insert(data.end(), {1, 2, 3});
-  idr_packet1.dataPtr = data.data();
-  idr_packet1.sizeBytes = data.size();
+  VCMPacket sps_pps_packet = GetDefaultPacket();
+  AddSps(&sps_pps_packet, 0, &data);
+  AddPps(&sps_pps_packet, 0, 1, &data);
+  sps_pps_packet.dataPtr = data.data();
+  sps_pps_packet.sizeBytes = data.size();
+  sps_pps_packet.width = 320;
+  sps_pps_packet.height = 240;
   EXPECT_EQ(H264SpsPpsTracker::kInsert,
-            tracker_.CopyAndFixBitstream(&idr_packet1));
+            tracker_.CopyAndFixBitstream(&sps_pps_packet));
 
-  EXPECT_EQ(320, idr_packet1.width);
-  EXPECT_EQ(240, idr_packet1.height);
-  delete[] idr_packet1.dataPtr;
+  VCMPacket idr_packet = GetDefaultPacket();
+  idr_packet.video_header.is_first_packet_in_frame = true;
+  AddIdr(&idr_packet, 1);
+  data.insert(data.end(), {1, 2, 3});
+  idr_packet.dataPtr = data.data();
+  idr_packet.sizeBytes = data.size();
+  EXPECT_EQ(H264SpsPpsTracker::kInsert,
+            tracker_.CopyAndFixBitstream(&idr_packet));
+
+  EXPECT_EQ(320, idr_packet.width);
+  EXPECT_EQ(240, idr_packet.height);
+  delete[] idr_packet.dataPtr;
 }
 
 }  // namespace video_coding