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

Existing setCodecPreferences NOTE is wrong and should be deleted #2933

Closed
henbos opened this issue Feb 2, 2024 · 8 comments · Fixed by #2941
Closed

Existing setCodecPreferences NOTE is wrong and should be deleted #2933

henbos opened this issue Feb 2, 2024 · 8 comments · Fixed by #2941

Comments

@henbos
Copy link
Contributor

henbos commented Feb 2, 2024

The second NOTE under setCodecPreferences says:

NOTE
Due to a recommendation in [SDP], calls to createAnswer SHOULD use only the common subset of the codec preferences and the codecs that appear in the offer. For example, if codec preferences are "C, B, A", but only codecs "A, B" were offered, the answer should only contain codecs "B, A". However, [RFC8829] (section 5.3.1.) allows adding codecs that were not in the offer, so implementations can behave differently.

This does not advocate what we want for the unidirectional use case and I can't find a justification for it. I think it's probably based on a misunderstanding about the difference between preferences and PT mappings.

Proposal: Delete this note.

@fippo
Copy link
Contributor

fippo commented Feb 2, 2024

This is fine, it refers to this text in 5.3.1?

In either case, the media formats in the answer MUST include at least one format that is present in the offer but MAY include
formats that are locally supported but not present in the offer, as mentioned in [[RFC3264](https://www.rfc-
editor.org/rfc/rfc8829.html#RFC3264)], Section 6.1

@henbos
Copy link
Contributor Author

henbos commented Feb 2, 2024

JSEP says MAY add new formats, this note says SHOULD as in, should NOT add new formats but it could "so implementations can behave different", and webrtc-pc createAnswer says to use the preferences from setCodecPreferences, which has normative steps to use the preference, I take that as we MUST NOT follow our own NOTE.

Am I confused or is the note confused?

@fippo
Copy link
Contributor

fippo commented Feb 2, 2024

It says the answer SHOULD be a subset of the offer which is a good expectation in general. But I agree, this seems redundant to JSEP and not helpful at this level.

@alvestrand
Copy link
Contributor

I tried to make the rules for what codecs to include more explicit in #2935 - I don't know if I succeeded, but I didn't touch the note there.

@alvestrand
Copy link
Contributor

Discussion on WG meeting Feb 20, 2024: Delete the note.

@fippo
Copy link
Contributor

fippo commented Feb 20, 2024

Will add a test along the lines of

A: sCP(vp8, vp9)
B: sCP(h264, vp9, vp8)
Answer from B says (vp9, vp8)

which should sufficiently cover the note that is going to be gone soon. 🤞 that the additional codecs used are available on all browsers.

@dontcallmedom-bot
Copy link

This issue had an associated resolution in WebRTC February 2024 meeting – 20 February 2024 (Existing setCodecPreferences NOTE is wrong and should be deleted #2933):

RESOLUTION: Fippo to submit a PR to remove note and a matching WPT test

@fippo
Copy link
Contributor

fippo commented Feb 22, 2024

image
As expected 🎉

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.

5 participants