w3c / webrtc-pc

WebRTC 1.0 API
https://w3c.github.io/webrtc-pc/
Other
436 stars 115 forks source link

Existing setCodecPreferences NOTE is wrong and should be deleted #2933

Closed henbos closed 7 months ago

henbos commented 8 months ago

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 commented 8 months ago

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], Section 6.1

henbos commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

I tried to make the rules for what codecs to include more explicit in https://github.com/w3c/webrtc-pc/pull/2935 - I don't know if I succeeded, but I didn't touch the note there.

alvestrand commented 7 months ago

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

fippo commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

image As expected 🎉