w3c / webrtc-pc

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

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

Open jan-ivar opened 2 months ago

jan-ivar commented 2 months ago

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:

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

jan-ivar commented 2 months ago

This turns out to be regression from https://github.com/w3c/webrtc-pc/pull/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 https://github.com/w3c/webrtc-pc/pull/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 commented 2 months ago

@alvestrand what are next steps? Do we back out https://github.com/w3c/webrtc-pc/pull/2935 for now, or do you have a PR in the hopper?

From https://github.com/w3c/webrtc-pc/issues/2925#issue-2078387100:

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

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

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.