-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementations only update getParameters().codecs when negotiation has finished #2967
Comments
Seems like we want this getter to reflect the content of the currently negotiated SDP description, and this is independent of the SendCodecs and ReceiveCodecs (it's a shorthand for an SDP parser). |
Yes, that seems a useful way to look at it.
Internal slots are cheap, as long as we're not confusing things we can do it a couple of ways. But for consistency with 1.0 (and TR) we may wish to keep [[SendCodecs]] and [[ReceiveCodecs]] the same, doing their old job of reflecting currentDescription (at least as implemented when it comes to [[ReceiveCodecs]]). E.g. start out empty (when currentDescription is null). In #2956 (comment) I proposed some new internal slots. |
Having slept on it, it seems that we've been using SendCodecs and ReceiveCodecs for two different things:
Suggested new names: |
(I'll refer to the https://www.w3.org/TR/webrtc version of the spec here to avoid the #2956 regression on github tip.)
When I test Chrome, Edge, and Safari with https://jsfiddle.net/jib1/9kz2bf85/ by selecting "VP9" and clicking
Negotiate
, I see (because it uses setCodecPreferences with a truncated array for brevity):All three browsers return an empty codec array until initial negotiation has finished (
"stable"
).If I now select "H264" and hit
Negotiate
again, instead of empty codecs, I see "VP9" until renegotiation has finished and it's replaced with "H264".This seems nice and simple: sender/receiver.getParameters().codecs reflects what's been (fully) negotiated.
We should probably align the spec to say this.
Fixing the spec means:
The text was updated successfully, but these errors were encountered: