[libcamera-devel] [PATCH 2/2] libcamera: pipeline: ipu3: Use new Size grownBy() and shrunkBy() helpers

[Jacopo Mondi]

Hi Laurent,

On Wed, Oct 13, 2021 at 04:26:30AM +0300, Laurent Pinchart wrote:
> The Size class has new helpers that can simplify the code in the IPU3
> pipeline handler. Use them.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 92e869257e53..262b9a23703e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -438,11 +438,10 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
>  			 * \todo Clarify the alignment constraints as explained
>  			 * in validate()
>  			 */
> -			size = sensorResolution.boundedTo(IMGU_OUTPUT_MAX_SIZE);
> -			size.width = utils::alignDown(size.width - 1,
> -						      IMGU_OUTPUT_WIDTH_MARGIN);
> -			size.height = utils::alignDown(size.height - 1,
> -						       IMGU_OUTPUT_HEIGHT_MARGIN);
> +			size = sensorResolution.boundedTo(IMGU_OUTPUT_MAX_SIZE)
> +				.shrunkBy({ 1, 1 })
> +				.alignedDownTo(IMGU_OUTPUT_WIDTH_MARGIN,
> +					       IMGU_OUTPUT_HEIGHT_MARGIN);

Can you align the dots ?

			size = sensorResolution.boundedTo(IMGU_OUTPUT_MAX_SIZE)
				               .shrunkBy({ 1, 1 })
				               .alignedDownTo(IMGU_OUTPUT_WIDTH_MARGIN,
					                      IMGU_OUTPUT_HEIGHT_MARGIN);

Same below.

Or was it intentional ?

(I'm honest, I found the previous version more immediate compared than
going through shrunk).

Anyway, there's a repeating pattern of

                {size.width - 1, size.height - 1}.alignDownTo(margins)
                {size.width + 1, size.height + 1}.alignUpTo(margins)

Which makes me wonder if what we're looking for is actually a 'Align
to the next strictly minor/major value' function.

Thanks
   j


>  			pixelFormat = formats::NV12;
>  			bufferCount = IPU3_BUFFER_COUNT;
>  			streamFormats[pixelFormat] = { { IMGU_OUTPUT_MIN_SIZE, size } };
> @@ -996,8 +995,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
>  	 */
>
>  	/* The strictly smaller size than the sensor resolution, aligned to margins. */
> -	Size minSize = Size(sensor->resolution().width - 1,
> -			    sensor->resolution().height - 1)
> +	Size minSize = sensor->resolution().shrunkBy({ 1, 1 })
>  		       .alignedDownTo(IMGU_OUTPUT_WIDTH_MARGIN,
>  				      IMGU_OUTPUT_HEIGHT_MARGIN);
>
> @@ -1005,8 +1003,7 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
>  	 * Either the smallest margin-aligned size larger than the viewfinder
>  	 * size or the adjusted sensor resolution.
>  	 */
> -	minSize = Size(IPU3ViewfinderSize.width + 1,
> -		       IPU3ViewfinderSize.height + 1)
> +	minSize = IPU3ViewfinderSize.grownBy({ 1, 1 })
>  		  .alignedUpTo(IMGU_OUTPUT_WIDTH_MARGIN,
>  			       IMGU_OUTPUT_HEIGHT_MARGIN)
>  		  .boundedTo(minSize);
> --
> Regards,
>
> Laurent Pinchart
>