[libcamera-devel] [PATCH v3 28/33] libcamera: v4l2_videodevice: Use FileDescriptor where appropriate

[Laurent Pinchart]

Hi Niklas,

On Fri, Jan 10, 2020 at 08:38:03PM +0100, Niklas Söderlund wrote:
> From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> Return a FileDescriptor instead of a numerical fd from exportDmabufFd().
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

Feel free to squash this with the patch that introduced
exportDmabufFd(). If the rebase gets too messy I'm fine keeping it
separate, but please let me know explicitly in this case.

> ---
>  src/libcamera/include/v4l2_videodevice.h |  2 +-
>  src/libcamera/v4l2_videodevice.cpp       | 14 ++++++--------
>  2 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index 6959b87a35496b9a..172d670c328b2345 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -217,7 +217,7 @@ private:
>  
>  	int requestBuffers(unsigned int count);
>  	std::unique_ptr<FrameBuffer> createBuffer(const struct v4l2_buffer &buf);
> -	int exportDmabufFd(unsigned int index, unsigned int plane);
> +	FileDescriptor exportDmabufFd(unsigned int index, unsigned int plane);
>  
>  	void bufferAvailable(EventNotifier *notifier);
>  	FrameBuffer *dequeueBuffer();
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 59a6d1b5537d776f..c0381d3980063799 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1041,24 +1041,22 @@ V4L2VideoDevice::createBuffer(const struct v4l2_buffer &buf)
>  
>  	std::vector<FrameBuffer::Plane> planes;
>  	for (unsigned int nplane = 0; nplane < numPlanes; nplane++) {
> -		int fd = exportDmabufFd(buf.index, nplane);
> -		if (fd < 0)
> +		FileDescriptor fd = exportDmabufFd(buf.index, nplane);
> +		if (fd.fd() < 0)
>  			return nullptr;
>  
>  		FrameBuffer::Plane plane;
> -		plane.fd = FileDescriptor(fd);
> +		plane.fd = std::move(fd);
>  		plane.length = multiPlanar ?
>  			buf.m.planes[nplane].length : buf.length;
>  
>  		planes.push_back(std::move(plane));
> -
> -		::close(fd);
>  	}
>  
>  	return utils::make_unique<FrameBuffer>(std::move(planes));
>  }
>  
> -int V4L2VideoDevice::exportDmabufFd(unsigned int index, unsigned int plane)
> +FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index, unsigned int plane)
>  {
>  	struct v4l2_exportbuffer expbuf = {};
>  	int ret;
> @@ -1072,10 +1070,10 @@ int V4L2VideoDevice::exportDmabufFd(unsigned int index, unsigned int plane)
>  	if (ret < 0) {
>  		LOG(V4L2, Error)
>  			<< "Failed to export buffer: " << strerror(-ret);
> -		return ret;
> +		return FileDescriptor();
>  	}
>  
> -	return expbuf.fd;
> +	return FileDescriptor(expbuf.fd);
>  }
>  
>  /**

-- 
Regards,

Laurent Pinchart