Re: [PATCH 3/3] soc-camera: let camera host drivers decide upon pixel format

[Robert Jarzmik <robert.jarzmik@free.fr>]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c
> index 2a811f8..a375872 100644
> --- a/drivers/media/video/pxa_camera.c
> +++ b/drivers/media/video/pxa_camera.c
> @@ -907,17 +907,43 @@ static int pxa_camera_try_bus_param(struct soc_camera_device *icd, __u32 pixfmt)
>  static int pxa_camera_set_fmt_cap(struct soc_camera_device *icd,
>  				  __u32 pixfmt, struct v4l2_rect *rect)
>  {
> -	return icd->ops->set_fmt_cap(icd, pixfmt, rect);
> +	const struct soc_camera_data_format *cam_fmt;
> +	int ret;
> +
> +	/*
> +	 * TODO: find a suitable supported by the SoC output format, check
> +	 * whether the sensor supports one of acceptable input formats.
> +	 */
> +	if (pixfmt) {
> +		cam_fmt = soc_camera_format_by_fourcc(icd, pixfmt);
> +		if (!cam_fmt)
> +			return -EINVAL;
> +	}
All right, here is something I don't understand.

Let's take an example : the pxa_camera was asked a YUV422P pixel format. It can
deserve it by asking the sensor a UYVY format. So the logical step would be to
do something like :

	if (pixfmt == V4L2_PIX_FMT_YUV422P)
		pixfmt = V4L2_PIX_FMT_UYVY;

at the beginning of pxa_camera_set_fmt_cap().

> +
> +	ret = icd->ops->set_fmt_cap(icd, pixfmt, rect);
> +	if (pixfmt && !ret)
> +		icd->current_fmt = cam_fmt;
So here, icd->current_fmt = V4L2_PIX_FMT_UYVY, and not V4L2_PIX_FMT_YUV422P;

> @@ -345,14 +325,21 @@ static int soc_camera_s_fmt_vid_cap(struct file *file, void *priv,
>  	rect.width	= f->fmt.pix.width;
>  	rect.height	= f->fmt.pix.height;
>  	ret = ici->ops->set_fmt_cap(icd, f->fmt.pix.pixelformat, &rect);
> -	if (ret < 0)
> +	if (ret < 0) {
>  		return ret;
> +	} else if (!icd->current_fmt ||
> +		   icd->current_fmt->fourcc != f->fmt.pix.pixelformat) {
> +		dev_err(&ici->dev, "Host driver hasn't set up current "
> +			"format correctly!\n");
> +		return -EINVAL;
> +	}
And here, we fall into the error case, because icd->current_fmt is
V4L2_PIX_FMT_UYVY, and f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUV422P.

So there is still something to improve, or have I missed something ?

--
Robert

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list