Reduce VideoSendStream recreations due to FlexFEC.

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 65833873f68c0e1a6f76edf67cba5fede33405d6..ec0bbcf795fd5c275b201d9265504c4c4d5c5f97 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -43,14 +43,18 @@ using DegradationPreference = webrtc::VideoSendStream::DegradationPreference;
 namespace cricket {
 namespace {
 // If this field trial is enabled, we will enable sending FlexFEC and disable
-// sending ULPFEC whenever the former has been negotiated.
-// FlexFEC can only be negotiated when the "flexfec-03" SDP codec is enabled,
-// which is done by enabling the "WebRTC-FlexFEC-03-Advertised" field trial; see
-// internalencoderfactory.cc.
+// sending ULPFEC whenever the former has been negotiated in the SDPs.
 bool IsFlexfecFieldTrialEnabled() {
   return webrtc::field_trial::IsEnabled("WebRTC-FlexFEC-03");
 }
 
+// If this field trial is enabled, the "flexfec-03" codec may have been
+// advertised as being supported in the local SDP. That means that we must be
+// ready to receive FlexFEC packets. See internalencoderfactory.cc.
+bool IsFlexfecAdvertisedFieldTrialEnabled() {
+  return webrtc::field_trial::IsEnabled("WebRTC-FlexFEC-03-Advertised");
+}
+
 // If this field trial is enabled, we will report VideoContentType RTP extension
 // in capabilities (thus, it will end up in the default SDP and extension will
 // be sent for all key-frames).
@@ -643,12 +647,15 @@ WebRtcVideoChannel2::WebRtcVideoChannel2(
       external_encoder_factory_(external_encoder_factory),
       external_decoder_factory_(external_decoder_factory),
       default_send_options_(options),
-      last_stats_log_ms_(-1) {
+      last_stats_log_ms_(-1),
+      map_flexfec_recv_(IsFlexfecAdvertisedFieldTrialEnabled()),
+      map_flexfec_send_(IsFlexfecFieldTrialEnabled()) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
 
   rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc;
   sending_ = false;
-  recv_codecs_ = MapCodecs(GetSupportedCodecs(external_encoder_factory));
+  recv_codecs_ = MapCodecs(GetSupportedCodecs(external_encoder_factory),
+                           map_flexfec_recv_);
 }
 
 WebRtcVideoChannel2::~WebRtcVideoChannel2() {
@@ -710,7 +717,7 @@ bool WebRtcVideoChannel2::GetChangedSendParameters(
 
   // Select one of the remote codecs that will be used as send codec.
   const rtc::Optional<VideoCodecSettings> selected_send_codec =
-      SelectSendVideoCodec(MapCodecs(params.codecs));
+      SelectSendVideoCodec(MapCodecs(params.codecs, map_flexfec_send_));
 
   if (!selected_send_codec) {
     LOG(LS_ERROR) << "No video codecs supported.";
@@ -951,7 +958,7 @@ bool WebRtcVideoChannel2::GetChangedRecvParameters(
 
   // Handle receive codecs.
   const std::vector<VideoCodecSettings> mapped_codecs =
-      MapCodecs(params.codecs);
+      MapCodecs(params.codecs, map_flexfec_recv_);
   if (mapped_codecs.empty()) {
     LOG(LS_ERROR) << "SetRecvParameters called without any video codecs.";
     return false;
@@ -1121,7 +1128,7 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) {
       call_, sp, std::move(config), default_send_options_,
       external_encoder_factory_, video_config_.enable_cpu_overuse_detection,
       bitrate_config_.max_bitrate_bps, send_codec_, send_rtp_extensions_,
-      send_params_);
+      send_params_, map_flexfec_send_);
 
   uint32_t ssrc = sp.first_ssrc();
   RTC_DCHECK(ssrc != 0);
@@ -1274,7 +1281,8 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp(
   config->rtp.extensions = recv_rtp_extensions_;
 
   // TODO(brandtr): Generalize when we add support for multistream protection.
-  if (sp.GetFecFrSsrc(ssrc, &flexfec_config->remote_ssrc)) {
+  if (map_flexfec_recv_ &&
+      sp.GetFecFrSsrc(ssrc, &flexfec_config->remote_ssrc)) {
     flexfec_config->protected_media_ssrcs = {ssrc};
     flexfec_config->local_ssrc = config->rtp.local_ssrc;
     flexfec_config->rtcp_mode = config->rtp.rtcp_mode;
@@ -1572,7 +1580,8 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
     const rtc::Optional<std::vector<webrtc::RtpExtension>>& rtp_extensions,
     // TODO(deadbeef): Don't duplicate information between send_params,
     // rtp_extensions, options, etc.
-    const VideoSendParameters& send_params)
+    const VideoSendParameters& send_params,
+    bool map_flexfec_send)
     : worker_thread_(rtc::Thread::Current()),
       ssrcs_(sp.ssrcs),
       ssrc_groups_(sp.ssrc_groups),
@@ -1604,13 +1613,13 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::WebRtcVideoSendStream(
   // FlexFEC SSRCs.
   // TODO(brandtr): This code needs to be generalized when we add support for
   // multistream protection.
-  if (IsFlexfecFieldTrialEnabled()) {
+  if (map_flexfec_send) {

[perkj_webrtc, 2017/05/12 12:05:05]

rename variable or keep function call. is_flexfec_field_trial_.

[brandtr, 2017/05/15 12:24:13]

Kept function call.

     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 "
+          LOG(LS_INFO) << "Multiple FlexFEC streams in local SDP, but "
                           "our implementation only supports a single FlexFEC "
                           "stream. Will not enable FlexFEC for proposed "
                           "stream with SSRC: "
@@ -1785,10 +1794,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodec(
     parameters_.config.encoder_settings.internal_source = false;
   }
   parameters_.config.rtp.ulpfec = codec_settings.ulpfec;
-  if (IsFlexfecFieldTrialEnabled()) {
-    parameters_.config.rtp.flexfec.payload_type =
-        codec_settings.flexfec_payload_type;
-  }
+  parameters_.config.rtp.flexfec.payload_type =
+      codec_settings.flexfec_payload_type;
 
   // Set RTX payload type if RTX is enabled.
   if (!parameters_.config.rtp.rtx.ssrcs.empty()) {
@@ -2502,7 +2509,8 @@ bool WebRtcVideoChannel2::VideoCodecSettings::operator!=(
 }
 
 std::vector<WebRtcVideoChannel2::VideoCodecSettings>
-WebRtcVideoChannel2::MapCodecs(const std::vector<VideoCodec>& codecs) {
+WebRtcVideoChannel2::MapCodecs(const std::vector<VideoCodec>& codecs,
+                               bool map_flexfec) {

[perkj_webrtc, 2017/05/12 12:05:05]

remove and use IsFlexfecFieldTrialEnabled() in here instead.

[brandtr, 2017/05/15 12:24:13]

Done.

As was clear from our F2F discussion, however, this optimization is only helpful
for the sender side. So I moved the logic to GetChangedSendParameters instead.
This makes it easier to understand.

   RTC_DCHECK(!codecs.empty());
 
   std::vector<VideoCodecSettings> video_codecs;
@@ -2594,7 +2602,13 @@ 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_payload_type = flexfec_payload_type;
+    // Only map the FlexFEC payload type if we know that FlexFEC will actually
+    // be used. By doing that, we reduce the number of
+    // VideoReceiveStream/VideoSendStream object recreations during SDP
+    // renegotiation.
+    if (map_flexfec) {
+      video_codecs[i].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) {