Updated SCTP SDP attributes according to draft-ietf-mmusic-sctp-sdp-07

- SDP sctpmap attribute replaced with fmtp attribute
- SDP sctp-port attribute is newly added

BUG=3592
R=jiayl@webrtc.org, juberti@webrtc.org

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

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

AUTHORS

@@ -6,6 +6,7 @@ Ben Strong <bstrong@gmail.com>
 Bridger Maxwell <bridgeyman@gmail.com>
 Christophe Dumez <ch.dumez@samsung.com>
 Eric Rescorla, RTFM Inc.
+Giji Gangadharan <giji.g@samsung.com>
 Jie Mao <maojie0924@gmail.com>
 Luke Weber
 Martin Storsjo <martin@martin.st>

talk/app/webrtc/webrtcsdp.cc

@@ -155,6 +155,9 @@ static const char kAttributeRecvOnly[] = "recvonly";
 static const char kAttributeRtcpFb[] = "rtcp-fb";
 static const char kAttributeSendRecv[] = "sendrecv";
 static const char kAttributeInactive[] = "inactive";
+// draft-ietf-mmusic-sctp-sdp-07
+// a=sctp-port
+static const char kAttributeSctpPort[] = "sctp-port";
 
 // Experimental flags
 static const char kAttributeXGoogleFlag[] = "x-google-flag";
@@ -1100,6 +1103,26 @@ bool ParseIceOptions(const std::string& line,
   return true;
 }
 
+bool ParseSctpPort(const std::string& line,
+                   int* sctp_port,
+                   SdpParseError* error) {
+  // draft-ietf-mmusic-sctp-sdp-07
+  // a=sctp-port
+  std::vector<std::string> fields;
+  rtc::split(line.substr(kLinePrefixLength),
+                   kSdpDelimiterSpace, &fields);
+  const size_t expected_min_fields = 2;
+  if (fields.size() < expected_min_fields) {
+    return ParseFailedExpectMinFieldNum(line, expected_min_fields, error);
+  }
+  if (!rtc::FromString(fields[1], sctp_port)) {
+    return ParseFailed(line,
+                       "Invalid sctp port value.",
+                       error);
+  }
+  return true;
+}
+
 bool ParseExtmap(const std::string& line, RtpHeaderExtension* extmap,
                  SdpParseError* error) {
   // RFC 5285
@@ -1563,6 +1586,24 @@ void AddRtcpFbLines(const T& codec, std::string* message) {
   }
 }
 
