unified plan: answer SDP without a=mid is rejected |
|||||||||
Issue descriptionChrome Version: (copy from chrome://version) OS: (e.g. Win10, MacOS 10.12, etc...) What steps will reproduce the problem? (1) go to http://jsfiddle.net/29npv3kd/1/ (2) observe the console What is the expected result? It still works? It works using plan-b What happens instead? The answer is rejected with a complaint about the order of m-lines not matching. Which is ironic, there is only a single one. JSEP is quite clear on this at least, http://rtcweb-wg.github.io/jsep/#rfc.section.5.10 If the m= section does not include a MID (i.e., the remote endpoint does not support the MID extension), generate a value for the RtpTransceiver mid property, following the guidance for "a=mid" mentioned in Section 5.2.1.
,
Nov 5
Yes, this looks like a bug.
,
Nov 5
,
Nov 6
I started a fix but ran into some problems. WIP here: https://webrtc-review.googlesource.com/c/src/+/109487 A lot of code assumes the SDP MID (content_name) is a unique identifier. Right now the place to generate the MID if a=mid is not present is relatively late. For example, transports have already been created (and they assume MID is a unique identifier). Furthermore, according to JSEP the MID should always be generated in setRemoteDescription, even if one is already associated. I confirmed this is what Firefox implements (at least in the latest nightly). I'm not sure how to make our code work with that behavior, given that everything is indexed by MID. Given the fix is quite involved, requires some risky refactoring, and the workaround is not too onerous, I'm going to bump the priority. Let me know if the assessment is inaccurate.
,
Nov 6
yeah, the workaround would be to parse the SDP (there are libraries for that) and backfill the mid from the localdescription before calling setRemoteDescription. Probably worth documenting the current issues in the migration document -- unless you want me and inaki doing so ;-)
,
Dec 3
5.3.1 also says "If and only if present in the offer, an "a=mid" line, as specified in [RFC5888], Section 9.1. The "mid" value MUST match that specified in the offer." If I tweak the fiddle to set it as an offer instead it says "yay". Is the concern a Unified Plan client offering to a client that ignores a=mid lines and answers without them?
,
Dec 4
I did not venture into the woods to find this one, there is still software out there that predates RFC 5888 and SDP takes care of negotiating all this.
,
Dec 17
,
Dec 20
Requesting merge to M72 for the WebRTC commits in #17 and #20 (see bugs.webrtc.org/9540). These are low-risk bugfixes to unblock the Unified Plan rollout in M72.
,
Dec 20
This bug requires manual review: M72 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 20
Are these changes behind a flag? Are they well tested in canary?
,
Dec 20
Looks like the Chromium roll CLs are: https://chromiumdash.appspot.com/commit/45e536a9674fabe334db2fe58c1f5cd0b9757425 https://chromiumdash.appspot.com/commit/947a70c1735706f398d8bc081640222ad4483232 Only the first has landed in Canary so far: Commit 45e536a9... initially landed in 73.0.3646.0 (The first is the bulk of the change, the second is just a small follow-up.) These changes are not behind a flag, though they mostly affect 'unified-plan' which is behind an experiment.
,
Dec 26
Approving merge for M72. Branch:3626
,
Dec 27
Merged. https://webrtc-review.googlesource.com/c/src/+/115683 https://webrtc-review.googlesource.com/c/src/+/115684 https://webrtc-review.googlesource.com/c/src/+/115685 |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by hbos@chromium.org
, Nov 5Labels: -Pri-3 M-72 Pri-2
Owner: steveanton@chromium.org
Status: Assigned (was: Untriaged)