Browse Source

Changed remote description logic to resolve candidates asyncronously

Paul-Louis Ageneau 5 years ago
parent
commit
44cdbab8dc

+ 4 - 1
include/rtc/candidate.hpp

@@ -29,9 +29,12 @@ class Candidate {
 public:
 	Candidate(string candidate, string mid = "");
 
+	enum class ResolveMode { Simple, Lookup };
+	bool resolve(ResolveMode mode = ResolveMode::Simple);
+	bool isResolved() const;
+
 	string candidate() const;
 	string mid() const;
-	bool isResolved() const;
 	operator string() const;
 
 private:

+ 2 - 0
include/rtc/description.hpp

@@ -47,8 +47,10 @@ public:
 
 	void setFingerprint(string fingerprint);
 	void setSctpPort(uint16_t port);
+
 	void addCandidate(Candidate candidate);
 	void endCandidates();
+	std::vector<Candidate> extractCandidates();
 
 	operator string() const;
 

+ 4 - 0
include/rtc/peerconnection.hpp

@@ -30,6 +30,8 @@
 
 #include <atomic>
 #include <functional>
+#include <list>
+#include <thread>
 #include <unordered_map>
 
 namespace rtc {
@@ -110,6 +112,8 @@ private:
 	std::atomic<State> mState;
 	std::atomic<GatheringState> mGatheringState;
 
+	std::list<std::thread> mResolveThreads;
+
 	std::function<void(std::shared_ptr<DataChannel> dataChannel)> mDataChannelCallback;
 	std::function<void(const Description &description)> mLocalDescriptionCallback;
 	std::function<void(const Candidate &candidate)> mLocalCandidateCallback;

+ 14 - 4
src/candidate.cpp

@@ -48,6 +48,11 @@ Candidate::Candidate(string candidate, string mid) : mIsResolved(false) {
 
 	mCandidate = std::move(candidate);
 	mMid = std::move(mid);
+}
+
+bool Candidate::resolve(ResolveMode mode) {
+	if (mIsResolved)
+		return true;
 
 	// See RFC 5245 for format
 	std::stringstream ss(mCandidate);
@@ -64,6 +69,10 @@ Candidate::Candidate(string candidate, string mid) : mIsResolved(false) {
 			hints.ai_socktype = SOCK_DGRAM;
 			hints.ai_protocol = IPPROTO_UDP;
 		}
+
+		if (mode == ResolveMode::Simple)
+			hints.ai_flags |= AI_NUMERICHOST;
+
 		struct addrinfo *result = nullptr;
 		if (getaddrinfo(node.c_str(), service.c_str(), &hints, &result) == 0) {
 			for (auto p = result; p; p = p->ai_next)
@@ -83,22 +92,23 @@ Candidate::Candidate(string candidate, string mid) : mIsResolved(false) {
 						if (!left.empty())
 							ss << left;
 						mCandidate = ss.str();
-						mIsResolved = true;
-						break;
+						return mIsResolved = true;
 					}
 				}
 		}
 
 		freeaddrinfo(result);
 	}
+
+	return false;
 }
 
+bool Candidate::isResolved() const { return mIsResolved; }
+
 string Candidate::candidate() const { return "candidate:" + mCandidate; }
 
 string Candidate::mid() const { return mMid; }
 
