[libcamera-devel] [PATCH 02/14] libcamera: v4l2_device: Overload open() to duplicate File descriptor

[Jacopo Mondi]

On Thu, Sep 08, 2022 at 08:48:38PM +0200, Xavier Roumegue via libcamera-devel wrote:
> Signed-off-by: Xavier Roumegue <xavier.roumegue at oss.nxp.com>
> ---
>  include/libcamera/internal/v4l2_device.h |  2 ++
>  src/libcamera/v4l2_device.cpp            | 33 ++++++++++++++++++++++++
>  2 files changed, 35 insertions(+)
>
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index 75304be1..8b759311 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -15,6 +15,7 @@
>  #include <linux/videodev2.h>
>
>  #include <libcamera/base/log.h>
> +#include <libcamera/base/shared_fd.h>
>  #include <libcamera/base/signal.h>
>  #include <libcamera/base/span.h>
>  #include <libcamera/base/unique_fd.h>
> @@ -54,6 +55,7 @@ protected:
>  	~V4L2Device();
>
>  	int open(unsigned int flags);
> +	int open(SharedFD handle);
>  	int setFd(UniqueFD fd);
>
>  	int ioctl(unsigned long request, void *argp);
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 756188ea..67847bb0 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -102,6 +102,39 @@ int V4L2Device::open(unsigned int flags)
>  	return 0;
>  }
>
> +/**
> + * \brief Open a V4L2 device node

" from a shared file descriptor"

> + * \param[in] Shared File descriptor Handle

Either "FileDescriptor" or "file descriptor"

> + *
> + * Duplicate a shared file descriptor handle and use it for further device
> + * syscalls

'.' at the end of the line.

Do we need to specify what is the intended use case instead of saying
what the function does ?

"Open a V4L2 device from a shared file descriptor. This function is
used by V4L2 memory-to-memory device where a single video device node
represents multiple capture/output queues.

With comment on patch 3 addressed
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2Device::open(SharedFD handle)
> +{
> +	int ret;
> +
> +	UniqueFD newFd = handle.dup();
> +	if (!newFd.isValid()) {
> +		ret = -errno;
> +		LOG(V4L2, Error) << "Failed to duplicate file handle: "
> +				 << strerror(-ret);
> +		return ret;
> +	}
> +
> +	ret = setFd(std::move(newFd));
> +	if (ret < 0) {
> +		LOG(V4L2, Error) << "Failed to set file handle: "
> +				 << strerror(-ret);
> +		return ret;
> +	}
> +
> +	listControls();
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Set the file descriptor of a V4L2 device
>   * \param[in] fd The file descriptor handle
> --
> 2.37.3
>