Bug 191169 - Safari rejects offer generated by peerconnection.createOffer()
Summary: Safari rejects offer generated by peerconnection.createOffer()
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified macOS 10.14
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-01 14:26 PDT by Iñaki Baz
Modified: 2022-07-04 05:14 PDT (History)
2 users (show)

See Also:


Attachments
reduced test (956 bytes, text/html)
2018-11-02 02:36 PDT, youenn fablet
no flags Details
reduced test without max bundle (914 bytes, text/html)
2018-11-02 02:38 PDT, youenn fablet
no flags Details
reduced test with UI (1.49 KB, text/html)
2018-11-02 03:00 PDT, youenn fablet
no flags Details
Test script 2 with sender.replaceTrack(null) (1.46 KB, text/javascript)
2018-11-05 03:03 PST, Iñaki Baz
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Iñaki Baz 2018-11-01 14:26:56 PDT
Adding a transceiver for sending audio, then stopping it, then adding a new transceiver for sending video. During the second SDP O/A, pc.setLocalDescription() fails with "OperationError: Failed to set local offer sdp: The m= section:1 should be rejected."

It's a clear bug when offer produced by pc.createOffer() is later rejected in pc.setLocalDescription(offer).

Below the code to reproduce it. IMPORTANT:

* Safari Tech Preview 12.1.
* "WebRTC Unified-Plan" enabled in menu "Develop / Experimental Features".

Just open the browser console and paste this code:


-------------------------------------------------------------
let micTrack;
let webcamTrack;
let pc;
let micTransceiver;
let webcamTransceiver;

navigator.mediaDevices.getUserMedia({ audio: true, video: true })
  .then((stream) =>
  {
    micTrack = stream.getAudioTracks()[0];
    webcamTrack = stream.getVideoTracks()[0];
  })
  .then(() =>
  {
    pc = new RTCPeerConnection(
      {
        iceServers    : [],
        bundlePolicy  : 'max-bundle',
        rtcpMuxPolicy : 'require',
        sdpSemantics  : 'unified-plan'
      });

    console.warn('pc: adding mic track');

    micTransceiver =
      pc.addTransceiver(micTrack, { direction: 'sendonly' });
  })
  .then(() => doSdpStuff())
  .then(() =>
  {
    console.warn('pc: removing mic track');

    micTrack.stop();

    pc.removeTrack(micTransceiver.sender);

    // NOTE: Chrome does not implement transceive.stop().
    try { micTransceiver.stop(); } catch(error) {}

    // NOTE: Safari 12.1 does not allow setting transceiver.direction.
    try { micTransceiver.direction = 'inactive'; } catch(error) {}
  })
  .then(() =>
  {
    // Add the webcam track to pc.

    console.warn('pc: adding webcam');

    webcamTransceiver =
      pc.addTransceiver(webcamTrack, { direction: 'sendonly' });
  })
  .then(() => doSdpStuff())
  .catch((error) =>
  {
    console.error(error);
  });

function doSdpStuff()
{
  let errored = false;

  return Promise.resolve()
    .then(() => pc.createOffer())
    .catch((error) =>
    {
      console.error('pc.createOffer() failed');

      errored = true;
      throw error;
    })
    .then((offer) =>
    {
      console.warn('pc.createOffer() succeeded:\n%s', offer.sdp);

      return pc.setLocalDescription(offer);
    })
    .catch((error) =>
    {
      if (errored)
        throw error;

      console.error('pc.setLocalDescription() failed');

      throw error;
    })
}
-------------------------------------------------------------

NOTE: This bug does not happen in Chrome.
Comment 1 Iñaki Baz 2018-11-01 17:50:24 PDT
If you need a way to reproduce it, I can deploy it and provide exact instructions.
Comment 2 youenn fablet 2018-11-02 02:36:19 PDT
Created attachment 353683 [details]
reduced test
Comment 3 youenn fablet 2018-11-02 02:38:32 PDT
Created attachment 353684 [details]
reduced test without max bundle
Comment 4 youenn fablet 2018-11-02 02:42:10 PDT
Thanks Inaki,

I wrote a reduced test (in attachment).

