Closed Bug 1344556 Opened 7 years ago Closed 7 years ago

Nor a=simulcast/a=rid if calling sender.setParameters() before pc.createAnswer()

Categories

(Core :: WebRTC: Signaling, enhancement, P2)

54 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: ibc, Assigned: bwc)

References

Details

Attachments

(6 files)

Firefox does add the a=simulcast and a=rid lines to the SDP offer but does not add them in SDP answers.

This is, this works:

-----------------------
let sender = pc.getSenders()[1];

sender.setParameters(
  {
    encodings :
    [
      { rid: '1', active: true, priority: 'high', maxBitrate: 40000 },
      { rid: '2', active: true, priority: 'medium', maxBitrate: 10000 }
    ]
  });

pc.createOffer()

// Generated SDP m video has simulcast/rid lines and two ssrcs:
a=rid:1 send
a=rid:2 send
a=simulcast: send rid=1;2
a=ssrc:1589989957 cname:{4783b6ed-5282-8340-affe-a5e4c1f41993}
a=ssrc:2091640471 cname:{4783b6ed-5282-8340-affe-a5e4c1f41993}
-----------------------


However, if Firefox receives an initial SDP offer with valid simulcast stuff (according to draft simulcast version 03 as implemented in Firefox):

-----------------------
a=rid:1 recv
a=rid:2 recv
a=simulcast: recv rid=1;2
-----------------------

and then we set sender.setParameters(...same_as_above...) and pc.createAnswer(), then Firefox *crashes*.


As a workaround, I initially call sender.setParameters() and pc.createOffer() (and just discard the generated offer). And then I receive the initial simulcast-enabled remote offer, call pc.setRemoteDescription(), sender.setParameters() and then pc.createAnswer().

The generated SDP has NO simulcast/rid attributes in the m video. However, it DOES HAVE two ssrcs and it DOES SEND RTP packets with RID header extension (always rid 2 BTW...).

