| Message ID | 20211129161336.1113408-1-vedantparanjape160201@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Vedant, Thank you for the patch. 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@gmail.com> This looks fine, Reviewed-by: Laurent Pinchart <laurent.pinchart@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 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); >
Hi Laurent, On Mon, Nov 29, 2021 at 10:01 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Vedant, > > Thank you for the patch. > > 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@gmail.com> > > This looks fine, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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 ? > 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
Hi Vedant, 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@gmail.com> > > > > This looks fine, > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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. > > 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
Hi Vedant, 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@gmail.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > 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); > > -- > 2.25.1 >
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@gmail.com> > > > > > > This looks fine, > > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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); > > > >
Hello Laurent, Sorry for the delay, I've had a busy week, and will check it's working this week and report back. Regards, Vedant Paranjape On Mon, 6 Dec, 2021, 18:05 Laurent Pinchart, < laurent.pinchart@ideasonboard.com> wrote: > 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@gmail.com> > > > > > > > > This looks fine, > > > > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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 >
Hello Laurent,
I have been able to test the patch. The steps I have followed:
1) clone this repo: https://git.libcamera.org/libcamera/vivid.git
2) Apply the patch in the trailing email, there's a small conflict
while doing so in v4l2_camera_proxy.h (as the patch was sent some time
back, probably needs to be rebased)
3) meson build
4) ninja -C build
5) sudo modprobe vivid
6) export LIBCAMERA_LOG_LEVELS='V4L2Compat:0'
7) run the following command:
LD_PRELOAD=build/src/v4l2/v4l2-compat.so v4l2-ctl --device
/dev/video0 --stream-mmap --out-device /dev/video3 --stream-out-dmabuf
I have attached the output of lsv4l2 here as well:
video0: HD WebCam: HD WebCam
video1: HD WebCam: HD WebCam
video2: vivid-000-vid-cap
video3: vivid-000-vid-out
Here's the terminal log that I got: https://paste.debian.net/1225055/
Regards,
Vedant Paranjape
Hi Vedant, On Tue, Dec 28, 2021 at 11:27:01PM +0530, Vedant Paranjape wrote: > Hello Laurent, > I have been able to test the patch. The steps I have followed: > > 1) clone this repo: https://git.libcamera.org/libcamera/vivid.git > 2) Apply the patch in the trailing email, there's a small conflict > while doing so in v4l2_camera_proxy.h (as the patch was sent some time > back, probably needs to be rebased) > 3) meson build > 4) ninja -C build > 5) sudo modprobe vivid > 6) export LIBCAMERA_LOG_LEVELS='V4L2Compat:0' > 7) run the following command: > LD_PRELOAD=build/src/v4l2/v4l2-compat.so v4l2-ctl --device > /dev/video0 --stream-mmap --out-device /dev/video3 --stream-out-dmabuf > > I have attached the output of lsv4l2 here as well: > > video0: HD WebCam: HD WebCam > video1: HD WebCam: HD WebCam > video2: vivid-000-vid-cap > video3: vivid-000-vid-out > > Here's the terminal log that I got: https://paste.debian.net/1225055/ The command doesn't seem to capture any frame :-(
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);
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@gmail.com> --- src/v4l2/v4l2_camera_proxy.cpp | 30 ++++++++++++++++++++++++++++++ src/v4l2/v4l2_camera_proxy.h | 1 + 2 files changed, 31 insertions(+)