Only append SPS/PPS to bitstream if supplied out of bound.

webrtc/modules/video_coding/h264_sps_pps_tracker.cc

 /*
  *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 19 matching lines @@
 
 H264SpsPpsTracker::PacketAction H264SpsPpsTracker::CopyAndFixBitstream(
     VCMPacket* packet) {
   RTC_DCHECK(packet->codec == kVideoCodecH264);
 
   const uint8_t* data = packet->dataPtr;
   const size_t data_size = packet->sizeBytes;
   const RTPVideoHeader& video_header = packet->video_header;
   const RTPVideoHeaderH264& codec_header = video_header.codecHeader.H264;
 
-  int pps_id = -1;
-  int sps_id = -1;
-  bool append_sps_pps = codec_header.nalus_length == 0;
-  size_t required_size = 0;
+  bool append_sps_pps = false;
+  auto sps = sps_data_.end();
+  auto pps = pps_data_.end();
+
   for (size_t i = 0; i < codec_header.nalus_length; ++i) {
     const NaluInfo& nalu = codec_header.nalus[i];
     switch (nalu.type) {
       case H264::NaluType::kSps: {
-        // Save SPS.
-        sps_data_[nalu.sps_id].size = nalu.size;
-        sps_data_[nalu.sps_id].data.reset(new uint8_t[nalu.size]);
-        memcpy(sps_data_[nalu.sps_id].data.get(), data + nalu.offset,
-               nalu.size);
         sps_data_[nalu.sps_id].width = packet->width;
         sps_data_[nalu.sps_id].height = packet->height;
         break;
       }
       case H264::NaluType::kPps: {
-        // Save PPS.
         pps_data_[nalu.pps_id].sps_id = nalu.sps_id;
-        pps_data_[nalu.pps_id].size = nalu.size;
-        pps_data_[nalu.pps_id].data.reset(new uint8_t[nalu.size]);
-        memcpy(pps_data_[nalu.pps_id].data.get(), data + nalu.offset,
-               nalu.size);
         break;
       }
       case H264::NaluType::kIdr: {
         // If this is the first packet of an IDR, make sure we have the required
         // SPS/PPS and also calculate how much extra space we need in the buffer
         // to prepend the SPS/PPS to the bitstream with start codes.
         if (video_header.is_first_packet_in_frame) {
           if (nalu.pps_id == -1) {
             LOG(LS_WARNING) << "No PPS id in IDR nalu.";
             return kRequestKeyframe;
           }
 
-          auto pps = pps_data_.find(nalu.pps_id);
+          pps = pps_data_.find(nalu.pps_id);
           if (pps == pps_data_.end()) {
             LOG(LS_WARNING) << "No PPS with id << " << nalu.pps_id
                             << " received";
             return kRequestKeyframe;
           }
 
-          sps_id = pps->second.sps_id;
-          auto sps = sps_data_.find(sps_id);
+          sps = sps_data_.find(pps->second.sps_id);
           if (sps == sps_data_.end()) {
-            LOG(LS_WARNING) << "No SPS with id << "
-                            << pps_data_[nalu.pps_id].sps_id << " received";
+            LOG(LS_WARNING)
+                << "No SPS with id << " << pps->second.sps_id << " received";
             return kRequestKeyframe;
           }
 
-          pps_id = nalu.pps_id;
-          required_size += pps->second.size + sizeof(start_code_h264);
-          required_size += sps->second.size + sizeof(start_code_h264);
+          if (sps->second.data && pps->second.data) {
+            RTC_DCHECK_LE(0, sps->second.size);
+            RTC_DCHECK_LE(0, pps->second.size);

[stefan-webrtc, 2017/06/20 08:56:31]

I'd prefer RTC_DCHECK_GT(sps->second.size, 0)

[philipel, 2017/06/20 11:27:34]

Done.

+            append_sps_pps = true;
+          }
         }
         FALLTHROUGH();

[stefan-webrtc, 2017/06/20 08:56:31]

No point in FALLTHROUGH now, right? Might as well break

[philipel, 2017/06/20 11:27:34]

Done.

       }
-      default: {
-        // Something other than an SPS/PPS nalu in this packet, then the SPS/PPS
-        // should be appended.
-        append_sps_pps = true;
-      }
+      default: { break; }

[stefan-webrtc, 2017/06/20 08:56:31]

Remove {}

[philipel, 2017/06/20 11:27:34]

Done.

     }
   }
 
-  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;
+  RTC_CHECK(!append_sps_pps ||
+            (sps != sps_data_.end() && pps != pps_data_.end()));
+
+  // Calculate how much space we need for the rest of the bitstream.
+  size_t required_size = 0;
+  if (append_sps_pps) {
+    required_size += sps->second.size + sizeof(start_code_h264);
+    required_size += pps->second.size + sizeof(start_code_h264);
   }
 
-  // Calculate how much space we need for the rest of the bitstream.
   if (codec_header.packetization_type == kH264StapA) {
     const uint8_t* nalu_ptr = data + 1;
     while (nalu_ptr < data + data_size) {
       RTC_DCHECK(video_header.is_first_packet_in_frame);
       required_size += sizeof(start_code_h264);
 
       // The first two bytes describe the length of a segment.
       uint16_t segment_length = nalu_ptr[0] << 8 | nalu_ptr[1];
       nalu_ptr += 2;
 
       required_size += segment_length;
       nalu_ptr += segment_length;
     }
   } else {
     if (video_header.is_first_packet_in_frame)
       required_size += sizeof(start_code_h264);
     required_size += data_size;
   }
 
   // Then we copy to the new buffer.
   uint8_t* buffer = new uint8_t[required_size];
   uint8_t* insert_at = buffer;
 
-  // If pps_id != -1 then we have the SPS/PPS and they should be prepended
-  // to the bitstream with start codes inserted.
-  if (pps_id != -1) {
+  if (append_sps_pps) {
     // Insert SPS.
     memcpy(insert_at, start_code_h264, sizeof(start_code_h264));
     insert_at += sizeof(start_code_h264);
-    memcpy(insert_at, sps_data_[pps_data_[pps_id].sps_id].data.get(),
-           sps_data_[pps_data_[pps_id].sps_id].size);
-    insert_at += sps_data_[pps_data_[pps_id].sps_id].size;
+    memcpy(insert_at, sps->second.data.get(), sps->second.size);
+    insert_at += sps->second.size;
 
     // Insert PPS.
     memcpy(insert_at, start_code_h264, sizeof(start_code_h264));
     insert_at += sizeof(start_code_h264);
-    memcpy(insert_at, pps_data_[pps_id].data.get(), pps_data_[pps_id].size);
-    insert_at += pps_data_[pps_id].size;
+    memcpy(insert_at, pps->second.data.get(), pps->second.size);
+    insert_at += pps->second.size;
   }
 
   // Copy the rest of the bitstream and insert start codes.
   if (codec_header.packetization_type == kH264StapA) {
     const uint8_t* nalu_ptr = data + 1;
     while (nalu_ptr < data + data_size) {
       memcpy(insert_at, start_code_h264, sizeof(start_code_h264));
       insert_at += sizeof(start_code_h264);
 
       // The first two bytes describe the length of a segment.
@@ skipping 11 matching lines @@
       nalu_ptr += segment_length;
     }
   } else {
     if (video_header.is_first_packet_in_frame) {
       memcpy(insert_at, start_code_h264, sizeof(start_code_h264));
       insert_at += sizeof(start_code_h264);
     }
     memcpy(insert_at, data, data_size);
   }
 
-  if (sps_id != -1) {
-    packet->width = sps_data_[sps_id].width;
-    packet->height = sps_data_[sps_id].height;
+  if (sps != sps_data_.end()) {
+    packet->width = sps->second.width;
+    packet->height = sps->second.height;

[stefan-webrtc, 2017/06/20 08:56:31]

Should this be moved up lines 125-137? Maybe also with a comment of why we do
it?

[philipel, 2017/06/20 11:27:34]

Done.

   }
 
   packet->dataPtr = buffer;
   packet->sizeBytes = required_size;
   return kInsert;
 }
 
 void H264SpsPpsTracker::InsertSpsPpsNalus(const std::vector<uint8_t>& sps,
                                           const std::vector<uint8_t>& pps) {
   constexpr size_t kNaluHeaderOffset = 1;
@@ skipping 49 matching lines @@
   pps_info.data.reset(pps_data);
   pps_data_[parsed_pps->id] = std::move(pps_info);
 
   LOG(LS_INFO) << "Inserted SPS id " << parsed_sps->id << " and PPS id "
                << parsed_pps->id << " (referencing SPS " << parsed_pps->sps_id
                << ")";
 }
 
 }  // namespace video_coding
 }  // namespace webrtc