- 
                Notifications
    You must be signed in to change notification settings 
- Fork 18
Always negotiate datachannels #242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1fc5bf4    to
    9f4960d      
    Compare
  
    | </section> | ||
| <section id="always-negotiating-datachannels-interface-extensions"> | ||
| <h3> | ||
| JSEP modifications | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@docfaraday what can we expect wrt rtcweb-wg/jsep#1039 here? What's next steps?
| <p> | ||
| In the algorithms for generating initial offers in [[RTCWEB-JSEP]] section 5.2.1, | ||
| replace | ||
| "Lastly, if a data channel has been created, a m= section MUST be generated for data" | ||
| with | ||
| "Lastly, if a data channel has been created and no m=section has been generated for data, a m= section MUST be generated for data". | ||
| </p> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a note somewhere to clarify this changes the m-line order? As presented, this effect and its benefit wrt BUNDLE seems important to include as a reason why someone may want to use this API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added, don't forget to squash (or I can squash)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to note the "bundle-tag rejection trap avoidance" benefit, we might also want to mention that this benefit only applies with bundlePolicy=="max-bundle" (for "max-compat" the problem does not exist in the first place, and for "balanced" it won't help).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for balanced it would help since (surprise!) if you reject the bundle-tagged m-line you can still not move to the next per https://datatracker.ietf.org/doc/html/rfc8843#section-7.3.3 ?
(and it looks like Firefox and Chrome disagree what max-compat means... Firefox does not bundle, Chrome bundles)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand. If it is in a group all by itself, (which is what "balanced" says to do, sort of, it's worded poorly), rejecting it destroys that transport, but since nothing else is in there it's fine. On the other side, if you have multiple m=audio, those will share a transport, but rejecting that bundle tag still messes things up; it's not the first m-section overall, but it still can destroy a transport and bring down everything bundled with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://jsfiddle.net/fippo/5y6ag2Lw/1/-- I don't see separate bundle groups with balanced? (nor do I see the second audio m-line marked as bundle-only)
Either way, do we agree this is cheap and solves a (max-bundle) headache without overpromising?
extending JSEP rules to negotiate datachannels before createDataChannel
and negotiate them as the first m-line in the SDP.
```
const pc = new RTCPeerConnection({alwaysNegotiateDataChannels: true});
pc.addTransceiver('audio');
await offer = pc.createOffer();
// assert that the first m= line is m=application
```
JSEP issue: rtcweb-wg/jsep#1039
Fixes w3c#241
    9f4960d    to
    bda5b11      
    Compare
  
    | This is strikingly similar to the offerToReceiveX options on createOffer. Hypothetically, if we followed those as an example, we would get something pretty similar to what this proposal does. I think we would need to do some extra work to ensure that the m=application section was up top though. One potential benefit would be to allow this to be used after the initial negotiation; there's no way to ensure that this m=application is up top, but it still would allow the creation of one without forcing the offerer to create a channel. Something to consider. | 
| offerTo* is dubbed "legacy" (ignoring that a significant percentage of Chrome usage does this) and (more importantly) does not work with  | 
| https://w3c.github.io/webrtc-pc/#webidl-862292147 also needs a slight tweak, changing 
 I think the condition for that can be expressed using [[SctpTransport]] not being null. Expected behavior: Drive-by: 
 | 
| @fippo, are you planning to update the PR according your latest comment? | 
| @youennf this requires a PR against webrtc-pc | 
| we have multiple places in webrtc-extensions that contain what's effectively instructions to edit webrtc-pc when it gets merged. | 
| Done. [[SctpTransport]] is missing an export in the dfn on line 1311. Should I file issues for the drive-by questions? | 
which always negotiates datachannels and puts them first in the SDP. Spec: w3c/webrtc-extensions#242
which always negotiates datachannels and puts them first in the SDP. Spec: w3c/webrtc-extensions#242
d812995    to
    7f5bdb5      
    Compare
  
    
extending JSEP rules to negotiate datachannels before createDataChannel and negotiate them as the first m-line in the SDP.
JSEP issue: rtcweb-wg/jsep#1039
Fixes #241
Preview | Diff