| Message ID | 20230814112849.176943-4-gabbymg94@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Gabby George (2023-08-14 12:28:47) > To perform a memory mapping using mmap, the MappedFrameBuffer class needs the plane offset and file descriptor information of the frame buffer's plane(s). This information is provided in the response to REQBUF, which happens during buffer allocation. Store the plane offset and file descriptor information in the buffer's plane at the time of allocation. > > Currently, there is a metadata buffer type (metadata format UVCH) that does not support exporting buffers using EXPBUF, so this should only be done if the buffer type is metadata capture. The wrapping on all of the above needs to be considered. > > Signed-off-by: Gabby George <gabbymg94@gmail.com> > --- > src/libcamera/v4l2_videodevice.cpp | 32 ++++++++++++++++++++---------- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp > index a72ef64d..8cf427c0 100644 > --- a/src/libcamera/v4l2_videodevice.cpp > +++ b/src/libcamera/v4l2_videodevice.cpp > @@ -1402,18 +1402,28 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) > > std::vector<FrameBuffer::Plane> planes; > for (unsigned int nplane = 0; nplane < numPlanes; nplane++) { > - UniqueFD fd = exportDmabufFd(buf.index, nplane); > - if (!fd.isValid()) > - return nullptr; > - > FrameBuffer::Plane plane; > - plane.fd = SharedFD(std::move(fd)); > - /* > - * V4L2 API doesn't provide dmabuf offset information of plane. > - * Set 0 as a placeholder offset. > - * \todo Set the right offset once V4L2 API provides a way. > - */ > - plane.offset = 0; > + > + if (buf.type != V4L2_BUF_TYPE_META_CAPTURE) { I think it would be better to construct this so you handle the 'truth' rather than the 'negative' i.e. if (buf.type == V4L2_BUF_TYPE_META_CAPTURE) { } else { } It could be a switch case too if that's cleaner, but it depends if there will be other conditional handling here in the future I guess. > + UniqueFD fd = exportDmabufFd(buf.index, nplane); > + if (!fd.isValid()) > + return nullptr; > + plane.fd = SharedFD(std::move(fd)); > + > + /* > + * V4L2 API doesn't provide dmabuf offset information of plane. > + * Set 0 as a placeholder offset. > + * \todo Set the right offset once V4L2 API provides a way. > + */ I see you've taken this comment and indented it. The comment prefixes need to be corrected, and the lines likely need to be re-wrapped to fit to 80 chars nicely. > + plane.offset = 0; > + } else { > + /* Dmabuf fd is not exported for metadata, so store > + * the offset from the querybuf call and this device's fd. > + */ Please fix this comment style issue. > + SharedFD tmp(this->fd()); 'tmp' is rarely a good name for a variable. > + plane.fd = tmp; What about plane.fd = SharedFD(this->fd()); Does that do the same thing ? > + plane.offset = buf.m.offset; > + } > plane.length = multiPlanar ? buf.m.planes[nplane].length : buf.length; > > planes.push_back(std::move(plane)); > -- > 2.34.1 >
diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index a72ef64d..8cf427c0 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1402,18 +1402,28 @@ std::unique_ptr<FrameBuffer> V4L2VideoDevice::createBuffer(unsigned int index) std::vector<FrameBuffer::Plane> planes; for (unsigned int nplane = 0; nplane < numPlanes; nplane++) { - UniqueFD fd = exportDmabufFd(buf.index, nplane); - if (!fd.isValid()) - return nullptr; - FrameBuffer::Plane plane; - plane.fd = SharedFD(std::move(fd)); - /* - * V4L2 API doesn't provide dmabuf offset information of plane. - * Set 0 as a placeholder offset. - * \todo Set the right offset once V4L2 API provides a way. - */ - plane.offset = 0; + + if (buf.type != V4L2_BUF_TYPE_META_CAPTURE) { + UniqueFD fd = exportDmabufFd(buf.index, nplane); + if (!fd.isValid()) + return nullptr; + plane.fd = SharedFD(std::move(fd)); + + /* + * V4L2 API doesn't provide dmabuf offset information of plane. + * Set 0 as a placeholder offset. + * \todo Set the right offset once V4L2 API provides a way. + */ + plane.offset = 0; + } else { + /* Dmabuf fd is not exported for metadata, so store + * the offset from the querybuf call and this device's fd. + */ + SharedFD tmp(this->fd()); + plane.fd = tmp; + plane.offset = buf.m.offset; + } plane.length = multiPlanar ? buf.m.planes[nplane].length : buf.length; planes.push_back(std::move(plane));
To perform a memory mapping using mmap, the MappedFrameBuffer class needs the plane offset and file descriptor information of the frame buffer's plane(s). This information is provided in the response to REQBUF, which happens during buffer allocation. Store the plane offset and file descriptor information in the buffer's plane at the time of allocation. Currently, there is a metadata buffer type (metadata format UVCH) that does not support exporting buffers using EXPBUF, so this should only be done if the buffer type is metadata capture. Signed-off-by: Gabby George <gabbymg94@gmail.com> --- src/libcamera/v4l2_videodevice.cpp | 32 ++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-)