Merge pull request #141 from paullouisageneau/fix-sdp-media

Fix media SDP generation

include/rtc/description.hpp

@@ -43,6 +43,7 @@ public:
 	Role role() const;
 	string roleString() const;
 	string dataMid() const;
+	string bundleMid() const;
 	std::optional<string> fingerprint() const;
 	std::optional<uint16_t> sctpPort() const;
 	std::optional<size_t> maxMessageSize() const;
@@ -63,6 +64,7 @@ public:
 
 	operator string() const;
 	string generateSdp(const string &eol) const;
+	string generateDataSdp(const string &eol) const;
 
 private:
 	Type mType;

src/description.cpp

@@ -150,6 +150,14 @@ string Description::roleString() const { return roleToString(mRole); }
 
 string Description::dataMid() const { return mData.mid; }
 
+string Description::bundleMid() const {
+	// Get the mid of the first media
+	if (auto it = mMedia.find(0); it != mMedia.end())
+		return it->second.mid;
+	else
+		return mData.mid;
+}
+
 std::optional<string> Description::fingerprint() const { return mFingerprint; }
 
 std::optional<uint16_t> Description::sctpPort() const { return mData.sctpPort; }
@@ -199,9 +207,6 @@ void Description::addMedia(const Description &source) {
 Description::operator string() const { return generateSdp("\r\n"); }
 
 string Description::generateSdp(const string &eol) const {
-	if (!mFingerprint)
-		throw std::logic_error("Fingerprint must be set to generate an SDP string");
-
 	std::ostringstream sdp;
 
 	// Header
@@ -221,8 +226,6 @@ string Description::generateSdp(const string &eol) const {
 			sdp << ' ' << mData.mid;
 	sdp << eol;
 
-	sdp << "a=msid-semantic: WMS" << eol;
-
 	// Non-data media
 	if (!mMedia.empty()) {
 		// Lip-sync
@@ -232,7 +235,19 @@ string Description::generateSdp(const string &eol) const {
 		sdp << eol;
 	}
 
-	// Descriptions and attributes
+	// Session-level attributes
+	sdp << "a=msid-semantic:WMS *" << eol;
+	sdp << "a=setup:" << roleToString(mRole) << eol;
+	sdp << "a=ice-ufrag:" << mIceUfrag << eol;
+	sdp << "a=ice-pwd:" << mIcePwd << eol;
+
+	if (!mEnded)
+		sdp << "a=ice-options:trickle" << eol;
+
+	if (mFingerprint)
+		sdp << "a=fingerprint:sha-256 " << *mFingerprint << eol;
+
+	// Media descriptions and attributes
 	for (int i = 0; i < int(mMedia.size() + 1); ++i) {
 		if (auto it = mMedia.find(i); it != mMedia.end()) {
 			// Non-data media
@@ -252,6 +267,7 @@ string Description::generateSdp(const string &eol) const {
 			if (!mMedia.empty())
 				sdp << "a=bundle-only" << eol;
 			sdp << "a=mid:" << mData.mid << eol;
+			sdp << "a=sendrecv" << eol;
 			if (mData.sctpPort)
 				sdp << "a=sctp-port:" << *mData.sctpPort << eol;
 			if (mData.maxMessageSize)
@@ -259,14 +275,41 @@ string Description::generateSdp(const string &eol) const {
 		}
 	}
 
-	// Common
-	if (!mEnded)
-		sdp << "a=ice-options:trickle" << eol;
+	// Candidates
+	for (const auto &candidate : mCandidates)
+		sdp << string(candidate) << eol;
 
+	if (mEnded)
+		sdp << "a=end-of-candidates" << eol;
+
+	return sdp.str();
+}
+
+string Description::generateDataSdp(const string &eol) const {
+	std::ostringstream sdp;
+
+	// Header
+	sdp << "v=0" << eol;
+	sdp << "o=- " << mSessionId << " 0 IN IP4 127.0.0.1" << eol;
+	sdp << "s=-" << eol;
+	sdp << "t=0 0" << eol;
+
+	// Data
+	sdp << "m=application 9 UDP/DTLS/SCTP webrtc-datachannel";
+	sdp << "c=IN IP4 0.0.0.0" << eol;
+	sdp << "a=mid:" << mData.mid << eol;
+	sdp << "a=sendrecv" << eol;
+	if (mData.sctpPort)
+		sdp << "a=sctp-port:" << *mData.sctpPort << eol;
+	if (mData.maxMessageSize)
+		sdp << "a=max-message-size:" << *mData.maxMessageSize << eol;
+
+	sdp << "a=setup:" << roleToString(mRole) << eol;
 	sdp << "a=ice-ufrag:" << mIceUfrag << eol;
 	sdp << "a=ice-pwd:" << mIcePwd << eol;
-	sdp << "a=setup:" << roleToString(mRole) << eol;
-	sdp << "a=tls-id:1" << eol;
+
+	if (!mEnded)
+		sdp << "a=ice-options:trickle" << eol;
 
 	if (mFingerprint)
 		sdp << "a=fingerprint:sha-256 " << *mFingerprint << eol;
@@ -274,6 +317,7 @@ string Description::generateSdp(const string &eol) const {
 	// Candidates
 	for (const auto &candidate : mCandidates)
 		sdp << string(candidate) << eol;
+
 	if (mEnded)
 		sdp << "a=end-of-candidates" << eol;
 

src/icetransport.cpp

@@ -123,9 +123,9 @@ Description IceTransport::getLocalDescription(Description::Type type) const {
 void IceTransport::setRemoteDescription(const Description &description) {
 	mRole = description.role() == Description::Role::Active ? Description::Role::Passive
 	                                                        : Description::Role::Active;
-	mMid = description.dataMid();
-	if (juice_set_remote_description(mAgent.get(), string(description).c_str()) < 0)
-		throw std::runtime_error("Failed to parse remote SDP");
+	mMid = description.bundleMid();
+	if (juice_set_remote_description(mAgent.get(), description.generateDataSdp("\r\n").c_str()) < 0)
+		throw std::runtime_error("Failed to parse ICE settings from remote SDP");
 }
 
 bool IceTransport::addRemoteCandidate(const Candidate &candidate) {
@@ -483,12 +483,13 @@ Description IceTransport::getLocalDescription(Description::Type type) const {
 void IceTransport::setRemoteDescription(const Description &description) {
 	mRole = description.role() == Description::Role::Active ? Description::Role::Passive
 	                                                        : Description::Role::Active;
-	mMid = description.dataMid();
+	mMid = description.bundleMid();
 	mTrickleTimeout = !description.ended() ? 30s : 0s;
 
 	// Warning: libnice expects "\n" as end of line
-	if (nice_agent_parse_remote_sdp(mNiceAgent.get(), description.generateSdp("\n").c_str()) < 0)
-		throw std::runtime_error("Failed to parse remote SDP");
+	if (nice_agent_parse_remote_sdp(mNiceAgent.get(), description.generateDataSdp("\n").c_str()) <
+	    0)
+		throw std::runtime_error("Failed to parse ICE settings from remote SDP");
 }
 
 bool IceTransport::addRemoteCandidate(const Candidate &candidate) {

src/peerconnection.cpp

@@ -102,6 +102,9 @@ void PeerConnection::setRemoteDescription(Description description,
                                           std::optional<Description> mediaDescription) {
 	PLOG_VERBOSE << "Setting remote description: " << string(description);
 
+	if (!description.fingerprint())
+		throw std::runtime_error("Remote description is incomplete");
+
 	description.hintType(localDescription() ? Description::Type::Answer : Description::Type::Offer);
 	auto type = description.type();
 	auto remoteCandidates = description.extractCandidates(); // Candidates will be added at the end

src/sctptransport.cpp

@@ -600,7 +600,7 @@ void SctpTransport::processNotification(const union sctp_notification *notify, s
 	}
 
 	auto type = notify->sn_header.sn_type;
-	PLOG_VERBOSE << "Process notification, type=" << type;
+	PLOG_VERBOSE << "Processing notification, type=" << type;
 
 	switch (type) {
 	case SCTP_ASSOC_CHANGE: {
@@ -622,7 +622,8 @@ void SctpTransport::processNotification(const union sctp_notification *notify, s
 	}
 
 	case SCTP_SENDER_DRY_EVENT: {
-		// It not should be necessary since the send callback should have been called already,
+		PLOG_VERBOSE << "SCTP dry event";
+		// It should not be necessary since the send callback should have been called already,
 		// but to be sure, let's try to send now.
 		safeFlush();
 		break;
@@ -633,6 +634,28 @@ void SctpTransport::processNotification(const union sctp_notification *notify, s
 		const int count = (reset_event.strreset_length - sizeof(reset_event)) / sizeof(uint16_t);
 		const uint16_t flags = reset_event.strreset_flags;
 
+		IF_PLOG(plog::verbose) {
+			std::ostringstream desc;
+			desc << "flags=";
+			if (flags & SCTP_STREAM_RESET_OUTGOING_SSN && flags & SCTP_STREAM_RESET_INCOMING_SSN)
+				desc << "outgoing|incoming";
+			else if (flags & SCTP_STREAM_RESET_OUTGOING_SSN)
+				desc << "outgoing";
+			else if (flags & SCTP_STREAM_RESET_INCOMING_SSN)
+				desc << "incoming";
+			else
+				desc << "0";
+
+			desc << ", streams=[";
+			for (int i = 0; i < count; ++i) {
+				uint16_t streamId = reset_event.strreset_stream_list[i];
+				desc << (i != 0 ? "," : "") << streamId;
+			}
+			desc << "]";
+
+			PLOG_VERBOSE << "SCTP reset event, " << desc.str();
+		}
+
 		if (flags & SCTP_STREAM_RESET_OUTGOING_SSN) {
 			for (int i = 0; i < count; ++i) {
 				uint16_t streamId = reset_event.strreset_stream_list[i];