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 > > > > >> > >> 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 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? 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; > >>> > >> > > >
