WIP

src/impl/peerconnection.cpp

@@ -69,7 +69,6 @@ PeerConnection::~PeerConnection() {
 }
 
 void PeerConnection::close() {
-	negotiationNeeded = false;
 	if (!closing.exchange(true)) {
 		PLOG_VERBOSE << "Closing PeerConnection";
 		if (auto transport = std::atomic_load(&mSctpTransport))
@@ -802,25 +801,27 @@ void PeerConnection::iterateTracks(std::function<void(shared_ptr<Track> track)>
 
 void PeerConnection::openTracks() {
 #if RTC_ENABLE_MEDIA
-	if (auto transport = std::atomic_load(&mDtlsTransport)) {
-		auto srtpTransport = std::dynamic_pointer_cast<DtlsSrtpTransport>(transport);
-
-		iterateTracks([&](const shared_ptr<Track> &track) {
-			if (!track->isOpen()) {
-				if (srtpTransport) {
-					track->open(srtpTransport);
-				} else {
-					// A track was added during a latter renegotiation, whereas SRTP transport was
-					// not initialized. This is an optimization to use the library with data
-					// channels only. Set forceMediaTransport to true to initialize the transport
-					// before dynamically adding tracks.
-					auto errorMsg = "The connection has no media transport";
-					PLOG_ERROR << errorMsg;
-					track->triggerError(errorMsg);
-				}
+	auto remote = remoteDescription();
+	auto transport = std::atomic_load(&mDtlsTransport);
+	if (!remote || !transport)
+		return;
+
+	auto srtpTransport = std::dynamic_pointer_cast<DtlsSrtpTransport>(transport);
+	iterateTracks([&](const shared_ptr<Track> &track) {
+		if (remote->hasMid(track->mid()) && !track->isOpen()) {
+			if (srtpTransport) {
+				track->open(srtpTransport);
+			} else {
+				// A track was added during a latter renegotiation, whereas SRTP transport was
+				// not initialized. This is an optimization to use the library with data
+				// channels only. Set forceMediaTransport to true to initialize the transport
+				// before dynamically adding tracks.
+				auto errorMsg = "The connection has no media transport";
+				PLOG_ERROR << errorMsg;
+				track->triggerError(errorMsg);
 			}
-		});
-	}
+		}
+	});
 #endif
 }
 
@@ -1003,8 +1004,7 @@ void PeerConnection::processLocalDescription(Description description) {
 			}
 		}
 
-		// There might be no media at this point if the user created a Track, deleted it,
-		// then called setLocalDescription().
+		// There might be no media at this point, for instance if the user deleted tracks
 		if (description.mediaCount() == 0)
 			throw std::runtime_error("No DataChannel or Track to negotiate");
 	}
@@ -1132,6 +1132,27 @@ string PeerConnection::localBundleMid() const {
 	return mLocalDescription ? mLocalDescription->bundleMid() : "0";
 }
 
+bool PeerConnection::negotiationNeeded() const {
+	auto description = localDescription();
+
+	{
+		std::shared_lock lock(mDataChannelsMutex);
+		if (!mDataChannels.empty() || !mUnassignedDataChannels.empty())
+			if((!description || !description->hasApplication()))
+				return true;
+	}
+
+	{
+		std::shared_lock lock(mTracksMutex);
+		for (auto it = mTrackLines.begin(); it != mTrackLines.end(); ++it)
+			if (auto track = it->lock())
+				if (!description || !description->hasMid(track->mid()))
+					return true;
+	}
+
+	return false;
+}
+
 void PeerConnection::setMediaHandler(shared_ptr<MediaHandler> handler) {
 	std::unique_lock lock(mMediaHandlerMutex);
 	mMediaHandler = handler;

src/impl/peerconnection.hpp

@@ -80,6 +80,8 @@ struct PeerConnection : std::enable_shared_from_this<PeerConnection> {
 	void processRemoteCandidate(Candidate candidate);
 	string localBundleMid() const;
 
+	bool negotiationNeeded() const;
+
 	void setMediaHandler(shared_ptr<MediaHandler> handler);
 	shared_ptr<MediaHandler> getMediaHandler();
 
@@ -113,7 +115,6 @@ struct PeerConnection : std::enable_shared_from_this<PeerConnection> {
 	std::atomic<IceState> iceState = IceState::New;
 	std::atomic<GatheringState> gatheringState = GatheringState::New;
 	std::atomic<SignalingState> signalingState = SignalingState::Stable;
-	std::atomic<bool> negotiationNeeded = false;
 	std::atomic<bool> closing = false;
 	std::mutex signalingMutex;
 
@@ -148,12 +149,12 @@ struct PeerConnection : std::enable_shared_from_this<PeerConnection> {
 
 	std::unordered_map<uint16_t, weak_ptr<DataChannel>> mDataChannels; // by stream ID
 	std::vector<weak_ptr<DataChannel>> mUnassignedDataChannels;
-	std::shared_mutex mDataChannelsMutex;
+	mutable std::shared_mutex mDataChannelsMutex;
 
 	std::unordered_map<string, weak_ptr<Track>> mTracks;         // by mid
 	std::unordered_map<uint32_t, weak_ptr<Track>> mTracksBySsrc; // by SSRC
 	std::vector<weak_ptr<Track>> mTrackLines;                    // by SDP order
-	std::shared_mutex mTracksMutex;
+	mutable std::shared_mutex mTracksMutex;
 
 	Queue<shared_ptr<DataChannel>> mPendingDataChannels;
 	Queue<shared_ptr<Track>> mPendingTracks;

src/peerconnection.cpp

@@ -99,7 +99,7 @@ void PeerConnection::setLocalDescription(Description::Type type) {
 	}
 
 	// Only a local offer resets the negotiation needed flag
-	if (type == Description::Type::Offer && !impl()->negotiationNeeded.exchange(false)) {
+	if (type == Description::Type::Offer && !impl()->negotiationNeeded()) {
 		PLOG_DEBUG << "No negotiation needed";
 		return;
 	}
@@ -146,9 +146,11 @@ void PeerConnection::setLocalDescription(Description::Type type) {
 	impl()->changeSignalingState(newSignalingState);
 	signalingLock.unlock();
 
-	if (impl()->gatheringState == GatheringState::New) {
+	if (impl()->gatheringState == GatheringState::New)
 		iceTransport->gatherLocalCandidates(impl()->localBundleMid());
-	}
+
+	if(newSignalingState == SignalingState::Stable && impl()->negotiationNeeded())
+		setLocalDescription(Description::Type::Offer);
 }
 
 void PeerConnection::setRemoteDescription(Description description) {
@@ -241,6 +243,9 @@ void PeerConnection::setRemoteDescription(Description description) {
 
 	for (const auto &candidate : remoteCandidates)
 		addRemoteCandidate(candidate);
+
+	if(newSignalingState == SignalingState::Stable && impl()->negotiationNeeded())
+		setLocalDescription();
 }
 
 void PeerConnection::addRemoteCandidate(Candidate candidate) {
@@ -271,11 +276,6 @@ shared_ptr<DataChannel> PeerConnection::createDataChannel(string label, DataChan
 	auto channelImpl = impl()->emplaceDataChannel(std::move(label), std::move(init));
 	auto channel = std::make_shared<DataChannel>(channelImpl);
 
-	// Renegotiation is needed iff the current local description does not have application
-	auto local = impl()->localDescription();
-	if (!local || !local->hasApplication())
-		impl()->negotiationNeeded = true;
-
 	if (!impl()->config.disableAutoNegotiation)
 		setLocalDescription();
 
@@ -292,9 +292,6 @@ std::shared_ptr<Track> PeerConnection::addTrack(Description::Media description)
 	auto trackImpl = impl()->emplaceTrack(std::move(description));
 	auto track = std::make_shared<Track>(trackImpl);
 
-	// Renegotiation is needed for the new or updated track
-	impl()->negotiationNeeded = true;
-
 	return track;
 }