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

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, May 22, 2020 at 07:14:35PM +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>
> ---
> Marking this as RFC, as I don't actually yet know what the correct
> kernel version is to condition against.

I think it will be v5.9 if we're lucky :-)

> Currently, the QCam application can not stream the VIMC Camera for users
> of QT Version less that 5.14 as that is when the BGR888 format becomes a
> 'preferred' native format.
> 
> Users of QT < 5.14 will find that qcam will chose a 'preferred' format
> which is reported as supported by the kernel, when in fact it isn't.
> 
> Ensure that those formats are not reported as supported by the vimc
> pipeline handler on older kernels.
> 
>  src/libcamera/pipeline/vimc/vimc.cpp | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 68d65bc785b0..78fb0ece21f8 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -171,6 +171,7 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
>  	const StreamRoles &roles)
>  {
>  	CameraConfiguration *config = new VimcCameraConfiguration();
> +	VimcCameraData *data = cameraData(camera);
>  
>  	if (roles.empty())
>  		return config;
> @@ -178,6 +179,19 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
>  	std::map<PixelFormat, std::vector<SizeRange>> formats;
>  
>  	for (const auto &pixelformat : pixelformats) {
> +		/*
> +		 * \todo: Update this when the kernel correctly supports other
> +		 * reported formats.
> +		 */
> +		if (data->media_->version() <= KERNEL_VERSION(5, 7, 0)) {

I wouldn't hardcode any kernel version until we know when the issue will
be fixed. I would remove the unsupported formats from the pixelformats
map now, and deal with versioning later.

> +			if (pixelformat.first != PixelFormat(DRM_FORMAT_BGR888)) {
> +				LOG(VIMC, Info)
> +					<< "Skipping unsupported pixel format"
> +					<< pixelformat.first.toString();
> +				continue;
> +			}
> +		}
> +

You also need to patch the VimcCameraConfiguration::validate() and
PipelineHandlerVimc::configure() functions, as they both use the
pixelformats map. One option would be to use
StreamConfiguration::formats().pixelformats() in both locations, so only
one location would need to deal with kernel versions.

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

-- 
Regards,

Laurent Pinchart