Also, if pc.createAnswer() is called without calling sender.setParameters() before, then Firefox also crashes.
A simpler question is: does Firefox allow setting simulcast stuff when the initial offer comes from the server?
Firefox does not support being the receiver of a simulcast stream (the assumption here is that we don't see a point in a browser receiving simulcast streams).

But I guess offering to receive is a valid use case which we haven't considered so far.
Rank: 25
Priority: -- → P2
Yes, I've never suggested that Firefox should support receiving simulcast. That's not stated in WebRTC nor in ORTC. This is just about being able to receive an offer with "a=simulcast: recv rid=1;2" so Firefox can use "sender.setParameters()" to create a simulcast enabled SDP answer.
Yeah, there's a bug here for sure. Let me look into it.
Assignee: nobody → docfaraday
Comment on attachment 8845101 [details]
Bug 1344556 - Part 1: Test cases for simulcast answerer.

https://reviewboard.mozilla.org/r/118316/#review120222

::: commit-message-3d341:1
(Diff revision 1)
> +Bug 1344556 - Part 1: Test cases for simulcast answerer. r?drno

Is the existence of a file called 'commit-message' a bug in your client or some awesome new feature of mozreview which I'm not aware of yet?

::: dom/media/tests/mochitest/test_peerConnection_simulcastAnswer.html:33
(Diff revision 1)
> +    pushPrefs(['media.peerconnection.simulcast', true],
> +              // 180Kbps was determined empirically, set well-higher than
> +              // the 80Kbps+overhead needed for the two simulcast streams.
> +              // 100Kbps was apparently too low.
> +              ['media.peerconnection.video.min_bitrate_estimate', 180*1000]).then(() => {
> +      SimpleTest.requestCompleteLog();

I think this should be commented out before landing.

::: dom/media/tests/mochitest/test_peerConnection_simulcastAnswer.html:61
(Diff revision 1)
> +          });
> +        },
> +        function PC_LOCAL_ADD_RIDS_TO_OFFER(test) {
> +          // Create a dummy offer, and use it to set simulcast stuff on the
> +          // offer we will actually be using.
> +          return test.createOffer(test.pcRemote).then(offer => {

Wouldn't it be easier to simply add a list of hard coded simulcast attributes to the offer without creating an offer on pcRemote?
Attachment #8845101 - Flags: review?(drno) → review+
Comment on attachment 8845102 [details]
Bug 1344556 - Part 2: Some helper functions for working with SDP direction attributes. Makes subsequent work simpler.

https://reviewboard.mozilla.org/r/118318/#review120308
Attachment #8845102 - Flags: review?(drno) → review+
Comment on attachment 8845103 [details]
Bug 1344556 - Part 3: When answering, pay attention to JS simulcast constraints.

https://reviewboard.mozilla.org/r/118320/#review120358
Attachment #8845103 - Flags: review?(drno) → review+
Comment on attachment 8845104 [details]
Bug 1344556 - Part 4: Fix bug where direction on negotiated extmaps was recorded backwards.

https://reviewboard.mozilla.org/r/118322/#review120368
Attachment #8845104 - Flags: review?(drno) → review+
Comment on attachment 8845105 [details]
Bug 1344556 - Part 5: Fix bug where sendonly extmaps would be configured on recv tracks, and vice versa.

https://reviewboard.mozilla.org/r/118324/#review120372
Attachment #8845105 - Flags: review?(drno) → review+
Comment on attachment 8845106 [details]
Bug 1344556 - Part 6: Sanity check extmaps in answers.

https://reviewboard.mozilla.org/r/118326/#review120384
Attachment #8845106 - Flags: review?(drno) → review+
Hi, let me a question. Let's assume that Firefox receives an initial SDP offer with:

-----------------------
a=rid:1 recv
a=rid:2 recv
a=simulcast: recv rid=1;2
-----------------------

In order to send simulcast, would I need to call sender.setParameters() with two encodings before creating the SDP answer? I understand that it shouldn't be needed because it would be so hard (the JS should inspect the SDP offer, get the a=rid values and call sender.setParameters() with those exact a=rid values...). So I expect that there is no need to "manually" enable simulcast if the initial SDP offer includes it, right?

Also, what about re-offers from the remote? Just imagine that the remote sends later a re-offer with:

-----------------------
a=rid:1 recv
a=rid:3 recv
a=simulcast: recv rid=1;3
-----------------------

Would Firefox properly handle it? (Note that this may involve destroy previous sending SSRCs, change RID values, etc...).
Comment on attachment 8845101 [details]
Bug 1344556 - Part 1: Test cases for simulcast answerer.

https://reviewboard.mozilla.org/r/118316/#review120222

> Is the existence of a file called 'commit-message' a bug in your client or some awesome new feature of mozreview which I'm not aware of yet?

Good question. Let's see what happens when it lands. :D

> I think this should be commented out before landing.

Hmm. That seems to have come from the simulcastOffer test, added by bug 1242199. It seems that this intermittent failure has cleared up, so we probably don't need it anymore. I'll go ahead and take care of it.

> Wouldn't it be easier to simply add a list of hard coded simulcast attributes to the offer without creating an offer on pcRemote?

Perhaps, but if we ever wanted to alter the parameters we would need to alter them in two separate places.
(In reply to Iñaki Baz Castillo from comment #17)
> Hi, let me a question. Let's assume that Firefox receives an initial SDP
> offer with:
> 
> -----------------------
> a=rid:1 recv
> a=rid:2 recv
> a=simulcast: recv rid=1;2
> -----------------------
> 
> In order to send simulcast, would I need to call sender.setParameters() with
> two encodings before creating the SDP answer? I understand that it shouldn't
> be needed because it would be so hard (the JS should inspect the SDP offer,
> get the a=rid values and call sender.setParameters() with those exact a=rid
> values...). So I expect that there is no need to "manually" enable simulcast
> if the initial SDP offer includes it, right?
> 
> Also, what about re-offers from the remote? Just imagine that the remote
> sends later a re-offer with:
> 
> -----------------------
> a=rid:1 recv
> a=rid:3 recv
> a=simulcast: recv rid=1;3
> -----------------------
> 
> Would Firefox properly handle it? (Note that this may involve destroy
> previous sending SSRCs, change RID values, etc...).

I'm pretty sure the spec requires JS to be in harmony with SDP here. At one time we had proposed that JS have a "I'm willing to do simulcast with n layers, if anyone asks" knob, that would allow the other side to request simulcast, but that idea was not adopted. As for changing rid values, you can't do that from JS; you must create a whole new transceiver. If that is true, then what is the proper reaction if the other side changes them in SDP? Seems like the expectation is that you re-create the transceiver with the new parameters, then negotiate the change. Kinda awkward though.
(In reply to Byron Campen [:bwc] from comment #19)
> I'm pretty sure the spec requires JS to be in harmony with SDP here. At one
> time we had proposed that JS have a "I'm willing to do simulcast with n
> layers, if anyone asks" knob, that would allow the other side to request
> simulcast, but that idea was not adopted

I'm "fine" with that (as I already parse the received SDP offer and can deal with existing a=rid and a=simulcast and call sender.setParameters() according). However there is a bunch of potential issues here in case I call setParameters() with RID values non matching those in the received simulcast offer, which would generate a wrong SDP answer. If Firefox fails on createAnswer() in that case, I'm 100% fine.


> As for changing rid values, you can't do that from JS; you must create a whole new transceiver.

Wouldn't a new call to sender.setParameters() with proper values (according to the new values in the received re-offer) do the work? Why should I generate a new transceiver?

Sorry, auto-reply: Most of the sender parameters cannot be modified later, you are 100% right:
https://w3c.github.io/webrtc-pc/#dom-rtcrtpsender-setparameters


> If that is
> true, then what is the proper reaction if the other side changes them in
> SDP? Seems like the expectation is that you re-create the transceiver with
> the new parameters, then negotiate the change. Kinda awkward though.

Yes... In fact I don't know how to "re-create the transceiver". It may be done by removing its local video track (assuming sendonly direction) and then adding the local video track again to the PeerConnection. But it may happen that the local video is inserted into another free m=video slot... buff, better not go deeper into this game :)
Is any of this work something that should be called to attention in Firefox X for developers? And does it affect 53 as well?
Flags: needinfo?(docfaraday)
It affects 53, for sure. I really doubt this feature is seeing much use; it has been really broken in many ways, and we see very few bug reports on it. Calling attention to this change may not have much impact at this stage.
Flags: needinfo?(docfaraday)
Hi, may I know the status of this issue? AFAIU it's not merged in Nightly yet, right?
Gah, I dropped this one. Too much stuff in flight right now...
Flags: needinfo?(docfaraday)
please request uplift once you're happy with it in m-c.
Pushed by bcampen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/558b0c0a037a
Part 1: Test cases for simulcast answerer. r=drno
https://hg.mozilla.org/integration/autoland/rev/f6d39ce84b05
Part 2: Some helper functions for working with SDP direction attributes. Makes subsequent work simpler. r=drno
https://hg.mozilla.org/integration/autoland/rev/0d9a2cd15793
Part 3: When answering, pay attention to JS simulcast constraints. r=drno
https://hg.mozilla.org/integration/autoland/rev/f0157e3c75b6
Part 4: Fix bug where direction on negotiated extmaps was recorded backwards. r=drno
https://hg.mozilla.org/integration/autoland/rev/d903d2a7b205
Part 5: Fix bug where sendonly extmaps would be configured on recv tracks, and vice versa. r=drno
https://hg.mozilla.org/integration/autoland/rev/a1608351bbd4
Part 6: Sanity check extmaps in answers. r=drno
Amazing work. Thanks.
Flags: needinfo?(docfaraday)
Depends on: 1351531
Depends on: 1351590
I've tested the following scenario in Firefox Nightly 55.0a1 (today):

1) The server sends an offer with a m=video a=recvonly containing:

  a=simulcast: recv rid=1,2
  a=rid:1 recv
  a=rid:2 recv

2) In Firefox, call setRemoteDescription(offer), then createAnswer(), then mangle the generated SDP answer by adding:

  a=simulcast: send rid=1,2
  a=rid:1 send
  a=rid:2 send

3) Call setLocalDescription(answer) with the mangled answer.

It works :)

Firefox sends two streams with different SSRCs and the corresponding RID extension.

However, if Firefox receives a re-offer (let's assume it's identical to the first one) I must repeat step 2) above because createAnswer() does not generate the simulcast stuff.

I assume that, instead of mangling the SDP, I should create a transceiver with proper data (including simulcast) according to the received offer, so that would be done just once, am I right?
Blocks: 1361206
You need to log in before you can comment on or make changes to this bug.