Skip to content
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

receiver.getParameters().codecs is broken (regression) #2956

Closed
jan-ivar opened this issue Apr 16, 2024 · 4 comments · Fixed by #2999 · May be fixed by #2972
Closed

receiver.getParameters().codecs is broken (regression) #2956

jan-ivar opened this issue Apr 16, 2024 · 4 comments · Fixed by #2999 · May be fixed by #2972

Comments

@jan-ivar
Copy link
Member

receiver.getParameters has this non-normative note:

image

It suggests that post sRD(answer), receiver.getParameters().codecs is updated to only reflect what's been negotiated.

But I can't find any normative steps to support this note:

  • [[ReceiveCodecs]] is initialized to "the list of implemented receive codecs for kind"
  • [[ReceiveCodecs]] is only ever assigned to [[LastStableStateReceiveCodecs]] (on rollback)
  • [[ReceiveCodecs]] does not appear to ever be modified
  • The "enabled" flag is only ever set to true

Is the note wrong or are we missing some normative steps? What was the intent?

@jan-ivar
Copy link
Member Author

This turns out to be regression from #2935. Prior to that it said this in sLD:

image

...and this in create an RTCRtpTransceiver:
image

I.e. it starts out empty and is then populated based on negotiation (which may be non-empty or empty).

But with #2935 this got replaced with this in sLD:
image
...and this in create an RTCRtpTransceiver:
image

I.e. it starts out being entirely implementation-defined whether it is empty or includes the full set or something in between , and then "enabled" is only ever set to true, never false.

This means it cannot possibly represent the (subset of) codecs that were negotiated, which seems broken.

@jan-ivar jan-ivar changed the title receiver.getParameters().codecs seems under-specified. receiver.getParameters().codecs is broken (regression) Apr 26, 2024
@jan-ivar
Copy link
Member Author

@alvestrand what are next steps? Do we back out #2935 for now, or do you have a PR in the hopper?

From #2925 (comment):

Suggested revised model

... When calling setCodecPreferences, checking is done against receiver.[[codecs]], not against sender/receiver.getCapabilities(). If setCodecPreferences() includes a codec with the “enabled” flag set to false in the receiver’s [[codecs]] slot, it is set to “true”.

We probably need new internal slots for this instead, e.g. transceiver.[[SendCodecCapabilities]] and [[ReceiveCodecCapabilities]] (defaulting to the implementation list), as appropriating [[SendCodecs]] or [[ReceiveCodecs]] for this (defaulting to empty) seems to interfere with their existing purpose of reflecting what's been negotiated (#2967).

@jan-ivar
Copy link
Member Author

From #2925 (comment):

Suggested revised model
... When calling setCodecPreferences, checking is done against receiver.[[codecs]], not against sender/receiver.getCapabilities(). If setCodecPreferences() includes a codec with the “enabled” flag set to false in the receiver’s [[codecs]] slot, it is set to “true”.

Also, was part of the intent here to repurpose receiver.getParameters().codecs to be an input source for setCodecPreferences?

If so, do we need non-static versions of sender/receiver.getCapabilities()?

@alvestrand
Copy link
Contributor

alvestrand commented Apr 29, 2024

May look as if we had a name collision between previous [[SendCodecs]] at the PC level and [[SendCodecs]] at the sender/receiver level, and I did not detect it when moving [[SendCodecs]] (which I thought unique) to the sender/receiver level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants