Fixing some issues with ICE restart signaling.

talk/session/media/mediasession.cc

 /*
  * libjingle
  * Copyright 2004 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 498 matching lines @@
       // Store the new StreamParams in current_streams.
       // This is necessary so that we can use the CNAME for other media types.
       current_streams->push_back(stream_param);
     } else {
       content_description->AddStream(*param);
     }
   }
   return true;
 }
 
-// Updates the transport infos of the |sdesc| according to the given
-// |bundle_group|. The transport infos of the content names within the
-// |bundle_group| should be updated to use the ufrag and pwd of the first
-// content within the |bundle_group|.
-static bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group,
-                                         SessionDescription* sdesc) {
-  // The bundle should not be empty.
-  if (!sdesc || !bundle_group.FirstContentName()) {
-    return false;
-  }
-
-  // We should definitely have a transport for the first content.
-  const std::string& selected_content_name = *bundle_group.FirstContentName();
-  const TransportInfo* selected_transport_info =
-      sdesc->GetTransportInfoByName(selected_content_name);
-  if (!selected_transport_info) {
-    return false;
-  }
-
-  // Set the other contents to use the same ICE credentials.
-  const std::string& selected_ufrag =
-      selected_transport_info->description.ice_ufrag;
-  const std::string& selected_pwd =
-      selected_transport_info->description.ice_pwd;
-  for (TransportInfos::iterator it =
-           sdesc->transport_infos().begin();
-       it != sdesc->transport_infos().end(); ++it) {
-    if (bundle_group.HasContentName(it->content_name) &&
-        it->content_name != selected_content_name) {
-      it->description.ice_ufrag = selected_ufrag;
-      it->description.ice_pwd = selected_pwd;
-    }
-  }
-  return true;
-}
-
 // Gets the CryptoParamsVec of the given |content_name| from |sdesc|, and
 // sets it to |cryptos|.
 static bool GetCryptosByName(const SessionDescription* sdesc,
                              const std::string& content_name,
                              CryptoParamsVec* cryptos) {
   if (!sdesc || !cryptos) {
     return false;
   }
 
   const ContentInfo* content = sdesc->GetContentByName(content_name);
@@ skipping 722 matching lines @@
 
   // Bundle the contents together, if we've been asked to do so, and update any
   // parameters that need to be tweaked for BUNDLE.
   if (options.bundle_enabled) {
     ContentGroup offer_bundle(GROUP_TYPE_BUNDLE);
     for (ContentInfos::const_iterator content = offer->contents().begin();
        content != offer->contents().end(); ++content) {
       offer_bundle.AddContentName(content->name);
     }
     offer->AddGroup(offer_bundle);
-    if (!UpdateTransportInfoForBundle(offer_bundle, offer.get())) {
-      LOG(LS_ERROR) << "CreateOffer failed to UpdateTransportInfoForBundle.";
-      return NULL;
-    }
     if (!UpdateCryptoParamsForBundle(offer_bundle, offer.get())) {
       LOG(LS_ERROR) << "CreateOffer failed to UpdateCryptoParamsForBundle.";
       return NULL;
     }
   }
 
   return offer.release();
 }
 
 SessionDescription* MediaSessionDescriptionFactory::CreateAnswer(
@@ skipping 36 matching lines @@
     const ContentGroup* offer_bundle = offer->GetGroupByName(GROUP_TYPE_BUNDLE);
     ContentGroup answer_bundle(GROUP_TYPE_BUNDLE);
     for (ContentInfos::const_iterator content = answer->contents().begin();
        content != answer->contents().end(); ++content) {
       if (!content->rejected && offer_bundle->HasContentName(content->name)) {
         answer_bundle.AddContentName(content->name);
       }
     }
     if (answer_bundle.FirstContentName()) {
       answer->AddGroup(answer_bundle);
-
-      // Share the same ICE credentials and crypto params across all contents,
-      // as BUNDLE requires.

[Taylor Brandstetter, 2015/11/18 22:40:17]

Actually, BUNDLE doesn't require this, as far as I can tell. And this code
introduces bugs sometimes. So I just removed it. Didn't seem to break any unit
tests.

-      if (!UpdateTransportInfoForBundle(answer_bundle, answer.get())) {
-        LOG(LS_ERROR) << "CreateAnswer failed to UpdateTransportInfoForBundle.";
-        return NULL;
-      }
-
       if (!UpdateCryptoParamsForBundle(answer_bundle, answer.get())) {
         LOG(LS_ERROR) << "CreateAnswer failed to UpdateCryptoParamsForBundle.";
         return NULL;
       }
     }
   }
 
   return answer.release();
 }
 
@@ skipping 71 matching lines @@
 
   // Add our default RTP header extensions that are not in
   // |current_description|.
   FindRtpHdrExtsToOffer(audio_rtp_header_extensions(), audio_extensions,
                         &all_extensions, &used_ids);
   FindRtpHdrExtsToOffer(video_rtp_header_extensions(), video_extensions,
                         &all_extensions, &used_ids);
 }
 
 bool MediaSessionDescriptionFactory::AddTransportOffer(
-  const std::string& content_name,
-  const TransportOptions& transport_options,
-  const SessionDescription* current_desc,
-  SessionDescription* offer_desc) const {
+    const std::string& content_name,
+    const TransportOptions& transport_options,
+    bool ice_restart,
+    const SessionDescription* current_desc,
+    SessionDescription* offer_desc) const {
   if (!transport_desc_factory_)
      return false;
   const TransportDescription* current_tdesc =
       GetTransportDescription(content_name, current_desc);
   rtc::scoped_ptr<TransportDescription> new_tdesc(
-      transport_desc_factory_->CreateOffer(transport_options, current_tdesc));
+      transport_desc_factory_->CreateOffer(transport_options, ice_restart,
+                                           current_tdesc));
   bool ret = (new_tdesc.get() != NULL &&
       offer_desc->AddTransportInfo(TransportInfo(content_name, *new_tdesc)));
   if (!ret) {
     LOG(LS_ERROR)
         << "Failed to AddTransportOffer, content name=" << content_name;
   }
   return ret;
 }
 
 TransportDescription* MediaSessionDescriptionFactory::CreateTransportAnswer(
     const std::string& content_name,
     const SessionDescription* offer_desc,
     const TransportOptions& transport_options,
+    bool ice_restart,
     const SessionDescription* current_desc) const {
   if (!transport_desc_factory_)
     return NULL;
   const TransportDescription* offer_tdesc =
       GetTransportDescription(content_name, offer_desc);
   const TransportDescription* current_tdesc =
       GetTransportDescription(content_name, current_desc);
-  return
-      transport_desc_factory_->CreateAnswer(offer_tdesc, transport_options,
-                                            current_tdesc);
+  return transport_desc_factory_->CreateAnswer(offer_tdesc, transport_options,
+                                               ice_restart, current_tdesc);
 }
 
 bool MediaSessionDescriptionFactory::AddTransportAnswer(
     const std::string& content_name,
     const TransportDescription& transport_desc,
     SessionDescription* answer_desc) const {
   if (!answer_desc->AddTransportInfo(TransportInfo(content_name,
                                                    transport_desc))) {
     LOG(LS_ERROR)
         << "Failed to AddTransportAnswer, content name=" << content_name;
@@ skipping 42 matching lines @@
   } else {
     if (options.recv_audio) {
       audio->set_direction(MD_RECVONLY);
     } else {
       audio->set_direction(MD_INACTIVE);
     }
   }
 
   desc->AddContent(CN_AUDIO, NS_JINGLE_RTP, audio.release());
   if (!AddTransportOffer(CN_AUDIO, options.transport_options,
-                         current_description, desc)) {
+                         options.audio_ice_restart, current_description,
+                         desc)) {
     return false;
   }
 
   return true;
 }
 
 bool MediaSessionDescriptionFactory::AddVideoContentForOffer(
     const MediaSessionOptions& options,
     const SessionDescription* current_description,
     const RtpHeaderExtensions& video_rtp_extensions,
@@ skipping 34 matching lines @@
   } else {
     if (options.recv_video) {
       video->set_direction(MD_RECVONLY);
     } else {
       video->set_direction(MD_INACTIVE);
     }
   }
 
   desc->AddContent(CN_VIDEO, NS_JINGLE_RTP, video.release());
   if (!AddTransportOffer(CN_VIDEO, options.transport_options,
-                         current_description, desc)) {
+                         options.video_ice_restart, current_description,
+                         desc)) {
     return false;
   }
 
   return true;
 }
 
 bool MediaSessionDescriptionFactory::AddDataContentForOffer(
     const MediaSessionOptions& options,
     const SessionDescription* current_description,
     DataCodecs* data_codecs,
@@ skipping 38 matching lines @@
   }
 
   if (is_sctp) {
     desc->AddContent(CN_DATA, NS_JINGLE_DRAFT_SCTP, data.release());
   } else {
     data->set_bandwidth(options.data_bandwidth);
     SetMediaProtocol(secure_transport, data.get());
     desc->AddContent(CN_DATA, NS_JINGLE_RTP, data.release());
   }
   if (!AddTransportOffer(CN_DATA, options.transport_options,
-                         current_description, desc)) {
+                         options.data_ice_restart, current_description, desc)) {
     return false;
   }
   return true;
 }
 
 bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
     const SessionDescription* offer,
     const MediaSessionOptions& options,
     const SessionDescription* current_description,
     StreamParamsVec* current_streams,
     SessionDescription* answer) const {
   const ContentInfo* audio_content = GetFirstAudioContent(offer);
 
-  scoped_ptr<TransportDescription> audio_transport(
-      CreateTransportAnswer(audio_content->name, offer,
-                            options.transport_options,
-                            current_description));
+  scoped_ptr<TransportDescription> audio_transport(CreateTransportAnswer(
+      audio_content->name, offer, options.transport_options,
+      options.audio_ice_restart, current_description));
   if (!audio_transport) {
     return false;
   }
 
   AudioCodecs audio_codecs = audio_codecs_;
   if (!options.vad_enabled) {
     StripCNCodecs(&audio_codecs);
   }
 
   bool bundle_enabled =
@@ skipping 35 matching lines @@
   return true;
 }
 
 bool MediaSessionDescriptionFactory::AddVideoContentForAnswer(
     const SessionDescription* offer,
     const MediaSessionOptions& options,
     const SessionDescription* current_description,
     StreamParamsVec* current_streams,
     SessionDescription* answer) const {
   const ContentInfo* video_content = GetFirstVideoContent(offer);
-  scoped_ptr<TransportDescription> video_transport(
-      CreateTransportAnswer(video_content->name, offer,
-                            options.transport_options,
-                            current_description));
+  scoped_ptr<TransportDescription> video_transport(CreateTransportAnswer(
+      video_content->name, offer, options.transport_options,
+      options.video_ice_restart, current_description));
   if (!video_transport) {
     return false;
   }
 
   scoped_ptr<VideoContentDescription> video_answer(
       new VideoContentDescription());
   // Do not require or create SDES cryptos if DTLS is used.
   cricket::SecurePolicy sdes_policy =
       video_transport->secure() ? cricket::SEC_DISABLED : secure();
   bool bundle_enabled =
@@ skipping 32 matching lines @@
   return true;
 }
 
 bool MediaSessionDescriptionFactory::AddDataContentForAnswer(
     const SessionDescription* offer,
     const MediaSessionOptions& options,
     const SessionDescription* current_description,
     StreamParamsVec* current_streams,
     SessionDescription* answer) const {
   const ContentInfo* data_content = GetFirstDataContent(offer);
-  scoped_ptr<TransportDescription> data_transport(
-      CreateTransportAnswer(data_content->name, offer,
-                            options.transport_options,
-                            current_description));
+  scoped_ptr<TransportDescription> data_transport(CreateTransportAnswer(
+      data_content->name, offer, options.transport_options,
+      options.data_ice_restart, current_description));
   if (!data_transport) {
     return false;
   }
   bool is_sctp = (options.data_channel_type == DCT_SCTP);
   std::vector<DataCodec> data_codecs(data_codecs_);
   FilterDataCodecs(&data_codecs, is_sctp);
 
   scoped_ptr<DataContentDescription> data_answer(
       new DataContentDescription());
   // Do not require or create SDES cryptos if DTLS is used.
@@ skipping 47 matching lines @@
 }
 
 bool IsVideoContent(const ContentInfo* content) {
   return IsMediaContentOfType(content, MEDIA_TYPE_VIDEO);
 }
 
 bool IsDataContent(const ContentInfo* content) {
   return IsMediaContentOfType(content, MEDIA_TYPE_DATA);
 }
 
-static const ContentInfo* GetFirstMediaContent(const ContentInfos& contents,
-                                               MediaType media_type) {
+const ContentInfo* GetFirstMediaContent(const ContentInfos& contents,
+                                        MediaType media_type) {
   for (ContentInfos::const_iterator content = contents.begin();
        content != contents.end(); content++) {
     if (IsMediaContentOfType(&*content, media_type)) {
       return &*content;
     }
   }
   return NULL;
 }
 
 const ContentInfo* GetFirstAudioContent(const ContentInfos& contents) {
@@ skipping 47 matching lines @@
       GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_VIDEO));
 }
 
 const DataContentDescription* GetFirstDataContentDescription(
     const SessionDescription* sdesc) {
   return static_cast<const DataContentDescription*>(
       GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA));
 }
 
 }  // namespace cricket