[libcamera-devel] [PATCH v3] v4l2: V4L2CameraProxy: Add EXPBUF as one of the supported ioctl

[Laurent Pinchart]

Hi Vedant,

On Mon, Nov 29, 2021 at 09:18:38PM +0200, Laurent Pinchart wrote:
> On Tue, Nov 30, 2021 at 12:29:45AM +0530, Vedant Paranjape wrote:
> > On Mon, Nov 29, 2021 at 10:01 PM Laurent Pinchart wrote:
> > > On Mon, Nov 29, 2021 at 09:43:36PM +0530, Vedant Paranjape wrote:
> > > > To support DMABUF as one of the memory buffer, we need to implement
> > > > EXPBUF in the v4l2 compat layer.
> > > >
> > > > This patch implements vidioc_expbuf as one of the supported ioctls.
> > > >
> > > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=89
> > > >
> > > > Signed-off-by: Vedant Paranjape <vedantparanjape160201 at gmail.com>
> > >
> > > This looks fine,
> > >
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > >
> > > Now we need to get this tested before merging it :-) v4l2-ctl could be
> > > an option, with (taken from the example in the manpage)
> > >
> > > v4l2-ctl --device /dev/video1 --stream-mmap \
> > >         --out-device /dev/video2 --stream-out-dmabuf
> > 
> > This should be preceded by LD_PRELOD, right ?
> 
> Otherwise you wouldn't be testing much, yes :-) You should enable
> logging and make sure the the "Servicing vidioc_expbuf" message is
> printed during your tests.

Have you been able to test this patch successfully (with enough
confidence that the test case actually exercises the EXPBUF code path) ?

> > > using the vivid output device.
> > >
> > > > ---
> > > >  src/v4l2/v4l2_camera_proxy.cpp | 30 ++++++++++++++++++++++++++++++
> > > >  src/v4l2/v4l2_camera_proxy.h   |  1 +
> > > >  2 files changed, 31 insertions(+)
> > > >
> > > > diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> > > > index 3610e63cade3..f194e06345b7 100644
> > > > --- a/src/v4l2/v4l2_camera_proxy.cpp
> > > > +++ b/src/v4l2/v4l2_camera_proxy.cpp
> > > > @@ -624,6 +624,32 @@ int V4L2CameraProxy::vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,
> > > >       return 0;
> > > >  }
> > > >
> > > > +int V4L2CameraProxy::vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg)
> > > > +{
> > > > +     LOG(V4L2Compat, Debug) << "Servicing vidioc_expbuf fd = " << file->efd();
> > > > +
> > > > +     if (!hasOwnership(file))
> > > > +             return -EBUSY;
> > > > +
> > > > +     /* \todo Verify that the memory type is MMAP when adding DMABUF support */
> > > > +     if (!validateBufferType(arg->type))
> > > > +             return -EINVAL;
> > > > +
> > > > +     if (arg->index >= bufferCount_)
> > > > +             return -EINVAL;
> > > > +
> > > > +     if (arg->flags & ~(O_CLOEXEC | O_ACCMODE))
> > > > +             return -EINVAL;
> > > > +
> > > > +     memset(arg->reserved, 0, sizeof(arg->reserved));
> > > > +
> > > > +     /* \todo honor the O_ACCMODE flags passed to this function */
> > > > +     arg->fd = fcntl(vcam_->getBufferFd(arg->index).fd(),
> > > > +                     arg->flags & O_CLOEXEC ? F_DUPFD_CLOEXEC : F_DUPFD, 0);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > >  int V4L2CameraProxy::vidioc_streamon(V4L2CameraFile *file, int *arg)
> > > >  {
> > > >       LOG(V4L2Compat, Debug) << "Servicing vidioc_streamon fd = " << file->efd();
> > > > @@ -685,6 +711,7 @@ const std::set<unsigned long> V4L2CameraProxy::supportedIoctls_ = {
> > > >       VIDIOC_QUERYBUF,
> > > >       VIDIOC_QBUF,
> > > >       VIDIOC_DQBUF,
> > > > +     VIDIOC_EXPBUF,
> > > >       VIDIOC_STREAMON,
> > > >       VIDIOC_STREAMOFF,
> > > >  };
> > > > @@ -755,6 +782,9 @@ int V4L2CameraProxy::ioctl(V4L2CameraFile *file, unsigned long request, void *ar
> > > >       case VIDIOC_DQBUF:
> > > >               ret = vidioc_dqbuf(file, static_cast<struct v4l2_buffer *>(arg), &locker);
> > > >               break;
> > > > +     case VIDIOC_EXPBUF:
> > > > +             ret = vidioc_expbuf(file, static_cast<struct v4l2_exportbuffer *>(arg));
> > > > +             break;
> > > >       case VIDIOC_STREAMON:
> > > >               ret = vidioc_streamon(file, static_cast<int *>(arg));
> > > >               break;
> > > > diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> > > > index fccec241879d..81ef7788e9fe 100644
> > > > --- a/src/v4l2/v4l2_camera_proxy.h
> > > > +++ b/src/v4l2/v4l2_camera_proxy.h
> > > > @@ -58,6 +58,7 @@ private:
> > > >       int vidioc_qbuf(V4L2CameraFile *file, struct v4l2_buffer *arg);
> > > >       int vidioc_dqbuf(V4L2CameraFile *file, struct v4l2_buffer *arg,
> > > >                        libcamera::MutexLocker *locker);
> > > > +     int vidioc_expbuf(V4L2CameraFile *file, struct v4l2_exportbuffer *arg);
> > > >       int vidioc_streamon(V4L2CameraFile *file, int *arg);
> > > >       int vidioc_streamoff(V4L2CameraFile *file, int *arg);
> > > >

-- 
Regards,

Laurent Pinchart