From c3535fb5971209586d16bba1105de8507b373a51 Mon Sep 17 00:00:00 2001 From: Paul-Louis Ageneau Date: Sat, 17 Feb 2024 20:32:08 +0100 Subject: [PATCH] WIP --- src/impl/peerconnection.cpp | 63 ++++++++++++++++++++++++------------- src/impl/peerconnection.hpp | 7 +++-- src/peerconnection.cpp | 19 +++++------ 3 files changed, 54 insertions(+), 35 deletions(-) diff --git a/src/impl/peerconnection.cpp b/src/impl/peerconnection.cpp index 3133fe328..0cd95daf2 100644 --- a/src/impl/peerconnection.cpp +++ b/src/impl/peerconnection.cpp @@ -69,7 +69,6 @@ PeerConnection::~PeerConnection() { } void PeerConnection::close() { - negotiationNeeded = false; if (!closing.exchange(true)) { PLOG_VERBOSE << "Closing PeerConnection"; if (auto transport = std::atomic_load(&mSctpTransport)) @@ -802,25 +801,27 @@ void PeerConnection::iterateTracks(std::function track)> void PeerConnection::openTracks() { #if RTC_ENABLE_MEDIA - if (auto transport = std::atomic_load(&mDtlsTransport)) { - auto srtpTransport = std::dynamic_pointer_cast(transport); - - iterateTracks([&](const shared_ptr &track) { - if (!track->isOpen()) { - if (srtpTransport) { - track->open(srtpTransport); - } else { - // A track was added during a latter renegotiation, whereas SRTP transport was - // not initialized. This is an optimization to use the library with data - // channels only. Set forceMediaTransport to true to initialize the transport - // before dynamically adding tracks. - auto errorMsg = "The connection has no media transport"; - PLOG_ERROR << errorMsg; - track->triggerError(errorMsg); - } + auto remote = remoteDescription(); + auto transport = std::atomic_load(&mDtlsTransport); + if (!remote || !transport) + return; + + auto srtpTransport = std::dynamic_pointer_cast(transport); + iterateTracks([&](const shared_ptr &track) { + if (remote->hasMid(track->mid()) && !track->isOpen()) { + if (srtpTransport) { + track->open(srtpTransport); + } else { + // A track was added during a latter renegotiation, whereas SRTP transport was + // not initialized. This is an optimization to use the library with data + // channels only. Set forceMediaTransport to true to initialize the transport + // before dynamically adding tracks. + auto errorMsg = "The connection has no media transport"; + PLOG_ERROR << errorMsg; + track->triggerError(errorMsg); } - }); - } + } + }); #endif } @@ -1003,8 +1004,7 @@ void PeerConnection::processLocalDescription(Description description) { } } - // There might be no media at this point if the user created a Track, deleted it, - // then called setLocalDescription(). + // There might be no media at this point, for instance if the user deleted tracks if (description.mediaCount() == 0) throw std::runtime_error("No DataChannel or Track to negotiate"); } @@ -1132,6 +1132,27 @@ string PeerConnection::localBundleMid() const { return mLocalDescription ? mLocalDescription->bundleMid() : "0"; } +bool PeerConnection::negotiationNeeded() const { + auto description = localDescription(); + + { + std::shared_lock lock(mDataChannelsMutex); + if (!mDataChannels.empty() || !mUnassignedDataChannels.empty()) + if((!description || !description->hasApplication())) + return true; + } + + { + std::shared_lock lock(mTracksMutex); + for (auto it = mTrackLines.begin(); it != mTrackLines.end(); ++it) + if (auto track = it->lock()) + if (!description || !description->hasMid(track->mid())) + return true; + } + + return false; +} + void PeerConnection::setMediaHandler(shared_ptr handler) { std::unique_lock lock(mMediaHandlerMutex); mMediaHandler = handler; diff --git a/src/impl/peerconnection.hpp b/src/impl/peerconnection.hpp index ad6f41e28..c44976098 100644 --- a/src/impl/peerconnection.hpp +++ b/src/impl/peerconnection.hpp @@ -80,6 +80,8 @@ struct PeerConnection : std::enable_shared_from_this { void processRemoteCandidate(Candidate candidate); string localBundleMid() const; + bool negotiationNeeded() const; + void setMediaHandler(shared_ptr handler); shared_ptr getMediaHandler(); @@ -113,7 +115,6 @@ struct PeerConnection : std::enable_shared_from_this { std::atomic iceState = IceState::New; std::atomic gatheringState = GatheringState::New; std::atomic signalingState = SignalingState::Stable; - std::atomic negotiationNeeded = false; std::atomic closing = false; std::mutex signalingMutex; @@ -148,12 +149,12 @@ struct PeerConnection : std::enable_shared_from_this { std::unordered_map> mDataChannels; // by stream ID std::vector> mUnassignedDataChannels; - std::shared_mutex mDataChannelsMutex; + mutable std::shared_mutex mDataChannelsMutex; std::unordered_map> mTracks; // by mid std::unordered_map> mTracksBySsrc; // by SSRC std::vector> mTrackLines; // by SDP order - std::shared_mutex mTracksMutex; + mutable std::shared_mutex mTracksMutex; Queue> mPendingDataChannels; Queue> mPendingTracks; diff --git a/src/peerconnection.cpp b/src/peerconnection.cpp index dd1ec7c82..b1101dc7b 100644 --- a/src/peerconnection.cpp +++ b/src/peerconnection.cpp @@ -99,7 +99,7 @@ void PeerConnection::setLocalDescription(Description::Type type) { } // Only a local offer resets the negotiation needed flag - if (type == Description::Type::Offer && !impl()->negotiationNeeded.exchange(false)) { + if (type == Description::Type::Offer && !impl()->negotiationNeeded()) { PLOG_DEBUG << "No negotiation needed"; return; } @@ -146,9 +146,11 @@ void PeerConnection::setLocalDescription(Description::Type type) { impl()->changeSignalingState(newSignalingState); signalingLock.unlock(); - if (impl()->gatheringState == GatheringState::New) { + if (impl()->gatheringState == GatheringState::New) iceTransport->gatherLocalCandidates(impl()->localBundleMid()); - } + + if(newSignalingState == SignalingState::Stable && impl()->negotiationNeeded()) + setLocalDescription(Description::Type::Offer); } void PeerConnection::setRemoteDescription(Description description) { @@ -241,6 +243,9 @@ void PeerConnection::setRemoteDescription(Description description) { for (const auto &candidate : remoteCandidates) addRemoteCandidate(candidate); + + if(newSignalingState == SignalingState::Stable && impl()->negotiationNeeded()) + setLocalDescription(); } void PeerConnection::addRemoteCandidate(Candidate candidate) { @@ -271,11 +276,6 @@ shared_ptr PeerConnection::createDataChannel(string label, DataChan auto channelImpl = impl()->emplaceDataChannel(std::move(label), std::move(init)); auto channel = std::make_shared(channelImpl); - // Renegotiation is needed iff the current local description does not have application - auto local = impl()->localDescription(); - if (!local || !local->hasApplication()) - impl()->negotiationNeeded = true; - if (!impl()->config.disableAutoNegotiation) setLocalDescription(); @@ -292,9 +292,6 @@ std::shared_ptr PeerConnection::addTrack(Description::Media description) auto trackImpl = impl()->emplaceTrack(std::move(description)); auto track = std::make_shared(trackImpl); - // Renegotiation is needed for the new or updated track - impl()->negotiationNeeded = true; - return track; }