[libcamera-devel] [PATCH v1] v4l2: V4L2CameraProxy: Add support for PREPARE_BUF as one of the supported ioctl
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Jan 11 20:21:49 CET 2022
On Tue, Jan 11, 2022 at 09:10:53PM +0200, Laurent Pinchart wrote:
> 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 ||
Actually, this isn't correct, you need to check arg->flags here.
> > + 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;
And let's group all the flags check:
if (arg->flags & V4L2_BUF_FLAG_REQUEST_FD)
return -EINVAL;
if (!validateBufferType(arg->type) || !validateMemoryType(arg->memory))
return -EINVAL;
struct v4l2_buffer &buffer = buffers_[arg->index];
if (buffer.flags & V4L2_BUF_FLAG_PREPARED ||
buffer.flags & V4L2_BUF_FLAG_QUEUED ||
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);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list