From c2e51c86187f1beaee0d5653f6a8eedc5c63fc94 Mon Sep 17 00:00:00 2001 From: Byron Campen Date: Fri, 5 Aug 2022 14:03:43 +0000 Subject: [PATCH] Bug 1769802: Key JsepTransceiver by UUID instead of a simple integer index. r=mjf The integer index we're replacing here is based on the order in which transceivers were added. If we clone the JSEP engine for an sRD that happens to result in the creation of a transceiver, and at the same time JS calls addTransceiver, we have a situation where the sRD transceiver is added first to the cloned JSEP engine, but the addTransceiver transceiver is added first to the old JSEP engine, resulting in them having the same index. So, let's just use a proper key for this stuff. Differential Revision: https://phabricator.services.mozilla.com/D150169 --- dom/media/webrtc/jsapi/PeerConnectionImpl.cpp | 82 +++-- dom/media/webrtc/jsapi/PeerConnectionImpl.h | 5 +- dom/media/webrtc/jsapi/RTCRtpTransceiver.cpp | 6 +- dom/media/webrtc/jsapi/RTCRtpTransceiver.h | 22 +- dom/media/webrtc/jsep/JsepSession.h | 22 +- dom/media/webrtc/jsep/JsepSessionImpl.cpp | 76 ++--- dom/media/webrtc/jsep/JsepSessionImpl.h | 18 +- dom/media/webrtc/jsep/JsepTransceiver.h | 18 +- .../signaling/gtest/jsep_session_unittest.cpp | 317 +++++++++--------- 9 files changed, 293 insertions(+), 273 deletions(-) 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); }