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 captureTimestamp and senderCaptureTimeOffset to frame metadata #228

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Orphis
Copy link

@Orphis Orphis commented May 2, 2024

Fixes #225


Preview | Diff

@Orphis
Copy link
Author

Orphis commented May 2, 2024

@aboba Can we add this to the next interim's agenda please?

@aboba
Copy link
Contributor

aboba commented May 3, 2024

@Orphis Yes.

@dontcallmedom-bot
Copy link

This issue was mentioned in WEBRTCWG-2024-05-21 (Page 22)

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@Orphis Orphis force-pushed the capture_timestamp branch from 71a2372 to 740b9cf Compare June 19, 2024 15:11
index.bs Outdated Show resolved Hide resolved
@Orphis Orphis force-pushed the capture_timestamp branch from 740b9cf to 1613039 Compare June 20, 2024 11:26
@Orphis Orphis force-pushed the capture_timestamp branch from 1613039 to 1a211a1 Compare June 20, 2024 14:42
Copy link
Member

@jan-ivar jan-ivar left a comment

Choose a reason for hiding this comment

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

PR overall LGTM modulo a normative reference issue.

@@ -48,6 +48,11 @@ spec:webidl; type:dfn; text:resolve
"CloneArrayBuffer": {
"href": "https://tc39.es/ecma262/#sec-clonearraybuffer",
"title": "CloneArrayBuffer"
},
"RTP-EXT-CAPTURE-TIME": {
"href": "http://www.webrtc.org/experiments/rtp-hdrext/abs-capture-time",
Copy link
Member

@jan-ivar jan-ivar Jun 20, 2024

Choose a reason for hiding this comment

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

This normative reference redirects to a googlesource link. I'll need to check with folks internally how to handle this.

Can we add a note mentioning any efforts to standardize this?

My understanding is after an IETF effort it would end up here?

Copy link
Member

@jan-ivar jan-ivar Jun 25, 2024

Choose a reason for hiding this comment

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

See w3c/webrtc-extensions#216. I worry if we merge this it creates a blocker for advancement.

Copy link
Contributor

Choose a reason for hiding this comment

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

We now have Harold’s draft (which will be up for adoption in IETF AVTCORE WG): https://datatracker.ietf.org/doc/draft-alvestrand-avtcore-abs-capture-time/

@@ -358,6 +381,8 @@ dictionary RTCEncodedVideoFrameMetadata {
sequence<unsigned long> contributingSources;
long long timestamp; // microseconds
unsigned long rtpTimestamp;
DOMHighResTimeStamp captureTimestamp;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename to captureTime for consistency with rVFC and others.

@@ -48,6 +48,11 @@ spec:webidl; type:dfn; text:resolve
"CloneArrayBuffer": {
"href": "https://tc39.es/ecma262/#sec-clonearraybuffer",
"title": "CloneArrayBuffer"
},
"RTP-EXT-CAPTURE-TIME": {
"href": "http://www.webrtc.org/experiments/rtp-hdrext/abs-capture-time",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"href": "http://www.webrtc.org/experiments/rtp-hdrext/abs-capture-time",
"href": "https://www.webrtc.org/experiments/rtp-hdrext/abs-capture-time",

@jan-ivar
Copy link
Member

jan-ivar commented Dec 5, 2024

Editors can integrate pending the adoption of https://datatracker.ietf.org/doc/draft-alvestrand-avtcore-abs-capture-time/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants