-
Notifications
You must be signed in to change notification settings - Fork 139
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
Relationship between VideoFrameMetadata and VideoFrameCallbackMetadata #599
Comments
What advantage does VideoFrameCallbackMetadata get from deriving from VideoFrameMetadata, versus having a VideoFrameMetadata member instead? Is this to avoid the duplication of fields if they are moved around? I assume all fields in VideoFrameMetada will be optional. Otherwise, this puts an extra burden on video.requestVideoFrameCallback implementations. |
It's a good question. Thanks for asking @youennf. I think they're a bit unwieldy for direct derivation in either direction. E.g., Presentation time metadata and the fact that I prefer just cross-pollinating the fields we think are important between the two without worrying too much about perfect sync. I.e., I agree the ones you list in 2 seem reasonable to add to |
The main question is whether there will be any field in VideoFrameMetadata we would not want to expose in VideoFrameCallbackMetadata. Hence why deriving VideoFrameCallbackMetadata from VideoFrameMetadata is nice (one issue is name clashing though). I'll file an issue about adding captureTime, receiveTime and rtpTimestamp to VideoFrameMetadata. |
I think that's actually a bit neat in reverse. So I'm in favor of |
I think that we are in agreement, I am pushing for something like |
Related PR: #813 Once this PR is merged, can we close this issue? |
https://wicg.github.io/video-rvfc/#dictdef-videoframecallbackmetadata defines some metadata that might be useful to be in VideoFrameMetadata.
It might also be useful to expose future VideoFrameMetadata fields in VideoFrameCallbackMetadata.
The following could be done:
@tguilbert-google, @dalecurtis, thoughts?
The text was updated successfully, but these errors were encountered: