New issue
Advanced search Search tips

Issue 894231 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Calling addTransceiver a second time with the same MediaStreamTrack generates a different MSID for the second m= section.

Reported by mmalava...@twilio.com, Oct 10

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.100 Safari/537.36

Steps to reproduce the problem:
• Run https://jsfiddle.net/1hu2gLvz/ in a new tab.
• Click on "Add the same audio track the second time".
• Observe JavaScript console.

What is the expected behavior?
MSID in the sdp should match the MediaStreamTrack's ID for both m= sections.

What went wrong?
MSID in the sdp does not match the MediaStreamTrack ID in the second m= section.

Did this work before? N/A 

Does this work in other browsers? N/A

Chrome version: 69.0.3497.100  Channel: stable
OS Version: OS X 10.13.6
Flash Version: 

According to http://rtcweb-wg.github.io/jsep/#rfc.section.5.2.2: "If a MediaStreamTrack is attached to the transceiver's RtpSender, the "a=msid" lines MUST use that track's ID."
 
Labels: Needs-Triage-M69
Cc: susan.boorgula@chromium.org
Labels: Triaged-ET Target-71 M-71 FoundIn-71 FoundIn-70 FoundIn-69 OS-Linux OS-Windows
Status: Untriaged (was: Unconfirmed)
mmalavalli@ Thanks for the issue.

Able to reproduce the issue on Windows 10, Mac OS 10.13.3 and Ubuntu 17.10 on the reported version 69.0.3497.100 and the latest Canary 71.0.3573.0.
Attached is the screen shot for reference.

This is a Non-Regression issue as this is observed from M-69(69.0.3497.7) chrome build, from where MediaStreamTrack is introduced in Chrome.
Hence marking this as Untriaged for further updates from Dev.

Thanks..
894231.png
250 KB View Download
Components: -Blink>WebRTC Blink>WebRTC>PeerConnection
Cc: hta@chromium.org shampson@chromium.org steveanton@chromium.org
For "initial offers" the "a=msid" line MUST be the track ID.
For "subsequent offers", the "a=msid" line MUST stay the same, regardless of changes to the track. If no "a=msid" line is present in the current description, "a=msid" lines MUST be generated according to the same rules as the initial offer.
And it says the same thing with regards to "initial answers" and "subsequent answers".

It sounds pretty clear that "a=msid" should be the track ID...

But https://tools.ietf.org/html/draft-ietf-mmusic-msid-16 says...

   The identifier (msid-id) uniquely identifies a group within the scope
   of an SDP description.

   There may be multiple msid attributes in a single media description.
   This represents the case where a single MediaStreamTrack is present
   in multiple MediaStreams; the value of "msid-appdata" MUST be
   identical for all occurences.

   Multiple media descriptions with the same value for msid-id and msid-
   appdata are not permitted.

If I read this correctly, "msid-id" refers to the stream ID and "msid-appdata" refers to the track ID.
It sounds like same track ID can be present in multiple lines with different stream IDs, but not in multiple lines with the same stream ID.
Sounds like "addTransceiver(track); addTransceiver(track);" which would cause two tracks without a stream (stream ID = "-") is NOT allowed to have the same track ID because both occurrences would have "-", so to avoid a spec contradiction we generate a new ID.
Doing this on the other hand: "addTransceiver(track, {streams:[stream1]}); addTransceiver(track, {streams:[stream2]});" the right thing sounds like it would be to generate "a=msid" lines such that the same track belongs to two different streams.

I think the codesnippet is wrong on this point due to spec contradictions (which should be clarified), but even if modify it such that it takes different streams as argument you get the same behavior of track ID not matching on the second line.

However, I think we have spec bugs. The local and remote track IDs are not guaranteed to match. In fact, I don't know why we send "a=msid" at all... The thing you need to look at is the transceiver's "mid" attribute. The way WebRTC 1.0 works is transceivers that are used for an incoming track might have been created before we got the offer, meaning the receiver and its track had already been created, with some other track ID.
  You should never rely on track IDs, only transceiver MIDs.

I think I'll file a bug on webrtc-pc spec to be clear about what to do, because as-is the referenced specs contradicts themselves.
As for fixing the implementation, I'm not sure if this is a WontFix or legitimate thing we want to fix depending on how the spec is clarified.
Thoughts CC'd folks?
"In fact, I don't know why we send "a=msid" at all..."

From the mmusic-msid-16 spec: "This new attribute allows endpoints to associate RTP streams that are described in different media descriptions with the same MediaStreams..." The msid lines aren't used to track the m sections (this is done with mids). They're used to associate tracks (MediaStreamTracks) to different streams (MediaStreams). This is handled when setting the remote description by setting the MediaStream objects appropriately:
https://cs.chromium.org/chromium/src/third_party/webrtc/pc/peerconnection.cc?l=2408

Henrik, it seems to me that the correct thing to do would be to generate the second track ID "msid-appdata", but keep the msid-id the same. This would allow to still map the sender/track to the media stream ID, but not have duplicate "a=msid .." lines. I guess the alternative is to change the spec to allow duplicate lines. Do we know the reason for disallowing this?
Status: Available (was: Untriaged)
As pointed out in the spec issue JSEP decided to remove track ID from a=msid: https://github.com/rtcweb-wg/jsep/issues/842

There's also a pull request to change the spec language, but no one appears to have looked at it yet: https://github.com/rtcweb-wg/jsep/pull/850

So I guess we could go ahead and make the change before JSEP gets updated?

Sign in to add a comment