Browse Source

Removed useless role from ICE transport constructor

Paul-Louis Ageneau 4 years ago
parent
commit
447624322c
4 changed files with 26 additions and 25 deletions
  1. 1 1
      include/rtc/peerconnection.hpp
  2. 19 10
      src/icetransport.cpp
  3. 2 2
      src/icetransport.hpp
  4. 4 12
      src/peerconnection.cpp

+ 1 - 1
include/rtc/peerconnection.hpp

@@ -129,7 +129,7 @@ public:
 	void onTrack(std::function<void(std::shared_ptr<Track> track)> callback);
 	void onTrack(std::function<void(std::shared_ptr<Track> track)> callback);
 
 
 private:
 private:
-	std::shared_ptr<IceTransport> initIceTransport(Description::Role role);
+	std::shared_ptr<IceTransport> initIceTransport();
 	std::shared_ptr<DtlsTransport> initDtlsTransport();
 	std::shared_ptr<DtlsTransport> initDtlsTransport();
 	std::shared_ptr<SctpTransport> initSctpTransport();
 	std::shared_ptr<SctpTransport> initSctpTransport();
 	void closeTransports();
 	void closeTransports();

+ 19 - 10
src/icetransport.cpp

@@ -46,11 +46,12 @@ using std::chrono::system_clock;
 
 
 namespace rtc {
 namespace rtc {
 
 
-IceTransport::IceTransport(const Configuration &config, Description::Role role,
-                           candidate_callback candidateCallback, state_callback stateChangeCallback,
+IceTransport::IceTransport(const Configuration &config, candidate_callback candidateCallback,
+                           state_callback stateChangeCallback,
                            gathering_state_callback gatheringStateChangeCallback)
                            gathering_state_callback gatheringStateChangeCallback)
-    : Transport(nullptr, std::move(stateChangeCallback)), mRole(role), mMid("0"),
-      mGatheringState(GatheringState::New), mCandidateCallback(std::move(candidateCallback)),
+    : Transport(nullptr, std::move(stateChangeCallback)), mRole(Description::Role::ActPass),
+      mMid("0"), mGatheringState(GatheringState::New),
+      mCandidateCallback(std::move(candidateCallback)),
       mGatheringStateChangeCallback(std::move(gatheringStateChangeCallback)),
       mGatheringStateChangeCallback(std::move(gatheringStateChangeCallback)),
       mAgent(nullptr, nullptr) {
       mAgent(nullptr, nullptr) {
 
 
@@ -139,6 +140,9 @@ Description IceTransport::getLocalDescription(Description::Type type) const {
 	if (juice_get_local_description(mAgent.get(), sdp, JUICE_MAX_SDP_STRING_LEN) < 0)
 	if (juice_get_local_description(mAgent.get(), sdp, JUICE_MAX_SDP_STRING_LEN) < 0)
 		throw std::runtime_error("Failed to generate local SDP");
 		throw std::runtime_error("Failed to generate local SDP");
 
 
+	// RFC 5763: The endpoint that is the offerer MUST use the setup attribute value of
+	// setup:actpass.
+	// See https://tools.ietf.org/html/rfc5763#section-5
 	return Description(string(sdp), type,
 	return Description(string(sdp), type,
 	                   type == Description::Type::Offer ? Description::Role::ActPass : mRole);
 	                   type == Description::Type::Offer ? Description::Role::ActPass : mRole);
 }
 }
@@ -147,7 +151,7 @@ void IceTransport::setRemoteDescription(const Description &description) {
 	if (mRole == Description::Role::ActPass)
 	if (mRole == Description::Role::ActPass)
 		mRole = description.role() == Description::Role::Active ? Description::Role::Passive
 		mRole = description.role() == Description::Role::Active ? Description::Role::Passive
 		                                                        : Description::Role::Active;
 		                                                        : Description::Role::Active;
-	if(mRole == description.role())
+	if (mRole == description.role())
 		throw std::logic_error("Incompatible roles with remote description");
 		throw std::logic_error("Incompatible roles with remote description");
 
 
 	mMid = description.bundleMid();
 	mMid = description.bundleMid();
@@ -319,11 +323,12 @@ void IceTransport::LogCallback(juice_log_level_t level, const char *message) {
 
 
 namespace rtc {
 namespace rtc {
 
 
-IceTransport::IceTransport(const Configuration &config, Description::Role role,
-                           candidate_callback candidateCallback, state_callback stateChangeCallback,
+IceTransport::IceTransport(const Configuration &config, candidate_callback candidateCallback,
+                           state_callback stateChangeCallback,
                            gathering_state_callback gatheringStateChangeCallback)
                            gathering_state_callback gatheringStateChangeCallback)
-    : Transport(nullptr, std::move(stateChangeCallback)), mRole(role), mMid("0"),
-      mGatheringState(GatheringState::New), mCandidateCallback(std::move(candidateCallback)),
+    : Transport(nullptr, std::move(stateChangeCallback)), mRole(Description::Role::ActPass),
+      mMid("0"), mGatheringState(GatheringState::New),
+      mCandidateCallback(std::move(candidateCallback)),
       mGatheringStateChangeCallback(std::move(gatheringStateChangeCallback)),
       mGatheringStateChangeCallback(std::move(gatheringStateChangeCallback)),
       mNiceAgent(nullptr, nullptr), mMainLoop(nullptr, nullptr) {
       mNiceAgent(nullptr, nullptr), mMainLoop(nullptr, nullptr) {
 
 
@@ -529,6 +534,10 @@ Description IceTransport::getLocalDescription(Description::Type type) const {
 
 
 	std::unique_ptr<gchar[], void (*)(void *)> sdp(nice_agent_generate_local_sdp(mNiceAgent.get()),
 	std::unique_ptr<gchar[], void (*)(void *)> sdp(nice_agent_generate_local_sdp(mNiceAgent.get()),
 	                                               g_free);
 	                                               g_free);
+
+	// RFC 5763: The endpoint that is the offerer MUST use the setup attribute value of
+	// setup:actpass.
+	// See https://tools.ietf.org/html/rfc5763#section-5
 	return Description(string(sdp.get()), type,
 	return Description(string(sdp.get()), type,
 	                   type == Description::Type::Offer ? Description::Role::ActPass : mRole);
 	                   type == Description::Type::Offer ? Description::Role::ActPass : mRole);
 }
 }
@@ -537,7 +546,7 @@ void IceTransport::setRemoteDescription(const Description &description) {
 	if (mRole == Description::Role::ActPass)
 	if (mRole == Description::Role::ActPass)
 		mRole = description.role() == Description::Role::Active ? Description::Role::Passive
 		mRole = description.role() == Description::Role::Active ? Description::Role::Passive
 		                                                        : Description::Role::Active;
 		                                                        : Description::Role::Active;
-	if(mRole == description.role())
+	if (mRole == description.role())
 		throw std::logic_error("Incompatible roles with remote description");
 		throw std::logic_error("Incompatible roles with remote description");
 
 
 	mMid = description.bundleMid();
 	mMid = description.bundleMid();

+ 2 - 2
src/icetransport.hpp

@@ -45,8 +45,8 @@ public:
 	using candidate_callback = std::function<void(const Candidate &candidate)>;
 	using candidate_callback = std::function<void(const Candidate &candidate)>;
 	using gathering_state_callback = std::function<void(GatheringState state)>;
 	using gathering_state_callback = std::function<void(GatheringState state)>;
 
 
-	IceTransport(const Configuration &config, Description::Role role,
-	             candidate_callback candidateCallback, state_callback stateChangeCallback,
+	IceTransport(const Configuration &config, candidate_callback candidateCallback,
+	             state_callback stateChangeCallback,
 	             gathering_state_callback gatheringStateChangeCallback);
 	             gathering_state_callback gatheringStateChangeCallback);
 	~IceTransport();
 	~IceTransport();
 
 

+ 4 - 12
src/peerconnection.cpp

@@ -187,13 +187,7 @@ void PeerConnection::setLocalDescription(Description::Type type) {
 	}
 	}
 	}
 	}
 
 
-	auto iceTransport = std::atomic_load(&mIceTransport);
-	if (!iceTransport) {
-		// RFC 5763: The endpoint that is the offerer MUST use the setup attribute value of
-		// setup:actpass.
-		// See https://tools.ietf.org/html/rfc5763#section-5
-		iceTransport = initIceTransport(Description::Role::ActPass);
-	}
+	auto iceTransport = initIceTransport();
 
 
 	Description localDescription = iceTransport->getLocalDescription(type);
 	Description localDescription = iceTransport->getLocalDescription(type);
 	processLocalDescription(std::move(localDescription));
 	processLocalDescription(std::move(localDescription));
@@ -273,9 +267,7 @@ void PeerConnection::setRemoteDescription(Description description) {
 	auto remoteCandidates = description.extractCandidates();
 	auto remoteCandidates = description.extractCandidates();
 	auto type = description.type();
 	auto type = description.type();
 
 
-	auto iceTransport = std::atomic_load(&mIceTransport);
-	if (!iceTransport)
-		iceTransport = initIceTransport(Description::Role::ActPass);
+	auto iceTransport = initIceTransport();
 
 
 	iceTransport->setRemoteDescription(description);
 	iceTransport->setRemoteDescription(description);
 	processRemoteDescription(std::move(description));
 	processRemoteDescription(std::move(description));
@@ -405,14 +397,14 @@ void PeerConnection::onTrack(std::function<void(std::shared_ptr<Track>)> callbac
 	mTrackCallback = callback;
 	mTrackCallback = callback;
 }
 }
 
 
-shared_ptr<IceTransport> PeerConnection::initIceTransport(Description::Role role) {
+shared_ptr<IceTransport> PeerConnection::initIceTransport() {
 	PLOG_VERBOSE << "Starting ICE transport";
 	PLOG_VERBOSE << "Starting ICE transport";
 	try {
 	try {
 		if (auto transport = std::atomic_load(&mIceTransport))
 		if (auto transport = std::atomic_load(&mIceTransport))
 			return transport;
 			return transport;
 
 
 		auto transport = std::make_shared<IceTransport>(
 		auto transport = std::make_shared<IceTransport>(
-		    mConfig, role, weak_bind(&PeerConnection::processLocalCandidate, this, _1),
+		    mConfig, weak_bind(&PeerConnection::processLocalCandidate, this, _1),
 		    [this, weak_this = weak_from_this()](IceTransport::State state) {
 		    [this, weak_this = weak_from_this()](IceTransport::State state) {
 			    auto shared_this = weak_this.lock();
 			    auto shared_this = weak_this.lock();
 			    if (!shared_this)
 			    if (!shared_this)