Wire up FlexFEC in VideoEngine2.

webrtc/media/engine/webrtcvideoengine2.cc

Index: webrtc/media/engine/webrtcvideoengine2.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index 646ed8e4476033081b24738cf10babb1a319fb02..e34e884a59b126f7bbc1fc53308e5f90c3dd97da 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -38,6 +38,18 @@
 namespace cricket {
 namespace {
 
+// Three things happen when the FlexFEC field trial is enabled:
+// 1) FlexFEC is exposed in the default codec list, eventually showing up
+//    in the default SDP.
+// 2) FlexFEC send parameters are set in the VideoSendStream config.
+// 3) FlexFEC receive parameters are set in the FlexfecReceiveStream config,
+//    and the corresponding object is instantiated.
+const char kFlexfecFieldTrialName[] = "WebRTC-FlexFEC-03";
+
+bool IsFlexfecEnabled() {
+  return webrtc::field_trial::FindFullName(kFlexfecFieldTrialName) == "Enabled";
+}
+
 // Wrap cricket::WebRtcVideoEncoderFactory as a webrtc::VideoEncoderFactory.
 class EncoderFactoryAdapter : public webrtc::VideoEncoderFactory {
  public:
@@ -639,6 +651,21 @@ static std::vector<VideoCodec> GetSupportedCodecs(
                  << CodecVectorToString(external_codecs);
   }
 
+  // TODO(brandtr): Move this block of code to InternalEncoderFactory ctor
+  // when FlexFEC is no longer behind a field trial. Until then, we need
+  // to have this block here, since the singleton encoder factory does
+  // not respect the ScopedFieldTrials that we use in the tests.

[brandtr, 2016/11/17 17:28:58]

magjed: Is this OK?

Further, checking the field trial in the InternalEncoderFactory ctor may not be
a good idea if the application wants to control the field trial on a per-call
basis.

[magjed_webrtc, 2016/11/18 14:28:00]

This won't work because the payload won't be assigned (it will be 0).
I have relanded my CL and removed the singleton pattern from
InternalEncoderFactory so that you can put the field trial in the ctor.

[brandtr, 2016/11/21 08:52:03]

I see, moving to the ctor. (Thanks for changing your CL to accomodate this!)

Btw, should I check in the tests that the generated payload type is valid?

+  if (IsFlexfecEnabled()) {
+    cricket::VideoCodec flexfec_codec(kFlexfecCodecName);
+    // This value is currently arbitrarily set to 10 seconds. (The unit
+    // is microseconds.) This parameter MUST be present in the SDP, but
+    // 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");
+    unified_codecs.push_back(flexfec_codec);
+    LOG(LS_INFO) << "Codec added by field trial: " << flexfec_codec.ToString();
+  }
+
   return unified_codecs;
 }
 
@@ -1196,7 +1223,8 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
     receive_ssrcs_.insert(used_ssrc);
 
   webrtc::VideoReceiveStream::Config config(this);
-  ConfigureReceiverRtp(&config, sp);
+  webrtc::FlexfecConfig flexfec_config;
+  ConfigureReceiverRtp(&config, &flexfec_config, sp);
 
   // Set up A/V sync group based on sync label.
   config.sync_group = sp.sync_label;
@@ -1209,13 +1237,14 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
 
   receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
       call_, sp, std::move(config), external_decoder_factory_, default_stream,
-      recv_codecs_);
+      recv_codecs_, flexfec_config);
 
   return true;
 }
 
 void WebRtcVideoChannel2::ConfigureReceiverRtp(
     webrtc::VideoReceiveStream::Config* config,
+    webrtc::FlexfecConfig* flexfec_config,
     const StreamParams& sp) const {
   uint32_t ssrc = sp.first_ssrc();
 
@@ -1254,6 +1283,14 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp(
       rtx.payload_type = recv_codecs_[i].rtx_payload_type;
     }
   }
