Closed Bug 1367930 Opened 3 years ago Closed 3 years ago

Handle RID Encondings mis-match for simulcast

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: drno, Assigned: drno)

References

Details

Attachments

(1 file)

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
@bwc: thoughts?
Flags: needinfo?(docfaraday)
Rank: 25
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.
Rank: 25 → 15
Priority: P2 → P1
The correct thing to do is probably the stuff in setParameters that is a subset of what was in the remote offer.
Flags: needinfo?(docfaraday)
Attachment #8872021 - Flags: review?(docfaraday)
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 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.
Assignee: nobody → drno
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
Attachment #8872021 - Flags: review?(docfaraday) → review+
See Also: → 1369601
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.
Pushed by drno@ohlmeier.org:
https://hg.mozilla.org/integration/autoland/rev/2ad1181dadf3
adjust ssrc's and encondings as simulcast answerer. r=bwc
https://hg.mozilla.org/mozilla-central/rev/2ad1181dadf3
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.