Skip to content

Add more complete descriptions for PixelFormats #165

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

Closed
chcunningham opened this issue Apr 5, 2021 · 7 comments
Closed

Add more complete descriptions for PixelFormats #165

chcunningham opened this issue Apr 5, 2021 · 7 comments
Assignees
Labels
need-definition An issues where something needs to be specified normatively

Comments

@chcunningham
Copy link
Collaborator

For instance, for I420 we could mention

  • the number of planes is 3
  • they are Y, U, and V
  • their horizontal pixel sampling rate is 1, 2, and 2, respectively.
  • their vertical pixel sampling rate is ""

This is good for interop and documentation. It also allows us to give more complete steps to validate PlaneInit dictionaries.

chcunningham added a commit that referenced this issue Apr 5, 2021
Making it symmetric w/ AudioFrame.
Adds clone/close
Adds [[resource reference]]
Updates constructors accordingly (and fix cruft/obsolete steps).
Removes destroy (replaced by close).

Fixes #129 (in comboniation w/ PR #162).
Notes issues #165 and #166 for follow up.
@chcunningham
Copy link
Collaborator Author

Triage note: marking 'editorial' as we have broad consensus on the meaning of well known pixel formats (certainly true for the single I420 currently in the spec), and this issue merely tracks the work to document it's meaning.

@chcunningham chcunningham added the editorial changes to wording, grammar, etc that don't modify the intended behavior label May 12, 2021
@padenot
Copy link
Collaborator

padenot commented May 17, 2021

Yes, but this needs to be specified normatively and unambiguously before any implement ships. I think we all agree what it is though:

(w x h) pixels of luma, and then immediately (w x h / 4) pixels for the U chroma plane and then immediately(w x h / 4) pixels for V Chroma plane. w and h are the coded width and height, not necessarily the display width and height.

In this format, a component for a pixel is 8-bits in all 3 planes.

Because of the API, I don't think it's necessary to specify whether the Y, U and V planes linear and contiguous in memory when the buffer is in regular memory: it's always access via readInto (modulo names changes).

If an implementation wants to ship any other format it needs to be specified unambiguously in the same fashion.

@padenot padenot added need-definition An issues where something needs to be specified normatively and removed editorial changes to wording, grammar, etc that don't modify the intended behavior labels May 17, 2021
@padenot
Copy link
Collaborator

padenot commented May 17, 2021

Also Y U and V are in this precise order.

@chcunningham
Copy link
Collaborator Author

I believe we agree on the definition given above. @sandersdan to double check me.

@padenot padenot self-assigned this May 18, 2021
@sandersdan
Copy link
Contributor

sandersdan commented May 18, 2021

Also Y U and V are in this precise order.

Just to clarify, that is how the APIs handle the planes but does not describe their layout in memory (which as you mentioned is arbitrary).

I'll add only that there are also general restrictions I would like to apply to all formats that also apply to I420:

  • coded sizes must be a multiple of the largest subsampling factor: I420 coded sizes must be even.
  • crop coordinates must be a multiple of the largest subsampling factor: I420 crops must have even origin and size values.

We could go further and specify byte ordering (eg. top-left at lowest memory address, increasing along rows up the the stride size, then wrapping to the left of the next row). Again this is probably the same for all formats, but it can cause confusion due to GL using y-flipped coordinates relative to this definition.

(Edit: just realized that stride wasn't mentioned in any post before this one. Stride behavior should be the same for all formats, but it does affect the calculations mentioned above; w * h is only meaningful when stride == planeCodedWidth, in general it's h * stride bytes per I420 plane.)

@padenot
Copy link
Collaborator

padenot commented May 19, 2021

Thanks for the comment. I agree with everything and will write a PR with all of this.

@chcunningham
Copy link
Collaborator Author

@padenot good to close now that #288 is merged?

@padenot padenot closed this as completed Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need-definition An issues where something needs to be specified normatively
Projects
None yet
Development

No branches or pull requests

3 participants