Browse Source

Enforce candidates uniqueness in description

Paul-Louis Ageneau 4 years ago
parent
commit
100039eba8
3 changed files with 28 additions and 11 deletions
  1. 1 0
      include/rtc/description.hpp
  2. 15 4
      src/description.cpp
  3. 12 7
      src/peerconnection.cpp

+ 1 - 0
include/rtc/description.hpp

@@ -53,6 +53,7 @@ public:
 	void hintType(Type type);
 	void hintType(Type type);
 	void setFingerprint(string fingerprint);
 	void setFingerprint(string fingerprint);
 
 
+	bool hasCandidate(const Candidate &candidate) const;
 	void addCandidate(Candidate candidate);
 	void addCandidate(Candidate candidate);
 	void addCandidates(std::vector<Candidate> candidates);
 	void addCandidates(std::vector<Candidate> candidates);
 	void endCandidates();
 	void endCandidates();

+ 15 - 4
src/description.cpp

@@ -171,16 +171,27 @@ void Description::setFingerprint(string fingerprint) {
 	mFingerprint.emplace(std::move(fingerprint));
 	mFingerprint.emplace(std::move(fingerprint));
 }
 }
 
 
+bool Description::hasCandidate(const Candidate &candidate) const {
+	for (const Candidate &existing : mCandidates)
+		if (existing == candidate)
+			return true;
+
+	return false;
+}
+
 void Description::addCandidate(Candidate candidate) {
 void Description::addCandidate(Candidate candidate) {
 	candidate.hintMid(bundleMid());
 	candidate.hintMid(bundleMid());
+
+	for (Candidate &other : mCandidates)
+		if (candidate == other)
+			return;
+
 	mCandidates.emplace_back(std::move(candidate));
 	mCandidates.emplace_back(std::move(candidate));
 }
 }
 
 
 void Description::addCandidates(std::vector<Candidate> candidates) {
 void Description::addCandidates(std::vector<Candidate> candidates) {
-	for (Candidate candidate : candidates) {
-		candidate.hintMid(bundleMid());
-		mCandidates.emplace_back(std::move(candidate));
-	}
+	for (Candidate candidate : candidates)
+		addCandidate(std::move(candidate));
 }
 }
 
 
 void Description::endCandidates() { mEnded = true; }
 void Description::endCandidates() { mEnded = true; }

+ 12 - 7
src/peerconnection.cpp

@@ -1079,7 +1079,7 @@ void PeerConnection::processLocalCandidate(Candidate candidate) {
 	if (!mLocalDescription)
 	if (!mLocalDescription)
 		throw std::logic_error("Got a local candidate without local description");
 		throw std::logic_error("Got a local candidate without local description");
 
 
-	candidate.resolve(Candidate::ResolveMode::Simple); // for proper SDP generation later
+	candidate.resolve(Candidate::ResolveMode::Simple);
 	mLocalDescription->addCandidate(candidate);
 	mLocalDescription->addCandidate(candidate);
 
 
 	PLOG_VERBOSE << "Issuing local candidate: " << candidate;
 	PLOG_VERBOSE << "Issuing local candidate: " << candidate;
@@ -1116,21 +1116,26 @@ void PeerConnection::processRemoteCandidate(Candidate candidate) {
 
 
 	candidate.hintMid(mRemoteDescription->bundleMid());
 	candidate.hintMid(mRemoteDescription->bundleMid());
 
 
+	if (mRemoteDescription->hasCandidate(candidate))
+		return; // already in description, ignore
+
 	if (candidate.resolve(Candidate::ResolveMode::Simple)) {
 	if (candidate.resolve(Candidate::ResolveMode::Simple)) {
-		iceTransport->addRemoteCandidate(candidate);
+		mRemoteDescription->addCandidate(candidate);
+		iceTransport->addRemoteCandidate(std::move(candidate));
 	} else {
 	} else {
+		mRemoteDescription->addCandidate(candidate); // still add the unresolved candidate
+
 		// OK, we might need a lookup, do it asynchronously
 		// OK, we might need a lookup, do it asynchronously
 		// We don't use the thread pool because we have no control on the timeout
 		// We don't use the thread pool because we have no control on the timeout
 		weak_ptr<IceTransport> weakIceTransport{iceTransport};
 		weak_ptr<IceTransport> weakIceTransport{iceTransport};
-		std::thread t([weakIceTransport, candidate]() mutable {
-			if (candidate.resolve(Candidate::ResolveMode::Lookup))
+		std::thread t([weakIceTransport, candidate = std::move(candidate)]() mutable {
+			if (candidate.resolve(Candidate::ResolveMode::Lookup)) {
 				if (auto iceTransport = weakIceTransport.lock())
 				if (auto iceTransport = weakIceTransport.lock())
-					iceTransport->addRemoteCandidate(candidate);
+					iceTransport->addRemoteCandidate(std::move(candidate));
+			}
 		});
 		});
 		t.detach();
 		t.detach();
 	}
 	}
-
-	mRemoteDescription->addCandidate(std::move(candidate));
 }
 }
 
 
 void PeerConnection::triggerDataChannel(weak_ptr<DataChannel> weakDataChannel) {
 void PeerConnection::triggerDataChannel(weak_ptr<DataChannel> weakDataChannel) {