From f4c39ff06f2de7195c9a6be36fdd54b8a9cbb7f2 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Sat, 6 Apr 2019 14:01:37 +0300 Subject: [PATCH 01/30] Rewrite ICE connectivity checking Last implementation cut corners and wasn't robust enough. It actually didn't even follow the specification because a race condition in receiving STUN Requests/Responses inverted the roles of agents and caused the nomination to fail. --- src/stun.cpp | 187 ++++++++++++++++++++++++++++++++++++++++++--------- src/stun.h | 25 +++++++ 2 files changed, 181 insertions(+), 31 deletions(-) diff --git a/src/stun.cpp b/src/stun.cpp index c350f39b..ae1440b7 100644 --- a/src/stun.cpp +++ b/src/stun.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include @@ -66,6 +67,21 @@ bool Stun::waitForStunResponse(unsigned long timeout) return timer.isActive(); } +bool Stun::waitForStunRequest(unsigned long timeout) +{ + QTimer timer; + timer.setSingleShot(true); + QEventLoop loop; + + connect(this, &Stun::requestRecv, &loop, &QEventLoop::quit); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + + timer.start(timeout); + loop.exec(); + + return timer.isActive(); +} + bool Stun::waitForNominationRequest(unsigned long timeout) { QTimer timer; @@ -81,64 +97,170 @@ bool Stun::waitForNominationRequest(unsigned long timeout) return timer.isActive(); } -bool Stun::sendBindingRequest(ICEPair *pair, bool controller) +// if we're the controlling agent, sending binding request is rather straight-forward: +// send request, wait for response and return +bool Stun::controllerSendBindingRequest(ICEPair *pair) { - if (controller) - qDebug() << "[controller] BINDING " << pair->local->address<< " TO PORT " << pair->local->port; - else - qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; + STUNMessage msg = stunmsg_.createRequest(); + msg.addAttribute(STUN_ATTR_ICE_CONTROLLING); + msg.addAttribute(STUN_ATTR_PRIORITY, pair->priority); - if (!udp_.bindRaw(QHostAddress(pair->local->address), pair->local->port)) + // we expect a response to this message from remote, by caching the TransactionID + // and associated address/port pair, we can succesfully validate the response's TransactionID + stunmsg_.expectReplyFrom(msg, pair->remote->address, pair->remote->port); + + QByteArray message = stunmsg_.hostToNetwork(msg); + bool msgReceived = false; + + for (int i = 0; i < 20; ++i) { - qDebug() << "Binding failed! Cannot send STUN Binding Requests to " - << pair->remote->address << ":" << pair->remote->port; + udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + + if (waitForStunResponse(20 * (i + 1))) + { + qDebug() << "GOT RESPONSE!"; + msgReceived = true; + break; + } + } + + if (msgReceived == false) + { + qDebug() << "WARNING: Failed to receive STUN Binding Response from remote!"; + qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; + udp_.unbind(); return false; } - connect(&udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + // the first part of connectivity check is done, now we must wait for + // remote's binding request and responed to them + msgReceived = false; + + for (int i = 0; i < 20; ++i) + { + if (waitForStunRequest(20 * (i + 1))) + { + qDebug() << "[CONTROLLER] GOT STUN REQUEST!"; + + msg = stunmsg_.createResponse(); + msg.addAttribute(STUN_ATTR_ICE_CONTROLLING); + + message = stunmsg_.hostToNetwork(msg); + msgReceived = true; + + for (int i = 0; i < 5; ++i) + { + udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + QThread::msleep(20); + } - // TODO this code looks ugly -> refactor + break; + } + } + udp_.unbind(); + return msgReceived; +} + +bool Stun::controlleeSendBindingRequest(ICEPair *pair) +{ + Q_ASSERT(pair != nullptr); + + bool msgReceived = false; + QByteArray message = QByteArray("hole punch"); + STUNMessage msg; + + for (int i = 0; i < 20; ++i) + { + udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + + if (waitForStunRequest(20 * (i + 1))) + { + msg = stunmsg_.createResponse(); + msg.addAttribute(STUN_ATTR_ICE_CONTROLLED); + + message = stunmsg_.hostToNetwork(msg); + msgReceived = true; + + for (int i = 0; i < 3; ++i) + { + udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + QThread::msleep(20); + } + + break; + } + } + + if (msgReceived == false) + { + qDebug() << "WARNING: Failed to receive STUN Binding Request from remote!"; + qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; + udp_.unbind(); + return false; + } + + // the first part of connective check is done (remote sending us binding request) + // now we must do the same but this we're sending the request and they're responding + msgReceived = false; + + // we've succesfully responded to remote's binding request, now it's our turn to + // send request and remote must respond to them STUNMessage request = stunmsg_.createRequest(); - request.addAttribute(controller ? STUN_ATTR_ICE_CONTROLLING : STUN_ATTR_ICE_CONTROLLED); + request.addAttribute(STUN_ATTR_ICE_CONTROLLED); request.addAttribute(STUN_ATTR_PRIORITY, pair->priority); - // we expect a response to this message from remote, by caching the TransactionID - // and associated address port pair, we can succesfully validate the response TransactionID - stunmsg_.expectReplyFrom(request, pair->remote->address, pair->remote->port); - - QByteArray message = stunmsg_.hostToNetwork(request); + message = stunmsg_.hostToNetwork(request); - bool ok = false; + // we're expecting a response from remote to this request + stunmsg_.cacheRequest(request); - for (int i = 0; i < 50; ++i) + for (int i = 0; i < 20; ++i) { udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); if (waitForStunResponse(20 * (i + 1))) { - qDebug() << "got stun response!"; - ok = true; + msgReceived = true; break; } } - if (waitForStunResponse(100)) + udp_.unbind(); + return msgReceived; +} + +bool Stun::sendBindingRequest(ICEPair *pair, bool controller) +{ + if (controller) + qDebug() << "[controller] BINDING " << pair->local->address<< " TO PORT " << pair->local->port; + else + qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; + + if (!udp_.bindRaw(QHostAddress(pair->local->address), pair->local->port)) + { + qDebug() << "Binding failed! Cannot send STUN Binding Requests to " + << pair->remote->address << ":" << pair->remote->port; + return false; + } + + connect(&udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + + if (controller) { - ok = true; + return controllerSendBindingRequest(pair); } - udp_.unbind(); - return ok; + return controlleeSendBindingRequest(pair); } bool Stun::sendBindingResponse(STUNMessage& request, QString addressRemote, int portRemote) { STUNMessage response = stunmsg_.createResponse(request); - response.addAttribute(STUN_ATTR_PRIORITY, 0x1337); + response.addAttribute(STUN_ATTR_PRIORITY, 1337); - request.addAttribute( + response.addAttribute( request.hasAttribute(STUN_ATTR_ICE_CONTROLLED) ? STUN_ATTR_ICE_CONTROLLING : STUN_ATTR_ICE_CONTROLLED @@ -149,6 +271,11 @@ bool Stun::sendBindingResponse(STUNMessage& request, QString addressRemote, int for (int i = 0; i < 25; ++i) { udp_.sendData(message, QHostAddress(addressRemote), portRemote, false); + + if (!waitForStunRequest(20 * (i + 1))) + { + break; + } } } @@ -167,13 +294,11 @@ void Stun::recvStunMessage(QNetworkDatagram message) if (stunMsg.hasAttribute(STUN_ATTR_USE_CANDIATE)) { - sendBindingResponse(stunMsg, message.senderAddress().toString(), message.senderPort()); emit nominationRecv(); + return; } - else - { - sendBindingResponse(stunMsg, message.senderAddress().toString(), message.senderPort()); - } + + emit requestRecv(); } } else if (stunMsg.getType() == STUN_RESPONSE) diff --git a/src/stun.h b/src/stun.h index 7f477aad..f1f18591 100644 --- a/src/stun.h +++ b/src/stun.h @@ -46,6 +46,7 @@ class Stun : public QObject void stunError(); void parsingDone(); void nominationRecv(); + void requestRecv(); private slots: void handleHostaddress(QHostInfo info); @@ -54,8 +55,32 @@ private slots: private: bool waitForStunResponse(unsigned long timeout); + bool waitForStunRequest(unsigned long timeout); bool waitForNominationRequest(unsigned long timeout); + // If we're the controlling agent we start by sending STUN Binding Requests to remote + // When we receive a response to one of our STUN Binding Requests, we start listening to + // remote's STUN Binding Requests. When we receive one, we acknowledge it by sending STUN + // Binding Response to remote. + // Now the we've concluded that the communication works and we can stop testing. + // + // Return true if the testing succeeded, false otherwise + bool controllerSendBindingRequest(ICEPair *pair); + + // if we're the controlled agent, we must first start sending "dummy" binding requests to + // make a hole in the firewall and after we've gotten a request from remote (controlling agent) + // we can switch to sending responses + + // if we're the controlled agent, we start sending STUN Binding Requests to create a hole in the + // firewall. When we get a response from remote (the controlling agent) we acknowledge it by sending + // STUN Binding Request. + // + // After we've received a STUN Request from remote, we start listening to responses for our STUN Binding Requests. + // When we've received a response we mark this pair as valid as we've established that communication works to both directions + // + // Return true if the testing succeeded, false otherwise + bool controlleeSendBindingRequest(ICEPair *pair); + // TODO [Encryption] Use TLS to send packet UDPServer udp_; From 4c851274a72e2325411d122c555b9ff707108f97 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Sat, 6 Apr 2019 14:04:12 +0300 Subject: [PATCH 02/30] Terminate threads before killing the FlowAgent Failing to do this caused segmentation faults when candidate nomination didn't succeed --- src/iceflowcontrol.cpp | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/iceflowcontrol.cpp b/src/iceflowcontrol.cpp index 98afee60..294e0cfa 100644 --- a/src/iceflowcontrol.cpp +++ b/src/iceflowcontrol.cpp @@ -60,8 +60,8 @@ void FlowController::run() Stun stun; // TODO how long should we sleep??? - for (volatile int i = 0; i < 10000000; ++i) - ; + /* for (volatile int i = 0; i < 10000000; ++i) */ + /* ; */ if (candidates_ == nullptr || candidates_->size() == 0) { @@ -83,13 +83,7 @@ void FlowController::run() workerThreads.back()->start(); } - // we've spawned threads for each candidate, wait for responses at most 10 seconds - if (!waitForResponses(10000)) - { - qDebug() << "Remote didn't respond to our request in time!"; - emit ready(nullptr, nullptr, sessionID_); - return; - } + bool nominationSucceeded = waitForResponses(10000); // we got a response, suspend all threads and start nomination for (size_t i = 0; i < workerThreads.size(); ++i) @@ -98,6 +92,14 @@ void FlowController::run() workerThreads[i]->wait(); } + // we've spawned threads for each candidate, wait for responses at most 10 seconds + if (!nominationSucceeded) + { + qDebug() << "Remote didn't respond to our request in time!"; + emit ready(nullptr, nullptr, sessionID_); + return; + } + if (!stun.sendNominationRequest(nominated_rtp_)) { qDebug() << "Failed to nominate RTP candidate!"; @@ -143,20 +145,24 @@ void FlowControllee::run() workerThreads.back()->start(); } - // wait for nomination from remote, wait at most 20 seconds - if (!waitForResponses(20000)) - { - qDebug() << "Nomination from remote was not received in time!"; - emit ready(nullptr, nullptr, sessionID_); - return; - } - // we got a nomination from remote, kill all threads and start the media transmission + bool nominationSucceeded = waitForResponses(20000); + + // kill all threads, regardless of whether nomination succeeded or not for (size_t i = 0; i < workerThreads.size(); ++i) { workerThreads[i]->quit(); workerThreads[i]->wait(); } + // wait for nomination from remote, wait at most 20 seconds + if (!nominationSucceeded) + { + qDebug() << "Nomination from remote was not received in time!"; + emit ready(nullptr, nullptr, sessionID_); + return; + } + + // media transmission can be started emit ready(nominated_rtp_, nominated_rtcp_, sessionID_); } From 742c7b5a285072da06d36b4e7b9ef8956a6ea01e Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Sat, 6 Apr 2019 14:05:04 +0300 Subject: [PATCH 03/30] Add debug messages to ConnectionTester --- src/connectiontester.cpp | 17 ++++++++++++++++- src/connectiontester.h | 2 ++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/connectiontester.cpp b/src/connectiontester.cpp index 5391704c..2a436dfb 100644 --- a/src/connectiontester.cpp +++ b/src/connectiontester.cpp @@ -23,6 +23,14 @@ void ConnectionTester::isController(bool controller) controller_ = controller; } +void ConnectionTester::printMessage(QString message) +{ + if (controller_) + qDebug() << "[controller]" << message; + else + qDebug() << "[controllee]" << message; +} + void ConnectionTester::run() { if (rtp_pair_ == nullptr || rtcp_pair_ == nullptr) @@ -36,22 +44,29 @@ void ConnectionTester::run() rtcp_pair_->state = PAIR_WAITING; rtp_pair_->state = PAIR_IN_PROGRESS; + printMessage("STARTING"); + if (stun.sendBindingRequest(rtp_pair_, controller_)) { rtp_pair_->state = PAIR_SUCCEEDED; rtcp_pair_->state = PAIR_IN_PROGRESS; + printMessage("RTP NOMINATED!!!"); + if (stun.sendBindingRequest(rtcp_pair_, controller_)) { + printMessage("RTCP NOMINATED!"); rtcp_pair_->state = PAIR_SUCCEEDED; } else { + printMessage("RTCP FAILED!"); rtcp_pair_->state = PAIR_FAILED; } } else { + printMessage("RTP FAILED!"); rtp_pair_->state = PAIR_FAILED; } @@ -73,7 +88,7 @@ void ConnectionTester::run() if (!stun.sendNominationResponse(rtp_pair_)) { qDebug() << "failed to receive nomination for RTP candidate:\n" - << "\tlocal:" << rtp_pair_->local->address << ":" << rtcp_pair_->local->port << "\n" + << "\t\local:" << rtp_pair_->local->address << ":" << rtcp_pair_->local->port << "\n" << "\tremote:" << rtp_pair_->remote->address << ":" << rtcp_pair_->remote->port; return; } diff --git a/src/connectiontester.h b/src/connectiontester.h index 361c1757..73a7b499 100644 --- a/src/connectiontester.h +++ b/src/connectiontester.h @@ -20,6 +20,8 @@ class ConnectionTester : public QThread // requests after it has concluded the candidate verification void isController(bool controller); + void printMessage(QString message); + signals: // testingDone() is emitted when the connection testing has ended // From 57e68aa7c6f555f830759ef98fcd503b6216b7d9 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 8 Apr 2019 08:52:17 +0300 Subject: [PATCH 04/30] Use the cached requests's TransactionID for "orphan" responses --- src/stunmsgfact.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/stunmsgfact.cpp b/src/stunmsgfact.cpp index 1a4ea94e..f7e3baf4 100644 --- a/src/stunmsgfact.cpp +++ b/src/stunmsgfact.cpp @@ -22,6 +22,7 @@ STUNMessage StunMessageFactory::createResponse() { STUNMessage response(STUN_RESPONSE); + response.setTransactionID(latestRequest_.getTransactionID()); return response; } From 9d1e08fd5237fbcb2f92905e9e12d8dd18832c68 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 8 Apr 2019 09:34:28 +0300 Subject: [PATCH 05/30] Add function for creating candidates --- src/ice.cpp | 109 +++++++++++++++++++++++----------------------------- src/ice.h | 12 +++--- 2 files changed, 54 insertions(+), 67 deletions(-) diff --git a/src/ice.cpp b/src/ice.cpp index 56b89069..7b08d6d8 100644 --- a/src/ice.cpp +++ b/src/ice.cpp @@ -11,10 +11,8 @@ const uint16_t MAX_ICE_PORT = 22500; const uint16_t MAX_PORTS = 100; ICE::ICE(): - portPair(22000), stun_(), - stun_entry_rtp_(nullptr), - stun_entry_rtcp_(nullptr), + stunAddress_(QHostAddress("")), parameters_() { parameters_.setPortRange(MIN_ICE_PORT, MAX_ICE_PORT, MAX_PORTS); @@ -30,8 +28,6 @@ ICE::ICE(): ICE::~ICE() { - delete stun_entry_rtp_; - delete stun_entry_rtcp_; } // TODO lue speksi uudelleen tämä osalta @@ -68,6 +64,7 @@ QList ICE::generateICECandidates() qsrand((uint)time.msec()); QList candidates; + std::pair candidate; foreach (const QHostAddress& address, QNetworkInterface::allAddresses()) { @@ -77,45 +74,62 @@ QList ICE::generateICECandidates() address.toString().startsWith("172.") || address == QHostAddress(QHostAddress::LocalHost))) { - ICEInfo *entry_rtp = new ICEInfo; - ICEInfo *entry_rtcp = new ICEInfo; + candidate = makeCandidate(address, "host"); - entry_rtp->address = address.toString(); - entry_rtcp->address = address.toString(); + candidates.push_back(candidate.first); + candidates.push_back(candidate.second); + } + } - entry_rtp->port = parameters_.nextAvailablePortPair(); - entry_rtcp->port = entry_rtp->port + 1; + if (stunAddress_ != QHostAddress("")) + { + std::pair cands = makeCandidate(stunAddress_, "srflx"); - // for each RTP/RTCP pair foundation is the same - const QString foundation = generateFoundation(); + candidates.push_back(cands.first); + candidates.push_back(cands.second); + } - entry_rtp->foundation = foundation; - entry_rtcp->foundation = foundation; + return candidates; +} - entry_rtp->transport = "UDP"; - entry_rtcp->transport = "UDP"; +std::pair ICE::makeCandidate(QHostAddress address, QString type) +{ + ICEInfo *entry_rtp = new ICEInfo; + ICEInfo *entry_rtcp = new ICEInfo; - entry_rtp->component = RTP; - entry_rtcp->component = RTCP; + entry_rtp->address = address.toString(); + entry_rtcp->address = address.toString(); - entry_rtp->priority = calculatePriority(126, 1, RTP); - entry_rtcp->priority = calculatePriority(126, 1, RTCP); + entry_rtp->port = parameters_.nextAvailablePortPair(); + entry_rtcp->port = entry_rtp->port + 1; - entry_rtp->type = "host"; - entry_rtcp->type = "host"; + // for each RTP/RTCP pair foundation is the same + const QString foundation = generateFoundation(); - candidates.push_back(entry_rtp); - candidates.push_back(entry_rtcp); - } - } + entry_rtp->foundation = foundation; + entry_rtcp->foundation = foundation; + + entry_rtp->transport = "UDP"; + entry_rtcp->transport = "UDP"; - if (stun_entry_rtp_ != nullptr && stun_entry_rtcp_ != nullptr) + entry_rtp->component = RTP; + entry_rtcp->component = RTCP; + + if (type == "host") + { + entry_rtp->priority = calculatePriority(126, 1, RTP); + entry_rtcp->priority = calculatePriority(126, 1, RTCP); + } + else { - candidates.push_back(stun_entry_rtp_); - candidates.push_back(stun_entry_rtcp_); + entry_rtp->priority = calculatePriority(0, 1, RTP); + entry_rtcp->priority = calculatePriority(0, 1, RTCP); } - return candidates; + entry_rtp->type = type; + entry_rtcp->type = type; + + return std::make_pair(entry_rtp, entry_rtcp); } void ICE::createSTUNCandidate(QHostAddress address) @@ -123,37 +137,9 @@ void ICE::createSTUNCandidate(QHostAddress address) if (address == QHostAddress("")) { qDebug() << "WARNING: Failed to resolve public IP! Server-reflexive candidates won't be created!"; - return; } - qDebug() << "Creating STUN candidate..."; - - stun_entry_rtp_ = new ICEInfo; - stun_entry_rtcp_ = new ICEInfo; - - stun_entry_rtp_->address = address.toString(); - stun_entry_rtcp_->address = address.toString(); - - stun_entry_rtp_->port = portPair++; - stun_entry_rtcp_->port = portPair++; - - // for each RTP/RTCP pair foundation is the same - const QString foundation = generateFoundation(); - - stun_entry_rtp_->foundation = foundation; - stun_entry_rtcp_->foundation = foundation; - - stun_entry_rtp_->transport = "UDP"; - stun_entry_rtcp_->transport = "UDP"; - - stun_entry_rtp_->component = RTP; - stun_entry_rtcp_->component = RTCP; - - stun_entry_rtp_->priority = calculatePriority(126, 0, RTP); - stun_entry_rtcp_->priority = calculatePriority(126, 0, RTCP); - - stun_entry_rtp_->type = "srflx"; - stun_entry_rtcp_->type = "srflx"; + stunAddress_ = address; } void ICE::printCandidate(ICEInfo *candidate) @@ -238,6 +224,7 @@ void ICE::respondToNominations(QList& local, QList& remote FlowControllee *caller = nominationInfo_[sessionID].controllee; QObject::connect(caller, &FlowControllee::ready, this, &ICE::handleCallerEndOfNomination, Qt::DirectConnection); + /* caller->setHevcCandidates( */ caller->setCandidates(nominationInfo_[sessionID].pairs); caller->setSessionID(sessionID); caller->start(); @@ -304,7 +291,7 @@ void ICE::handleEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *ca // // because ports are allocated in pairs (RTP is port X and RTCP is port X + 1), // we need to call makePortPairAvailable() only for RTP candidate - if (pair->local != stun_entry_rtp_ && pair->local != stun_entry_rtcp_ && pair->local->component == RTP) + if (pair->local->component == RTP) { parameters_.makePortPairAvailable(pair->local->port); } diff --git a/src/ice.h b/src/ice.h index 3da25f2e..bf8e2ad2 100644 --- a/src/ice.h +++ b/src/ice.h @@ -72,6 +72,10 @@ class ICE : public QObject void createSTUNCandidate(QHostAddress address); private: + // create media candidate (RTP and RTCP connection) + // "type" marks whether this candidate is host or server reflexive candidate (affects priority) + std::pair makeCandidate(QHostAddress address, QString type); + void handleEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID); int calculatePriority(int type, int local, int component); @@ -79,16 +83,12 @@ class ICE : public QObject void printCandidate(ICEInfo *candidate); QList *makeCandiatePairs(QList& local, QList& remote); - uint16_t portPair; bool nominatingConnection_; bool iceDisabled_; Stun stun_; - - ICEInfo *stun_entry_rtp_; - ICEInfo *stun_entry_rtcp_; + QHostAddress stunAddress_; + SDPParameterManager parameters_; QMap nominationInfo_; - - SDPParameterManager parameters_; }; From ce2ce48f1db9cb0627b4b4b290dd570149dd466f Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 9 Apr 2019 09:36:55 +0300 Subject: [PATCH 06/30] Add help function setMediaPair MediaInfo is updated after ICE has finished, this helper function makes the calling code much cleaner --- src/globalsdpstate.cpp | 63 +++++++++++++++++++++--------------------- src/globalsdpstate.h | 3 ++ 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/globalsdpstate.cpp b/src/globalsdpstate.cpp index f3871810..3a435c0d 100644 --- a/src/globalsdpstate.cpp +++ b/src/globalsdpstate.cpp @@ -167,25 +167,19 @@ GlobalSDPState::localFinalSDP(SDPMessageInfo &remoteSDP, QHostAddress localAddre return nullptr; } - // getNominated() returns two (TODO: four!) valid candidate pairs if ICE succeeded - // (ie. ICE was not disabled and remote responded to our requests) - auto nominated = ice_->getNominated(sessionID); + ICEMediaInfo nominated = ice_->getNominated(sessionID); - if (nominated.first && nominated.second) + // first is RTP, second is RTCP + if (nominated.opus.first != nullptr && nominated.opus.second != nullptr) { - // RTP - sdp->media[0].receivePort = nominated.first->local->port; - sdp->media[0].connection_address = nominated.first->local->address; - - remoteSDP.media[0].receivePort = nominated.first->remote->port; - remoteSDP.media[0].connection_address = nominated.first->remote->address; - - // RTCP - sdp->media[1].receivePort = nominated.second->local->port; - sdp->media[1].connection_address = nominated.second->local->address; + setMediaPair(sdp->media[0], nominated.opus.first->local); + setMediaPair(remoteSDP.media[0], nominated.opus.first->remote); + } - remoteSDP.media[1].receivePort = nominated.second->remote->port; - remoteSDP.media[1].connection_address = nominated.second->remote->address; + if (nominated.hevc.first != nullptr && nominated.hevc.second != nullptr) + { + setMediaPair(sdp->media[1], nominated.hevc.first->local); + setMediaPair(remoteSDP.media[1], nominated.hevc.first->remote); } } @@ -264,26 +258,31 @@ void GlobalSDPState::startICECandidateNegotiation(QList& local, QList ice_->startNomination(local, remote, sessionID); } -void GlobalSDPState::updateFinalSDPs(SDPMessageInfo& localSDP, SDPMessageInfo& remoteSDP, uint32_t sessionID) +void GlobalSDPState::setMediaPair(MediaInfo& media, ICEInfo *mediaInfo) { - auto nominated = ice_->getNominated(sessionID); - - if (nominated.first && nominated.second) + if (mediaInfo == nullptr) { - // local RTP - localSDP.media[0].connection_address = nominated.first->local->address; - localSDP.media[0].receivePort = nominated.first->local->port; + return; + } - // local RTCP - localSDP.media[1].connection_address = nominated.second->local->address; - localSDP.media[1].receivePort = nominated.second->local->port; + media.connection_address = mediaInfo->address; + media.receivePort = mediaInfo->port; +} - // remote RTP - remoteSDP.media[0].connection_address = nominated.first->remote->address; - remoteSDP.media[0].receivePort = nominated.first->remote->port; +void GlobalSDPState::updateFinalSDPs(SDPMessageInfo& localSDP, SDPMessageInfo& remoteSDP, uint32_t sessionID) +{ + ICEMediaInfo nominated = ice_->getNominated(sessionID); - // remote RTCP - remoteSDP.media[1].connection_address = nominated.second->remote->address; - remoteSDP.media[1].receivePort = nominated.second->remote->port; + // first is RTP, second is RTCP + if (nominated.opus.first != nullptr && nominated.opus.second != nullptr) + { + setMediaPair(localSDP.media[0], nominated.opus.first->local); + setMediaPair(remoteSDP.media[0], nominated.opus.first->remote); + } + + if (nominated.hevc.first != nullptr && nominated.hevc.second != nullptr) + { + setMediaPair(localSDP.media[1], nominated.hevc.first->local); + setMediaPair(remoteSDP.media[1], nominated.hevc.first->remote); } } diff --git a/src/globalsdpstate.h b/src/globalsdpstate.h index c614e2b1..20439f6e 100644 --- a/src/globalsdpstate.h +++ b/src/globalsdpstate.h @@ -56,6 +56,9 @@ class GlobalSDPState bool checkSDPOffer(SDPMessageInfo& offer); + // update MediaInfo of SDP after ICE has finished + void setMediaPair(MediaInfo& media, ICEInfo *mediaInfo); + QString localUsername_; std::unique_ptr ice_; From 85d13f1463f053cea62f73d4688a05286b326234 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 12 Apr 2019 10:58:11 +0300 Subject: [PATCH 07/30] Don't initialize port range by default in SDPParameterManager --- src/globalsdpstate.cpp | 6 ++++++ src/sdpparametermanager.cpp | 31 +++++++++++++++++++++++++------ src/sdpparametermanager.h | 7 ++++--- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/globalsdpstate.cpp b/src/globalsdpstate.cpp index 3a435c0d..7fbffa81 100644 --- a/src/globalsdpstate.cpp +++ b/src/globalsdpstate.cpp @@ -1,10 +1,16 @@ #include "globalsdpstate.h" #include +const uint16_t MIN_SIP_PORT = 21500; +const uint16_t MAX_SIP_PORT = 22000; + +const uint16_t MAX_PORTS = 42; + GlobalSDPState::GlobalSDPState(): localUsername_("") { ice_ = std::make_unique(); + parameters_.setPortRange(MIN_SIP_PORT, MAX_SIP_PORT, MAX_PORTS); } void GlobalSDPState::setLocalInfo(QString username) diff --git a/src/sdpparametermanager.cpp b/src/sdpparametermanager.cpp index 3f6044df..d8a29825 100644 --- a/src/sdpparametermanager.cpp +++ b/src/sdpparametermanager.cpp @@ -3,18 +3,12 @@ #include #include -const uint16_t MIN_SIP_PORT = 21500; -const uint16_t MAX_SIP_PORT = 22000; - -const uint16_t MAX_PORTS = 42; SDPParameterManager::SDPParameterManager() :remainingPorts_(0) { - setPortRange(MIN_SIP_PORT, MAX_SIP_PORT, MAX_PORTS); } - void SDPParameterManager::setPortRange(uint16_t minport, uint16_t maxport, uint16_t maxRTPConnections) { if(minport >= maxport) @@ -70,6 +64,31 @@ QString SDPParameterManager::sessionDescription() const return "A Kvazzup initiated video call"; } +uint16_t SDPParameterManager::allocateMediaPorts() +{ + if (remainingPorts_ < 4) + { + qDebug() << "ERROR: Not enough free ports, remaining:" << remainingPorts_; + return 0; + } + + portLock_.lock(); + + uint16_t start = availablePorts_.at(0); + + availablePorts_.pop_front(); + availablePorts_.pop_front(); + remainingPorts_ -= 4; + + portLock_.unlock(); + + return start; +} + +uint16_t SDPParameterManager::deallocateMediaPorts(uint16_t start) +{ +} + uint16_t SDPParameterManager::nextAvailablePortPair() { // TODO: I'm suspecting this may sometimes hang Kvazzup at the start diff --git a/src/sdpparametermanager.h b/src/sdpparametermanager.h index 4344c5eb..9a9cfdc8 100644 --- a/src/sdpparametermanager.h +++ b/src/sdpparametermanager.h @@ -36,10 +36,11 @@ class SDPParameterManager uint16_t nextAvailablePortPair(); void makePortPairAvailable(uint16_t lowerPort); -private: - - + // allocate contiguous port range + uint16_t allocateMediaPorts(); + uint16_t deallocateMediaPorts(uint16_t start); +private: uint16_t remainingPorts_; QMutex portLock_; From f571d3680e9f8d1890ea33dc6948dcb2064c22bc Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 12 Apr 2019 11:11:42 +0300 Subject: [PATCH 08/30] Create separate connections for HEVC and Opus --- src/ice.cpp | 65 ++++++++++++++++++++++++++++++++++---------- src/ice.h | 12 ++++---- src/icetypes.h | 7 +++++ src/mediamanager.cpp | 8 +++--- 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/ice.cpp b/src/ice.cpp index 7b08d6d8..d4628633 100644 --- a/src/ice.cpp +++ b/src/ice.cpp @@ -23,7 +23,7 @@ ICE::ICE(): QSettings settings("kvazzup.ini", QSettings::IniFormat); int ice = settings.value("sip/ice").toInt(); - iceDisabled_ = (ice != 1) ? true : false; + iceDisabled_ = (ice == 1) ? true : false; } ICE::~ICE() @@ -71,8 +71,7 @@ QList ICE::generateICECandidates() if (address.protocol() == QAbstractSocket::IPv4Protocol && (address.toString().startsWith("10.") || address.toString().startsWith("192.") || - address.toString().startsWith("172.") || - address == QHostAddress(QHostAddress::LocalHost))) + address.toString().startsWith("172."))) { candidate = makeCandidate(address, "host"); @@ -100,7 +99,7 @@ std::pair ICE::makeCandidate(QHostAddress address, QString entry_rtp->address = address.toString(); entry_rtcp->address = address.toString(); - entry_rtp->port = parameters_.nextAvailablePortPair(); + entry_rtp->port = parameters_.allocateMediaPorts(); entry_rtcp->port = entry_rtp->port + 1; // for each RTP/RTCP pair foundation is the same @@ -266,7 +265,7 @@ bool ICE::calleeConnectionNominated(uint32_t sessionID) return nominationInfo_[sessionID].connectionNominated; } -void ICE::handleEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID) +void ICE::handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID) { // nothing needs to be cleaned if ICE was disabled if (iceDisabled_ == true) @@ -274,11 +273,43 @@ void ICE::handleEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *ca return; } - nominationInfo_[sessionID].connectionNominated = (candidateRTP != nullptr && candidateRTCP != nullptr); - - if (nominationInfo_[sessionID].connectionNominated) + if (candidateRTP == nullptr || candidateRTCP == nullptr) + { + qDebug() << "ERROR: Nomination failed! Unable to start call."; + nominationInfo_[sessionID].connectionNominated = false; + } + else { - nominationInfo_[sessionID].nominatedPair = std::make_pair((ICEPair *)candidateRTP, (ICEPair *)candidateRTCP); + nominationInfo_[sessionID].connectionNominated = true; + nominationInfo_[sessionID].nominatedHEVC = std::make_pair((ICEPair *)candidateRTP, (ICEPair *)candidateRTCP); + + // Create opus candidate on the fly. When this candidate (candidateRTP && candidateRTCP) was created + // we intentionally allocated 4 ports instead of 2 for use. + // + // This is because we don't actually need to test that both HEVC and Opus work separately. Insted we can just + // test HEVC and if that works we can assume that Opus works too (no reason why it shouldn't) + ICEPair *opusPairRTP = new ICEPair; + opusPairRTP->local = new ICEInfo; + opusPairRTP->remote = new ICEInfo; + + ICEPair *opusPairRTCP = new ICEPair; + opusPairRTCP->local = new ICEInfo; + opusPairRTCP->remote = new ICEInfo; + + memcpy(opusPairRTP->local, candidateRTP->local, sizeof(ICEInfo)); + memcpy(opusPairRTP->remote, candidateRTP->remote, sizeof(ICEInfo)); + + memcpy(opusPairRTCP->local, candidateRTCP->local, sizeof(ICEInfo)); + memcpy(opusPairRTCP->remote, candidateRTCP->remote, sizeof(ICEInfo)); + + opusPairRTP->local->port += 2; // hevc rtp, hevc rtcp and then opus rtp + opusPairRTCP->local->port += 2; // hevc rtp, hevc, rtcp, opus rtp and then opus rtcp + + // same for remote candidate + opusPairRTP->remote->port += 2; // hevc rtp, hevc rtcp and then opus rtp + opusPairRTCP->remote->port += 2; // hev rtp, hevc, rtcp, opus rtp and then opus rtcp + + nominationInfo_[sessionID].nominatedOpus = std::make_pair((ICEPair *)opusPairRTP, (ICEPair *)opusPairRTCP); } foreach (ICEPair *pair, *nominationInfo_[sessionID].pairs) @@ -320,14 +351,18 @@ void ICE::handleCalleeEndOfNomination(struct ICEPair *candidateRTP, struct ICEPa this->handleEndOfNomination(candidateRTP, candidateRTCP, sessionID); } -std::pair ICE::getNominated(uint32_t sessionID) +ICEMediaInfo ICE::getNominated(uint32_t sessionID) { if (nominationInfo_.contains(sessionID) && iceDisabled_ == false) { - return nominationInfo_[sessionID].nominatedPair; - } - else - { - return std::make_pair(nullptr, nullptr); + return { + nominationInfo_[sessionID].nominatedHEVC, + nominationInfo_[sessionID].nominatedOpus + }; } + + return { + std::make_pair(nullptr, nullptr), + std::make_pair(nullptr, nullptr) + }; } diff --git a/src/ice.h b/src/ice.h index bf8e2ad2..d2af6bab 100644 --- a/src/ice.h +++ b/src/ice.h @@ -26,7 +26,8 @@ struct nominationInfo // all but nominatedPair are freed in handleEndOfNomination() QList *pairs; - std::pair nominatedPair; + std::pair nominatedHEVC; + std::pair nominatedOpus; bool connectionNominated; }; @@ -49,7 +50,8 @@ class ICE : public QObject void respondToNominations(QList& local, QList& remote, uint32_t sessionID); // get nominated ICE pair using sessionID - std::pair getNominated(uint32_t sessionID); + /* std::pair getNominated(uint32_t sessionID); */ + ICEMediaInfo getNominated(uint32_t sessionID); // caller must call this function to check if ICE has finished // sessionID must given so ICE can know which ongoing nomination should be checked @@ -63,12 +65,12 @@ class ICE : public QObject // when FlowControllee has finished its job, it emits "ready" signal which is caught by this slot function // handleCallerEndOfNomination() check if the nomination succeeed, saves the nominated pair to hashmap and // releases caller_mtx to signal that negotiation is done - void handleCallerEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID); + void handleCallerEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID); // when FlowController has finished its job, it emits "ready" signal which is caught by this slot function // handleCalleeEndOfNomination() check if the nomination succeeed, saves the nominated pair to hashmap and // releases callee_mtx to signal that negotiation is done - void handleCalleeEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID); + void handleCalleeEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID); void createSTUNCandidate(QHostAddress address); private: @@ -76,7 +78,7 @@ class ICE : public QObject // "type" marks whether this candidate is host or server reflexive candidate (affects priority) std::pair makeCandidate(QHostAddress address, QString type); - void handleEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID); + void handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID); int calculatePriority(int type, int local, int component); QString generateFoundation(); diff --git a/src/icetypes.h b/src/icetypes.h index 40c1ce35..df632c99 100644 --- a/src/icetypes.h +++ b/src/icetypes.h @@ -37,3 +37,10 @@ struct ICEPair int state; }; +struct ICEMediaInfo +{ + // first ICEPair is for RTP, second for RTCP + // ICEPair contains both local and remote address/port pairs (see above) + std::pair hevc; + std::pair opus; +}; diff --git a/src/mediamanager.cpp b/src/mediamanager.cpp index 1e0470af..70ca29d4 100644 --- a/src/mediamanager.cpp +++ b/src/mediamanager.cpp @@ -69,10 +69,10 @@ void MediaManager::addParticipant(uint32_t sessionID, std::shared_ptrmedia[0].connection_address << ":" << localInfo->media[0].receivePort; - qDebug() << localInfo->media[1].connection_address << ":" << localInfo->media[1].receivePort; - qDebug() << peerInfo->media[0].connection_address << ":" << peerInfo->media[0].receivePort; - qDebug() << peerInfo->media[0].connection_address << ":" << peerInfo->media[1].receivePort << "\n"; + qDebug() << "local opus" << localInfo->media[0].connection_address << ":" << localInfo->media[0].receivePort; + qDebug() << "local hevc" << localInfo->media[1].connection_address << ":" << localInfo->media[1].receivePort; + qDebug() << "remote opus" << peerInfo->media[0].connection_address << ":" << peerInfo->media[0].receivePort; + qDebug() << "remote hevc" << peerInfo->media[0].connection_address << ":" << peerInfo->media[1].receivePort << "\n"; #endif if(peerInfo->connection_nettype == "IN") From 5d0d6d7ac67df93c19e869c65cfc1add200f4a12 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 12 Apr 2019 14:22:38 +0300 Subject: [PATCH 09/30] Use UDPServer as pointer This allows us to more easily push pre-initialized UDPServer for use --- src/stun.cpp | 59 +++++++++++++++++++++++++++++----------------------- src/stun.h | 3 ++- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/stun.cpp b/src/stun.cpp index ae1440b7..b0853170 100644 --- a/src/stun.cpp +++ b/src/stun.cpp @@ -16,9 +16,16 @@ const uint16_t GOOGLE_STUN_PORT = 19302; const uint16_t STUN_PORT = 21000; Stun::Stun(): - udp_(), + udp_(new UDPServer), stunmsg_() -{} +{ +} + +Stun::Stun(UDPServer *server): + udp_(server), + stunmsg_() +{ +} void Stun::wantAddress(QString stunServer) { @@ -41,15 +48,15 @@ void Stun::handleHostaddress(QHostInfo info) return; } - udp_.bind(QHostAddress::AnyIPv4, STUN_PORT); - QObject::connect(&udp_, SIGNAL(messageAvailable(QByteArray)), this, SLOT(processReply(QByteArray))); + udp_->bind(QHostAddress::AnyIPv4, STUN_PORT); + QObject::connect(udp_, SIGNAL(messageAvailable(QByteArray)), this, SLOT(processReply(QByteArray))); STUNMessage request = stunmsg_.createRequest(); QByteArray message = stunmsg_.hostToNetwork(request); stunmsg_.cacheRequest(request); - udp_.sendData(message, address, GOOGLE_STUN_PORT, false); + udp_->sendData(message, address, GOOGLE_STUN_PORT, false); } bool Stun::waitForStunResponse(unsigned long timeout) @@ -114,7 +121,7 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) for (int i = 0; i < 20; ++i) { - udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); if (waitForStunResponse(20 * (i + 1))) { @@ -128,7 +135,7 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) { qDebug() << "WARNING: Failed to receive STUN Binding Response from remote!"; qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; - udp_.unbind(); + udp_->unbind(); return false; } @@ -150,7 +157,7 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) for (int i = 0; i < 5; ++i) { - udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); QThread::msleep(20); } @@ -158,7 +165,7 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) } } - udp_.unbind(); + udp_->unbind(); return msgReceived; } @@ -172,7 +179,7 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) for (int i = 0; i < 20; ++i) { - udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); if (waitForStunRequest(20 * (i + 1))) { @@ -184,7 +191,7 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) for (int i = 0; i < 3; ++i) { - udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); QThread::msleep(20); } @@ -196,7 +203,7 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) { qDebug() << "WARNING: Failed to receive STUN Binding Request from remote!"; qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; - udp_.unbind(); + udp_->unbind(); return false; } @@ -217,7 +224,7 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) for (int i = 0; i < 20; ++i) { - udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); if (waitForStunResponse(20 * (i + 1))) { @@ -226,7 +233,7 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) } } - udp_.unbind(); + udp_->unbind(); return msgReceived; } @@ -237,14 +244,14 @@ bool Stun::sendBindingRequest(ICEPair *pair, bool controller) else qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; - if (!udp_.bindRaw(QHostAddress(pair->local->address), pair->local->port)) + if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) { qDebug() << "Binding failed! Cannot send STUN Binding Requests to " << pair->remote->address << ":" << pair->remote->port; return false; } - connect(&udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); if (controller) { @@ -270,7 +277,7 @@ bool Stun::sendBindingResponse(STUNMessage& request, QString addressRemote, int for (int i = 0; i < 25; ++i) { - udp_.sendData(message, QHostAddress(addressRemote), portRemote, false); + udp_->sendData(message, QHostAddress(addressRemote), portRemote, false); if (!waitForStunRequest(20 * (i + 1))) { @@ -322,13 +329,13 @@ bool Stun::sendNominationRequest(ICEPair *pair) { qDebug() << "[controller] BINDING " << pair->local->address << " TO PORT " << pair->local->port; - if (!udp_.bindRaw(QHostAddress(pair->local->address), pair->local->port)) + if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) { qDebug() << "Binding failed! Cannot send STUN Binding Requests to " << pair->remote->address << ":" << pair->remote->address; return false; } - connect(&udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); STUNMessage request = stunmsg_.createRequest(); request.addAttribute(STUN_ATTR_ICE_CONTROLLING); @@ -343,7 +350,7 @@ bool Stun::sendNominationRequest(ICEPair *pair) for (int i = 0; i < 25; ++i) { - udp_.sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); if (waitForStunResponse(20 * (i + 1))) { @@ -352,7 +359,7 @@ bool Stun::sendNominationRequest(ICEPair *pair) } } - udp_.unbind(); + udp_->unbind(); return ok; } @@ -360,13 +367,13 @@ bool Stun::sendNominationResponse(ICEPair *pair) { qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; - if (!udp_.bindRaw(QHostAddress(pair->local->address), pair->local->port)) + if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) { qDebug() << "Binding failed! Cannot send STUN Binding Requests to " << pair->remote->address << ":" << pair->remote->address; return false; } - connect(&udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); // first send empty stun binding requests to remote to create a hole in the firewall // when the first nomination request is received (if any), start sending nomination response @@ -378,7 +385,7 @@ bool Stun::sendNominationResponse(ICEPair *pair) for (int i = 0; i < 25; ++i) { - udp_.sendData(reqMessage, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(reqMessage, QHostAddress(pair->remote->address), pair->remote->port, false); if (waitForNominationRequest(20 * (i + 1))) { @@ -399,7 +406,7 @@ bool Stun::sendNominationResponse(ICEPair *pair) for (int i = 0; i < 25; ++i) { - udp_.sendData(respMessage, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(respMessage, QHostAddress(pair->remote->address), pair->remote->port, false); // when we no longer get nomination request it means that remote has received // our response message and end the nomination process @@ -417,7 +424,7 @@ bool Stun::sendNominationResponse(ICEPair *pair) } } - udp_.unbind(); + udp_->unbind(); return nominationRecv; } diff --git a/src/stun.h b/src/stun.h index f1f18591..7e21801f 100644 --- a/src/stun.h +++ b/src/stun.h @@ -15,6 +15,7 @@ class Stun : public QObject Q_OBJECT public: Stun(); + Stun(UDPServer *udp); // send stun binding request to remote // this function is used to establish a gateway between clients @@ -82,7 +83,7 @@ private slots: bool controlleeSendBindingRequest(ICEPair *pair); // TODO [Encryption] Use TLS to send packet - UDPServer udp_; + UDPServer *udp_; StunMessageFactory stunmsg_; From 784ae95013127951d874ee97abb5f483da88a9a0 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Sun, 14 Apr 2019 10:01:59 +0300 Subject: [PATCH 10/30] Create multiplexed socket support for UDPServer Now multiple STUN objects can listen to same port and receive message only from remote candidate they wish to. --- src/udpserver.cpp | 58 +++++++++++++++++++++++++++++++++++++++-------- src/udpserver.h | 19 ++++++++++++++-- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/src/udpserver.cpp b/src/udpserver.cpp index 6439be01..dea55319 100644 --- a/src/udpserver.cpp +++ b/src/udpserver.cpp @@ -1,12 +1,14 @@ #include "udpserver.h" #include +#include +#include "stun.h" UDPServer::UDPServer(): socket_(nullptr), sendPort_(0) {} -bool UDPServer::bindSocket(const QHostAddress& address, quint16 port, bool raw) +bool UDPServer::bindSocket(const QHostAddress& address, quint16 port, enum SOCKET_TYPE type) { this->unbind(); @@ -20,13 +22,19 @@ bool UDPServer::bindSocket(const QHostAddress& address, quint16 port, bool raw) return false; } - if (raw) + switch (type) { - connect(socket_, &QUdpSocket::readyRead, this, &UDPServer::readRawData); - } - else - { - connect(socket_, &QUdpSocket::readyRead, this, &UDPServer::readData); + case SOCKET_NORMAL: + connect(socket_, &QUdpSocket::readyRead, this, &UDPServer::readData); + break; + + case SOCKET_RAW: + connect(socket_, &QUdpSocket::readyRead, this, &UDPServer::readRawData); + break; + + case SOCKET_MULTIPLEXED: + connect(socket_, &QUdpSocket::readyRead, this, &UDPServer::readMultiplexData); + break; } return true; @@ -47,12 +55,17 @@ void UDPServer::unbind() void UDPServer::bind(const QHostAddress &address, quint16 port) { - (void)bindSocket(address, port, false); + (void)bindSocket(address, port, SOCKET_NORMAL); } bool UDPServer::bindRaw(const QHostAddress &address, quint16 port) { - return bindSocket(address, port, true); + return bindSocket(address, port, SOCKET_RAW); +} + +bool UDPServer::bindMultiplexed(const QHostAddress& address, quint16 port) +{ + return bindSocket(address, port, SOCKET_MULTIPLEXED); } void UDPServer::sendData(QByteArray& data, const QHostAddress &address, quint16 port, bool untilReply) @@ -99,3 +112,30 @@ void UDPServer::readRawData() emit rawMessageAvailable(socket_->receiveDatagram()); } } + +void UDPServer::readMultiplexData() +{ + while (socket_->hasPendingDatagrams()) + { + QNetworkDatagram datagram = socket_->receiveDatagram(); + + // is anyone listening to messages from this sender? + if (listeners_.contains(datagram.senderAddress().toString())) + { + if (listeners_[datagram.senderAddress().toString()].contains(datagram.senderPort())) + { + QMetaObject::invokeMethod( + listeners_[datagram.senderAddress().toString()][datagram.senderPort()], + "recvStunMessage", + Qt::DirectConnection, + Q_ARG(QNetworkDatagram, datagram) + ); + } + } + } +} + +void UDPServer::expectReplyFrom(Stun *stun, QString& address, quint16 port) +{ + listeners_[address][port] = stun; +} diff --git a/src/udpserver.h b/src/udpserver.h index 56d1d836..a54c70d3 100644 --- a/src/udpserver.h +++ b/src/udpserver.h @@ -8,7 +8,15 @@ #include +enum SOCKET_TYPE +{ + SOCKET_NORMAL = 0, + SOCKET_RAW = 1, + SOCKET_MULTIPLEXED = 2, +}; + class QUdpSocket; +class Stun; class UDPServer : public QObject { @@ -18,14 +26,16 @@ class UDPServer : public QObject void bind(const QHostAddress& address, quint16 port); bool bindRaw(const QHostAddress& address, quint16 port); + bool bindMultiplexed(const QHostAddress& address, quint16 port); void unbind(); // sends the data using Qt UDP classes. void sendData(QByteArray& data, const QHostAddress &address, quint16 port, bool untilReply); -signals: + void expectReplyFrom(Stun *stun, QString& address, quint16 port); +signals: // send message data forward. void messageAvailable(QByteArray message); void rawMessageAvailable(QNetworkDatagram message); @@ -37,11 +47,16 @@ private slots: // read the data when it becomes available void readRawData(); + // read datagram and return it to caller who is listening connection from sender + void readMultiplexData(); + private: - bool bindSocket(const QHostAddress &address, quint16 port, bool raw); + bool bindSocket(const QHostAddress& address, quint16 port, enum SOCKET_TYPE type); QUdpSocket* socket_; + QMap> listeners_; + QTimer resendTimer_; bool waitingReply_; From d49c49668eac08a70778ce7e81b38c5ad80aeb92 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Sun, 14 Apr 2019 10:05:04 +0300 Subject: [PATCH 11/30] Add multiplexing support for STUN Now multiple STUN instances can listen to the same socket. This is necessary step to make connectivity checks run in parallel. --- src/stun.cpp | 90 +++++++++++++++++++++++++++++++++------------------- src/stun.h | 4 ++- 2 files changed, 60 insertions(+), 34 deletions(-) diff --git a/src/stun.cpp b/src/stun.cpp index b0853170..64b61bfc 100644 --- a/src/stun.cpp +++ b/src/stun.cpp @@ -17,13 +17,15 @@ const uint16_t STUN_PORT = 21000; Stun::Stun(): udp_(new UDPServer), - stunmsg_() + stunmsg_(), + multiplex_(false) { } Stun::Stun(UDPServer *server): udp_(server), - stunmsg_() + stunmsg_(), + multiplex_(true) { } @@ -135,7 +137,11 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) { qDebug() << "WARNING: Failed to receive STUN Binding Response from remote!"; qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; - udp_->unbind(); + + if (multiplex_ == false) + { + udp_->unbind(); + } return false; } @@ -147,8 +153,6 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) { if (waitForStunRequest(20 * (i + 1))) { - qDebug() << "[CONTROLLER] GOT STUN REQUEST!"; - msg = stunmsg_.createResponse(); msg.addAttribute(STUN_ATTR_ICE_CONTROLLING); @@ -165,7 +169,10 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) } } - udp_->unbind(); + if (multiplex_ == false) + { + udp_->unbind(); + } return msgReceived; } @@ -203,7 +210,11 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) { qDebug() << "WARNING: Failed to receive STUN Binding Request from remote!"; qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; - udp_->unbind(); + + if (multiplex_ == false) + { + udp_->unbind(); + } return false; } @@ -233,25 +244,26 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) } } - udp_->unbind(); + if (multiplex_ == false) + { + udp_->unbind(); + } return msgReceived; } bool Stun::sendBindingRequest(ICEPair *pair, bool controller) { - if (controller) - qDebug() << "[controller] BINDING " << pair->local->address<< " TO PORT " << pair->local->port; - else - qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; - - if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) + if (multiplex_ == false) { - qDebug() << "Binding failed! Cannot send STUN Binding Requests to " - << pair->remote->address << ":" << pair->remote->port; - return false; - } + if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) + { + qDebug() << "Binding failed! Cannot send STUN Binding Requests to " + << pair->remote->address << ":" << pair->remote->port; + return false; + } - connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + } if (controller) { @@ -327,15 +339,18 @@ void Stun::recvStunMessage(QNetworkDatagram message) bool Stun::sendNominationRequest(ICEPair *pair) { - qDebug() << "[controller] BINDING " << pair->local->address << " TO PORT " << pair->local->port; - - if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) + if (multiplex_ == false) { - qDebug() << "Binding failed! Cannot send STUN Binding Requests to " << pair->remote->address << ":" << pair->remote->address; - return false; - } + qDebug() << "Binding" << pair->local->address << " to port " << pair->local->port; - connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) + { + qDebug() << "Binding failed! Cannot send STUN Binding Requests to " << pair->remote->address << ":" << pair->remote->address; + return false; + } + + connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + } STUNMessage request = stunmsg_.createRequest(); request.addAttribute(STUN_ATTR_ICE_CONTROLLING); @@ -359,7 +374,10 @@ bool Stun::sendNominationRequest(ICEPair *pair) } } - udp_->unbind(); + if (multiplex_ == false) + { + udp_->unbind(); + } return ok; } @@ -367,13 +385,16 @@ bool Stun::sendNominationResponse(ICEPair *pair) { qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; - if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) + if (multiplex_ == false) { - qDebug() << "Binding failed! Cannot send STUN Binding Requests to " << pair->remote->address << ":" << pair->remote->address; - return false; - } + if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) + { + qDebug() << "Binding failed! Cannot send STUN Binding Requests to " << pair->remote->address << ":" << pair->remote->address; + return false; + } - connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); + } // first send empty stun binding requests to remote to create a hole in the firewall // when the first nomination request is received (if any), start sending nomination response @@ -424,7 +445,10 @@ bool Stun::sendNominationResponse(ICEPair *pair) } } - udp_->unbind(); + if (multiplex_ == false) + { + udp_->unbind(); + } return nominationRecv; } diff --git a/src/stun.h b/src/stun.h index 7e21801f..23bbf8bb 100644 --- a/src/stun.h +++ b/src/stun.h @@ -87,5 +87,7 @@ private slots: StunMessageFactory stunmsg_; - uint8_t transactionID_[12]; + // If multiplex_ is true, it means that the UDPServer has already been created for us + // and we shouldn't unbind/rebind it or attach listeners to it. + bool multiplex_; }; From 5b7a4b6a451e984da50193e9eaaf8c69a22b65b7 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 15 Apr 2019 09:49:15 +0300 Subject: [PATCH 12/30] Perform 4-way handshake when nominating the connection This way both agents now that the nomination has succeeded --- src/stun.cpp | 174 ++++++++++++++++++++++++++++++++------------------- src/stun.h | 1 + 2 files changed, 112 insertions(+), 63 deletions(-) diff --git a/src/stun.cpp b/src/stun.cpp index 64b61bfc..31df6f41 100644 --- a/src/stun.cpp +++ b/src/stun.cpp @@ -106,6 +106,21 @@ bool Stun::waitForNominationRequest(unsigned long timeout) return timer.isActive(); } +bool Stun::waitForNominationResponse(unsigned long timeout) +{ + QTimer timer; + timer.setSingleShot(true); + QEventLoop loop; + + connect(this, &Stun::nominationRecv, &loop, &QEventLoop::quit); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + + timer.start(timeout); + loop.exec(); + + return timer.isActive(); +} + // if we're the controlling agent, sending binding request is rather straight-forward: // send request, wait for response and return bool Stun::controllerSendBindingRequest(ICEPair *pair) @@ -127,7 +142,6 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) if (waitForStunResponse(20 * (i + 1))) { - qDebug() << "GOT RESPONSE!"; msgReceived = true; break; } @@ -159,7 +173,7 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) message = stunmsg_.hostToNetwork(msg); msgReceived = true; - for (int i = 0; i < 5; ++i) + for (int i = 0; i < 3; ++i) { udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); QThread::msleep(20); @@ -181,7 +195,7 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) Q_ASSERT(pair != nullptr); bool msgReceived = false; - QByteArray message = QByteArray("hole punch"); + QByteArray message = QByteArray("hole punch normal"); STUNMessage msg; for (int i = 0; i < 20; ++i) @@ -273,31 +287,6 @@ bool Stun::sendBindingRequest(ICEPair *pair, bool controller) return controlleeSendBindingRequest(pair); } -bool Stun::sendBindingResponse(STUNMessage& request, QString addressRemote, int portRemote) -{ - STUNMessage response = stunmsg_.createResponse(request); - - response.addAttribute(STUN_ATTR_PRIORITY, 1337); - - response.addAttribute( - request.hasAttribute(STUN_ATTR_ICE_CONTROLLED) - ? STUN_ATTR_ICE_CONTROLLING - : STUN_ATTR_ICE_CONTROLLED - ); - - QByteArray message = stunmsg_.hostToNetwork(response); - - for (int i = 0; i < 25; ++i) - { - udp_->sendData(message, QHostAddress(addressRemote), portRemote, false); - - if (!waitForStunRequest(20 * (i + 1))) - { - break; - } - } -} - // either we got Stun binding request -> send binding response // or Stun binding response -> mark candidate as valid void Stun::recvStunMessage(QNetworkDatagram message) @@ -322,9 +311,13 @@ void Stun::recvStunMessage(QNetworkDatagram message) } else if (stunMsg.getType() == STUN_RESPONSE) { + if (stunMsg.hasAttribute(STUN_ATTR_USE_CANDIATE)) + { + emit nominationRecv(); + } // if this message is a response to a request we just sent, its TransactionID should be cached // if not, vertaa viimeksi lähetetyn TransactionID:tä vasten - if (stunmsg_.validateStunResponse(stunMsg, message.senderAddress(), message.senderPort())) + else if (stunmsg_.validateStunResponse(stunMsg, message.senderAddress(), message.senderPort())) { emit parsingDone(); } @@ -367,13 +360,50 @@ bool Stun::sendNominationRequest(ICEPair *pair) { udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); - if (waitForStunResponse(20 * (i + 1))) + if (waitForNominationResponse(20 * (i + 1))) { ok = true; break; } } + if (ok == false) + { + qDebug() << "WARNING: Failed to receive Nomination Response from remote!"; + qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; + + if (multiplex_ == false) + { + udp_->unbind(); + } + return false; + } + + // the first part of connectivity check is done, now we must wait for + // remote's binding request and responed to them + ok = false; + + for (int i = 0; i < 20; ++i) + { + if (waitForNominationRequest(20 * (i + 1))) + { + STUNMessage msg = stunmsg_.createResponse(); + msg.addAttribute(STUN_ATTR_ICE_CONTROLLING); + msg.addAttribute(STUN_ATTR_USE_CANDIATE); + + message = stunmsg_.hostToNetwork(msg); + ok = true; + + for (int i = 0; i < 3; ++i) + { + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + QThread::msleep(20); + } + + break; + } + } + if (multiplex_ == false) { udp_->unbind(); @@ -383,10 +413,10 @@ bool Stun::sendNominationRequest(ICEPair *pair) bool Stun::sendNominationResponse(ICEPair *pair) { - qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; - if (multiplex_ == false) { + qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; + if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) { qDebug() << "Binding failed! Cannot send STUN Binding Requests to " << pair->remote->address << ":" << pair->remote->address; @@ -396,52 +426,69 @@ bool Stun::sendNominationResponse(ICEPair *pair) connect(udp_, &UDPServer::rawMessageAvailable, this, &Stun::recvStunMessage); } - // first send empty stun binding requests to remote to create a hole in the firewall - // when the first nomination request is received (if any), start sending nomination response - STUNMessage request = stunmsg_.createRequest(); - request.addAttribute(STUN_ATTR_ICE_CONTROLLED); - - QByteArray reqMessage = stunmsg_.hostToNetwork(request); - bool nominationRecv = false; + STUNMessage msg; + QByteArray message = QByteArray("hole punch nomination"); + bool nominationRecv = false; - for (int i = 0; i < 25; ++i) + for (int i = 0; i < 128; ++i) { - udp_->sendData(reqMessage, QHostAddress(pair->remote->address), pair->remote->port, false); + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); if (waitForNominationRequest(20 * (i + 1))) { + msg = stunmsg_.createResponse(); + msg.addAttribute(STUN_ATTR_ICE_CONTROLLED); + msg.addAttribute(STUN_ATTR_USE_CANDIATE); + + message = stunmsg_.hostToNetwork(msg); nominationRecv = true; + + for (int i = 0; i < 3; ++i) + { + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + QThread::msleep(20); + } + break; } } - if (nominationRecv) + if (nominationRecv == false) { - nominationRecv = false; + qDebug() << "WARNING: Failed to receive Nomination from remote!"; + qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; - STUNMessage response = stunmsg_.createResponse(); - response.addAttribute(STUN_ATTR_ICE_CONTROLLED); - response.addAttribute(STUN_ATTR_USE_CANDIATE); + if (multiplex_ == false) + { + udp_->unbind(); + } + return false; + } + + // the first part of connective check is done (remote sending us binding request) + // now we must do the same but this we're sending the request and they're responding + nominationRecv = false; - QByteArray respMessage = stunmsg_.hostToNetwork(response); + // we've succesfully responded to remote's binding request, now it's our turn to + // send request and remote must respond to them + STUNMessage request = stunmsg_.createRequest(); + request.addAttribute(STUN_ATTR_ICE_CONTROLLED); + request.addAttribute(STUN_ATTR_PRIORITY, pair->priority); + request.addAttribute(STUN_ATTR_USE_CANDIATE); - for (int i = 0; i < 25; ++i) + message = stunmsg_.hostToNetwork(request); + + // we're expecting a response from remote to this request + stunmsg_.cacheRequest(request); + + for (int i = 0; i < 20; ++i) + { + udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); + + if (waitForNominationResponse(20 * (i + 1))) { - udp_->sendData(respMessage, QHostAddress(pair->remote->address), pair->remote->port, false); - - // when we no longer get nomination request it means that remote has received - // our response message and end the nomination process - // - // We can stop sending nomination responses when the waitForNominationRequest() timeouts - // - // Because we got here (we received nomination request in the previous step) we can assume - // that the connection works in both ends and waitForNominationRequest() doesn't just timeout - // because it doesn't receive anything - if (!waitForNominationRequest(20 * (i + 1))) - { - nominationRecv = true; - break; - } + nominationRecv = true; + break; } } @@ -449,6 +496,7 @@ bool Stun::sendNominationResponse(ICEPair *pair) { udp_->unbind(); } + return nominationRecv; } diff --git a/src/stun.h b/src/stun.h index 23bbf8bb..aaee8d4b 100644 --- a/src/stun.h +++ b/src/stun.h @@ -58,6 +58,7 @@ private slots: bool waitForStunResponse(unsigned long timeout); bool waitForStunRequest(unsigned long timeout); bool waitForNominationRequest(unsigned long timeout); + bool waitForNominationResponse(unsigned long timeout); // If we're the controlling agent we start by sending STUN Binding Requests to remote // When we receive a response to one of our STUN Binding Requests, we start listening to From af09119d0980d1ab7e01c92a04fa979eb2af404e Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 15 Apr 2019 09:50:38 +0300 Subject: [PATCH 13/30] Use one ConnectionTester to test only one connection This way RTP and RTCP can be tested in parallel --- src/connectiontester.cpp | 83 ++++++++++------------------------------ src/connectiontester.h | 16 ++++---- 2 files changed, 29 insertions(+), 70 deletions(-) diff --git a/src/connectiontester.cpp b/src/connectiontester.cpp index 2a436dfb..4a0cc7d0 100644 --- a/src/connectiontester.cpp +++ b/src/connectiontester.cpp @@ -2,8 +2,7 @@ #include "stun.h" ConnectionTester::ConnectionTester(): - rtp_pair_(nullptr), - rtcp_pair_(nullptr), + pair_(nullptr), controller_(false) { } @@ -12,10 +11,9 @@ ConnectionTester::~ConnectionTester() { } -void ConnectionTester::setCandidatePair(ICEPair *rtp_pair, ICEPair *rtcp_pair) +void ConnectionTester::setCandidatePair(ICEPair *pair) { - rtp_pair_ = rtp_pair; - rtcp_pair_ = rtcp_pair; + pair_ = pair; } void ConnectionTester::isController(bool controller) @@ -23,84 +21,45 @@ void ConnectionTester::isController(bool controller) controller_ = controller; } -void ConnectionTester::printMessage(QString message) +void ConnectionTester::setStun(Stun *stun) { - if (controller_) - qDebug() << "[controller]" << message; - else - qDebug() << "[controllee]" << message; + stun_ = stun; } void ConnectionTester::run() { - if (rtp_pair_ == nullptr || rtcp_pair_ == nullptr) + if (pair_ == nullptr) { - qDebug() << "Unable to test connection, RTP or RTCP candidate is NULL!"; + qDebug() << "Unable to test connection, candidate is NULL!"; return; } - Stun stun; - - rtcp_pair_->state = PAIR_WAITING; - rtp_pair_->state = PAIR_IN_PROGRESS; - - printMessage("STARTING"); - - if (stun.sendBindingRequest(rtp_pair_, controller_)) - { - rtp_pair_->state = PAIR_SUCCEEDED; - rtcp_pair_->state = PAIR_IN_PROGRESS; - - printMessage("RTP NOMINATED!!!"); - - if (stun.sendBindingRequest(rtcp_pair_, controller_)) - { - printMessage("RTCP NOMINATED!"); - rtcp_pair_->state = PAIR_SUCCEEDED; - } - else - { - printMessage("RTCP FAILED!"); - rtcp_pair_->state = PAIR_FAILED; - } - } - else - { - printMessage("RTP FAILED!"); - rtp_pair_->state = PAIR_FAILED; - } + pair_->state = PAIR_IN_PROGRESS; - /* remote did not respond to our binding requests -> terminate */ - if (rtp_pair_->state == PAIR_FAILED || rtcp_pair_->state == PAIR_FAILED) + if (!stun_->sendBindingRequest(pair_, controller_)) { + qDebug() << "FAILED!"; return; } - // nomination is handled by the FlowController so if we're the controller, - // terminate connection testing - if (controller_) - { - emit testingDone(rtp_pair_, rtcp_pair_); - return; - } + pair_->state = PAIR_SUCCEEDED; - // otherwise continue sending requests to remote to keep the hole in firewall open - if (!stun.sendNominationResponse(rtp_pair_)) + if (controller_) { - qDebug() << "failed to receive nomination for RTP candidate:\n" - << "\t\local:" << rtp_pair_->local->address << ":" << rtcp_pair_->local->port << "\n" - << "\tremote:" << rtp_pair_->remote->address << ":" << rtcp_pair_->remote->port; + qDebug() << "pair success" << pair_->local->address << pair_->local->port << pair_->remote->address << pair_->remote->port << pair_->local->component; + emit testingDone(pair_); return; } - if (!stun.sendNominationResponse(rtcp_pair_)) + if (!stun_->sendNominationResponse(pair_)) { - qDebug() << "failed to receive nomination for RTCP!"; + qDebug() << "failed to receive nomination for candidate:\n" + << "\tlocal:" << pair_->local->address << ":" << pair_->local->port << "\n" + << "\tremote:" << pair_->remote->address << ":" << pair_->remote->port; + pair_->state = PAIR_FAILED; return; } - rtp_pair_->state = PAIR_NOMINATED; - rtcp_pair_->state = PAIR_NOMINATED; - - emit testingDone(rtp_pair_, rtcp_pair_); + pair_->state = PAIR_NOMINATED; + emit testingDone(pair_); } diff --git a/src/connectiontester.h b/src/connectiontester.h index 73a7b499..f8d555a3 100644 --- a/src/connectiontester.h +++ b/src/connectiontester.h @@ -2,6 +2,8 @@ #include #include "icetypes.h" +#include "udpserver.h" +#include "stun.h" class ConnectionTester : public QThread { @@ -10,7 +12,8 @@ class ConnectionTester : public QThread public: ConnectionTester(); ~ConnectionTester(); - void setCandidatePair(ICEPair *pair_rtp, ICEPair *pair_rtcp); + void setStun(Stun *stun); + void setCandidatePair(ICEPair *pair); // controller_ defines the course of action after candiate pair has been validated. // If the ConnectionTester belongs to FlowController it terminates immediately after @@ -26,16 +29,13 @@ class ConnectionTester : public QThread // testingDone() is emitted when the connection testing has ended // // if the tested candidate succeeded (remote responded to our requests), - // rtp and rtcp point to valid ICEPairs - // - // if something failed, rtp and rtcp are nullptr - void testingDone(ICEPair *rtp, ICEPair *rtcp); + // connection points to valid ICEPair, otherwise it's nullptr + void testingDone(ICEPair *connection); protected: void run(); - ICEPair *rtp_pair_; - ICEPair *rtcp_pair_; - + ICEPair *pair_; bool controller_; + Stun *stun_; }; From 9ff6afdaf340e5401ff33e204270c40f0fec1cb4 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 15 Apr 2019 09:52:57 +0300 Subject: [PATCH 14/30] Use multiplexed UDP socket for candidates having the same local address:port --- src/iceflowcontrol.cpp | 187 +++++++++++++++++++++++++++++++++++------ src/iceflowcontrol.h | 9 +- 2 files changed, 171 insertions(+), 25 deletions(-) diff --git a/src/iceflowcontrol.cpp b/src/iceflowcontrol.cpp index 294e0cfa..45e64d3d 100644 --- a/src/iceflowcontrol.cpp +++ b/src/iceflowcontrol.cpp @@ -1,10 +1,24 @@ #include #include +#include #include "connectiontester.h" #include "iceflowcontrol.h" #include "ice.h" +struct Pair +{ + Stun *stun; + ICEPair *pair; +}; + +// TODO explain +struct ConnectionBucket +{ + UDPServer *server; + QList pairs; +}; + FlowAgent::FlowAgent(): candidates_(nullptr), sessionID_(0) @@ -28,15 +42,27 @@ void FlowAgent::setSessionID(uint32_t sessionID) sessionID_ = sessionID; } -void FlowAgent::nominationDone(ICEPair *rtp, ICEPair *rtcp) +void FlowAgent::nominationDone(ICEPair *connection) { - if (nominated_mtx.try_lock()) + if (connection->local->component == RTP) { - nominated_rtp_ = rtp; - nominated_rtcp_ = rtcp; + nominated_[connection->local->address].first = connection; + } + else + { + nominated_[connection->local->address].second = connection; } - emit endNomination(); + if (nominated_[connection->local->address].first != nullptr && + nominated_[connection->local->address].second != nullptr) + { + if (nominated_mtx.try_lock()) + { + nominated_rtp_ = nominated_[connection->local->address].first; + nominated_rtcp_ = nominated_[connection->local->address].second; + emit endNomination(); + } + } } bool FlowAgent::waitForResponses(unsigned long timeout) @@ -49,7 +75,7 @@ bool FlowAgent::waitForResponses(unsigned long timeout) connect(this, &FlowAgent::endNomination, &loop, &QEventLoop::quit); connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); - timer.start(20000); + timer.start(timeout); loop.exec(); return timer.isActive(); @@ -59,10 +85,6 @@ void FlowController::run() { Stun stun; - // TODO how long should we sleep??? - /* for (volatile int i = 0; i < 10000000; ++i) */ - /* ; */ - if (candidates_ == nullptr || candidates_->size() == 0) { qDebug() << "ERROR: invalid candidates, unable to perform ICE candidate negotiation!"; @@ -70,17 +92,65 @@ void FlowController::run() return; } + // because we can only bind to a port once (no multithreaded access), we must divide + // the candidates into "buckets" and each ConnectionTester is responsible for testing the bucket + // + // Each bucket contains all candidates for every interface. + // ConnectionTester then binds to this interface and takes care of sending the STUN Binding requests std::vector> workerThreads; + QList buckets; - for (int i = 0; i < candidates_->size(); i += 2) - { - workerThreads.push_back(std::make_unique()); + int bucketNum = -1; + QString prevAddr = ""; + uint16_t prevPort = 0; - connect(workerThreads.back().get(), &ConnectionTester::testingDone, this, &FlowAgent::nominationDone, Qt::DirectConnection); + for (int i = 0; i < candidates_->size(); ++i) + { + if (candidates_->at(i)->local->address != prevAddr || candidates_->at(i)->local->port != prevPort) + { + bucketNum++; + buckets.push_back({ + .server = new UDPServer, + .pairs = QList() + }); + + // because we cannot modify create new objects from child threads (in this case new socket) + // we must initialize both UDPServer and Stun objects here so that ConnectionTester doesn't have to do + // anything but to test the connection + (void)buckets[bucketNum].server->bindMultiplexed(QHostAddress(candidates_->at(i)->local->address), candidates_->at(i)->local->port); + } + + buckets[bucketNum].pairs.push_back({ + .stun = new Stun(buckets[bucketNum].server), + .pair = candidates_->at(i) + }); + + // when the UDPServer receives a datagram from remote->address:remote->port, + // it will send a signal containing the datagram to this Stun object + // + // This way multiple Stun instances can listen to same socket + buckets[bucketNum].server->expectReplyFrom(buckets[bucketNum].pairs.back().stun, + candidates_->at(i)->remote->address, + candidates_->at(i)->remote->port); + + prevAddr = candidates_->at(i)->local->address; + prevPort = candidates_->at(i)->local->port; + } - workerThreads.back()->setCandidatePair(candidates_->at(i), candidates_->at(i + 1)); - workerThreads.back()->isController(true); - workerThreads.back()->start(); + for (int i = 0; i < buckets.size(); ++i) + { + for (int k = 0; k < buckets[i].pairs.size(); ++k) + { + workerThreads.push_back(std::make_unique()); + + connect(workerThreads.back().get(), &ConnectionTester::testingDone, this, &FlowAgent::nominationDone, Qt::DirectConnection); + + buckets[i].pairs[k].stun->moveToThread(workerThreads.back().get()); + workerThreads.back()->setCandidatePair(buckets[i].pairs[k].pair); + workerThreads.back()->setStun(buckets[i].pairs[k].stun); + workerThreads.back()->isController(true); + workerThreads.back()->start(); + } } bool nominationSucceeded = waitForResponses(10000); @@ -92,6 +162,18 @@ void FlowController::run() workerThreads[i]->wait(); } + // deallocate all memory consumed by connection buckets + for (int i = 0; i < buckets.size(); ++i) + { + buckets[i].server->unbind(); + delete buckets[i].server; + + for (int k = 0; k < buckets[i].pairs.size(); ++k) + { + delete buckets[i].pairs[k].stun; + } + } + // we've spawned threads for each candidate, wait for responses at most 10 seconds if (!nominationSucceeded) { @@ -132,19 +214,65 @@ void FlowControllee::run() return; } + // because we can only bind to a port once (no multithreaded access), we must divide + // the candidates into "buckets" and each ConnectionTester is responsible for testing the bucket + // + // Each bucket contains all candidates for every interface. + // ConnectionTester then binds to this interface and takes care of sending the STUN Binding requests std::vector> workerThreads; + QList buckets; - for (int i = 0; i < candidates_->size(); i += 2) + int bucketNum = -1; + QString prevAddr = ""; + uint16_t prevPort = 0; + + for (int i = 0; i < candidates_->size(); ++i) { - workerThreads.push_back(std::make_unique()); + if (candidates_->at(i)->local->address != prevAddr || candidates_->at(i)->local->port != prevPort) + { + bucketNum++; + buckets.push_back({ + .server = new UDPServer, + .pairs = QList() + }); + + // because we cannot modify create new objects from child threads (in this case new socket) + // we must initialize both UDPServer and Stun objects here so that ConnectionTester doesn't have to do + // anything but to test the connection + (void)buckets[bucketNum].server->bindMultiplexed(QHostAddress(candidates_->at(i)->local->address), candidates_->at(i)->local->port); + } + + buckets[bucketNum].pairs.push_back({ + .stun = new Stun(buckets[bucketNum].server), + .pair = candidates_->at(i) + }); + + // when the UDPServer receives a datagram from remote->address:remote->port, + // it will send a signal containing the datagram to this Stun object + // + // This way multiple Stun instances can listen to same socket + buckets[bucketNum].server->expectReplyFrom(buckets[bucketNum].pairs.back().stun, + candidates_->at(i)->remote->address, + candidates_->at(i)->remote->port); + + prevAddr = candidates_->at(i)->local->address; + prevPort = candidates_->at(i)->local->port; + } - connect(workerThreads.back().get(), &ConnectionTester::testingDone, this, &FlowAgent::nominationDone, Qt::DirectConnection); + for (int i = 0; i < buckets.size(); ++i) + { + for (int k = 0; k < buckets[i].pairs.size(); ++k) + { + workerThreads.push_back(std::make_unique()); - workerThreads.back()->setCandidatePair(candidates_->at(i), candidates_->at(i + 1)); - workerThreads.back()->isController(false); - workerThreads.back()->start(); - } + connect(workerThreads.back().get(), &ConnectionTester::testingDone, this, &FlowAgent::nominationDone, Qt::DirectConnection); + workerThreads.back()->setCandidatePair(buckets[i].pairs[k].pair); + workerThreads.back()->setStun(buckets[i].pairs[k].stun); + workerThreads.back()->isController(false); + workerThreads.back()->start(); + } + } bool nominationSucceeded = waitForResponses(20000); @@ -155,6 +283,17 @@ void FlowControllee::run() workerThreads[i]->wait(); } + // deallocate all memory consumed by connection buckets + for (int i = 0; i < buckets.size(); ++i) + { + delete buckets[i].server; + + for (int k = 0; k < buckets[i].pairs.size(); ++k) + { + delete buckets[i].pairs[k].stun; + } + } + // wait for nomination from remote, wait at most 20 seconds if (!nominationSucceeded) { diff --git a/src/iceflowcontrol.h b/src/iceflowcontrol.h index 472c058c..345d0a9c 100644 --- a/src/iceflowcontrol.h +++ b/src/iceflowcontrol.h @@ -24,7 +24,7 @@ class FlowAgent : public QThread void endNomination(); public slots: - void nominationDone(ICEPair *rtp, ICEPair *rtcp); + void nominationDone(ICEPair *connection); protected: void run(); @@ -32,6 +32,13 @@ public slots: QList *candidates_; uint32_t sessionID_; + // temporary storage for succeeded pairs, when both RTP and RTCP + // of some candidate pair succeeds, endNomination() signal is emitted + // and the succeeded pair is copied to nominated_rtp_ and nominated_rtcp_ + // + // the first candidate pair that has both RTP and RTCP tested is chosen + QMap> nominated_; + ICEPair *nominated_rtp_; ICEPair *nominated_rtcp_; QMutex nominated_mtx; From 18f890763bdf4f7e371e182e67b30ded8bc77a18 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 15 Apr 2019 10:38:50 +0300 Subject: [PATCH 15/30] Match Host/Server-reflexive and RTP/RTCP candidates --- src/ice.cpp | 52 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/ice.cpp b/src/ice.cpp index d4628633..8fdddbbc 100644 --- a/src/ice.cpp +++ b/src/ice.cpp @@ -82,12 +82,19 @@ QList ICE::generateICECandidates() if (stunAddress_ != QHostAddress("")) { - std::pair cands = makeCandidate(stunAddress_, "srflx"); + candidate = makeCandidate(stunAddress_, "srflx"); - candidates.push_back(cands.first); - candidates.push_back(cands.second); + candidates.push_back(candidate.first); + candidates.push_back(candidate.second); } +#if 0 + candidate = makeCandidate(QHostAddress("172.17.0.1"), "host"); + + candidates.push_back(candidate.first); + candidates.push_back(candidate.second); +#endif + return candidates; } @@ -147,23 +154,38 @@ void ICE::printCandidate(ICEInfo *candidate) << candidate->address << ":" << candidate->port; } -// TODO sort them by priority/type/whatever first and make sure ports match!!! QList *ICE::makeCandiatePairs(QList& local, QList& remote) { QList *pairs = new QList; - // create pairs - for (int i = 0; i < qMin(local.size(), remote.size()); ++i) + // match all host candidates with remote (remote does the same) + for (int i = 0; i < local.size(); ++i) { - ICEPair *pair = new ICEPair; + for (int k = 0; k < remote.size(); ++k) + { + // type (host/server reflexive) and component (RTP/RTCP) has to match + if (local[i]->type == remote[k]->type && + local[i]->component == remote[k]->component) + { + ICEPair *pair = new ICEPair; - pair->local = local[i]; - pair->remote = remote[i]; - pair->priority = qMin(local[i]->priority, remote[i]->priority); - pair->state = PAIR_FROZEN; + pair->local = local[i]; + pair->remote = remote[k]; + pair->priority = qMin(local[i]->priority, remote[k]->priority); // TODO spec + pair->state = PAIR_FROZEN; - pairs->push_back(pair); + pairs->push_back(pair); + } + } + } + +#if 0 + for (int i = 0; i < pairs->size(); ++i) + { + qDebug() << pairs->at(i)->local->type << ":" << pairs->at(i)->local->address << ":" << pairs->at(i)->local->port + << pairs->at(i)->remote->address << ":" << pairs->at(i)->remote->port; } +#endif return pairs; } @@ -324,11 +346,11 @@ void ICE::handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, u // we need to call makePortPairAvailable() only for RTP candidate if (pair->local->component == RTP) { - parameters_.makePortPairAvailable(pair->local->port); + parameters_.deallocateMediaPorts(pair->local->port); } - delete pair->remote; - delete pair; + /* delete pair->remote; */ + /* delete pair; */ } } From 136f470f7a4595b2be77af2b878fa85cab8237c1 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 15 Apr 2019 11:44:42 +0300 Subject: [PATCH 16/30] Don't test the connection if binding to socket fails --- src/iceflowcontrol.cpp | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/iceflowcontrol.cpp b/src/iceflowcontrol.cpp index 45e64d3d..330e4e45 100644 --- a/src/iceflowcontrol.cpp +++ b/src/iceflowcontrol.cpp @@ -117,7 +117,16 @@ void FlowController::run() // because we cannot modify create new objects from child threads (in this case new socket) // we must initialize both UDPServer and Stun objects here so that ConnectionTester doesn't have to do // anything but to test the connection - (void)buckets[bucketNum].server->bindMultiplexed(QHostAddress(candidates_->at(i)->local->address), candidates_->at(i)->local->port); + if (buckets[bucketNum].server->bindMultiplexed(QHostAddress(candidates_->at(i)->local->address), candidates_->at(i)->local->port) == false) + { + delete buckets[bucketNum].server; + buckets[bucketNum].server = nullptr; + } + } + + if (buckets[bucketNum].server == nullptr) + { + continue; } buckets[bucketNum].pairs.push_back({ @@ -239,7 +248,18 @@ void FlowControllee::run() // because we cannot modify create new objects from child threads (in this case new socket) // we must initialize both UDPServer and Stun objects here so that ConnectionTester doesn't have to do // anything but to test the connection - (void)buckets[bucketNum].server->bindMultiplexed(QHostAddress(candidates_->at(i)->local->address), candidates_->at(i)->local->port); + // + // Binding might fail, if so happens no STUN objects are created for this socket + if (buckets[bucketNum].server->bindMultiplexed(QHostAddress(candidates_->at(i)->local->address), candidates_->at(i)->local->port) == false) + { + delete buckets[bucketNum].server; + buckets[bucketNum].server = nullptr; + } + } + + if (buckets[bucketNum].server == nullptr) + { + continue; } buckets[bucketNum].pairs.push_back({ From c4b16cf1a585d4a2062685cf733a370b72cdfade Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 15 Apr 2019 11:50:42 +0300 Subject: [PATCH 17/30] Deallocate unused resources in ice.cpp --- src/ice.cpp | 4 ++-- src/sdpparametermanager.cpp | 9 ++++++++- src/sdpparametermanager.h | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/ice.cpp b/src/ice.cpp index 8fdddbbc..32d47ec2 100644 --- a/src/ice.cpp +++ b/src/ice.cpp @@ -349,8 +349,8 @@ void ICE::handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, u parameters_.deallocateMediaPorts(pair->local->port); } - /* delete pair->remote; */ - /* delete pair; */ + delete pair->remote; + delete pair; } } diff --git a/src/sdpparametermanager.cpp b/src/sdpparametermanager.cpp index d8a29825..88f1e80f 100644 --- a/src/sdpparametermanager.cpp +++ b/src/sdpparametermanager.cpp @@ -85,8 +85,15 @@ uint16_t SDPParameterManager::allocateMediaPorts() return start; } -uint16_t SDPParameterManager::deallocateMediaPorts(uint16_t start) +void SDPParameterManager::deallocateMediaPorts(uint16_t start) { + portLock_.lock(); + + availablePorts_.push_back(start); + availablePorts_.push_back(start + 2); + remainingPorts_ += 4; + + portLock_.unlock(); } uint16_t SDPParameterManager::nextAvailablePortPair() diff --git a/src/sdpparametermanager.h b/src/sdpparametermanager.h index 9a9cfdc8..616eca4f 100644 --- a/src/sdpparametermanager.h +++ b/src/sdpparametermanager.h @@ -38,7 +38,7 @@ class SDPParameterManager // allocate contiguous port range uint16_t allocateMediaPorts(); - uint16_t deallocateMediaPorts(uint16_t start); + void deallocateMediaPorts(uint16_t start); private: uint16_t remainingPorts_; From dd592e1c89075fa5a684bf178b1e9532bc54919e Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Mon, 15 Apr 2019 12:25:30 +0300 Subject: [PATCH 18/30] Terminate all other ConnectionTesters when nomination has finished This significantly speeds up the process as the FlowAgent doensn't have to wait for other ConnectionTesters to finish their connectivity checks --- src/connectiontester.cpp | 8 +++++ src/connectiontester.h | 7 +++++ src/stun.cpp | 68 +++++++++++++++++++++++++++++++++++----- src/stun.h | 9 ++++++ 4 files changed, 84 insertions(+), 8 deletions(-) diff --git a/src/connectiontester.cpp b/src/connectiontester.cpp index 4a0cc7d0..dc38d379 100644 --- a/src/connectiontester.cpp +++ b/src/connectiontester.cpp @@ -24,6 +24,14 @@ void ConnectionTester::isController(bool controller) void ConnectionTester::setStun(Stun *stun) { stun_ = stun; + + QObject::connect(this, &ConnectionTester::stopTesting, stun, &Stun::stopTesting); +} + +void ConnectionTester::quit() +{ + emit stopTesting(); + QThread::quit(); } void ConnectionTester::run() diff --git a/src/connectiontester.h b/src/connectiontester.h index f8d555a3..bc4d9ca1 100644 --- a/src/connectiontester.h +++ b/src/connectiontester.h @@ -25,6 +25,9 @@ class ConnectionTester : public QThread void printMessage(QString message); +public slots: + void quit(); + signals: // testingDone() is emitted when the connection testing has ended // @@ -32,6 +35,10 @@ class ConnectionTester : public QThread // connection points to valid ICEPair, otherwise it's nullptr void testingDone(ICEPair *connection); + // send signal to Stun object that it should terminate testing the candidate + // (break from the event loop associated with testing) + void stopTesting(); + protected: void run(); diff --git a/src/stun.cpp b/src/stun.cpp index 31df6f41..c1a7114d 100644 --- a/src/stun.cpp +++ b/src/stun.cpp @@ -18,14 +18,16 @@ const uint16_t STUN_PORT = 21000; Stun::Stun(): udp_(new UDPServer), stunmsg_(), - multiplex_(false) + multiplex_(false), + interrupted_(false) { } Stun::Stun(UDPServer *server): udp_(server), stunmsg_(), - multiplex_(true) + multiplex_(true), + interrupted_(false) { } @@ -67,8 +69,9 @@ bool Stun::waitForStunResponse(unsigned long timeout) timer.setSingleShot(true); QEventLoop loop; - connect(this, &Stun::parsingDone, &loop, &QEventLoop::quit); - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + connect(this, &Stun::parsingDone, &loop, &QEventLoop::quit); + connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); timer.start(timeout); loop.exec(); @@ -82,8 +85,9 @@ bool Stun::waitForStunRequest(unsigned long timeout) timer.setSingleShot(true); QEventLoop loop; - connect(this, &Stun::requestRecv, &loop, &QEventLoop::quit); - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + connect(this, &Stun::requestRecv, &loop, &QEventLoop::quit); + connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); timer.start(timeout); loop.exec(); @@ -98,7 +102,8 @@ bool Stun::waitForNominationRequest(unsigned long timeout) QEventLoop loop; connect(this, &Stun::nominationRecv, &loop, &QEventLoop::quit); - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); timer.start(timeout); loop.exec(); @@ -113,7 +118,8 @@ bool Stun::waitForNominationResponse(unsigned long timeout) QEventLoop loop; connect(this, &Stun::nominationRecv, &loop, &QEventLoop::quit); - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); timer.start(timeout); loop.exec(); @@ -142,6 +148,11 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) if (waitForStunResponse(20 * (i + 1))) { + if (interrupted_) + { + return false; + } + msgReceived = true; break; } @@ -167,6 +178,11 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) { if (waitForStunRequest(20 * (i + 1))) { + if (interrupted_) + { + return false; + } + msg = stunmsg_.createResponse(); msg.addAttribute(STUN_ATTR_ICE_CONTROLLING); @@ -204,6 +220,11 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) if (waitForStunRequest(20 * (i + 1))) { + if (interrupted_) + { + return false; + } + msg = stunmsg_.createResponse(); msg.addAttribute(STUN_ATTR_ICE_CONTROLLED); @@ -253,6 +274,11 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) if (waitForStunResponse(20 * (i + 1))) { + if (interrupted_) + { + return false; + } + msgReceived = true; break; } @@ -362,6 +388,11 @@ bool Stun::sendNominationRequest(ICEPair *pair) if (waitForNominationResponse(20 * (i + 1))) { + if (interrupted_) + { + return false; + } + ok = true; break; } @@ -387,6 +418,11 @@ bool Stun::sendNominationRequest(ICEPair *pair) { if (waitForNominationRequest(20 * (i + 1))) { + if (interrupted_) + { + return false; + } + STUNMessage msg = stunmsg_.createResponse(); msg.addAttribute(STUN_ATTR_ICE_CONTROLLING); msg.addAttribute(STUN_ATTR_USE_CANDIATE); @@ -436,6 +472,11 @@ bool Stun::sendNominationResponse(ICEPair *pair) if (waitForNominationRequest(20 * (i + 1))) { + if (interrupted_) + { + return false; + } + msg = stunmsg_.createResponse(); msg.addAttribute(STUN_ATTR_ICE_CONTROLLED); msg.addAttribute(STUN_ATTR_USE_CANDIATE); @@ -487,6 +528,11 @@ bool Stun::sendNominationResponse(ICEPair *pair) if (waitForNominationResponse(20 * (i + 1))) { + if (interrupted_) + { + return false; + } + nominationRecv = true; break; } @@ -532,3 +578,9 @@ void Stun::processReply(QByteArray data) emit stunError(); } } + +void Stun::stopTesting() +{ + interrupted_ = true; + emit stopEventLoop(); +} diff --git a/src/stun.h b/src/stun.h index aaee8d4b..8f6c6746 100644 --- a/src/stun.h +++ b/src/stun.h @@ -42,12 +42,16 @@ class Stun : public QObject void wantAddress(QString stunServer); +public slots: + void stopTesting(); + signals: void addressReceived(QHostAddress address); void stunError(); void parsingDone(); void nominationRecv(); void requestRecv(); + void stopEventLoop(); private slots: void handleHostaddress(QHostInfo info); @@ -91,4 +95,9 @@ private slots: // If multiplex_ is true, it means that the UDPServer has already been created for us // and we shouldn't unbind/rebind it or attach listeners to it. bool multiplex_; + + // When waitFor(Stun|Nomination)(Request|Response) returns, the calling code should + // check whether interrupt flag has been set. It means that the running thread has + // decided to terminate processing and Stun shouln't continue with the process any further + bool interrupted_; }; From 93197f6420202449bae9926608c817b304935753 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 16 Apr 2019 08:58:56 +0300 Subject: [PATCH 19/30] Mark the pair nominated only when it's actually nominated --- src/connectiontester.cpp | 1 - src/iceflowcontrol.cpp | 5 ++++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/connectiontester.cpp b/src/connectiontester.cpp index dc38d379..f1c4638a 100644 --- a/src/connectiontester.cpp +++ b/src/connectiontester.cpp @@ -68,6 +68,5 @@ void ConnectionTester::run() return; } - pair_->state = PAIR_NOMINATED; emit testingDone(pair_); } diff --git a/src/iceflowcontrol.cpp b/src/iceflowcontrol.cpp index 330e4e45..2e9aec8b 100644 --- a/src/iceflowcontrol.cpp +++ b/src/iceflowcontrol.cpp @@ -27,7 +27,6 @@ FlowAgent::FlowAgent(): FlowAgent::~FlowAgent() { - delete candidates_; } void FlowAgent::run() { } @@ -60,6 +59,10 @@ void FlowAgent::nominationDone(ICEPair *connection) { nominated_rtp_ = nominated_[connection->local->address].first; nominated_rtcp_ = nominated_[connection->local->address].second; + + nominated_rtp_->state = PAIR_NOMINATED; + nominated_rtcp_->state = PAIR_NOMINATED; + emit endNomination(); } } From 0d6e087f9374d56cbc068494b4f01b3a0f543372 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 16 Apr 2019 09:18:08 +0300 Subject: [PATCH 20/30] Create nominated connection pair before releasing mutex ICE process failed sometimes because the mutex was released too early and the calling code thought the process had failed even though it didn't --- src/ice.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ice.cpp b/src/ice.cpp index 32d47ec2..4a3e5b18 100644 --- a/src/ice.cpp +++ b/src/ice.cpp @@ -359,18 +359,18 @@ void ICE::handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, u void ICE::handleCallerEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID) { + this->handleEndOfNomination(candidateRTP, candidateRTCP, sessionID); + nominationInfo_[sessionID].caller_mtx->unlock(); nominationInfo_[sessionID].controllee->quit(); - - this->handleEndOfNomination(candidateRTP, candidateRTCP, sessionID); } void ICE::handleCalleeEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID) { + this->handleEndOfNomination(candidateRTP, candidateRTCP, sessionID); + nominationInfo_[sessionID].callee_mtx->unlock(); nominationInfo_[sessionID].controller->quit(); - - this->handleEndOfNomination(candidateRTP, candidateRTCP, sessionID); } ICEMediaInfo ICE::getNominated(uint32_t sessionID) From 9beb2a3c1cb14bf587ff588f560400b448783381 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 9 Apr 2019 13:48:56 +0300 Subject: [PATCH 21/30] [ICE] (bug_fix) Use default size for all outgoing STUN message This is done to mitigate a Windows-specific problem where the unique_ptr is deallocated as soon as it's allocated. --- src/stunmsgfact.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/stunmsgfact.cpp b/src/stunmsgfact.cpp index f7e3baf4..dc428079 100644 --- a/src/stunmsgfact.cpp +++ b/src/stunmsgfact.cpp @@ -1,6 +1,15 @@ #include "stunmsgfact.h" #include +// allocate 256 bytes of temporary memory for each outgoing STUN message +// Usually the size of message is less than 100 bytes but just in case add +// some wiggle room. +// +// This has to be done because 64-bit MinGW seems to call unique_ptr's +// *destructor* when allocating space for the message. Allocating buffer large +// enough fixes the problem +const int STUN_MSG_MAX_SIZE = 256; + StunMessageFactory::StunMessageFactory() { qsrand(QDateTime::currentSecsSinceEpoch() / 2 + 1); @@ -135,7 +144,7 @@ QByteArray StunMessageFactory::hostToNetwork(STUNMessage& message) auto attrs = message.getAttributes(); const size_t MSG_SIZE = sizeof(STUNRawMessage) + message.getLength(); - auto ptr = std::unique_ptr{ new unsigned char[MSG_SIZE] }; + auto ptr = std::unique_ptr{ new unsigned char[STUN_MSG_MAX_SIZE] }; STUNRawMessage *rawMessage = static_cast(static_cast(ptr.get())); rawMessage->type = qToBigEndian((short)message.getType()); From f535b26247dae496193e0e3b79fe76cfbb8ed693 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Thu, 18 Apr 2019 10:22:30 +0300 Subject: [PATCH 22/30] [ICE] (refactoring) Switch to 2-way handshanking when performing the nomination It has already been established that the connection works to both ends, so as an optimization we can just send the nomination and wait for response --- src/connectiontester.cpp | 4 +- src/stun.cpp | 197 +++++++++++---------------------------- 2 files changed, 60 insertions(+), 141 deletions(-) diff --git a/src/connectiontester.cpp b/src/connectiontester.cpp index f1c4638a..bd6071a3 100644 --- a/src/connectiontester.cpp +++ b/src/connectiontester.cpp @@ -46,7 +46,9 @@ void ConnectionTester::run() if (!stun_->sendBindingRequest(pair_, controller_)) { - qDebug() << "FAILED!"; + qDebug() << "Connectivity checks failed for" + << pair_->local->address << pair_->local->port + << pair_->remote->address << pair_->remote->port; return; } diff --git a/src/stun.cpp b/src/stun.cpp index c1a7114d..6faae434 100644 --- a/src/stun.cpp +++ b/src/stun.cpp @@ -69,9 +69,9 @@ bool Stun::waitForStunResponse(unsigned long timeout) timer.setSingleShot(true); QEventLoop loop; - connect(this, &Stun::parsingDone, &loop, &QEventLoop::quit); - connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit); - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + connect(this, &Stun::parsingDone, &loop, &QEventLoop::quit, Qt::DirectConnection); + connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit, Qt::DirectConnection); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit, Qt::DirectConnection); timer.start(timeout); loop.exec(); @@ -85,9 +85,9 @@ bool Stun::waitForStunRequest(unsigned long timeout) timer.setSingleShot(true); QEventLoop loop; - connect(this, &Stun::requestRecv, &loop, &QEventLoop::quit); - connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit); - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + connect(this, &Stun::requestRecv, &loop, &QEventLoop::quit, Qt::DirectConnection); + connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit, Qt::DirectConnection); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit, Qt::DirectConnection); timer.start(timeout); loop.exec(); @@ -101,25 +101,9 @@ bool Stun::waitForNominationRequest(unsigned long timeout) timer.setSingleShot(true); QEventLoop loop; - connect(this, &Stun::nominationRecv, &loop, &QEventLoop::quit); - connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit); - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); - - timer.start(timeout); - loop.exec(); - - return timer.isActive(); -} - -bool Stun::waitForNominationResponse(unsigned long timeout) -{ - QTimer timer; - timer.setSingleShot(true); - QEventLoop loop; - - connect(this, &Stun::nominationRecv, &loop, &QEventLoop::quit); - connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit); - connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit); + connect(this, &Stun::nominationRecv, &loop, &QEventLoop::quit, Qt::DirectConnection); + connect(this, &Stun::stopEventLoop, &loop, &QEventLoop::quit, Qt::DirectConnection); + connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit, Qt::DirectConnection); timer.start(timeout); loop.exec(); @@ -189,7 +173,7 @@ bool Stun::controllerSendBindingRequest(ICEPair *pair) message = stunmsg_.hostToNetwork(msg); msgReceived = true; - for (int i = 0; i < 3; ++i) + for (int k = 0; k < 3; ++k) { udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); QThread::msleep(20); @@ -231,7 +215,7 @@ bool Stun::controlleeSendBindingRequest(ICEPair *pair) message = stunmsg_.hostToNetwork(msg); msgReceived = true; - for (int i = 0; i < 3; ++i) + for (int k = 0; k < 3; ++k) { udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); QThread::msleep(20); @@ -313,54 +297,12 @@ bool Stun::sendBindingRequest(ICEPair *pair, bool controller) return controlleeSendBindingRequest(pair); } -// either we got Stun binding request -> send binding response -// or Stun binding response -> mark candidate as valid -void Stun::recvStunMessage(QNetworkDatagram message) -{ - QByteArray data = message.data(); - STUNMessage stunMsg = stunmsg_.networkToHost(data); - - if (stunMsg.getType() == STUN_REQUEST) - { - if (stunmsg_.validateStunRequest(stunMsg)) - { - stunmsg_.cacheRequest(stunMsg); - - if (stunMsg.hasAttribute(STUN_ATTR_USE_CANDIATE)) - { - emit nominationRecv(); - return; - } - - emit requestRecv(); - } - } - else if (stunMsg.getType() == STUN_RESPONSE) - { - if (stunMsg.hasAttribute(STUN_ATTR_USE_CANDIATE)) - { - emit nominationRecv(); - } - // if this message is a response to a request we just sent, its TransactionID should be cached - // if not, vertaa viimeksi lähetetyn TransactionID:tä vasten - else if (stunmsg_.validateStunResponse(stunMsg, message.senderAddress(), message.senderPort())) - { - emit parsingDone(); - } - } - else - { - qDebug() << "Got unknown STUN message, type: " << stunMsg.getType() - << " from " << message.senderAddress() << ":" << message.senderPort() - << " to" << message.destinationAddress() << ":" << message.destinationPort(); - } -} bool Stun::sendNominationRequest(ICEPair *pair) { if (multiplex_ == false) { - qDebug() << "Binding" << pair->local->address << " to port " << pair->local->port; + qDebug() << "[ICE-CONTROLLING] Binding" << pair->local->address << " to port " << pair->local->port; if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) { @@ -380,25 +322,25 @@ bool Stun::sendNominationRequest(ICEPair *pair) QByteArray message = stunmsg_.hostToNetwork(request); - bool ok = false; + bool responseRecv = false; for (int i = 0; i < 25; ++i) { udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); - if (waitForNominationResponse(20 * (i + 1))) + if (waitForStunResponse(20 * (i + 1))) { if (interrupted_) { return false; } - ok = true; + responseRecv = true; break; } } - if (ok == false) + if (responseRecv == false) { qDebug() << "WARNING: Failed to receive Nomination Response from remote!"; qDebug() << "Remote: " << pair->remote->address << ":" << pair->remote->port; @@ -410,48 +352,18 @@ bool Stun::sendNominationRequest(ICEPair *pair) return false; } - // the first part of connectivity check is done, now we must wait for - // remote's binding request and responed to them - ok = false; - - for (int i = 0; i < 20; ++i) - { - if (waitForNominationRequest(20 * (i + 1))) - { - if (interrupted_) - { - return false; - } - - STUNMessage msg = stunmsg_.createResponse(); - msg.addAttribute(STUN_ATTR_ICE_CONTROLLING); - msg.addAttribute(STUN_ATTR_USE_CANDIATE); - - message = stunmsg_.hostToNetwork(msg); - ok = true; - - for (int i = 0; i < 3; ++i) - { - udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); - QThread::msleep(20); - } - - break; - } - } - if (multiplex_ == false) { udp_->unbind(); } - return ok; + return responseRecv; } bool Stun::sendNominationResponse(ICEPair *pair) { if (multiplex_ == false) { - qDebug() << "[controllee] BINDING " << pair->local->address << " TO PORT " << pair->local->port; + qDebug() << "[ICE-CONTROLLED] Binding" << pair->local->address << " TO PORT " << pair->local->port; if (!udp_->bindRaw(QHostAddress(pair->local->address), pair->local->port)) { @@ -479,12 +391,11 @@ bool Stun::sendNominationResponse(ICEPair *pair) msg = stunmsg_.createResponse(); msg.addAttribute(STUN_ATTR_ICE_CONTROLLED); - msg.addAttribute(STUN_ATTR_USE_CANDIATE); message = stunmsg_.hostToNetwork(msg); nominationRecv = true; - for (int i = 0; i < 3; ++i) + for (int i = 0; i < 5; ++i) { udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); QThread::msleep(20); @@ -506,38 +417,6 @@ bool Stun::sendNominationResponse(ICEPair *pair) return false; } - // the first part of connective check is done (remote sending us binding request) - // now we must do the same but this we're sending the request and they're responding - nominationRecv = false; - - // we've succesfully responded to remote's binding request, now it's our turn to - // send request and remote must respond to them - STUNMessage request = stunmsg_.createRequest(); - request.addAttribute(STUN_ATTR_ICE_CONTROLLED); - request.addAttribute(STUN_ATTR_PRIORITY, pair->priority); - request.addAttribute(STUN_ATTR_USE_CANDIATE); - - message = stunmsg_.hostToNetwork(request); - - // we're expecting a response from remote to this request - stunmsg_.cacheRequest(request); - - for (int i = 0; i < 20; ++i) - { - udp_->sendData(message, QHostAddress(pair->remote->address), pair->remote->port, false); - - if (waitForNominationResponse(20 * (i + 1))) - { - if (interrupted_) - { - return false; - } - - nominationRecv = true; - break; - } - } - if (multiplex_ == false) { udp_->unbind(); @@ -584,3 +463,41 @@ void Stun::stopTesting() interrupted_ = true; emit stopEventLoop(); } + +// either we got Stun binding request -> send binding response +// or Stun binding response -> mark candidate as valid +void Stun::recvStunMessage(QNetworkDatagram message) +{ + QByteArray data = message.data(); + STUNMessage stunMsg = stunmsg_.networkToHost(data); + + if (stunMsg.getType() == STUN_REQUEST) + { + if (stunmsg_.validateStunRequest(stunMsg)) + { + stunmsg_.cacheRequest(stunMsg); + + if (stunMsg.hasAttribute(STUN_ATTR_USE_CANDIATE)) + { + emit nominationRecv(); + return; + } + + emit requestRecv(); + } + } + else if (stunMsg.getType() == STUN_RESPONSE) + { + if (stunmsg_.validateStunResponse(stunMsg, message.senderAddress(), message.senderPort())) + { + emit parsingDone(); + } + } + else + { + qDebug() << "Got unknown STUN message, type: " << stunMsg.getType() + << " from " << message.senderAddress() << ":" << message.senderPort() + << " to" << message.destinationAddress() << ":" << message.destinationPort(); + } +} + From 921caece5141c5c538296373bee89c0099b6dc74 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 19 Apr 2019 08:42:18 +0300 Subject: [PATCH 23/30] [ICE] (bug_fix) Lock nomination mutex before updating the connection status --- src/iceflowcontrol.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/iceflowcontrol.cpp b/src/iceflowcontrol.cpp index 2e9aec8b..92095917 100644 --- a/src/iceflowcontrol.cpp +++ b/src/iceflowcontrol.cpp @@ -43,6 +43,8 @@ void FlowAgent::setSessionID(uint32_t sessionID) void FlowAgent::nominationDone(ICEPair *connection) { + nominated_mtx.lock(); + if (connection->local->component == RTP) { nominated_[connection->local->address].first = connection; @@ -55,17 +57,18 @@ void FlowAgent::nominationDone(ICEPair *connection) if (nominated_[connection->local->address].first != nullptr && nominated_[connection->local->address].second != nullptr) { - if (nominated_mtx.try_lock()) - { - nominated_rtp_ = nominated_[connection->local->address].first; - nominated_rtcp_ = nominated_[connection->local->address].second; + nominated_rtp_ = nominated_[connection->local->address].first; + nominated_rtcp_ = nominated_[connection->local->address].second; - nominated_rtp_->state = PAIR_NOMINATED; - nominated_rtcp_->state = PAIR_NOMINATED; + nominated_rtp_->state = PAIR_NOMINATED; + nominated_rtcp_->state = PAIR_NOMINATED; - emit endNomination(); - } + // mutex need not to be unlocked here because this marks the end of nomination + emit endNomination(); + return; } + + nominated_mtx.unlock(); } bool FlowAgent::waitForResponses(unsigned long timeout) From 4dca43bc46a7d2fe0e32be8adc3f7c2a6b759501 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 19 Apr 2019 08:43:17 +0300 Subject: [PATCH 24/30] [ICE] (bug_fix) Destroy UDP server only if it was allocated --- src/iceflowcontrol.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/iceflowcontrol.cpp b/src/iceflowcontrol.cpp index 92095917..aa878ead 100644 --- a/src/iceflowcontrol.cpp +++ b/src/iceflowcontrol.cpp @@ -180,8 +180,11 @@ void FlowController::run() // deallocate all memory consumed by connection buckets for (int i = 0; i < buckets.size(); ++i) { - buckets[i].server->unbind(); - delete buckets[i].server; + if (buckets[i].server) + { + buckets[i].server->unbind(); + delete buckets[i].server; + } for (int k = 0; k < buckets[i].pairs.size(); ++k) { @@ -312,7 +315,11 @@ void FlowControllee::run() // deallocate all memory consumed by connection buckets for (int i = 0; i < buckets.size(); ++i) { - delete buckets[i].server; + if (buckets[i].server) + { + buckets[i].server->unbind(); + delete buckets[i].server; + } for (int k = 0; k < buckets[i].pairs.size(); ++k) { From 02131603db5e2300f877fc03c61baa5ef84de1da Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 19 Apr 2019 09:55:10 +0300 Subject: [PATCH 25/30] [ICE] (refactoring) Start using smart pointers --- src/connectiontester.cpp | 6 +-- src/connectiontester.h | 8 +-- src/globalsdpstate.cpp | 30 +++++------ src/globalsdpstate.h | 6 +-- src/ice.cpp | 110 ++++++++++++++------------------------- src/ice.h | 32 +++++++----- src/iceflowcontrol.cpp | 12 +++-- src/iceflowcontrol.h | 20 ++++--- src/icetypes.h | 17 ++++-- src/sip/sdptypes.h | 3 +- src/sip/sipcontent.cpp | 12 ++--- 11 files changed, 126 insertions(+), 130 deletions(-) diff --git a/src/connectiontester.cpp b/src/connectiontester.cpp index bd6071a3..4517f97b 100644 --- a/src/connectiontester.cpp +++ b/src/connectiontester.cpp @@ -11,7 +11,7 @@ ConnectionTester::~ConnectionTester() { } -void ConnectionTester::setCandidatePair(ICEPair *pair) +void ConnectionTester::setCandidatePair(std::shared_ptr pair) { pair_ = pair; } @@ -44,7 +44,7 @@ void ConnectionTester::run() pair_->state = PAIR_IN_PROGRESS; - if (!stun_->sendBindingRequest(pair_, controller_)) + if (!stun_->sendBindingRequest(pair_.get(), controller_)) { qDebug() << "Connectivity checks failed for" << pair_->local->address << pair_->local->port @@ -61,7 +61,7 @@ void ConnectionTester::run() return; } - if (!stun_->sendNominationResponse(pair_)) + if (!stun_->sendNominationResponse(pair_.get())) { qDebug() << "failed to receive nomination for candidate:\n" << "\tlocal:" << pair_->local->address << ":" << pair_->local->port << "\n" diff --git a/src/connectiontester.h b/src/connectiontester.h index bc4d9ca1..8c43e282 100644 --- a/src/connectiontester.h +++ b/src/connectiontester.h @@ -1,6 +1,8 @@ #pragma once #include +#include + #include "icetypes.h" #include "udpserver.h" #include "stun.h" @@ -13,7 +15,7 @@ class ConnectionTester : public QThread ConnectionTester(); ~ConnectionTester(); void setStun(Stun *stun); - void setCandidatePair(ICEPair *pair); + void setCandidatePair(std::shared_ptr pair); // controller_ defines the course of action after candiate pair has been validated. // If the ConnectionTester belongs to FlowController it terminates immediately after @@ -33,7 +35,7 @@ public slots: // // if the tested candidate succeeded (remote responded to our requests), // connection points to valid ICEPair, otherwise it's nullptr - void testingDone(ICEPair *connection); + void testingDone(std::shared_ptr connection); // send signal to Stun object that it should terminate testing the candidate // (break from the event loop associated with testing) @@ -42,7 +44,7 @@ public slots: protected: void run(); - ICEPair *pair_; + std::shared_ptr pair_; bool controller_; Stun *stun_; }; diff --git a/src/globalsdpstate.cpp b/src/globalsdpstate.cpp index 7fbffa81..7ba58f0c 100644 --- a/src/globalsdpstate.cpp +++ b/src/globalsdpstate.cpp @@ -25,7 +25,7 @@ std::shared_ptr GlobalSDPState::localSDPSuggestion(QHostAddress } std::shared_ptr -GlobalSDPState::generateSDP(QHostAddress localAddress, QList *remoteCandidates) +GlobalSDPState::generateSDP(QHostAddress localAddress, QList> *remoteCandidates) { // TODO: This should ask media manager, what options it supports. qDebug() << "Generating new SDP message with our address as:" << localAddress; @@ -176,16 +176,16 @@ GlobalSDPState::localFinalSDP(SDPMessageInfo &remoteSDP, QHostAddress localAddre ICEMediaInfo nominated = ice_->getNominated(sessionID); // first is RTP, second is RTCP - if (nominated.opus.first != nullptr && nominated.opus.second != nullptr) + if (nominated.audio.first != nullptr && nominated.audio.second != nullptr) { - setMediaPair(sdp->media[0], nominated.opus.first->local); - setMediaPair(remoteSDP.media[0], nominated.opus.first->remote); + setMediaPair(sdp->media[0], nominated.audio.first->local); + setMediaPair(remoteSDP.media[0], nominated.audio.first->remote); } - if (nominated.hevc.first != nullptr && nominated.hevc.second != nullptr) + if (nominated.video.first != nullptr && nominated.video.second != nullptr) { - setMediaPair(sdp->media[1], nominated.hevc.first->local); - setMediaPair(remoteSDP.media[1], nominated.hevc.first->remote); + setMediaPair(sdp->media[1], nominated.video.first->local); + setMediaPair(remoteSDP.media[1], nominated.video.first->remote); } } @@ -259,12 +259,12 @@ void GlobalSDPState::endSession(std::shared_ptr sessionSDP) } } -void GlobalSDPState::startICECandidateNegotiation(QList& local, QList& remote, uint32_t sessionID) +void GlobalSDPState::startICECandidateNegotiation(QList>& local, QList>& remote, uint32_t sessionID) { ice_->startNomination(local, remote, sessionID); } -void GlobalSDPState::setMediaPair(MediaInfo& media, ICEInfo *mediaInfo) +void GlobalSDPState::setMediaPair(MediaInfo& media, std::shared_ptr mediaInfo) { if (mediaInfo == nullptr) { @@ -280,15 +280,15 @@ void GlobalSDPState::updateFinalSDPs(SDPMessageInfo& localSDP, SDPMessageInfo& r ICEMediaInfo nominated = ice_->getNominated(sessionID); // first is RTP, second is RTCP - if (nominated.opus.first != nullptr && nominated.opus.second != nullptr) + if (nominated.audio.first != nullptr && nominated.audio.second != nullptr) { - setMediaPair(localSDP.media[0], nominated.opus.first->local); - setMediaPair(remoteSDP.media[0], nominated.opus.first->remote); + setMediaPair(localSDP.media[0], nominated.audio.first->local); + setMediaPair(remoteSDP.media[0], nominated.audio.first->remote); } - if (nominated.hevc.first != nullptr && nominated.hevc.second != nullptr) + if (nominated.video.first != nullptr && nominated.video.second != nullptr) { - setMediaPair(localSDP.media[1], nominated.hevc.first->local); - setMediaPair(remoteSDP.media[1], nominated.hevc.first->remote); + setMediaPair(localSDP.media[1], nominated.video.first->local); + setMediaPair(remoteSDP.media[1], nominated.video.first->remote); } } diff --git a/src/globalsdpstate.h b/src/globalsdpstate.h index 20439f6e..ccdb794c 100644 --- a/src/globalsdpstate.h +++ b/src/globalsdpstate.h @@ -36,7 +36,7 @@ class GlobalSDPState // frees the ports when they are not needed in rest of the program void endSession(std::shared_ptr sessionSDP); - void startICECandidateNegotiation(QList& local, QList& remote, uint32_t sessionID); + void startICECandidateNegotiation(QList>& local, QList>& remote, uint32_t sessionID); // update the MediaInfo of remote and locals SDPs to include the nominated connections void updateFinalSDPs(SDPMessageInfo& localSDP, SDPMessageInfo& remoteSDP, uint32_t sessionID); @@ -49,7 +49,7 @@ class GlobalSDPState private: // TODO: This should be moved to MediaManager. - std::shared_ptr generateSDP(QHostAddress localAddress, QList *remoteCandidates); + std::shared_ptr generateSDP(QHostAddress localAddress, QList> *remoteCandidates); bool generateAudioMedia(MediaInfo &audio); bool generateVideoMedia(MediaInfo &video); @@ -57,7 +57,7 @@ class GlobalSDPState bool checkSDPOffer(SDPMessageInfo& offer); // update MediaInfo of SDP after ICE has finished - void setMediaPair(MediaInfo& media, ICEInfo *mediaInfo); + void setMediaPair(MediaInfo& media, std::shared_ptr mediaInfo); QString localUsername_; diff --git a/src/ice.cpp b/src/ice.cpp index 4a3e5b18..bb8d60f8 100644 --- a/src/ice.cpp +++ b/src/ice.cpp @@ -23,7 +23,7 @@ ICE::ICE(): QSettings settings("kvazzup.ini", QSettings::IniFormat); int ice = settings.value("sip/ice").toInt(); - iceDisabled_ = (ice == 1) ? true : false; + iceDisabled_ = (ice != 1) ? true : false; } ICE::~ICE() @@ -58,13 +58,13 @@ QString ICE::generateFoundation() return randomString; } -QList ICE::generateICECandidates() +QList> ICE::generateICECandidates() { QTime time = QTime::currentTime(); qsrand((uint)time.msec()); - QList candidates; - std::pair candidate; + QList> candidates; + std::pair, std::shared_ptr> candidate; foreach (const QHostAddress& address, QNetworkInterface::allAddresses()) { @@ -98,10 +98,10 @@ QList ICE::generateICECandidates() return candidates; } -std::pair ICE::makeCandidate(QHostAddress address, QString type) +std::pair, std::shared_ptr> ICE::makeCandidate(QHostAddress address, QString type) { - ICEInfo *entry_rtp = new ICEInfo; - ICEInfo *entry_rtcp = new ICEInfo; + std::shared_ptr entry_rtp = std::make_shared(); + std::shared_ptr entry_rtcp = std::make_shared(); entry_rtp->address = address.toString(); entry_rtcp->address = address.toString(); @@ -154,9 +154,9 @@ void ICE::printCandidate(ICEInfo *candidate) << candidate->address << ":" << candidate->port; } -QList *ICE::makeCandiatePairs(QList& local, QList& remote) +QList> ICE::makeCandidatePairs(QList>& local, QList>& remote) { - QList *pairs = new QList; + QList> pairs; // match all host candidates with remote (remote does the same) for (int i = 0; i < local.size(); ++i) @@ -167,37 +167,28 @@ QList *ICE::makeCandiatePairs(QList& local, QListtype == remote[k]->type && local[i]->component == remote[k]->component) { - ICEPair *pair = new ICEPair; + std::shared_ptr pair = std::make_shared(); pair->local = local[i]; pair->remote = remote[k]; pair->priority = qMin(local[i]->priority, remote[k]->priority); // TODO spec pair->state = PAIR_FROZEN; - pairs->push_back(pair); + pairs.push_back(pair); } } } -#if 0 - for (int i = 0; i < pairs->size(); ++i) - { - qDebug() << pairs->at(i)->local->type << ":" << pairs->at(i)->local->address << ":" << pairs->at(i)->local->port - << pairs->at(i)->remote->address << ":" << pairs->at(i)->remote->port; - } -#endif - return pairs; } - // callee (flow controller) // // this function spawns a control thread and exist right away so the 200 OK // response can be set as fast as possible and the remote can start respoding to our requests // // Thread spawned by startNomination() must keep track of which candidates failed and which succeeded -void ICE::startNomination(QList& local, QList& remote, uint32_t sessionID) +void ICE::startNomination(QList>& local, QList>& remote, uint32_t sessionID) { if (iceDisabled_ == true) { @@ -210,13 +201,13 @@ void ICE::startNomination(QList& local, QList& remote, uin nominationInfo_[sessionID].callee_mtx = new QMutex; nominationInfo_[sessionID].callee_mtx->lock(); - nominationInfo_[sessionID].pairs = makeCandiatePairs(local, remote); + nominationInfo_[sessionID].pairs = makeCandidatePairs(local, remote); nominationInfo_[sessionID].connectionNominated = false; FlowController *callee = nominationInfo_[sessionID].controller; QObject::connect(callee, &FlowController::ready, this, &ICE::handleCalleeEndOfNomination, Qt::DirectConnection); - callee->setCandidates(nominationInfo_[sessionID].pairs); + callee->setCandidates(&nominationInfo_[sessionID].pairs); callee->setSessionID(sessionID); callee->start(); } @@ -226,7 +217,7 @@ void ICE::startNomination(QList& local, QList& remote, uin // respondToNominations() spawns a control thread that starts testing all candidates // It doesn't do any external book keeping as it's responsible for only responding to STUN requets // When it has gone through all candidate pairs it exits -void ICE::respondToNominations(QList& local, QList& remote, uint32_t sessionID) +void ICE::respondToNominations(QList>& local, QList>& remote, uint32_t sessionID) { if (iceDisabled_ == true) { @@ -239,14 +230,13 @@ void ICE::respondToNominations(QList& local, QList& remote nominationInfo_[sessionID].caller_mtx = new QMutex; nominationInfo_[sessionID].caller_mtx->lock(); - nominationInfo_[sessionID].pairs = makeCandiatePairs(local, remote); + nominationInfo_[sessionID].pairs = makeCandidatePairs(local, remote); nominationInfo_[sessionID].connectionNominated = false; FlowControllee *caller = nominationInfo_[sessionID].controllee; QObject::connect(caller, &FlowControllee::ready, this, &ICE::handleCallerEndOfNomination, Qt::DirectConnection); - /* caller->setHevcCandidates( */ - caller->setCandidates(nominationInfo_[sessionID].pairs); + caller->setCandidates(&nominationInfo_[sessionID].pairs); caller->setSessionID(sessionID); caller->start(); } @@ -287,7 +277,7 @@ bool ICE::calleeConnectionNominated(uint32_t sessionID) return nominationInfo_[sessionID].connectionNominated; } -void ICE::handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID) +void ICE::handleEndOfNomination(std::shared_ptr rtp, std::shared_ptr rtcp, uint32_t sessionID) { // nothing needs to be cleaned if ICE was disabled if (iceDisabled_ == true) @@ -295,7 +285,7 @@ void ICE::handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, u return; } - if (candidateRTP == nullptr || candidateRTCP == nullptr) + if (rtp == nullptr || rtcp == nullptr) { qDebug() << "ERROR: Nomination failed! Unable to start call."; nominationInfo_[sessionID].connectionNominated = false; @@ -303,71 +293,49 @@ void ICE::handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, u else { nominationInfo_[sessionID].connectionNominated = true; - nominationInfo_[sessionID].nominatedHEVC = std::make_pair((ICEPair *)candidateRTP, (ICEPair *)candidateRTCP); + nominationInfo_[sessionID].nominatedVideo = std::make_pair(rtp, rtcp); - // Create opus candidate on the fly. When this candidate (candidateRTP && candidateRTCP) was created + // Create opus candidate on the fly. When this candidate (rtp && rtcp) was created // we intentionally allocated 4 ports instead of 2 for use. // // This is because we don't actually need to test that both HEVC and Opus work separately. Insted we can just // test HEVC and if that works we can assume that Opus works too (no reason why it shouldn't) - ICEPair *opusPairRTP = new ICEPair; - opusPairRTP->local = new ICEInfo; - opusPairRTP->remote = new ICEInfo; + std::shared_ptr opusPairRTP = std::make_shared(); + std::shared_ptr opusPairRTCP = std::make_shared(); - ICEPair *opusPairRTCP = new ICEPair; - opusPairRTCP->local = new ICEInfo; - opusPairRTCP->remote = new ICEInfo; + opusPairRTP->local = std::make_shared(); + opusPairRTP->remote = std::make_shared(); - memcpy(opusPairRTP->local, candidateRTP->local, sizeof(ICEInfo)); - memcpy(opusPairRTP->remote, candidateRTP->remote, sizeof(ICEInfo)); + memcpy(opusPairRTP->local.get(), rtp->local.get(), sizeof(ICEInfo)); + memcpy(opusPairRTP->remote.get(), rtp->remote.get(), sizeof(ICEInfo)); - memcpy(opusPairRTCP->local, candidateRTCP->local, sizeof(ICEInfo)); - memcpy(opusPairRTCP->remote, candidateRTCP->remote, sizeof(ICEInfo)); + opusPairRTCP->local = std::make_shared(); + opusPairRTCP->remote = std::make_shared(); + + memcpy(opusPairRTCP->local.get(), rtcp->local.get(), sizeof(ICEInfo)); + memcpy(opusPairRTCP->remote.get(), rtcp->remote.get(), sizeof(ICEInfo)); opusPairRTP->local->port += 2; // hevc rtp, hevc rtcp and then opus rtp opusPairRTCP->local->port += 2; // hevc rtp, hevc, rtcp, opus rtp and then opus rtcp - // same for remote candidate opusPairRTP->remote->port += 2; // hevc rtp, hevc rtcp and then opus rtp opusPairRTCP->remote->port += 2; // hev rtp, hevc, rtcp, opus rtp and then opus rtcp - nominationInfo_[sessionID].nominatedOpus = std::make_pair((ICEPair *)opusPairRTP, (ICEPair *)opusPairRTCP); - } - - foreach (ICEPair *pair, *nominationInfo_[sessionID].pairs) - { - if (pair->state != PAIR_NOMINATED) - { - // pair->local cannot be freed here because we send final SDP - // to remote which contains our candidate offers but we can release - // pair->local->port (unless local is stun_entry_rtp_ or stun_entry_rtcp_) - // - // because ports are allocated in pairs (RTP is port X and RTCP is port X + 1), - // we need to call makePortPairAvailable() only for RTP candidate - if (pair->local->component == RTP) - { - parameters_.deallocateMediaPorts(pair->local->port); - } - - delete pair->remote; - delete pair; - } + nominationInfo_[sessionID].nominatedAudio = std::make_pair(opusPairRTP, opusPairRTCP); } - - delete nominationInfo_[sessionID].pairs; } -void ICE::handleCallerEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID) +void ICE::handleCallerEndOfNomination(std::shared_ptr rtp, std::shared_ptr rtcp, uint32_t sessionID) { - this->handleEndOfNomination(candidateRTP, candidateRTCP, sessionID); + this->handleEndOfNomination(rtp, rtcp, sessionID); nominationInfo_[sessionID].caller_mtx->unlock(); nominationInfo_[sessionID].controllee->quit(); } -void ICE::handleCalleeEndOfNomination(struct ICEPair *candidateRTP, struct ICEPair *candidateRTCP, uint32_t sessionID) +void ICE::handleCalleeEndOfNomination(std::shared_ptr rtp, std::shared_ptr rtcp, uint32_t sessionID) { - this->handleEndOfNomination(candidateRTP, candidateRTCP, sessionID); + this->handleEndOfNomination(rtp, rtcp, sessionID); nominationInfo_[sessionID].callee_mtx->unlock(); nominationInfo_[sessionID].controller->quit(); @@ -378,8 +346,8 @@ ICEMediaInfo ICE::getNominated(uint32_t sessionID) if (nominationInfo_.contains(sessionID) && iceDisabled_ == false) { return { - nominationInfo_[sessionID].nominatedHEVC, - nominationInfo_[sessionID].nominatedOpus + nominationInfo_[sessionID].nominatedVideo, + nominationInfo_[sessionID].nominatedAudio }; } diff --git a/src/ice.h b/src/ice.h index d2af6bab..b31bfe0c 100644 --- a/src/ice.h +++ b/src/ice.h @@ -23,11 +23,17 @@ struct nominationInfo FlowController *controller; // list of all candidates, remote and local - // all but nominatedPair are freed in handleEndOfNomination() - QList *pairs; + QList> pairs; - std::pair nominatedHEVC; - std::pair nominatedOpus; + std::pair< + std::shared_ptr, + std::shared_ptr + > nominatedVideo; + + std::pair< + std::shared_ptr, + std::shared_ptr + > nominatedAudio; bool connectionNominated; }; @@ -41,13 +47,13 @@ class ICE : public QObject ~ICE(); // generate a list of local candidates for media streaming - QList generateICECandidates(); + QList> generateICECandidates(); // TODO - void startNomination(QList& local, QList& remote, uint32_t sessionID); + void startNomination(QList>& local, QList>& remote, uint32_t sessionID); // TODO - void respondToNominations(QList& local, QList& remote, uint32_t sessionID); + void respondToNominations(QList>& local, QList>& remote, uint32_t sessionID); // get nominated ICE pair using sessionID /* std::pair getNominated(uint32_t sessionID); */ @@ -65,25 +71,27 @@ class ICE : public QObject // when FlowControllee has finished its job, it emits "ready" signal which is caught by this slot function // handleCallerEndOfNomination() check if the nomination succeeed, saves the nominated pair to hashmap and // releases caller_mtx to signal that negotiation is done - void handleCallerEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID); + void handleCallerEndOfNomination(std::shared_ptr rtp, std::shared_ptr rtcp, uint32_t sessionID); // when FlowController has finished its job, it emits "ready" signal which is caught by this slot function // handleCalleeEndOfNomination() check if the nomination succeeed, saves the nominated pair to hashmap and // releases callee_mtx to signal that negotiation is done - void handleCalleeEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID); + void handleCalleeEndOfNomination(std::shared_ptr rtp, std::shared_ptr rtcp, uint32_t sessionID); + void createSTUNCandidate(QHostAddress address); private: // create media candidate (RTP and RTCP connection) // "type" marks whether this candidate is host or server reflexive candidate (affects priority) - std::pair makeCandidate(QHostAddress address, QString type); + std::pair, std::shared_ptr> makeCandidate(QHostAddress address, QString type); - void handleEndOfNomination(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID); + void handleEndOfNomination(std::shared_ptr rtp, std::shared_ptr rtcp, uint32_t sessionID); int calculatePriority(int type, int local, int component); QString generateFoundation(); void printCandidate(ICEInfo *candidate); - QList *makeCandiatePairs(QList& local, QList& remote); + + QList> makeCandidatePairs(QList>& local, QList>& remote); bool nominatingConnection_; bool iceDisabled_; diff --git a/src/iceflowcontrol.cpp b/src/iceflowcontrol.cpp index aa878ead..5b0cb01f 100644 --- a/src/iceflowcontrol.cpp +++ b/src/iceflowcontrol.cpp @@ -9,7 +9,8 @@ struct Pair { Stun *stun; - ICEPair *pair; + /* ICEPair *pair; */ + std::shared_ptr pair; }; // TODO explain @@ -31,7 +32,7 @@ FlowAgent::~FlowAgent() void FlowAgent::run() { } -void FlowAgent::setCandidates(QList *candidates) +void FlowAgent::setCandidates(QList> *candidates) { candidates_ = candidates; } @@ -41,7 +42,7 @@ void FlowAgent::setSessionID(uint32_t sessionID) sessionID_ = sessionID; } -void FlowAgent::nominationDone(ICEPair *connection) +void FlowAgent::nominationDone(std::shared_ptr connection) { nominated_mtx.lock(); @@ -200,14 +201,14 @@ void FlowController::run() return; } - if (!stun.sendNominationRequest(nominated_rtp_)) + if (!stun.sendNominationRequest(nominated_rtp_.get())) { qDebug() << "Failed to nominate RTP candidate!"; emit ready(nullptr, nullptr, sessionID_); return; } - if (!stun.sendNominationRequest(nominated_rtcp_)) + if (!stun.sendNominationRequest(nominated_rtcp_.get())) { qDebug() << "Failed to nominate RTCP candidate!"; emit ready(nominated_rtp_, nullptr, sessionID_); @@ -296,6 +297,7 @@ void FlowControllee::run() connect(workerThreads.back().get(), &ConnectionTester::testingDone, this, &FlowAgent::nominationDone, Qt::DirectConnection); + buckets[i].pairs[k].stun->moveToThread(workerThreads.back().get()); workerThreads.back()->setCandidatePair(buckets[i].pairs[k].pair); workerThreads.back()->setStun(buckets[i].pairs[k].stun); workerThreads.back()->isController(false); diff --git a/src/iceflowcontrol.h b/src/iceflowcontrol.h index 345d0a9c..aa39139e 100644 --- a/src/iceflowcontrol.h +++ b/src/iceflowcontrol.h @@ -4,6 +4,7 @@ #include #include #include +#include #include "stun.h" #include "icetypes.h" @@ -15,21 +16,21 @@ class FlowAgent : public QThread public: FlowAgent(); ~FlowAgent(); - void setCandidates(QList *candidates); + void setCandidates(QList> *candidates); void setSessionID(uint32_t sessionID); bool waitForResponses(unsigned long timeout); signals: - void ready(ICEPair *candidateRTP, ICEPair *candidateRTCP, uint32_t sessionID); + void ready(std::shared_ptr candidateRTP, std::shared_ptr candidateRTCP, uint32_t sessionID); void endNomination(); public slots: - void nominationDone(ICEPair *connection); + void nominationDone(std::shared_ptr connection); protected: void run(); - QList *candidates_; + QList> *candidates_; uint32_t sessionID_; // temporary storage for succeeded pairs, when both RTP and RTCP @@ -37,10 +38,15 @@ public slots: // and the succeeded pair is copied to nominated_rtp_ and nominated_rtcp_ // // the first candidate pair that has both RTP and RTCP tested is chosen - QMap> nominated_; + QMap, + std::shared_ptr + > + > nominated_; - ICEPair *nominated_rtp_; - ICEPair *nominated_rtcp_; + std::shared_ptr nominated_rtp_; + std::shared_ptr nominated_rtcp_; QMutex nominated_mtx; }; diff --git a/src/icetypes.h b/src/icetypes.h index df632c99..98ab5849 100644 --- a/src/icetypes.h +++ b/src/icetypes.h @@ -1,5 +1,7 @@ #pragma once +#include + enum PAIR { PAIR_WAITING = 0, PAIR_IN_PROGRESS = 1, @@ -31,8 +33,8 @@ struct ICEInfo struct ICEPair { - struct ICEInfo *local; - struct ICEInfo *remote; + std::shared_ptr local; + std::shared_ptr remote; int priority; int state; }; @@ -41,6 +43,13 @@ struct ICEMediaInfo { // first ICEPair is for RTP, second for RTCP // ICEPair contains both local and remote address/port pairs (see above) - std::pair hevc; - std::pair opus; + std::pair< + std::shared_ptr, + std::shared_ptr + > video; + + std::pair< + std::shared_ptr, + std::shared_ptr + > audio; }; diff --git a/src/sip/sdptypes.h b/src/sip/sdptypes.h index 676b180a..e8c35520 100644 --- a/src/sip/sdptypes.h +++ b/src/sip/sdptypes.h @@ -2,6 +2,7 @@ #include #include +#include #include #include "icetypes.h" @@ -115,7 +116,7 @@ struct SDPMessageInfo QList valueAttributes; QList media;// m=, zero or more - QList candidates; + QList> candidates; }; Q_DECLARE_METATYPE(SDPMessageInfo); // used in qvariant for content diff --git a/src/sip/sipcontent.cpp b/src/sip/sipcontent.cpp index e32834b7..36663f3b 100644 --- a/src/sip/sipcontent.cpp +++ b/src/sip/sipcontent.cpp @@ -31,12 +31,12 @@ bool parseEncryptionKey(QStringListIterator& lineIterator, char& type, QStringLi // a= bool parseAttributes(QStringListIterator &lineIterator, char &type, QStringList& words, QList& flags, QList& values, - QList& codecs, QList& candidates); + QList& codecs, QList>& candidates); void parseFlagAttribute(SDPAttributeType type, QRegularExpressionMatch& match, QList& attributes); void parseValueAttribute(SDPAttributeType type, QRegularExpressionMatch& match, QList valueAttributes); void parseRTPMap(QRegularExpressionMatch& match, QString secondWord, QList& codecs); -bool parseICECandidate(QStringList& words, QList& candidates); +bool parseICECandidate(QStringList& words, QList>& candidates); bool checkSDPValidity(const SDPMessageInfo &sdpInfo) { @@ -172,7 +172,7 @@ QString composeSDPContent(const SDPMessageInfo &sdpInfo) } } - for (ICEInfo *info : sdpInfo.candidates) + for (auto info : sdpInfo.candidates) { sdp += "a=candidate:" + info->foundation + " " + QString::number(info->component) + " " @@ -538,7 +538,7 @@ bool parseSDPContent(const QString& content, SDPMessageInfo &sdp) bool parseAttributes(QStringListIterator &lineIterator, char &type, QStringList& words, QList& flags, QList& values, - QList& codecs, QList& candidates) + QList& codecs, QList>& candidates) { while(type == 'a') { @@ -819,14 +819,14 @@ bool parseEncryptionKey(QStringListIterator& lineIterator, char& type, QStringLi return true; } -bool parseICECandidate(QStringList& words, QList& candidates) +bool parseICECandidate(QStringList& words, QList>& candidates) { if (words.size() != 8) { return false; } - ICEInfo *candidate = new ICEInfo; + std::shared_ptr candidate = std::make_shared(); candidate->foundation = words.at(0).split(":").at(1); candidate->component = words.at(1).toInt(); From bfdd527920c61c1dd51ff1dcab0d03c9442c430a Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 19 Apr 2019 10:01:02 +0300 Subject: [PATCH 26/30] [ICE] (new_feature) Free all ICE-related resources after the call has ended --- src/globalsdpstate.cpp | 5 +++++ src/globalsdpstate.h | 2 ++ src/ice.cpp | 17 +++++++++++++++++ src/ice.h | 3 +++ src/sip/siptransactions.cpp | 2 ++ 5 files changed, 29 insertions(+) diff --git a/src/globalsdpstate.cpp b/src/globalsdpstate.cpp index 7ba58f0c..7e6c0ab9 100644 --- a/src/globalsdpstate.cpp +++ b/src/globalsdpstate.cpp @@ -292,3 +292,8 @@ void GlobalSDPState::updateFinalSDPs(SDPMessageInfo& localSDP, SDPMessageInfo& r setMediaPair(remoteSDP.media[1], nominated.video.first->remote); } } + +void GlobalSDPState::ICECleanup(uint32_t sessionID) +{ + ice_->cleanupSession(sessionID); +} diff --git a/src/globalsdpstate.h b/src/globalsdpstate.h index ccdb794c..2835ab58 100644 --- a/src/globalsdpstate.h +++ b/src/globalsdpstate.h @@ -38,6 +38,8 @@ class GlobalSDPState void startICECandidateNegotiation(QList>& local, QList>& remote, uint32_t sessionID); + void ICECleanup(uint32_t sessionID); + // update the MediaInfo of remote and locals SDPs to include the nominated connections void updateFinalSDPs(SDPMessageInfo& localSDP, SDPMessageInfo& remoteSDP, uint32_t sessionID); diff --git a/src/ice.cpp b/src/ice.cpp index bb8d60f8..6d4bc3c3 100644 --- a/src/ice.cpp +++ b/src/ice.cpp @@ -356,3 +356,20 @@ ICEMediaInfo ICE::getNominated(uint32_t sessionID) std::make_pair(nullptr, nullptr) }; } + +void ICE::cleanupSession(uint32_t sessionID) +{ + if (nominationInfo_.contains(sessionID) && iceDisabled_ == false) + { + for (int i = 0; i < nominationInfo_[sessionID].pairs.size(); ++i) + { + if (nominationInfo_[sessionID].pairs.at(i)->local && + nominationInfo_[sessionID].pairs.at(i)->local->component == RTP) + { + parameters_.deallocateMediaPorts(nominationInfo_[sessionID].pairs.at(i)->local->port); + } + } + + nominationInfo_.remove(sessionID); + } +} diff --git a/src/ice.h b/src/ice.h index b31bfe0c..5649d35f 100644 --- a/src/ice.h +++ b/src/ice.h @@ -67,6 +67,9 @@ class ICE : public QObject // sessionID must given so ICE can know which ongoing nomination should be checked bool calleeConnectionNominated(uint32_t sessionID); + // free all ICE-related resources + void cleanupSession(uint32_t sessionID); + public slots: // when FlowControllee has finished its job, it emits "ready" signal which is caught by this slot function // handleCallerEndOfNomination() check if the nomination succeeed, saves the nominated pair to hashmap and diff --git a/src/sip/siptransactions.cpp b/src/sip/siptransactions.cpp index 2665a90d..276fdddd 100644 --- a/src/sip/siptransactions.cpp +++ b/src/sip/siptransactions.cpp @@ -264,6 +264,7 @@ void SIPTransactions::endCall(uint32_t sessionID) { std::shared_ptr dialog = dialogs_.at(sessionID - 1); dialog->client->endCall(); + sdp_.ICECleanup(sessionID); destroyDialog(sessionID); } @@ -289,6 +290,7 @@ void SIPTransactions::endAllCalls() { if(dialogs_.at(i) != nullptr) { + sdp_.ICECleanup(i + 1); destroyDialog(i + 1); } } From 5b34d06ae1b9fd9dcdec1267255e5f3642b14d4b Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Fri, 19 Apr 2019 10:09:31 +0300 Subject: [PATCH 27/30] [ICE] (refactoring) Rename iceDisabled to iceEnabled --- src/ice.cpp | 24 +++++++++++++++--------- src/ice.h | 2 +- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/ice.cpp b/src/ice.cpp index 6d4bc3c3..4a07522e 100644 --- a/src/ice.cpp +++ b/src/ice.cpp @@ -21,9 +21,15 @@ ICE::ICE(): stun_.wantAddress("stun.l.google.com"); QSettings settings("kvazzup.ini", QSettings::IniFormat); - int ice = settings.value("sip/ice").toInt(); - iceDisabled_ = (ice != 1) ? true : false; + if (settings.value("sip/ice").toInt() == 1) + { + iceEnabled_ = true; + } + else + { + iceEnabled_ = false; + } } ICE::~ICE() @@ -190,7 +196,7 @@ QList> ICE::makeCandidatePairs(QList>& local, QList>& remote, uint32_t sessionID) { - if (iceDisabled_ == true) + if (iceEnabled_ == false) { return; } @@ -219,7 +225,7 @@ void ICE::startNomination(QList>& local, QList>& local, QList>& remote, uint32_t sessionID) { - if (iceDisabled_ == true) + if (iceEnabled_ == false) { return; } @@ -244,7 +250,7 @@ void ICE::respondToNominations(QList>& local, QList rtp, std::shared_ptr rtcp, uint32_t sessionID) { // nothing needs to be cleaned if ICE was disabled - if (iceDisabled_ == true) + if (iceEnabled_ == false) { return; } @@ -343,7 +349,7 @@ void ICE::handleCalleeEndOfNomination(std::shared_ptr rtp, std::shared_ ICEMediaInfo ICE::getNominated(uint32_t sessionID) { - if (nominationInfo_.contains(sessionID) && iceDisabled_ == false) + if (nominationInfo_.contains(sessionID) && iceEnabled_ == true) { return { nominationInfo_[sessionID].nominatedVideo, @@ -359,7 +365,7 @@ ICEMediaInfo ICE::getNominated(uint32_t sessionID) void ICE::cleanupSession(uint32_t sessionID) { - if (nominationInfo_.contains(sessionID) && iceDisabled_ == false) + if (nominationInfo_.contains(sessionID) && iceEnabled_ == true) { for (int i = 0; i < nominationInfo_[sessionID].pairs.size(); ++i) { diff --git a/src/ice.h b/src/ice.h index 5649d35f..73fdd829 100644 --- a/src/ice.h +++ b/src/ice.h @@ -97,7 +97,7 @@ class ICE : public QObject QList> makeCandidatePairs(QList>& local, QList>& remote); bool nominatingConnection_; - bool iceDisabled_; + bool iceEnabled_; Stun stun_; QHostAddress stunAddress_; From 2fc162f7848aa856e238c1cbfbeb9a748e67122d Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 23 Apr 2019 09:23:40 +0300 Subject: [PATCH 28/30] [ICE] (refactoring) Remove unused parameter from generateSDP --- src/globalsdpstate.cpp | 7 +++---- src/globalsdpstate.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/globalsdpstate.cpp b/src/globalsdpstate.cpp index 7e6c0ab9..41b78d7e 100644 --- a/src/globalsdpstate.cpp +++ b/src/globalsdpstate.cpp @@ -21,11 +21,10 @@ void GlobalSDPState::setLocalInfo(QString username) std::shared_ptr GlobalSDPState::localSDPSuggestion(QHostAddress localAddress) { qDebug() << "Getting local SDP suggestion"; - return generateSDP(localAddress, nullptr); + return generateSDP(localAddress); } -std::shared_ptr -GlobalSDPState::generateSDP(QHostAddress localAddress, QList> *remoteCandidates) +std::shared_ptr GlobalSDPState::generateSDP(QHostAddress localAddress) { // TODO: This should ask media manager, what options it supports. qDebug() << "Generating new SDP message with our address as:" << localAddress; @@ -149,7 +148,7 @@ GlobalSDPState::localFinalSDP(SDPMessageInfo &remoteSDP, QHostAddress localAddre // if we have not made a suggestion, then base our final SDP on their suggestion. if(localSuggestion == nullptr) { - sdp = generateSDP(localAddress, &remoteSDP.candidates); + sdp = generateSDP(localAddress); sdp->sessionName = remoteSDP.sessionName; qDebug() << "\n\n\nHELLO HERE AGAIN!!\n\n\n"; diff --git a/src/globalsdpstate.h b/src/globalsdpstate.h index 2835ab58..02ee48ce 100644 --- a/src/globalsdpstate.h +++ b/src/globalsdpstate.h @@ -51,7 +51,7 @@ class GlobalSDPState private: // TODO: This should be moved to MediaManager. - std::shared_ptr generateSDP(QHostAddress localAddress, QList> *remoteCandidates); + std::shared_ptr generateSDP(QHostAddress localAddress); bool generateAudioMedia(MediaInfo &audio); bool generateVideoMedia(MediaInfo &video); From 96c87417357b0423889efb46d32db88bc2c47849 Mon Sep 17 00:00:00 2001 From: Aaro Altonen Date: Tue, 23 Apr 2019 09:26:06 +0300 Subject: [PATCH 29/30] [ICE] (refactoring) Add comments --- src/connectiontester.cpp | 2 ++ src/connectiontester.h | 5 +++-- src/gui/customsettings.cpp | 2 +- src/ice.cpp | 16 +++++++--------- src/ice.h | 16 +++++++++++++--- src/iceflowcontrol.cpp | 6 +++--- src/iceflowcontrol.h | 13 ++++++++++++- src/stun.h | 18 ++++++++++++++++-- 8 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/connectiontester.cpp b/src/connectiontester.cpp index 4517f97b..8223f081 100644 --- a/src/connectiontester.cpp +++ b/src/connectiontester.cpp @@ -54,6 +54,7 @@ void ConnectionTester::run() pair_->state = PAIR_SUCCEEDED; + // controller performs the nomination process in FlowController so exit from ConnectionTester when this connection has been tested... if (controller_) { qDebug() << "pair success" << pair_->local->address << pair_->local->port << pair_->remote->address << pair_->remote->port << pair_->local->component; @@ -61,6 +62,7 @@ void ConnectionTester::run() return; } + //... otherwise start waitin for nomination requests if (!stun_->sendNominationResponse(pair_.get())) { qDebug() << "failed to receive nomination for candidate:\n" diff --git a/src/connectiontester.h b/src/connectiontester.h index 8c43e282..573e5bfb 100644 --- a/src/connectiontester.h +++ b/src/connectiontester.h @@ -25,9 +25,9 @@ class ConnectionTester : public QThread // requests after it has concluded the candidate verification void isController(bool controller); - void printMessage(QString message); - public slots: + // Because the Stun object used by ConnectionTester has it's own event loop, we must + // override the default quit function, call Stun::stopTesting() and then exit from ConnectionTester void quit(); signals: @@ -42,6 +42,7 @@ public slots: void stopTesting(); protected: + void printMessage(QString message); void run(); std::shared_ptr pair_; diff --git a/src/gui/customsettings.cpp b/src/gui/customsettings.cpp index 7dfbbe11..b8e902c0 100644 --- a/src/gui/customsettings.cpp +++ b/src/gui/customsettings.cpp @@ -1,6 +1,6 @@ #include "customsettings.h" -#include "ui_customsettings.h" +#include "ui_customSettings.h" #include