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

Remove sender, receiver and transceiver. #628

Merged
merged 5 commits into from
Jun 9, 2022

Conversation

henbos
Copy link
Collaborator

@henbos henbos commented Apr 7, 2022

One of the steps needed for #621.

AFTER THIS PR HAS BEEN APPROVED BUT BEFORE LANDING IT:
Create a corresponding webrtc-provisional-stats PR adding these metrics to that document instead.

This removes the sender, receiver and transceiver stats objects that were never implemented.

This PR also moves some attributes from transceiver to inbound/outbound-rtp to fill the missing holes.

  • trackIdentifier and kind are added at inbound-rtp. This info is currently only implemented in obsolete track stats, so without receiver stats this information needs to be available in inbound-rtp in order to get rid of obsolete track stats.
  • They're NOT added to outbound-rtp though, since this information is already available via the media-source in that case.
  • mid is added to inbound/outbound-rtp. Though technically this one is at risk, so perhaps we should just remove it?

Preview | Diff

@henbos
Copy link
Collaborator Author

henbos commented Apr 7, 2022

If this PR lands, I would lobby towards merging the "handler" stats dictionaries since these are no longer used outside of the obsolete section, and the only remaining reference to these things is for the definition of the obsolete RTCMediaStreamTrackStats dictionary.

It would be a lot more clear to the reader if the obsolete section actually contained what is implemented and observed in JavaScript rather than a hierarchy in the spec that does not map to anything observable (in either code or JS)

@henbos
Copy link
Collaborator Author

henbos commented Jun 9, 2022

Now that sender and receiver stats are gone we'll be able to flatten the hierarchy... we can just have obsolete "track" stats and get rid of the entire confusing hierarchy that was there for the sake of not having to define "track", "sender" and "receiver" metrics in multiple places...

In the interest of making those editorial changes possible, I'll simply NOT add sender, receiver and transceiver to the provisional spec. Nobody every implemented any of it and if we want to add them back later it will be easier to add them back from scratch without the hierarchy

FYI @alvestrand @vr000m

@jan-ivar
Copy link
Member

This merge includes normative changes lacking agreement. It purports to fix Editorial #621, a "move to ... webrtc-provisional-stats"), yet "sender", "receiver" and "transceiver" stats are not there, but gone. That's not "editorial".

This WG has regular weekly Editor's Meetings on Thursdays where PRs are merged or labeled Editors can integrate for merging outside of those meetings. This didn't happen (May 3rd was a Tuesday). The PR also lacks review.

Instead, a new non-editorial issue should have been opened for visibility, and member participation sought. I've opened #643 to have this discussion now.

@henbos
Copy link
Collaborator Author

henbos commented Jun 18, 2022

Yes it was a Tuesday because that’s when me, Varun and Harald have the biweekly webrtc stats meeting. This is how the spec has been sheperded for several years. It was reviewed as part of that meeting (Varun was present, Harald was not but had given the thumbs up to do merges while he was gone.)

See #628 (comment) documenting what happened there (which was discussed on Tuesday). But to correct this not being editorial I filed w3c/webrtc-provisional-stats#32 to make sure they get added back to the provisional spec.

The lifetime of the objects is definitely worth following up on but the current status is that sender/receiver/transceiver objects add no new information except for a lifetime statement that has lacked implementer interest since the dictionaries’ conception.

Let’s add them to provisional and continue the lifetime discussion in a separate issue

@henbos
Copy link
Collaborator Author

henbos commented Jun 18, 2022

@alvestrand @vr000m

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

Successfully merging this pull request may close these issues.

3 participants