On 02/02/2019 01:48 AM, Vivek Kasireddy wrote: > On Fri, 1 Feb 2019 10:08:52 +0100 > Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > Hi Hans, > >> On 2/1/19 3:29 AM, Vivek Kasireddy wrote: >>> On Thu, 31 Jan 2019 14:36:42 +0100 >>> Hans Verkuil <hverkuil@xxxxxxxxx> wrote: >>> >>> Hi Hans, >>> >>>> Hi Vivek, >>>> >>>> On 1/29/19 3:32 AM, Vivek Kasireddy wrote: >>>>> From: "Kasireddy, Vivek" <vivek.kasireddy@xxxxxxxxx> >>>>> >>>>> 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. > 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). The v4l2-tpg AYUV format is really, really correct given the V4L2 specification of this format. 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.kasireddy@xxxxxxxxx> >>>>> --- >>>>> 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; >>>>> >>>> >>> >> >