The problem seems to arise only with the max bundle option so you might have a workaround for now.

Looking at the generated SDP, I would have thought the m section to be recycled.
It seems to me the SDP should only have one m-section.
Comment 5 youenn fablet 2018-11-02 03:00:14 PDT
Created attachment 353686 [details]
reduced test with UI
Comment 6 youenn fablet 2018-11-02 03:01:42 PDT
(In reply to youenn fablet from comment #5)
> Created attachment 353686 [details]
> reduced test with UI

Tried the test with Firefox and it generates one m section, but with two a=mid lines.
Comment 7 youenn fablet 2018-11-02 03:28:04 PDT
(In reply to youenn fablet from comment #6)
> (In reply to youenn fablet from comment #5)
> > Created attachment 353686 [details]
> > reduced test with UI
> 
> Tried the test with Firefox and it generates one m section, but with two
> a=mid lines.

Looking more closely:
Firefox generates two m sections, one zero-port/inactive, the other sendrecv.
WebKit generates two m sections, one zero-port/recvonly, the other sendrecv
Comment 8 youenn fablet 2018-11-02 03:33:52 PDT
WebKit is generating "a=group:BUNDLE 0 1" but 0 is zero ported and 1 is not.
Comment 9 Iñaki Baz 2018-11-02 17:21:51 PDT
Hi, I've changed my mind and I no longer call transceiver.stop() nor produce "remote" SDP answers with port 0 in any m= section. It's painful as it involves transport closure if there is no other active stream (DTLS close alert is sent by the browser). It's much easier by just removing the sending track with pc.receiveTrack(sender).
Comment 10 Iñaki Baz 2018-11-05 02:59:54 PST
Unfortunately calling sender.replaceTrack(null) does not set "a=inactive". Instead it remains "a=sendonly" even if I set transceiver.direction = "inactive" once the replaceTrack(null) promise resolves.

But this is wrose, as by using replaceTrack(null) and later replaceTrack(newTrack), a *NEW* transceiver / m= section is created!

Attaching a new script.
Comment 11 Iñaki Baz 2018-11-05 03:03:35 PST
Created attachment 353844 [details]
Test script 2 with sender.replaceTrack(null)

Same script as before but instead of pc.removeTrack(sender) it uses sender.replaceTrack(null).then(() => transceiver.direction = "inactive"):

* Once replaceTrack(null) resolves the script changes the transceiver direction to "inactive" and generates an offer. The offer has a=sendonly, so this is not a valid solution yet.

* And much worse: If after that I call sameSender.replaceTrack(newTrack), it generates a new audio transceiver with its corresponding new m=audio section, both with "a=sendonly". This is an important bug somewhere IMHO.
Comment 12 Iñaki Baz 2018-11-05 03:18:39 PST
The second script "Test script 2 with sender.replaceTrack(null)" in Chrome and Firefox works fine.

NOTE: In Safari, changing transceiver.direction produces an exception (something like "cannot change readonly property") if I do it before the replaceTrack(null) promise resolves. This does not happen in Chrome or Firefox and the spec says nothing about when transceiver.direction can be changed. It's supposed to be changed at any time. It just happens that its effective value (pc.currentDirection) takes an effective value later, may be after a proper SDP O/A.
Comment 13 Iñaki Baz 2018-11-05 03:35:06 PST
I'm sorry, I've written here comments related to issue #191202... My fault. Feel free to delete them. I'll write them in the corresponding issue.
Comment 14 Iñaki Baz 2018-11-13 16:06:27 PST
Hi, any news about this issue?
Comment 15 youenn fablet 2018-11-13 16:51:35 PST
(In reply to Iñaki Baz from comment #14)
> Hi, any news about this issue?

We are working with the libwebrtc team on a fix. See https://bugs.chromium.org/p/webrtc/issues/detail?id=9954
Comment 16 Brent Fulgham 2022-07-01 16:32:56 PDT
Youenn: It appears that libwebrtc has now fixed the issue. Can you confirm we have the correct fix (your test case appears to PASS).
Comment 17 youenn fablet 2022-07-04 05:14:49 PDT
Fixed upstream