Sets the SCTP port codec in the native SessionDescription.
Previously it's only set when a SDP string is parsed into SessionDescription, causing failuring for native client.
BUG=3141
R=juberti@webrtc.org, wu@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/11999004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@6036 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc
index bfee48c..f6003f6 100644
--- a/talk/app/webrtc/peerconnection_unittest.cc
+++ b/talk/app/webrtc/peerconnection_unittest.cc
@@ -1345,6 +1345,20 @@
kMaxWaitMs);
}
+// This test sets up a Jsep call with SCTP DataChannel and verifies the
+// negotiation is completed without error.
+#ifdef HAVE_SCTP
+TEST_F(JsepPeerConnectionP2PTestClient, CreateOfferWithSctpDataChannel) {
+ MAYBE_SKIP_TEST(talk_base::SSLStreamAdapter::HaveDtlsSrtp);
+ FakeConstraints constraints;
+ constraints.SetMandatory(
+ MediaConstraintsInterface::kEnableDtlsSrtp, true);
+ ASSERT_TRUE(CreateTestClients(&constraints, &constraints));
+ initializing_client()->CreateDataChannel();
+ initializing_client()->Negotiate(false, false);
+}
+#endif
+
// This test sets up a call between two parties with audio, and video.
// During the call, the initializing side restart ice and the test verifies that
// new ice candidates are generated and audio and video still can flow.
@@ -1406,6 +1420,4 @@
EnableVideoDecoderFactory();
LocalP2PTest();
}
-
#endif // if !defined(THREAD_SANITIZER)
-
diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc
index 2a773b6..e726dab 100644
--- a/talk/app/webrtc/webrtcsdp.cc
+++ b/talk/app/webrtc/webrtcsdp.cc
@@ -207,7 +207,6 @@
static const int kIsacWbDefaultRate = 32000; // From acm_common_defs.h
static const int kIsacSwbDefaultRate = 56000; // From acm_common_defs.h
-static const int kDefaultSctpFmt = 5000;
static const char kDefaultSctpmapProtocol[] = "webrtc-datachannel";
struct SsrcInfo {
@@ -240,7 +239,7 @@
const TransportInfo* transport_info,
const MediaType media_type,
std::string* message);
-static void BuildSctpContentAttributes(std::string* message);
+static void BuildSctpContentAttributes(std::string* message, int sctp_port);
static void BuildRtpContentAttributes(
const MediaContentDescription* media_desc,
const MediaType media_type,
@@ -1166,6 +1165,7 @@
ASSERT(media_desc != NULL);
bool is_sctp = (media_desc->protocol() == cricket::kMediaProtocolDtlsSctp);
+ int sctp_port = cricket::kSctpDefaultPort;
// RFC 4566
// m=<media> <port> <proto> <fmt>
@@ -1200,14 +1200,22 @@
fmt.append(talk_base::ToString<int>(it->id));
}
} else if (media_type == cricket::MEDIA_TYPE_DATA) {
+ const DataContentDescription* data_desc =
+ static_cast<const DataContentDescription*>(media_desc);
if (is_sctp) {
fmt.append(" ");
- // TODO(jiayl): Replace the hard-coded string with the fmt read out of the
- // ContentDescription.
- fmt.append(talk_base::ToString<int>(kDefaultSctpFmt));
+
+ for (std::vector<cricket::DataCodec>::const_iterator it =
+ data_desc->codecs().begin();
+ it != data_desc->codecs().end(); ++it) {
+ if (it->id == cricket::kGoogleSctpDataCodecId &&
+ it->GetParam(cricket::kCodecParamPort, &sctp_port)) {
+ break;
+ }
+ }
+
+ fmt.append(talk_base::ToString<int>(sctp_port));
} else {
- const DataContentDescription* data_desc =
- static_cast<const DataContentDescription*>(media_desc);
for (std::vector<cricket::DataCodec>::const_iterator it =
data_desc->codecs().begin();
it != data_desc->codecs().end(); ++it) {
@@ -1289,18 +1297,18 @@
AddLine(os.str(), message);
if (is_sctp) {
- BuildSctpContentAttributes(message);
+ BuildSctpContentAttributes(message, sctp_port);
} else {
BuildRtpContentAttributes(media_desc, media_type, message);
}
}
-void BuildSctpContentAttributes(std::string* message) {
+void BuildSctpContentAttributes(std::string* message, int sctp_port) {
// draft-ietf-mmusic-sctp-sdp-04
// a=sctpmap:sctpmap-number protocol [streams]
std::ostringstream os;
InitAttrLine(kAttributeSctpmap, &os);
- os << kSdpDelimiterColon << kDefaultSctpFmt << kSdpDelimiterSpace
+ os << kSdpDelimiterColon << sctp_port << kSdpDelimiterSpace
<< kDefaultSctpmapProtocol << kSdpDelimiterSpace
<< (cricket::kMaxSctpSid + 1);
AddLine(os.str(), message);
@@ -2166,6 +2174,7 @@
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);
}
diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc
index aec1447..e02b11f 100644
--- a/talk/app/webrtc/webrtcsdp_unittest.cc
+++ b/talk/app/webrtc/webrtcsdp_unittest.cc
@@ -299,7 +299,7 @@
"a=mid:data_content_name\r\n"
"a=sctpmap:5000 webrtc-datachannel 1024\r\n";
- static const char kSdpConferenceString[] =
+static const char kSdpConferenceString[] =
"v=0\r\n"
"o=- 18446744069414584320 18446462598732840960 IN IP4 127.0.0.1\r\n"
"s=-\r\n"
@@ -1466,6 +1466,37 @@
EXPECT_EQ(message, expected_sdp);
}
+TEST_F(WebRtcSdpTest, SerializeWithSctpDataChannelAndNewPort) {
+ AddSctpDataChannel();
+ JsepSessionDescription jsep_desc(kDummyString);
+
+ ASSERT_TRUE(jsep_desc.Initialize(desc_.Copy(), kSessionId, kSessionVersion));
+ DataContentDescription* dcdesc = static_cast<DataContentDescription*>(
+ jsep_desc.description()->GetContentDescriptionByName(kDataContentName));
+
+ const int kNewPort = 1234;
+ cricket::DataCodec codec(
+ cricket::kGoogleSctpDataCodecId, cricket::kGoogleSctpDataCodecName, 0);
+ codec.SetParam(cricket::kCodecParamPort, kNewPort);
+ dcdesc->AddOrReplaceCodec(codec);
+
+ std::string message = webrtc::SdpSerialize(jsep_desc);
+
+ std::string expected_sdp = kSdpString;
+ expected_sdp.append(kSdpSctpDataChannelString);
+
+ char default_portstr[16];
+ char new_portstr[16];
+ talk_base::sprintfn(default_portstr, sizeof(default_portstr), "%d",
+ kDefaultSctpPort);
+ talk_base::sprintfn(new_portstr, sizeof(new_portstr), "%d", kNewPort);
+ talk_base::replace_substrs(default_portstr, strlen(default_portstr),
+ new_portstr, strlen(new_portstr),
+ &expected_sdp);
+
+ EXPECT_EQ(expected_sdp, message);
+}
+
TEST_F(WebRtcSdpTest, SerializeSessionDescriptionWithDataChannelAndBandwidth) {
AddRtpDataChannel();
data_desc_->set_bandwidth(100*1000);
@@ -1898,6 +1929,7 @@
talk_base::sprintfn(unusual_portstr, sizeof(unusual_portstr), "%d",
kUnusualSctpPort);
+ // First setup the expected JsepSessionDescription.
JsepSessionDescription jdesc(kDummyString);
// take our pre-built session description and change the SCTP port.
cricket::SessionDescription* mutant = desc_.Copy();
@@ -1907,11 +1939,13 @@
EXPECT_EQ(codecs.size(), 1UL);
EXPECT_EQ(codecs[0].id, cricket::kGoogleSctpDataCodecId);
codecs[0].SetParam(cricket::kCodecParamPort, kUnusualSctpPort);
+ dcdesc->set_codecs(codecs);
// note: mutant's owned by jdesc now.
ASSERT_TRUE(jdesc.Initialize(mutant, kSessionId, kSessionVersion));
mutant = NULL;
+ // Then get the deserialized JsepSessionDescription.
std::string sdp_with_data = kSdpString;
sdp_with_data.append(kSdpSctpDataChannelString);
talk_base::replace_substrs(default_portstr, strlen(default_portstr),
diff --git a/talk/media/base/codec.h b/talk/media/base/codec.h
index 56fc975..120c17b 100644
--- a/talk/media/base/codec.h
+++ b/talk/media/base/codec.h
@@ -120,6 +120,8 @@
name = c.name;
clockrate = c.clockrate;
preference = c.preference;
+ params = c.params;
+ feedback_params = c.feedback_params;
return *this;
}
@@ -127,7 +129,9 @@
return this->id == c.id && // id is reserved in objective-c
name == c.name &&
clockrate == c.clockrate &&
- preference == c.preference;
+ preference == c.preference &&
+ params == c.params &&
+ feedback_params == c.feedback_params;
}
bool operator!=(const Codec& c) const {
diff --git a/talk/media/base/codec_unittest.cc b/talk/media/base/codec_unittest.cc
index f0ffd8f..f2bf4c7 100644
--- a/talk/media/base/codec_unittest.cc
+++ b/talk/media/base/codec_unittest.cc
@@ -40,6 +40,43 @@
CodecTest() {}
};
+TEST_F(CodecTest, TestCodecOperators) {
+ Codec c0(96, "D", 1000, 0);
+ c0.SetParam("a", 1);
+
+ Codec c1 = c0;
+ EXPECT_TRUE(c1 == c0);
+
+ int param_value0;
+ int param_value1;
+ EXPECT_TRUE(c0.GetParam("a", ¶m_value0));
+ EXPECT_TRUE(c1.GetParam("a", ¶m_value1));
+ EXPECT_EQ(param_value0, param_value1);
+
+ c1.id = 86;
+ EXPECT_TRUE(c0 != c1);
+
+ c1 = c0;
+ c1.name = "x";
+ EXPECT_TRUE(c0 != c1);
+
+ c1 = c0;
+ c1.clockrate = 2000;
+ EXPECT_TRUE(c0 != c1);
+
+ c1 = c0;
+ c1.preference = 1;
+ EXPECT_TRUE(c0 != c1);
+
+ c1 = c0;
+ c1.SetParam("a", 2);
+ EXPECT_TRUE(c0 != c1);
+
+ Codec c5;
+ Codec c6(0, "", 0, 0);
+ EXPECT_TRUE(c5 == c6);
+}
+
TEST_F(CodecTest, TestAudioCodecOperators) {
AudioCodec c0(96, "A", 44100, 20000, 2, 3);
AudioCodec c1(95, "A", 44100, 20000, 2, 3);
@@ -238,23 +275,6 @@
EXPECT_FALSE(c1.Matches(DataCodec(95, "D", 0)));
}
-TEST_F(CodecTest, TestDataCodecOperators) {
- DataCodec c0(96, "D", 3);
- DataCodec c1(95, "D", 3);
- DataCodec c2(96, "x", 3);
- DataCodec c3(96, "D", 1);
- EXPECT_TRUE(c0 != c1);
- EXPECT_TRUE(c0 != c2);
- EXPECT_TRUE(c0 != c3);
-
- DataCodec c4;
- DataCodec c5(0, "", 0);
- DataCodec c6 = c0;
- EXPECT_TRUE(c5 == c4);
- EXPECT_TRUE(c6 != c4);
- EXPECT_TRUE(c6 == c0);
-}
-
TEST_F(CodecTest, TestSetParamAndGetParam) {
AudioCodec codec;
codec.SetParam("a", "1");
diff --git a/talk/media/sctp/sctpdataengine.cc b/talk/media/sctp/sctpdataengine.cc
index 59e252a..75c2a4c 100644
--- a/talk/media/sctp/sctpdataengine.cc
+++ b/talk/media/sctp/sctpdataengine.cc
@@ -105,12 +105,6 @@
typedef talk_base::ScopedMessageData<SctpInboundPacket> InboundPacketMessage;
typedef talk_base::ScopedMessageData<talk_base::Buffer> OutboundPacketMessage;
-// This is the SCTP port to use. It is passed along the wire and the listener
-// and connector must be using the same port. It is not related to the ports at
-// the IP level. (Corresponds to: sockaddr_conn.sconn_port in usrsctp.h)
-//
-// TODO(ldixon): Allow port to be set from higher level code.
-static const int kSctpDefaultPort = 5001;
// TODO(ldixon): Find where this is defined, and also check is Sctp really
// respects this.
static const size_t kSctpMtu = 1280;
@@ -277,10 +271,9 @@
}
usrsctp_engines_count++;
- // We don't put in a codec because we don't want one offered when we
- // use the hybrid data engine.
- // codecs_.push_back(cricket::DataCodec( kGoogleSctpDataCodecId,
- // kGoogleSctpDataCodecName, 0));
+ cricket::DataCodec codec(kGoogleSctpDataCodecId, kGoogleSctpDataCodecName, 0);
+ codec.SetParam(kCodecParamPort, kSctpDefaultPort);
+ codecs_.push_back(codec);
}
SctpDataEngine::~SctpDataEngine() {
@@ -308,8 +301,8 @@
SctpDataMediaChannel::SctpDataMediaChannel(talk_base::Thread* thread)
: worker_thread_(thread),
- local_port_(-1),
- remote_port_(-1),
+ local_port_(kSctpDefaultPort),
+ remote_port_(kSctpDefaultPort),
sock_(NULL),
sending_(false),
receiving_(false),
@@ -423,12 +416,6 @@
bool SctpDataMediaChannel::Connect() {
LOG(LS_VERBOSE) << debug_name_ << "->Connect().";
- if (remote_port_ < 0) {
- remote_port_ = kSctpDefaultPort;
- }
- if (local_port_ < 0) {
- local_port_ = kSctpDefaultPort;
- }
// If we already have a socket connection, just return.
if (sock_) {
diff --git a/talk/media/sctp/sctpdataengine.h b/talk/media/sctp/sctpdataengine.h
index f2322ab..7561977 100644
--- a/talk/media/sctp/sctpdataengine.h
+++ b/talk/media/sctp/sctpdataengine.h
@@ -58,6 +58,12 @@
// tell SCTP we're going to use.
const uint32 kMaxSctpSid = 1023;
+// This is the default SCTP port to use. It is passed along the wire and the
+// connectee and connector must be using the same port. It is not related to the
+// ports at the IP level. (Corresponds to: sockaddr_conn.sconn_port in
+// usrsctp.h)
+const int kSctpDefaultPort = 5000;
+
// A DataEngine that interacts with usrsctp.
//
// From channel calls, data flows like this:
diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc
index 17f7a1a..8219ec8 100644
--- a/talk/session/media/mediasession.cc
+++ b/talk/session/media/mediasession.cc
@@ -308,6 +308,20 @@
}
}
+// Filters the data codecs for the data channel type.
+void FilterDataCodecs(std::vector<DataCodec>* codecs, bool sctp) {
+ // Filter RTP codec for SCTP and vice versa.
+ int codec_id = sctp ? kGoogleRtpDataCodecId : kGoogleSctpDataCodecId;
+ for (std::vector<DataCodec>::iterator iter = codecs->begin();
+ iter != codecs->end();) {
+ if (iter->id == codec_id) {
+ iter = codecs->erase(iter);
+ } else {
+ ++iter;
+ }
+ }
+}
+
template <typename IdStruct>
class UsedIds {
public:
@@ -1204,6 +1218,8 @@
scoped_ptr<DataContentDescription> data(new DataContentDescription());
bool is_sctp = (options.data_channel_type == DCT_SCTP);
+ FilterDataCodecs(&data_codecs, is_sctp);
+
cricket::SecurePolicy sdes_policy =
IsDtlsActive(CN_DATA, current_description) ?
cricket::SEC_DISABLED : secure();
@@ -1397,6 +1413,10 @@
if (!data_transport) {
return NULL;
}
+ 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.
diff --git a/talk/session/media/mediasession.h b/talk/session/media/mediasession.h
index dff254f..a130b15 100644
--- a/talk/session/media/mediasession.h
+++ b/talk/session/media/mediasession.h
@@ -318,6 +318,16 @@
void AddCodec(const C& codec) {
codecs_.push_back(codec);
}
+ void AddOrReplaceCodec(const C& codec) {
+ for (typename std::vector<C>::iterator iter = codecs_.begin();
+ iter != codecs_.end(); ++iter) {
+ if (iter->id == codec.id) {
+ *iter = codec;
+ return;
+ }
+ }
+ AddCodec(codec);
+ }
void AddCodecs(const std::vector<C>& codecs) {
typename std::vector<C>::const_iterator codec;
for (codec = codecs.begin(); codec != codecs.end(); ++codec) {
