diff --git a/include/rtc/peerconnection.hpp b/include/rtc/peerconnection.hpp index b0af009..5f0c521 100644 --- a/include/rtc/peerconnection.hpp +++ b/include/rtc/peerconnection.hpp @@ -168,6 +168,7 @@ private: std::atomic mState; std::atomic mGatheringState; + std::atomic mNegociationNeeded; synchronized_callback> mDataChannelCallback; synchronized_callback mLocalDescriptionCallback; diff --git a/src/peerconnection.cpp b/src/peerconnection.cpp index 7707a05..a59251a 100644 --- a/src/peerconnection.cpp +++ b/src/peerconnection.cpp @@ -44,7 +44,7 @@ PeerConnection::PeerConnection() : PeerConnection(Configuration()) {} PeerConnection::PeerConnection(const Configuration &config) : mConfig(config), mCertificate(make_certificate()), mProcessor(std::make_unique()), - mState(State::New), mGatheringState(GatheringState::New) { + mState(State::New), mGatheringState(GatheringState::New), mNegociationNeeded(false) { PLOG_VERBOSE << "Creating PeerConnection"; if (config.portRangeEnd && config.portRangeBegin > config.portRangeEnd) @@ -60,6 +60,8 @@ PeerConnection::~PeerConnection() { void PeerConnection::close() { PLOG_VERBOSE << "Closing PeerConnection"; + mNegociationNeeded = false; + // Close data channels asynchronously mProcessor->enqueue(std::bind(&PeerConnection::closeDataChannels, this)); @@ -100,8 +102,8 @@ bool PeerConnection::hasMedia() const { void PeerConnection::setLocalDescription() { PLOG_VERBOSE << "Setting local description"; - if (std::atomic_load(&mIceTransport)) { - PLOG_DEBUG << "Local description is already set, ignoring"; + if (!mNegociationNeeded.exchange(false)) { + PLOG_DEBUG << "No negociation needed"; return; } @@ -117,9 +119,6 @@ void PeerConnection::setLocalDescription() { void PeerConnection::setRemoteDescription(Description description) { PLOG_VERBOSE << "Setting remote description: " << string(description); - if (hasRemoteDescription()) - throw std::logic_error("Remote description is already set"); - if (description.mediaCount() == 0) throw std::invalid_argument("Remote description has no media line"); @@ -139,20 +138,23 @@ void PeerConnection::setRemoteDescription(Description description) { if (!description.fingerprint()) throw std::invalid_argument("Remote description has no fingerprint"); + if (auto local = localDescription()) { + if (description.iceUfrag() == local->iceUfrag() && description.icePwd() == local->icePwd()) + throw std::logic_error("Got the local description as remote description"); + } + description.hintType(hasLocalDescription() ? Description::Type::Answer : Description::Type::Offer); - if (description.type() == Description::Type::Offer) { - if (hasLocalDescription()) { - PLOG_ERROR << "Got a remote offer description while an answer was expected"; - throw std::logic_error("Got an unexpected remote offer description"); - } - } else { // Answer - if (auto local = localDescription()) { - if (description.iceUfrag() == local->iceUfrag() && - description.icePwd() == local->icePwd()) - throw std::logic_error("Got the local description as remote description"); - } else { + // If there is no remote description, this is the first negociation + // Check it is what we expect + if (!hasRemoteDescription()) { + if (description.type() == Description::Type::Offer) { + if (hasLocalDescription()) { + PLOG_ERROR << "Got a remote offer description while an answer was expected"; + throw std::logic_error("Got an unexpected remote offer description"); + } + } else { // Answer PLOG_ERROR << "Got a remote answer description while an offer was expected"; throw std::logic_error("Got an unexpected remote answer description"); } @@ -169,16 +171,23 @@ void PeerConnection::setRemoteDescription(Description description) { { // Set as remote description std::lock_guard lock(mRemoteDescriptionMutex); + + std::vector existingCandidates; + if(mRemoteDescription) + existingCandidates = mRemoteDescription->extractCandidates(); + mRemoteDescription.emplace(std::move(description)); + for (const auto &candidate : existingCandidates) + mRemoteDescription->addCandidate(candidate); } if (description.type() == Description::Type::Offer) { - // This is an offer and we are the answerer. + // This is an offer, we need to answer Description localDescription = iceTransport->getLocalDescription(Description::Type::Answer); processLocalDescription(localDescription); iceTransport->gatherLocalCandidates(); } else { - // This is an answer and we are the offerer. + // This is an answer auto sctpTransport = std::atomic_load(&mSctpTransport); if (!sctpTransport && iceTransport->role() == Description::Role::Active) { // Since we assumed passive role during DataChannel creation, we need to shift the @@ -238,11 +247,6 @@ std::optional PeerConnection::remoteAddress() const { shared_ptr PeerConnection::addDataChannel(string label, string protocol, Reliability reliability) { - if (auto local = localDescription(); local && !local->hasApplication()) { - PLOG_ERROR << "The PeerConnection was negociated without DataChannel support."; - throw std::runtime_error("No DataChannel support on the PeerConnection"); - } - // RFC 5763: The answerer MUST use either a setup attribute value of setup:active or // setup:passive. [...] Thus, setup:active is RECOMMENDED. // See https://tools.ietf.org/html/rfc5763#section-5 @@ -257,6 +261,11 @@ shared_ptr PeerConnection::addDataChannel(string label, string prot if (transport->state() == SctpTransport::State::Connected) channel->open(transport); + // Renegociation is needed if the current local description does not have application + std::lock_guard lock(mLocalDescriptionMutex); + if (!mLocalDescription || !mLocalDescription->hasApplication()) + mNegociationNeeded = true; + return channel; } @@ -303,6 +312,10 @@ std::shared_ptr PeerConnection::addTrack(Description::Media description) #endif auto track = std::make_shared(std::move(description)); mTracks.emplace(std::make_pair(track->mid(), track)); + + // Renegociation is needed for the new track + mNegociationNeeded = true; + return track; } @@ -454,31 +467,31 @@ shared_ptr PeerConnection::initSctpTransport() { uint16_t sctpPort = remote->application()->sctpPort().value_or(DEFAULT_SCTP_PORT); auto lower = std::atomic_load(&mDtlsTransport); auto transport = std::make_shared( - lower, sctpPort, weak_bind(&PeerConnection::forwardMessage, this, _1), - weak_bind(&PeerConnection::forwardBufferedAmount, this, _1, _2), - [this, weak_this = weak_from_this()](SctpTransport::State state) { - auto shared_this = weak_this.lock(); - if (!shared_this) - return; - switch (state) { - case SctpTransport::State::Connected: - changeState(State::Connected); - mProcessor->enqueue(std::bind(&PeerConnection::openDataChannels, this)); - break; - case SctpTransport::State::Failed: - LOG_WARNING << "SCTP transport failed"; - changeState(State::Failed); - mProcessor->enqueue(std::bind(&PeerConnection::remoteCloseDataChannels, this)); - break; - case SctpTransport::State::Disconnected: - changeState(State::Disconnected); - mProcessor->enqueue(std::bind(&PeerConnection::remoteCloseDataChannels, this)); - break; - default: - // Ignore - break; - } - }); + lower, sctpPort, weak_bind(&PeerConnection::forwardMessage, this, _1), + weak_bind(&PeerConnection::forwardBufferedAmount, this, _1, _2), + [this, weak_this = weak_from_this()](SctpTransport::State state) { + auto shared_this = weak_this.lock(); + if (!shared_this) + return; + switch (state) { + case SctpTransport::State::Connected: + changeState(State::Connected); + mProcessor->enqueue(std::bind(&PeerConnection::openDataChannels, this)); + break; + case SctpTransport::State::Failed: + LOG_WARNING << "SCTP transport failed"; + changeState(State::Failed); + mProcessor->enqueue(std::bind(&PeerConnection::remoteCloseDataChannels, this)); + break; + case SctpTransport::State::Disconnected: + changeState(State::Disconnected); + mProcessor->enqueue(std::bind(&PeerConnection::remoteCloseDataChannels, this)); + break; + default: + // Ignore + break; + } + }); std::atomic_store(&mSctpTransport, transport); if (mState == State::Closed) { @@ -728,7 +741,6 @@ void PeerConnection::openTracks() { #endif } - void PeerConnection::processLocalDescription(Description description) { int activeMediaCount = 0; @@ -821,7 +833,14 @@ void PeerConnection::processLocalDescription(Description description) { { // Set as local description std::lock_guard lock(mLocalDescriptionMutex); + + std::vector existingCandidates; + if(mLocalDescription) + existingCandidates = mLocalDescription->extractCandidates(); + mLocalDescription.emplace(std::move(description)); + for (const auto &candidate : existingCandidates) + mLocalDescription->addCandidate(candidate); } mProcessor->enqueue([this, description = *mLocalDescription]() {