1367930 - Handle RID Encondings mis-match for simulcast

Status: RESOLVED FIXED

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

Description

[Nils Ohlmeier [:drno]]

Fippo accidentally discovered that in the simulcast answerer scenario we can encounter a mis-match between the amount of encodings requested via JS and the amount of RID's signaled in the offer from the remote side. Currently this ends up an assertion when trying to start the sender complaining that the amount of SSRCs does not match the amount of RIDs.

I did not find anything in the spec how to handle this scenario.

I see two options:
#1 take min(remoteRIDs, requestedEncodings) and proceed with that
#2 throw some form of error when doing createAnswer() (?) pointing out that there is a mis-match between these two values

Comment 1

[Nils Ohlmeier [:drno]]

@bwc: thoughts?

Comment 2

[Nils Ohlmeier [:drno]]

To make matters worse: any offer with simulcast and RID attributes in it, even when the JS code never uses setParameters() leads to the same crash/assert it looks like.

Comment 3

[Byron Campen [:bwc]]

The correct thing to do is probably the stuff in setParameters that is a subset of what was in the remote offer.

Comment 4

[Nils Ohlmeier [:drno]]

Attachment: Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

Review commit: https://reviewboard.mozilla.org/r/143546/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/143546/

Comment 5

[Byron Campen [:bwc]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

https://reviewboard.mozilla.org/r/143546/#review147944

So, are we ending up in a situation where JsepTrack has more encodings than SSRCs? If so, I would like to fix JsepTrack instead...

Comment 6

[Nils Ohlmeier [:drno]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

https://reviewboard.mozilla.org/r/143546/#review147944

Yes. Both scenarios can happen: more encodings then SSRCs OR more SSRCs then encodings.

Comment 7

[Nils Ohlmeier [:drno]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/143546/diff/1-2/

Comment 8

[Nils Ohlmeier [:drno]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/143546/diff/2-3/

Comment 9

[Byron Campen [:bwc]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

https://reviewboard.mozilla.org/r/143546/#review148906

What happens if the remote SDP uses different RID values (eg; hi, med, lo) than were in the JS encodings?

::: media/webrtc/signaling/src/jsep/JsepTrack.cpp:283
(Diff revision 3)
> -  // For each rid in the remote, make sure we have an encoding, and configure
> +  size_t max_streams = 1;
> +
> +  if (mJsEncodeConstraints.size()) {
> +    max_streams = std::min(rids.size(), mJsEncodeConstraints.size());
> +  }
> +  // Drop SSRC's if less RID's where offered then we have encoding constraints

Editorial nits:
s/where/were
s/then/than
s/SSRC's/SSRCs
s/RID's/RIDs

Comment 10

[Nils Ohlmeier [:drno]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/143546/diff/3-4/

Comment 11

[Nils Ohlmeier [:drno]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/143546/diff/4-5/

Comment 12

[Nils Ohlmeier [:drno]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/143546/diff/5-6/

Comment 13

[Nils Ohlmeier [:drno]]

Comment on attachment 8872021 [details]
Bug 1367930: adjust ssrc's and encondings as simulcast answerer.

https://reviewboard.mozilla.org/r/143546/#review148906

The answer appears to depend on how the mis-match looks exactly.
I filed a follow up bug 1369601 to solve that problem.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2ad1181dadf3
adjust ssrc's and encondings as simulcast answerer. r=bwc

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2ad1181dadf3