From ef20795a464d66a69ecdeff55d66c3ceec4e04c1 Mon Sep 17 00:00:00 2001
From: Magnus Jedvert <magjed@webrtc.org>
Date: Wed, 25 Oct 2017 17:08:04 +0200
Subject: [PATCH] Remove FlexFEC feedback params from internal encoder factory

I want to move away from the old encoder factory interface
cricket::WebRtcEncoderFactory to the new webrtc::VideoEncoderFactory. I
created a new webrtc::SdpVideoFormat that essentially is a subset of the
cricket::VideoCodec variables. E.g. the encoder factories shouldn't have
to assign payload types to the codecs, so the payload is not part of
webrtc::SdpVideoFormat. I also didn't add the "feedback_params" that is
used in cricket::VideoCodec to webrtc::SdpVideoFormat. This is causing
problems now, because the internal encoder factory is adding flexfec
feedback params. To avoid this problem, I add these feedback params in
WebRtcVideoEngine instead, like we do for the other codecs.

Bug: webrtc:7925
Change-Id: I7c6ae8d1e1f47f3631c4804c223ec21da8d73685
Reviewed-on: https://webrtc-review.googlesource.com/15223
Commit-Queue: Magnus Jedvert <magjed@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20444}
---
 media/engine/internalencoderfactory.cc |  4 ----
 media/engine/webrtcvideoengine.cc      | 17 ++++++++++-------
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/media/engine/internalencoderfactory.cc b/media/engine/internalencoderfactory.cc
index e55ca91495..23c85cb956 100644
--- a/media/engine/internalencoderfactory.cc
+++ b/media/engine/internalencoderfactory.cc
@@ -51,10 +51,6 @@ InternalEncoderFactory::InternalEncoderFactory() {
     // we never use the actual value anywhere in our code however.
     // TODO(brandtr): Consider honouring this value in the sender and receiver.
     flexfec_codec.SetParam(kFlexfecFmtpRepairWindow, "10000000");
-    flexfec_codec.AddFeedbackParam(
-        FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
-    flexfec_codec.AddFeedbackParam(
-        FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty));
     supported_codecs_.push_back(flexfec_codec);
   }
 }
diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc
index f5c6a2998e..9767aadbc2 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -192,12 +192,18 @@ bool IsFlexfecAdvertisedFieldTrialEnabled() {
 }
 
 void AddDefaultFeedbackParams(VideoCodec* codec) {
-  codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamCcm, kRtcpFbCcmParamFir));
-  codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));
-  codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kRtcpFbNackParamPli));
+  // Don't add any feedback params for RED and ULPFEC.
+  if (codec->name == kRedCodecName || codec->name == kUlpfecCodecName)
+    return;
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty));
   codec->AddFeedbackParam(
       FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
+  // Don't add any more feedback params for FLEXFEC.
+  if (codec->name == kFlexfecCodecName)
+    return;
+  codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamCcm, kRtcpFbCcmParamFir));
+  codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));
+  codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kRtcpFbNackParamPli));
 }
 
 static std::string CodecVectorToString(const std::vector<VideoCodec>& codecs) {
@@ -515,10 +521,7 @@ std::vector<VideoCodec> AssignPayloadTypesAndAddAssociatedRtxCodecs(
   std::vector<VideoCodec> output_codecs;
   for (VideoCodec codec : input_codecs) {
     codec.id = payload_type;
-    if (codec.name != kRedCodecName && codec.name != kUlpfecCodecName &&
-        codec.name != kFlexfecCodecName) {
-      AddDefaultFeedbackParams(&codec);
-    }
+    AddDefaultFeedbackParams(&codec);
     output_codecs.push_back(codec);
 
     // Increment payload type.