[libcamera-devel] [PATCH 06/15] v4l2: v4l2_camera_proxy: Implement VIDIOC_ENUM_FRAMESIZES

[Jacopo Mondi]

Hi Paul,

On Tue, Jun 16, 2020 at 10:12:35PM +0900, Paul Elder wrote:
> Implement VIDIOC_ENUM_FRAMESIZES in the V4L2 compatibility layer.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/v4l2/v4l2_camera_proxy.cpp | 24 ++++++++++++++++++++++++
>  src/v4l2/v4l2_camera_proxy.h   |  1 +
>  2 files changed, 25 insertions(+)
>
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 6c0dacc..a5fa478 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -251,6 +251,27 @@ int V4L2CameraProxy::vidioc_querycap(struct v4l2_capability *arg)
>  	return 0;
>  }
>
> +int V4L2CameraProxy::vidioc_enum_framesizes(int fd, struct v4l2_frmsizeenum *arg)
> +{
> +	LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_framesizes fd = " << fd;
> +
> +	if (arg == nullptr)
> +		return -EFAULT;
> +
> +	PixelFormat argFormat = v4l2ToDrm(arg->pixel_format);
 just format ?

> +	const std::vector<Size> &frameSizes = streamConfig_.formats().sizes(argFormat);

just sizes ?

Anyway, very few pipeline handlers report StreamFormats at the moment,
I would record this with a \todo entry.

Another point is that the streamConfig_ you here use to enumerate formats is
generated using the Viewfinder role only. In this case, I expect
pipeline handlers not to report any entry for, for example, raw
formats, while the Camera actually supports it.

There's no easy way around this I'm afraid, or at least I don't see an
easy one. I had to do a similar thing for Android, as you can see in
src/android/camera_device.cpp:243

There I decided to create a list of image resolutions, and test
them against the application provided pixel format, by using
CameraConfiguration::validate(). This probably won't scale here, as
the list of resolution would be quite arbitrary, while in Android I
have a specification to follow which dictates which resolution are
mandatory to be supported.

Generating multiple CameraConfiguration (one for each supported role)
and test all of them until we don't find on which reports the desired
image format is another option, but would require you to introduce an
API to do so in your V4L2Camera class. In both cases, a bit of rework
is required. What do you think ?

> +
> +	if (arg->index >= frameSizes.size())
> +		return -EINVAL;

That's fine, as if the format is not supported StreamFormats::sizes()
will return an empty vector, but the v4l2 specs do not say what is the
expected behaviour in that case. Does v4l2-compliance try this call
with an invalid format ?

> +
> +	arg->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +	arg->discrete.width = frameSizes[arg->index].width;
> +	arg->discrete.height = frameSizes[arg->index].height;
> +	memset(arg->reserved, 0, sizeof(arg->reserved));
> +
> +	return 0;
> +}
> +
>  int V4L2CameraProxy::vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg)
>  {
>  	LOG(V4L2Compat, Debug) << "Servicing vidioc_enum_fmt fd = " << fd;
> @@ -676,6 +697,9 @@ int V4L2CameraProxy::ioctl(int fd, unsigned long request, void *arg)
>  	case VIDIOC_QUERYCAP:
>  		ret = vidioc_querycap(static_cast<struct v4l2_capability *>(arg));
>  		break;
> +	case VIDIOC_ENUM_FRAMESIZES:
> +		ret = vidioc_enum_framesizes(fd, static_cast<struct v4l2_frmsizeenum *>(arg));
> +		break;
>  	case VIDIOC_ENUM_FMT:
>  		ret = vidioc_enum_fmt(fd, static_cast<struct v4l2_fmtdesc *>(arg));
>  		break;
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index 2e90bfd..2ff9571 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -45,6 +45,7 @@ private:
>  	int freeBuffers();
>
>  	int vidioc_querycap(struct v4l2_capability *arg);
> +	int vidioc_enum_framesizes(int fd, struct v4l2_frmsizeenum *arg);
>  	int vidioc_enum_fmt(int fd, struct v4l2_fmtdesc *arg);
>  	int vidioc_g_fmt(int fd, struct v4l2_format *arg);
>  	int vidioc_s_fmt(int fd, struct v4l2_format *arg);
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel