Recreate FlexfecReceiveStream separately from VideoReceiveStream.

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 631 matching lines @@
       video_config_(config.video),
       external_encoder_factory_(external_encoder_factory),
       external_decoder_factory_(external_decoder_factory),
       default_send_options_(options),
       last_stats_log_ms_(-1) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
 
   rtcp_receiver_report_ssrc_ = kDefaultRtcpReceiverReportSsrc;
   sending_ = false;
   recv_codecs_ = MapCodecs(GetSupportedCodecs(external_encoder_factory));
+  recv_flexfec_payload_type_ = recv_codecs_.front().flexfec_payload_type;
 }
 
 WebRtcVideoChannel2::~WebRtcVideoChannel2() {
   for (auto& kv : send_streams_)
     delete kv.second;
   for (auto& kv : receive_streams_)
     delete kv.second;
 }
 
 rtc::Optional<WebRtcVideoChannel2::VideoCodecSettings>
 WebRtcVideoChannel2::SelectSendVideoCodec(
     const std::vector<VideoCodecSettings>& remote_mapped_codecs) const {
   const std::vector<VideoCodec> local_supported_codecs =
       GetSupportedCodecs(external_encoder_factory_);
   // Select the first remote codec that is supported locally.
   for (const VideoCodecSettings& remote_mapped_codec : remote_mapped_codecs) {
     // For H264, we will limit the encode level to the remote offered level
     // regardless if level asymmetry is allowed or not. This is strictly not
     // following the spec in https://tools.ietf.org/html/rfc6184#section-8.2.2
     // since we should limit the encode level to the lower of local and remote
     // level when level asymmetry is not allowed.
     if (FindMatchingCodec(local_supported_codecs, remote_mapped_codec.codec))
       return rtc::Optional<VideoCodecSettings>(remote_mapped_codec);
   }
   // No remote codec was supported.
   return rtc::Optional<VideoCodecSettings>();
 }
 
-bool WebRtcVideoChannel2::ReceiveCodecsHaveChanged(
+bool WebRtcVideoChannel2::NonFlexfecReceiveCodecsHaveChanged(
     std::vector<VideoCodecSettings> before,
     std::vector<VideoCodecSettings> after) {
   if (before.size() != after.size()) {
     return true;
   }
+
   // The receive codec order doesn't matter, so we sort the codecs before
   // comparing. This is necessary because currently the
   // only way to change the send codec is to munge SDP, which causes
   // the receive codec list to change order, which causes the streams
   // to be recreates which causes a "blink" of black video.  In order
   // to support munging the SDP in this way without recreating receive
   // streams, we ignore the order of the received codecs so that
   // changing the order doesn't cause this "blink".
   auto comparison =
       [](const VideoCodecSettings& codec1, const VideoCodecSettings& codec2) {
         return codec1.codec.id > codec2.codec.id;
       };
   std::sort(before.begin(), before.end(), comparison);
   std::sort(after.begin(), after.end(), comparison);
-  return before != after;
+
+  // Changes in FlexFEC payload type are handled separately in
+  // WebRtcVideoChannel2::GetChangedRecvParameters, so disregard FlexFEC in the
+  // comparison here.
+  return !std::equal(before.begin(), before.end(), after.begin(),
+                     VideoCodecSettings::EqualsDisregardingFlexfec);
 }
 
 bool WebRtcVideoChannel2::GetChangedSendParameters(
     const VideoSendParameters& params,
     ChangedSendParameters* changed_params) const {
   if (!ValidateCodecFormats(params.codecs) ||
       !ValidateRtpExtensions(params.extensions)) {
     return false;
   }
 
@@ skipping 259 matching lines @@
   const std::vector<VideoCodec> local_supported_codecs =
       GetSupportedCodecs(external_encoder_factory_);
   for (const VideoCodecSettings& mapped_codec : mapped_codecs) {
     if (!FindMatchingCodec(local_supported_codecs, mapped_codec.codec)) {
       LOG(LS_ERROR) << "SetRecvParameters called with unsupported video codec: "
                     << mapped_codec.codec.ToString();
       return false;
     }
   }
 
-  if (ReceiveCodecsHaveChanged(recv_codecs_, mapped_codecs)) {
+  if (NonFlexfecReceiveCodecsHaveChanged(recv_codecs_, mapped_codecs)) {
     changed_params->codec_settings =
         rtc::Optional<std::vector<VideoCodecSettings>>(mapped_codecs);
   }
 
   // Handle RTP header extensions.
   std::vector<webrtc::RtpExtension> filtered_extensions = FilterRtpExtensions(
       params.extensions, webrtc::RtpExtension::IsSupportedForVideo, false);
   if (filtered_extensions != recv_rtp_extensions_) {
     changed_params->rtp_header_extensions =
         rtc::Optional<std::vector<webrtc::RtpExtension>>(filtered_extensions);
   }
 
+  int flexfec_payload_type = mapped_codecs.front().flexfec_payload_type;
+  if (flexfec_payload_type != recv_flexfec_payload_type_) {
+    changed_params->flexfec_payload_type =
+        rtc::Optional<int>(flexfec_payload_type);
+  }
+
   return true;
 }
 
 bool WebRtcVideoChannel2::SetRecvParameters(const VideoRecvParameters& params) {
   TRACE_EVENT0("webrtc", "WebRtcVideoChannel2::SetRecvParameters");
   LOG(LS_INFO) << "SetRecvParameters: " << params.ToString();
   ChangedRecvParameters changed_params;
   if (!GetChangedRecvParameters(params, &changed_params)) {
     return false;
   }
+  if (changed_params.flexfec_payload_type) {
+    LOG(LS_INFO) << "Changing FlexFEC payload type (recv) from "
+                 << recv_flexfec_payload_type_ << " to "
+                 << *changed_params.flexfec_payload_type;
+    recv_flexfec_payload_type_ = *changed_params.flexfec_payload_type;
+  }
   if (changed_params.rtp_header_extensions) {
     recv_rtp_extensions_ = *changed_params.rtp_header_extensions;
   }
   if (changed_params.codec_settings) {
     LOG(LS_INFO) << "Changing recv codecs from "
                  << CodecSettingsVectorToString(recv_codecs_) << " to "
                  << CodecSettingsVectorToString(*changed_params.codec_settings);
     recv_codecs_ = *changed_params.codec_settings;
   }
 
@@ skipping 263 matching lines @@
 
   config->rtp.remb = send_codec_ ? HasRemb(send_codec_->codec) : false;
   config->rtp.transport_cc =
       send_codec_ ? HasTransportCc(send_codec_->codec) : false;
 
   sp.GetFidSsrc(ssrc, &config->rtp.rtx_ssrc);
 
   config->rtp.extensions = recv_rtp_extensions_;
 
   // TODO(brandtr): Generalize when we add support for multistream protection.
+  flexfec_config->payload_type = recv_flexfec_payload_type_;
   if (IsFlexfecAdvertisedFieldTrialEnabled() &&
       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;
     // TODO(brandtr): We should be spec-compliant and set |transport_cc| here
     // based on the rtcp-fb for the FlexFEC codec, not the media codec.
     flexfec_config->transport_cc = config->rtp.transport_cc;
     flexfec_config->rtp_header_extensions = config->rtp.extensions;
   }
@@ skipping 154 matching lines @@
   }
 
   // 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 (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.flexfec_payload_type) {
+        payload_type == codec.ulpfec.ulpfec_payload_type) {
       return;
     }
   }
