[PATCH weston v9 01/62] libweston: Add pixel-format helpers

[Emil Velikov]

On 16 March 2017 at 10:54, Daniel Stone <daniel at fooishbar.org> wrote:
> Hey Emil,
>
> On 13 March 2017 at 17:57, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 10 March 2017 at 15:28, Daniel Stone <daniel at fooishbar.org> wrote:
>>> On 9 March 2017 at 22:57, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> According to WL_bind_wayland_display and the i965/i915 drivers
>>>> (afaict) above five should have two planes. Then again, the FOURCC and
>>>> DRM formats are pretty clear that we're talking about a packed format
>>>> in a single plain... Did you test these, do we have a spec bug, is the
>>>> i915/i965 iimplementation working/been tested ?
>>>
>>> No, they're all right, in their own way. ;) In terms of storage, the
>>> buffers are stored purely in one plane, containing (bytewise)
>>> YUYV/etc, as encoded in the FourCC. However, to use it from GL, you
>>> need to bind the one plane to two samplers: one for luma and one for
>>> chroma. Given that the luma and chroma have different subsampling
>>> values, you can't bind them to the same sampler. So I think this is
>>> correct here.
>>
>> Right - I was confused there by the name "num_planes". Worth changing
>> to num_of_samplers ?
>
> Maybe, but then it would also be wrong: YUYV's current num_planes == 1
> would have to change to num_of_samplers == 2. But num_of_samplers is
> already uniquely identified by the sampler type: Y_UV_WL and Y_XUXV_WL
> are two samplers, and Y_U_V_WL is three samplers. So in this case we
> would just remove it rather than redundantly encode it.
>
> We have some simple checks for these in, e.g., dmabuf, to reject
> incoming buffer-creation requests where num_planes does not match the
> format. So scrapping num_planes in pixel-formats means that we'd also
> have to remove the checks.
>
Yeah, let's not do that now - unless you/others insist.

I've been mostly wondering on how to make it clearer. At the end of
the day pick whichever you think is more suitable.

>>>> Seems like DRM_FORMAT_AYUV is going to act strange with the
>>>> pixel_format_is_opaque() helper. As-in latter will return true,
>>>> despite the format has alpha.
>>>
>>> Indeed. I'd be tempted to just delete AYUV tbh.
>>>
>> Emmanuel mentioned that some userspace (mpv iirc) uses AYUV. Not sure
>> if/who much that's applicable here.
>> Worst case - one can add it as a follow-up.
>
> Yeah, given that it's not actually supported by Weston at the moment,
> removing it wouldn't be a loss. ;)
>
>>>>  - YUV formats - 1/2/3 num_planes ->
>>>> EGL_TEXTURE_Y_XUXV_WL/EGL_TEXTURE_Y_XUXV_WL/EGL_TEXTURE_Y_XUXV_WL
>>>> respectively
>>>
>>> Well, you still need the chroma order to figure out if it's XUXV or
>>> XVXU; and did you mean to paste the same sampler type three times? ;)
>> Nope, they should read Y_U_V_WL, Y_UV_WL and Y_XUXV_WL.
>>
>>> I get the point though. The idea was for the combination of
>>> ORDER_{LUMA_CHROMA,CHROMA_LUMA} and ORDER_{UV,VU}, as well as
>>> num_planes, to uniquely identify the sampler type.
>>>
>> No objection against the order - thinking that the num_sampler(s) can
>> imply the EGL_TEXTURE_FORMAT.
>
> Hm maybe, but at the cost of some ambiguity (i.e. num_samplers == 2
> could be Y_UV or Y_XUXV). On the other hand, num_samplers is uniquely
> deriveable from the texture format, so I'd prefer to stick to the
> other way around.
>
Right of course - silly me was still thinking of Y_UV is
num_samplers/num_planes == 1.

Thank you very much for the extensive reply - it cleared a dull moment
or two on my end.
Emil