Reland "Refactoring DataContentDescription class"

This reverts commit 1859dc04fd.

Reason for revert: Issue likely unrelated to this CL.

Original change's description:
> Revert "Refactoring DataContentDescription class"
>
> This reverts commit 8a9193c217.
>
> Reason for revert: Breaks downstreams
>
> Original change's description:
> > Refactoring DataContentDescription class
> >
> > This CL splits the cricket::DataContentDescription class into
> > two classes: cricket::DataContentDescription (used for RTP data) and
> > cricket::SctpDataContentDescription (used for SCTP only).
> >
> > SctpDataContentDescription no longer inherits from
> > MediaContentDescriptionImpl, and no longer contains "codecs".
> >
> > Design document:
> > https://docs.google.com/document/d/1H5LfQxJA2ikMWTQ8FZ3_GAmaXM7knfVQWiSz6ph8VQ0/edit#
> >
> > Bug: webrtc:10358
> > Change-Id: Ie7160610506aeef56d1f821b5fdb5d9492201f43
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132700
> > Reviewed-by: Steve Anton <steveanton@webrtc.org>
> > Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#27651}
>
> TBR=steveanton@webrtc.org,kwiberg@webrtc.org,hbos@webrtc.org,hta@webrtc.org
>
> Change-Id: I3b8a68cd481c41ce30eeb5ffbc5da735a9659019
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:10358
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133360
> Reviewed-by: Seth Hampson <shampson@webrtc.org>
> Commit-Queue: Seth Hampson <shampson@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27652}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:10358
Change-Id: Ie58f862f8c55d2a994eaee1caa107ef701b0770f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133624
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27698}

pc/BUILD.gn

@@ -72,6 +72,7 @@ rtc_static_library("rtc_pc_base") {
   ]
 
   deps = [
+    ":media_protocol_names",
     "../api:array_view",
     "../api:audio_options_api",
     "../api:call_api",
@@ -121,6 +122,13 @@ rtc_source_set("rtc_pc") {
   ]
 }
 
