w3c / mediacapture-extensions

Extensions to Media Capture and Streams by the WebRTC Working Group
https://w3c.github.io/mediacapture-extensions/
Other
19 stars 15 forks source link

Move MediaStreamTrack stats in its own spec? #132

Open dontcallmedom opened 10 months ago

dontcallmedom commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 https://github.com/w3c/mediacapture-extensions/pull/117 merged prematurely, and that audio track stats lack consensus.

dontcallmedom commented 10 months ago

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 https://github.com/w3c/mediacapture-extensions/pull/117#issue-1918823390); 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 commented 10 months ago

I'm confused - MST stats had review and consensus in the meetings they were reviewed (according to https://github.com/w3c/mediacapture-extensions/pull/117#issue-1918823390); and https://github.com/w3c/mediacapture-extensions/pull/117 was merged after it was https://github.com/w3c/mediacapture-extensions/pull/117#issuecomment-1781210861 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 https://github.com/w3c/mediacapture-extensions/issues/129. So while we did follow up and merge another PR for latency metrics based on Paul and Youenn approving it here (https://github.com/w3c/mediacapture-extensions/pull/124), the dropped frames disagreement remains unresolved.

dontcallmedom commented 10 months ago

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 commented 10 months ago

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

jan-ivar commented 10 months ago

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.