H.264 packetization mode 0 (try 2)

webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc

Index: webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc
diff --git a/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc b/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc
index f29a4895ff181c72c4f5342082d5efb14e147955..88d66b2891d4109631348c9d75cdcb3706a794d7 100644
--- a/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc
+++ b/webrtc/modules/video_coding/codecs/h264/h264_encoder_impl.cc
@@ -150,6 +150,7 @@ static void RtpFragmentize(EncodedImage* encoded_image,
 
 H264EncoderImpl::H264EncoderImpl()
     : openh264_encoder_(nullptr),
+      max_payload_size_(0),
       number_of_cores_(0),
       encoded_image_callback_(nullptr),
       has_reported_init_(false),
@@ -161,7 +162,7 @@ H264EncoderImpl::~H264EncoderImpl() {
 
 int32_t H264EncoderImpl::InitEncode(const VideoCodec* codec_settings,
                                     int32_t number_of_cores,
-                                    size_t /*max_payload_size*/) {
+                                    size_t max_payload_size) {
   ReportInit();
   if (!codec_settings ||
       codec_settings->codecType != kVideoCodecH264) {
@@ -201,11 +202,15 @@ int32_t H264EncoderImpl::InitEncode(const VideoCodec* codec_settings,
   // else WELS_LOG_DEFAULT is used by default.
 
   number_of_cores_ = number_of_cores;
+  RTC_DCHECK(codec_settings->codecSpecific.H264.packetization_mode !=
+             kH264PacketizationModeNotSet);

[hbos, 2016/10/31 10:32:26]

nit: RTC_DCHECK_NE

[hta-webrtc, 2016/10/31 15:03:04]

Acknowledged.

   codec_settings_ = *codec_settings;
+  max_payload_size_ = max_payload_size;
   if (codec_settings_.targetBitrate == 0)
     codec_settings_.targetBitrate = codec_settings_.startBitrate;
 
   SEncParamExt encoder_params = CreateEncoderParams();
+
   // Initialize.
   if (openh264_encoder_->InitializeExt(&encoder_params) != 0) {
     LOG(LS_ERROR) << "Failed to initialize OpenH264 encoder";
@@ -367,6 +372,8 @@ int32_t H264EncoderImpl::Encode(const VideoFrame& input_frame,
     // Deliver encoded image.
     CodecSpecificInfo codec_specific;
     codec_specific.codecType = kVideoCodecH264;
+    codec_specific.codecSpecific.H264.packetization_mode =
+        codec_settings_.codecSpecific.H264.packetization_mode;
     encoded_image_callback_->Encoded(encoded_image_, &codec_specific,
                                      &frag_header);
 
@@ -439,6 +446,27 @@ SEncParamExt H264EncoderImpl::CreateEncoderParams() const {
       encoder_params.iTargetBitrate;
   encoder_params.sSpatialLayers[0].iMaxSpatialBitrate =
       encoder_params.iMaxBitrate;
+  switch (codec_settings_.H264().packetization_mode) {
+    case kH264PacketizationMode0:
+#if (OPENH264_MAJOR == 1) && (OPENH264_MINOR <= 5)
+      // Limit the size of packets produced.
+      encoder_params.sSpatialLayers[0].sSliceCfg.uiSliceMode = SM_DYN_SLICE;
+      encoder_params.sSpatialLayers[0]
+          .sSliceCfg.sSliceArgument.uiSliceSizeConstraint =
+          static_cast<unsigned int>(max_payload_size_);
+#else
+      // When uiSliceMode = SM_FIXEDSLCNUM_SLICE, uiSliceNum = 0 means auto
+      // design
+      // it with cpu core number.
+      encoder_params.sSpatialLayers[0].sSliceArgument.uiSliceNum = 0;

[hbos, 2016/10/31 10:32:26]

+sprang tried this and uiSliceNum != 1 resulted in borked rate control resulting
in major quality regressions and having to revert I believe.
This makes me afraid that we might run into the same problem if slicing happens
due to uiSlizeSizeConstraint, but not sure if that is the same problem.

Do you have anything to add, sprang?

See
https://codereview.webrtc.org/2458673002/diff2/1:20001/webrtc/modules/video_c...

[sprang_webrtc, 2016/10/31 14:42:49]

Not sure what will even happen when uiSliceMode = SM_SIZELIMITED_SLICE rather
than   SM_FIXEDSLCNUM_SLICE. It's simple enough to test though. Just run
video_loopback with the desired parameters and see if it looks like crap.

[hta-webrtc, 2016/10/31 15:03:04]

In mode 0, the packet size is the important thing to control. In mode 1, I defer
to sprang. My compare-codecs tests indicate that yes, there is borkedness in the
rate control of openh264.

[hbos, 2016/10/31 16:27:15]

When merging this with master later there should be a conflict with sprang's CL
and you can keep the values he sets.

+      encoder_params.sSpatialLayers[0].sSliceArgument.uiSliceMode =
+          SM_SIZELIMITED_SLICE;
+      encoder_params.sSpatialLayers[0]
+          .sSliceCfg.sSliceArgument.uiSliceSizeConstraint =
+          static_cast<unsigned int>(max_payload_size_);
+#endif
+      break;
+    case kH264PacketizationMode1:
 #if (OPENH264_MAJOR == 1) && (OPENH264_MINOR <= 5)
   // Slice num according to number of threads.
   encoder_params.sSpatialLayers[0].sSliceCfg.uiSliceMode = SM_AUTO_SLICE;
@@ -449,7 +477,10 @@ SEncParamExt H264EncoderImpl::CreateEncoderParams() const {
   encoder_params.sSpatialLayers[0].sSliceArgument.uiSliceMode =
       SM_FIXEDSLCNUM_SLICE;
 #endif
-
+  break;
+    default:
+      RTC_NOTREACHED() << "Illegal packetization mode specified";
+  }
   return encoder_params;
 }