+  if (payload_type == recv_flexfec_payload_type_) {
+    return;
+  }
 
   switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc)) {
     case UnsignalledSsrcHandler::kDropPacket:
       return;
     case UnsignalledSsrcHandler::kDeliverPacket:
       break;
   }
 
   if (call_->Receiver()->DeliverPacket(
           webrtc::MediaType::VIDEO,
@@ skipping 698 matching lines @@
       config_(std::move(config)),
       flexfec_config_(flexfec_config),
       flexfec_stream_(nullptr),
       external_decoder_factory_(external_decoder_factory),
       sink_(NULL),
       first_frame_timestamp_(-1),
       estimated_remote_start_ntp_time_ms_(0) {
   config_.renderer = this;
   std::vector<AllocatedDecoder> old_decoders;
   ConfigureCodecs(recv_codecs, &old_decoders);
-  RecreateWebRtcStream();
+  ConfigureFlexfecCodec(flexfec_config.payload_type);
+  MaybeRecreateWebRtcFlexfecStream();
+  RecreateWebRtcVideoStream();
   RTC_DCHECK(old_decoders.empty());
 }
 
 WebRtcVideoChannel2::WebRtcVideoReceiveStream::AllocatedDecoder::
     AllocatedDecoder(webrtc::VideoDecoder* decoder,
                      webrtc::VideoCodecType type,
                      bool external)
     : decoder(decoder),
       external_decoder(nullptr),
       type(type),
@@ skipping 81 matching lines @@
     config_.decoders.push_back(decoder);
   }
 
   config_.rtp.rtx_payload_types.clear();
   for (const VideoCodecSettings& recv_codec : recv_codecs) {
     config_.rtp.rtx_payload_types[recv_codec.codec.id] =
         recv_codec.rtx_payload_type;
   }
 
   config_.rtp.ulpfec = recv_codecs.front().ulpfec;
-  flexfec_config_.payload_type = recv_codecs.front().flexfec_payload_type;
 
   config_.rtp.nack.rtp_history_ms =
       HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0;
 }
 
