[libcamera-devel] [PATCH v2 11/20] libcamera: ipu3: Adjust full frame picture to 32 pixels

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Jul 10, 2020 at 09:15:16AM +0200, Jacopo Mondi wrote:
> On Thu, Jul 09, 2020 at 03:42:54PM +0200, Niklas Söderlund wrote:
> > On 2020-07-09 10:41:19 +0200, Jacopo Mondi wrote:
> > > To respect the same constraint introduced in validate() that the maximum
> > > ImgU output size shall be at least 32 pixels smaller than the full frame
> > > size, adjust the sizes assigned to the StillCapture role in
> > > generateConfiguration().
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 18f4a02cc270..d07f1a7b5ae8 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -320,10 +320,14 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> > >  			/*
> > >  			 * Use the sensor resolution aligned to the ImgU
> > >  			 * output constraints.
> > > +			 *
> > > +			 * \todo Give 32 pixels from the sensor frame size
> > > +			 * for the IF and BDS rectangles to scale. See
> > > +			 * the todo note for te same operation in validate().
> >
> > s/te/the/
> >
> > Is this a todo? Don't this change introduce the 32 pixels "taking"?
> 
> I would have used a FIXME, but I wanted this collected by Doxygen, as
> we used \todo to mark this kind of items, right ?

Maybe

			 * Give 32 pixels from the sensor frame size for the IF
			 * and BDS rectangles to scale.
			 *
			 * \todo This suffers from the same issue as described
			 * in validate().


Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > >  			 */
> > > -			size.width = std::min(sensorResolution.width,
> > > +			size.width = std::min(sensorResolution.width - 32,
> > >  					      IPU3_OUTPUT_MAX_WIDTH);
> > > -			size.height = std::min(sensorResolution.height,
> > > +			size.height = std::min(sensorResolution.height - 32,
> > >  					       IPU3_OUTPUT_MAX_HEIGHT);
> > >  			size.width &= ~IPU3_OUTPUT_WIDTH_ALIGN;
> > >  			size.height &= ~IPU3_OUTPUT_HEIGHT_ALIGN;

-- 
Regards,

Laurent Pinchart