[libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support for PREPARE_BUF as one of the supported ioctl

[Laurent Pinchart]

Hi Vedant,

Thank you for the patch.

On Tue, Jan 11, 2022 at 10:03:29PM +0530, Vedant Paranjape wrote:
> Add support for PREPARE_BUF as one of the ioctl. Since this is a compat
> layer, there doesn't seem to be an equivalent to the "transfer ownership
> of the buffer to kernel driver" in V4L2Camera class.
> 
> To match the error checks done by kernel implementation, we'd have to
> check if dmabuf fd is valid and that the buffer size is large enough.
> Doing so will not add any particular value to the program as
> applications don't depend on these conditions being handled correctly.

s/don't depend/most likely don't depend/ (there can always be corner
cases, developers sometimes get inventive)

> Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> ---
> Source builds and tests fine, also passes the v4l2-compliance
> 
> <snip>
> vedant at pc ~/libcamera$ ./test/v4l2_compat/v4l2_compat_test.py ./build/src/v4l2/v4l2-compat.so
> Testing /dev/video0 with uvcvideo driver... success
> </snip>
> 
> Still figuring out how to test this with v4l2-ctl, if you have hints
> please let me know :)

v4l2-compliance tests VIDIOC_PREPARE buf, that should be good enough as
a test.

> ---
> src/v4l2/v4l2_camera_proxy.cpp | 28 ++++++++++++++++++++++++++++
>  src/v4l2/v4l2_camera_proxy.h   |  1 +
>  2 files changed, 29 insertions(+)
> 
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 4d529bc29a4d..8c28e8738a06 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -544,6 +544,33 @@ int V4L2CameraProxy::vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *a
>  	return 0;
>  }
>  
> +int V4L2CameraProxy::vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg)
> +{
> +	LOG(V4L2Compat, Debug) << "Servicing vidioc_prepare_buf, index = "
> +			       << arg->index << " fd = " << file->efd();
> +
> +	if (!hasOwnership(file))
> +		return -EBUSY;
> +
> +	if (arg->index >= bufferCount_)
> +		return -EINVAL;
> +

	struct v4l2_buffer &buffer = buffers_[arg->index];

and use buffer below.

With this change, and v4l2-compliance passing,

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

> +	if (buffers_[arg->index].flags & V4L2_BUF_FLAG_REQUEST_FD ||
> +	    buffers_[arg->index].flags & V4L2_BUF_FLAG_PREPARED)
> +		return -EINVAL;
> +
> +	if (buffers_[arg->index].flags & V4L2_BUF_FLAG_QUEUED ||
> +	    !validateBufferType(arg->type) ||
> +	    !validateMemoryType(arg->memory))
> +		return -EINVAL;
> +
> +	buffers_[arg->index].flags |= V4L2_BUF_FLAG_PREPARED;
> +
> +	arg->flags = buffers_[arg->index].flags;
> +
> +	return 0;
> +}
> +
>  int V4L2CameraProxy::vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg)
>  {
>  	LOG(V4L2Compat, Debug) << "Servicing vidioc_qbuf, index = "
> @@ -709,6 +736,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {
>  	VIDIOC_S_INPUT,
>  	VIDIOC_REQBUFS,
>  	VIDIOC_QUERYBUF,
> +	VIDIOC_PREPARE_BUF,
>  	VIDIOC_QBUF,
>  	VIDIOC_DQBUF,
>  	VIDIOC_EXPBUF,
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index 14e027c3e7d1..6baba94262a9 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -57,6 +57,7 @@ private:
>  	int vidioc_s_input(V4L2CameraFile *file, int *arg);
>  	int vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuffers *arg);
>  	int vidioc_querybuf(V4L2CameraFile *file, struct v4l2_buffer *arg);
> +	int vidioc_prepare_buf(V4L2CameraFile *file, struct v4l2_buffer *arg);
>  	int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);
>  	int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,
>  			 libcamera::Mutex *lock) LIBCAMERA_TSA_REQUIRES(*lock);
> -- 
> 2.25.1
> 

-- 
Regards,

Laurent Pinchart