+
+  // TODO(brandtr): This code needs to be generalized when we add support for
+  // multistream protection.
+  uint32_t flexfec_ssrc;
+  if (sp.GetFecFrSsrc(ssrc, &flexfec_ssrc)) {
+    flexfec_config->flexfec_ssrc = flexfec_ssrc;
+    flexfec_config->protected_media_ssrcs = {ssrc};
+  }
 }
 
 bool WebRtcVideoChannel2::RemoveRecvStream(uint32_t ssrc) {
@@ -1411,14 +1448,15 @@ void WebRtcVideoChannel2::OnPacketReceived(
   }
 
   // See if this payload_type is registered as one that usually gets its own
-  // SSRC (RTX) or at least is safe to drop either way (ULPFEC). If it is, and
+  // SSRC (RTX) or at least is safe to drop either way (FEC). If it is, and
   // it wasn't handled above by DeliverPacket, that means we don't know what
   // stream it associates with, and we shouldn't ever create an implicit channel
   // for these.
   for (auto& codec : recv_codecs_) {
     if (payload_type == codec.rtx_payload_type ||
         payload_type == codec.ulpfec.red_rtx_payload_type ||
-        payload_type == codec.ulpfec.ulpfec_payload_type) {
+        payload_type == codec.ulpfec.ulpfec_payload_type ||
+        payload_type == codec.flexfec.flexfec_payload_type) {
       return;
     }
   }
@@ -1561,8 +1599,35 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
   parameters_.conference_mode = send_params.conference_mode;
 
   sp.GetPrimarySsrcs(&parameters_.config.rtp.ssrcs);
+
+  // RTX.
   sp.GetFidSsrcs(parameters_.config.rtp.ssrcs,
                  &parameters_.config.rtp.rtx.ssrcs);
+
+  // FlexFEC.
+  // TODO(brandtr): This code needs to be generalized when we add support for
+  // multistream protection.
+  if (IsFlexfecEnabled()) {
+    uint32_t flexfec_ssrc;
+    bool flexfec_enabled = false;
+    for (uint32_t primary_ssrc : parameters_.config.rtp.ssrcs) {
+      if (sp.GetFecFrSsrc(primary_ssrc, &flexfec_ssrc)) {
+        if (flexfec_enabled) {
+          LOG(LS_INFO) << "Multiple FlexFEC streams proposed by remote, but "
+                          "our implementation only supports a single FlexFEC "
+                          "stream. Will not enable FlexFEC for proposed "
+                          "stream with SSRC: "
+                       << flexfec_ssrc << ".";
+          continue;
+        }
+
+        flexfec_enabled = true;
+        parameters_.config.rtp.flexfec.flexfec_ssrc = flexfec_ssrc;
+        parameters_.config.rtp.flexfec.protected_media_ssrcs = {primary_ssrc};
+      }
+    }
+  }
+
   parameters_.config.rtp.c_name = sp.cname;
   if (rtp_extensions) {
     parameters_.config.rtp.extensions = *rtp_extensions;
@@ -1744,6 +1809,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodec(
         external_encoder_factory_->EncoderTypeHasInternalSource(type);
   }
   parameters_.config.rtp.ulpfec = codec_settings.ulpfec;
+  parameters_.config.rtp.flexfec.flexfec_payload_type =
+      codec_settings.flexfec.flexfec_payload_type;
 
   // Set RTX payload type if RTX is enabled.
   if (!parameters_.config.rtp.rtx.ssrcs.empty()) {
@@ -2119,12 +2186,15 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(
     webrtc::VideoReceiveStream::Config config,
     WebRtcVideoDecoderFactory* external_decoder_factory,
     bool default_stream,
-    const std::vector<VideoCodecSettings>& recv_codecs)
+    const std::vector<VideoCodecSettings>& recv_codecs,
+    const webrtc::FlexfecConfig& flexfec_config)
     : call_(call),
       stream_params_(sp),
       stream_(NULL),
       default_stream_(default_stream),
       config_(std::move(config)),
+      flexfec_config_(flexfec_config),
+      flexfec_stream_(nullptr),
       external_decoder_factory_(external_decoder_factory),
       sink_(NULL),
       first_frame_timestamp_(-1),
@@ -2253,6 +2323,8 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureCodecs(
 
   // TODO(pbos): Reconfigure RTX based on incoming recv_codecs.
   config_.rtp.ulpfec = recv_codecs.front().ulpfec;
+  flexfec_config_.flexfec_payload_type =
+      recv_codecs.front().flexfec.flexfec_payload_type;
   config_.rtp.nack.rtp_history_ms =
       HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0;
 }
@@ -2324,11 +2396,19 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {
-  if (stream_ != NULL) {
+  if (flexfec_stream_) {
+    call_->DestroyFlexfecReceiveStream(flexfec_stream_);
+    flexfec_stream_ = nullptr;
+  }
+  if (stream_) {
     call_->DestroyVideoReceiveStream(stream_);
   }
   stream_ = call_->CreateVideoReceiveStream(config_.Copy());
   stream_->Start();
+  if (IsFlexfecEnabled() && flexfec_config_.IsCompleteAndEnabled()) {
+    flexfec_stream_ = call_->CreateFlexfecReceiveStream(flexfec_config_);
+    flexfec_stream_->Start();
+  }
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ClearDecoders(
@@ -2443,11 +2523,8 @@ WebRtcVideoChannel2::VideoCodecSettings::VideoCodecSettings()
 
 bool WebRtcVideoChannel2::VideoCodecSettings::operator==(
     const WebRtcVideoChannel2::VideoCodecSettings& other) const {
-  return codec == other.codec &&
-         ulpfec.ulpfec_payload_type == other.ulpfec.ulpfec_payload_type &&
-         ulpfec.red_payload_type == other.ulpfec.red_payload_type &&
-         ulpfec.red_rtx_payload_type == other.ulpfec.red_rtx_payload_type &&
-         rtx_payload_type == other.rtx_payload_type;
+  return codec == other.codec && ulpfec == other.ulpfec &&
+         flexfec == other.flexfec && rtx_payload_type == other.rtx_payload_type;
 }
 
 bool WebRtcVideoChannel2::VideoCodecSettings::operator!=(
@@ -2466,6 +2543,7 @@ WebRtcVideoChannel2::MapCodecs(const std::vector<VideoCodec>& codecs) {
   std::map<int, int> rtx_mapping;
 
   webrtc::UlpfecConfig ulpfec_config;
+  int flexfec_payload_type = -1;
 
   for (size_t i = 0; i < codecs.size(); ++i) {
     const VideoCodec& in_codec = codecs[i];
@@ -2482,20 +2560,22 @@ WebRtcVideoChannel2::MapCodecs(const std::vector<VideoCodec>& codecs) {
     switch (in_codec.GetCodecType()) {
       case VideoCodec::CODEC_RED: {
         // RED payload type, should not have duplicates.
-        RTC_DCHECK(ulpfec_config.red_payload_type == -1);
+        RTC_DCHECK_EQ(-1, ulpfec_config.red_payload_type);
         ulpfec_config.red_payload_type = in_codec.id;
         continue;
       }
 
       case VideoCodec::CODEC_ULPFEC: {
         // ULPFEC payload type, should not have duplicates.
-        RTC_DCHECK(ulpfec_config.ulpfec_payload_type == -1);
+        RTC_DCHECK_EQ(-1, ulpfec_config.ulpfec_payload_type);
         ulpfec_config.ulpfec_payload_type = in_codec.id;
         continue;
       }
 
       case VideoCodec::CODEC_FLEXFEC: {
-        // TODO(brandtr): To be implemented.
+        // FlexFEC payload type, should not have duplicates.
+        RTC_DCHECK_EQ(-1, flexfec_payload_type);
+        flexfec_payload_type = in_codec.id;
         continue;
       }
 
@@ -2545,6 +2625,7 @@ WebRtcVideoChannel2::MapCodecs(const std::vector<VideoCodec>& codecs) {
 
   for (size_t i = 0; i < video_codecs.size(); ++i) {
     video_codecs[i].ulpfec = ulpfec_config;
+    video_codecs[i].flexfec.flexfec_payload_type = flexfec_payload_type;
     if (rtx_mapping[video_codecs[i].codec.id] != 0 &&
         rtx_mapping[video_codecs[i].codec.id] !=
             ulpfec_config.red_payload_type) {