Inconsistent initialization of scaleResolutionDownBy
addTransceiver says: "7. If the scaleResolutionDownBy attributes of sendEncodings are still undefined, initialize [them] ... 9. Create an RTCRtpSender with track, kind, streams and sendEncodings ..."
Create an RTCRtpSender says: "If sendEncodings is given as input to this algorithm, and is non-empty, set the [[SendEncodings]] slot to sendEncodings. Otherwise, set it to a list containing a single RTCRtpEncodingParameters with active set to true."
This means that (according to spec) the output of sender.getParameters().encodings[0] will differ in the following cases:
pc.addTrack(track) // {active: true}
pc.addTransceiver(track) // {active: true}
pc.addTransceiver(track, {sendEncodings: []}) // {active: true}
pc.addTransceiver(track, {sendEncodings: [{}]}) // {active: true, scaleResolutionDownBy: 1}
pc.addTransceiver(track, {sendEncodings: [{rid: 1}]}) // {active: true, scaleResolutionDownBy: 1}
A quick test (working around #2729) shows Chrome never initializes scaleResolutionDownBy, while Safari always does.
I think we're better off always initializing scaleResolutionDownBy, or we need to fix setParameters: "Verify that each encoding in encodings has a scaleResolutionDownBy member whose value is greater than or equal to 1.0. If one of the scaleResolutionDownBy values does not meet this requirement, return a promise rejected with a newly created RangeError."
It would be tempting to default scaleResolutionDownBy to 1 in the IDL, but that can't be done because the sendEncodings field is defined for both audio and video (although I'm unaware of anyone using it for audio).
The initialization rule given in https://w3c.github.io/webrtc-pc/#dom-rtcrtpencodingparameters-scaleresolutiondownby is consistent with always initializing it to 1 when there is a single layer.
It would be tempting to default scaleResolutionDownBy to 1 in the IDL, but that can't be done because the sendEncodings field is defined for both audio and video (although I'm unaware of anyone using it for audio).
The initialization rule given in https://w3c.github.io/webrtc-pc/#dom-rtcrtpencodingparameters-scaleresolutiondownby is consistent with always initializing it to 1 when there is a single layer.
On the subject of audio; what should we be doing when JS passes scaleResolutionDownBy to addTransceiver('audio')? TypeError? The spec clearly says that it is invalid to use scaleResolutionDownBy in the audio case.
Also, the steps in addTransceiver require auto-filling scaleResolutionDownBy regardless of the media kind (either the "fill with 1" case, or the "powers of 2" case). This is almost certainly not what we want. That might also be a TypeError?
Another thing; it probably doesn't make much sense to throw RangeError when scaleResolutionDownBy is undefined in setParameters. If we want to throw something in this case, I think InvalidModificationError is appropriate.
Another thing; it probably doesn't make much sense to throw RangeError when scaleResolutionDownBy is undefined in setParameters. If we want to throw something in this case, I think InvalidModificationError is appropriate.
I don't think we should throw. Safari (the only browser that defaults to scaleResolutionDownBy: 1 correctly) treats absence of scaleResolutionDownBy in setParameters as scaleResolutionDownBy: 1.
This seems correct as it's what would have happened had we been able to specify a default value for scaleResolutionDownBy in WebIDL like we do for the other parameters, as @alvestrand mentions in https://github.com/w3c/webrtc-pc/issues/2730#issuecomment-1120469775.
... had we been able to specify a default value for scaleResolutionDownBy in WebIDL ...
Hmm, except the overall default in addTransceiver isn't all 1s, but a "smart" 2^(length of sendEncodings - encoding index - 1) if all are untouched, but nobody implements that yet it seems?
So should the following produce 1,1,1 or 4,2,1?
const pc = new RTCPeerConnection();
const tc = pc.addTransceiver("video", {sendEncodings: [{rid: 1}, {rid: 2}, {rid: 3}]);
const params = tc.sender.getParameters();
delete params.encodings[0].scaleResolutionDownBy;
delete params.encodings[1].scaleResolutionDownBy;
delete params.encodings[2].scaleResolutionDownBy;
await tc.sender.setParameters(params);
So should the following produce 1,1,1 or 4,2,1?
That's an interesting question. It gets more interesting when you ask what happens when that setParameters occurs right after a sRD(initial simulcast offer), which is then rolled back.
Or, what happens if only some of the scaleResolutionDownBy are unset?
I think my inclination is to just set to 1 regardless. Too many wonky corner cases pop out if you do anything but that. Or, I guess we could throw an error. Seems weird that anyone would want to do something like that.
I got this comment out-of-band:
have you considered changing scaleResolutionDownBy to a field that is ignored for audio (instead of being an error)? That would mean you 1) could set a default in WebIDL, and 2) wouldn't need to come up with an error for passing it to audio.
This seems reasonable to me, as dictionaries are generally only examined for expected inputs. We'd have to not just ignore it but remove it, or it'd show up in getParameters() which would be undesirable, but I think this is a winner. Updating PR.
It gets more interesting when you ask what happens when that setParameters occurs right after a sRD(initial simulcast offer), which is then rolled back.
I think that's https://github.com/w3c/webrtc-pc/issues/2764 which seems orthogonal to defaults.
I think deleting parameters is odd enough that 1,1,1 seems fine.
Note we still can't set a default in WebIDL, because we need to detect absence to apply the smart (4,2,1) default.