[libcamera-devel] [PATCH v2 04/17] v4l2: v4l2_camera_proxy: Fix v4l2-compliance support for extended formats

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Jun 19, 2020 at 02:41:10PM +0900, Paul Elder wrote:
> Fix the following v4l2-compliance error:
> 
> fail: v4l2-compliance.cpp(652): !(caps & V4L2_CAP_EXT_PIX_FORMAT)
> 
> Simply add V4L2_CAP_EXT_PIX_FORMAT to capabilities in querycap.
> 
> In addition, populate the necessary fields in struct v4l2_pix_format to
> support extended pixel formats in try_fmt and g/s_fmt, and clear the
> reserved field for enum_fmt.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Acked-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> ---
> Changes in v2:
> - squashed the "Fix v4l2-compliance format tests" into this one, since
>   those fixes were a necessity of this fix (plus they're related)
> - use _DEFAULT for ycbcr_enc and quantization
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index cff6562..117d7ff 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -168,6 +168,9 @@ void V4L2CameraProxy::setFmtFromConfig(StreamConfiguration &streamConfig)
>  			  curV4L2Format_.fmt.pix.width,
>  			  curV4L2Format_.fmt.pix.height);
>  	curV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;
> +	curV4L2Format_.fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> +	curV4L2Format_.fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> +	curV4L2Format_.fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT;

Shouldn't you also set xfer_func ? Same below.

>  }
>  
>  unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConfig)
> @@ -194,7 +197,9 @@ void V4L2CameraProxy::querycap(std::shared_ptr<Camera> camera)
>  		       sizeof(capabilities_.bus_info));
>  	/* \todo Put this in a header/config somewhere. */
>  	capabilities_.version = KERNEL_VERSION(5, 2, 0);
> -	capabilities_.device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> +	capabilities_.device_caps = V4L2_CAP_VIDEO_CAPTURE
> +				  | V4L2_CAP_STREAMING

You could keep these two in a single line, up to you.

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

> +				  | V4L2_CAP_EXT_PIX_FORMAT;
>  	capabilities_.capabilities = capabilities_.device_caps
>  				   | V4L2_CAP_DEVICE_CAPS;
>  	memset(capabilities_.reserved, 0, sizeof(capabilities_.reserved));
> @@ -243,11 +248,15 @@ int V4L2CameraProxy::vidioc_enum_fmt(V4L2CameraFile *cf, struct v4l2_fmtdesc *ar
>  	    arg->index >= streamConfig_.formats().pixelformats().size())
>  		return -EINVAL;
>  
> +	/* \todo Set V4L2_FMT_FLAG_COMPRESSED for compressed formats. */
> +	arg->flags = 0;
>  	/* \todo Add map from format to description. */
> -	utils::strlcpy(reinterpret_cast<char *>(arg->description), "Video Format Description",
> -		       sizeof(arg->description));
> +	utils::strlcpy(reinterpret_cast<char *>(arg->description),
> +		       "Video Format Description", sizeof(arg->description));
>  	arg->pixelformat = drmToV4L2(streamConfig_.formats().pixelformats()[arg->index]);
>  
> +	memset(arg->reserved, 0, sizeof(arg->reserved));
> +
>  	return 0;
>  }
>  
> @@ -287,6 +296,9 @@ void V4L2CameraProxy::tryFormat(struct v4l2_format *arg)
>  					      arg->fmt.pix.width,
>  					      arg->fmt.pix.height);
>  	arg->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;
> +	arg->fmt.pix.priv         = V4L2_PIX_FMT_PRIV_MAGIC;
> +	arg->fmt.pix.ycbcr_enc    = V4L2_YCBCR_ENC_DEFAULT;
> +	arg->fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT;
>  }
>  
>  int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *cf, struct v4l2_format *arg)

-- 
Regards,

Laurent Pinchart