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 Sat, 2 Feb 2019 09:03:17 +0100
Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
Hi Hans,

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

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

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

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