What is setCodecPreferences's contract?

[jan-ivar]

If codecs were interfaces (platform objects passed by reference) with read only members, then prose in setCodecPreferences would make sense: "The codecs sequence passed into setCodecPreferences can only contain codecs that are returned by RTCRtpSender.getCapabilities(kind) or RTCRtpReceiver.getCapabilities(kind)".

The user agent could easily enforce this, giving us a nice and tight contract:

const {codecs} = RTCRtpSender.getCapabilities(transceiver.receiver.track.kind);
transceiver.setCodecPreferences([codecs[0]]); // OK
transceiver.setCodecPreferences([{mimeType: "video/VP8", clockRate: 90000}]); // InvalidModificationError

...because the second input clearly did NOT come from RTCRtpSender.getCapabilities() or RTCRtpReceiver.getCapabilities().

Q1: Was this the intended contract?

Unfortunately, codecs aren't platform objects passed by reference, so there's no way to ensure they came from either.

" ... Additionally, the RTCRtpCodecCapability dictionary members cannot be modified."

Um, yes they can!

" ... If codecs does not fulfill these requirements, the user agent MUST throw an InvalidModificationError."

The intent here seems to be to ensure the dictionaries have not been modified since they were obtained using RTCRtp*.getCapabilities(). Again, since we have no way to know they came from there, the closest thing we could do would be to check that a serialized representation of the dictionary matches a serialized representation of one (or more) dictionaries that RTCRtp*.getCapabilities() regularly produces (i.e. a member-wise comparison).

But knowing this, should the second input now succeed or throw? Here it is again:

transceiver.setCodecPreferences([{mimeType: "video/VP8", clockRate: 90000}]);

1. The answer is NO in the spec, because we're missing channels and sdpFmtpLine, failing a member-wise compare with the output of RTCRtp*.getCapabilities():
2. The answer is YES in Chrome today, which only implements clockRate and mimeType so far, so MATCH! [Edit: This was pilot error on my part. channels is only audio-only]
3. The answer should be NO in Safari, because it implements sdpFmtpLine, but it's YES in WPT for some reason. [Edit: This was also pilot error on my part, since not all codecs have a sdpFmtpLine member.]

IOW whether a hard-coded input works or not seems dependent on implementation, and might change in the future.

This seems pretty bad. If such code becomes common practice, vendors might find themselves unable to extend the RTCRtpCodec dictionary in the future without breaking sites.

Options

1. Disallow hard-coded inputs (define codecs as interfaces (platform objects passed by reference) with read only members)
2. Allow partial-match hard-coded inputs (define some subset of members that satisfy a codec's identity for comparison purposes (e.g. mimeType and clockRate), and never change it.

[jan-ivar]

(I prematurely hit send when filling this, so please read the edited version above).

[Orphis]

Chrome does support sdpFmtpLine checks and you need to have a (loosely defined) match from values passed in setCodecPreferences() to the ones returned from getCapabilities().codecs. See: https://jsfiddle.net/e6z12na4/1/

Yes, those are dictionaries, you can create them out of thin air and (accidentally, or intentionally) match one of an implementation. It's meant to be a value semantics paradigm, and that's nothing new to developers. It's also probably too late to change those to platform objects as it will probably break existing code.

While the specification is not as precise as it should, the intent of the related APIs has been clear to developers for a while and most are also probably using it correctly.

By removing this part, we can get rid of most of your issues:

Additionally, the RTCRtpCodecCapability dictionary members cannot be modified. If codecs does not fulfill these requirements, the user agent MUST throw an InvalidModificationError.

And then we should just need to define a proper equality algorithm for codec objects, checking all fields from RTCRtpCodec just like current implementations are doing, which I said I'd be working on last Thursday.

[jan-ivar]

Our concern is that the API allows hardcoded inputs, which if it becomes a prevalent pattern causes implementation-dependent failures, and web compat headaches for non-dominant browsers in particular.

This is consistent with w3c/webrtc-extensions#130, where we mitigated the risk by requiring input that was harder to hardcode by accident (by requiring the same array length as that of getter output). Unfortunately, that doesn't work here, since setCodecPreferences has different (don't use) semantics for missing entries.

It's worth noting w3c/webrtc-extensions#130 was resolved without resorting to vending platform objects. Switching to platform objects would cause its own headaches, like making it harder to partially shim these APIs, unless we provide a constructors for the platform objects, but then the mitigation isn't that strong.

Given all that, I see a strong argument to align with existing implementations here, and a match algorithm like the one proposed in #2847 might be the right direction, but would like to hear from @youennf since I believe the WPT with hardcoded codec came from Safari?

But it's not clear from the PR whether we are:

1. Cementing codec comparison to ALWAYS be by clockRate, mimeType, channels and sdpFmtpLine, and no future members, or
2. We intend to extend the comparison algorithm in the future to include future members.

If it's 2, don't we risk breaking existing sites? Also, if it's 2, we can simplify the algorithm to compare ALL members so we don't need to update it in the future. @Orphis

[Orphis]

Our concern is that the API allows hardcoded inputs, which if it becomes a prevalent pattern causes implementation-dependent failures, and web compat headaches for non-dominant browsers in particular.

I don't think hardcoded inputs would work well for applications or most codecs, except maybe for VP8. Most other codecs have FMTP parameters and make them too inconvenient to hardcode in most applications, since those may vary depending on the platform or HW support. While it's a risk, I believe it to be low as it is a lot harder to hardcode values than to do something like:

transceiver.setCodecPreferences(findCodecs(getCapabilities('video').codecs, ['VP9', 'H264']));

But it's not clear from the PR whether we are:

• Cementing codec comparison to ALWAYS be by clockRate, mimeType, channels and sdpFmtpLine, and no future members, or
• We intend to extend the comparison algorithm in the future to include future members.

I believe we should probably match what I would call keyable fields of the dictionaries, which are now located in RTCRtpCodec and uniquely identify a codec that would result in the a=rtpmap entries in SDP.

For example, should be have kept scalabilityModes from WEBRTC-SVC as an extension to RTCRtpCodecCapability, I believe that we may not find it relevant to check the array values as they are a property of a codec but don't uniquely identify one.
In this regard, I believe we could change setCodecPreferences() to maybe take a sequence<RTCRtpCodec> instead.

In any case, any future change would have to be backwards compatible with current observed usage of the API. Considering that even adding a field changes the iteration order, which is guaranteed by WebIDL, even that could break applications. While we can't be certain of never ever breaking any application, we can only do our best to avoid it, and we're never at risk that some application relies on some probably unreasonable parts of our exposed API.

If it's 2, don't we risk breaking existing sites? Also, if it's 2, we can simplify the algorithm to compare ALL members so we don't need to update it in the future.

If you can provide me with language doing this, I could integrate it in the PR, otherwise we could update the algorithm whenever we need to. It's simple enough as it is.

[youennf]

but would like to hear from @youennf since I believe the WPT with hardcoded codec came from Safari?

@jan-ivar , if you are talking about WPT, this was hardcoded as a convenience, not as a pattern. We could definitely update this test.

My take on this is that MediaCapabilities should provide a convenient way to get a well formed codec object. See w3c/media-capabilities#185.