Re: [PATCH] media: v4l2-tpg: Fix the memory layout of AYUV buffers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux