w3c / webrtc-provisional-stats

https://w3c.github.io/webrtc-provisional-stats/
Other
7 stars 11 forks source link

TimingInfo metrics #40

Open henbos opened 1 year ago

henbos commented 1 year ago

In Chrome, the legacy getStats() contains this non-standard metric: googTimingFrameInfo.

Apparently it is very useful. It is exposed as a string that has to be parsed rather than the following attributes:

I supposed it is one serialized struct because all of these metrics relate to a single video frame. The modern way to do this would be to create a new RTCStatsType and have one stats object containing all of these attributes, then reference that frame object from the "inbound-rtp".

I can guess what some of these are but all of them needs a precise definition. @ilyanikolaevskiy Can you enlighten us?

henbos commented 1 year ago

I want things deemed "too useful to be dropped" to migrate into standardized getStats() so that we can delete legacy getStats(). My sense is that we're close to being able to unship it, but then stuff like this crops up.

ilyanikolaevskiy commented 1 year ago

Yes, it's very useful. It makes sense to create a new type for all of it, I agree. All these numbers correspond to the same frame.

The GetStats reports the frame with the longest e2e time for the last second, so that if polled at 1s interval, the worst frames would be observed by the application. The reports are attached to the frames because either timer or because the frame is bigger than average. The metrics are timestamps mostly:

Because some timestamps are send-side and some are receive-side, a clock synchronisation is performed by the browser. But before this happens, First 7 timestamps (capture ... network2) would be reported as negative. In that case, the app can't compare send-side and receive-side timestamps. But it would still be fine to subtract timestamps from the same side. If the capture time is non-negative, when all clocks are synchronised and the app can subtract whatever it want to calculate e.g. end-to-end delay.

On Fri, Oct 7, 2022 at 1:23 PM henbos @.***> wrote:

I want things deemed "too useful to be dropped" to migrate into standardized getStats() so that we can delete legacy getStats(). My sense is that we're close to being able to unship it, but then stuff like this crops up.

— Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-provisional-stats/issues/40, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF74EIY4JXL5NZWUZEXKIBDWCAB4LANCNFSM6AAAAAAQ7P4LKU . You are receiving this because you were mentioned.Message ID: @.***>

-- Best regards, Ilya

henbos commented 1 year ago

What is the relationship between these metrics and https://webrtc.github.io/webrtc-org/experiments/rtp-hdrext/video-timing/# ?

fippo commented 1 year ago

if we want to get this into the stats model we need to do it differently since this is a per-frame thing which fits something like requestVideoFrameCallback much better?

I tried some of this in https://chromium-review.googlesource.com/c/chromium/src/+/2675937 but then didn't find time to finish.

ilyanikolaevskiy commented 1 year ago

These metrics are the way for the app to actually get the data, passed in video-timing RTP header extension.

On Fri, Oct 7, 2022 at 3:01 PM henbos @.***> wrote:

What is the relationship between these metrics and https://webrtc.github.io/webrtc-org/experiments/rtp-hdrext/video-timing/# ?

— Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-provisional-stats/issues/40, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF74EI3TBGFDUMWYUBPQ2L3WCANLLANCNFSM6AAAAAAQ7P4LKU . You are receiving this because you were mentioned.Message ID: @.***>

-- Best regards, Ilya

ilyanikolaevskiy commented 1 year ago

Philip, it's per frame, but it's not available on all the frames. So if we can add it as optional, requestVideoFrameCallback looks like a better option.

The current way of reporting this metric is that because at the time it was implemented, there were no per-frame callbacks.

On Fri, Oct 7, 2022 at 3:07 PM Philipp Hancke @.***> wrote:

if we want to get this into the stats mdoel we need to do it differently since this is a per-frame thing which fits something like requestVideoFrameCallback much better?

I tried some of this in https://chromium-review.googlesource.com/c/chromium/src/+/2675937 but then didn't find time to finish.

— Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-provisional-stats/issues/40, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF74EI6QPNRJASCULEFXCQTWCAOAFANCNFSM6AAAAAAQ7P4LKU . You are receiving this because you were mentioned.Message ID: @.***>

-- Best regards, Ilya

henbos commented 1 year ago

Are we "just exposing what is in the header extension", or are we combining it with our own local measurements? Exposing the contents of a header extension makes sense to me and limiting it to a header extension specific dictionary.

ilyanikolaevskiy commented 1 year ago

We are combining header extension data with local measurements and also adjusting the data from the header extension to the local clock. Roughly the first half of the reported timestamps comes from the extension, the latter half - local measurements.

On Fri, Oct 7, 2022 at 3:16 PM henbos @.***> wrote:

Are we "just exposing what is in the header extension", or are we combining it with our own local measurements? Exposing the contents of a header extension makes sense to me and limiting it to a header extension specific dictionary.

— Reply to this email directly, view it on GitHub https://github.com/w3c/webrtc-provisional-stats/issues/40, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF74EIY5PLEPYU6OGWHKQK3WCAPBRANCNFSM6AAAAAAQ7P4LKU . You are receiving this because you were mentioned.Message ID: @.***>

-- Best regards, Ilya

henbos commented 1 year ago

It looks like we're inheriting some amount of complexity from this header extension of which I am uncertain how widely implemented/standardized it is.

I'll transfer this issue to webrtc-provisional-stats. I suspect we'll need something in there to reflect what is currently exposed in legacy getStats() but we may need to revisit what a webrtc-stats solution should look like long term.

With my Chrome hat on, I think documenting and exposing what is in the modern API will help with the deprecation and unshipping of legacy API. Even if provisional will need to reflect something somewhat work-in-progress.

vr000m commented 1 year ago

I wonder if getContributingSources be a better place for this than getstats, assuming the getCS has relevant timing information