diff --git a/webrtc/api/datachannel.cc b/webrtc/api/datachannel.cc index 6b13bf2c70..066e60c2ee 100644 --- a/webrtc/api/datachannel.cc +++ b/webrtc/api/datachannel.cc @@ -57,7 +57,8 @@ void SctpSidAllocator::ReleaseSid(int sid) { } bool SctpSidAllocator::IsSidAvailable(int sid) const { - if (sid < 0 || sid > static_cast(cricket::kMaxSctpSid)) { + if (sid < static_cast(cricket::kMinSctpSid) || + sid > static_cast(cricket::kMaxSctpSid)) { return false; } return used_sids_.find(sid) == used_sids_.end(); diff --git a/webrtc/api/webrtcsdp.cc b/webrtc/api/webrtcsdp.cc index 0b8d6f6a3d..7238131cfe 100644 --- a/webrtc/api/webrtcsdp.cc +++ b/webrtc/api/webrtcsdp.cc @@ -1391,7 +1391,7 @@ void BuildSctpContentAttributes(std::string* message, int sctp_port) { InitAttrLine(kAttributeSctpmap, &os); os << kSdpDelimiterColon << sctp_port << kSdpDelimiterSpace << kDefaultSctpmapProtocol << kSdpDelimiterSpace - << (cricket::kMaxSctpSid + 1); + << cricket::kMaxSctpStreams; AddLine(os.str(), message); } diff --git a/webrtc/media/sctp/sctpdataengine.cc b/webrtc/media/sctp/sctpdataengine.cc index fa70ab16af..08b441afa8 100644 --- a/webrtc/media/sctp/sctpdataengine.cc +++ b/webrtc/media/sctp/sctpdataengine.cc @@ -284,12 +284,9 @@ void InitializeUsrSctp() { // See: http://svnweb.freebsd.org/base?view=revision&revision=229805 // usrsctp_sysctl_set_sctp_blackhole(2); - // Set the number of default outgoing streams. This is the number we'll - // send in the SCTP INIT message. The 'appropriate default' in the - // second paragraph of - // http://tools.ietf.org/html/draft-ietf-rtcweb-data-channel-05#section-6.2 - // is kMaxSctpSid. - usrsctp_sysctl_set_sctp_nr_outgoing_streams_default(kMaxSctpSid); + // Set the number of default outgoing streams. This is the number we'll + // send in the SCTP INIT message. + usrsctp_sysctl_set_sctp_nr_outgoing_streams_default(kMaxSctpStreams); } void UninitializeUsrSctp() { @@ -751,23 +748,22 @@ bool SctpDataMediaChannel::AddStream(const StreamParams& stream) { } const uint32_t ssrc = stream.first_ssrc(); - if (ssrc >= kMaxSctpSid) { + if (ssrc > kMaxSctpSid) { LOG(LS_WARNING) << debug_name_ << "->Add(Send|Recv)Stream(...): " << "Not adding data stream '" << stream.id - << "' with ssrc=" << ssrc - << " because stream ssrc is too high."; + << "' with sid=" << ssrc << " because sid is too high."; return false; } else if (open_streams_.find(ssrc) != open_streams_.end()) { LOG(LS_WARNING) << debug_name_ << "->Add(Send|Recv)Stream(...): " << "Not adding data stream '" << stream.id - << "' with ssrc=" << ssrc + << "' with sid=" << ssrc << " because stream is already open."; return false; } else if (queued_reset_streams_.find(ssrc) != queued_reset_streams_.end() || sent_reset_streams_.find(ssrc) != sent_reset_streams_.end()) { LOG(LS_WARNING) << debug_name_ << "->Add(Send|Recv)Stream(...): " << "Not adding data stream '" << stream.id - << "' with ssrc=" << ssrc + << "' with sid=" << ssrc << " because stream is still closing."; return false; } diff --git a/webrtc/media/sctp/sctpdataengine.h b/webrtc/media/sctp/sctpdataengine.h index f9a3b5a654..6591383916 100644 --- a/webrtc/media/sctp/sctpdataengine.h +++ b/webrtc/media/sctp/sctpdataengine.h @@ -37,9 +37,19 @@ struct sctp_stream_reset_event; // Defined by struct socket; namespace cricket { -// The highest stream ID (Sid) that SCTP allows, and the number of streams we -// tell SCTP we're going to use. -const uint32_t kMaxSctpSid = 1023; +// The number of outgoing streams that we'll negotiate. Since stream IDs (SIDs) +// are 0-based, the highest usable SID is 1023. +// +// It's recommended to use the maximum of 65535 in: +// https://tools.ietf.org/html/draft-ietf-rtcweb-data-channel-13#section-6.2 +// However, we use 1024 in order to save memory. usrsctp allocates 104 bytes +// for each pair of incoming/outgoing streams (on a 64-bit system), so 65535 +// streams would waste ~6MB. +// +// Note: "max" and "min" here are inclusive. +constexpr uint16_t kMaxSctpStreams = 1024; +constexpr uint16_t kMaxSctpSid = kMaxSctpStreams - 1; +constexpr uint16_t kMinSctpSid = 0; // 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 diff --git a/webrtc/media/sctp/sctpdataengine_unittest.cc b/webrtc/media/sctp/sctpdataengine_unittest.cc index 51eb27a416..b32778d8c3 100644 --- a/webrtc/media/sctp/sctpdataengine_unittest.cc +++ b/webrtc/media/sctp/sctpdataengine_unittest.cc @@ -490,8 +490,8 @@ TEST_F(SctpDataMediaChannelTest, EngineSignalsRightChannel) { TEST_F(SctpDataMediaChannelTest, RefusesHighNumberedChannels) { SetupConnectedChannels(); - EXPECT_TRUE(AddStream(1022)); - EXPECT_FALSE(AddStream(1023)); + EXPECT_TRUE(AddStream(kMaxSctpSid)); + EXPECT_FALSE(AddStream(kMaxSctpSid + 1)); } // Flaky, see webrtc:4453. diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index 28da7315dd..65fe363f8e 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -28,12 +28,6 @@ #include "webrtc/pc/channelmanager.h" #include "webrtc/pc/srtpfilter.h" -#ifdef HAVE_SCTP -#include "webrtc/media/sctp/sctpdataengine.h" -#else -static const uint32_t kMaxSctpSid = 1023; -#endif - namespace { const char kInline[] = "inline:"; @@ -282,33 +276,6 @@ static void GenerateSsrcs(const StreamParamsVec& params_vec, } } -// Returns false if we exhaust the range of SIDs. -static bool GenerateSctpSid(const StreamParamsVec& params_vec, uint32_t* sid) { - if (params_vec.size() > kMaxSctpSid) { - LOG(LS_WARNING) << - "Could not generate an SCTP SID: too many SCTP streams."; - return false; - } - while (true) { - uint32_t candidate = rtc::CreateRandomNonZeroId() % kMaxSctpSid; - if (!GetStreamBySsrc(params_vec, candidate)) { - *sid = candidate; - return true; - } - } -} - -static bool GenerateSctpSids(const StreamParamsVec& params_vec, - std::vector* sids) { - uint32_t sid; - if (!GenerateSctpSid(params_vec, &sid)) { - LOG(LS_WARNING) << "Could not generated an SCTP SID."; - return false; - } - sids->push_back(sid); - return true; -} - // Finds all StreamParams of all media types and attach them to stream_params. static void GetCurrentStreamParams(const SessionDescription* sdesc, StreamParamsVec* stream_params) { @@ -454,6 +421,11 @@ static bool AddStreamParams(MediaType media_type, StreamParamsVec* current_streams, MediaContentDescriptionImpl* content_description, const bool add_legacy_stream) { + // SCTP streams are not negotiated using SDP/ContentDescriptions. + if (IsSctp(content_description)) { + return true; + } + const bool include_rtx_streams = ContainsRtxCodec(content_description->codecs()); @@ -461,12 +433,8 @@ static bool AddStreamParams(MediaType media_type, if (streams.empty() && add_legacy_stream) { // TODO(perkj): Remove this legacy stream when all apps use StreamParams. std::vector ssrcs; - if (IsSctp(content_description)) { - GenerateSctpSids(*current_streams, &ssrcs); - } else { - int num_ssrcs = include_rtx_streams ? 2 : 1; - GenerateSsrcs(*current_streams, num_ssrcs, &ssrcs); - } + int num_ssrcs = include_rtx_streams ? 2 : 1; + GenerateSsrcs(*current_streams, num_ssrcs, &ssrcs); if (include_rtx_streams) { content_description->AddLegacyStream(ssrcs[0], ssrcs[1]); content_description->set_multistream(true); @@ -489,11 +457,7 @@ static bool AddStreamParams(MediaType media_type, if (!param) { // This is a new stream. std::vector ssrcs; - if (IsSctp(content_description)) { - GenerateSctpSids(*current_streams, &ssrcs); - } else { - GenerateSsrcs(*current_streams, stream_it->num_sim_layers, &ssrcs); - } + GenerateSsrcs(*current_streams, stream_it->num_sim_layers, &ssrcs); StreamParams stream_param; stream_param.id = stream_it->id; // Add the generated ssrc.