-bool Candidate::isResolved() const { return mIsResolved; }
-
 Candidate::operator string() const {
 	std::ostringstream line;
 	line << "a=" << candidate();

+ 7 - 2
src/description.cpp

@@ -110,12 +110,17 @@ void Description::setFingerprint(string fingerprint) {
 void Description::setSctpPort(uint16_t port) { mSctpPort.emplace(port); }
 
 void Description::addCandidate(Candidate candidate) {
-	if (candidate.isResolved())
-		mCandidates.emplace_back(std::move(candidate));
+	mCandidates.emplace_back(std::move(candidate));
 }
 
 void Description::endCandidates() { mTrickle = false; }
 
+std::vector<Candidate> Description::extractCandidates() {
+	std::vector<Candidate> result;
+	std::swap(mCandidates, result);
+	return result;
+}
+
 Description::operator string() const {
 	if (!mFingerprint)
 		throw std::logic_error("Fingerprint must be set to generate a SDP");

+ 9 - 9
src/icetransport.cpp

@@ -149,15 +149,6 @@ void IceTransport::setRemoteDescription(const Description &description) {
 		throw std::runtime_error("Failed to parse remote SDP");
 }
 
-void IceTransport::gatherLocalCandidates() {
-	// Change state now as candidates calls can be synchronous
-	changeGatheringState(GatheringState::InProgress);
-
-	if (!nice_agent_gather_candidates(mNiceAgent.get(), mStreamId)) {
-		throw std::runtime_error("Failed to gather local ICE candidates");
-	}
-}
-
 bool IceTransport::addRemoteCandidate(const Candidate &candidate) {
 	// Don't try to pass unresolved candidates to libnice for more safety
 	if (!candidate.isResolved())
@@ -178,6 +169,15 @@ bool IceTransport::addRemoteCandidate(const Candidate &candidate) {
 	return ret > 0;
 }
 
+void IceTransport::gatherLocalCandidates() {
+	// Change state now as candidates calls can be synchronous
+	changeGatheringState(GatheringState::InProgress);
+
+	if (!nice_agent_gather_candidates(mNiceAgent.get(), mStreamId)) {
+		throw std::runtime_error("Failed to gather local ICE candidates");
+	}
+}
+
 bool IceTransport::send(message_ptr message) {
 	if (!message || !mStreamId)
 		return false;

+ 1 - 1
src/icetransport.hpp

@@ -61,8 +61,8 @@ public:
 	GatheringState gatheringState() const;
 	Description getLocalDescription(Description::Type type) const;
 	void setRemoteDescription(const Description &description);
-	void gatherLocalCandidates();
 	bool addRemoteCandidate(const Candidate &candidate);
+	void gatherLocalCandidates();
 
 	bool send(message_ptr message);
 

+ 26 - 10
src/peerconnection.cpp

@@ -36,7 +36,10 @@ PeerConnection::PeerConnection() : PeerConnection(Configuration()) {}
 PeerConnection::PeerConnection(const Configuration &config)
     : mConfig(config), mCertificate(make_certificate("libdatachannel")), mState(State::New) {}
 
-PeerConnection::~PeerConnection() {}
+PeerConnection::~PeerConnection() {
+	for (auto &t : mResolveThreads)
+		t.join();
+}
 
 const Configuration *PeerConnection::config() const { return &mConfig; }
 
@@ -49,24 +52,37 @@ std::optional<Description> PeerConnection::localDescription() const { return mLo
 std::optional<Description> PeerConnection::remoteDescription() const { return mRemoteDescription; }
 
 void PeerConnection::setRemoteDescription(Description description) {
-	if (!mIceTransport) {
+	auto remoteCandidates = description.extractCandidates();
+	mRemoteDescription.emplace(std::move(description));
+
+	if (!mIceTransport)
 		initIceTransport(Description::Role::ActPass);
-		mIceTransport->setRemoteDescription(description);
+
+	mIceTransport->setRemoteDescription(*mRemoteDescription);
+	if (mRemoteDescription->type() == Description::Type::Offer) {
 		processLocalDescription(mIceTransport->getLocalDescription(Description::Type::Answer));
 		mIceTransport->gatherLocalCandidates();
-	} else {
-		mIceTransport->setRemoteDescription(description);
 	}
 
-	mRemoteDescription.emplace(std::move(description));
+	for (const auto &candidate : remoteCandidates)
+		addRemoteCandidate(candidate);
 }
 
 void PeerConnection::addRemoteCandidate(Candidate candidate) {
 	if (!mRemoteDescription || !mIceTransport)
 		throw std::logic_error("Remote candidate set without remote description");
 
-	if (mIceTransport->addRemoteCandidate(candidate))
-		mRemoteDescription->addCandidate(std::move(candidate));
+	mRemoteDescription->addCandidate(candidate);
+
+	if (candidate.resolve(Candidate::ResolveMode::Simple)) {
+		mIceTransport->addRemoteCandidate(candidate);
+	} else {
+		// OK, we might need a lookup, do it asynchronously
+		mResolveThreads.emplace_back(std::thread([this, candidate]() mutable {
+			if (candidate.resolve(Candidate::ResolveMode::Lookup))
+				mIceTransport->addRemoteCandidate(candidate);
+		}));
+	}
 }
 
 shared_ptr<DataChannel> PeerConnection::createDataChannel(const string &label,
@@ -264,9 +280,9 @@ void PeerConnection::closeDataChannels() {
 void PeerConnection::processLocalDescription(Description description) {
 	auto remoteSctpPort = mRemoteDescription ? mRemoteDescription->sctpPort() : nullopt;
 
-	description.setFingerprint(mCertificate->fingerprint());
-	description.setSctpPort(remoteSctpPort.value_or(DEFAULT_SCTP_PORT));
 	mLocalDescription.emplace(std::move(description));
+	mLocalDescription->setFingerprint(mCertificate->fingerprint());
+	mLocalDescription->setSctpPort(remoteSctpPort.value_or(DEFAULT_SCTP_PORT));
 
 	if (mLocalDescriptionCallback)
 		mLocalDescriptionCallback(*mLocalDescription);