1531803 - RTCTrackEvent-fire.html wpt wants ontrack events when a=msid is altered in remote description

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect, P2)

Description

[Byron Campen [:bwc]]

This is pretty debatable, because the JSEP spec forbids a=msid changing, which implies an expectation that JS won't see new tracks unless a transceiver is added. On the other hand, it was not always this way, and existing JS might want to see the events.

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

As I recall, we deprecated msid for identification in favor of mid, removing the track id part that made them unique. Its remaining use: keeping track of stream associations, which can change with sender.setStreams() (bug 1510802), causing events. E.g. two tracks in the same stream now have the same msid.

I think this renders the following JSEP text obsolete: "For RtpTransceivers that are not stopped, the "a=msid" line(s) MUST stay the same if they are present in the current description, regardless of changes to the transceiver's direction or track."

If I didn't want to change JSEP, I'd narrowly interpret this as: "must not change when the track changes", and since we've removed the relevant part it was speaking to (the unique track id), we can ignore it.

Comment 2

[Byron Campen [:bwc]]

Just to be clear, your opinion is that this check should be removed?

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Sorry for not being clear: the check is correct and should stay. I meant "JSEP spec forbids a=msid changing" is obsolete.

MSID reflects stream associations, which can change according to webrtc-pc, and must fire events. From SRD(offer):

6. If direction is "sendrecv" or "recvonly", let msids be a list of the MSIDs that the media description indicates transceiver.[[Receiver]].[[ReceiverTrack]] is to be associated with. Otherwise, let msids be an empty list.

7. Set the associated remote streams given transceiver.[[Receiver]], msids, addList, and removeList.

8. If the previous step increased the length of addList, or if transceiver's [[FiredDirection]] slot is neither "sendrecv" nor "recvonly", process the addition of a remote track for the media description, given transceiver and trackEventInits.

Comment 4

[Byron Campen [:bwc]]

As much as I dislike the JSEP spec around this, I don't think your narrow interpretation holds up. The jsep draft says that a=msid is invariant until the transceiver is stopped, and this is purposeful. Definitely, we should not be changing our own a=msid attributes (unless draft-jsep is changed, which would require us to raise an almighty fuss at this point).

Let's bring this up on w3c.

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

The jsep draft says that a=msid is invariant until the transceiver is stopped, and this is purposeful.

What purpose does that solve when it is no longer unique? E.g. the typical case of a stream with an audio track and a video track, the tracks will both have the same msid now.

Let's bring this up on w3c.

Please file an issue on webrtc-pc or jsep if you think there is an issue here.

Comment 6

[Byron Campen [:bwc]]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #5)

The jsep draft says that a=msid is invariant until the transceiver is stopped, and this is purposeful.

What purpose does that solve when it is no longer unique? E.g. the typical case of a stream with an audio track and a video track, the tracks will both have the same msid now.

Exactly the question I asked here: https://github.com/w3c/webrtc-pc/issues/2115

Comment 7

[Byron Campen [:bwc]]

Ok, so it looks like the consensus is that JSEP is buggy here, and we're going to surface this to JS.

Comment 8

[Byron Campen [:bwc]]

Attachment: Bug 1531803: (WIP) Rip out most of the track-id business, and a=msid invariance.

Comment 9

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8412aa062e6e76deaedd3e33c6da1d4f2546bc3

Comment 10

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=716c91114a5af4cce32226f66ba62e8925218e6e

Comment 11

[Byron Campen [:bwc]]

Attachment: Bug 1531803 - Part 0: Re-enable a couple of tests. r?jib

Comment 12

[Byron Campen [:bwc]]

Attachment: Bug 1531803 - Part 1: Stop copying the previous msid when generating new SDP. r?mjf

Depends on D25793

Comment 13

[Byron Campen [:bwc]]

Attachment: Bug 1531803 - Part 2: Get rid of JsepTrack::GetTrackId (since it doesn't mean anything), and re-focus on stream ids. r?mjf

Depends on D25794

Comment 14

[Byron Campen [:bwc]]

Attachment: Bug 1531803 - Part 3: Only set track id on JsepTrack if we're configured to emit track ids in SDP, and simplify some code. r?mjf

Depends on D25795

Comment 15

[Byron Campen [:bwc]]

Attachment: Bug 1531803 - Part 4: Remove some track-id-centric ChromeOnly API on RTCRtpTransceiver. r?jib,smaug

Depends on D25796

Comment 16

[Byron Campen [:bwc]]

Attachment: Bug 1531803 - Part 5: Rework local track checking in our mochitests to ignore track ids, and decruft. r?jib

Depends on D25797

Comment 17

[Byron Campen [:bwc]]

Attachment: Bug 1531803 - Part 6: Fire track events when that track has been added to a stream. r?jib

Depends on D25798

Comment 18

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=640b95b5af737423107b194bd94bd4603f93547d

Comment 19

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=866ebe7df970a3430c9326d9c6877f0b4834e953

Comment 20

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=171974c931c59abb58d5973b6304c29b16d11ab8

Comment 21

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b6a2b8dd3db16ca213f545091318727e2ae6a0e

Comment 22

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ca63dfad1ede6a8d46bd19cf2016af8c2605281

Comment 23

[Byron Campen [:bwc]]

Wow, what happened on try there? Oranges in lots of unrelated tests... I think I'll rebase and see if it is less of a mess.

Comment 24

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfd3e0065a787dcabc6ab7441e183b1ef59762f8

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/86910675df13
Part 0: Re-enable a couple of tests. r=jib
https://hg.mozilla.org/integration/autoland/rev/cbe9386e0d7a
Part 1: Stop copying the previous msid when generating new SDP. r=mjf
https://hg.mozilla.org/integration/autoland/rev/14e1bfbd0060
Part 2: Get rid of JsepTrack::GetTrackId (since it doesn't mean anything), and re-focus on stream ids. r=mjf
https://hg.mozilla.org/integration/autoland/rev/f8a6a019f873
Part 3: Only set track id on JsepTrack if we're configured to emit track ids in SDP, and simplify some code. r=mjf
https://hg.mozilla.org/integration/autoland/rev/4864459fde54
Part 4: Remove some track-id-centric ChromeOnly API on RTCRtpTransceiver. r=jib,smaug
https://hg.mozilla.org/integration/autoland/rev/6bf3fd241bb8
Part 5: Rework local track checking in our mochitests to ignore track ids, and decruft. r=jib
https://hg.mozilla.org/integration/autoland/rev/d2ba7da778f0
Part 6: Fire track events when that track has been added to a stream. r=jib,smaug

Comment 26

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/86910675df13
https://hg.mozilla.org/mozilla-central/rev/cbe9386e0d7a
https://hg.mozilla.org/mozilla-central/rev/14e1bfbd0060
https://hg.mozilla.org/mozilla-central/rev/f8a6a019f873
https://hg.mozilla.org/mozilla-central/rev/4864459fde54
https://hg.mozilla.org/mozilla-central/rev/6bf3fd241bb8
https://hg.mozilla.org/mozilla-central/rev/d2ba7da778f0