[libcamera-devel,v3,16/17] v4l2: v4l2_camera: Return int in getBufferFd()
diff mbox series

Message ID 20211128235752.10836-17-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: Introduce UniqueFD
Related show

Commit Message

Laurent Pinchart Nov. 28, 2021, 11:57 p.m. UTC
From: Hirokazu Honda <hiroh@chromium.org>

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.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 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(-)

Comments

Jacopo Mondi Nov. 29, 2021, 4 p.m. UTC | #1
Hi Laurent

On Mon, Nov 29, 2021 at 01:57:51AM +0200, Laurent Pinchart wrote:
> From: Hirokazu Honda <hiroh@chromium.org>
>
> 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.
>
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> ---
>  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 157ab94e0544..464507505c79 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 0cea111561dd..9817fd393d59 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -51,7 +51,7 @@ public:
>
>  	int allocBuffers(unsigned int count);
>  	void freeBuffers();
> -	libcamera::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 3610e63cade3..627e38d90f5b 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -114,14 +114,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
>

Patch
diff mbox series

diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 157ab94e0544..464507505c79 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 0cea111561dd..9817fd393d59 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -51,7 +51,7 @@  public:
 
 	int allocBuffers(unsigned int count);
 	void freeBuffers();
-	libcamera::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 3610e63cade3..627e38d90f5b 100644
--- a/src/v4l2/v4l2_camera_proxy.cpp
+++ b/src/v4l2/v4l2_camera_proxy.cpp
@@ -114,14 +114,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;