[libcamera-devel] [PATCH v3 3/6] libcamera: v4l2_videodevice: Support M2M devices

Jacopo Mondi jacopo at jmondi.org
Tue Aug 13 12:12:13 CEST 2019


Hi Kieran,
   a few notes

On Tue, Aug 13, 2019 at 10:40:17AM +0100, Kieran Bingham wrote:
> V4L2 M2M devices represent a V4L2Device with two queues: One output, and
> one capture on a single device node.
>
> Represent this by instantiating a V4L2VideoDevice for each queue type,
> and preparing each device for its queue.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/include/v4l2_videodevice.h |  25 +++
>  src/libcamera/v4l2_videodevice.cpp       | 194 ++++++++++++++++++++++-
>  2 files changed, 218 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index f5c8da93fcb5..0fc2dcd81d2b 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -71,6 +71,11 @@ struct V4L2Capability final : v4l2_capability {
>  					V4L2_CAP_VIDEO_OUTPUT |
>  					V4L2_CAP_VIDEO_OUTPUT_MPLANE);
>  	}
> +	bool isM2M() const
> +	{
> +		return device_caps() & (V4L2_CAP_VIDEO_M2M |
> +					V4L2_CAP_VIDEO_M2M_MPLANE);
> +	}
>  	bool isMeta() const
>  	{
>  		return device_caps() & (V4L2_CAP_META_CAPTURE |
> @@ -124,6 +129,7 @@ public:
>  	V4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete;
>
>  	int open();
> +	int open(int handle, enum v4l2_buf_type type);
>  	void close();
>
>  	const char *driverName() const { return caps_.driver(); }
> @@ -182,6 +188,25 @@ private:
>  	EventNotifier *fdEvent_;
>  };
>
> +class V4L2M2MDevice
> +{
> +public:
> +	V4L2M2MDevice(const std::string &deviceNode);
> +	~V4L2M2MDevice();
> +
> +	int open();
> +	void close();
> +
> +	V4L2VideoDevice *output() { return output_; }
> +	V4L2VideoDevice *capture() { return capture_; }
> +
> +private:
> +	std::string deviceNode_;
> +
> +	V4L2VideoDevice *output_;
> +	V4L2VideoDevice *capture_;
> +};
> +
>  } /* namespace libcamera */
>
>  #endif /* __LIBCAMERA_V4L2_VIDEODEVICE_H__ */
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 81098dd70190..5e41f44eeb50 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -89,6 +89,12 @@ LOG_DECLARE_CATEGORY(V4L2)
>   * \return True if the video device can capture or output images
>   */
>
> +/**
> + * \fn V4L2Capability::isM2M()
> + * \brief Identify if the device is an M2M device

nit: I would spell memory-to-memory. As you prefer though.

> + * \return True if the device can capture and output images using the M2M API
> + */
> +
>  /**
>   * \fn V4L2Capability::isMeta()
>   * \brief Identify if the video device captures or outputs image meta-data
> @@ -295,7 +301,8 @@ V4L2VideoDevice::~V4L2VideoDevice()
>  }
>
>  /**
> - * \brief Open a V4L2 video device and query its capabilities
> + * \brief Open the V4L2 video device node and query its capabilities
> + *
>   * \return 0 on success or a negative error code otherwise
>   */
>  int V4L2VideoDevice::open()
> @@ -355,6 +362,92 @@ int V4L2VideoDevice::open()
>  	return 0;
>  }
>
> +/**
> + * \brief Open a V4L2 video device from an opened file handle and query its
> + * capabilities
> + * \param[in] handle The file descriptor to set
> + * \param[in] type The device type to operate on
> + *
> + * This methods opens a video device from the existing file descriptor \a
> + * handle. Like open(), this method queries the capabilities of the device, but
> + * handles it according to the given device \a type instead of determining its
> + * type from the capabilities. This can be used to force a given device type for
> + * M2M devices.
> + *
> + * The file descriptor \a handle is duplicated, and the caller is responsible
> + * for closing the \a handle when it has no further use for it. The close()
> + * method will close the duplicated file descriptor, leaving \a handle
> + * untouched.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type)
> +{
> +	int ret;
> +	int newFd;
> +
> +	newFd = dup(handle);
> +	if (newFd < 0) {
> +		ret = -errno;
> +		LOG(V4L2, Error) << "Failed to duplicate file handle: "
> +				 << strerror(-ret);
> +		return ret;
> +	}
> +
> +	ret = V4L2Device::setFd(newFd);
> +	if (ret < 0) {
> +		LOG(V4L2, Error) << "Failed to set file handle: "
> +				 << strerror(-ret);
> +		::close(newFd);
> +		return ret;
> +	}
> +
> +	ret = ioctl(VIDIOC_QUERYCAP, &caps_);
> +	if (ret < 0) {
> +		LOG(V4L2, Error)
> +			<< "Failed to query device capabilities: "

Shouldn't you close newfd here and in the below error paths too?

> +			<< strerror(-ret);
> +		return ret;
> +	}
> +
> +	if (!caps_.hasStreaming()) {
> +		LOG(V4L2, Error) << "Device does not support streaming I/O";
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Set buffer type and wait for read notifications on CAPTURE video
> +	 * devices (POLLIN), and write notifications for OUTPUT video devices
> +	 * (POLLOUT).
> +	 */
> +	switch (type) {
> +	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> +		fdEvent_ = new EventNotifier(fd(), EventNotifier::Write);
> +		bufferType_ = caps_.isMultiplanar()
> +			    ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE
> +			    : V4L2_BUF_TYPE_VIDEO_OUTPUT;
> +		break;
> +	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> +		fdEvent_ = new EventNotifier(fd(), EventNotifier::Read);
> +		bufferType_ = caps_.isMultiplanar()
> +			    ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
> +			    : V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +		break;
> +	default:
> +		LOG(V4L2, Error) << "Unsupported buffer type";
> +		return -EINVAL;
> +	}
> +
> +	fdEvent_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
> +	fdEvent_->setEnabled(false);
> +
> +	LOG(V4L2, Debug)
> +		<< "Opened device " << caps_.bus_info() << ": "
> +		<< caps_.driver() << ": " << caps_.card();
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Close the video device, releasing any resources acquired by open()
>   */
> @@ -1143,4 +1236,103 @@ V4L2VideoDevice *V4L2VideoDevice::fromEntityName(const MediaDevice *media,
>  	return new V4L2VideoDevice(mediaEntity);
>  }
>
> +/**
> + * \class V4L2M2MDevice
> + * \brief Memory2Memory video device
> + *
> + * The V4L2M2MDevice manages two V4L2VideoDevice instances on the same
> + * deviceNode which operate together using two queues to implement the V4L2
> + * Memory to Memory API.
> + *
> + * The two devices should be opened by calling open() on the V4L2M2MDevice, and
> + * can be closed by calling close on the V4L2M2MDevice.
> + *
> + * Calling V4L2VideoDevice::open() and V4L2VideoDevice::close() on the capture
> + * or output V4L2VideoDevice is not permitted.
> + */
> +
> +/**
> + * \fn V4L2M2MDevice::output
> + * \brief Retrieve the output V4L2VideoDevice instance
> + * \return The output V4L2Device instance
V4L2VideoDevice

> + */
> +
> +/**
> + * \fn V4L2M2MDevice::capture
> + * \brief Retrieve the capture V4L2VideoDevice instance
> + * \return The capture V4L2Device instance

same here

> + */
> +
> +/**
> + * \brief Create a new V4L2M2MDevice from the \a deviceNode

missing parameter documentation

> + */
> +V4L2M2MDevice::V4L2M2MDevice(const std::string &deviceNode)
> +	: deviceNode_(deviceNode)
> +{
> +	output_ = new V4L2VideoDevice(deviceNode);
> +	capture_ = new V4L2VideoDevice(deviceNode);
> +}
> +
> +V4L2M2MDevice::~V4L2M2MDevice()
> +{
> +	delete capture_;
> +	delete output_;
> +}
> +
> +/**
> + * \brief Open a V4L2 Memory to Memory device
> + *
> + * Open the device node and prepare the two V4L2VideoDevice instances to handle
> + * their respective buffer queues.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2M2MDevice::open()
> +{
> +	int fd;
> +	int ret;
> +
> +	/*
> +	 * The output and capture V4L2VideoDevice instances use the same file
> +	 * handle for the same device node. The local file handle can be closed
> +	 * as the V4L2VideoDevice::open() retains a handle by duplicating the
> +	 * fd passed in.
> +	 */
> +	fd = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK);
> +	if (fd < 0) {
> +		ret = -errno;
> +		LOG(V4L2, Error)
> +			<< "Failed to open V4L2 M2M device: " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	ret = output_->open(fd, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> +	if (ret)
> +		goto err;
> +
> +	ret = capture_->open(fd, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +	if (ret)
> +		goto err;
> +
> +	::close(fd);
> +
> +	return 0;
> +
> +err:
> +	close();
> +	::close(fd);
> +
> +	return ret;
> +}
> +
> +/**
> + * \brief Close the memory-to-memory device, releasing any resources acquired by
> + * open()
> + */
> +void V4L2M2MDevice::close()
> +{
> +	capture_->close();
> +	output_->close();
> +}
> +

Minors apart,
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  } /* namespace libcamera */
> --
> 2.20.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190813/98121cd9/attachment.sig>


More information about the libcamera-devel mailing list