Bug 1498793: Accept msid without track id r=mjf,jib

Differential Revision: https://phabricator.services.mozilla.com/D10925

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Byron Campen [:bwc] 2018-11-12 19:25:59 +00:00
parent 84d5b8eb27
commit 33bc9bd6b6
7 changed files with 52 additions and 21 deletions

View File

@ -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()

View File

@ -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.

View File

@ -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;
};

View File

@ -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<JsConstraints> 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<JsConstraints> constraints;
@ -185,6 +188,7 @@ JsepTrack::SetJsConstraints(
void
JsepTrack::AddToMsection(
const std::vector<UniquePtr<JsepCodecDescription>>& 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;

View File

@ -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<uint16_t>* uniquePayloadTypes,
JsepCodecDescription* codec);
void AddToMsection(const std::vector<UniquePtr<JsepCodecDescription>>& codecs,
bool encodeTrackId,
SdpMediaSection* msection);
void GetRids(const SdpMediaSection& msection,
sdp::Direction direction,

View File

@ -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();

View File

@ -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"]);
},