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

Is RTCEncodedVideoFrameMetadata.frame_id actually an unsigned long long or does it wrap at 16 bits? #220

Closed
tonyherre opened this issue Jan 5, 2024 · 7 comments

Comments

@tonyherre
Copy link
Contributor

frame_id is defined as a unsigned long long, but currently doesn't have any textual definition in the members section beneath. Same for dependencies.

My rough understanding is that it corresponds to the Dependency Descriptor Header Extension's frame_number field as a codec-agnostic frame numbering method, but that field is only 16 bits, so is the UA assumed to be handling the 16 bit wrapping and converting up to a 64 bit unsigned long long? That's the current Chromium implementation at least. I'm unsure of other browsers.

@aboba
Copy link
Contributor

aboba commented Jan 5, 2024

@tonyherre I had the same question. Is it likely you'd have a dependency that would require the full 64 bits of an unsigned long long??

Now that Eugene is looking at implementing frame_id and dependencies in WebCodecs, it would be nice to get this figured out.

@fippo
Copy link
Collaborator

fippo commented Jan 5, 2024

We had a somewhat similar discussion for the inbound rtpTimestamp in #154

If the field is present both outbound and inbound we should aim for consistency even if that means expanding to the long form.

One problem of the short form is that it may be codec dependent, e.g. VP8 has a 15 bit field (and a 7 bit one but lets pretend it does not exist...) so how would we expose the limit?

@tonyherre tonyherre changed the title Is RTCEncodedVideoFrameMetadata.frame_id actually an unsigned long long of does it wrap at 16 bits? Is RTCEncodedVideoFrameMetadata.frame_id actually an unsigned long long or does it wrap at 16 bits? Jan 8, 2024
@tonyherre
Copy link
Contributor Author

tonyherre commented Jan 8, 2024

If it might only be using 15 or 16 bits, then a web app doing anything with managing frame dependencies (broadcast with fanout, dropping layers etc) would have to handle wraps after 18/36 minutes of 30fps which is a pain. 32 bits would be enough I'd think, if someone's worried about size.

I'd suggest we leave the type as-is, and just add definitions along the lines of:

frameId: A monotonically increasing frame counter. Its lower 16 bits will match the frame_number of the Dependency Descriptor Header Extension, if present.

dependencies: List of the frameIds of RTCEncodedVideoFrames required to decode this frame.

WDYT?

@alvestrand
Copy link
Contributor

WFM, but need to have language that it matches the frame ID in the encoded frame in its lower bits when that exists.
(Is the DD frame ID defined to be 16 bits? How does it match up with VP8's 7-bit frame ID variant?)

@fippo
Copy link
Collaborator

fippo commented Jan 8, 2024

On the receive side we might make this depend on the presence of DD to avoid codec-specific wrapping (the 7bit VP8 variant is broken anyway but the normal one would be 15 bits not 16)?

@aboba
Copy link
Contributor

aboba commented Jan 10, 2024

Do we understand how frameId is supposed to behave across codecs? For WebCodecs PR 756, we are going to need to specify how it is initialized, incremented, etc.

@aboba
Copy link
Contributor

aboba commented Jan 16, 2024

@tonyherre dependencies is the list of frames that the current frame references. It is not the complete list of frameIds necessary to enable the frame to be decoded. Since a non-keyframe dependency will itself have dependencies, even if a receiver has received all the dependencies it still may not be able to decode the frame if the dependency chain has been broken. This is why DD includes chains as well as dependencies.

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

No branches or pull requests

4 participants