Explicitly only add transport-cc RTCP feedback param to default FlexFEC codec.

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.
  */
 
@@ skipping 130 matching lines @@
   // Disable overloaded virtual function warning. TODO(magjed): Remove once
   // http://crbug/webrtc/6402 is fixed.
   using cricket::WebRtcVideoEncoderFactory::CreateVideoEncoder;
 
   cricket::WebRtcVideoEncoderFactory* factory_;
   // A list of encoders that were created without being wrapped in a
   // SimulcastEncoderAdapter.
   std::vector<webrtc::VideoEncoder*> non_simulcast_encoders_;
 };
 
-void AddDefaultFeedbackParams(VideoCodec* codec) {
+void AddDefaultMediaFeedbackParams(VideoCodec* codec) {
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamCcm, kRtcpFbCcmParamFir));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kRtcpFbNackParamPli));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty));
   codec->AddFeedbackParam(
       FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
 }
 
+void AddDefaultFlexfecFeedbackParams(VideoCodec* codec) {
+  codec->AddFeedbackParam(
+      FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));

[stefan-webrtc, 2017/01/10 11:38:56]

I think you'd want to add remb here as well in case of receive-side bwe.

[brandtr, 2017/01/10 12:33:51]

That's right. Done.

+}
+
 static std::string CodecVectorToString(const std::vector<VideoCodec>& codecs) {
   std::stringstream out;
   out << '{';
   for (size_t i = 0; i < codecs.size(); ++i) {
     out << codecs[i].ToString();
     if (i != codecs.size() - 1) {
       out << ", ";
     }
   }
   out << '}';
@@ skipping 393 matching lines @@
     if (!is_payload_used[i - kFirstDynamicPayloadType])
       return rtc::Optional<int>(i);
   }
   // No free payload type.
   return rtc::Optional<int>();
 }
 
 // This is a helper function for GetSupportedCodecs below. It will append new
 // unique codecs from |input_codecs| to |unified_codecs|. It will add default
 // feedback params to the codecs and will also add an associated RTX codec for
-// recognized codecs (VP8, VP9, H264, and Red).
+// recognized codecs (VP8, VP9, H264, and RED).
 static void AppendVideoCodecs(const std::vector<VideoCodec>& input_codecs,
                               std::vector<VideoCodec>* unified_codecs) {
   for (VideoCodec codec : input_codecs) {
     const rtc::Optional<int> payload_type =
         NextFreePayloadType(*unified_codecs);
     if (!payload_type)
       return;
     codec.id = *payload_type;
     // TODO(magjed): Move the responsibility of setting these parameters to the
     // encoder factories instead.
-    if (codec.name != kRedCodecName && codec.name != kUlpfecCodecName)
-      AddDefaultFeedbackParams(&codec);
+    if (codec.name != kRedCodecName && codec.name != kUlpfecCodecName &&
+        codec.name != kFlexfecCodecName) {
+      AddDefaultMediaFeedbackParams(&codec);
+    } else if (codec.name == kFlexfecCodecName) {
+      AddDefaultFlexfecFeedbackParams(&codec);

[magjed_webrtc, 2017/01/10 10:46:57]

Like the TODO above says, we eventually want this moved into the codec factories
instead. So I would prefer if you move the flexfec feedback params to the
InternalEncoderFactory instead, and only add 'codec.name != kFlexfecCodecName'
to the if-statement here.

[brandtr, 2017/01/10 12:33:51]

Right, should have looked more carefully. Done.

+    }
     // Don't add same codec twice.
     if (FindMatchingCodec(*unified_codecs, codec))
       continue;
 
     unified_codecs->push_back(codec);
 
     // Add associated RTX codec for recognized codecs.
     // TODO(deadbeef): Should we add RTX codecs for external codecs whose names
     // we don't recognize?
     if (CodecNamesEq(codec.name, kVp8CodecName) ||
@@ skipping 1989 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