Add definitions of corruption detection measurements

[sprangerik]

Fixes #787

This PR adds new statistics for a corruption probability measure of a received video stream.

We include both the sum and sum of squares for this measure, so that a variance can be calculated.

---

Preview | Diff

[alvestrand]

Awaiting the TPAC presentation.

[jan-ivar]

Thanks! Sounds like a good idea to me. Marking as draft pending upcoming discussion.

Marking CR blocking due to the google source reference. Any volunteers to pushing the RTP header extension through IETF?

[handellm]

Depending on the math behind totalCorruptionProbability and totalSquaredCorruptionProbability and the fact that these are doubles and if these are ever-increasing quantities, could there be numerical issues mounting by incrementing over longer times?

[sprangerik]

Thanks! Sounds like a good idea to me. Marking as draft pending upcoming discussion.

Marking CR blocking due to the google source reference. Any volunteers to pushing the RTP header extension through IETF?

I have update the text to make it more clear that method is not normative. ptal!

Also, I volunteer to push that header extension through IETF :)

[sprangerik]

Depending on the math behind totalCorruptionProbability and totalSquaredCorruptionProbability and the fact that these are doubles and if these are ever-increasing quantities, could there be numerical issues mounting by incrementing over longer times?

Doubtful. Even in the case where a call is run for a very very long time, remember that the individual samples are capped in the [0.0, 1.0] range and are likely only (individually) useful to a few decimal places. With a base of 52 bits, that's still an enormous amounts of samples that can be accumulated without noticeable loss. Even further reducing the scope of this problem is that we expect the "normal" values to be essentially 0.0. If you run extremely long sessions with constant corruption, might you have other problems to prioritize first.

[sprangerik]

@jan-ivar would you mind reviewing this?

[sprangerik]

@youennf would you mind reviewing this?:

[henbos]

Ping @youennf, can we merge this as is or is there more work needed?

[sprangerik]

ping @youennf, any further comments?

[sprangerik]

LGTM. I would file an issue once this lands to track the work to replace the webrtc.org link with an IETF link.

Thanks! Yes, that makes sense.

[sprangerik]

@jan-ivar anything further from your side?

[sprangerik]

LGTM. As a newb on this, it might be good to give the reader an example formula they can use when reading these stats. Like we do in https://w3c.github.io/webrtc-stats/#dom-rtcinboundrtpstreamstats-totalinterframedelay

But not a blocker.

Good point, but let med do that as a follow-up.

[alvestrand]

@jan-ivar why did you add the "CR Blocker" tag on this one?

[jan-ivar]

@alvestrand see https://github.com/w3c/webrtc-stats/pull/788#issuecomment-2361162211. To my understanding there's still a normative reference to a webrtc.org doc here. @sprangerik volunteered in https://github.com/w3c/webrtc-stats/pull/788#issuecomment-2394326319 to push it through IETF.

So merging this would block this spec from recycling at CR. We may be OK with that, but we should track it somehow.

Erik, can you open a follow-up issue to come back and fix this link whenever your IETF efforts have concluded? Then we can label that issue and merge this, if that's how we'd like to proceed.

[jan-ivar]

Reading through the thread again I see there was an effort to make it non-normative, but this part still sounds normative to me:

• "If the corruption-detection header extension is present in the RTP packets, corruption probability measurements MUST be present."

[sprangerik]

@alvestrand see #788 (comment). To my understanding there's still a normative reference to a webrtc.org doc here. @sprangerik volunteered in #788 (comment) to push it through IETF.

So merging this would block this spec from recycling at CR. We may be OK with that, but we should track it somehow.

Erik, can you open a follow-up issue to come back and fix this link whenever your IETF efforts have concluded? Then we can label that issue and merge this, if that's how we'd like to proceed.

Yes, that was my intention! https://github.com/w3c/webrtc-stats/pull/788#issuecomment-2444255504 might not have been clear on that point.

[sprangerik]

Reading through the thread again I see there was an effort to make it non-normative, but this part still sounds normative to me:

• "If the corruption-detection header extension is present in the RTP packets, corruption probability measurements MUST be present."

In the sense that if you have decided to implement support for that extensions and have negotiated it for a particular RTP stream, then it makes sense to me to say that we should surface the stats value for it. Do you have a different formulation in mind you think is more suitable?

[youennf]

I think being normative and using MUST is good here. I think we just need to keep track of making sure the link is changed before we go to CR.

[jan-ivar]

Agree normative is good.

[sprangerik]

I filed https://github.com/w3c/webrtc-stats/issues/790 to track updating the definitions. I don't seem to have access to change labels though. @jan-ivar can you assist with that?

[henbos]

I added the CR blocker label on the issue filed and removed it from this PR, sounds good?