diff --git a/dom/media/DOMMediaStream.cpp b/dom/media/DOMMediaStream.cpp index c4f71e09f48..4a5217848a3 100644 --- a/dom/media/DOMMediaStream.cpp +++ b/dom/media/DOMMediaStream.cpp @@ -5,6 +5,7 @@ #include "DOMMediaStream.h" #include "nsContentUtils.h" +#include "nsIUUIDGenerator.h" #include "mozilla/dom/MediaStreamBinding.h" #include "mozilla/dom/LocalMediaStreamBinding.h" #include "mozilla/dom/AudioNode.h" @@ -146,6 +147,20 @@ DOMMediaStream::DOMMediaStream() mStream(nullptr), mHintContents(0), mTrackTypesAvailable(0), mNotifiedOfMediaStreamGraphShutdown(false), mCORSMode(CORS_NONE) { + nsresult rv; + nsCOMPtr uuidgen = + do_GetService("@mozilla.org/uuid-generator;1", &rv); + + if (NS_SUCCEEDED(rv) && uuidgen) { + nsID uuid; + memset(&uuid, 0, sizeof(uuid)); + rv = uuidgen->GenerateUUIDInPlace(&uuid); + if (NS_SUCCEEDED(rv)) { + char buffer[NSID_LENGTH]; + uuid.ToProvidedString(buffer); + mID = NS_ConvertASCIItoUTF16(buffer); + } + } } DOMMediaStream::~DOMMediaStream() diff --git a/dom/media/DOMMediaStream.h b/dom/media/DOMMediaStream.h index 21ab01de185..d48ea63e6a7 100644 --- a/dom/media/DOMMediaStream.h +++ b/dom/media/DOMMediaStream.h @@ -180,6 +180,10 @@ public: */ void NotifyStreamStateChanged(); + // Webrtc allows the remote side to name a stream whatever it wants, and we + // need to surface this to content. + void AssignId(const nsAString& aID) { mID = aID; } + // Indicate what track types we eventually expect to add to this stream enum { HINT_CONTENTS_AUDIO = 1 << 0, diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp index 17e3fad5e78..aac951094db 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp @@ -200,14 +200,11 @@ public: return; } - CSFLogInfo(logTag, "Returning success for OnAddStream()"); - // We are running on main thread here so we shouldn't have a race - // on this callback - nsTArray> tracks; aStream->GetTracks(tracks); - bool newStream = true; + std::string streamId = PeerConnectionImpl::GetStreamId(*aStream); + bool notifyStream = true; for (size_t i = 0; i < tracks.Length(); i++) { std::string trackId; @@ -215,7 +212,7 @@ public: // track. It would be nice if we could specify this along with the numeric // track id from the start, but we're stuck doing this fixup after the // fact. - nsresult rv = wrapper.impl()->GetRemoteTrackId(aStream, + nsresult rv = wrapper.impl()->GetRemoteTrackId(streamId, tracks[i]->GetTrackID(), &trackId); @@ -232,16 +229,15 @@ public: if (origTrackId == trackId) { // Pre-existing track - newStream = false; + notifyStream = false; continue; } tracks[i]->AssignId(NS_ConvertUTF8toUTF16(trackId.c_str())); JSErrorResult jrv; + CSFLogInfo(logTag, "Calling OnAddTrack(%s)", trackId.c_str()); mObserver->OnAddTrack(*tracks[i], jrv); - CSFLogInfo(logTag, "Calling OnAddTrack(%s)", - PeerConnectionImpl::GetTrackId(*tracks[i]).c_str()); if (jrv.Failed()) { CSFLogError(logTag, ": OnAddTrack(%u) failed! Error: %u", static_cast(i), @@ -249,14 +245,14 @@ public: } } - if (newStream) { + if (notifyStream) { // Start currentTime from the point where this stream was successfully // returned. aStream->SetLogicalStreamStartTime( aStream->GetStream()->GetCurrentTime()); JSErrorResult rv; - CSFLogInfo(logTag, "Calling OnAddStream"); + CSFLogInfo(logTag, "Calling OnAddStream(%s)", streamId.c_str()); mObserver->OnAddStream(*aStream, rv); if (rv.Failed()) { CSFLogError(logTag, ": OnAddStream() failed! Error: %u", @@ -1663,6 +1659,12 @@ PeerConnectionImpl::SetRemoteDescription(int32_t action, const char* aSDP) return NS_OK; } CSFLogDebug(logTag, "Added remote stream %s", info->GetId().c_str()); + +#ifdef MOZILLA_INTERNAL_API + info->GetMediaStream()->AssignId(NS_ConvertUTF8toUTF16(streamId.c_str())); +#else + info->GetMediaStream()->AssignId((streamId)); +#endif } size_t numNewAudioTracks = 0; @@ -1929,7 +1931,7 @@ PeerConnectionImpl::PrincipalChanged(DOMMediaStream* aMediaStream) { #ifdef MOZILLA_INTERNAL_API nsresult -PeerConnectionImpl::GetRemoteTrackId(DOMMediaStream* mediaStream, +PeerConnectionImpl::GetRemoteTrackId(const std::string streamId, TrackID numericTrackId, std::string* trackId) const { @@ -1937,7 +1939,7 @@ PeerConnectionImpl::GetRemoteTrackId(DOMMediaStream* mediaStream, return NS_ERROR_UNEXPECTED; } - return mMedia->GetRemoteTrackId(mediaStream, numericTrackId, trackId); + return mMedia->GetRemoteTrackId(streamId, numericTrackId, trackId); } #endif @@ -1953,6 +1955,18 @@ PeerConnectionImpl::GetTrackId(const MediaStreamTrack& aTrack) #endif } +std::string +PeerConnectionImpl::GetStreamId(const DOMMediaStream& aStream) +{ +#ifdef MOZILLA_INTERNAL_API + nsString wideStreamId; + aStream.GetId(wideStreamId); + return NS_ConvertUTF16toUTF8(wideStreamId).get(); +#else + return aStream.GetId(); +#endif +} + nsresult PeerConnectionImpl::AddTrack(MediaStreamTrack& aTrack, const Sequence>& aStreams) @@ -1976,10 +1990,9 @@ PeerConnectionImpl::AddTrack(MediaStreamTrack& aTrack, } uint32_t num = mMedia->LocalStreamsLength(); - std::string streamId; + std::string streamId = PeerConnectionImpl::GetStreamId(aMediaStream); std::string trackId = PeerConnectionImpl::GetTrackId(aTrack); - // TODO(bug 1089798): streamId should really come from the MS. - nsresult res = mMedia->AddTrack(&aMediaStream, &streamId, trackId); + nsresult res = mMedia->AddTrack(&aMediaStream, streamId, trackId); if (NS_FAILED(res)) { return res; } @@ -2043,8 +2056,8 @@ PeerConnectionImpl::RemoveTrack(MediaStreamTrack& aTrack) { return NS_ERROR_INVALID_ARG; } - nsRefPtr info = - media()->GetLocalStreamByDomStream(*stream); + std::string streamId = PeerConnectionImpl::GetStreamId(*stream); + nsRefPtr info = media()->GetLocalStreamById(streamId); if (!info) { CSFLogError(logTag, "%s: Unknown stream", __FUNCTION__); @@ -2114,8 +2127,8 @@ PeerConnectionImpl::ReplaceTrack(MediaStreamTrack& aThisTrack, // TrackID thisID = aThisTrack.GetTrackID(); // - nsRefPtr info = - media()->GetLocalStreamByDomStream(aStream); + std::string streamId = PeerConnectionImpl::GetStreamId(aStream); + nsRefPtr info = media()->GetLocalStreamById(streamId); if (!info || !info->HasTrack(origTrackId)) { CSFLogError(logTag, "Track to replace (%s) was never added", diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h index ad31af2e0ef..e7c01ce1314 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h @@ -598,11 +598,12 @@ public: // PeerConnectionMedia can't do it because it doesn't know about principals virtual void PrincipalChanged(DOMMediaStream* aMediaStream) MOZ_OVERRIDE; - nsresult GetRemoteTrackId(DOMMediaStream* mediaStream, + nsresult GetRemoteTrackId(const std::string streamId, TrackID numericTrackId, std::string* trackId) const; #endif + static std::string GetStreamId(const DOMMediaStream& aStream); static std::string GetTrackId(const dom::MediaStreamTrack& track); private: diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp index 3b9917fe54b..a0b219a66d5 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp @@ -581,7 +581,7 @@ PeerConnectionMedia::UpdateIceMediaStream_s(size_t aMLine, nsresult PeerConnectionMedia::AddTrack(DOMMediaStream* aMediaStream, - std::string* streamId, + const std::string& streamId, const std::string& trackId) { ASSERT_ON_THREAD(mMainThread); @@ -594,21 +594,14 @@ PeerConnectionMedia::AddTrack(DOMMediaStream* aMediaStream, CSFLogDebug(logTag, "%s: MediaStream: %p", __FUNCTION__, aMediaStream); nsRefPtr localSourceStream = - GetLocalStreamByDomStream(*aMediaStream); + GetLocalStreamById(streamId); if (!localSourceStream) { - std::string id; - if (!mUuidGen->Generate(&id)) { - CSFLogError(logTag, "Failed to generate UUID for stream"); - return NS_ERROR_FAILURE; - } - - localSourceStream = new LocalSourceStreamInfo(aMediaStream, this, id); + localSourceStream = new LocalSourceStreamInfo(aMediaStream, this, streamId); mLocalSourceStreams.AppendElement(localSourceStream); } localSourceStream->AddTrack(trackId); - *streamId = localSourceStream->GetId(); return NS_OK; } @@ -621,20 +614,15 @@ PeerConnectionMedia::RemoveLocalTrack(const std::string& streamId, CSFLogDebug(logTag, "%s: stream: %s track: %s", __FUNCTION__, streamId.c_str(), trackId.c_str()); - size_t i; - for (i = 0; i < mLocalSourceStreams.Length(); ++i) { - if (mLocalSourceStreams[i]->GetId() == streamId) { - break; - } - } - - if (i == mLocalSourceStreams.Length()) { + nsRefPtr localSourceStream = + GetLocalStreamById(streamId); + if (!localSourceStream) { return NS_ERROR_ILLEGAL_VALUE; } - mLocalSourceStreams[i]->RemoveTrack(trackId); - if (!(mLocalSourceStreams[i]->GetTrackCount())) { - mLocalSourceStreams.RemoveElementAt(i); + localSourceStream->RemoveTrack(trackId); + if (!localSourceStream->GetTrackCount()) { + mLocalSourceStreams.RemoveElement(localSourceStream); } return NS_OK; } @@ -648,32 +636,27 @@ PeerConnectionMedia::RemoveRemoteTrack(const std::string& streamId, CSFLogDebug(logTag, "%s: stream: %s track: %s", __FUNCTION__, streamId.c_str(), trackId.c_str()); - size_t i; - for (i = 0; i < mRemoteSourceStreams.Length(); ++i) { - if (mRemoteSourceStreams[i]->GetId() == streamId) { - break; - } - } - - if (i == mRemoteSourceStreams.Length()) { + nsRefPtr remoteSourceStream = + GetRemoteStreamById(streamId); + if (!remoteSourceStream) { return NS_ERROR_ILLEGAL_VALUE; } - mRemoteSourceStreams[i]->RemoveTrack(trackId); - if (!(mRemoteSourceStreams[i]->GetTrackCount())) { - mRemoteSourceStreams.RemoveElementAt(i); + remoteSourceStream->RemoveTrack(trackId); + if (!remoteSourceStream->GetTrackCount()) { + mRemoteSourceStreams.RemoveElement(remoteSourceStream); } return NS_OK; } nsresult -PeerConnectionMedia::GetRemoteTrackId(DOMMediaStream* mediaStream, +PeerConnectionMedia::GetRemoteTrackId(const std::string streamId, TrackID numericTrackId, std::string* trackId) const { auto* ncThis = const_cast(this); const RemoteSourceStreamInfo* info = - ncThis->GetRemoteStreamByDomStream(*mediaStream); + ncThis->GetRemoteStreamById(streamId); if (!info) { CSFLogError(logTag, "%s: Could not find stream info", __FUNCTION__); @@ -776,35 +759,6 @@ PeerConnectionMedia::GetLocalStreamById(const std::string& id) } } - MOZ_ASSERT(false); - return nullptr; -} - -LocalSourceStreamInfo* -PeerConnectionMedia::GetLocalStreamByDomStream(const DOMMediaStream& stream) -{ - ASSERT_ON_THREAD(mMainThread); - for (size_t i = 0; i < mLocalSourceStreams.Length(); ++i) { - if (&stream == mLocalSourceStreams[i]->GetMediaStream()) { - return mLocalSourceStreams[i]; - } - } - - return nullptr; -} - -RemoteSourceStreamInfo* -PeerConnectionMedia::GetRemoteStreamByDomStream( - const DOMMediaStream& stream) -{ - ASSERT_ON_THREAD(mMainThread); - for (size_t i = 0; i < mRemoteSourceStreams.Length(); ++i) { - if (&stream == mRemoteSourceStreams[i]->GetMediaStream()) { - return mRemoteSourceStreams[i]; - } - } - - MOZ_ASSERT(false); return nullptr; } @@ -826,10 +780,6 @@ PeerConnectionMedia::GetRemoteStreamById(const std::string& id) } } - // This does not have a MOZ_ASSERT like GetLocalStreamById because in the - // case of local streams, the stream id and stream info are created - // simultaneously, whereas in the remote case the stream id exists first, - // meaning we have to be able to check. return nullptr; } diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h index e282196056b..014d4eb6254 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h @@ -241,10 +241,8 @@ class PeerConnectionMedia : public sigslot::has_slots<> { nsresult UpdateMediaPipelines(const JsepSession& session); // Add a track (main thread only) - // TODO(bug 1089798): Once DOMMediaStream has an id field, use it instead of - // letting PCMedia choose a streamId nsresult AddTrack(DOMMediaStream* aMediaStream, - std::string* streamId, + const std::string& streamId, const std::string& trackId); nsresult RemoveLocalTrack(const std::string& streamId, @@ -252,7 +250,7 @@ class PeerConnectionMedia : public sigslot::has_slots<> { nsresult RemoveRemoteTrack(const std::string& streamId, const std::string& trackId); - nsresult GetRemoteTrackId(DOMMediaStream* mediaStream, + nsresult GetRemoteTrackId(const std::string streamId, TrackID numericTrackId, std::string* trackId) const; @@ -263,8 +261,6 @@ class PeerConnectionMedia : public sigslot::has_slots<> { } LocalSourceStreamInfo* GetLocalStreamByIndex(int index); LocalSourceStreamInfo* GetLocalStreamById(const std::string& id); - LocalSourceStreamInfo* GetLocalStreamByDomStream( - const DOMMediaStream& stream); // Get a specific remote stream uint32_t RemoteStreamsLength() @@ -274,8 +270,6 @@ class PeerConnectionMedia : public sigslot::has_slots<> { RemoteSourceStreamInfo* GetRemoteStreamByIndex(size_t index); RemoteSourceStreamInfo* GetRemoteStreamById(const std::string& id); - RemoteSourceStreamInfo* GetRemoteStreamByDomStream( - const DOMMediaStream& stream); // Add a remote stream. nsresult AddRemoteStream(nsRefPtr aInfo); diff --git a/media/webrtc/signaling/test/FakeMediaStreams.h b/media/webrtc/signaling/test/FakeMediaStreams.h index 54eb755ecc6..a7afecbe1b2 100644 --- a/media/webrtc/signaling/test/FakeMediaStreams.h +++ b/media/webrtc/signaling/test/FakeMediaStreams.h @@ -285,6 +285,8 @@ public: virtual void RemoveDirectListener(Fake_MediaStreamListener *aListener) {} Fake_MediaStream *GetStream() { return mMediaStream; } + std::string GetId() const { return mID; } + void AssignId(const std::string& id) { mID = id; } // Hints to tell the SDP generator about whether this // MediaStream probably has audio and/or video @@ -343,6 +345,8 @@ private: uint32_t mHintContents; nsRefPtr mVideoTrack; nsRefPtr mAudioTrack; + + std::string mID; }; class Fake_MediaStreamBase : public Fake_MediaStream {