[libcamera-devel] [PATCH v4 14/21] libcamera: ipu3: Fill stride and frameSize at config validation

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Wed, Jul 08, 2020 at 10:44:10PM +0900, Paul Elder wrote:
> Fill the stride and frameSize fields of the StreamConfiguration at
> configuration validation time instead of at camera configuration time.
> This allows applications to get the stride when trying a configuration
> without modifying the active configuration of the camera.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v4:
> - fix stride and frameSize calculation
> - mention motivation in commit message
> 
> New in v3
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 00559ce..4a2d924 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -278,6 +278,21 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  				<< cfg.toString();
>  			status = Adjusted;
>  		}
> +
> +		const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> +		bool packedRaw = info.packed &&
> +				 info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> +
> +		if (packedRaw) {

I think you can write

		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {

as we can't support other raw formats than the IPU3 packed formats.

> +			cfg.stride = info.stride(cfg.size.width, 0, 64);
> +			cfg.frameSize = cfg.stride * cfg.size.height;
> +		} else {
> +			cfg.stride = info.stride(cfg.size.width, 0);
> +			std::array<unsigned int, 3> strides;
> +			for (unsigned int j = 0; j < 3; j++)
> +				strides[i] = info.stride(cfg.size.width, i, packedRaw ? 64 : 0);

packedRaw is always false here.

> +			cfg.frameSize = info.frameSize(cfg.size, strides);

I think you can thus just write

			cfg.stride = info.stride(cfg.size.width, 0);
			cfg.frameSize = info.frameSize(cfg.size);

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

> +		}
>  	}
>  
>  	return status;
> @@ -495,21 +510,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  			if (ret)
>  				return ret;
>  
> -			cfg.stride = outputFormat.planes[0].bpl;
>  			outActive = true;
>  		} else if (stream == vfStream) {
>  			ret = imgu->configureViewfinder(cfg, &outputFormat);
>  			if (ret)
>  				return ret;
>  
> -			cfg.stride = outputFormat.planes[0].bpl;
>  			vfActive = true;
> -		} else {
> -			/*
> -			 * The RAW stream is configured as part of the CIO2 and
> -			 * no configuration is needed for the ImgU.
> -			 */
> -			cfg.stride = cio2Format.planes[0].bpl;
>  		}
>  	}
>  

-- 
Regards,

Laurent Pinchart