Re: [PATCH/RFC] v4l: vsp1: Align crop rectangle to even boundary for YUV formats

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Damian,

On Monday 18 May 2015 17:17:52 Damian Hobson-Garcia wrote:
> On 2015-05-04 7:13 AM, Laurent Pinchart wrote:
> > On Thursday 30 April 2015 14:22:02 Sergei Shtylyov wrote:
> >> On 4/29/2015 8:05 PM, Yoshihiro Kaneko wrote:
> >>> From: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> >>> 
> >>> Make sure that there are valid values in the crop rectangle to ensure
> >>> that the color plane doesn't get shifted when cropping.
> >>> Since there is no distintion between 12bit and 16bit YUV formats in
> >> 
> >> Вistinсtion.
> >> 
> >>> at the subdev level, use the more restrictive 12bit limits for all YUV
> >>> formats.
> > 
> > I would like to mention in the commit message that only the top coordinate
> > constraints differ between the YUV formats, as the subsampling coefficient
> > is always two in the horizontal direction.
> 
> I believe that the height value has the same constraint as the top.

Sure, I meant only the vertical coordinates, sorry.

> > Do you foresee a use case for odd cropping top coordinates ?
> 
> There might be a case when you're blending surfaces together and one
> extends beyond the boundary of the other and you want to clip away the
> non-overlapping portions.

Let's wait until someone requests support for that use case :-)

> >>>   	if (rwpf->entity.type == VSP1_ENTITY_WPF) {
> >>> -		sel->r.left = min_t(unsigned int, sel->r.left, 255);
> >>> -		sel->r.top = min_t(unsigned int, sel->r.top, 255);
> >>> +		int maxcrop =
> > 
> > I would declare maxcrop as an unsigned int.
> > 
> >>> +			format->code == MEDIA_BUS_FMT_AYUV8_1X32 ? 254 : 255;
> >> 
> >> I think you need an empty line here.
> >> 
> >>> +		sel->r.left = min_t(unsigned int, sel->r.left, maxcrop);
> >>> +		sel->r.top = min_t(unsigned int, sel->r.top, maxcrop);
> > 
> > Is this needed ? Based on what I understand from the datasheet the WPF
> > crops the image before passing it to the DMA engine. At that point YUV
> > data isn't subsampled, so it looks like we don't need to restrict the
> > left and top to even values.
> 
> I think that you're correct. There is no subsampling at this stage in
> the pipeline so the maxcrop setting should be fine at 255 regardless of
> the format.

-- 
Regards,

Laurent Pinchart