[libcamera-devel] [PATCH v2] libcamera: pipeline: vimc: Skip unsupported formats

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Jun 04, 2020 at 10:04:27AM +0100, Kieran Bingham wrote:
> Older kernels do not support all 'reported' formats. Skip them on those
> kernels.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> 
> v2:
>  - Remove todo, and confirm the kernel version.
>  - Adjust validate() to reference the available formats in the
>    configuration.
> 
>  src/libcamera/pipeline/vimc/vimc.cpp | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 68d65bc785b0..914df3476eb9 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -136,8 +136,9 @@ CameraConfiguration::Status VimcCameraConfiguration::validate()
>  	StreamConfiguration &cfg = config_[0];
>  
>  	/* Adjust the pixel format. */
> -	if (pixelformats.find(cfg.pixelFormat) == pixelformats.end()) {
> -		LOG(VIMC, Debug) << "Adjusting format to RGB24";
> +	const std::vector<libcamera::PixelFormat> formats = cfg.formats().pixelformats();
> +	if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) == formats.end()) {
> +		LOG(VIMC, Debug) << "Adjusting format to BGR888";
>  		cfg.pixelFormat = PixelFormat(DRM_FORMAT_BGR888);
>  		status = Adjusted;
>  	}
> @@ -171,6 +172,7 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
>  	CameraConfiguration *config = new VimcCameraConfiguration();
> +	VimcCameraData *data = cameraData(camera);
>  
>  	if (roles.empty())
>  		return config;
> @@ -178,6 +180,19 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
>  	std::map<PixelFormat, std::vector<SizeRange>> formats;
>  
>  	for (const auto &pixelformat : pixelformats) {
> +		/*
> +		 * Kernels previous to v5.7 incorrectly report support for

s/previous/prior/ ?

> +		 * RGB888, but it isn't functional within the pipeline.
> +		 */
> +		if (data->media_->version() < KERNEL_VERSION(5, 7, 0)) {
> +			if (pixelformat.first != PixelFormat(DRM_FORMAT_BGR888)) {
> +				LOG(VIMC, Info)
> +					<< "Skipping unsupported pixel format "
> +					<< pixelformat.first.toString();
> +				continue;
> +			}
> +		}

I think we'll need more complex logic in the future when new formats
will be supported, but that can wait.

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

> +
>  		/* The scaler hardcodes a x3 scale-up ratio. */
>  		std::vector<SizeRange> sizes{
>  			SizeRange{ { 48, 48 }, { 4096, 2160 } }

-- 
Regards,

Laurent Pinchart