Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should RTCRtpSender.setParameters invalidate LastReturnedParameters? #1857

Closed
fippo opened this issue Apr 27, 2018 · 3 comments
Closed

should RTCRtpSender.setParameters invalidate LastReturnedParameters? #1857

fippo opened this issue Apr 27, 2018 · 3 comments

Comments

@fippo
Copy link
Contributor

fippo commented Apr 27, 2018

from discussion between me and @Orphis:

I assume we want getParameters/setParameters to be used like this:

const sender = pc.getSenders()[0];
const parameters = sender.getParameters();
parameters.encodings[0].maxBitrate = 500000;
sender.setParameters(parameters);

but in Chrome's current implementation (very new; still behind flag) one can use those parameters multiple times which afaics matches the spec.
I assume we do not want this:

const sender = pc.getSenders()[0];
const parameters = sender.getParameters();
parameters.encodings[0].maxBitrate = 500000;
sender.setParameters(parameters)
.then(() => {
  // fiddle with something else in parameters;
  sender.setParameters(parameters);
});

as we would have to determine whether the modification conflicts.
The easiest way to achieve this seems to have setParameters clear LastReturnedParameters.

Test-as-you-commit:
needs a test showing that the second bad example fails.

@alvestrand
Copy link
Contributor

I concur, and believe that the current procedure was a drafting error.

@taylor-b
Copy link
Contributor

Related question: does setRemoteDescription (or setLocalDescription) invalidate the parameters? Currently it appears they don't. But setRemoteDescription could cause a completely different set of parameters to be returned.

@aboba
Copy link
Contributor

aboba commented Apr 27, 2018

@taylor-b In general, sender.setParameters() cannot set the negotiationneeded flag. If this is a hard and fast rule, this suggests that the writeable attributes in RTCRtpSendParameters cannot be changed by SLD/SRD. My reasoning is that if a writeable attribute could be changed by SLD/SRD, then calling sender.setParameters()to change that writeable attribute to something else afterwards should set the negotiationneeded flag, which we agreed could not happen.

Does that argument make sense?

In looking through the RTCRtpSenderParameters, the only potential gray area about writeable attributes being changed by SLD/SRD appears to be codecPayloadType. While read-only attributes such as rid can be changed by SLD/SRD that is expected.

For example, consider the sender side:

dictionary RTCRtpSendParameters : RTCRtpParameters {
required DOMString transactionId;
required sequence encodings;
RTCDegradationPreference degradationPreference = "balanced";
};

Neither SRD nor SLD change degradationPreference. With respect to encoding parameters, we have:

dictionary RTCRtpEncodingParameters : RTCRtpCodingParameters {
octet codecPayloadType;
RTCDtxStatus dtx;
boolean active = true;
RTCPriorityType priority = "low";
unsigned long ptime;
unsigned long maxBitrate;
double maxFramerate;
double scaleResolutionDownBy;
};

The rid attribute (read-only) can be changed by SLD/SRD. codecPayloadType is fuzzier. The text says that the codecPayloadType attribute is used to select a codec to be sent, but assuming it is one of the codecs included in createOffer, calling SLD should not change it. However, if an Answer selects another codec, would codecPayloadType change when SRD is called? Overall, I'm not very clear why it is valuable to have codecPayloadType in RTCRtpEncodingParameters; if the goal is to set codec preferences, isn't that handled by setCodecPreferences?

For the receiver, situation is somewhat more unclear, since I'm not sure why we need the rid attribute in RTCRtpDecodingParameters, or whether a codecPayloadType attribute is needed there. I think I've convinced myself that neither attribute is needed.

Orphis added a commit to Orphis/webrtc-pc that referenced this issue May 2, 2018
This prevents sequences of getParameters() and then
multiple setParameters() call.

Fixes w3c#1857
Orphis added a commit to Orphis/webrtc-pc that referenced this issue May 3, 2018
This prevents sequences of getParameters() and then
multiple setParameters() call.

Fixes w3c#1857
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants