diff --git a/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp b/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp index 2789066f5790..d74227eb9780 100644 --- a/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp +++ b/dom/media/webrtc/jsapi/PeerConnectionImpl.cpp @@ -851,8 +851,7 @@ nsresult PeerConnectionImpl::GetDatachannelParameters( transportId->clear(); RefPtr datachannelTransceiver; - for (const auto& [id, transceiver] : mJsepSession->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mJsepSession->GetTransceivers()) { if ((transceiver->GetMediaType() == SdpMediaSection::kApplication) && transceiver->mSendTrack.GetNegotiatedDetails()) { datachannelTransceiver = transceiver; @@ -958,18 +957,8 @@ already_AddRefed PeerConnectionImpl::AddTransceiver( return nullptr; } - RefPtr jsepTransceiver = new JsepTransceiver(*type); - - RefPtr transceiver = - CreateTransceiver(jsepTransceiver, aInit, aSendTrack, aRv); - - if (aRv.Failed()) { - // Would be nice if we could peek at the rv without stealing it, so we - // could log... - CSFLogError(LOGTAG, "%s: failed", __FUNCTION__); - return nullptr; - } - + RefPtr jsepTransceiver = + new JsepTransceiver(*type, *mUuidGen); jsepTransceiver->SetRtxIsAllowed(mRtxIsAllowed); // Do this last, since it is not possible to roll back. @@ -981,6 +970,18 @@ already_AddRefed PeerConnectionImpl::AddTransceiver( return nullptr; } + RefPtr transceiver = CreateTransceiver( + jsepTransceiver->GetUuid(), + jsepTransceiver->GetMediaType() == SdpMediaSection::kVideo, aInit, + aSendTrack, aRv); + + if (aRv.Failed()) { + // Would be nice if we could peek at the rv without stealing it, so we + // could log... + CSFLogError(LOGTAG, "%s: failed", __FUNCTION__); + return nullptr; + } + mTransceivers.AppendElement(transceiver); return transceiver.forget(); } @@ -1073,8 +1074,7 @@ PeerConnectionImpl::CreateDataChannel( CSFLogDebug(LOGTAG, "%s: making DOMDataChannel", __FUNCTION__); RefPtr dcTransceiver; - for (auto& [id, transceiver] : mJsepSession->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (auto& transceiver : mJsepSession->GetTransceivers()) { if (transceiver->GetMediaType() == SdpMediaSection::kApplication) { dcTransceiver = transceiver; break; @@ -1082,8 +1082,8 @@ PeerConnectionImpl::CreateDataChannel( } if (!dcTransceiver) { - dcTransceiver = - new JsepTransceiver(SdpMediaSection::MediaType::kApplication); + dcTransceiver = new JsepTransceiver( + SdpMediaSection::MediaType::kApplication, *mUuidGen); mJsepSession->AddTransceiver(dcTransceiver); } @@ -1609,25 +1609,37 @@ PeerConnectionImpl::SetRemoteDescription(int32_t action, const char* aSDP) { mPCObserver->OnSetDescriptionError(*buildJSErrorData(result, errorString), jrv); } else { - for (const auto& [id, jsepTransceiver] : mJsepSession->GetTransceivers()) { + for (const auto& jsepTransceiver : mJsepSession->GetTransceivers()) { if (jsepTransceiver->GetMediaType() == SdpMediaSection::MediaType::kApplication) { continue; } - if (originalTransceivers.count(id)) { - continue; + CSFLogDebug(LOGTAG, "%s: Looking for match", __FUNCTION__); + RefPtr transceiver; + for (auto& temp : mTransceivers) { + if (temp->GetJsepTransceiverId() == jsepTransceiver->GetUuid()) { + CSFLogDebug(LOGTAG, "%s: Found match", __FUNCTION__); + transceiver = temp; + break; + } } - dom::RTCRtpTransceiverInit init; - init.mDirection = RTCRtpTransceiverDirection::Recvonly; - RefPtr transceiver = - CreateTransceiver(jsepTransceiver, init, nullptr, jrv); - if (jrv.Failed()) { - appendHistory(); - return NS_ERROR_FAILURE; + if (!transceiver) { + CSFLogDebug(LOGTAG, "%s: No match, making new", __FUNCTION__); + dom::RTCRtpTransceiverInit init; + init.mDirection = RTCRtpTransceiverDirection::Recvonly; + IgnoredErrorResult rv; + transceiver = CreateTransceiver( + jsepTransceiver->GetUuid(), + jsepTransceiver->GetMediaType() == SdpMediaSection::kVideo, init, + nullptr, rv); + if (NS_WARN_IF(rv.Failed())) { + MOZ_ASSERT(false); + break; + } + mTransceivers.AppendElement(transceiver); } - mTransceivers.AppendElement(transceiver); } if (wasRestartingIce) { @@ -3213,8 +3225,7 @@ bool PeerConnectionImpl::ShouldForceProxy() const { } void PeerConnectionImpl::EnsureTransports(const JsepSession& aSession) { - for (const auto& [id, transceiver] : aSession.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : aSession.GetTransceivers()) { if (transceiver->HasOwnTransport()) { mTransportHandler->EnsureProvisionalTransport( transceiver->mTransport.mTransportId, @@ -3264,8 +3275,7 @@ void PeerConnectionImpl::RemoveRTCDtlsTransportsExcept( nsresult PeerConnectionImpl::UpdateTransports(const JsepSession& aSession, const bool forceIceTcp) { std::set finalTransports; - for (const auto& [id, transceiver] : aSession.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : aSession.GetTransceivers()) { if (transceiver->HasOwnTransport()) { finalTransports.insert(transceiver->mTransport.mTransportId); UpdateTransport(*transceiver, forceIceTcp); @@ -3636,7 +3646,7 @@ void PeerConnectionImpl::EnsureIceGathering(bool aDefaultRouteOnly, } already_AddRefed PeerConnectionImpl::CreateTransceiver( - JsepTransceiver* aJsepTransceiver, const RTCRtpTransceiverInit& aInit, + const std::string& aId, bool aIsVideo, const RTCRtpTransceiverInit& aInit, dom::MediaStreamTrack* aSendTrack, ErrorResult& aRv) { PeerConnectionCtx* ctx = PeerConnectionCtx::GetInstance(); if (!mCall) { @@ -3649,8 +3659,8 @@ already_AddRefed PeerConnectionImpl::CreateTransceiver( } RefPtr transceiver = new RTCRtpTransceiver( - mWindow, PrivacyNeeded(), this, mTransportHandler, aJsepTransceiver, - mSTSThread.get(), aSendTrack, mCall.get(), mIdGenerator); + mWindow, PrivacyNeeded(), this, mTransportHandler, mJsepSession.get(), + aId, aIsVideo, mSTSThread.get(), aSendTrack, mCall.get(), mIdGenerator); transceiver->Init(aInit, aRv); if (aRv.Failed()) { diff --git a/dom/media/webrtc/jsapi/PeerConnectionImpl.h b/dom/media/webrtc/jsapi/PeerConnectionImpl.h index 95c668c87830..817ae0d0e559 100644 --- a/dom/media/webrtc/jsapi/PeerConnectionImpl.h +++ b/dom/media/webrtc/jsapi/PeerConnectionImpl.h @@ -126,6 +126,9 @@ class RemoteSourceStreamInfo; class PCUuidGenerator : public mozilla::JsepUuidGenerator { public: virtual bool Generate(std::string* idp) override; + virtual mozilla::JsepUuidGenerator* Clone() const override { + return new PCUuidGenerator(*this); + } private: nsCOMPtr mGenerator; @@ -725,7 +728,7 @@ class PeerConnectionImpl final nsresult UpdateMediaPipelines(); already_AddRefed CreateTransceiver( - JsepTransceiver* aJsepTransceiver, + const std::string& aId, bool aIsVideo, const dom::RTCRtpTransceiverInit& aInit, dom::MediaStreamTrack* aSendTrack, ErrorResult& aRv); diff --git a/dom/media/webrtc/jsapi/RTCRtpTransceiver.cpp b/dom/media/webrtc/jsapi/RTCRtpTransceiver.cpp index 025f517955f0..104c2fc3288f 100644 --- a/dom/media/webrtc/jsapi/RTCRtpTransceiver.cpp +++ b/dom/media/webrtc/jsapi/RTCRtpTransceiver.cpp @@ -156,13 +156,15 @@ NS_INTERFACE_MAP_END RTCRtpTransceiver::RTCRtpTransceiver( nsPIDOMWindowInner* aWindow, bool aPrivacyNeeded, PeerConnectionImpl* aPc, - MediaTransportHandler* aTransportHandler, JsepTransceiver* aJsepTransceiver, + MediaTransportHandler* aTransportHandler, JsepSession* aJsepSession, + const std::string& aTransceiverId, bool aIsVideo, nsISerialEventTarget* aStsThread, dom::MediaStreamTrack* aSendTrack, WebrtcCallWrapper* aCallWrapper, RTCStatsIdGenerator* aIdGenerator) : mWindow(aWindow), mPc(aPc), mTransportHandler(aTransportHandler), - mJsepTransceiver(aJsepTransceiver), + mTransceiverId(aTransceiverId), + mJsepTransceiver(aJsepSession->GetTransceiver(mTransceiverId)), mStsThread(aStsThread), mCallWrapper(aCallWrapper), mSendTrack(aSendTrack), diff --git a/dom/media/webrtc/jsapi/RTCRtpTransceiver.h b/dom/media/webrtc/jsapi/RTCRtpTransceiver.h index 780184634976..7fd5eb4ce0d7 100644 --- a/dom/media/webrtc/jsapi/RTCRtpTransceiver.h +++ b/dom/media/webrtc/jsapi/RTCRtpTransceiver.h @@ -12,7 +12,7 @@ #include "nsTArray.h" #include "mozilla/dom/MediaStreamTrack.h" #include "ErrorList.h" -#include "jsep/JsepTransceiver.h" +#include "jsep/JsepSession.h" #include "transport/transportlayer.h" // For TransportLayer::State #include "mozilla/dom/RTCRtpTransceiverBinding.h" @@ -20,7 +20,6 @@ class nsIPrincipal; namespace mozilla { class PeerIdentity; -class JsepTransceiver; class MediaSessionConduit; class VideoSessionConduit; class AudioSessionConduit; @@ -58,14 +57,12 @@ class RTCRtpTransceiver : public nsISupports, /** * |aSendTrack| might or might not be set. */ - RTCRtpTransceiver(nsPIDOMWindowInner* aWindow, bool aPrivacyNeeded, - PeerConnectionImpl* aPc, - MediaTransportHandler* aTransportHandler, - JsepTransceiver* aJsepTransceiver, - nsISerialEventTarget* aStsThread, - MediaStreamTrack* aSendTrack, - WebrtcCallWrapper* aCallWrapper, - RTCStatsIdGenerator* aIdGenerator); + RTCRtpTransceiver( + nsPIDOMWindowInner* aWindow, bool aPrivacyNeeded, PeerConnectionImpl* aPc, + MediaTransportHandler* aTransportHandler, JsepSession* aJsepSession, + const std::string& aTransceiverId, bool aIsVideo, + nsISerialEventTarget* aStsThread, MediaStreamTrack* aSendTrack, + WebrtcCallWrapper* aCallWrapper, RTCStatsIdGenerator* aIdGenerator); void Init(const RTCRtpTransceiverInit& aInit, ErrorResult& aRv); @@ -145,6 +142,8 @@ class RTCRtpTransceiver : public nsISupports, MediaSessionConduit* GetConduit() const { return mConduit; } + const std::string& GetJsepTransceiverId() const { return mTransceiverId; } + // nsISupports NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(RTCRtpTransceiver) @@ -188,7 +187,8 @@ class RTCRtpTransceiver : public nsISupports, nsCOMPtr mWindow; RefPtr mPc; RefPtr mTransportHandler; - const RefPtr mJsepTransceiver; + const std::string mTransceiverId; + RefPtr mJsepTransceiver; nsCOMPtr mStsThread; // state for webrtc.org that is shared between all transceivers RefPtr mCallWrapper; diff --git a/dom/media/webrtc/jsep/JsepSession.h b/dom/media/webrtc/jsep/JsepSession.h index 8bde35aa2281..99f2a69004f1 100644 --- a/dom/media/webrtc/jsep/JsepSession.h +++ b/dom/media/webrtc/jsep/JsepSession.h @@ -107,8 +107,7 @@ class JsepSession { template void ForEachCodec(UnaryFunction& function) { std::for_each(Codecs().begin(), Codecs().end(), function); - for (auto& [id, transceiver] : GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (auto& transceiver : GetTransceivers()) { transceiver->mSendTrack.ForEachCodec(function); transceiver->mRecvTrack.ForEachCodec(function); } @@ -117,16 +116,24 @@ class JsepSession { template void SortCodecs(BinaryPredicate& sorter) { std::stable_sort(Codecs().begin(), Codecs().end(), sorter); - for (auto& [id, transceiver] : GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (auto& transceiver : GetTransceivers()) { transceiver->mSendTrack.SortCodecs(sorter); transceiver->mRecvTrack.SortCodecs(sorter); } } - virtual const std::map>& GetTransceivers() + // Returns transceivers in the order they were added. + virtual const std::vector>& GetTransceivers() const = 0; - virtual std::map>& GetTransceivers() = 0; + virtual std::vector>& GetTransceivers() = 0; + RefPtr GetTransceiver(const std::string& aId) const { + for (const auto& transceiver : GetTransceivers()) { + if (transceiver->GetUuid() == aId) { + return transceiver; + } + } + return nullptr; + } virtual nsresult AddTransceiver(RefPtr transceiver) = 0; class Result { @@ -198,8 +205,7 @@ class JsepSession { memset(receiving, 0, sizeof(receiving)); memset(sending, 0, sizeof(sending)); - for (const auto& [id, transceiver] : GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : GetTransceivers()) { if (transceiver->mRecvTrack.GetActive() || transceiver->GetMediaType() == SdpMediaSection::kApplication) { receiving[transceiver->mRecvTrack.GetMediaType()]++; diff --git a/dom/media/webrtc/jsep/JsepSessionImpl.cpp b/dom/media/webrtc/jsep/JsepSessionImpl.cpp index 746912afe7ca..158c3236d048 100644 --- a/dom/media/webrtc/jsep/JsepSessionImpl.cpp +++ b/dom/media/webrtc/jsep/JsepSessionImpl.cpp @@ -115,7 +115,8 @@ JsepSessionImpl::GetLocalIceCredentials() const { nsresult JsepSessionImpl::AddTransceiver(RefPtr transceiver) { mLastError.clear(); - MOZ_MTLOG(ML_DEBUG, "[" << mName << "]: Adding transceiver."); + MOZ_MTLOG(ML_DEBUG, + "[" << mName << "]: Adding transceiver " << transceiver->GetUuid()); if (transceiver->GetMediaType() != SdpMediaSection::kApplication) { // Make sure we have an ssrc. Might already be set. @@ -126,6 +127,7 @@ nsresult JsepSessionImpl::AddTransceiver(RefPtr transceiver) { // (man I hate this) if (mEncodeTrackId) { std::string trackId; + // TODO: Maybe reuse the transceiver's UUID here? if (!mUuidGen->Generate(&trackId)) { JSEP_SET_ERROR("Failed to generate UUID for JsepTrack"); return NS_ERROR_FAILURE; @@ -138,8 +140,7 @@ nsresult JsepSessionImpl::AddTransceiver(RefPtr transceiver) { // of asserting. transceiver->mJsDirection = SdpDirectionAttribute::kSendrecv; #ifdef DEBUG - for (const auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mTransceivers) { MOZ_ASSERT(transceiver->GetMediaType() != SdpMediaSection::kApplication); } #endif @@ -149,7 +150,7 @@ nsresult JsepSessionImpl::AddTransceiver(RefPtr transceiver) { transceiver->mRecvTrack.PopulateCodecs(mSupportedCodecs); // We do not set mLevel yet, we do that either on createOffer, or setRemote - mTransceivers[mTransceiverIdCounter++] = transceiver; + mTransceivers.push_back(transceiver); return NS_OK; } @@ -802,8 +803,8 @@ JsepSession::Result JsepSessionImpl::SetLocalDescription( if (type == kJsepSdpOffer) { // Save in case we need to rollback mOldTransceivers.clear(); - for (const auto& [id, transceiver] : mTransceivers) { - mOldTransceivers[id] = new JsepTransceiver(*transceiver); + for (const auto& transceiver : mTransceivers) { + mOldTransceivers.push_back(new JsepTransceiver(*transceiver)); } } @@ -1013,8 +1014,8 @@ JsepSession::Result JsepSessionImpl::SetRemoteDescription( // Save in case we need to rollback. if (type == kJsepSdpOffer) { mOldTransceivers.clear(); - for (const auto& [id, transceiver] : mTransceivers) { - mOldTransceivers[id] = new JsepTransceiver(*transceiver); + for (const auto& transceiver : mTransceivers) { + mOldTransceivers.push_back(new JsepTransceiver(*transceiver)); if (!transceiver->IsNegotiated()) { // We chose a level for this transceiver, but never negotiated it. // Discard this state. @@ -1110,8 +1111,7 @@ nsresult JsepSessionImpl::HandleNegotiatedSession( CopyBundleTransports(); std::vector remoteTracks; - for (const auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mTransceivers) { remoteTracks.push_back(&transceiver->mRecvTrack); } JsepTrack::SetUniquePayloadTypes(remoteTracks); @@ -1215,8 +1215,7 @@ void JsepSessionImpl::EnsureHasOwnTransport(const SdpMediaSection& msection, } void JsepSessionImpl::CopyBundleTransports() { - for (auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (auto& transceiver : mTransceivers) { if (transceiver->HasBundleLevel()) { MOZ_MTLOG(ML_DEBUG, "[" << mName << "] Transceiver " << transceiver->GetLevel() @@ -1497,8 +1496,7 @@ nsresult JsepSessionImpl::SetRemoteDescriptionAnswer(JsepSdpType type, } JsepTransceiver* JsepSessionImpl::GetTransceiverForLevel(size_t level) const { - for (auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mTransceivers) { if (transceiver->HasLevel() && (transceiver->GetLevel() == level)) { return transceiver.get(); } @@ -1509,8 +1507,7 @@ JsepTransceiver* JsepSessionImpl::GetTransceiverForLevel(size_t level) const { JsepTransceiver* JsepSessionImpl::GetTransceiverForMid( const std::string& mid) const { - for (auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mTransceivers) { if (transceiver->IsAssociated() && (transceiver->GetMid() == mid)) { return transceiver.get(); } @@ -1540,8 +1537,7 @@ JsepTransceiver* JsepSessionImpl::GetTransceiverForLocal(size_t level) { // There is no transceiver for |level| right now. // Look for an RTP transceiver - for (auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (auto& transceiver : mTransceivers) { if (transceiver->GetMediaType() != SdpMediaSection::kApplication && !transceiver->IsStopped() && !transceiver->HasLevel()) { transceiver->SetLevel(level); @@ -1550,8 +1546,7 @@ JsepTransceiver* JsepSessionImpl::GetTransceiverForLocal(size_t level) { } // Ok, look for a datachannel - for (auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (auto& transceiver : mTransceivers) { if (!transceiver->IsStopped() && !transceiver->HasLevel()) { transceiver->SetLevel(level); return transceiver.get(); @@ -1583,7 +1578,7 @@ JsepTransceiver* JsepSessionImpl::GetTransceiverForRemote( // Make a new transceiver RefPtr newTransceiver(new JsepTransceiver( - msection.GetMediaType(), SdpDirectionAttribute::kRecvonly)); + msection.GetMediaType(), *mUuidGen, SdpDirectionAttribute::kRecvonly)); newTransceiver->SetLevel(level); newTransceiver->SetCreatedBySetRemote(); nsresult rv = AddTransceiver(newTransceiver); @@ -1593,8 +1588,7 @@ JsepTransceiver* JsepSessionImpl::GetTransceiverForRemote( JsepTransceiver* JsepSessionImpl::GetTransceiverWithTransport( const std::string& transportId) const { - for (const auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mTransceivers) { if (transceiver->HasOwnTransport() && (transceiver->mTransport.mTransportId == transportId)) { MOZ_ASSERT(transceiver->HasLevel(), @@ -1657,8 +1651,7 @@ nsresult JsepSessionImpl::UpdateTransceiversFromRemoteDescription( JsepTransceiver* JsepSessionImpl::FindUnassociatedTransceiver( SdpMediaSection::MediaType type, bool magic) { // Look through transceivers that are not mapped to an m-section - for (auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (auto& transceiver : mTransceivers) { if (type == SdpMediaSection::kApplication && type == transceiver->GetMediaType()) { transceiver->RestartDatachannelTransceiver(); @@ -1675,30 +1668,32 @@ JsepTransceiver* JsepSessionImpl::FindUnassociatedTransceiver( } void JsepSessionImpl::RollbackLocalOffer() { - for (auto& [id, transceiver] : mTransceivers) { - if (mOldTransceivers.count(id)) { - transceiver->Rollback(*mOldTransceivers[id], false); - mOldTransceivers[id] = transceiver; + for (size_t i = 0; i < mTransceivers.size(); ++i) { + auto transceiver = mTransceivers[i]; + if (mOldTransceivers.size() > i) { + transceiver->Rollback(*mOldTransceivers[i], false); + mOldTransceivers[i] = transceiver; continue; } RefPtr temp( - new JsepTransceiver(transceiver->GetMediaType())); + new JsepTransceiver(transceiver->GetMediaType(), *mUuidGen)); temp->mSendTrack.PopulateCodecs(mSupportedCodecs); temp->mRecvTrack.PopulateCodecs(mSupportedCodecs); transceiver->Rollback(*temp, false); - mOldTransceivers[id] = transceiver; + mOldTransceivers.push_back(transceiver); } mTransceivers = std::move(mOldTransceivers); } void JsepSessionImpl::RollbackRemoteOffer() { - for (auto& [id, transceiver] : mTransceivers) { - if (mOldTransceivers.count(id)) { + for (size_t i = 0; i < mTransceivers.size(); ++i) { + auto transceiver = mTransceivers[i]; + if (mOldTransceivers.size() > i) { // Some stuff cannot be rolled back. Save this information. - transceiver->Rollback(*mOldTransceivers[id], true); - mOldTransceivers[id] = transceiver; + transceiver->Rollback(*mOldTransceivers[i], true); + mOldTransceivers[i] = transceiver; continue; } @@ -1709,7 +1704,7 @@ void JsepSessionImpl::RollbackRemoteOffer() { // We rollback even for transceivers we will remove, just to ensure we end // up at the starting state. RefPtr temp( - new JsepTransceiver(transceiver->GetMediaType())); + new JsepTransceiver(transceiver->GetMediaType(), *mUuidGen)); temp->mSendTrack.PopulateCodecs(mSupportedCodecs); temp->mRecvTrack.PopulateCodecs(mSupportedCodecs); transceiver->Rollback(*temp, true); @@ -1717,9 +1712,8 @@ void JsepSessionImpl::RollbackRemoteOffer() { if (shouldRemove) { transceiver->Stop(); transceiver->SetRemoved(); - } else { - mOldTransceivers[id] = transceiver; } + mOldTransceivers.push_back(transceiver); } mTransceivers = std::move(mOldTransceivers); @@ -2287,8 +2281,7 @@ nsresult JsepSessionImpl::UpdateDefaultCandidate( return NS_ERROR_UNEXPECTED; } - for (const auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mTransceivers) { // We set the default address for bundled m-sections, but not candidate // attributes. Ugh. if (transceiver->mTransport.mTransportId == transportId) { @@ -2402,8 +2395,7 @@ JsepSessionImpl::GetLastSdpParsingErrors() const { bool JsepSessionImpl::CheckNegotiationNeeded() const { MOZ_ASSERT(mState == kJsepStateStable); - for (const auto& [id, transceiver] : mTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mTransceivers) { if (transceiver->IsStopped()) { if (transceiver->IsAssociated()) { MOZ_MTLOG(ML_DEBUG, "[" << mName diff --git a/dom/media/webrtc/jsep/JsepSessionImpl.h b/dom/media/webrtc/jsep/JsepSessionImpl.h index 333fac883fdc..6db438cef18b 100644 --- a/dom/media/webrtc/jsep/JsepSessionImpl.h +++ b/dom/media/webrtc/jsep/JsepSessionImpl.h @@ -20,12 +20,6 @@ namespace mozilla { -class JsepUuidGenerator { - public: - virtual ~JsepUuidGenerator() {} - virtual bool Generate(std::string* id) = 0; -}; - class JsepSessionImpl : public JsepSession { public: JsepSessionImpl(const std::string& name, UniquePtr uuidgen) @@ -138,13 +132,12 @@ class JsepSessionImpl : public JsepSession { virtual std::set> GetLocalIceCredentials() const override; - virtual const std::map>& GetTransceivers() + virtual const std::vector>& GetTransceivers() const override { return mTransceivers; } - virtual std::map>& GetTransceivers() - override { + virtual std::vector>& GetTransceivers() override { return mTransceivers; } @@ -237,11 +230,10 @@ class JsepSessionImpl : public JsepSession { void SetIceRestarting(bool restarting); // !!!NOT INDEXED BY LEVEL!!! The level mapping is done with - // JsepTransceiver::mLevel. The keys are opaque, stable identifiers that are - // unique within the JsepSession. - std::map> mTransceivers; + // JsepTransceiver::mLevel. The keys are UUIDs. + std::vector> mTransceivers; // So we can rollback. Not as simple as just going back to the old, though... - std::map> mOldTransceivers; + std::vector> mOldTransceivers; Maybe mIsPendingOfferer; Maybe mIsCurrentOfferer; diff --git a/dom/media/webrtc/jsep/JsepTransceiver.h b/dom/media/webrtc/jsep/JsepTransceiver.h index e0ecdc7a8e01..dcc886181bda 100644 --- a/dom/media/webrtc/jsep/JsepTransceiver.h +++ b/dom/media/webrtc/jsep/JsepTransceiver.h @@ -19,12 +19,20 @@ namespace mozilla { +class JsepUuidGenerator { + public: + virtual ~JsepUuidGenerator() = default; + virtual bool Generate(std::string* id) = 0; + virtual JsepUuidGenerator* Clone() const = 0; +}; + class JsepTransceiver { private: ~JsepTransceiver(){}; public: explicit JsepTransceiver(SdpMediaSection::MediaType type, + JsepUuidGenerator& aUuidGen, SdpDirectionAttribute::Direction jsDirection = SdpDirectionAttribute::kSendrecv) : mJsDirection(jsDirection), @@ -37,7 +45,11 @@ class JsepTransceiver { mStopped(false), mRemoved(false), mNegotiated(false), - mCanRecycle(false) {} + mCanRecycle(false) { + if (!aUuidGen.Generate(&mUuid)) { + MOZ_CRASH(); + } + } // Can't use default copy c'tor because of the refcount members. Ugh. JsepTransceiver(const JsepTransceiver& orig) @@ -45,6 +57,7 @@ class JsepTransceiver { mSendTrack(orig.mSendTrack), mRecvTrack(orig.mRecvTrack), mTransport(orig.mTransport), + mUuid(orig.mUuid), mMid(orig.mMid), mLevel(orig.mLevel), mBundleLevel(orig.mBundleLevel), @@ -164,6 +177,8 @@ class JsepTransceiver { bool CanRecycle() const { return mCanRecycle; } + const std::string& GetUuid() const { return mUuid; } + // Convenience function SdpMediaSection::MediaType GetMediaType() const { MOZ_ASSERT(mRecvTrack.GetMediaType() == mSendTrack.GetMediaType()); @@ -192,6 +207,7 @@ class JsepTransceiver { JsepTransport mTransport; private: + std::string mUuid; // Stuff that is not negotiated std::string mMid; size_t mLevel; // SIZE_MAX if no level diff --git a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp index 51448c4e20c0..06f6b4888605 100644 --- a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp +++ b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp @@ -46,6 +46,10 @@ class FakeUuidGenerator : public mozilla::JsepUuidGenerator { return true; } + mozilla::JsepUuidGenerator* Clone() const { + return new FakeUuidGenerator(*this); + } + private: static uint64_t ctr; }; @@ -100,13 +104,12 @@ class JsepSessionTest : public JsepSessionTestBase, } void CheckTransceiverInvariants( - const std::map>& oldTransceivers, - const std::map>& newTransceivers) { + const std::vector>& oldTransceivers, + const std::vector>& newTransceivers) { ASSERT_LE(oldTransceivers.size(), newTransceivers.size()); std::set levels; - for (const auto& [id, newTransceiver] : newTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& newTransceiver : newTransceivers) { if (newTransceiver->HasLevel()) { ASSERT_FALSE(levels.count(newTransceiver->GetLevel())) << "Two new transceivers are mapped to level " @@ -124,8 +127,7 @@ class JsepSessionTest : public JsepSessionTestBase, "transceivers."; } - for (const auto& [id, oldTransceiver] : oldTransceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& oldTransceiver : oldTransceivers) { if (oldTransceiver->HasLevel()) { ASSERT_TRUE(levels.count(oldTransceiver->GetLevel())) << "Level " << oldTransceiver->GetLevel() @@ -137,17 +139,18 @@ class JsepSessionTest : public JsepSessionTestBase, } } - std::map> DeepCopy( - const std::map>& transceivers) { - std::map> copy; - for (const auto& [id, transceiver] : transceivers) { - copy[id] = new JsepTransceiver(*transceiver); + std::vector> DeepCopy( + const std::vector>& transceivers) { + std::vector> copy; + copy.reserve(transceivers.size()); + for (const auto& transceiver : transceivers) { + copy.push_back(new JsepTransceiver(*transceiver)); } return copy; } std::string CreateOffer(const Maybe& options = Nothing()) { - std::map> transceiversBefore = + std::vector> transceiversBefore = DeepCopy(mSessionOff->GetTransceivers()); JsepOfferOptions defaultOptions; const JsepOfferOptions& optionsRef = options ? *options : defaultOptions; @@ -267,11 +270,10 @@ class JsepSessionTest : public JsepSessionTestBase, void AddTracks(JsepSessionImpl& side, const std::vector& mediatypes, AddTrackMagic magic = ADDTRACK_MAGIC) { - FakeUuidGenerator uuid_gen; std::string stream_id; std::string track_id; - ASSERT_TRUE(uuid_gen.Generate(&stream_id)); + ASSERT_TRUE(mUuidGen.Generate(&stream_id)); AddTracksToStream(side, stream_id, mediatypes, magic); } @@ -296,16 +298,16 @@ class JsepSessionTest : public JsepSessionTestBase, AddTrackMagic magic = ADDTRACK_MAGIC) { - FakeUuidGenerator uuid_gen; std::string track_id; for (auto type : mediatypes) { - ASSERT_TRUE(uuid_gen.Generate(&track_id)); + ASSERT_TRUE(mUuidGen.Generate(&track_id)); RefPtr suitableTransceiver; size_t i; if (magic == ADDTRACK_MAGIC) { - for (auto& [id, transceiver] : side.GetTransceivers()) { + for (i = 0; i < side.GetTransceivers().size(); ++i) { + auto transceiver = side.GetTransceivers()[i]; if (transceiver->mSendTrack.GetMediaType() != type) { continue; } @@ -313,16 +315,15 @@ class JsepSessionTest : public JsepSessionTestBase, if (IsNull(transceiver->mSendTrack) || transceiver->GetMediaType() == SdpMediaSection::kApplication) { suitableTransceiver = transceiver; - i = id; break; } } } if (!suitableTransceiver) { - suitableTransceiver = new JsepTransceiver(type); + i = side.GetTransceivers().size(); + suitableTransceiver = new JsepTransceiver(type, mUuidGen); side.AddTransceiver(suitableTransceiver); - i = side.GetTransceivers().rbegin()->first; } std::cerr << "Updating send track for transceiver " << i << std::endl; @@ -352,8 +353,7 @@ class JsepSessionTest : public JsepSessionTestBase, std::vector GetLocalTracks(const JsepSession& session) const { std::vector result; - for (const auto& [id, transceiver] : session.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : session.GetTransceivers()) { if (!IsNull(transceiver->mSendTrack)) { result.push_back(transceiver->mSendTrack); } @@ -363,8 +363,7 @@ class JsepSessionTest : public JsepSessionTestBase, std::vector GetRemoteTracks(const JsepSession& session) const { std::vector result; - for (const auto& [id, transceiver] : session.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : session.GetTransceivers()) { if (!IsNull(transceiver->mRecvTrack)) { result.push_back(transceiver->mRecvTrack); } @@ -373,8 +372,7 @@ class JsepSessionTest : public JsepSessionTestBase, } JsepTransceiver* GetDatachannelTransceiver(JsepSession& side) { - for (const auto& [id, transceiver] : side.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : side.GetTransceivers()) { if (transceiver->mSendTrack.GetMediaType() == SdpMediaSection::MediaType::kApplication) { return transceiver.get(); @@ -385,8 +383,7 @@ class JsepSessionTest : public JsepSessionTestBase, } JsepTransceiver* GetNegotiatedTransceiver(JsepSession& side, size_t index) { - for (const auto& [id, transceiver] : side.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : side.GetTransceivers()) { if (transceiver->mSendTrack.GetNegotiatedDetails() || transceiver->mRecvTrack.GetNegotiatedDetails()) { if (index) { @@ -402,10 +399,8 @@ class JsepSessionTest : public JsepSessionTestBase, } JsepTransceiver* GetTransceiverByLevel( - const std::map>& transceivers, - size_t level) { - for (const auto& [id, transceiver] : transceivers) { - (void)id; // Lame, but no better way to do this right now. + const std::vector>& transceivers, size_t level) { + for (const auto& transceiver : transceivers) { if (transceiver->HasLevel() && transceiver->GetLevel() == level) { return transceiver.get(); } @@ -461,8 +456,7 @@ class JsepSessionTest : public JsepSessionTestBase, JsepTrack GetTrack(JsepSessionImpl& side, SdpMediaSection::MediaType type, size_t index) const { - for (const auto& [id, transceiver] : side.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : side.GetTransceivers()) { if (IsNull(transceiver->mSendTrack) || transceiver->mSendTrack.GetMediaType() != type) { continue; @@ -655,19 +649,16 @@ class JsepSessionTest : public JsepSessionTestBase, return true; } - bool Equals(const std::map>& t1, - const std::map>& t2) const { + bool Equals(const std::vector>& t1, + const std::vector>& t2) const { if (t1.size() != t2.size()) { std::cerr << "Size differs: t1.size = " << t1.size() << ", t2.size = " << t2.size() << std::endl; return false; } - for (const auto& [id, transceiver] : t1) { - if (!t2.count(id)) { - return false; - } - if (!Equals(*transceiver, *t2.at(id))) { + for (size_t i = 0; i < t1.size(); ++i) { + if (!Equals(*t1[i], *t2[i])) { return false; } } @@ -743,7 +734,7 @@ class JsepSessionTest : public JsepSessionTestBase, } std::string CreateAnswer() { - std::map> transceiversBefore = + std::vector> transceiversBefore = DeepCopy(mSessionAns->GetTransceivers()); JsepAnswerOptions options; @@ -779,7 +770,7 @@ class JsepSessionTest : public JsepSessionTestBase, void SetLocalOffer(const std::string& offer, uint32_t checkFlags = ALL_CHECKS) { - std::map> transceiversBefore = + std::vector> transceiversBefore = DeepCopy(mSessionOff->GetTransceivers()); JsepSession::Result result = @@ -795,8 +786,7 @@ class JsepSessionTest : public JsepSessionTestBase, if (checkFlags & CHECK_TRACKS) { // This assumes no recvonly or inactive transceivers. ASSERT_EQ(types.size(), mSessionOff->GetTransceivers().size()); - for (const auto& [id, transceiver] : mSessionOff->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionOff->GetTransceivers()) { if (!transceiver->HasLevel()) { continue; } @@ -820,7 +810,7 @@ class JsepSessionTest : public JsepSessionTestBase, void SetRemoteOffer(const std::string& offer, uint32_t checkFlags = ALL_CHECKS) { - std::map> transceiversBefore = + std::vector> transceiversBefore = DeepCopy(mSessionAns->GetTransceivers()); JsepSession::Result result = @@ -836,8 +826,7 @@ class JsepSessionTest : public JsepSessionTestBase, if (checkFlags & CHECK_TRACKS) { // This assumes no recvonly or inactive transceivers. ASSERT_EQ(types.size(), mSessionAns->GetTransceivers().size()); - for (const auto& [id, transceiver] : mSessionAns->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionAns->GetTransceivers()) { if (!transceiver->HasLevel()) { continue; } @@ -858,7 +847,7 @@ class JsepSessionTest : public JsepSessionTestBase, void SetLocalAnswer(const std::string& answer, uint32_t checkFlags = ALL_CHECKS) { - std::map> transceiversBefore = + std::vector> transceiversBefore = DeepCopy(mSessionAns->GetTransceivers()); JsepSession::Result result = @@ -873,8 +862,7 @@ class JsepSessionTest : public JsepSessionTestBase, if (checkFlags & CHECK_TRACKS) { // Verify that the right stuff is in the tracks. ASSERT_EQ(types.size(), mSessionAns->GetTransceivers().size()); - for (const auto& [id, transceiver] : mSessionAns->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionAns->GetTransceivers()) { if (!transceiver->HasLevel()) { continue; } @@ -906,7 +894,7 @@ class JsepSessionTest : public JsepSessionTestBase, void SetRemoteAnswer(const std::string& answer, uint32_t checkFlags = ALL_CHECKS) { - std::map> transceiversBefore = + std::vector> transceiversBefore = DeepCopy(mSessionOff->GetTransceivers()); JsepSession::Result result = @@ -921,8 +909,7 @@ class JsepSessionTest : public JsepSessionTestBase, if (checkFlags & CHECK_TRACKS) { // Verify that the right stuff is in the tracks. ASSERT_EQ(types.size(), mSessionOff->GetTransceivers().size()); - for (const auto& [id, transceiver] : mSessionOff->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionOff->GetTransceivers()) { if (!transceiver->HasLevel()) { continue; } @@ -950,8 +937,7 @@ class JsepSessionTest : public JsepSessionTestBase, } std::string GetTransportId(const JsepSession& session, size_t level) { - for (const auto& [id, transceiver] : session.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : session.GetTransceivers()) { if (transceiver->HasLevel() && transceiver->GetLevel() == level) { return transceiver->mTransport.mTransportId; } @@ -966,8 +952,7 @@ class JsepSessionTest : public JsepSessionTestBase, CandidateSet() {} void Gather(JsepSession& session, ComponentType maxComponent = RTCP) { - for (const auto& [id, transceiver] : session.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : session.GetTransceivers()) { if (transceiver->HasOwnTransport()) { Gather(session, transceiver->mTransport.mTransportId, RTP); if (transceiver->mTransport.mComponents > 1) { @@ -1199,8 +1184,7 @@ class JsepSessionTest : public JsepSessionTestBase, void CheckTransceiversAreBundled(const JsepSession& session, const std::string& context) { - for (const auto& [id, transceiver] : session.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : session.GetTransceivers()) { ASSERT_TRUE(transceiver->HasBundleLevel()) << context; ASSERT_EQ(0U, transceiver->BundleLevel()) << context; @@ -1357,8 +1341,7 @@ class JsepSessionTest : public JsepSessionTestBase, } void DumpTransceivers(const JsepSessionImpl& session) { - for (const auto& [id, transceiver] : session.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : session.GetTransceivers()) { std::cerr << "Transceiver "; if (transceiver->HasLevel()) { std::cerr << transceiver->GetLevel() << std::endl; @@ -1405,6 +1388,8 @@ class JsepSessionTest : public JsepSessionTestBase, std::vector types; std::vector> mGatheredCandidates; + FakeUuidGenerator mUuidGen; + private: void ValidateTransport(TransportData& source, const std::string& sdp_str, sdp::SdpType type) { @@ -1596,9 +1581,9 @@ TEST_P(JsepSessionTest, RenegotiationNoChange) { ValidateSetupAttribute(*mSessionOff, SdpSetupAttribute::kActpass); ValidateSetupAttribute(*mSessionAns, SdpSetupAttribute::kActive); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); std::string reoffer = CreateOffer(); @@ -1657,8 +1642,8 @@ TEST_P(JsepSessionTest, DISABLED_RenegotiationSwappedRolesNoChange) { ASSERT_TRUE(Equals(answererTransceivers, newOffererTransceivers)); } -static void RemoveLastN( - std::map>& aTransceivers, size_t aNum) { +static void RemoveLastN(std::vector>& aTransceivers, + size_t aNum) { while (aNum--) { // erase doesn't take reverse_iterator :( aTransceivers.erase(--aTransceivers.end()); @@ -1674,9 +1659,9 @@ TEST_P(JsepSessionTest, RenegotiationOffererAddsTrack) { ValidateSetupAttribute(*mSessionOff, SdpSetupAttribute::kActpass); ValidateSetupAttribute(*mSessionAns, SdpSetupAttribute::kActive); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); std::vector extraTypes; @@ -1711,9 +1696,9 @@ TEST_P(JsepSessionTest, RenegotiationAnswererAddsTrack) { ValidateSetupAttribute(*mSessionOff, SdpSetupAttribute::kActpass); ValidateSetupAttribute(*mSessionAns, SdpSetupAttribute::kActive); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); std::vector extraTypes; @@ -1723,10 +1708,12 @@ TEST_P(JsepSessionTest, RenegotiationAnswererAddsTrack) { types.insert(types.end(), extraTypes.begin(), extraTypes.end()); // We need to add a recvonly m-section to the offer for this to work - mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kAudio, SdpDirectionAttribute::Direction::kRecvonly)); - mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kVideo, SdpDirectionAttribute::Direction::kRecvonly)); + mSessionOff->AddTransceiver( + new JsepTransceiver(SdpMediaSection::kAudio, mUuidGen, + SdpDirectionAttribute::Direction::kRecvonly)); + mSessionOff->AddTransceiver( + new JsepTransceiver(SdpMediaSection::kVideo, mUuidGen, + SdpDirectionAttribute::Direction::kRecvonly)); std::string offer = CreateOffer(); SetLocalOffer(offer, CHECK_SUCCESS); @@ -1760,9 +1747,9 @@ TEST_P(JsepSessionTest, RenegotiationBothAddTrack) { ValidateSetupAttribute(*mSessionOff, SdpSetupAttribute::kActpass); ValidateSetupAttribute(*mSessionAns, SdpSetupAttribute::kActive); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); std::vector extraTypes; @@ -1923,12 +1910,12 @@ TEST_P(JsepSessionTest, RenegotiationOffererStopsTransceiver) { OfferAnswer(); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); - auto lastTransceiver = mSessionOff->GetTransceivers().rbegin()->second; + auto lastTransceiver = mSessionOff->GetTransceivers().back(); // Avoid bundle transport side effects; don't stop the BUNDLE-tag! lastTransceiver->Stop(); JsepTrack removedTrack(lastTransceiver->mSendTrack); @@ -1953,11 +1940,11 @@ TEST_P(JsepSessionTest, RenegotiationOffererStopsTransceiver) { ASSERT_EQ(origOffererTransceivers.size(), newOffererTransceivers.size()); - ASSERT_FALSE(origOffererTransceivers.rbegin()->second->IsStopped()); - ASSERT_TRUE(newOffererTransceivers.rbegin()->second->IsStopped()); + ASSERT_FALSE(origOffererTransceivers.back()->IsStopped()); + ASSERT_TRUE(newOffererTransceivers.back()->IsStopped()); - ASSERT_FALSE(origAnswererTransceivers.rbegin()->second->IsStopped()); - ASSERT_TRUE(newAnswererTransceivers.rbegin()->second->IsStopped()); + ASSERT_FALSE(origAnswererTransceivers.back()->IsStopped()); + ASSERT_TRUE(newAnswererTransceivers.back()->IsStopped()); RemoveLastN(origOffererTransceivers, 1); // Ignore this one RemoveLastN(newOffererTransceivers, 1); // Ignore this one RemoveLastN(origAnswererTransceivers, 1); // Ignore this one @@ -1976,15 +1963,14 @@ TEST_P(JsepSessionTest, RenegotiationAnswererStopsTransceiver) { OfferAnswer(); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); // Avoid bundle transport side effects; don't stop the BUNDLE-tag! - mSessionAns->GetTransceivers().rbegin()->second->Stop(); - JsepTrack removedTrack( - mSessionAns->GetTransceivers().rbegin()->second->mSendTrack); + mSessionAns->GetTransceivers().back()->Stop(); + JsepTrack removedTrack(mSessionAns->GetTransceivers().back()->mSendTrack); OfferAnswer(CHECK_SUCCESS); @@ -2007,10 +1993,10 @@ TEST_P(JsepSessionTest, RenegotiationAnswererStopsTransceiver) { ASSERT_EQ(origOffererTransceivers.size(), newOffererTransceivers.size()); - ASSERT_FALSE(origOffererTransceivers.rbegin()->second->IsStopped()); - ASSERT_TRUE(newOffererTransceivers.rbegin()->second->IsStopped()); - ASSERT_FALSE(origAnswererTransceivers.rbegin()->second->IsStopped()); - ASSERT_TRUE(newAnswererTransceivers.rbegin()->second->IsStopped()); + ASSERT_FALSE(origOffererTransceivers.back()->IsStopped()); + ASSERT_TRUE(newOffererTransceivers.back()->IsStopped()); + ASSERT_FALSE(origAnswererTransceivers.back()->IsStopped()); + ASSERT_TRUE(newAnswererTransceivers.back()->IsStopped()); RemoveLastN(origOffererTransceivers, 1); // Ignore this one RemoveLastN(newOffererTransceivers, 1); // Ignore this one RemoveLastN(origAnswererTransceivers, 1); // Ignore this one @@ -2029,18 +2015,18 @@ TEST_P(JsepSessionTest, RenegotiationBothStopSameTransceiver) { OfferAnswer(); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); // Avoid bundle transport side effects; don't stop the BUNDLE-tag! - mSessionOff->GetTransceivers().rbegin()->second->Stop(); + mSessionOff->GetTransceivers().back()->Stop(); JsepTrack removedTrackOffer( - mSessionOff->GetTransceivers().rbegin()->second->mSendTrack); - mSessionAns->GetTransceivers().rbegin()->second->Stop(); + mSessionOff->GetTransceivers().back()->mSendTrack); + mSessionAns->GetTransceivers().back()->Stop(); JsepTrack removedTrackAnswer( - mSessionAns->GetTransceivers().rbegin()->second->mSendTrack); + mSessionAns->GetTransceivers().back()->mSendTrack); OfferAnswer(CHECK_SUCCESS); @@ -2062,10 +2048,10 @@ TEST_P(JsepSessionTest, RenegotiationBothStopSameTransceiver) { ASSERT_EQ(origOffererTransceivers.size(), newOffererTransceivers.size()); - ASSERT_FALSE(origOffererTransceivers.rbegin()->second->IsStopped()); - ASSERT_TRUE(newOffererTransceivers.rbegin()->second->IsStopped()); - ASSERT_FALSE(origAnswererTransceivers.rbegin()->second->IsStopped()); - ASSERT_TRUE(newAnswererTransceivers.rbegin()->second->IsStopped()); + ASSERT_FALSE(origOffererTransceivers.back()->IsStopped()); + ASSERT_TRUE(newOffererTransceivers.back()->IsStopped()); + ASSERT_FALSE(origAnswererTransceivers.back()->IsStopped()); + ASSERT_TRUE(newAnswererTransceivers.back()->IsStopped()); RemoveLastN(origOffererTransceivers, 1); // Ignore this one RemoveLastN(newOffererTransceivers, 1); // Ignore this one RemoveLastN(origAnswererTransceivers, 1); // Ignore this one @@ -2087,18 +2073,18 @@ TEST_P(JsepSessionTest, RenegotiationBothStopTransceiverThenAddTrack) { OfferAnswer(); // Avoid bundle transport side effects; don't stop the BUNDLE-tag! - mSessionOff->GetTransceivers().rbegin()->second->Stop(); + mSessionOff->GetTransceivers().back()->Stop(); JsepTrack removedTrackOffer( - mSessionOff->GetTransceivers().rbegin()->second->mSendTrack); - mSessionOff->GetTransceivers().rbegin()->second->Stop(); + mSessionOff->GetTransceivers().back()->mSendTrack); + mSessionOff->GetTransceivers().back()->Stop(); JsepTrack removedTrackAnswer( - mSessionOff->GetTransceivers().rbegin()->second->mSendTrack); + mSessionOff->GetTransceivers().back()->mSendTrack); OfferAnswer(CHECK_SUCCESS); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); std::vector extraTypes; @@ -2117,11 +2103,11 @@ TEST_P(JsepSessionTest, RenegotiationBothStopTransceiverThenAddTrack) { newAnswererTransceivers.size()); // Ensure that the m-section was re-used; no gaps - ASSERT_EQ(origOffererTransceivers.rbegin()->second->GetLevel(), - newOffererTransceivers.rbegin()->second->GetLevel()); + ASSERT_EQ(origOffererTransceivers.back()->GetLevel(), + newOffererTransceivers.back()->GetLevel()); - ASSERT_EQ(origAnswererTransceivers.rbegin()->second->GetLevel(), - newAnswererTransceivers.rbegin()->second->GetLevel()); + ASSERT_EQ(origAnswererTransceivers.back()->GetLevel(), + newAnswererTransceivers.back()->GetLevel()); } TEST_P(JsepSessionTest, RenegotiationBothStopTransceiverDifferentMsection) { @@ -2204,9 +2190,9 @@ TEST_P(JsepSessionTest, RenegotiationAutoAssignedMsidIsStable) { SetRemoteAnswer(answer, CHECK_SUCCESS); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); ASSERT_EQ(origOffererTransceivers.size(), origAnswererTransceivers.size()); @@ -2241,8 +2227,7 @@ TEST_P(JsepSessionTest, RenegotiationOffererDisablesTelephoneEvent) { // check all the audio tracks to make sure they have 2 codecs (109 and 101), // and dtmf is enabled on all audio tracks std::vector tracks; - for (const auto& [id, transceiver] : mSessionOff->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionOff->GetTransceivers()) { tracks.push_back(transceiver->mSendTrack); tracks.push_back(transceiver->mRecvTrack); } @@ -2281,8 +2266,7 @@ TEST_P(JsepSessionTest, RenegotiationOffererDisablesTelephoneEvent) { // check all the audio tracks to make sure they have 1 codec (109), // and dtmf is disabled on all audio tracks tracks.clear(); - for (const auto& [id, transceiver] : mSessionOff->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionOff->GetTransceivers()) { tracks.push_back(transceiver->mSendTrack); tracks.push_back(transceiver->mRecvTrack); } @@ -2320,9 +2304,9 @@ TEST_P(JsepSessionTest, RenegotiationAnswererEnablesMsid) { SetRemoteAnswer(answer, CHECK_SUCCESS); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); offer = CreateOffer(); @@ -2366,9 +2350,9 @@ TEST_P(JsepSessionTest, RenegotiationAnswererDisablesMsid) { SetLocalAnswer(answer); SetRemoteAnswer(answer, CHECK_SUCCESS); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); offer = CreateOffer(); @@ -2426,9 +2410,9 @@ TEST_P(JsepSessionTest, RenegotiationOffererEnablesBundle) { SetLocalAnswer(answer); SetRemoteAnswer(answer); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); OfferAnswer(); @@ -2473,9 +2457,9 @@ TEST_P(JsepSessionTest, RenegotiationOffererDisablesBundleTransport) { GetTransceiverByLevel(*mSessionOff, 0)->Stop(); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); OfferAnswer(CHECK_SUCCESS); @@ -2524,9 +2508,9 @@ TEST_P(JsepSessionTest, RenegotiationAnswererDisablesBundleTransport) { OfferAnswer(); - std::map> origOffererTransceivers = + std::vector> origOffererTransceivers = DeepCopy(mSessionOff->GetTransceivers()); - std::map> origAnswererTransceivers = + std::vector> origAnswererTransceivers = DeepCopy(mSessionAns->GetTransceivers()); GetTransceiverByLevel(*mSessionAns, 0)->Stop(); @@ -3035,11 +3019,11 @@ INSTANTIATE_TEST_SUITE_P( TEST_F(JsepSessionTest, OfferAnswerRecvOnlyLines) { mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kAudio, SdpDirectionAttribute::kRecvonly)); + SdpMediaSection::kAudio, mUuidGen, SdpDirectionAttribute::kRecvonly)); mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kVideo, SdpDirectionAttribute::kRecvonly)); + SdpMediaSection::kVideo, mUuidGen, SdpDirectionAttribute::kRecvonly)); mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kVideo, SdpDirectionAttribute::kRecvonly)); + SdpMediaSection::kVideo, mUuidGen, SdpDirectionAttribute::kRecvonly)); std::string offer = CreateOffer(); UniquePtr parsedOffer(Parse(offer)); @@ -3099,11 +3083,10 @@ TEST_F(JsepSessionTest, OfferAnswerRecvOnlyLines) { SetLocalAnswer(answer, CHECK_SUCCESS); SetRemoteAnswer(answer, CHECK_SUCCESS); - std::map> transceivers( + std::vector> transceivers( mSessionOff->GetTransceivers()); ASSERT_EQ(3U, transceivers.size()); - for (const auto& [id, transceiver] : transceivers) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : transceivers) { const auto& msection = parsedOffer->GetMediaSection(transceiver->GetLevel()); const auto& ssrcs = msection.GetAttributeList().GetSsrc().mSsrcs; @@ -3167,7 +3150,7 @@ TEST_F(JsepSessionTest, OfferAnswerSendOnlyLines) { TEST_F(JsepSessionTest, OfferToReceiveAudioNotUsed) { mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kAudio, SdpDirectionAttribute::kRecvonly)); + SdpMediaSection::kAudio, mUuidGen, SdpDirectionAttribute::kRecvonly)); OfferAnswer(CHECK_SUCCESS); @@ -3190,7 +3173,7 @@ TEST_F(JsepSessionTest, OfferToReceiveAudioNotUsed) { TEST_F(JsepSessionTest, OfferToReceiveVideoNotUsed) { mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kVideo, SdpDirectionAttribute::kRecvonly)); + SdpMediaSection::kVideo, mUuidGen, SdpDirectionAttribute::kRecvonly)); OfferAnswer(CHECK_SUCCESS); @@ -3212,12 +3195,14 @@ TEST_F(JsepSessionTest, OfferToReceiveVideoNotUsed) { } TEST_F(JsepSessionTest, CreateOfferNoDatachannelDefault) { - RefPtr audio(new JsepTransceiver(SdpMediaSection::kAudio)); + RefPtr audio( + new JsepTransceiver(SdpMediaSection::kAudio, mUuidGen)); audio->mSendTrack.UpdateStreamIds( std::vector(1, "offerer_stream")); mSessionOff->AddTransceiver(audio); - RefPtr video(new JsepTransceiver(SdpMediaSection::kVideo)); + RefPtr video( + new JsepTransceiver(SdpMediaSection::kVideo, mUuidGen)); video->mSendTrack.UpdateStreamIds( std::vector(1, "offerer_stream")); mSessionOff->AddTransceiver(video); @@ -3238,12 +3223,14 @@ TEST_F(JsepSessionTest, ValidateOfferedVideoCodecParams) { types.push_back(SdpMediaSection::kAudio); types.push_back(SdpMediaSection::kVideo); - RefPtr audio(new JsepTransceiver(SdpMediaSection::kAudio)); + RefPtr audio( + new JsepTransceiver(SdpMediaSection::kAudio, mUuidGen)); audio->mSendTrack.UpdateStreamIds( std::vector(1, "offerer_stream")); mSessionOff->AddTransceiver(audio); - RefPtr video(new JsepTransceiver(SdpMediaSection::kVideo)); + RefPtr video( + new JsepTransceiver(SdpMediaSection::kVideo, mUuidGen)); video->mSendTrack.UpdateStreamIds( std::vector(1, "offerer_stream")); mSessionOff->AddTransceiver(video); @@ -3428,12 +3415,14 @@ TEST_F(JsepSessionTest, ValidateOfferedAudioCodecParams) { types.push_back(SdpMediaSection::kAudio); types.push_back(SdpMediaSection::kVideo); - RefPtr audio(new JsepTransceiver(SdpMediaSection::kAudio)); + RefPtr audio( + new JsepTransceiver(SdpMediaSection::kAudio, mUuidGen)); audio->mSendTrack.UpdateStreamIds( std::vector(1, "offerer_stream")); mSessionOff->AddTransceiver(audio); - RefPtr video(new JsepTransceiver(SdpMediaSection::kVideo)); + RefPtr video( + new JsepTransceiver(SdpMediaSection::kVideo, mUuidGen)); video->mSendTrack.UpdateStreamIds( std::vector(1, "offerer_stream")); mSessionOff->AddTransceiver(video); @@ -5106,8 +5095,7 @@ TEST_P(JsepSessionTest, TestRejectOfferRollback) { ASSERT_FALSE( mSessionAns->SetRemoteDescription(kJsepSdpRollback, "").mError.isSome()); ASSERT_EQ(kJsepStateStable, mSessionAns->GetState()); - for (const auto& [id, transceiver] : mSessionAns->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionAns->GetTransceivers()) { ASSERT_EQ(0U, transceiver->mRecvTrack.GetStreamIds().size()); } @@ -5160,8 +5148,7 @@ TEST_P(JsepSessionTest, TestInvalidRollback) { size_t GetActiveTransportCount(const JsepSession& session) { size_t activeTransportCount = 0; - for (const auto& [id, transceiver] : session.GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : session.GetTransceivers()) { if (!transceiver->HasBundleLevel() || (transceiver->BundleLevel() == transceiver->GetLevel())) { activeTransportCount += transceiver->mTransport.mComponents; @@ -5225,8 +5212,7 @@ TEST_P(JsepSessionTest, TestMaxBundle) { } SetLocalOffer(offer); - for (const auto& [id, transceiver] : mSessionOff->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionOff->GetTransceivers()) { if (transceiver->GetLevel() == 0) { // We do not set the bundle-level in have-local-offer unless the // m-section is bundle-only. @@ -6663,7 +6649,7 @@ TEST_F(JsepSessionTest, NoAddTrackMagicReplaceTrack) { ASSERT_EQ(2U, mSessionAns->GetTransceivers().size()); AddTracks(*mSessionOff, "audio"); mSessionAns->AddTransceiver( - new JsepTransceiver(SdpMediaSection::MediaType::kAudio)); + new JsepTransceiver(SdpMediaSection::MediaType::kAudio, mUuidGen)); mSessionAns->GetTransceivers()[2]->mSendTrack.UpdateStreamIds({"newstream"}); @@ -6710,7 +6696,7 @@ TEST_F(JsepSessionTest, AddTrackMakesTransceiverMagical) { ASSERT_EQ(2U, mSessionAns->GetTransceivers().size()); AddTracks(*mSessionOff, "audio"); mSessionAns->AddTransceiver( - new JsepTransceiver(SdpMediaSection::MediaType::kAudio)); + new JsepTransceiver(SdpMediaSection::MediaType::kAudio, mUuidGen)); ASSERT_EQ(3U, mSessionAns->GetTransceivers().size()); ASSERT_EQ(0U, mSessionAns->GetTransceivers()[0]->GetLevel()); @@ -6810,7 +6796,7 @@ TEST_F(JsepSessionTest, ComplicatedRemoteRollback) { SdpDirectionAttribute::Direction::kRecvonly; // We do nothing with the second audio transceiver; when we rollback, it will - // disappear entirely. + // be marked as removed. // This will not cause the third audio transceiver to stick around; having a // track is _not_ enough to preserve it. It must have addTrack "magic"! @@ -6819,15 +6805,14 @@ TEST_F(JsepSessionTest, ComplicatedRemoteRollback) { // Create a fourth audio transceiver. Rollback will leave it alone, since we // created it. mSessionAns->AddTransceiver( - new JsepTransceiver(SdpMediaSection::MediaType::kAudio, + new JsepTransceiver(SdpMediaSection::MediaType::kAudio, mUuidGen, SdpDirectionAttribute::Direction::kRecvonly)); ASSERT_FALSE( mSessionAns->SetRemoteDescription(kJsepSdpRollback, "").mError.isSome()); - // Three recvonly for audio, one sendrecv for video, and one (unmapped) for - // the second video track. - ASSERT_EQ(4U, mSessionAns->GetTransceivers().size()); + // Two of these (3 and 4) will be marked removed, if this all worked + ASSERT_EQ(6U, mSessionAns->GetTransceivers().size()); // First video transceiver ASSERT_FALSE(mSessionAns->GetTransceivers()[0]->HasLevel()); @@ -6835,6 +6820,7 @@ TEST_F(JsepSessionTest, ComplicatedRemoteRollback) { ASSERT_FALSE(mSessionAns->GetTransceivers()[0]->IsAssociated()); ASSERT_TRUE(mSessionAns->GetTransceivers()[0]->HasAddTrackMagic()); ASSERT_FALSE(IsNull(mSessionAns->GetTransceivers()[0]->mSendTrack)); + ASSERT_FALSE(mSessionAns->GetTransceivers()[0]->IsRemoved()); // Second video transceiver ASSERT_FALSE(mSessionAns->GetTransceivers()[1]->HasLevel()); @@ -6842,6 +6828,7 @@ TEST_F(JsepSessionTest, ComplicatedRemoteRollback) { ASSERT_FALSE(mSessionAns->GetTransceivers()[1]->IsAssociated()); ASSERT_TRUE(mSessionAns->GetTransceivers()[1]->HasAddTrackMagic()); ASSERT_FALSE(IsNull(mSessionAns->GetTransceivers()[1]->mSendTrack)); + ASSERT_FALSE(mSessionAns->GetTransceivers()[1]->IsRemoved()); // First audio transceiver, kept because AddTrack touched it, even though we // removed the send track after. @@ -6850,10 +6837,23 @@ TEST_F(JsepSessionTest, ComplicatedRemoteRollback) { ASSERT_FALSE(mSessionAns->GetTransceivers()[2]->IsAssociated()); ASSERT_TRUE(mSessionAns->GetTransceivers()[2]->HasAddTrackMagic()); ASSERT_TRUE(IsNull(mSessionAns->GetTransceivers()[2]->mSendTrack)); + ASSERT_FALSE(mSessionAns->GetTransceivers()[2]->IsRemoved()); // Second audio transceiver should be gone. + ASSERT_FALSE(mSessionAns->GetTransceivers()[3]->HasLevel()); + ASSERT_TRUE(mSessionAns->GetTransceivers()[3]->IsStopped()); + ASSERT_FALSE(mSessionAns->GetTransceivers()[3]->IsAssociated()); + ASSERT_FALSE(mSessionAns->GetTransceivers()[3]->HasAddTrackMagic()); + ASSERT_TRUE(IsNull(mSessionAns->GetTransceivers()[3]->mSendTrack)); + ASSERT_TRUE(mSessionAns->GetTransceivers()[3]->IsRemoved()); // Third audio transceiver should also be gone. + ASSERT_FALSE(mSessionAns->GetTransceivers()[4]->HasLevel()); + ASSERT_TRUE(mSessionAns->GetTransceivers()[4]->IsStopped()); + ASSERT_FALSE(mSessionAns->GetTransceivers()[4]->IsAssociated()); + ASSERT_FALSE(mSessionAns->GetTransceivers()[4]->HasAddTrackMagic()); + ASSERT_FALSE(IsNull(mSessionAns->GetTransceivers()[4]->mSendTrack)); + ASSERT_TRUE(mSessionAns->GetTransceivers()[4]->IsRemoved()); // Fourth audio transceiver, created after SetRemote ASSERT_FALSE(mSessionAns->GetTransceivers()[5]->HasLevel()); @@ -7126,7 +7126,7 @@ TEST_F(JsepSessionTest, TestOneWayRtx) { TEST_F(JsepSessionTest, TestRtxNoSsrcGroup) { mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kVideo, SdpDirectionAttribute::kRecvonly)); + SdpMediaSection::kVideo, mUuidGen, SdpDirectionAttribute::kRecvonly)); OfferAnswer(CHECK_SUCCESS); @@ -7140,7 +7140,7 @@ TEST_F(JsepSessionTest, TestRtxNoSsrcGroup) { TEST_F(JsepSessionTest, TestRtxSsrcGroupOnlyOffered) { mSessionOff->AddTransceiver(new JsepTransceiver( - SdpMediaSection::kVideo, SdpDirectionAttribute::kSendonly)); + SdpMediaSection::kVideo, mUuidGen, SdpDirectionAttribute::kSendonly)); OfferAnswer(CHECK_SUCCESS); @@ -7166,8 +7166,7 @@ TEST_F(JsepSessionTest, TestOfferRtxNoMsid) { AddTracks(*mSessionOff, "video"); std::vector streamIds; - for (const auto& [id, transceiver] : mSessionOff->GetTransceivers()) { - (void)id; // Lame, but no better way to do this right now. + for (const auto& transceiver : mSessionOff->GetTransceivers()) { if (!IsNull(transceiver->mSendTrack)) { transceiver->mSendTrack.UpdateStreamIds(streamIds); }