[libcamera-devel] [RFC PATCH 10/10] v4l2: V4L2Camera: Return int in getBufferFd()

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Thu, Apr 15, 2021 at 05:38:43PM +0900, Hirokazu Honda wrote:
> V4L2Camera::getBufferFd() returns FileDescriptor. However, the
> file descriptor is still owned by V4L2Camera. It should rather
> return an integer to represent V4L2Camera doesn't have the
> ownership of the file descriptor.

The FileDescriptor class doesn't imply ownership, as it's similar to a
shared_ptr<>. I'm not against this patch, but why do you think
FileDescriptor is bad here ?

> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/v4l2/v4l2_camera.cpp       | 6 +++---
>  src/v4l2/v4l2_camera.h         | 2 +-
>  src/v4l2/v4l2_camera_proxy.cpp | 6 +++---
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 97825c71..cccc6749 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -186,16 +186,16 @@ void V4L2Camera::freeBuffers()
>  	bufferAllocator_->free(stream);
>  }
>  
> -FileDescriptor V4L2Camera::getBufferFd(unsigned int index)
> +int V4L2Camera::getBufferFd(unsigned int index)
>  {
>  	Stream *stream = config_->at(0).stream();
>  	const std::vector<std::unique_ptr<FrameBuffer>> &buffers =
>  		bufferAllocator_->buffers(stream);
>  
>  	if (buffers.size() <= index)
> -		return FileDescriptor();
> +		return -1;
>  
> -	return buffers[index]->planes()[0].fd;
> +	return buffers[index]->planes()[0].fd.fd();
>  }
>  
>  int V4L2Camera::streamOn()
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index d2380462..8efe1642 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -53,7 +53,7 @@ public:
>  
>  	int allocBuffers(unsigned int count);
>  	void freeBuffers();
> -	FileDescriptor getBufferFd(unsigned int index);
> +	int getBufferFd(unsigned int index);
>  
>  	int streamOn();
>  	int streamOff();
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index f8bfe595..8f417421 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -112,14 +112,14 @@ void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,
>  		return MAP_FAILED;
>  	}
>  
> -	FileDescriptor fd = vcam_->getBufferFd(index);
> -	if (!fd.isValid()) {
> +	int fd = vcam_->getBufferFd(index);
> +	if (fd < 0) {
>  		errno = EINVAL;
>  		return MAP_FAILED;
>  	}
>  
>  	void *map = V4L2CompatManager::instance()->fops().mmap(addr, length, prot,
> -							       flags, fd.fd(), 0);
> +							       flags, fd, 0);
>  	if (map == MAP_FAILED)
>  		return map;
>  

-- 
Regards,

Laurent Pinchart