+rtc_source_set("media_protocol_names") {
+  sources = [
+    "media_protocol_names.cc",
+    "media_protocol_names.h",
+  ]
+}
+
 rtc_static_library("peerconnection") {
   visibility = [ "*" ]
   cflags = []

pc/media_protocol_names.cc

@@ -0,0 +1,41 @@
+/*
+ *  Copyright 2019 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.
+ */
+
+#include "pc/media_protocol_names.h"
+
+namespace cricket {
+
+const char kMediaProtocolRtpPrefix[] = "RTP/";
+
+const char kMediaProtocolSctp[] = "SCTP";
+const char kMediaProtocolDtlsSctp[] = "DTLS/SCTP";
+const char kMediaProtocolUdpDtlsSctp[] = "UDP/DTLS/SCTP";
+const char kMediaProtocolTcpDtlsSctp[] = "TCP/DTLS/SCTP";
+
+bool IsDtlsSctp(const std::string& protocol) {
+  return protocol == kMediaProtocolDtlsSctp ||
+         protocol == kMediaProtocolUdpDtlsSctp ||
+         protocol == kMediaProtocolTcpDtlsSctp;
+}
+
+bool IsPlainSctp(const std::string& protocol) {
+  return protocol == kMediaProtocolSctp;
+}
+
+bool IsRtpProtocol(const std::string& protocol) {
+  return protocol.empty() ||
+         (protocol.find(cricket::kMediaProtocolRtpPrefix) != std::string::npos);
+}
+
+bool IsSctpProtocol(const std::string& protocol) {
+  return IsPlainSctp(protocol) || IsDtlsSctp(protocol);
+}
+
+}  // namespace cricket

pc/media_protocol_names.h

@@ -0,0 +1,35 @@
+/*
+ *  Copyright 2019 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.
+ */
+
+#ifndef PC_MEDIA_PROTOCOL_NAMES_H_
+#define PC_MEDIA_PROTOCOL_NAMES_H_
+
+#include <string>
+
+namespace cricket {
+
+// Names or name prefixes of protocols as defined by SDP specifications.
+extern const char kMediaProtocolRtpPrefix[];
+extern const char kMediaProtocolSctp[];
+extern const char kMediaProtocolDtlsSctp[];
+extern const char kMediaProtocolUdpDtlsSctp[];
+extern const char kMediaProtocolTcpDtlsSctp[];
+
+// Returns true if the given media section protocol indicates use of RTP.
+bool IsRtpProtocol(const std::string& protocol);
+// Returns true if the given media section protocol indicates use of SCTP.
+bool IsSctpProtocol(const std::string& protocol);
+
+bool IsDtlsSctp(const std::string& protocol);
+bool IsPlainSctp(const std::string& protocol);
+
+}  // namespace cricket
+
+#endif  // PC_MEDIA_PROTOCOL_NAMES_H_

pc/media_session.cc

@@ -27,6 +27,7 @@
 #include "media/base/media_constants.h"
 #include "p2p/base/p2p_constants.h"
 #include "pc/channel_manager.h"
+#include "pc/media_protocol_names.h"
 #include "pc/rtp_media_utils.h"
 #include "pc/srtp_filter.h"
 #include "rtc_base/checks.h"
@@ -68,13 +69,6 @@ const char kMediaProtocolDtlsSavpf[] = "UDP/TLS/RTP/SAVPF";
 // but we tolerate "RTP/SAVPF" in offers we receive, for compatibility.
 const char kMediaProtocolSavpf[] = "RTP/SAVPF";
 
-const char kMediaProtocolRtpPrefix[] = "RTP/";
-
-const char kMediaProtocolSctp[] = "SCTP";
-const char kMediaProtocolDtlsSctp[] = "DTLS/SCTP";
-const char kMediaProtocolUdpDtlsSctp[] = "UDP/DTLS/SCTP";
-const char kMediaProtocolTcpDtlsSctp[] = "TCP/DTLS/SCTP";
-
 // Note that the below functions support some protocol strings purely for
 // legacy compatibility, as required by JSEP in Section 5.1.2, Profile Names
 // and Interoperability.
@@ -91,20 +85,6 @@ static bool IsPlainRtp(const std::string& protocol) {
          protocol == "RTP/SAVP" || protocol == "RTP/AVP";
 }
 
-static bool IsDtlsSctp(const std::string& protocol) {
-  return protocol == kMediaProtocolDtlsSctp ||
-         protocol == kMediaProtocolUdpDtlsSctp ||
-         protocol == kMediaProtocolTcpDtlsSctp;
-}
-
-static bool IsPlainSctp(const std::string& protocol) {
-  return protocol == kMediaProtocolSctp;
-}
-
-static bool IsSctp(const std::string& protocol) {
-  return IsPlainSctp(protocol) || IsDtlsSctp(protocol);
-}
-
 static RtpTransceiverDirection NegotiateRtpTransceiverDirection(
     RtpTransceiverDirection offer,
     RtpTransceiverDirection wants) {
@@ -489,7 +469,7 @@ static bool AddStreamParams(
     StreamParamsVec* current_streams,
     MediaContentDescriptionImpl<C>* content_description) {
   // SCTP streams are not negotiated using SDP/ContentDescriptions.
-  if (IsSctp(content_description->protocol())) {
+  if (IsSctpProtocol(content_description->protocol())) {
     return true;
   }
 
@@ -608,11 +588,6 @@ static void PruneCryptos(const CryptoParamsVec& filter,
       target_cryptos->end());
 }
 
-bool IsRtpProtocol(const std::string& protocol) {
-  return protocol.empty() ||
-         (protocol.find(cricket::kMediaProtocolRtpPrefix) != std::string::npos);
-}
-
 static bool IsRtpContent(SessionDescription* sdesc,
                          const std::string& content_name) {
   bool is_rtp = false;
@@ -741,32 +716,22 @@ static bool IsFlexfecCodec(const C& codec) {
 // crypto (in current_cryptos) and it is enabled (in secure_policy), crypto is
 // created (according to crypto_suites). The created content is added to the
 // offer.
-template <class C>
+static bool CreateContentOffer(
-static bool CreateMediaContentOffer(
     const MediaDescriptionOptions& media_description_options,
     const MediaSessionOptions& session_options,
-    const std::vector<C>& codecs,
     const SecurePolicy& secure_policy,
     const CryptoParamsVec* current_cryptos,
     const std::vector<std::string>& crypto_suites,
     const RtpHeaderExtensions& rtp_extensions,
     UniqueRandomIdGenerator* ssrc_generator,
     StreamParamsVec* current_streams,
-    MediaContentDescriptionImpl<C>* offer) {
+    MediaContentDescription* offer) {
-  offer->AddCodecs(codecs);
-
   offer->set_rtcp_mux(session_options.rtcp_mux_enabled);
   if (offer->type() == cricket::MEDIA_TYPE_VIDEO) {
     offer->set_rtcp_reduced_size(true);
   }
   offer->set_rtp_header_extensions(rtp_extensions);
 
-  if (!AddStreamParams(media_description_options.sender_options,
-                       session_options.rtcp_cname, ssrc_generator,
-                       current_streams, offer)) {
-    return false;
-  }
-
   AddSimulcastToMediaDescription(media_description_options, offer);
 
   if (secure_policy != SEC_DISABLED) {
@@ -785,6 +750,30 @@ static bool CreateMediaContentOffer(
   }
   return true;
 }
+template <class C>
+static bool CreateMediaContentOffer(
+    const MediaDescriptionOptions& media_description_options,
+    const MediaSessionOptions& session_options,
+    const std::vector<C>& codecs,
+    const SecurePolicy& secure_policy,
+    const CryptoParamsVec* current_cryptos,
+    const std::vector<std::string>& crypto_suites,
+    const RtpHeaderExtensions& rtp_extensions,
+    UniqueRandomIdGenerator* ssrc_generator,
+    StreamParamsVec* current_streams,
+    MediaContentDescriptionImpl<C>* offer) {
+  offer->AddCodecs(codecs);
+  if (!AddStreamParams(media_description_options.sender_options,
+                       session_options.rtcp_cname, ssrc_generator,
+                       current_streams, offer)) {
+    return false;
+  }
+
+  return CreateContentOffer(media_description_options, session_options,
+                            secure_policy, current_cryptos, crypto_suites,
+                            rtp_extensions, ssrc_generator, current_streams,
+                            offer);
+}
 
 template <class C>
 static bool ReferencedCodecsMatch(const std::vector<C>& codecs1,
@@ -1137,6 +1126,27 @@ static void StripCNCodecs(AudioCodecs* audio_codecs) {
                       audio_codecs->end());
 }
 
+template <class C>
+static bool SetCodecsInAnswer(
+    const MediaContentDescriptionImpl<C>* offer,
+    const std::vector<C>& local_codecs,
+    const MediaDescriptionOptions& media_description_options,
+    const MediaSessionOptions& session_options,
+    UniqueRandomIdGenerator* ssrc_generator,
+    StreamParamsVec* current_streams,
+    MediaContentDescriptionImpl<C>* answer) {
+  std::vector<C> negotiated_codecs;
+  NegotiateCodecs(local_codecs, offer->codecs(), &negotiated_codecs);
+  answer->AddCodecs(negotiated_codecs);
+  answer->set_protocol(offer->protocol());
+  if (!AddStreamParams(media_description_options.sender_options,
+                       session_options.rtcp_cname, ssrc_generator,
+                       current_streams, answer)) {
+    return false;  // Something went seriously wrong.
+  }
+  return true;
+}
+
 // Create a media content to be answered for the given |sender_options|
 // according to the given session_options.rtcp_mux, session_options.streams,
 // codecs, crypto, and current_streams.  If we don't currently have crypto (in
@@ -1144,12 +1154,10 @@ static void StripCNCodecs(AudioCodecs* audio_codecs) {
 // (according to crypto_suites). The codecs, rtcp_mux, and crypto are all
 // negotiated with the offer. If the negotiation fails, this method returns
 // false.  The created content is added to the offer.
-template <class C>
 static bool CreateMediaContentAnswer(
-    const MediaContentDescriptionImpl<C>* offer,
+    const MediaContentDescription* offer,
     const MediaDescriptionOptions& media_description_options,
     const MediaSessionOptions& session_options,
-    const std::vector<C>& local_codecs,
     const SecurePolicy& sdes_policy,
     const CryptoParamsVec* current_cryptos,
     const RtpHeaderExtensions& local_rtp_extenstions,
@@ -1157,12 +1165,7 @@ static bool CreateMediaContentAnswer(
     bool enable_encrypted_rtp_header_extensions,
     StreamParamsVec* current_streams,
     bool bundle_enabled,
-    MediaContentDescriptionImpl<C>* answer) {
+    MediaContentDescription* answer) {
-  std::vector<C> negotiated_codecs;
-  NegotiateCodecs(local_codecs, offer->codecs(), &negotiated_codecs);
-  answer->AddCodecs(negotiated_codecs);
-  answer->set_protocol(offer->protocol());
-
   answer->set_extmap_allow_mixed_enum(offer->extmap_allow_mixed_enum());
   RtpHeaderExtensions negotiated_rtp_extensions;
   NegotiateRtpHeaderExtensions(
@@ -1190,12 +1193,6 @@ static bool CreateMediaContentAnswer(
     return false;
   }
 
-  if (!AddStreamParams(media_description_options.sender_options,
-                       session_options.rtcp_cname, ssrc_generator,
-                       current_streams, answer)) {
-    return false;  // Something went seriously wrong.
-  }
-
   AddSimulcastToMediaDescription(media_description_options, answer);
 
   answer->set_direction(NegotiateRtpTransceiverDirection(
@@ -1780,7 +1777,10 @@ void MergeCodecsFromDescription(
     } else if (IsMediaContentOfType(content, MEDIA_TYPE_DATA)) {
       const DataContentDescription* data =
           content->media_description()->as_data();
-      MergeCodecs<DataCodec>(data->codecs(), data_codecs, used_pltypes);
+      if (data) {
+        // Only relevant for RTP datachannels
+        MergeCodecs<DataCodec>(data->codecs(), data_codecs, used_pltypes);
+      }
     }
   }
 }
@@ -1861,13 +1861,16 @@ void MediaSessionDescriptionFactory::GetCodecsForAnswer(
     } else if (IsMediaContentOfType(&content, MEDIA_TYPE_DATA)) {
       const DataContentDescription* data =
           content.media_description()->as_data();
-      for (const DataCodec& offered_data_codec : data->codecs()) {
+      if (data) {
-        if (!FindMatchingCodec<DataCodec>(data->codecs(),
+        // RTP data. This part is inactive for SCTP data.
-                                          filtered_offered_data_codecs,
+        for (const DataCodec& offered_data_codec : data->codecs()) {
-                                          offered_data_codec, nullptr) &&
+          if (!FindMatchingCodec<DataCodec>(data->codecs(),
-            FindMatchingCodec<DataCodec>(data->codecs(), data_codecs_,
+                                            filtered_offered_data_codecs,
-                                         offered_data_codec, nullptr)) {
+                                            offered_data_codec, nullptr) &&
-          filtered_offered_data_codecs.push_back(offered_data_codec);
+              FindMatchingCodec<DataCodec>(data->codecs(), data_codecs_,
+                                           offered_data_codec, nullptr)) {
+            filtered_offered_data_codecs.push_back(offered_data_codec);
+          }
         }
       }
     }
@@ -2140,6 +2143,90 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer(
   return true;
 }
 
+bool MediaSessionDescriptionFactory::AddSctpDataContentForOffer(
+    const MediaDescriptionOptions& media_description_options,
+    const MediaSessionOptions& session_options,
+    const ContentInfo* current_content,
+    const SessionDescription* current_description,
+    StreamParamsVec* current_streams,
+    SessionDescription* desc,
+    IceCredentialsIterator* ice_credentials) const {
+  std::unique_ptr<SctpDataContentDescription> data(
+      new SctpDataContentDescription());
+
+  bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED);
+
+  cricket::SecurePolicy sdes_policy =
+      IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED
+                                                         : secure();
+  std::vector<std::string> crypto_suites;
+  // SDES doesn't make sense for SCTP, so we disable it, and we only
+  // get SDES crypto suites for RTP-based data channels.
+  sdes_policy = cricket::SEC_DISABLED;
+  // Unlike SetMediaProtocol below, we need to set the protocol
+  // before we call CreateMediaContentOffer.  Otherwise,
+  // CreateMediaContentOffer won't know this is SCTP and will
+  // generate SSRCs rather than SIDs.
+  // TODO(deadbeef): Offer kMediaProtocolUdpDtlsSctp (or TcpDtlsSctp), once
+  // it's safe to do so. Older versions of webrtc would reject these
+  // protocols; see https://bugs.chromium.org/p/webrtc/issues/detail?id=7706.
+  data->set_protocol(secure_transport ? kMediaProtocolDtlsSctp
+                                      : kMediaProtocolSctp);
+
+  if (!CreateContentOffer(media_description_options, session_options,
+                          sdes_policy, GetCryptos(current_content),
+                          crypto_suites, RtpHeaderExtensions(), ssrc_generator_,
+                          current_streams, data.get())) {
+    return false;
+  }
+
+  desc->AddContent(media_description_options.mid, MediaProtocolType::kSctp,
+                   data.release());
+  if (!AddTransportOffer(media_description_options.mid,
+                         media_description_options.transport_options,
+                         current_description, desc, ice_credentials)) {
+    return false;
+  }
+  return true;
+}
+
+bool MediaSessionDescriptionFactory::AddRtpDataContentForOffer(
+    const MediaDescriptionOptions& media_description_options,
+    const MediaSessionOptions& session_options,
+    const ContentInfo* current_content,
+    const SessionDescription* current_description,
+    const DataCodecs& data_codecs,
+    StreamParamsVec* current_streams,
+    SessionDescription* desc,
+    IceCredentialsIterator* ice_credentials) const {
+  std::unique_ptr<DataContentDescription> data(new DataContentDescription());
+  bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED);
+
+  cricket::SecurePolicy sdes_policy =
+      IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED
+                                                         : secure();
+  std::vector<std::string> crypto_suites;
+    GetSupportedDataSdesCryptoSuiteNames(session_options.crypto_options,
+                                         &crypto_suites);
+  if (!CreateMediaContentOffer(
+          media_description_options, session_options, data_codecs, sdes_policy,
+          GetCryptos(current_content), crypto_suites, RtpHeaderExtensions(),
+          ssrc_generator_, current_streams, data.get())) {
+    return false;
+  }
+
+  data->set_bandwidth(kDataMaxBandwidth);
+  SetMediaProtocol(secure_transport, data.get());
+  desc->AddContent(media_description_options.mid, MediaProtocolType::kRtp,
+                   media_description_options.stopped, data.release());
+  if (!AddTransportOffer(media_description_options.mid,
+                         media_description_options.transport_options,
+                         current_description, desc, ice_credentials)) {
+    return false;
+  }
+  return true;
+}
+
 bool MediaSessionDescriptionFactory::AddDataContentForOffer(
     const MediaDescriptionOptions& media_description_options,
     const MediaSessionOptions& session_options,
@@ -2149,9 +2236,6 @@ bool MediaSessionDescriptionFactory::AddDataContentForOffer(
     StreamParamsVec* current_streams,
     SessionDescription* desc,
     IceCredentialsIterator* ice_credentials) const {
-  bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED);
-
-  std::unique_ptr<DataContentDescription> data(new DataContentDescription());
   bool is_sctp = (session_options.data_channel_type == DCT_SCTP);
   // If the DataChannel type is not specified, use the DataChannel type in
   // the current description.
@@ -2160,52 +2244,16 @@ bool MediaSessionDescriptionFactory::AddDataContentForOffer(
     is_sctp = (current_content->media_description()->protocol() ==
                kMediaProtocolSctp);
   }
-
-  cricket::SecurePolicy sdes_policy =
-      IsDtlsActive(current_content, current_description) ? cricket::SEC_DISABLED
-                                                         : secure();
-  std::vector<std::string> crypto_suites;
   if (is_sctp) {
-    // SDES doesn't make sense for SCTP, so we disable it, and we only
+    return AddSctpDataContentForOffer(
-    // get SDES crypto suites for RTP-based data channels.
+        media_description_options, session_options, current_content,
-    sdes_policy = cricket::SEC_DISABLED;
+        current_description, current_streams, desc, ice_credentials);
-    // Unlike SetMediaProtocol below, we need to set the protocol
-    // before we call CreateMediaContentOffer.  Otherwise,
-    // CreateMediaContentOffer won't know this is SCTP and will
-    // generate SSRCs rather than SIDs.
-    // TODO(deadbeef): Offer kMediaProtocolUdpDtlsSctp (or TcpDtlsSctp), once
-    // it's safe to do so. Older versions of webrtc would reject these
-    // protocols; see https://bugs.chromium.org/p/webrtc/issues/detail?id=7706.
-    data->set_protocol(secure_transport ? kMediaProtocolDtlsSctp
-                                        : kMediaProtocolSctp);
   } else {
-    GetSupportedDataSdesCryptoSuiteNames(session_options.crypto_options,
+    return AddRtpDataContentForOffer(media_description_options, session_options,
-                                         &crypto_suites);
+                                     current_content, current_description,
+                                     data_codecs, current_streams, desc,
+                                     ice_credentials);
   }
-
-  // Even SCTP uses a "codec".
-  if (!CreateMediaContentOffer(
-          media_description_options, session_options, data_codecs, sdes_policy,
-          GetCryptos(current_content), crypto_suites, RtpHeaderExtensions(),
-          ssrc_generator_, current_streams, data.get())) {
-    return false;
-  }
-
-  if (is_sctp) {
-    desc->AddContent(media_description_options.mid, MediaProtocolType::kSctp,
-                     data.release());
-  } else {
-    data->set_bandwidth(kDataMaxBandwidth);
-    SetMediaProtocol(secure_transport, data.get());
-    desc->AddContent(media_description_options.mid, MediaProtocolType::kRtp,
-                     media_description_options.stopped, data.release());
-  }
-  if (!AddTransportOffer(media_description_options.mid,
-                         media_description_options.transport_options,
-                         current_description, desc, ice_credentials)) {
-    return false;
-  }
-  return true;
 }
 
 // |audio_codecs| = set of all possible codecs that can be used, with correct
@@ -2287,9 +2335,15 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
   // Do not require or create SDES cryptos if DTLS is used.
   cricket::SecurePolicy sdes_policy =
       audio_transport->secure() ? cricket::SEC_DISABLED : secure();
+  if (!SetCodecsInAnswer(offer_audio_description, filtered_codecs,
+                         media_description_options, session_options,
+                         ssrc_generator_, current_streams,
+                         audio_answer.get())) {
+    return false;
+  }
   if (!CreateMediaContentAnswer(
           offer_audio_description, media_description_options, session_options,
-          filtered_codecs, sdes_policy, GetCryptos(current_content),
+          sdes_policy, GetCryptos(current_content),
           audio_rtp_header_extensions(), ssrc_generator_,
           enable_encrypted_rtp_header_extensions_, current_streams,
           bundle_enabled, audio_answer.get())) {
@@ -2376,9 +2430,15 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer(
   // Do not require or create SDES cryptos if DTLS is used.
   cricket::SecurePolicy sdes_policy =
       video_transport->secure() ? cricket::SEC_DISABLED : secure();
+  if (!SetCodecsInAnswer(offer_video_description, filtered_codecs,
+                         media_description_options, session_options,
+                         ssrc_generator_, current_streams,
+                         video_answer.get())) {
+    return false;
+  }
   if (!CreateMediaContentAnswer(
           offer_video_description, media_description_options, session_options,
-          filtered_codecs, sdes_policy, GetCryptos(current_content),
+          sdes_policy, GetCryptos(current_content),
           video_rtp_header_extensions(), ssrc_generator_,
           enable_encrypted_rtp_header_extensions_, current_streams,
           bundle_enabled, video_answer.get())) {
@@ -2426,28 +2486,51 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer(
     return false;
   }
 
-  std::unique_ptr<DataContentDescription> data_answer(
-      new DataContentDescription());
   // Do not require or create SDES cryptos if DTLS is used.
   cricket::SecurePolicy sdes_policy =
       data_transport->secure() ? cricket::SEC_DISABLED : secure();
   bool bundle_enabled = offer_description->HasGroup(GROUP_TYPE_BUNDLE) &&
                         session_options.bundle_enabled;
   RTC_CHECK(IsMediaContentOfType(offer_content, MEDIA_TYPE_DATA));
-  const DataContentDescription* offer_data_description =
+  std::unique_ptr<MediaContentDescription> data_answer;
-      offer_content->media_description()->as_data();
+  if (offer_content->media_description()->as_sctp()) {
-  if (!CreateMediaContentAnswer(
+    // SCTP data content
-          offer_data_description, media_description_options, session_options,
+    data_answer = absl::make_unique<SctpDataContentDescription>();
-          data_codecs, sdes_policy, GetCryptos(current_content),
+    const SctpDataContentDescription* offer_data_description =
-          RtpHeaderExtensions(), ssrc_generator_,
+        offer_content->media_description()->as_sctp();
-          enable_encrypted_rtp_header_extensions_, current_streams,
+    // Respond with the offerer's proto, whatever it is.
-          bundle_enabled, data_answer.get())) {
+    data_answer->as_sctp()->set_protocol(offer_data_description->protocol());
-    return false;  // Fails the session setup.
+    if (!CreateMediaContentAnswer(
-  }
+            offer_data_description, media_description_options, session_options,
+            sdes_policy, GetCryptos(current_content), RtpHeaderExtensions(),
+            ssrc_generator_, enable_encrypted_rtp_header_extensions_,
+            current_streams, bundle_enabled, data_answer.get())) {
+      return false;  // Fails the session setup.
+    }
+    // Respond with sctpmap if the offer uses sctpmap.
+    bool offer_uses_sctpmap = offer_data_description->use_sctpmap();
+    data_answer->as_sctp()->set_use_sctpmap(offer_uses_sctpmap);
+  } else {
+    // RTP offer
+    data_answer = absl::make_unique<DataContentDescription>();
 
-  // Respond with sctpmap if the offer uses sctpmap.
+    RTC_CHECK(offer_content->media_description()->as_data());
-  bool offer_uses_sctpmap = offer_data_description->use_sctpmap();
+    const DataContentDescription* offer_data_description =
-  data_answer->set_use_sctpmap(offer_uses_sctpmap);
+        offer_content->media_description()->as_data();
+    if (!SetCodecsInAnswer(offer_data_description, data_codecs,
+                           media_description_options, session_options,
+                           ssrc_generator_, current_streams,
+                           data_answer->as_data())) {
+      return false;
+    }
+    if (!CreateMediaContentAnswer(
+            offer_data_description, media_description_options, session_options,
+            sdes_policy, GetCryptos(current_content), RtpHeaderExtensions(),
+            ssrc_generator_, enable_encrypted_rtp_header_extensions_,
+            current_streams, bundle_enabled, data_answer.get())) {
+      return false;  // Fails the session setup.
+    }
+  }
 
   bool secure = bundle_transport ? bundle_transport->description.secure()
                                  : data_transport->secure();
@@ -2571,20 +2654,26 @@ const MediaContentDescription* GetFirstMediaContentDescription(
 
 const AudioContentDescription* GetFirstAudioContentDescription(
     const SessionDescription* sdesc) {
-  return static_cast<const AudioContentDescription*>(
+  auto desc = GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_AUDIO);
-      GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_AUDIO));
+  return desc ? desc->as_audio() : nullptr;
 }
 
 const VideoContentDescription* GetFirstVideoContentDescription(
     const SessionDescription* sdesc) {
-  return static_cast<const VideoContentDescription*>(
+  auto desc = GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_VIDEO);
-      GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_VIDEO));
+  return desc ? desc->as_video() : nullptr;
 }
 
 const DataContentDescription* GetFirstDataContentDescription(
     const SessionDescription* sdesc) {
-  return static_cast<const DataContentDescription*>(
+  auto desc = GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA);
-      GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA));
+  return desc ? desc->as_data() : nullptr;
+}
+
+const SctpDataContentDescription* GetFirstSctpDataContentDescription(
+    const SessionDescription* sdesc) {
+  auto desc = GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA);
+  return desc ? desc->as_sctp() : nullptr;
 }
 
 //
@@ -2643,20 +2732,26 @@ MediaContentDescription* GetFirstMediaContentDescription(
 
 AudioContentDescription* GetFirstAudioContentDescription(
     SessionDescription* sdesc) {
-  return static_cast<AudioContentDescription*>(
+  auto desc = GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_AUDIO);
-      GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_AUDIO));
+  return desc ? desc->as_audio() : nullptr;
 }
 
 VideoContentDescription* GetFirstVideoContentDescription(
     SessionDescription* sdesc) {
-  return static_cast<VideoContentDescription*>(
+  auto desc = GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_VIDEO);
-      GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_VIDEO));
+  return desc ? desc->as_video() : nullptr;
 }
 
 DataContentDescription* GetFirstDataContentDescription(
     SessionDescription* sdesc) {
-  return static_cast<DataContentDescription*>(
+  auto desc = GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA);
-      GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA));
+  return desc ? desc->as_data() : nullptr;
+}
+
+SctpDataContentDescription* GetFirstSctpDataContentDescription(
+    SessionDescription* sdesc) {
+  auto desc = GetFirstMediaContentDescription(sdesc, MEDIA_TYPE_DATA);
+  return desc ? desc->as_sctp() : nullptr;
 }
 
 }  // namespace cricket

pc/media_session.h

@@ -24,6 +24,7 @@
 #include "p2p/base/ice_credentials_iterator.h"
 #include "p2p/base/transport_description_factory.h"
 #include "pc/jsep_transport.h"
+#include "pc/media_protocol_names.h"
 #include "pc/session_description.h"
 #include "rtc_base/unique_id_generator.h"
 
@@ -239,6 +240,23 @@ class MediaSessionDescriptionFactory {
       SessionDescription* desc,
       IceCredentialsIterator* ice_credentials) const;
 
+  bool AddSctpDataContentForOffer(
+      const MediaDescriptionOptions& media_description_options,
+      const MediaSessionOptions& session_options,
+      const ContentInfo* current_content,
+      const SessionDescription* current_description,
+      StreamParamsVec* current_streams,
+      SessionDescription* desc,
+      IceCredentialsIterator* ice_credentials) const;
+  bool AddRtpDataContentForOffer(
+      const MediaDescriptionOptions& media_description_options,
+      const MediaSessionOptions& session_options,
+      const ContentInfo* current_content,
+      const SessionDescription* current_description,
+      const DataCodecs& data_codecs,
+      StreamParamsVec* current_streams,
+      SessionDescription* desc,
+      IceCredentialsIterator* ice_credentials) const;
   bool AddDataContentForOffer(
       const MediaDescriptionOptions& media_description_options,
       const MediaSessionOptions& session_options,
@@ -331,6 +349,8 @@ const VideoContentDescription* GetFirstVideoContentDescription(
     const SessionDescription* sdesc);
 const DataContentDescription* GetFirstDataContentDescription(
     const SessionDescription* sdesc);
+const SctpDataContentDescription* GetFirstSctpDataContentDescription(
+    const SessionDescription* sdesc);
 // Non-const versions of the above functions.
 // Useful when modifying an existing description.
 ContentInfo* GetFirstMediaContent(ContentInfos* contents, MediaType media_type);
@@ -348,6 +368,8 @@ VideoContentDescription* GetFirstVideoContentDescription(
     SessionDescription* sdesc);
 DataContentDescription* GetFirstDataContentDescription(
     SessionDescription* sdesc);
+SctpDataContentDescription* GetFirstSctpDataContentDescription(
+    SessionDescription* sdesc);
 
 // Helper functions to return crypto suites used for SDES.
 void GetSupportedAudioSdesCryptoSuites(
@@ -369,9 +391,6 @@ void GetSupportedDataSdesCryptoSuiteNames(
     const webrtc::CryptoOptions& crypto_options,
     std::vector<std::string>* crypto_suite_names);
 
-// Returns true if the given media section protocol indicates use of RTP.
-bool IsRtpProtocol(const std::string& protocol);
-
 }  // namespace cricket
 
 #endif  // PC_MEDIA_SESSION_H_

pc/media_session_unittest.cc

@@ -62,6 +62,7 @@ using cricket::MediaSessionOptions;
 using cricket::MediaType;
 using cricket::RidDescription;
 using cricket::RidDirection;
+using cricket::SctpDataContentDescription;
 using cricket::SEC_DISABLED;
 using cricket::SEC_ENABLED;
 using cricket::SEC_REQUIRED;
@@ -1336,15 +1337,16 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerUsesSctpmap) {
   ASSERT_TRUE(offer.get() != NULL);
   ContentInfo* dc_offer = offer->GetContentByName("data");
   ASSERT_TRUE(dc_offer != NULL);
-  DataContentDescription* dcd_offer = dc_offer->media_description()->as_data();
+  SctpDataContentDescription* dcd_offer =
+      dc_offer->media_description()->as_sctp();
   EXPECT_TRUE(dcd_offer->use_sctpmap());
 
   std::unique_ptr<SessionDescription> answer =
       f2_.CreateAnswer(offer.get(), opts, NULL);
   const ContentInfo* dc_answer = answer->GetContentByName("data");
   ASSERT_TRUE(dc_answer != NULL);
-  const DataContentDescription* dcd_answer =
+  const SctpDataContentDescription* dcd_answer =
-      dc_answer->media_description()->as_data();
+      dc_answer->media_description()->as_sctp();
   EXPECT_TRUE(dcd_answer->use_sctpmap());
 }
 
@@ -1356,15 +1358,16 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestCreateDataAnswerWithoutSctpmap) {
   ASSERT_TRUE(offer.get() != NULL);
   ContentInfo* dc_offer = offer->GetContentByName("data");
   ASSERT_TRUE(dc_offer != NULL);
-  DataContentDescription* dcd_offer = dc_offer->media_description()->as_data();
+  SctpDataContentDescription* dcd_offer =
+      dc_offer->media_description()->as_sctp();
   dcd_offer->set_use_sctpmap(false);
 
   std::unique_ptr<SessionDescription> answer =
       f2_.CreateAnswer(offer.get(), opts, NULL);
   const ContentInfo* dc_answer = answer->GetContentByName("data");
   ASSERT_TRUE(dc_answer != NULL);
-  const DataContentDescription* dcd_answer =
+  const SctpDataContentDescription* dcd_answer =
-      dc_answer->media_description()->as_data();
+      dc_answer->media_description()->as_sctp();
   EXPECT_FALSE(dcd_answer->use_sctpmap());
 }
 
@@ -1385,7 +1388,9 @@ TEST_F(MediaSessionDescriptionFactoryTest,
   ASSERT_TRUE(offer.get() != nullptr);
   ContentInfo* dc_offer = offer->GetContentByName("data");
   ASSERT_TRUE(dc_offer != nullptr);
-  DataContentDescription* dcd_offer = dc_offer->media_description()->as_data();
+  SctpDataContentDescription* dcd_offer =
+      dc_offer->media_description()->as_sctp();
+  ASSERT_TRUE(dcd_offer);
 
   std::vector<std::string> protos = {"DTLS/SCTP", "UDP/DTLS/SCTP",
                                      "TCP/DTLS/SCTP"};
@@ -1395,8 +1400,8 @@ TEST_F(MediaSessionDescriptionFactoryTest,
         f2_.CreateAnswer(offer.get(), opts, nullptr);
     const ContentInfo* dc_answer = answer->GetContentByName("data");
     ASSERT_TRUE(dc_answer != nullptr);
-    const DataContentDescription* dcd_answer =
+    const SctpDataContentDescription* dcd_answer =
-        dc_answer->media_description()->as_data();
+        dc_answer->media_description()->as_sctp();
     EXPECT_FALSE(dc_answer->rejected);
     EXPECT_EQ(proto, dcd_answer->protocol());
   }
@@ -1480,7 +1485,8 @@ TEST_F(MediaSessionDescriptionFactoryTest,
   ASSERT_TRUE(dc_offer != NULL);
   DataContentDescription* dcd_offer = dc_offer->media_description()->as_data();
   ASSERT_TRUE(dcd_offer != NULL);
-  std::string protocol = "a weird unknown protocol";
+  // Offer must be acceptable as an RTP protocol in order to be set.
+  std::string protocol = "RTP/a weird unknown protocol";
   dcd_offer->set_protocol(protocol);
 
   std::unique_ptr<SessionDescription> answer =

pc/peer_connection.cc

@@ -559,24 +559,13 @@ bool VerifyIceUfragPwdPresent(const SessionDescription* desc) {
 // Get the SCTP port out of a SessionDescription.
 // Return -1 if not found.
 int GetSctpPort(const SessionDescription* session_description) {
-  const cricket::DataContentDescription* data_desc =
+  const cricket::SctpDataContentDescription* data_desc =
-      GetFirstDataContentDescription(session_description);
+      GetFirstSctpDataContentDescription(session_description);
   RTC_DCHECK(data_desc);
   if (!data_desc) {
     return -1;
   }
-  std::string value;
+  return data_desc->port();
-  cricket::DataCodec match_pattern(cricket::kGoogleSctpDataCodecPlType,
-                                   cricket::kGoogleSctpDataCodecName);
-  for (const cricket::DataCodec& codec : data_desc->codecs()) {
-    if (!codec.Matches(match_pattern)) {
-      continue;
-    }
-    if (codec.GetParam(cricket::kCodecParamPort, &value)) {
-      return rtc::FromString<int>(value);
-    }
-  }
-  return -1;
 }
 
 // Returns true if |new_desc| requests an ICE restart (i.e., new ufrag/pwd).
@@ -2424,8 +2413,9 @@ RTCError PeerConnection::ApplyLocalDescription(
   if (data_content) {
     const cricket::DataContentDescription* data_desc =
         data_content->media_description()->as_data();
-    if (absl::StartsWith(data_desc->protocol(),
+    // data_desc will be null if this is an SCTP description.
-                         cricket::kMediaProtocolRtpPrefix)) {
+    if (data_desc && absl::StartsWith(data_desc->protocol(),
+                                      cricket::kMediaProtocolRtpPrefix)) {
       UpdateLocalRtpDataChannels(data_desc->streams());
     }
   }

pc/peer_connection_data_channel_unittest.cc

@@ -193,14 +193,11 @@ class PeerConnectionDataChannelBaseTest : public ::testing::Test {
   // Changes the SCTP data channel port on the given session description.
   void ChangeSctpPortOnDescription(cricket::SessionDescription* desc,
                                    int port) {
-    cricket::DataCodec sctp_codec(cricket::kGoogleSctpDataCodecPlType,
-                                  cricket::kGoogleSctpDataCodecName);
-    sctp_codec.SetParam(cricket::kCodecParamPort, port);
-
     auto* data_content = cricket::GetFirstDataContent(desc);
     RTC_DCHECK(data_content);
-    auto* data_desc = data_content->media_description()->as_data();
+    auto* data_desc = data_content->media_description()->as_sctp();
-    data_desc->set_codecs({sctp_codec});
+    RTC_DCHECK(data_desc);
+    data_desc->set_port(port);
   }
 
   std::unique_ptr<rtc::VirtualSocketServer> vss_;

pc/peer_connection_integrationtest.cc

@@ -3450,8 +3450,8 @@ TEST_P(PeerConnectionIntegrationTest, SctpDataChannelToAudioVideoUpgrade) {
 }
 
 static void MakeSpecCompliantSctpOffer(cricket::SessionDescription* desc) {
-  cricket::DataContentDescription* dcd_offer =
+  cricket::SctpDataContentDescription* dcd_offer =
-      GetFirstDataContentDescription(desc);
+      GetFirstSctpDataContentDescription(desc);
   ASSERT_TRUE(dcd_offer);
   dcd_offer->set_use_sctpmap(false);
   dcd_offer->set_protocol("UDP/DTLS/SCTP");

pc/session_description.h

@@ -26,6 +26,7 @@
 #include "media/base/stream_params.h"
 #include "p2p/base/transport_description.h"
 #include "p2p/base/transport_info.h"
+#include "pc/media_protocol_names.h"
 #include "pc/simulcast_description.h"
 #include "rtc_base/socket_address.h"
 
@@ -44,12 +45,6 @@ extern const char kMediaProtocolSavpf[];
 
 extern const char kMediaProtocolDtlsSavpf[];
 
-extern const char kMediaProtocolRtpPrefix[];
-
-extern const char kMediaProtocolSctp[];
-extern const char kMediaProtocolDtlsSctp[];
-extern const char kMediaProtocolUdpDtlsSctp[];
-extern const char kMediaProtocolTcpDtlsSctp[];
 
 // Options to control how session descriptions are generated.
 const int kAutoBandwidth = -1;
@@ -57,6 +52,7 @@ const int kAutoBandwidth = -1;
 class AudioContentDescription;
 class DataContentDescription;
 class VideoContentDescription;
+class SctpDataContentDescription;
 
 // Describes a session description media section. There are subclasses for each
 // media type (audio, video, data) that will have additional information.
@@ -82,6 +78,9 @@ class MediaContentDescription {
   virtual DataContentDescription* as_data() { return nullptr; }
   virtual const DataContentDescription* as_data() const { return nullptr; }
 
+  virtual SctpDataContentDescription* as_sctp() { return nullptr; }
+  virtual const SctpDataContentDescription* as_sctp() const { return nullptr; }
+
   virtual bool has_codecs() const = 0;
 
   virtual MediaContentDescription* Copy() const = 0;
@@ -89,7 +88,9 @@ class MediaContentDescription {
   // |protocol| is the expected media transport protocol, such as RTP/AVPF,
   // RTP/SAVPF or SCTP/DTLS.
   std::string protocol() const { return protocol_; }
-  void set_protocol(const std::string& protocol) { protocol_ = protocol; }
+  virtual void set_protocol(const std::string& protocol) {
+    protocol_ = protocol;
+  }
 
   webrtc::RtpTransceiverDirection direction() const { return direction_; }
   void set_direction(webrtc::RtpTransceiverDirection direction) {
@@ -247,12 +248,17 @@ using ContentDescription = MediaContentDescription;
 template <class C>
 class MediaContentDescriptionImpl : public MediaContentDescription {
  public:
+  void set_protocol(const std::string& protocol) override {
+    RTC_DCHECK(IsRtpProtocol(protocol));
+    protocol_ = protocol;
+  }
+
   typedef C CodecType;
 
   // Codecs should be in preference order (most preferred codec first).
   const std::vector<C>& codecs() const { return codecs_; }
   void set_codecs(const std::vector<C>& codecs) { codecs_ = codecs; }
-  virtual bool has_codecs() const { return !codecs_.empty(); }
+  bool has_codecs() const override { return !codecs_.empty(); }
   bool HasCodec(int id) {
     bool found = false;
     for (typename std::vector<C>::iterator iter = codecs_.begin();
@@ -318,12 +324,37 @@ class DataContentDescription : public MediaContentDescriptionImpl<DataCodec> {
   virtual MediaType type() const { return MEDIA_TYPE_DATA; }
   virtual DataContentDescription* as_data() { return this; }
   virtual const DataContentDescription* as_data() const { return this; }
+};
+
+class SctpDataContentDescription : public MediaContentDescription {
+ public:
+  SctpDataContentDescription() {}
+  SctpDataContentDescription* Copy() const override {
+    return new SctpDataContentDescription(*this);
+  }
+  MediaType type() const override { return MEDIA_TYPE_DATA; }
+  SctpDataContentDescription* as_sctp() override { return this; }
+  const SctpDataContentDescription* as_sctp() const override { return this; }
+  bool has_codecs() const override { return false; }
+  void set_protocol(const std::string& protocol) override {
+    RTC_DCHECK(IsSctpProtocol(protocol));
+    protocol_ = protocol;
+  }
 
   bool use_sctpmap() const { return use_sctpmap_; }
   void set_use_sctpmap(bool enable) { use_sctpmap_ = enable; }
+  int port() const { return port_; }
+  void set_port(int port) { port_ = port; }
+  int max_message_size() const { return max_message_size_; }
+  void set_max_message_size(int max_message_size) {
+    max_message_size_ = max_message_size;
+  }
 
  private:
-  bool use_sctpmap_ = true;
+  bool use_sctpmap_ = true;  // Note: "true" is no longer conformant.
+  // Defaults should be constants imported from SCTP. Quick hack.
+  int port_ = 5000;
+  int max_message_size_ = 256 * 1024;
 };
 
 // Protocol used for encoding media. This is the "top level" protocol that may

pc/webrtc_sdp.cc

@@ -54,29 +54,30 @@ using cricket::Candidates;
 using cricket::ContentInfo;
 using cricket::CryptoParams;
 using cricket::DataContentDescription;
-using cricket::ICE_CANDIDATE_COMPONENT_RTP;
 using cricket::ICE_CANDIDATE_COMPONENT_RTCP;
+using cricket::ICE_CANDIDATE_COMPONENT_RTP;
+using cricket::kCodecParamAssociatedPayloadType;
+using cricket::kCodecParamMaxAverageBitrate;
 using cricket::kCodecParamMaxBitrate;
+using cricket::kCodecParamMaxPlaybackRate;
 using cricket::kCodecParamMaxPTime;
 using cricket::kCodecParamMaxQuantization;
 using cricket::kCodecParamMinBitrate;
 using cricket::kCodecParamMinPTime;
 using cricket::kCodecParamPTime;
+using cricket::kCodecParamSctpProtocol;
+using cricket::kCodecParamSctpStreams;
 using cricket::kCodecParamSPropStereo;
 using cricket::kCodecParamStartBitrate;
 using cricket::kCodecParamStereo;
-using cricket::kCodecParamUseInbandFec;
 using cricket::kCodecParamUseDtx;
-using cricket::kCodecParamSctpProtocol;
+using cricket::kCodecParamUseInbandFec;
-using cricket::kCodecParamSctpStreams;
-using cricket::kCodecParamMaxAverageBitrate;
-using cricket::kCodecParamMaxPlaybackRate;
-using cricket::kCodecParamAssociatedPayloadType;
 using cricket::MediaContentDescription;
-using cricket::MediaType;
-using cricket::RtpHeaderExtensions;
 using cricket::MediaProtocolType;
+using cricket::MediaType;
 using cricket::RidDescription;
+using cricket::RtpHeaderExtensions;
+using cricket::SctpDataContentDescription;
 using cricket::SimulcastDescription;
 using cricket::SimulcastLayer;
 using cricket::SimulcastLayerList;
@@ -1337,8 +1338,6 @@ void BuildMediaDescription(const ContentInfo* content_info,
   const MediaContentDescription* media_desc = content_info->media_description();
   RTC_DCHECK(media_desc);
 
-  int sctp_port = cricket::kSctpDefaultPort;
-
   // RFC 4566
   // m=<media> <port> <proto> <fmt>
   // fmt is a list of payload type numbers that MAY be used in the session.
@@ -1366,24 +1365,18 @@ void BuildMediaDescription(const ContentInfo* content_info,
       fmt.append(rtc::ToString(codec.id));
     }
   } else if (media_type == cricket::MEDIA_TYPE_DATA) {
-    const DataContentDescription* data_desc = media_desc->as_data();
     if (IsDtlsSctp(media_desc->protocol())) {
+      const cricket::SctpDataContentDescription* data_desc =
+          media_desc->as_sctp();
       fmt.append(" ");
 
       if (data_desc->use_sctpmap()) {
-        for (const cricket::DataCodec& codec : data_desc->codecs()) {
+        fmt.append(rtc::ToString(data_desc->port()));
-          if (absl::EqualsIgnoreCase(codec.name,
-                                     cricket::kGoogleSctpDataCodecName) &&
-              codec.GetParam(cricket::kCodecParamPort, &sctp_port)) {
-            break;
-          }
-        }
-
-        fmt.append(rtc::ToString(sctp_port));
       } else {
         fmt.append(kDefaultSctpmapProtocol);
       }
     } else {
+      const DataContentDescription* data_desc = media_desc->as_data();
       for (const cricket::DataCodec& codec : data_desc->codecs()) {
         fmt.append(" ");
         fmt.append(rtc::ToString(codec.id));
@@ -1523,9 +1516,10 @@ void BuildMediaDescription(const ContentInfo* content_info,
   AddLine(os.str(), message);
 
   if (IsDtlsSctp(media_desc->protocol())) {
-    const DataContentDescription* data_desc = media_desc->as_data();
+    const cricket::SctpDataContentDescription* data_desc =
+        media_desc->as_sctp();
     bool use_sctpmap = data_desc->use_sctpmap();
-    BuildSctpContentAttributes(message, sctp_port, use_sctpmap);
+    BuildSctpContentAttributes(message, data_desc->port(), use_sctpmap);
   } else if (IsRtp(media_desc->protocol())) {
     BuildRtpContentAttributes(media_desc, media_type, msid_signaling, message);
   }
@@ -1834,43 +1828,6 @@ void AddRtcpFbLines(const T& codec, std::string* message) {
   }
 }
 
-cricket::DataCodec FindOrMakeSctpDataCodec(DataContentDescription* media_desc) {
-  for (const auto& codec : media_desc->codecs()) {
-    if (absl::EqualsIgnoreCase(codec.name, cricket::kGoogleSctpDataCodecName)) {
-      return codec;
-    }
-  }
-  cricket::DataCodec codec_port(cricket::kGoogleSctpDataCodecPlType,
-                                cricket::kGoogleSctpDataCodecName);
-  return codec_port;
-}
-
-bool AddOrModifySctpDataCodecPort(DataContentDescription* media_desc,
-                                  int sctp_port) {
-  // Add the SCTP Port number as a pseudo-codec "port" parameter
-  auto codec = FindOrMakeSctpDataCodec(media_desc);
-  int dummy;
-  if (codec.GetParam(cricket::kCodecParamPort, &dummy)) {
-    return false;
-  }
-  codec.SetParam(cricket::kCodecParamPort, sctp_port);
-  media_desc->AddOrReplaceCodec(codec);
-  return true;
-}
-
-bool AddOrModifySctpDataMaxMessageSize(DataContentDescription* media_desc,
-                                       int max_message_size) {
-  // Add the SCTP Max Message Size as a pseudo-parameter to the codec
-  auto codec = FindOrMakeSctpDataCodec(media_desc);
-  int dummy;
-  if (codec.GetParam(cricket::kCodecParamMaxMessageSize, &dummy)) {
-    return false;
-  }
-  codec.SetParam(cricket::kCodecParamMaxMessageSize, max_message_size);
-  media_desc->AddOrReplaceCodec(codec);
-  return true;
-}
-
 bool GetMinValue(const std::vector<int>& values, int* value) {
   if (values.empty()) {
     return false;
@@ -2748,24 +2705,30 @@ bool ParseMediaDescription(
           payload_types, pos, &content_name, &bundle_only,
           &section_msid_signaling, &transport, candidates, error);
     } else if (HasAttribute(line, kMediaTypeData)) {
-      std::unique_ptr<DataContentDescription> data_desc =
+      if (IsDtlsSctp(protocol)) {
-          ParseContentDescription<DataContentDescription>(
+        auto data_desc = absl::make_unique<SctpDataContentDescription>();
-              message, cricket::MEDIA_TYPE_DATA, mline_index, protocol,
-              payload_types, pos, &content_name, &bundle_only,
-              &section_msid_signaling, &transport, candidates, error);
-
-      if (data_desc && IsDtlsSctp(protocol)) {
         int p;
         if (rtc::FromString(fields[3], &p)) {
-          if (!AddOrModifySctpDataCodecPort(data_desc.get(), p)) {
+          data_desc->set_port(p);
-            return false;
-          }
         } else if (fields[3] == kDefaultSctpmapProtocol) {
           data_desc->set_use_sctpmap(false);
         }
+        if (!ParseContent(message, cricket::MEDIA_TYPE_DATA, mline_index,
+                          protocol, payload_types, pos, &content_name,
+                          &bundle_only, &section_msid_signaling,
+                          data_desc.get(), &transport, candidates, error)) {
+          return false;
+        }
+        content = std::move(data_desc);
+      } else {
+        // RTP
+        std::unique_ptr<DataContentDescription> data_desc =
+            ParseContentDescription<DataContentDescription>(
+                message, cricket::MEDIA_TYPE_DATA, mline_index, protocol,
+                payload_types, pos, &content_name, &bundle_only,
+                &section_msid_signaling, &transport, candidates, error);
+        content = std::move(data_desc);
       }
-
-      content = std::move(data_desc);
     } else {
       RTC_LOG(LS_WARNING) << "Unsupported media type: " << line;
       continue;
@@ -3138,13 +3101,15 @@ bool ParseContent(const std::string& message,
             line, "sctp-port attribute found in non-data media description.",
             error);
       }
+      if (media_desc->as_sctp()->use_sctpmap()) {
+        return ParseFailed(
+            line, "sctp-port attribute can't be used with sctpmap.", error);
+      }
       int sctp_port;
       if (!ParseSctpPort(line, &sctp_port, error)) {
         return false;
       }
-      if (!AddOrModifySctpDataCodecPort(media_desc->as_data(), sctp_port)) {
+      media_desc->as_sctp()->set_port(sctp_port);
-        return false;
-      }
     } else if (IsDtlsSctp(protocol) &&
                HasAttribute(line, kAttributeMaxMessageSize)) {
       if (media_type != cricket::MEDIA_TYPE_DATA) {
@@ -3157,10 +3122,7 @@ bool ParseContent(const std::string& message,
       if (!ParseSctpMaxMessageSize(line, &max_message_size, error)) {
         return false;
       }
-      if (!AddOrModifySctpDataMaxMessageSize(media_desc->as_data(),
+      media_desc->as_sctp()->set_max_message_size(max_message_size);
-                                             max_message_size)) {
-        return false;
-      }
     } else if (IsRtp(protocol)) {
       //
       // RTP specific attrubtes

pc/webrtc_sdp_unittest.cc

@@ -65,6 +65,7 @@ using cricket::MediaProtocolType;
 using cricket::RELAY_PORT_TYPE;
 using cricket::RidDescription;
 using cricket::RidDirection;
+using cricket::SctpDataContentDescription;
 using cricket::SessionDescription;
 using cricket::SimulcastDescription;
 using cricket::SimulcastLayer;
@@ -1445,10 +1446,17 @@ class WebRtcSdpTest : public ::testing::Test {
 
   void CompareDataContentDescription(const DataContentDescription* dcd1,
                                      const DataContentDescription* dcd2) {
-    EXPECT_EQ(dcd1->use_sctpmap(), dcd2->use_sctpmap());
     CompareMediaContentDescription<DataContentDescription>(dcd1, dcd2);
   }
 
+  void CompareSctpDataContentDescription(
+      const SctpDataContentDescription* dcd1,
+      const SctpDataContentDescription* dcd2) {
+    EXPECT_EQ(dcd1->use_sctpmap(), dcd2->use_sctpmap());
+    EXPECT_EQ(dcd1->port(), dcd2->port());
+    EXPECT_EQ(dcd1->max_message_size(), dcd2->max_message_size());
+  }
+
   void CompareSessionDescription(const SessionDescription& desc1,
                                  const SessionDescription& desc2) {
     // Compare content descriptions.
@@ -1484,10 +1492,21 @@ class WebRtcSdpTest : public ::testing::Test {
       }
 
       ASSERT_EQ(IsDataContent(&c1), IsDataContent(&c2));
-      if (IsDataContent(&c1)) {
+      if (c1.media_description()->as_sctp()) {
-        const DataContentDescription* dcd1 = c1.media_description()->as_data();
+        ASSERT_TRUE(c2.media_description()->as_sctp());
-        const DataContentDescription* dcd2 = c2.media_description()->as_data();
+        const SctpDataContentDescription* scd1 =
-        CompareDataContentDescription(dcd1, dcd2);
+            c1.media_description()->as_sctp();
+        const SctpDataContentDescription* scd2 =
+            c2.media_description()->as_sctp();
+        CompareSctpDataContentDescription(scd1, scd2);
+      } else {
+        if (IsDataContent(&c1)) {
+          const DataContentDescription* dcd1 =
+              c1.media_description()->as_data();
+          const DataContentDescription* dcd2 =
+              c2.media_description()->as_data();
+          CompareDataContentDescription(dcd1, dcd2);
+        }
       }
 
       CompareSimulcastDescription(
@@ -1760,14 +1779,12 @@ class WebRtcSdpTest : public ::testing::Test {
   }
 
   void AddSctpDataChannel(bool use_sctpmap) {
-    std::unique_ptr<DataContentDescription> data(new DataContentDescription());
+    std::unique_ptr<SctpDataContentDescription> data(
-    data_desc_ = data.get();
+        new SctpDataContentDescription());
-    data_desc_->set_use_sctpmap(use_sctpmap);
+    sctp_desc_ = data.get();
-    data_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp);
+    sctp_desc_->set_use_sctpmap(use_sctpmap);
-    DataCodec codec(cricket::kGoogleSctpDataCodecPlType,
+    sctp_desc_->set_protocol(cricket::kMediaProtocolDtlsSctp);
-                    cricket::kGoogleSctpDataCodecName);
+    sctp_desc_->set_port(kDefaultSctpPort);
-    codec.SetParam(cricket::kCodecParamPort, kDefaultSctpPort);
-    data_desc_->AddCodec(codec);
     desc_.AddContent(kDataContentName, MediaProtocolType::kSctp,
                      data.release());
     desc_.AddTransportInfo(TransportInfo(
@@ -2044,6 +2061,7 @@ class WebRtcSdpTest : public ::testing::Test {
   AudioContentDescription* audio_desc_;
   VideoContentDescription* video_desc_;
   DataContentDescription* data_desc_;
+  SctpDataContentDescription* sctp_desc_;
   Candidates candidates_;
   std::unique_ptr<IceCandidateInterface> jcandidate_;
   JsepSessionDescription jdesc_;
@@ -2215,21 +2233,26 @@ TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithSctpDataChannel) {
   EXPECT_EQ(message, expected_sdp);
 }
 
+void MutateJsepSctpPort(JsepSessionDescription* jdesc,
+                        const SessionDescription& desc,
+                        int port) {
+  // Take our pre-built session description and change the SCTP port.
+  cricket::SessionDescription* mutant = desc.Copy();
+  SctpDataContentDescription* dcdesc =
+      mutant->GetContentDescriptionByName(kDataContentName)->as_sctp();
+  dcdesc->set_port(port);
+  // Note: mutant's owned by jdesc now.
+  ASSERT_TRUE(jdesc->Initialize(mutant, kSessionId, kSessionVersion));
+}
+
 TEST_F(WebRtcSdpTest, SerializeWithSctpDataChannelAndNewPort) {
   bool use_sctpmap = true;
   AddSctpDataChannel(use_sctpmap);
   JsepSessionDescription jsep_desc(kDummyType);
   MakeDescriptionWithoutCandidates(&jsep_desc);
-  DataContentDescription* dcdesc =
-      jsep_desc.description()
-          ->GetContentDescriptionByName(kDataContentName)
-          ->as_data();
 
   const int kNewPort = 1234;
-  cricket::DataCodec codec(cricket::kGoogleSctpDataCodecPlType,
+  MutateJsepSctpPort(&jsep_desc, desc_, kNewPort);
-                           cricket::kGoogleSctpDataCodecName);
-  codec.SetParam(cricket::kCodecParamPort, kNewPort);
-  dcdesc->AddOrReplaceCodec(codec);
 
   std::string message = webrtc::SdpSerialize(jsep_desc);
 
@@ -2868,14 +2891,12 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithSctpColonPort) {
 // Helper function to set the max-message-size parameter in the
 // SCTP data codec.
 void MutateJsepSctpMaxMessageSize(const SessionDescription& desc,
-                                  const std::string& new_value,
+                                  int new_value,
                                   JsepSessionDescription* jdesc) {
   cricket::SessionDescription* mutant = desc.Copy();
-  DataContentDescription* dcdesc =
+  SctpDataContentDescription* dcdesc =
-      mutant->GetContentDescriptionByName(kDataContentName)->as_data();
+      mutant->GetContentDescriptionByName(kDataContentName)->as_sctp();
-  std::vector<cricket::DataCodec> codecs(dcdesc->codecs());
+  dcdesc->set_max_message_size(new_value);
-  codecs[0].SetParam(cricket::kCodecParamMaxMessageSize, new_value);
-  dcdesc->set_codecs(codecs);
   jdesc->Initialize(mutant, kSessionId, kSessionVersion);
 }
 
@@ -2887,7 +2908,7 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsWithMaxMessageSize) {
 
   sdp_with_data.append(kSdpSctpDataChannelStringWithSctpColonPort);
   sdp_with_data.append("a=max-message-size:12345\r\n");
-  MutateJsepSctpMaxMessageSize(desc_, "12345", &jdesc);
+  MutateJsepSctpMaxMessageSize(desc_, 12345, &jdesc);
   JsepSessionDescription jdesc_output(kDummyType);
 
   // Verify with DTLS/SCTP.
@@ -2937,29 +2958,13 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithCorruptedSctpDataChannels) {
   // No crash is a pass.
 }
 
-void MutateJsepSctpPort(JsepSessionDescription* jdesc,
-                        const SessionDescription& desc) {
-  // take our pre-built session description and change the SCTP port.
-  std::unique_ptr<cricket::SessionDescription> mutant = desc.Clone();
-  DataContentDescription* dcdesc =
-      mutant->GetContentDescriptionByName(kDataContentName)->as_data();
-  std::vector<cricket::DataCodec> codecs(dcdesc->codecs());
-  EXPECT_EQ(1U, codecs.size());
-  EXPECT_EQ(cricket::kGoogleSctpDataCodecPlType, codecs[0].id);
-  codecs[0].SetParam(cricket::kCodecParamPort, kUnusualSctpPort);
-  dcdesc->set_codecs(codecs);
-
-  ASSERT_TRUE(
-      jdesc->Initialize(std::move(mutant), kSessionId, kSessionVersion));
-}
-
 TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelAndUnusualPort) {
   bool use_sctpmap = true;
   AddSctpDataChannel(use_sctpmap);
 
   // First setup the expected JsepSessionDescription.
   JsepSessionDescription jdesc(kDummyType);
-  MutateJsepSctpPort(&jdesc, desc_);
+  MutateJsepSctpPort(&jdesc, desc_, kUnusualSctpPort);
 
   // Then get the deserialized JsepSessionDescription.
   std::string sdp_with_data = kSdpString;
@@ -2979,7 +2984,7 @@ TEST_F(WebRtcSdpTest,
   AddSctpDataChannel(use_sctpmap);
 
   JsepSessionDescription jdesc(kDummyType);
-  MutateJsepSctpPort(&jdesc, desc_);
+  MutateJsepSctpPort(&jdesc, desc_, kUnusualSctpPort);
 
   // We need to test the deserialized JsepSessionDescription from
   // kSdpSctpDataChannelStringWithSctpPort for
@@ -3015,7 +3020,7 @@ TEST_F(WebRtcSdpTest, DeserializeSdpWithSctpDataChannelsAndBandwidth) {
   bool use_sctpmap = true;
   AddSctpDataChannel(use_sctpmap);
   JsepSessionDescription jdesc(kDummyType);
-  DataContentDescription* dcd = GetFirstDataContentDescription(&desc_);
+  SctpDataContentDescription* dcd = GetFirstSctpDataContentDescription(&desc_);
   dcd->set_bandwidth(100 * 1000);
   ASSERT_TRUE(jdesc.Initialize(desc_.Clone(), kSessionId, kSessionVersion));