Update talk to 54527154.

TBR=wu

Review URL: https://webrtc-codereview.appspot.com/2389004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@4954 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/session/media/channel.cc

@@ -1081,32 +1081,42 @@ bool BaseChannel::SetMaxSendBandwidth_w(int max_bandwidth) {
   return media_channel()->SetSendBandwidth(true, max_bandwidth);
 }
 
+// |dtls| will be set to true if DTLS is active for transport channel and
+// crypto is empty.
+bool BaseChannel::CheckSrtpConfig(const std::vector<CryptoParams>& cryptos,
+                                  bool* dtls) {
+  *dtls = transport_channel_->IsDtlsActive();
+  if (*dtls && !cryptos.empty()) {
+    LOG(LS_WARNING) << "Cryptos must be empty when DTLS is active.";
+    return false;
+  }
+  return true;
+}
+
 bool BaseChannel::SetSrtp_w(const std::vector<CryptoParams>& cryptos,
                             ContentAction action, ContentSource src) {
   bool ret = false;
+  bool dtls = false;
+  ret = CheckSrtpConfig(cryptos, &dtls);
   switch (action) {
     case CA_OFFER:
-      ret = srtp_filter_.SetOffer(cryptos, src);
+      // If DTLS is already active on the channel, we could be renegotiating
+      // here. We don't update the srtp filter.
+      if (ret && !dtls) {
+        ret = srtp_filter_.SetOffer(cryptos, src);
+      }
       break;
     case CA_PRANSWER:
       // If we're doing DTLS-SRTP, we don't want to update the filter
       // with an answer, because we already have SRTP parameters.
-      if (transport_channel_->IsDtlsActive()) {
-        LOG(LS_INFO) <<
-          "Ignoring SDES answer parameters because we are using DTLS-SRTP";
-        ret = true;
-      } else {
+      if (ret && !dtls) {
         ret = srtp_filter_.SetProvisionalAnswer(cryptos, src);
       }
       break;
     case CA_ANSWER:
       // If we're doing DTLS-SRTP, we don't want to update the filter
       // with an answer, because we already have SRTP parameters.
-      if (transport_channel_->IsDtlsActive()) {
-        LOG(LS_INFO) <<
-          "Ignoring SDES answer parameters because we are using DTLS-SRTP";
-        ret = true;
-      } else {
+      if (ret && !dtls) {
         ret = srtp_filter_.SetAnswer(cryptos, src);
       }
       break;
@@ -2582,6 +2592,9 @@ bool DataChannel::SetLocalContent_w(const MediaContentDescription* content,
     ret = UpdateLocalStreams_w(data->streams(), action);
     if (ret) {
       set_local_content_direction(content->direction());
+      // As in SetRemoteContent_w, make sure we set the local SCTP port
+      // number as specified in our DataContentDescription.
+      ret = media_channel()->SetRecvCodecs(data->codecs());
     }
   } else {
     ret = SetBaseLocalContent_w(content, action);
@@ -2620,6 +2633,9 @@ bool DataChannel::SetRemoteContent_w(const MediaContentDescription* content,
     ret = UpdateRemoteStreams_w(content->streams(), action);
     if (ret) {
       set_remote_content_direction(content->direction());
+      // We send the SCTP port number (not to be confused with the underlying
+      // UDP port number) as a codec parameter.  Make sure it gets there.
+      ret = media_channel()->SetSendCodecs(data->codecs());
     }
   } else {
     // If the remote data doesn't have codecs and isn't an update, it

talk/session/media/channel.h

@@ -318,6 +318,7 @@ class BaseChannel
   virtual bool SetRemoteContent_w(const MediaContentDescription* content,
                                   ContentAction action) = 0;
 
+  bool CheckSrtpConfig(const std::vector<CryptoParams>& cryptos, bool* dtls);
   bool SetSrtp_w(const std::vector<CryptoParams>& params, ContentAction action,
                  ContentSource src);
   bool SetRtcpMux_w(bool enable, ContentAction action, ContentSource src);

talk/session/media/channel_unittest.cc

@@ -233,6 +233,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> {
         this, &ChannelTest<T>::OnMediaChannelError);
     channel1_->SignalAutoMuted.connect(
         this, &ChannelTest<T>::OnMediaMuted);
+    if ((flags1 & DTLS) && (flags2 & DTLS)) {
+      flags1 = (flags1 & ~SECURE);
+      flags2 = (flags2 & ~SECURE);
+    }
     CreateContent(flags1, kPcmuCodec, kH264Codec,
                   &local_media_content1_);
     CreateContent(flags2, kPcmuCodec, kH264Codec,

talk/session/media/mediasession.cc

@@ -978,6 +978,42 @@ static void SetMediaProtocol(bool secure_transport,
     desc->set_protocol(kMediaProtocolAvpf);
 }
 
+// Gets the TransportInfo of the given |content_name| from the
+// |current_description|. If doesn't exist, returns a new one.
+static const TransportDescription* GetTransportDescription(
+    const std::string& content_name,
+    const SessionDescription* current_description) {
+  const TransportDescription* desc = NULL;
+  if (current_description) {
+    const TransportInfo* info =
+        current_description->GetTransportInfoByName(content_name);
+    if (info) {
+      desc = &info->description;
+    }
+  }
+  return desc;
+}
+
+// Gets the current DTLS state from the transport description.
+static bool IsDtlsActive(
+    const std::string& content_name,
+    const SessionDescription* current_description) {
+  if (!current_description)
+    return false;
+
+  const ContentInfo* content =
+      current_description->GetContentByName(content_name);
+  if (!content)
+    return false;
+
+  const TransportDescription* current_tdesc =
+      GetTransportDescription(content_name, current_description);
+  if (!current_tdesc)
+    return false;
+
+  return current_tdesc->secure();
+}
+
 void MediaSessionOptions::AddStream(MediaType type,
                                     const std::string& id,
                                     const std::string& sync_label) {
@@ -1053,13 +1089,17 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer(
 
   // Handle m=audio.
   if (options.has_audio) {
+    cricket::SecurePolicy sdes_policy =
+        IsDtlsActive(CN_AUDIO, current_description) ?
+            cricket::SEC_DISABLED : secure();
+
     scoped_ptr<AudioContentDescription> audio(new AudioContentDescription());
     std::vector<std::string> crypto_suites;
     GetSupportedAudioCryptoSuites(&crypto_suites);
     if (!CreateMediaContentOffer(
             options,
             audio_codecs,
-            secure(),
+            sdes_policy,
             GetCryptos(GetFirstAudioContentDescription(current_description)),
             crypto_suites,
             audio_rtp_extensions,
@@ -1080,13 +1120,17 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer(
 
   // Handle m=video.
   if (options.has_video) {
+    cricket::SecurePolicy sdes_policy =
+        IsDtlsActive(CN_VIDEO, current_description) ?
+            cricket::SEC_DISABLED : secure();
+
     scoped_ptr<VideoContentDescription> video(new VideoContentDescription());
     std::vector<std::string> crypto_suites;
     GetSupportedVideoCryptoSuites(&crypto_suites);
     if (!CreateMediaContentOffer(
             options,
             video_codecs,
-            secure(),
+            sdes_policy,
             GetCryptos(GetFirstVideoContentDescription(current_description)),
             crypto_suites,
             video_rtp_extensions,
@@ -1110,8 +1154,10 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer(
     scoped_ptr<DataContentDescription> data(new DataContentDescription());
     bool is_sctp = (options.data_channel_type == DCT_SCTP);
 
+    cricket::SecurePolicy sdes_policy =
+        IsDtlsActive(CN_DATA, current_description) ?
+            cricket::SEC_DISABLED : secure();
     std::vector<std::string> crypto_suites;
-    cricket::SecurePolicy sdes_policy = secure();
     if (is_sctp) {
       // SDES doesn't make sense for SCTP, so we disable it, and we only
       // get SDES crypto suites for RTP-based data channels.
@@ -1371,22 +1417,6 @@ SessionDescription* MediaSessionDescriptionFactory::CreateAnswer(
   return answer.release();
 }
 
-// Gets the TransportInfo of the given |content_name| from the
-// |current_description|. If doesn't exist, returns a new one.
-static const TransportDescription* GetTransportDescription(
-    const std::string& content_name,
-    const SessionDescription* current_description) {
-  const TransportDescription* desc = NULL;
-  if (current_description) {
-    const TransportInfo* info =
-        current_description->GetTransportInfoByName(content_name);
-    if (info) {
-      desc = &info->description;
-    }
-  }
-  return desc;
-}
-
 void MediaSessionDescriptionFactory::GetCodecsToOffer(
     const SessionDescription* current_description,
     AudioCodecs* audio_codecs,

talk/session/media/mediasession_unittest.cc

@@ -1814,6 +1814,26 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCryptoDtls) {
   ASSERT_TRUE(video_trans_desc != NULL);
   ASSERT_TRUE(audio_trans_desc->identity_fingerprint.get() != NULL);
   ASSERT_TRUE(video_trans_desc->identity_fingerprint.get() != NULL);
+
+  // Try creating offer again. DTLS enabled now, crypto's should be empty
+  // in new offer.
+  offer.reset(f1_.CreateOffer(options, offer.get()));
+  ASSERT_TRUE(offer.get() != NULL);
+  audio_media_desc = static_cast<const cricket::MediaContentDescription*>(
+      offer->GetContentDescriptionByName("audio"));
+  ASSERT_TRUE(audio_media_desc != NULL);
+  video_media_desc = static_cast<const cricket::MediaContentDescription*>(
+      offer->GetContentDescriptionByName("video"));
+  ASSERT_TRUE(video_media_desc != NULL);
+  EXPECT_TRUE(audio_media_desc->cryptos().empty());
+  EXPECT_TRUE(video_media_desc->cryptos().empty());
+
+  audio_trans_desc = offer->GetTransportDescriptionByName("audio");
+  ASSERT_TRUE(audio_trans_desc != NULL);
+  video_trans_desc = offer->GetTransportDescriptionByName("video");
+  ASSERT_TRUE(video_trans_desc != NULL);
+  ASSERT_TRUE(audio_trans_desc->identity_fingerprint.get() != NULL);
+  ASSERT_TRUE(video_trans_desc->identity_fingerprint.get() != NULL);
 }
 
 // Test that an answer can't be created if cryptos are required but the offer is