[libcamera-devel] [PATCH 2/2] v4l2: camera_proxy: Break out try_fmt

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Jan 07, 2020 at 05:50:38PM +0100, Jacopo Mondi wrote:
> Calling vidioc_s_fmt() calls vidioc_try_fmt() duplicating prinouts.
> 
> Breakout try format procedure and call it from both functions.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 53 +++++++++++++++++++---------------
>  src/v4l2/v4l2_camera_proxy.h   |  1 +
>  2 files changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index dbcf333a761f..a56318cb4fa3 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -236,13 +236,38 @@ int V4L2CameraProxy::vidioc_g_fmt(struct v4l2_format *arg)
>  	return 0;
>  }
>  
> +void V4L2CameraProxy::tryFmt(struct v4l2_format *arg)

Let's name this tryFormat.

I think we need to take this one step further and base the tryFormat()
implementation on generateConfiguration() + validate(), but for now,

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

> +{
> +	PixelFormat format = v4l2ToDrm(arg->fmt.pix.pixelformat);
> +	const std::vector<PixelFormat> &formats =
> +		streamConfig_.formats().pixelformats();
> +	if (std::find(formats.begin(), formats.end(), format) == formats.end())
> +		format = streamConfig_.formats().pixelformats()[0];
> +
> +	Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> +	const std::vector<Size> &sizes = streamConfig_.formats().sizes(format);
> +	if (std::find(sizes.begin(), sizes.end(), size) == sizes.end())
> +		size = streamConfig_.formats().sizes(format)[0];
> +
> +	arg->fmt.pix.width        = size.width;
> +	arg->fmt.pix.height       = size.height;
> +	arg->fmt.pix.pixelformat  = drmToV4L2(format);
> +	arg->fmt.pix.field        = V4L2_FIELD_NONE;
> +	arg->fmt.pix.bytesperline = bplMultiplier(drmToV4L2(format)) *
> +				    arg->fmt.pix.width;
> +	arg->fmt.pix.sizeimage    = imageSize(drmToV4L2(format),
> +					      arg->fmt.pix.width,
> +					      arg->fmt.pix.height);
> +	arg->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;
> +}
> +
>  int V4L2CameraProxy::vidioc_s_fmt(struct v4l2_format *arg)
>  {
>  	LOG(V4L2Compat, Debug) << "Servicing vidioc_s_fmt";
> +	if (!validateBufferType(arg->type))
> +		return -EINVAL;
>  
> -	int ret = vidioc_try_fmt(arg);
> -	if (ret < 0)
> -		return ret;
> +	tryFmt(arg);
>  
>  	Size size(arg->fmt.pix.width, arg->fmt.pix.height);
>  	vcam_->invokeMethod(&V4L2Camera::configure, ConnectionTypeBlocking,
> @@ -268,27 +293,7 @@ int V4L2CameraProxy::vidioc_try_fmt(struct v4l2_format *arg)
>  	if (!validateBufferType(arg->type))
>  		return -EINVAL;
>  
> -	PixelFormat format = v4l2ToDrm(arg->fmt.pix.pixelformat);
> -	const std::vector<PixelFormat> &formats =
> -		streamConfig_.formats().pixelformats();
> -	if (std::find(formats.begin(), formats.end(), format) == formats.end())
> -		format = streamConfig_.formats().pixelformats()[0];
> -
> -	Size size(arg->fmt.pix.width, arg->fmt.pix.height);
> -	const std::vector<Size> &sizes = streamConfig_.formats().sizes(format);
> -	if (std::find(sizes.begin(), sizes.end(), size) == sizes.end())
> -		size = streamConfig_.formats().sizes(format)[0];
> -
> -	arg->fmt.pix.width        = size.width;
> -	arg->fmt.pix.height       = size.height;
> -	arg->fmt.pix.pixelformat  = drmToV4L2(format);
> -	arg->fmt.pix.field        = V4L2_FIELD_NONE;
> -	arg->fmt.pix.bytesperline = bplMultiplier(drmToV4L2(format)) *
> -				    arg->fmt.pix.width;
> -	arg->fmt.pix.sizeimage    = imageSize(drmToV4L2(format),
> -					      arg->fmt.pix.width,
> -					      arg->fmt.pix.height);
> -	arg->fmt.pix.colorspace   = V4L2_COLORSPACE_SRGB;
> +	tryFmt(arg);
>  
>  	return 0;
>  }
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index bef0f0afab3b..d34de925cf29 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -39,6 +39,7 @@ private:
>  	void setFmtFromConfig(StreamConfiguration &streamConfig);
>  	unsigned int calculateSizeImage(StreamConfiguration &streamConfig);
>  	void querycap(std::shared_ptr<Camera> camera);
> +	void tryFmt(struct v4l2_format *arg);
>  	void updateBuffers();
>  	int freeBuffers();
>  

-- 
Regards,

Laurent Pinchart