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

Fix descriptions according to the upgraded draft #689

Closed
wants to merge 1 commit into from
Closed

Fix descriptions according to the upgraded draft #689

wants to merge 1 commit into from

Conversation

balazskreith
Copy link
Contributor

@balazskreith balazskreith commented Sep 24, 2022

  • Remove description reference to non-existing-field totalSamplesDecoded

Preview | Diff

 * Remove description reference to non-existing-field totalSamplesDecoded
@w3cbot
Copy link

w3cbot commented Sep 24, 2022

dontcallmedom marked as non substantive for IPR from ash-nazg.

@vr000m
Copy link
Contributor

vr000m commented Oct 4, 2022

hmmm... we should link to totalSamplesDecoded in the provisional spec. @dontcallmedom is there a way to do this, apart from hyperlinking directly? https://w3c.github.io/webrtc-provisional-stats/#dom-rtcinboundrtpstreamstats-totalsamplesdecoded

@dontcallmedom
Copy link
Member

no other mechanism at this stage, since provisional stats aren't indexed in our definition db at this time.

@@ -1238,7 +1238,7 @@ <h2>
This metric is not incremented for frames that are not decoded,
i.e. {{RTCReceivedRtpStreamStats/framesDropped}}.
The average processing delay can be calculated by dividing the {{totalProcessingDelay}} with the
{{framesDecoded}} for video (or povisional stats spec <code>totalSamplesDecoded</code> for audio).
{{framesDecoded}} for video.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"For video" should be at the start of the paragraph.

Copy link
Collaborator

@henbos henbos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make totalProcessingDelay video-only since it's only implemented for video, then it makes sense to not reference totalSamplesDecoded

@vr000m
Copy link
Contributor

vr000m commented Oct 4, 2022

Perhaps a better solution would be to caveat this metric for video, ergo adding a preamble that it is only for video.

@alvestrand
Copy link
Contributor

The issue #660 is about marking up the stats that are only implemented for one stat.

@henbos
Copy link
Collaborator

henbos commented Oct 18, 2022

Let's follow up in #660

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

Successfully merging this pull request may close these issues.

6 participants