+bool AddSctpDataCodec(DataContentDescription* media_desc,
+                      int sctp_port) {
+  if (media_desc->HasCodec(cricket::kGoogleSctpDataCodecId)) {
+    return ParseFailed("",
+                       "Can't have multiple sctp port attributes.",
+                       NULL);
+  }
+  // Add the SCTP Port number as a pseudo-codec "port" parameter
+  cricket::DataCodec codec_port(
+      cricket::kGoogleSctpDataCodecId, cricket::kGoogleSctpDataCodecName,
+      0);
+  codec_port.SetParam(cricket::kCodecParamPort, sctp_port);
+  LOG(INFO) << "AddSctpDataCodec: Got SCTP Port Number "
+            << sctp_port;
+  media_desc->AddCodec(codec_port);
+  return true;
+}
+
 bool GetMinValue(const std::vector<int>& values, int* value) {
   if (values.empty()) {
     return false;
@@ -2129,18 +2170,20 @@ bool ParseMediaDescription(const std::string& message,
 
     // <fmt>
     std::vector<int> codec_preference;
-    for (size_t j = 3 ; j < fields.size(); ++j) {
+    if (!is_sctp) {
-      // TODO(wu): Remove when below bug is fixed.
+      for (size_t j = 3 ; j < fields.size(); ++j) {
-      // https://bugzilla.mozilla.org/show_bug.cgi?id=996329
+        // TODO(wu): Remove when below bug is fixed.
-      if (fields[j] == "" && j == fields.size() - 1) {
+        // https://bugzilla.mozilla.org/show_bug.cgi?id=996329
-        continue;
+        if (fields[j] == "" && j == fields.size() - 1) {
-      }
+          continue;
+        }
 
-      int pl = 0;
+        int pl = 0;
-      if (!GetValueFromString(line, fields[j], &pl, error)) {
+        if (!GetValueFromString(line, fields[j], &pl, error)) {
-        return false;
+          return false;
+        }
+        codec_preference.push_back(pl);
       }
-      codec_preference.push_back(pl);
     }
 
     // Make a temporary TransportDescription based on |session_td|.
@@ -2173,20 +2216,14 @@ bool ParseMediaDescription(const std::string& message,
                     codec_preference, pos, &content_name,
                     &transport, candidates, error);
 
-      if (desc && protocol == cricket::kMediaProtocolDtlsSctp) {
+      int p;
-        // Add the SCTP Port number as a pseudo-codec "port" parameter
+      if (desc && protocol == cricket::kMediaProtocolDtlsSctp &&
-        cricket::DataCodec codec_port(
+          rtc::FromString(fields[3], &p)) {
-            cricket::kGoogleSctpDataCodecId, cricket::kGoogleSctpDataCodecName,
+        if (!AddSctpDataCodec(desc, p))
-            0);
+          return false;
-        codec_port.SetParam(cricket::kCodecParamPort, fields[3]);
-        LOG(INFO) << "ParseMediaDescription: Got SCTP Port Number "
-                  << fields[3];
-        ASSERT(!desc->HasCodec(cricket::kGoogleSctpDataCodecId));
-        desc->AddCodec(codec_port);
       }
 
       content.reset(desc);
-
       // We should always use the default bandwidth for RTP-based data
       // channels.  Don't allow SDP to set the bandwidth, because that
       // would give JS the opportunity to "break the Internet".
@@ -2518,6 +2555,15 @@ bool ParseContent(const std::string& message,
       if (!ParseDtlsSetup(line, &(transport->connection_role), error)) {
         return false;
       }
+    } else if (HasAttribute(line, kAttributeSctpPort)) {
+      int sctp_port;
+      if (!ParseSctpPort(line, &sctp_port, error)) {
+        return false;
+      }
+      if (!AddSctpDataCodec(static_cast<DataContentDescription*>(media_desc),
+                            sctp_port)) {
+        return false;
+      }
     } else if (is_rtp) {
       //
       // RTP specific attrubtes

talk/app/webrtc/webrtcsdp_unittest.cc

@@ -284,6 +284,16 @@ static const char kSdpSctpDataChannelString[] =
     "a=mid:data_content_name\r\n"
     "a=sctpmap:5000 webrtc-datachannel 1024\r\n";
 
+// draft-ietf-mmusic-sctp-sdp-07
+static const char kSdpSctpDataChannelStringWithSctpPort[] =
+    "m=application 1 DTLS/SCTP webrtc-datachannel\r\n"
+    "a=fmtp:webrtc-datachannel max-message-size=100000\r\n"
+    "a=sctp-port 5000\r\n"
+    "c=IN IP4 0.0.0.0\r\n"
+    "a=ice-ufrag:ufrag_data\r\n"
+    "a=ice-pwd:pwd_data\r\n"
+    "a=mid:data_content_name\r\n";
+
 static const char kSdpSctpDataChannelWithCandidatesString[] =
     "m=application 2345 DTLS/SCTP 5000\r\n"
     "c=IN IP4 74.125.127.126\r\n"
@@ -2023,6 +2033,36 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannels) {
   EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
 }
 
+TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpPort) {
+  AddSctpDataChannel();
+  JsepSessionDescription jdesc(kDummyString);
+  ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
+
+  std::string sdp_with_data = kSdpString;
+  sdp_with_data.append(kSdpSctpDataChannelStringWithSctpPort);
+  JsepSessionDescription jdesc_output(kDummyString);
+
+  EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
+  EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
+}
+
+// Test to check the behaviour if sctp-port is specified
+// on the m= line and in a=sctp-port.
+TEST_F(WebRtcSdpTest, DeserializeSdpWithMultiSctpPort) {
+  AddSctpDataChannel();
+  JsepSessionDescription jdesc(kDummyString);
+  ASSERT_TRUE(jdesc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
+
+  std::string sdp_with_data = kSdpString;
+  // Append m= attributes
+  sdp_with_data.append(kSdpSctpDataChannelString);
+  // Append a=sctp-port attribute
+  sdp_with_data.append("a=sctp-port 5000\r\n");
+  JsepSessionDescription jdesc_output(kDummyString);
+
+  EXPECT_FALSE(SdpDeserialize(sdp_with_data, &jdesc_output));
+}
+
 // For crbug/344475.
 TEST_F(WebRtcSdpTest, DeserializeSdpWithCorruptedSctpDataChannels) {
   std::string sdp_with_data = kSdpString;
@@ -2071,6 +2111,19 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndNewPort) {
 
   EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
   EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
+
+  // We need to test the deserialized JsepSessionDescription from
+  // kSdpSctpDataChannelStringWithSctpPort for
+  // draft-ietf-mmusic-sctp-sdp-07
+  // a=sctp-port
+  sdp_with_data = kSdpString;
+  sdp_with_data.append(kSdpSctpDataChannelStringWithSctpPort);
+  rtc::replace_substrs(default_portstr, strlen(default_portstr),
+                             unusual_portstr, strlen(unusual_portstr),
+                             &sdp_with_data);
+
+  EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output));
+  EXPECT_TRUE(CompareSessionDescription(jdesc, jdesc_output));
 }
 
 TEST_F(WebRtcSdpTest, DeserializeSdpWithRtpDataChannelsAndBandwidth) {