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

Update WebRTC media source stats #32538

Merged
merged 15 commits into from
Mar 22, 2024
Merged

Update WebRTC media source stats #32538

merged 15 commits into from
Mar 22, 2024

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Mar 4, 2024

This updates docs for RTCAudioSourceStats, and adds docs for RTCVideoSourceStats - both of which are type=media-source objects you can get by iterating a RTCStatsReport (differentiating via their kind property).

This is part of the ongoing work to finished RTC stats, which was started in #27028

The changes to Audio source are mostly about API docs consistency.

This has an associated BCD update in mdn/browser-compat-data#22510

@github-actions github-actions bot added Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed labels Mar 4, 2024
Copy link
Contributor

github-actions bot commented Mar 4, 2024

Preview URLs (17 pages)
Flaws (7)

Note! 10 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/RTCAudioSourceStats/audioLevel
Title: RTCAudioSourceStats: audioLevel property
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/RTCInboundRtpStreamStats/audioLevel does not exist

URL: /en-US/docs/Web/API/RTCAudioSourceStats/totalAudioEnergy
Title: RTCAudioSourceStats: totalAudioEnergy property
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/RTCInboundRtpStreamStats/totalAudioEnergy does not exist

URL: /en-US/docs/Web/API/RTCAudioSourceStats/totalSamplesDuration
Title: RTCAudioSourceStats: totalSamplesDuration property
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/RTCInboundRtpStreamStats/totalSamplesDuration does not exist

URL: /en-US/docs/Web/API/RTCVideoSourceStats/frames
Title: RTCVideoSourceStats: frames property
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: api.RTCStatsReport.type_media-source.frames

URL: /en-US/docs/Web/API/RTCVideoSourceStats/width
Title: RTCVideoSourceStats: width property
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: api.RTCStatsReport.type_media-source.width

URL: /en-US/docs/Web/API/RTCVideoSourceStats/framesPerSecond
Title: RTCVideoSourceStats: framesPerSecond property
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: api.RTCStatsReport.type_media-source.framesPerSecond

URL: /en-US/docs/Web/API/RTCVideoSourceStats/height
Title: RTCVideoSourceStats: height property
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: api.RTCStatsReport.type_media-source.height

(comment last updated: 2024-03-19 00:00:34)


These statistics can be obtained by iterating the {{domxref("RTCStatsReport")}} returned by {{domxref("RTCRtpSender.getStats()")}} or {{domxref("RTCPeerConnection.getStats()")}} until you find a report with the [`type`](#type) of `media-source` and a [`kind`](#kind) of `audio`.

> **Note:** For audio information about remotely sourced tracks (that are being received), see {{domxref("RTCInboundRtpStreamStats")}}.

## Instance properties

- {{domxref("RTCAudioSourceStats.audioLevel", "audioLevel")}} {{Experimental_Inline}}
- {{domxref("RTCAudioSourceStats.audioLevel", "audioLevel")}} {{Experimental_Inline}}{{optional_inline}}
Copy link
Collaborator Author

@hamishwillee hamishwillee Mar 5, 2024

Choose a reason for hiding this comment

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

The ones that are not marked required in the IDL have been marked as optional.

@hamishwillee hamishwillee marked this pull request as ready for review March 5, 2024 01:20
@hamishwillee hamishwillee requested a review from a team as a code owner March 5, 2024 01:20
@hamishwillee hamishwillee requested review from wbamberg and removed request for a team March 5, 2024 01:20
files/en-us/web/api/rtcvideosourcestats/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/rtcvideosourcestats/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/rtcvideosourcestats/frames/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/rtcvideosourcestats/height/index.md Outdated Show resolved Hide resolved
@@ -7,27 +7,29 @@ browser-compat: api.RTCStatsReport.type_media-source

{{APIRef("WebRTC")}}

The [WebRTC API](/en-US/docs/Web/API/WebRTC_API)'s **`RTCAudioSourceStats`** dictionary provides information about an audio track that is attached to one or more senders.
The **`RTCAudioSourceStats`** dictionary of the [WebRTC API](/en-US/docs/Web/API/WebRTC_API) provides statistics information about an audio track ({{domxref("MediaStreamTrack")}}) that is attached to one or more senders ({{domxref("RTCRtpSender")}}).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't comment on the line, but the spec URL is wrong for this, it is https://w3c.github.io/webrtc-stats/#dom-rtcstatstype-media-source but should be something like https://w3c.github.io/webrtc-stats/#dom-rtcaudiosourcestats.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was because the media-source "type" is shared by both audio and video objects, and this is how the items are sorted in BCD. I added a spec-urls frontmatter for video and audio to fix this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you also need that for the individual members? And this looks like a systemic issue, e.g. https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnectionStats#specifications

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@wbamberg Whlle it is "systemic" I am fairly sure we don't need to panic about this.

Taking RTCPeerConnectionStats as an example, this links to https://w3c.github.io/webrtc-stats/#dom-rtcstatstype-peer-connection. This is the top level enum description for the type, which is used for searching. BUT, since all types except media-source map 1:1 with a stats object, the text usefully explains a particular stats object and links direct to the object itself.
I don't think that needs to be fixed. But if you do, we could fix in the same way (I'd prefer not to mess with BCD because I think it would be harder).

The individual members should be fine.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I agree that setting spec-url is easier. I think it is wrong to link to the enum type but don't care much, since we don't AFAIK have particularly precise policies for what to link to in spec URLs.

files/en-us/web/api/rtcaudiosourcestats/index.md Outdated Show resolved Hide resolved
@hamishwillee hamishwillee requested a review from a team as a code owner March 18, 2024 03:00
@hamishwillee hamishwillee requested review from bsmth and removed request for a team March 18, 2024 03:00
@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Mar 18, 2024
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot removed the merge conflicts 🚧 [PR only] label Mar 18, 2024
@hamishwillee
Copy link
Collaborator Author

@wbamberg Thank you for the review. I think I've fixed this except for the question on optional chaining - please see my response here that I don't think it matters #32538

(If I'm wrong, how would you address this?)

@wbamberg
Copy link
Collaborator

@wbamberg Thank you for the review. I think I've fixed this except for the question on optional chaining - please see my response here that I don't think it matters #32538

(If I'm wrong, how would you address this?)

If I'm right :) I'd address it with something like:

// Note, test is conditional in case the stats object
// does not include video source stats

@hamishwillee
Copy link
Collaborator Author

Thanks very much. I must warn you, this is an "easy one" compared to the remote inbound stats :-0. There you have to get your head around who is reporting what.

files/en-us/web/api/rtcvideosourcestats/index.md Outdated Show resolved Hide resolved
@@ -7,27 +7,29 @@ browser-compat: api.RTCStatsReport.type_media-source

{{APIRef("WebRTC")}}

The [WebRTC API](/en-US/docs/Web/API/WebRTC_API)'s **`RTCAudioSourceStats`** dictionary provides information about an audio track that is attached to one or more senders.
The **`RTCAudioSourceStats`** dictionary of the [WebRTC API](/en-US/docs/Web/API/WebRTC_API) provides statistics information about an audio track ({{domxref("MediaStreamTrack")}}) that is attached to one or more senders ({{domxref("RTCRtpSender")}}).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you also need that for the individual members? And this looks like a systemic issue, e.g. https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnectionStats#specifications

@bsmth bsmth removed their request for review March 19, 2024 10:14
@bsmth
Copy link
Member

bsmth commented Mar 19, 2024

Removing myself from review as Will's got it covered! 😉

@hamishwillee
Copy link
Collaborator Author

Ready for you to review when ready @wbamberg - the systemic question, is addressed in #32538 (comment)

Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

👍 thank you Hamish!

@wbamberg wbamberg merged commit d32ba6a into mdn:main Mar 22, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants