On 2/5/19 3:04 AM, Vivek Kasireddy wrote:
> On Sat, 2 Feb 2019 09:03:17 +0100
> Hans Verkuil <hverk...@xs4all.nl> wrote:
> Hi Hans,
>
>> On 02/02/2019 01:48 AM, Vivek Kasireddy wrote:
>>> On Fri, 1 Feb 2019 10:08:52 +0100
>>> Hans Verkuil <hverk...@xs4all.nl> wrote:
>>> Hi Hans,
>>>
>>>> On 2/1/19 3:29 AM, Vivek Kasireddy wrote:
>>>>> On Thu, 31 Jan 2019 14:36:42 +0100
>>>>> Hans Verkuil <hverk...@xs4all.nl> wrote:
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>>> Hi Vivek,
>>>>>>
>>>>>> On 1/29/19 3:32 AM, Vivek Kasireddy wrote:
>>>>>>> From: "Kasireddy, Vivek" <vivek.kasire...@intel.com>
>>>>>>>
>>>>>>> The memory layout of AYUV buffers (V4L2_PIX_FMT_YUV32) should be
>>>>>>> similar to V4L2_PIX_FMT_ABGR32 instead of V4L2_PIX_FMT_ARGB32.
>>>>>>>
>>>>>>> While displaying the packed AYUV buffers generated by the Vivid
>>>>>>> driver using v4l2-tpg on Weston, it was observed that these AYUV
>>>>>>> images were not getting displayed correctly. Changing the memory
>>>>>>> layout makes them display as expected.
>>>>>>
>>>>>> Our YUV32 fourcc is defined as follows:
>>>>>>
>>>>>> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/pixfmt-packed-yuv.html
>>>>>>
>>>>>> As far as I see the format that the TPG generates is according to
>>>>>> the V4L2 spec.
>>>>>
>>>>> I looked into the above link, and I am now wondering whether YUV32
>>>>> is the same as the format referred to as AYUV here or not:
>>>>>
>>>>> https://docs.microsoft.com/en-us/windows/desktop/medfound/recommended-8-bit-yuv-formats-for-video-rendering#ayuv
>>>>>
>>>>
>>>> It's not the same format.
>>>>
>>>>>
>>>>> If YUV32 is not the same as AYUV, should I send another patch
>>>>> adding a new format named AYUV with the reversed memory
>>>>> layout?
>>>>
>>>> That can only be done if there is also a driver that uses it.
>>> There are some drm drivers that already use the AYUV format defined
>>> here:
>>> https://git.linuxtv.org/media_tree.git/tree/drivers/gpu/drm/drm_fourcc.c#n228
>>>
>>
>> I would have to check this with Mauro whether this is a good enough
>> excuse to add a new format.
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>> Philipp, can you check the YUV32 format that the imx-pxp driver
>>>>>> uses? Is that according to our spec?
>>>>
>>>> Philipp, would it be possible to add such a format to imx-pxp? That
>>>> might be a nice approach because once imx-pxp can do it, then it
>>>> can also be added to the TPG.
>>> I was going to send in a patch to add the AYUV (and maybe XYUV)
>>> format but I came across this line that leaves me confused:
>>> https://git.linuxtv.org/media_tree.git/tree/drivers/media/platform/vivid/vivid-vid-common.c#n164
>>>
>>
>> It indicates the order of the components in memory, which is indeed
>> AYUV. But it has nothing to do with the microsoft AYUV fourcc.
> Does it mean that based on the V4L2 spec, the order of components in
> memory is always guaranteed regardless of the endianness of the
> underlying platform? Is it the Vivid/v4l2 core driver's responsibility
> to make sure this happens?
Correct. But I'm sure it is likely to be horribly broken in many drivers
since they are rarely if ever tested on a big endian system.
>
>>
>>> It appears that when YUV32 was added, the intention was to mimic
>>> AYUV. And, unless I am utterly mistaken, the alpha_mask of
>>> 0x000000ff suggests that the alpha component is expected to be
>>> found in the LSB (LE) bits indicating a memory layout of VUYA. Am I
>>> interpreting this incorrectly?
>>
>> Yes. When you write 0x000000ff to memory in a LE environment, it ends
>> up as 0xff 0x00 0x00 0x00 in memory (in increasing memory addresses).
> This would only work if the alpha component A is always in the MSB
> which suggests endianness-independence. And, what would happen when
> writing the alpha mask 0x000000ff in a BE environment which would end up
> as 0x00 0x00 0x00 0xff in memory?
It would be wrong. But this alpha mask is used to test an esoteric corner
case of v4l2 (video overlays), and only for RGB 5:6:5 and 1:5:5:5 formats.
If I ever would test this on a big endian system I would have to make some
fixes, I'm sure.
>
>>
>> The v4l2-tpg AYUV format is really, really correct given the V4L2
>> specification of this format.
> Ok, should I send a patch adding support for VUYA format? Should the
> new fourcc be VUY4?
I would prefer to have it along the line of how RGB32 works, so:
VUYX32 and VUYA32
(slightly different from how e.g. ABGR32 since that should have been
BGRA32, but let's do it right)
While you are at it, also add AYUV32 and XYUV32 for completeness. The
old YUV32 didn't indicate anything about whether the hardware would use
or ignore the A byte, we corrected that for RGB formats but never for
this YUV32 format.
Don't forget to update the documentation and to add support for this
to v4l2-tpg and vivid.
And mention that this allows you to use the tpg/vivid in combination with
drm drivers that use VUYX32.
Regards,
Hans
>
> Thanks,
> Vivek
>
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> Thanks,
>>> Vivek
>>>
>>>>
>>>>>>
>>>>>> At some point we probably want to add a VUY32 format which is
>>>>>> what Weston expects, but we certainly cannot change what the TPG
>>>>>> generates for YUV32 since that is correct.
>>>>> Weston does not know much about the details of pixel formats and
>>>>> instead relies on the Mesa i965 DRI driver to do the heavy
>>>>> lifting. And, this driver implemented AYUV support looking at the
>>>>> above Microsoft link. Also, is the description of V4l pixel
>>>>> formats mentioned in the below link accurate:
>>>>> https://afrantzis.com/pixel-format-guide/v4l2.html
>>>>
>>>> Don't use it, just use the V4L2 Specification, that's always kept
>>>> up to date.
>>>>
>>>> Regards,
>>>>
>>>> Hans
>>>>
>>>>>
>>>>> Thanks,
>>>>> Vivek
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Hans
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Vivek Kasireddy <vivek.kasire...@intel.com>
>>>>>>> ---
>>>>>>> drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 2 +-
>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
>>>>>>> b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c index
>>>>>>> d9a590ae7545..825667f67c92 100644 ---
>>>>>>> a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c +++
>>>>>>> b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c @@ -1269,7
>>>>>>> +1269,6 @@ static void gen_twopix(struct tpg_data *tpg, case
>>>>>>> V4L2_PIX_FMT_HSV32: alpha = 0;
>>>>>>> /* fall through */
>>>>>>> - case V4L2_PIX_FMT_YUV32:
>>>>>>> case V4L2_PIX_FMT_ARGB32:
>>>>>>> buf[0][offset] = alpha;
>>>>>>> buf[0][offset + 1] = r_y_h;
>>>>>>> @@ -1280,6 +1279,7 @@ static void gen_twopix(struct tpg_data
>>>>>>> *tpg, case V4L2_PIX_FMT_XBGR32:
>>>>>>> alpha = 0;
>>>>>>> /* fall through */
>>>>>>> + case V4L2_PIX_FMT_YUV32:
>>>>>>> case V4L2_PIX_FMT_ABGR32:
>>>>>>> buf[0][offset] = b_v;
>>>>>>> buf[0][offset + 1] = g_u_s;
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
