Save width/height of SPS nalus and restore them on the first packet of an IDR.

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 27 matching lines @@
   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;
   size_t required_size = 0;
   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;
@@ skipping 15 matching lines @@
             return kRequestKeyframe;
           }
 
           auto 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";
             return kRequestKeyframe;
           }
 
+          sps_id = pps->second.sps_id;
           pps_id = nalu.pps_id;
           required_size += pps->second.size + sizeof(start_code_h264);
           required_size += sps->second.size + sizeof(start_code_h264);
         }
         FALLTHROUGH();
       }
       default: {
         // Something other than an SPS/PPS nalu in this packet, then it should
         // be inserted into the PacketBuffer.
         insert_packet = true;
@@ skipping 67 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_data_[sps_id].width > 0 && sps_data_[sps_id].height) {

[stefan-webrtc, 2017/03/14 10:32:20]

height > 0

Or, if you anyway initialize width/height to -1 you might as well always do this
assignment?

[philipel, 2017/03/14 11:51:12]

The problem is that I can't be certain that the SPS/PPS packet will have the
width/height set (the width, height is always in the SPS, but I save info from
the RTP header).

My experience with H264 so far is that no two implementations works the same,
and the way to get it to work is to make our implementation as forgiving as
possible, and this is the more general solution.

[stefan-webrtc, 2017/03/14 12:06:36]

I'm fairly sure width/height is required in the sps nalu, but if you want to be
on the safe side...

I think my main point was that sps_data_[sps_id].width is initialized to -1, and
if there was no width/height in the sps then I'd expect that we also want
packet->width to be -1? That would still be what happens if we remove the if
statement.

[philipel, 2017/03/14 13:52:41]

I looked at the code and we only know the width/height if we receive an SPS, so
yeah, this if check is redundant.

But it also made me realize that we need to save this info in InsertSpsPpsNalus.

+    packet->width = sps_data_[sps_id].width;
+    packet->height = sps_data_[sps_id].height;
+  }
+
   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;
   if (sps.size() < kNaluHeaderOffset) {
     LOG(LS_WARNING) << "SPS size  " << sps.size() << " is smaller than "
@@ skipping 45 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