[webrtc-pc] Negotiation methods are vestigial, racy with a pushy SFU. (#2221)

jan-ivar has just created a new issue for https://github.com/w3c/webrtc-pc:

== Negotiation methods are vestigial, racy with a pushy SFU. ==
Not covered in ["Perfect negotiation in WebRTC"](https://blog.mozilla.org/webrtc/perfect-negotiation-in-webrtc/), is dealing with an SFU. Fippo explained to me that SFUs can be “pushy”: They’ll send an offer, followed immediately by a second “better” offer. One strategy is the “FIFO peer”:
io.onmessage = async ({data: {description, candidate}}) => {
  if (description) {                              // FIFO peer:
    await Promise.all([                           // ←- Avoids race!
      pc.setRemoteDescription(description),       // ←- Always an “offer”. Fixed roles
      pc.createAnswer(),                          // ←- 
      pc.setLocalDescription({type: "answer"})    // ←- Unusual, but works today
    ]);                                           // ←- 
    io.send({description: pc.localDescription});
A second offer may come in while we’re busy responding to the first offer. As a workaround we use `Promise.all` to front-load the peer connection’s queue with all our methods to get back to "stable" before any other methods get a go!

Even with https://github.com/w3c/webrtc-pc/issues/2165 fixed, we’re not able to fully get rid of `Promise.all` here. We’ll still need:
io.onmessage = async ({data: {description, candidate}}) => {
  if (description) {
    await Promise.all([                          // ←- Still needed
    io.send({description: pc.localDescription});

The SFU FIFO case shows SLD() & SRD() are racy and a bit outdated: from an earlier time when SDP mangling between createOffer/Answer to SLD was allowed (it is now forbidden), and when rejecting m-lines in the answer was common. I propose we give people a simpler and safer alternative that’s race-free and glare-proof:
  io.onmessage = async ({data: {description, candidate}}) => {
    if (description) {                                      // FIFO peer:
      await pc.setRemoteAndLocalDescriptions(description);  // ←- Always an “offer”.
      io.send({description: pc.localDescription});          // because of fixed roles
    } else if (candidate) {
      await pc.addIceCandidate(candidate);
**Proposal:** `pc.setRemoteAndLocalDescriptions(description)` works like regular SRD, plus, if description is an offer, generates and sets a local answer before resolving. This would be behavior-neutral with the `Promise.all` case. I.e. if SRD succeeds, but SLD fails, we won’t roll back on failure.

Please view or discuss this issue at https://github.com/w3c/webrtc-pc/issues/2221 using your GitHub account

Received on Monday, 1 July 2019 21:37:41 UTC