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

Move MediaStreamTrack stats in its own spec? #132

Open
dontcallmedom opened this issue Nov 17, 2023 · 10 comments
Open

Move MediaStreamTrack stats in its own spec? #132

dontcallmedom opened this issue Nov 17, 2023 · 10 comments

Comments

@dontcallmedom
Copy link
Member

With the document in this repo being a mix of ideas with different levels of support, implementation and maintenance, I think we should only add new features in it if there isn't a better place.

The default would be to add them in mediacatpure-main, but presumably we would only want to do that once we have double implementation to avoid creating features at risk in the current CR (although maybe that wouldn't be so bad given that they could be be kept as "candidate additions" in a possible future Rec).

But when we have features that layer clearly on top of mediacapture-main (e.g. don't need monkey patching algorithms), a reasonably simple approach would be to publish them as independent spec. This has other benefits - clearly separating the relevant tests in WPT, clearly separating the relevant issues in a repo of their own; simplifying their exposure in webref and its downstream ecosystem (MDN, BCD, WPT, …)

This seems to apply to the recent addition of stats (some of which are starting to ship in Chromium from what I gather), and so I would suggest we move the two said sections in a spec of their own. Thoughts? I'm happy to take care of the logistics of the split, although we would need to have a committed editor.

@alvestrand
Copy link
Contributor

Seems like a good idea to me. This spec would (I think) remain rather small, much smaller than webrtc-stats, but I guess that's not much of a disadvantage. @henbos ?

@henbos
Copy link
Contributor

henbos commented Nov 17, 2023

Carving out separate features like stats into a separate doc makes sense to me, especially as we start getting implementations. Chromium shipped video track stats already (M120, currently Beta) and we're hoping to get an intent to prototype approved for the audio stats shortly.

I can help with editing. Though it would still be owned by the same WG, so not much changes in terms of the practicalities?

@jan-ivar
Copy link
Member

With the document in this repo being a mix of ideas with different levels of support, implementation and maintenance, I think we should only add new features in it if there isn't a better place.

This seems inconsistent with the SPECNAME-extensions process in our merge guide, which says: "For extensions that are "small" (add a few methods, extend an argument list or enum, the function description is explanation enough), pull requests against SPECNAME-extensions is preferred."

MST stats seems "small" by that definition, and confined to device capture, the domain of mediacapture-main.

Are you suggesting a change to that process? I.e. do your concerns apply equally to webrtc-extensions, other features here (e.g. transferable MST), or is there something specific about stats?

FWIW I don't personally anticipate more MediaStreamTrack stats so I don't feel it needs its own standards track.

... clearly separating the relevant tests in WPT,

https://wpt.fyi/results/mediacapture-extensions right now:
image
Seems fine to me. Would backgroundBlur and faceFraming get their own specs as well?

clearly separating the relevant issues in a repo of their own

I confess I have the opposite problem of issues being spread over too many repos. This WG is tracking 19 publications already, and I'm not sure that's been a success. E.g. I'd personally prefer fewer issue trackers to check in on. Github hasn't exactly solved this problem.

@dontcallmedom
Copy link
Member Author

This seems inconsistent with the SPECNAME-extensions process in our merge guide

the merge guide is explicitly scoped to Rec-level documents, which Media Capture and Streams isn't, so this wouldn't be a change to the process afaict.

do your concerns apply equally to webrtc-extensions, other features here (e.g. transferable MST), or is there something specific about stats?

I don't think transferable MST can be developed independently of the rest of mediacapture-main, contrarily to stats; also, as far as I know, MST is implemented anywhere yet. I would be fine with having backgroundBlur and faceFraming moving to a separate document as well - I asked about stats because the problem of its inclusion in a document with an ill-defined status was brought up to me for this particular piece of the document.

I confess I have the opposite problem of issues being spread over too many repos. This WG is tracking 19 publications already, and I'm not sure that's been a success. E.g. I'd personally prefer fewer issue trackers to check in on. Github hasn't exactly solved this problem.

I'd be happy to look into solving that problem; I personally use a tool that lets me keep track of activity in issues and PRs across repos related to WebRTC that may be made to work for you as well; or we could decide to regroup issue tracking in fewer repos; or we could develop stats as a spec of its own in this very repo.

But I don't think this personal preference justify leaving normative content that is being implemented and shipped into a document whose legal and consensus status is impossible to relate to the W3C Process.

@jan-ivar
Copy link
Member

jan-ivar commented Nov 30, 2023

the merge guide is explicitly scoped to Rec-level documents, which Media Capture and Streams isn't, so this wouldn't be a change to the process afaict.

Sure, but it says: "... and the Mediacapture-Main spec is aiming for that status." implying that mediacapture-extensions exists in anticipation of mediacapture-main going to REC as the goals of not disturbing the main spec in order to stabilize it ahead of REC mimic those after.

Long-term I'd ask: is it destined for mediacapture-main or not? WebRTC getStats is defined in webrtc-pc, not webrtc-stats which is just a long list of identifiers I hope we won't see here.

I don't think transferable MST can be developed independently of the rest of mediacapture-main, contrarily to stats; also, as far as I know, MST is implemented anywhere yet. I would be fine with having backgroundBlur and faceFraming moving to a separate document as well - I asked about stats because the problem of its inclusion in a document with an ill-defined status was brought up to me for this particular piece of the document.

... But I don't think this personal preference justify leaving normative content that is being implemented and shipped into a document whose legal and consensus status is impossible to relate to the W3C Process.

mediacapture-extensions (and by extension webrtc-extensions) having an ill-defined status seems like the real problem here, to which moving MST stats is not the solution.

The status of MST stats is that it has not had a CfC, and concerns have been expressed that #117 merged prematurely, and that audio track stats lack consensus.

@dontcallmedom
Copy link
Member Author

Sure, but it says: "... and the Mediacapture-Main spec is aiming for that status." implying that mediacapture-extensions exists in anticipation of mediacapture-main going to REC as the goals of not disturbing the main spec in order to stabilize it ahead of REC mimic those after.

Correct; but mediacapture-main has not been getting to Rec in the 2+ years this guide has been adopted; it was fine as a temporary solution, but now we have shipping features spec'd in that document, and still no clear path to mediacapture-main getting to Rec.

Long-term I'd ask: is it destined for mediacapture-main or not? WebRTC getStats is defined in webrtc-pc, not webrtc-stats which is just a long list of identifiers I hope we won't see here.

I think it'd be fine to add it to mediacapture-main; and if we were to split it off to a separate document, we could easily merge it back once -main has gotten to Rec, and if it gains a second implementation before that.

mediacapture-extensions (and by extension webrtc-extensions) having an ill-defined status seems like the real problem here, to which moving MST stats is not the solution.

It is a partial solution at the very least, and one focused on a well-identified problem.

The status of MST stats is that it has not had a CfC, and concerns have been expressed that #117 merged prematurely, and that audio track stats lack consensus.

I'm confused - MST stats had review and consensus in the meetings they were reviewed (according to #117 (comment)); and #117 was merged after it was labeled "editors can integrate" with your oversight as far as I can tell. Now, that is obviously not to say that MST stats is finished; but I'm confused as to what consensus needs to be confirmed with a CfC at this point - now if we were to publish it as a FPWD, I agree this would definitely need a CfC.

@henbos
Copy link
Contributor

henbos commented Dec 4, 2023

I'm confused - MST stats had review and consensus in the meetings they were reviewed (according to #117 (comment)); and #117 was merged after it was #117 (comment) with your oversight as far as I can tell. Now, that is obviously not to say that MST stats is finished; but I'm confused as to what consensus needs to be confirmed with a CfC at this point - now if we were to publish it as a FPWD, I agree this would definitely need a CfC.

I got the editors can integrate based if Paul approves and Paul left comments regarding latency and stats caching, so as suggested by Jan-Ivar, I split up the PR as to merge and iterate with new PRs, but it turned out Paul was not happy about the dropped frames metrics for calculating glitches either. I did not understand this was controversial since all metrics had been approved at multiple Virtual Interims, but it was. So we followed up in the recent Virtual Interim and #129. So while we did follow up and merge another PR for latency metrics based on Paul and Youenn approving it here (#124), the dropped frames disagreement remains unresolved.

@dontcallmedom
Copy link
Member Author

thanks for the clarification @henbos - but IIUC this is about a specific item in the overall interface, not about the work happening at all

@henbos
Copy link
Contributor

henbos commented Dec 4, 2023

That's correct, the disagreement is only about a specific subset as far as I can tell

@jan-ivar
Copy link
Member

jan-ivar commented Dec 4, 2023

Correct; but mediacapture-main has not been getting to Rec in the 2+ years this guide has been adopted; it was fine as a temporary solution, but now we have shipping features spec'd in that document, and still no clear path to mediacapture-main getting to Rec.

The main problem there seems to be getting mediacapture-main to REC. Perhaps we should spend more meeting time on that?

I think it'd be fine to add it to mediacapture-main; and if we were to split it off to a separate document, we could easily merge it back once -main has gotten to Rec, and if it gains a second implementation before that.

I think if it's destined for mediacapture-main then it's correctly placed in mediacapture-extensions until we change our process.

Now, that is obviously not to say that MST stats is finished; but I'm confused as to what consensus needs to be confirmed with a CfC at this point - now if we were to publish it as a FPWD, I agree this would definitely need a CfC.

While I'm sensitive to pressure from individual vendor deadlines, consensus across multiple vendors on what to implement takes time, and the changes proposed here seem to come from pressure to move even faster, which is why I object to them.

Both "we have shipping features spec'd in that document" and "status of MST stats is that it has not had a CfC, and ... audio ... lack consensus." are both facts that to me suggest we need to slow down, not speed up. Thanks @henbos for answering @dontcallmedom's question about what happened there.

I appreciate any help with getting mediacapture-main to REC, as I bear some responsibility for not having been able to drive that faster.

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