+void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ConfigureFlexfecCodec(
+    int flexfec_payload_type) {
+  flexfec_config_.payload_type = flexfec_payload_type;
+}
+
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc(
     uint32_t local_ssrc) {
   // TODO(pbos): Consider turning this sanity check into a RTC_DCHECK. You
   // should not be able to create a sender with the same SSRC as a receiver, but
   // right now this can't be done due to unittests depending on receiving what
   // they are sending from the same MediaChannel.
   if (local_ssrc == config_.rtp.remote_ssrc) {
     LOG(LS_INFO) << "Ignoring call to SetLocalSsrc because parameters are "
                     "unchanged; local_ssrc=" << local_ssrc;
     return;
   }
 
   config_.rtp.local_ssrc = local_ssrc;
   flexfec_config_.local_ssrc = local_ssrc;
   LOG(LS_INFO)
       << "RecreateWebRtcStream (recv) because of SetLocalSsrc; local_ssrc="
       << local_ssrc;
-  RecreateWebRtcStream();
+  MaybeRecreateWebRtcFlexfecStream();
+  RecreateWebRtcVideoStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters(
     bool nack_enabled,
     bool remb_enabled,
     bool transport_cc_enabled,
     webrtc::RtcpMode rtcp_mode) {
   int nack_history_ms = nack_enabled ? kNackHistoryMs : 0;
   if (config_.rtp.nack.rtp_history_ms == nack_history_ms &&
       config_.rtp.remb == remb_enabled &&
@@ skipping 11 matching lines @@
   config_.rtp.transport_cc = transport_cc_enabled;
   config_.rtp.rtcp_mode = rtcp_mode;
   // TODO(brandtr): We should be spec-compliant and set |transport_cc| here
   // based on the rtcp-fb for the FlexFEC codec, not the media codec.
   flexfec_config_.transport_cc = config_.rtp.transport_cc;
   flexfec_config_.rtcp_mode = config_.rtp.rtcp_mode;
   LOG(LS_INFO)
       << "RecreateWebRtcStream (recv) because of SetFeedbackParameters; nack="
       << nack_enabled << ", remb=" << remb_enabled
       << ", transport_cc=" << transport_cc_enabled;
-  RecreateWebRtcStream();
+  MaybeRecreateWebRtcFlexfecStream();
+  RecreateWebRtcVideoStream();
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters(
     const ChangedRecvParameters& params) {
-  bool needs_recreation = false;
+  bool video_needs_recreation = false;
+  bool flexfec_needs_recreation = false;
   std::vector<AllocatedDecoder> old_decoders;
   if (params.codec_settings) {
     ConfigureCodecs(*params.codec_settings, &old_decoders);
-    needs_recreation = true;
+    video_needs_recreation = true;
   }
   if (params.rtp_header_extensions) {
     config_.rtp.extensions = *params.rtp_header_extensions;
     flexfec_config_.rtp_header_extensions = *params.rtp_header_extensions;
-    needs_recreation = true;
+    video_needs_recreation = true;
+    flexfec_needs_recreation = true;
   }
-  if (needs_recreation) {
-    LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetRecvParameters";
-    RecreateWebRtcStream();
+  if (params.flexfec_payload_type) {
+    ConfigureFlexfecCodec(*params.flexfec_payload_type);
+    flexfec_needs_recreation = true;
+  }
+  if (flexfec_needs_recreation) {
+    LOG(LS_INFO) << "MaybeRecreateWebRtcFlexfecStream (recv) because of "
+                    "SetRecvParameters";
+    MaybeRecreateWebRtcFlexfecStream();
+  }
+  if (video_needs_recreation) {
+    LOG(LS_INFO)
+        << "RecreateWebRtcVideoStream (recv) because of SetRecvParameters";
+    RecreateWebRtcVideoStream();
     ClearDecoders(&old_decoders);
   }
 }
 
-void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() {
+void WebRtcVideoChannel2::WebRtcVideoReceiveStream::
+    RecreateWebRtcVideoStream() {
   if (stream_) {
     call_->DestroyVideoReceiveStream(stream_);
     stream_ = nullptr;
   }
+  // TODO(nisse): There are way too many copies here. And why isn't
+  // the argument to CreateVideoReceiveStream a const ref?
+  webrtc::VideoReceiveStream::Config config = config_.Copy();
+  config.rtp.protected_by_flexfec = (flexfec_stream_ != nullptr);
+  stream_ = call_->CreateVideoReceiveStream(config.Copy());

[perkj_webrtc, 2017/05/30 05:49:54]

why config.copy? The config supports move.

[brandtr, 2017/05/30 07:57:34]

Done.

+  stream_->Start();
+}
+
+void WebRtcVideoChannel2::WebRtcVideoReceiveStream::
+    MaybeRecreateWebRtcFlexfecStream() {
   if (flexfec_stream_) {
     call_->DestroyFlexfecReceiveStream(flexfec_stream_);
     flexfec_stream_ = nullptr;
   }
-  const bool use_flexfec = flexfec_config_.IsCompleteAndEnabled();
-  // TODO(nisse): There are way too many copies here. And why isn't
-  // the argument to CreateVideoReceiveStream a const ref?
-  webrtc::VideoReceiveStream::Config config = config_.Copy();
-  config.rtp.protected_by_flexfec = use_flexfec;
-  stream_ = call_->CreateVideoReceiveStream(config.Copy());
-  stream_->Start();
-
-  if (use_flexfec) {
+  if (flexfec_config_.IsCompleteAndEnabled()) {
     flexfec_stream_ = call_->CreateFlexfecReceiveStream(flexfec_config_);
     flexfec_stream_->Start();
   }
 }
 
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::ClearDecoders(
     std::vector<AllocatedDecoder>* allocated_decoders) {
   for (size_t i = 0; i < allocated_decoders->size(); ++i) {
     if ((*allocated_decoders)[i].external) {
       external_decoder_factory_->DestroyVideoDecoder(
@@ skipping 106 matching lines @@
 WebRtcVideoChannel2::VideoCodecSettings::VideoCodecSettings()
     : flexfec_payload_type(-1), rtx_payload_type(-1) {}
 
 bool WebRtcVideoChannel2::VideoCodecSettings::operator==(
     const WebRtcVideoChannel2::VideoCodecSettings& other) const {
   return codec == other.codec && ulpfec == other.ulpfec &&
          flexfec_payload_type == other.flexfec_payload_type &&
          rtx_payload_type == other.rtx_payload_type;
 }
 
+bool WebRtcVideoChannel2::VideoCodecSettings::EqualsDisregardingFlexfec(
+    const WebRtcVideoChannel2::VideoCodecSettings& a,
+    const WebRtcVideoChannel2::VideoCodecSettings& b) {
+  return a.codec == b.codec && a.ulpfec == b.ulpfec &&
+         a.rtx_payload_type == b.rtx_payload_type;
+}
+
 bool WebRtcVideoChannel2::VideoCodecSettings::operator!=(
     const WebRtcVideoChannel2::VideoCodecSettings& other) const {
   return !(*this == other);
 }
 
 std::vector<WebRtcVideoChannel2::VideoCodecSettings>
 WebRtcVideoChannel2::MapCodecs(const std::vector<VideoCodec>& codecs) {
   RTC_DCHECK(!codecs.empty());
 
   std::vector<VideoCodecSettings> video_codecs;
@@ skipping 90 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