Use different RTX payload types for different H264 profiles

webrtc/media/engine/webrtcvideoengine2.cc

 /*
  *  Copyright (c) 2014 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.
  */
 
 #include "webrtc/media/engine/webrtcvideoengine2.h"
 
 #include <stdio.h>
 #include <algorithm>
 #include <set>
 #include <string>
 #include <utility>
 
 #include "webrtc/base/copyonwritebuffer.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/stringutils.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/base/trace_event.h"
 #include "webrtc/call.h"
+#include "webrtc/common_video/h264/profile_level_id.h"
 #include "webrtc/media/engine/constants.h"
 #include "webrtc/media/engine/simulcast.h"
 #include "webrtc/media/engine/webrtcmediaengine.h"
 #include "webrtc/media/engine/webrtcvideoencoderfactory.h"
 #include "webrtc/media/engine/webrtcvoiceengine.h"
 #include "webrtc/modules/video_coding/codecs/h264/include/h264.h"
 #include "webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.h"
 #include "webrtc/modules/video_coding/codecs/vp9/include/vp9.h"
 #include "webrtc/system_wrappers/include/field_trial.h"
 #include "webrtc/video_decoder.h"
@@ skipping 361 matching lines @@
 // don't recognize?
 void AddCodecAndMaybeRtxCodec(const VideoCodec& codec,
                               std::vector<VideoCodec>* codecs) {
   codecs->push_back(codec);
   int rtx_payload_type = 0;
   if (CodecNamesEq(codec.name, kVp8CodecName)) {
     rtx_payload_type = kDefaultRtxVp8PlType;
   } else if (CodecNamesEq(codec.name, kVp9CodecName)) {
     rtx_payload_type = kDefaultRtxVp9PlType;
   } else if (CodecNamesEq(codec.name, kH264CodecName)) {
-    rtx_payload_type = kDefaultRtxH264PlType;
+    // Parse H264 profile.
+    const rtc::Optional<webrtc::H264::ProfileLevelId> profile_level_id =
+        webrtc::H264::ParseSdpProfileLevelId(codec.params);
+    if (!profile_level_id)
+      return;
+    const webrtc::H264::Profile profile = profile_level_id->profile;
+    // Only supported rtx for constrained baseline and constrained high profile.

[hta-webrtc, 2016/11/15 10:41:07]

Grammar nit: "In H.264, we only support rtx for constrained baseline and
constrained high profile."

[magjed_webrtc, 2016/11/15 17:48:04]

Done.

+    if (profile == webrtc::H264::kProfileConstrainedBaseline) {
+      rtx_payload_type = kDefaultRtxH264ConstrainedBaselinePlType;
+    } else if (profile == webrtc::H264::kProfileConstrainedHigh) {
+      rtx_payload_type = kDefaultRtxH264ConstrainedHighPlType;
+    }

[hta-webrtc, 2016/11/15 10:41:07]

"Else" missing - if this is neither of the two above, it will fall through to
the "codecs->push_back" at the bottom with rtx_payload_type uninitialized.

I'm assuming the "else" should be "return" like the others.

Please add an unit test covering a profile for which RTX is not supported.

[magjed_webrtc, 2016/11/15 17:48:04]

Done.

   } else if (CodecNamesEq(codec.name, kRedCodecName)) {
     rtx_payload_type = kDefaultRtxRedPlType;
   } else {
     return;
   }
   codecs->push_back(VideoCodec::CreateRtxCodec(rtx_payload_type, codec.id));
 }
 
 std::vector<VideoCodec> DefaultVideoCodecList() {
   std::vector<VideoCodec> codecs;
@@ skipping 2135 matching lines @@
         rtx_mapping[video_codecs[i].codec.id] !=
             ulpfec_config.red_payload_type) {
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket