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

Add captureTime, receiveTime and rtpTimestamp to VideoFrameMetadata #813

Merged
merged 3 commits into from
Dec 10, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions video_frame_metadata_registry.src.html
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,18 @@
<td>segments</td>
<td>[Human face segmentation](https://w3c.github.io/mediacapture-extensions/#human-face-segmentation)</td>
</tr>
<tr>
<td>captureTime</td>
<td>[Capture time](https://wicg.github.io/video-rvfc/#dom-videoframecallbackmetadata-capturetime)</td>
Copy link
Collaborator

@aboba aboba Sep 13, 2024

Choose a reason for hiding this comment

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

The RVFC text is too RTP centric to be used here. I'd copy over the text and make some changes. For a WebCodecs application, captureTime can be serialized on the wire and set by a WebCodecs application for frames received from a remote peer. Also, NTP timestamp format does not imply a global clock so change “estimated using clock synchronization” to “aligned to the sender wallclock”.

Choose a reason for hiding this comment

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

The plan is to

  1. Define what the concept "capture time" is in mediacapture-extensions, and define "remote capture time" over at webrtc-extensions (this would refer to the new mediacapture-extensions "capture time" concept plus text copied from rVFC to describe estimation).
  2. Refer to these new definitions here (local tracks: look here; remote webrtc tracks: look here) instead of the rVFC reference, but use DOMHighResTimestamp relative to local Performance.timeOrigin.

That would leave captureTime unset otherwise. A webcodecs app could use whichever out-of-band techniques to compute & set a valid local DOMHighResTimestamp. Does it need to be mentioned in the spec?

Copy link
Author

Choose a reason for hiding this comment

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

The PR is now updated to reference mediacapture-extensions, where these concepts are now defined.

</tr>
<tr>
<td>receiveTime</td>
<td>[Receive time](https://wicg.github.io/video-rvfc/#dom-videoframecallbackmetadata-receivetime)</td>
Copy link
Collaborator

@aboba aboba Sep 13, 2024

Choose a reason for hiding this comment

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

Comment: RVFC text is also too WebRTC-centric. Can we allow this to be present in a WebCodecs application as well (set by the receiver)?

Choose a reason for hiding this comment

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

The plan is to describe this as "set for remote webrtc tracks" and use DOMHighResTimestamp relative to local Performance.timeOrigin. I agree it should be settable by a WebCodecs application.

</tr>
<tr>
<td>rtpTimestamp</td>
<td>[RTP timestamp](https://wicg.github.io/video-rvfc/#dom-videoframecallbackmetadata-rtptimestamp)</td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

The RVFC text is ok here but I'd still probaby copy it over rather than referencing it.

Choose a reason for hiding this comment

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

I planned to define what a "rtp timestamp" is over at webrtc-extensions and simply define here that it's present for "remote webrtc tracks" together with a reference.

Copy link
Author

Choose a reason for hiding this comment

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

Replaced with the reference to mediacapture-extensions.
Did not copy text to follow the format for Human face segmentation

</tr>
</table>


Expand Down
Loading