diff --git a/media/webrtc/signaling/gtest/jsep_track_unittest.cpp b/media/webrtc/signaling/gtest/jsep_track_unittest.cpp index 5a1841c88eeb..4377d00fa2ed 100644 --- a/media/webrtc/signaling/gtest/jsep_track_unittest.cpp +++ b/media/webrtc/signaling/gtest/jsep_track_unittest.cpp @@ -159,8 +159,8 @@ class JsepTrackTest : public ::testing::Test void CreateOffer() { - mSendOff.AddToOffer(mSsrcGenerator, &GetOffer()); - mRecvOff.AddToOffer(mSsrcGenerator, &GetOffer()); + mSendOff.AddToOffer(mSsrcGenerator, true, &GetOffer()); + mRecvOff.AddToOffer(mSsrcGenerator, true, &GetOffer()); } void CreateAnswer() @@ -169,8 +169,8 @@ class JsepTrackTest : public ::testing::Test mRecvAns.UpdateRecvTrack(*mOffer, GetOffer()); } - mSendAns.AddToAnswer(GetOffer(), mSsrcGenerator, &GetAnswer()); - mRecvAns.AddToAnswer(GetOffer(), mSsrcGenerator, &GetAnswer()); + mSendAns.AddToAnswer(GetOffer(), mSsrcGenerator, true, &GetAnswer()); + mRecvAns.AddToAnswer(GetOffer(), mSsrcGenerator, true, &GetAnswer()); } void Negotiate() diff --git a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp index 83ce9d84ff23..f2e60a4ca786 100644 --- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp +++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp @@ -91,6 +91,9 @@ JsepSessionImpl::Init() mRunSdpComparer = Preferences::GetBool("media.peerconnection.sdp.rust.compare", false); + mEncodeTrackId = + Preferences::GetBool("media.peerconnection.sdp.encode_track_id", true); + mIceUfrag = GetRandomHex(1); mIcePwd = GetRandomHex(4); return NS_OK; @@ -281,8 +284,8 @@ JsepSessionImpl::CreateOfferMsection(const JsepOfferOptions& options, nsresult rv = AddTransportAttributes(msection, SdpSetupAttribute::kActpass); NS_ENSURE_SUCCESS(rv, rv); - transceiver.mSendTrack.AddToOffer(mSsrcGenerator, msection); - transceiver.mRecvTrack.AddToOffer(mSsrcGenerator, msection); + transceiver.mSendTrack.AddToOffer(mSsrcGenerator, mEncodeTrackId, msection); + transceiver.mRecvTrack.AddToOffer(mSsrcGenerator, mEncodeTrackId, msection); AddExtmap(msection); @@ -672,8 +675,8 @@ JsepSessionImpl::CreateAnswerMsection(const JsepAnswerOptions& options, rv = AddTransportAttributes(&msection, role); NS_ENSURE_SUCCESS(rv, rv); - transceiver.mSendTrack.AddToAnswer(remoteMsection, mSsrcGenerator, &msection); - transceiver.mRecvTrack.AddToAnswer(remoteMsection, mSsrcGenerator, &msection); + transceiver.mSendTrack.AddToAnswer(remoteMsection, mSsrcGenerator, mEncodeTrackId, &msection); + transceiver.mRecvTrack.AddToAnswer(remoteMsection, mSsrcGenerator, mEncodeTrackId, &msection); // Add extmap attributes. This logic will probably be moved to the track, // since it can be specified on a per-sender basis in JS. diff --git a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h index e65ff463e8a3..c8e56f420299 100644 --- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h +++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h @@ -44,7 +44,8 @@ public: mUuidGen(std::move(uuidgen)), mSdpHelper(&mLastError), mRunRustParser(false), - mRunSdpComparer(false) + mRunSdpComparer(false), + mEncodeTrackId(true) { } @@ -298,6 +299,7 @@ private: SsrcGenerator mSsrcGenerator; bool mRunRustParser; bool mRunSdpComparer; + bool mEncodeTrackId; RsdparsaSdpParser mRsdparsaParser; }; diff --git a/media/webrtc/signaling/src/jsep/JsepTrack.cpp b/media/webrtc/signaling/src/jsep/JsepTrack.cpp index 9355a8431ffd..17fd7c500eee 100644 --- a/media/webrtc/signaling/src/jsep/JsepTrack.cpp +++ b/media/webrtc/signaling/src/jsep/JsepTrack.cpp @@ -116,9 +116,11 @@ JsepTrack::PopulateCodecs( } void -JsepTrack::AddToOffer(SsrcGenerator& ssrcGenerator, SdpMediaSection* offer) +JsepTrack::AddToOffer(SsrcGenerator& ssrcGenerator, + bool encodeTrackId, + SdpMediaSection* offer) { - AddToMsection(mPrototypeCodecs, offer); + AddToMsection(mPrototypeCodecs, encodeTrackId, offer); if (mDirection == sdp::kSend) { std::vector constraints; @@ -132,6 +134,7 @@ JsepTrack::AddToOffer(SsrcGenerator& ssrcGenerator, SdpMediaSection* offer) void JsepTrack::AddToAnswer(const SdpMediaSection& offer, SsrcGenerator& ssrcGenerator, + bool encodeTrackId, SdpMediaSection* answer) { // We do not modify mPrototypeCodecs here, since we're only creating an @@ -143,7 +146,7 @@ JsepTrack::AddToAnswer(const SdpMediaSection& offer, return; } - AddToMsection(codecs, answer); + AddToMsection(codecs, encodeTrackId, answer); if (mDirection == sdp::kSend) { std::vector constraints; @@ -185,6 +188,7 @@ JsepTrack::SetJsConstraints( void JsepTrack::AddToMsection( const std::vector>& codecs, + bool encodeTrackId, SdpMediaSection* msection) { MOZ_ASSERT(msection->GetMediaType() == mType); @@ -198,10 +202,10 @@ JsepTrack::AddToMsection( (mType != SdpMediaSection::kApplication) && msection->IsSending()) { if (mStreamIds.empty()) { - msection->AddMsid("-", mTrackId); + msection->AddMsid("-", encodeTrackId ? mTrackId : ""); } else { for (const std::string& streamId : mStreamIds) { - msection->AddMsid(streamId, mTrackId); + msection->AddMsid(streamId, encodeTrackId ? mTrackId : ""); // TODO(bug 1402912) Interop hack; older Firefox barfs if there is more // than one msid. Remove when safe. break; diff --git a/media/webrtc/signaling/src/jsep/JsepTrack.h b/media/webrtc/signaling/src/jsep/JsepTrack.h index 143a80294f3d..c356324ddcae 100644 --- a/media/webrtc/signaling/src/jsep/JsepTrack.h +++ b/media/webrtc/signaling/src/jsep/JsepTrack.h @@ -258,9 +258,11 @@ public: // These two are non-const because this is where ssrcs are chosen. virtual void AddToOffer(SsrcGenerator& ssrcGenerator, + bool encodeTrackId, SdpMediaSection* offer); virtual void AddToAnswer(const SdpMediaSection& offer, SsrcGenerator& ssrcGenerator, + bool encodeTrackId, SdpMediaSection* answer); virtual void Negotiate(const SdpMediaSection& answer, @@ -329,6 +331,7 @@ private: static void EnsurePayloadTypeIsUnique(std::set* uniquePayloadTypes, JsepCodecDescription* codec); void AddToMsection(const std::vector>& codecs, + bool encodeTrackId, SdpMediaSection* msection); void GetRids(const SdpMediaSection& msection, sdp::Direction direction, diff --git a/media/webrtc/signaling/src/sdp/SdpHelper.cpp b/media/webrtc/signaling/src/sdp/SdpHelper.cpp index 25fefe041c23..85cbffac658e 100644 --- a/media/webrtc/signaling/src/sdp/SdpHelper.cpp +++ b/media/webrtc/signaling/src/sdp/SdpHelper.cpp @@ -400,11 +400,6 @@ SdpHelper::GetIdsFromMsid(const Sdp& sdp, for (auto i = allMsids.begin(); i != allMsids.end(); ++i) { if (allMsidsAreWebrtc || webrtcMsids.count(i->identifier)) { - if (i->appdata.empty()) { - SDP_SET_ERROR("Invalid webrtc msid at level " << msection.GetLevel() - << ": Missing track id."); - return NS_ERROR_INVALID_ARG; - } if (!found) { *trackId = i->appdata; streamIds->clear(); diff --git a/testing/web-platform/tests/webrtc/RTCRtpTransceiver.https.html b/testing/web-platform/tests/webrtc/RTCRtpTransceiver.https.html index 860f5de2d2ac..3678f967ce0f 100644 --- a/testing/web-platform/tests/webrtc/RTCRtpTransceiver.https.html +++ b/testing/web-platform/tests/webrtc/RTCRtpTransceiver.https.html @@ -340,12 +340,35 @@ ]); const offer = await pc.createOffer(); - assert_true(offer.sdp.includes("a=msid:" + audioStream.id + " " + audio.id), + assert_true(offer.sdp.includes("a=msid:" + audioStream.id), "offer contains the expected audio msid"); - assert_true(offer.sdp.includes("a=msid:" + videoStream.id + " " + video.id), + assert_true(offer.sdp.includes("a=msid:" + videoStream.id), "offer contains the expected video msid"); }; + const checkMsidNoTrackId = async t => { + const pc1 = new RTCPeerConnection(); + const pc2 = new RTCPeerConnection(); + t.add_cleanup(() => pc1.close()); + t.add_cleanup(() => pc2.close()); + const stream = await navigator.mediaDevices.getUserMedia({audio: true}); + t.add_cleanup(() => stopTracks(stream)); + const track = stream.getAudioTracks()[0]; + pc1.addTrack(track, stream); + + const offer = await pc1.createOffer(); + await pc1.setLocalDescription(offer); + + // Remove track-id from msid + offer.sdp = offer.sdp.replace(/(a=msid:[^ \t]+).*\r\n/g, "$1\r\n"); + assert_true(offer.sdp.includes(`a=msid:${stream.id}\r\n`)); + await pc2.setRemoteDescription(offer); + + const answer = await pc2.createAnswer(); + await pc1.setRemoteDescription(answer); + await pc2.setLocalDescription(answer); + }; + const checkAddTransceiverWithOfferToReceive = async (t, kinds) => { const pc = new RTCPeerConnection(); t.add_cleanup(() => pc.close()); @@ -2313,6 +2336,7 @@ const tests = [ checkAddTransceiverWithAddTrack, checkAddTransceiverWithDirection, checkAddTransceiverWithStream, + checkMsidNoTrackId, function checkAddTransceiverWithOfferToReceiveAudio(t) { return checkAddTransceiverWithOfferToReceive(t, ["audio"]); },