Skip to content

RTCRtpSender.getParameters - remove returned dictionary?#18834

Merged
queengooborg merged 7 commits intomdn:mainfrom
hamishwillee:RTCRtpSender_getParameters
Feb 14, 2023
Merged

RTCRtpSender.getParameters - remove returned dictionary?#18834
queengooborg merged 7 commits intomdn:mainfrom
hamishwillee:RTCRtpSender_getParameters

Conversation

@hamishwillee
Copy link
Contributor

@hamishwillee hamishwillee commented Feb 7, 2023

RTCRtpSender.getParameters() returns a dictionary object of type RTCRtpSendParameters, which in turn derives from RTCRtpParameters. The values in RTCRtpParameters are returned in all browsers but are not implemented in Firefox - i.e. they are empty or nothing values.

@queengooborg My intent is to remove RTCRtpSendParameters and RTCRtpParameters because we don't document dictionaries in general.

I think the right thing to do is to add a subfeature for each option in the returned object under RTCRtpSender.getParameters().
Assuming I am correct, what is the naming pattern to represent each of the values of a returned object? Something like return_object-has_Xxxx?

Note the actual PR does not do that (yet). All it does is add a note in RTCRtpSender.getParameters() to indicate that the FF implementation is partial and omits some values. This is just a placeholder.
Note that this is IMO not sufficient because it loses information like the existance of the degradationPreference value in RTCRtpSendParameters (obsolete, but still present)

@github-actions github-actions bot added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Feb 7, 2023
@hamishwillee
Copy link
Contributor Author

hamishwillee commented Feb 7, 2023

@queengooborg NOTE that I only intend to do this case because the values in RTCRtpParameters are not otherwise in BCD (and I will have to also do this for RTCRtpReceiver.getParameters() too.

Each one of these keys is also a set of objects - so for example return_object_has_encodings references an array of
encodings RTCRtpEncodingParameters objects. It all gets pretty messy at this point so my intent is to stop after doing this one and not nest deeper.

Unless you think this is a cunning plan?

Co-authored-by: Queen Vinyl Da.i'gyu-Kazotetsu <vinyldarkscratch@gmail.com>
@hamishwillee
Copy link
Contributor Author

Thanks @queengooborg Please merge this.

I didn't see a response to the question below, so I'll just do it the way I want and see what you say :-)

Assuming I am correct, what is the naming pattern to represent each of the values of a returned object? Something like return_object-has_Xxxx?

@hamishwillee
Copy link
Contributor Author

hamishwillee commented Feb 13, 2023

@queengooborg Updated significantly - needs checking.

So we have a dict RTCRtpSendParameters which extends RTCRtpParameters. I've pulled the information from the dicts into the methods that use them as return values and as parameter options:

  • RTCRtpSender.getParameters() sets RTCRtpSendParameters
  • RTCRtpSender.setParameters() set RTCRtpSendParameters
  • RTCRtpReceiver.getParameters() - returns RTCRtpParameters

This copies the values for RTCRtpSendParameters in BCD - so those should be right.
RTCRtpParameters was not in BCD, but this is what "came first" so should more closely match the original implementation of method that calls it. Where possible I have tested - so for example I know that the values for safari should be correct.

  1. Is this naming convention/structure OK? In particular note the return value option names

  2. Note that FF110 brought FF "up to spec". However it is hard to clarify exactly what that means in terms of changes. Until we have concrete information about what changed, my intent is just to not mention FF110 at all.

  3. Next step would be to delete RTCRtpSendParameters and RTCRtpParameters (follow on PR)

"version_added": "≤79"
},
"firefox": {
"version_added": "46"
Copy link
Contributor Author

@hamishwillee hamishwillee Feb 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, there was a note in RTCRtpSendParameters compat data that this was implemented in RTCRtpParameters. However since we don't care about the dictionary I have simply removed the comment.

Is that OK? More specifically, spec compliance can be odd in that it expects you to implement in a particular way that will have no impact on compatibility.

Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is LGTM, thank you! Always down for things we can do to remove the dictionaries that are still around!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants