w3c / webrtc-pc

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

Allow level-id to be different in codec match #3023

Open henbos opened 1 day ago

henbos commented 1 day ago

Fixes #3020


Preview | Diff

henbos commented 1 day ago

@aboba Does this wording make sense to you?

fippo commented 1 day ago

if you want to go this route you will also need to take care of RFC6184s level-asymmetry-allowed, no? All of this is reasonably well-defined in IETF specs though (well, not quite for HEVC) already

henbos commented 1 day ago

if you want to go this route you will also need to take care of RFC6184s level-asymmetry-allowed, no?

No this is just about the codec matching algorithm used to sanity check input to setParameters/setCodecPreferences, what is actually negotiated based on this is defined somewhere else.

As for sending the selected codec, the description is kind of vague, RTCRtpEncodingParmaters.codec just says:

Optional value selecting which codec is used for this encoding's RTP stream. If absent, the user agent can chose to use any negotiated codec.

And setParameters just says:

In parallel, configure the media stack to use parameters to transmit sender.[[SenderTrack]].

So it's just "codec used for selecting what to send", we might want to create an issue about clarifying not to send greater than the level-id that was negotiated, but that seems more like an editorial note.

henbos commented 1 day ago

not to send greater than the level-id

We might want to update getParameters().encodings[i].codec to match the level-id that you find in getParamters().codecs though? Separate issue though because that is the same as what happens with setCodecPreferences() today

henbos commented 13 hours ago

Codec wizard @aboba, can you do another review?

henbos commented 11 hours ago

Btw I had said during the meeting that I thought all instances of "codec matching" could ignore level, but upon a second look I realized that we don't need to be that "loose" in most cases. For example if you addTransceiver, the only valid input is from getCapabilities since there is no chance that anything else has been negotiated for that transceiver yet. It might not hurt to be more loose here, but it does not seem to help either. Ultimately we only need to "ignoreLevels=true" at a few places:

In all other cases we can "ignoreLevels=false", which the PR reflects now.