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

RTCAudioReceiverStats and RTCAudioSenderStats aren't MTI #2458

Closed
dontcallmedom opened this issue Feb 3, 2020 · 5 comments · Fixed by #2548
Closed

RTCAudioReceiverStats and RTCAudioSenderStats aren't MTI #2458

dontcallmedom opened this issue Feb 3, 2020 · 5 comments · Fixed by #2548

Comments

@dontcallmedom
Copy link
Member

RTCVideoReceiverStats and RTCVideoSenderStats are listed as mandatory to implement but their audio equivalent RTCAudioReceiverStats and RTCAudioSenderStats aren't - I assume this isn't intended.

@jan-ivar
Copy link
Member

jan-ivar commented Feb 3, 2020

RTCAudioReceiverStats is empty, but RTCAudioSenderStats is not. (Updated)

In looking at this, the wording "with all required attributes from its inherited dictionaries" seems unclear in hindsight. I think we mean s/required/mandatory/ and not e.g. the required WebIDL attribute.

Otherwise I think we'd need to explicitly exclude obsolete members.

@dontcallmedom
Copy link
Member Author

Did you mean (as you linked to) RTCAudioSenderStats rather than RTCVideoReceiverStats?

in any case, there are other empty dictionaries listed as MTI, so I'm not sure it's an argument for or against inclusion (if you were suggesting it, which I'm not sure either :)

(separately, I wonder if the MTI stats should also list the RTCStatsType entries that are expected - listing dictionaries only very indirectly answer the questions of which objects need to appear in an MTI-compliant getStats().)

@alvestrand
Copy link
Contributor

Seems that we should remove all empty dictionaries from the list. They don't add anything to requirements.

@jan-ivar
Copy link
Member

jan-ivar commented Feb 6, 2020

Also, s/required/mandatory/

@henbos
Copy link
Contributor

henbos commented Feb 6, 2020

I suggest listing stats object (instances) rather than dictionaries

@henbos henbos self-assigned this Feb 6, 2020
dontcallmedom added a commit that referenced this issue Jun 22, 2020
rather than indirectly via the dictionaries that define their fields
per discussion in #2458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants