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

RTCRtpCodecCapability is superfluous and should be deleted from the spec. #2974

Closed
alvestrand opened this issue May 16, 2024 · 3 comments · Fixed by #2976
Closed

RTCRtpCodecCapability is superfluous and should be deleted from the spec. #2974

alvestrand opened this issue May 16, 2024 · 3 comments · Fixed by #2976

Comments

@alvestrand
Copy link
Contributor

At the moment we have the definition:

dictionary RTCRtpCodecCapability : RTCRtpCodec {
};

The text talks about what codecs should be a member of a set of RTCRtpCodecCapability, which doesn't belong there.

This dictionary isn't carrying its weight. Proposing to delete it.

@fippo
Copy link
Contributor

fippo commented Jun 14, 2024

As a result of this getCapabilities returns

dictionary RTCRtpCapabilities {
  required sequence<RTCRtpCodec> codecs;
  required sequence<RTCRtpHeaderExtensionCapability> headerExtensions;
};

which seems quite inconsistent? The more consistent way to collapse this would have been to fold RTCRtpCodec into this?

@jan-ivar
Copy link
Member

Is the consistency concern over naming or hierarchy?

RTCRtpCodecs appear both in getCapabilities() and getParameters() #2834 so it seems wrong to label them either.

They're valid inputs to setCodecPreferences and setParameters(Object.assign(params, {codec}))

Hierarchy-wise It seems wrong for RTCRtpCodecParameters to inherit from RTCRtpCodecCapability.

IMHO the word "Capability" isn't carrying a lot of weight here.

Would s/RTCRtpHeaderExtensionCapability/RTCRtpHeaderExtension/ help?

@fippo
Copy link
Contributor

fippo commented Jun 15, 2024

I've always seen the "capability" as a "I support" in the sense of XMPPs entity caps which had a nice mapping to ORTC.

The way this traditionally worked was that getCapabilities returned static capabilities, getParameters the negotiated parameters (which include a payload type which is a dynamic thing). The negotiation is adding the dynamic payload type to the parameters.
The semantic difference is that caps are "I can use" while parameters are "I use" (obviously you can not use something you are not capable of doing)

The goal of #2834 was fine but missing that these objects semantically build ontop of each other. Same for header extensions. Now whether the payload type member is even required is questionable, it is read-only and can not be changed as it is determined by SDP negotiation.

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

Successfully merging a pull request may close this issue.

3 participants