Initialize packetization mode in VideoToolbox

webrtc/sdk/objc/Framework/Classes/h264_video_toolbox_encoder.mm

 /*
  *  Copyright (c) 2015 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 324 matching lines @@
 
 namespace webrtc {
 
 // .5 is set as a mininum to prevent overcompensating for large temporary
 // overshoots. We don't want to degrade video quality too badly.
 // .95 is set to prevent oscillations. When a lower bitrate is set on the
 // encoder than previously set, its output seems to have a brief period of
 // drastically reduced bitrate, so we want to avoid that. In steady state
 // conditions, 0.95 seems to give us better overall bitrate over long periods
 // of time.
-H264VideoToolboxEncoder::H264VideoToolboxEncoder(
-    const cricket::VideoCodec& codec)
+H264VideoToolboxEncoder::H264VideoToolboxEncoder(const cricket::VideoCodec& codec)
     : callback_(nullptr),
       compression_session_(nullptr),
       bitrate_adjuster_(Clock::GetRealTimeClock(), .5, .95),
+      packetization_mode_(H264PacketizationMode::NonInterleaved),
       profile_(internal::ExtractProfile(codec)) {
   LOG(LS_INFO) << "Using profile " << internal::CFStringToString(profile_);
+  RTC_CHECK(cricket::CodecNamesEq(codec.name, cricket::kH264CodecName));
 }
 
 H264VideoToolboxEncoder::~H264VideoToolboxEncoder() {
   DestroyCompressionSession();
 }
 
 int H264VideoToolboxEncoder::InitEncode(const VideoCodec* codec_settings,
                                         int number_of_cores,
                                         size_t max_payload_size) {
   RTC_DCHECK(codec_settings);
   RTC_DCHECK_EQ(codec_settings->codecType, kVideoCodecH264);
 
   width_ = codec_settings->width;
   height_ = codec_settings->height;
   // We can only set average bitrate on the HW encoder.
   target_bitrate_bps_ = codec_settings->startBitrate;
   bitrate_adjuster_.SetTargetBitrateBps(target_bitrate_bps_);
 
   // TODO(tkchin): Try setting payload size via
   // kVTCompressionPropertyKey_MaxH264SliceBytes.
 
   return ResetCompressionSession();
 }
 
 int H264VideoToolboxEncoder::Encode(
     const VideoFrame& frame,
     const CodecSpecificInfo* codec_specific_info,

[hta-webrtc, 2016/12/15 14:29:00]

The next-level problem is probably that this parameter is garbage (if I read the
code right), since we have to overwrite it below.

[kthelgason, 2016/12/15 14:38:13]

I think this param is always NULL, at least when I inspected it while running. I
haven't traced where it comes from though.

     const std::vector<FrameType>* frame_types) {
   // |input_frame| size should always match codec settings.
   RTC_DCHECK_EQ(frame.width(), width_);
   RTC_DCHECK_EQ(frame.height(), height_);
   RTC_DCHECK(!frame.IsZeroSize());
   if (!callback_ || !compression_session_) {
     return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
   }
 #if defined(WEBRTC_IOS)
   if (!RTCIsUIApplicationActive()) {
@@ skipping 75 matching lines @@
   if (is_keyframe_required) {
     CFTypeRef keys[] = {kVTEncodeFrameOptionKey_ForceKeyFrame};
     CFTypeRef values[] = {kCFBooleanTrue};
     frame_properties = internal::CreateCFDictionary(keys, values, 1);
   }
   std::unique_ptr<internal::FrameEncodeParams> encode_params;
   encode_params.reset(new internal::FrameEncodeParams(
       this, codec_specific_info, width_, height_, frame.render_time_ms(),
       frame.timestamp(), frame.rotation()));
 
+  encode_params->codec_specific_info.codecSpecific.H264.packetization_mode =
+      packetization_mode_;

[hta-webrtc, 2016/12/15 14:29:01]

This is OK as a first-order fix; if internal::FrameEncodeParams' constructor is
working in the obvious way, we're overwriting a value copied from the
codec_specific_info parameter passed to this function.

It makes sense to add a TODO here.

[kthelgason, 2016/12/15 14:38:13]

In the implementation of this method in h264_encoder_impl.cc this param goes
unused. I think it's value when passed in is not important.

+
   // Update the bitrate if needed.
   SetBitrateBps(bitrate_adjuster_.GetAdjustedBitrateBps());
 
   OSStatus status = VTCompressionSessionEncodeFrame(
       compression_session_, pixel_buffer, presentation_time_stamp,
       kCMTimeInvalid, frame_properties, encode_params.release(), nullptr);
   if (frame_properties) {
     CFRelease(frame_properties);
   }
   if (pixel_buffer) {
@@ skipping 246 matching lines @@
   }
   bitrate_adjuster_.Update(frame._size);
 }
 
 VideoEncoder::ScalingSettings H264VideoToolboxEncoder::GetScalingSettings()
     const {
   return VideoEncoder::ScalingSettings(true, internal::kLowH264QpThreshold,
                                        internal::kHighH264QpThreshold);
 }
 }  // namespace webrtc