[libcamera-devel] [PATCH v2 2/2] libcamera: Introduce V4L2Device base class

Kieran Bingham kieran.bingham at ideasonboard.com
Wed Jun 12 15:50:44 CEST 2019


Hi Jacopo,

On 12/06/2019 14:38, Jacopo Mondi wrote:
> he V4L2 devices and subdevices share a few common operations,like

s/he/The/

> opening and closing a device node, and perform IOCTLs on the device.
> 
> With the forthcoming introduction of support for V4L2 controls, the
> quantity of shared code will drastically increase, as the control
> implementation is identical for the two.
> 
> To maximize code re-use and avoid duplications, provide a V4L2Device
> base class which groups the common operations and members.
> 
> The newly introduced base class provides methods to open/close a device
> node, access the file descriptor, and perform IOCTLs on the device.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


Thank you - this is much easier to review now :-D


A few comments below:

> ---
>  src/libcamera/include/v4l2_device.h      |  36 ++++++
>  src/libcamera/include/v4l2_subdevice.h   |   6 +-
>  src/libcamera/include/v4l2_videodevice.h |   5 +-
>  src/libcamera/meson.build                |   2 +
>  src/libcamera/v4l2_device.cpp            | 148 +++++++++++++++++++++++
>  src/libcamera/v4l2_subdevice.cpp         |  66 +++-------
>  src/libcamera/v4l2_videodevice.cpp       |  68 ++++-------
>  7 files changed, 227 insertions(+), 104 deletions(-)
>  create mode 100644 src/libcamera/include/v4l2_device.h
>  create mode 100644 src/libcamera/v4l2_device.cpp
> 
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> new file mode 100644
> index 000000000000..584a570a4703
> --- /dev/null
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * v4l2_device.h - Common base for V4L2 video devices and subdevices
> + */
> +#ifndef __LIBCAMERA_V4L2_DEVICE_H__
> +#define __LIBCAMERA_V4L2_DEVICE_H__
> +
> +#include <string>
> +
> +namespace libcamera {
> +
> +class V4L2Device
> +{
> +public:
> +	virtual ~V4L2Device() { close(); }
> +
> +protected:
> +	V4L2Device();
> +
> +	int fd() { return fd_; }
> +
> +	int open(const std::string &pathname, unsigned int flags);
> +	int close();
> +	bool isOpen() const;
> +
> +	int ioctl(unsigned long request, void *argp);
> +
> +private:
> +	int fd_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_V4L2_DEVICE_H__ */
> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> index 3e4e5107aebe..a34b719f8c52 100644
> --- a/src/libcamera/include/v4l2_subdevice.h
> +++ b/src/libcamera/include/v4l2_subdevice.h
> @@ -16,6 +16,7 @@
>  #include "formats.h"
>  #include "log.h"
>  #include "media_object.h"
> +#include "v4l2_device.h"
>  
>  namespace libcamera {
>  
> @@ -28,7 +29,7 @@ struct V4L2SubdeviceFormat {
>  	const std::string toString() const;
>  };
>  
> -class V4L2Subdevice : protected Loggable
> +class V4L2Subdevice : public V4L2Device, protected Loggable
>  {
>  public:
>  	explicit V4L2Subdevice(const MediaEntity *entity);
> @@ -37,8 +38,6 @@ public:
>  	~V4L2Subdevice();
>  
>  	int open();
> -	bool isOpen() const;
> -	void close();
>  
>  	const MediaEntity *entity() const { return entity_; }
>  
> @@ -64,7 +63,6 @@ private:
>  			 Rectangle *rect);
>  
>  	const MediaEntity *entity_;
> -	int fd_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index 6ecdb64e5f3c..15a83635b9bf 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -17,6 +17,7 @@
>  #include <libcamera/signal.h>
>  
>  #include "log.h"
> +#include "v4l2_device.h"
>  
>  namespace libcamera {
>  
> @@ -111,7 +112,7 @@ public:
>  	const std::string toString() const;
>  };
>  
> -class V4L2VideoDevice : protected Loggable
> +class V4L2VideoDevice : public V4L2Device, protected Loggable
>  {
>  public:
>  	explicit V4L2VideoDevice(const std::string &deviceNode);
> @@ -122,7 +123,6 @@ public:
>  	V4L2VideoDevice &operator=(const V4L2VideoDevice &) = delete;
>  
>  	int open();
> -	bool isOpen() const;
>  	void close();
>  
>  	const char *driverName() const { return caps_.driver(); }
> @@ -167,7 +167,6 @@ private:
>  	void bufferAvailable(EventNotifier *notifier);
>  
>  	std::string deviceNode_;
> -	int fd_;
>  	V4L2Capability caps_;
>  
>  	enum v4l2_buf_type bufferType_;
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 15ab53b1abbe..f26ad5b2dc57 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -23,6 +23,7 @@ libcamera_sources = files([
>      'stream.cpp',
>      'timer.cpp',
>      'utils.cpp',
> +    'v4l2_device.cpp',
>      'v4l2_subdevice.cpp',
>      'v4l2_videodevice.cpp',
>  ])
> @@ -41,6 +42,7 @@ libcamera_headers = files([
>      'include/media_object.h',
>      'include/pipeline_handler.h',
>      'include/utils.h',
> +    'include/v4l2_device.h',
>      'include/v4l2_subdevice.h',
>      'include/v4l2_videodevice.h',
>  ])
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> new file mode 100644
> index 000000000000..f89b2a2982ba
> --- /dev/null
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -0,0 +1,148 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * v4l2_device.cpp - Common base for V4L2 devices and subdevices
> + */
> +
> +#include "v4l2_device.h"
> +
> +#include <fcntl.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <unistd.h>
> +
> +#include "log.h"
> +
> +/**
> + * \file v4l2_device.h
> + * \brief Common base for V4L2 devices and subdevices
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(V4L2)
> +
> +/**
> + * \class V4L2Device
> + * \brief Base class for V4L2Videodev and V4L2Subdev
> + *
> + * The V4L2Device class groups together the methods and fields common to
> + * both V4L2Videodev and V4L2Subdev, and provide a base class which
> + * provides methods to open and close the device node associated with the
> + * device and to perform IOCTL system calls on it.
> + *
> + * The V4L2Device class cannot be instantiated directly, as its constructor
> + * is protected. Users should use instead one the derived classes to model
> + * either a V4L2 video device or a V4L2 subdevice.
> + */
> +
> +/**
> + * \brief Construct a V4L2Device
> + *
> + * The V4L2Device constructor is protected and can only be accessed by the
> + * V4L2Videodev and V4L2Subdev derived classes.
> + *
> + * Initialize the file descriptor to -1.
> + */
> +V4L2Device::V4L2Device()
> +	: fd_(-1)
> +{
> +}
> +
> +/**
> + * \fn V4L2Device::fd()
> + * \brief Retrieve the V4L2 device file descriptor
> + * \return The V4L2 device file descriptor, -1 if the device node is not open
> + */
> +
> +/**
> + * \brief Open a V4L2 device node
> + * \param pathname The filesystem path of the device node to open
> + * \param flags Access mode flags
> + *
> + * Initialize the file descriptor, which was initially set to -1.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2Device::open(const std::string &pathname, unsigned int flags)
> +{
> +	if (isOpen()) {
> +		LOG(V4L2, Error) << "Device already open";
> +		return -EBUSY;
> +	}
> +
> +	int ret = ::open(pathname.c_str(), flags);
> +	if (ret < 0) {
> +		ret = -errno;
> +		LOG(V4L2, Error) << "Failed to open V4L2 device: "
> +				 << strerror(-ret);
> +		return ret;
> +	}
> +
> +	fd_ = ret;
> +
> +	return 0;
> +}
> +
> +/**
> + * \brief Close the device node
> + *
> + * Reset the file descriptor to -1
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2Device::close()
> +{
> +	if (fd_ < 0)
> +		return 0;
> +
> +	int ret = ::close(fd_);
> +	if (ret < 0) {
> +		ret = -errno;
> +		LOG(V4L2, Error) << "Failed to close V4L2 device: "
> +				 << strerror(-ret);
> +		return ret;
> +	}
> +
> +	fd_ = -1;
> +
> +	return 0;
> +}
> +
> +/**
> + * \brief Check if the V4L2 device node is open
> + * \return True if the V4L2 device node is open, false otherwise
> + */
> +bool V4L2Device::isOpen() const
> +{
> +	return fd_ != -1;
> +}
> +
> +/**
> + * \brief Perform an IOCTL system call on the device node
> + * \param request The IOCTL request code
> + * \param argp A poiunter to the IOCTL argument


s/poiunter/pointer/


> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2Device::ioctl(unsigned long request, void *argp)
> +{
> +	/*
> +	 * Printing out an error message is usually better performed
> +	 * in the caller, which can provide more context.
> +	 */
> +	if (::ioctl(fd_, request, argp) < 0)
> +		return -errno;

I /really/ like this ... but note how many of the call sites for
V4L2Device::ioctl() still work with the -errno fuss, which should now
just deal directly with 'ret' where it's used.

'man ioctl' states:

RETURN VALUE
       Usually,  on  success zero is returned.  A few ioctl() requests
       use the return value as an output parameter and return a
       nonnegative value on success.  On error, -1 is returned, and
       errno is set appropriately.


I really like simplifying the -1, errno check pattern that is
everywhere. But do we need to be sure we'll never need the return code
of the ioctl?

(I don't think we do - and we /could/ have a different call for that if
necessary)



> +
> +	return 0;
> +}
> +
> +/**
> + * \var V4L2Device::fd_
> + * \brief The V4L2 device node file descriptor
> + *
> + * The file descriptor is initialized to -1 and reset to this value once
> + * the device node gets closed.
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 6681c1920065..b1e5f559b944 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -29,7 +29,7 @@
>  
>  namespace libcamera {
>  
> -LOG_DEFINE_CATEGORY(V4L2Subdev)
> +LOG_DECLARE_CATEGORY(V4L2)
>  
>  /**
>   * \struct V4L2SubdeviceFormat
> @@ -102,7 +102,7 @@ const std::string V4L2SubdeviceFormat::toString() const
>   * path
>   */
>  V4L2Subdevice::V4L2Subdevice(const MediaEntity *entity)
> -	: entity_(entity), fd_(-1)
> +	: V4L2Device(), entity_(entity)
>  {
>  }
>  
> @@ -117,45 +117,7 @@ V4L2Subdevice::~V4L2Subdevice()
>   */
>  int V4L2Subdevice::open()
>  {
> -	int ret;
> -
> -	if (isOpen()) {
> -		LOG(V4L2Subdev, Error) << "Device already open";
> -		return -EBUSY;
> -	}
> -
> -	ret = ::open(entity_->deviceNode().c_str(), O_RDWR);
> -	if (ret < 0) {
> -		ret = -errno;
> -		LOG(V4L2Subdev, Error)
> -			<< "Failed to open V4L2 subdevice '"
> -			<< entity_->deviceNode() << "': " << strerror(-ret);
> -		return ret;
> -	}
> -	fd_ = ret;
> -
> -	return 0;
> -}
> -
> -/**
> - * \brief Check if the subdevice is open
> - * \return True if the subdevice is open, false otherwise
> - */
> -bool V4L2Subdevice::isOpen() const
> -{
> -	return fd_ != -1;
> -}
> -
> -/**
> - * \brief Close the subdevice, releasing any resources acquired by open()
> - */
> -void V4L2Subdevice::close()
> -{
> -	if (!isOpen())
> -		return;
> -
> -	::close(fd_);
> -	fd_ = -1;
> +	return V4L2Device::open(entity_->deviceNode(), O_RDWR);
>  }
>  
>  /**
> @@ -207,7 +169,7 @@ FormatEnum V4L2Subdevice::formats(unsigned int pad)
>  	int ret;
>  
>  	if (pad >= entity_->pads().size()) {
> -		LOG(V4L2Subdev, Error) << "Invalid pad: " << pad;
> +		LOG(V4L2, Error) << "Invalid pad: " << pad;
>  		return formatMap;
>  	}
>  
> @@ -215,7 +177,7 @@ FormatEnum V4L2Subdevice::formats(unsigned int pad)
>  	mbusEnum.index = 0;
>  	mbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>  	while (true) {
> -		ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_MBUS_CODE, &mbusEnum);
> +		ret = ioctl(VIDIOC_SUBDEV_ENUM_MBUS_CODE, &mbusEnum);
>  		if (ret)
>  			break;
>  
> @@ -229,7 +191,7 @@ FormatEnum V4L2Subdevice::formats(unsigned int pad)
>  
>  	if (ret && (errno != EINVAL && errno != ENOTTY)) {
>  		ret = -errno;
> -		LOG(V4L2Subdev, Error)
> +		LOG(V4L2, Error)
>  			<< "Unable to enumerate formats on pad " << pad
>  			<< ": " << strerror(-ret);
>  		formatMap.clear();
> @@ -250,10 +212,10 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format)
>  	subdevFmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>  	subdevFmt.pad = pad;
>  
> -	int ret = ioctl(fd_, VIDIOC_SUBDEV_G_FMT, &subdevFmt);
> +	int ret = ioctl(VIDIOC_SUBDEV_G_FMT, &subdevFmt);
>  	if (ret) {
>  		ret = -errno;
> -		LOG(V4L2Subdev, Error)
> +		LOG(V4L2, Error)
>  			<< "Unable to get format on pad " << pad
>  			<< ": " << strerror(-ret);
>  		return ret;
> @@ -286,10 +248,10 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format)
>  	subdevFmt.format.height = format->size.height;
>  	subdevFmt.format.code = format->mbus_code;
>  
> -	int ret = ioctl(fd_, VIDIOC_SUBDEV_S_FMT, &subdevFmt);
> +	int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);
>  	if (ret) {
>  		ret = -errno;
> -		LOG(V4L2Subdev, Error)
> +		LOG(V4L2, Error)
>  			<< "Unable to set format on pad " << pad
>  			<< ": " << strerror(-ret);
>  		return ret;
> @@ -339,7 +301,7 @@ int V4L2Subdevice::enumPadSizes(unsigned int pad,unsigned int code,
>  	sizeEnum.code = code;
>  	sizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>  	while (true) {
> -		ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);
> +		ret = ioctl(VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);
>  		if (ret)
>  			break;
>  
> @@ -351,7 +313,7 @@ int V4L2Subdevice::enumPadSizes(unsigned int pad,unsigned int code,
>  
>  	if (ret && (errno != EINVAL && errno != ENOTTY)) {
>  		ret = -errno;
> -		LOG(V4L2Subdev, Error)
> +		LOG(V4L2, Error)
>  			<< "Unable to enumerate sizes on pad " << pad
>  			<< ": " << strerror(-ret);
>  		sizes->clear();
> @@ -377,10 +339,10 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
>  	sel.r.width = rect->w;
>  	sel.r.height = rect->h;
>  
> -	int ret = ioctl(fd_, VIDIOC_SUBDEV_S_SELECTION, &sel);
> +	int ret = ioctl(VIDIOC_SUBDEV_S_SELECTION, &sel);
>  	if (ret < 0) {
>  		ret = -errno;
> -		LOG(V4L2Subdev, Error)
> +		LOG(V4L2, Error)
>  			<< "Unable to set rectangle " << target << " on pad "
>  			<< pad << ": " << strerror(-ret);
>  		return ret;
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 8957cf8f97d3..14e711d5dde3 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -30,7 +30,7 @@
>   */
>  namespace libcamera {
>  
> -LOG_DEFINE_CATEGORY(V4L2)
> +LOG_DECLARE_CATEGORY(V4L2)
>  
>  /**
>   * \struct V4L2Capability
> @@ -270,7 +270,7 @@ const std::string V4L2DeviceFormat::toString() const
>   * \param[in] deviceNode The file-system path to the video device node
>   */
>  V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)
> -	: deviceNode_(deviceNode), fd_(-1), bufferPool_(nullptr),
> +	: V4L2Device(), deviceNode_(deviceNode), bufferPool_(nullptr),
>  	  queuedBuffersCount_(0), fdEvent_(nullptr)
>  {
>  	/*
> @@ -305,21 +305,11 @@ int V4L2VideoDevice::open()
>  {
>  	int ret;
>  
> -	if (isOpen()) {
> -		LOG(V4L2, Error) << "Device already open";
> -		return -EBUSY;
> -	}
> -
> -	ret = ::open(deviceNode_.c_str(), O_RDWR | O_NONBLOCK);
> -	if (ret < 0) {
> -		ret = -errno;
> -		LOG(V4L2, Error)
> -			<< "Failed to open V4L2 device: " << strerror(-ret);
> +	ret = V4L2Device::open(deviceNode_, O_RDWR | O_NONBLOCK);
> +	if (ret < 0)
>  		return ret;
> -	}
> -	fd_ = ret;
>  
> -	ret = ioctl(fd_, VIDIOC_QUERYCAP, &caps_);
> +	ret = ioctl(VIDIOC_QUERYCAP, &caps_);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2, Error)
> @@ -342,20 +332,20 @@ int V4L2VideoDevice::open()
>  	 * (POLLIN), and write notifications for OUTPUT devices (POLLOUT).
>  	 */
>  	if (caps_.isVideoCapture()) {
> -		fdEvent_ = new EventNotifier(fd_, EventNotifier::Read);
> +		fdEvent_ = new EventNotifier(fd(), EventNotifier::Read);
>  		bufferType_ = caps_.isMultiplanar()
>  			    ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
>  			    : V4L2_BUF_TYPE_VIDEO_CAPTURE;
>  	} else if (caps_.isVideoOutput()) {
> -		fdEvent_ = new EventNotifier(fd_, EventNotifier::Write);
> +		fdEvent_ = new EventNotifier(fd(), EventNotifier::Write);
>  		bufferType_ = caps_.isMultiplanar()
>  			    ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE
>  			    : V4L2_BUF_TYPE_VIDEO_OUTPUT;
>  	} else if (caps_.isMetaCapture()) {
> -		fdEvent_ = new EventNotifier(fd_, EventNotifier::Read);
> +		fdEvent_ = new EventNotifier(fd(), EventNotifier::Read);
>  		bufferType_ = V4L2_BUF_TYPE_META_CAPTURE;
>  	} else if (caps_.isMetaOutput()) {
> -		fdEvent_ = new EventNotifier(fd_, EventNotifier::Write);
> +		fdEvent_ = new EventNotifier(fd(), EventNotifier::Write);
>  		bufferType_ = V4L2_BUF_TYPE_META_OUTPUT;
>  	} else {
>  		LOG(V4L2, Error) << "Device is not a supported type";
> @@ -368,28 +358,16 @@ int V4L2VideoDevice::open()
>  	return 0;
>  }
>  
> -/**
> - * \brief Check if device is successfully opened
> - * \return True if the device is open, false otherwise
> - */
> -bool V4L2VideoDevice::isOpen() const
> -{
> -	return fd_ != -1;
> -}
> -
>  /**
>   * \brief Close the device, releasing any resources acquired by open()
>   */
>  void V4L2VideoDevice::close()
>  {
> -	if (fd_ < 0)
> +	if (fd() < 0)
>  		return;
>  
>  	releaseBuffers();
>  	delete fdEvent_;
> -
> -	::close(fd_);
> -	fd_ = -1;
>  }
>  
>  /**
> @@ -462,7 +440,7 @@ int V4L2VideoDevice::getFormatMeta(V4L2DeviceFormat *format)
>  	int ret;
>  
>  	v4l2Format.type = bufferType_;
> -	ret = ioctl(fd_, VIDIOC_G_FMT, &v4l2Format);
> +	ret = ioctl(VIDIOC_G_FMT, &v4l2Format);


I really like that ioctl now knows about the FD internally and thus is
simpler :)

"do this action with this structure..."


>  	if (ret) {
>  		ret = -errno;
>  		LOG(V4L2, Error) << "Unable to get format: " << strerror(-ret);

This is an example of things that I think should now be:

ret = ioctl(VIDIOC_G_FMT, &v4l2Format);
if (ret) {
	LOG(V4L2, Error) << "Unable to get format: " << strerror(ret);
	...
}

That probably applies to all uses of V4L2Device::ioctl()


> @@ -488,7 +466,7 @@ int V4L2VideoDevice::setFormatMeta(V4L2DeviceFormat *format)
>  	v4l2Format.type = bufferType_;
>  	pix->dataformat = format->fourcc;
>  	pix->buffersize = format->planes[0].size;
> -	ret = ioctl(fd_, VIDIOC_S_FMT, &v4l2Format);
> +	ret = ioctl(VIDIOC_S_FMT, &v4l2Format);
>  	if (ret) {
>  		ret = -errno;
>  		LOG(V4L2, Error) << "Unable to set format: " << strerror(-ret);
> @@ -516,7 +494,7 @@ int V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)
>  	int ret;
>  
>  	v4l2Format.type = bufferType_;
> -	ret = ioctl(fd_, VIDIOC_G_FMT, &v4l2Format);
> +	ret = ioctl(VIDIOC_G_FMT, &v4l2Format);
>  	if (ret) {
>  		ret = -errno;
>  		LOG(V4L2, Error) << "Unable to get format: " << strerror(-ret);
> @@ -554,7 +532,7 @@ int V4L2VideoDevice::setFormatMultiplane(V4L2DeviceFormat *format)
>  		pix->plane_fmt[i].sizeimage = format->planes[i].size;
>  	}
>  
> -	ret = ioctl(fd_, VIDIOC_S_FMT, &v4l2Format);
> +	ret = ioctl(VIDIOC_S_FMT, &v4l2Format);
>  	if (ret) {
>  		ret = -errno;
>  		LOG(V4L2, Error) << "Unable to set format: " << strerror(-ret);
> @@ -584,7 +562,7 @@ int V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)
>  	int ret;
>  
>  	v4l2Format.type = bufferType_;
> -	ret = ioctl(fd_, VIDIOC_G_FMT, &v4l2Format);
> +	ret = ioctl(VIDIOC_G_FMT, &v4l2Format);
>  	if (ret) {
>  		ret = -errno;
>  		LOG(V4L2, Error) << "Unable to get format: " << strerror(-ret);
> @@ -613,7 +591,7 @@ int V4L2VideoDevice::setFormatSingleplane(V4L2DeviceFormat *format)
>  	pix->pixelformat = format->fourcc;
>  	pix->bytesperline = format->planes[0].bpl;
>  	pix->field = V4L2_FIELD_NONE;
> -	ret = ioctl(fd_, VIDIOC_S_FMT, &v4l2Format);
> +	ret = ioctl(VIDIOC_S_FMT, &v4l2Format);
>  	if (ret) {
>  		ret = -errno;
>  		LOG(V4L2, Error) << "Unable to set format: " << strerror(-ret);
> @@ -643,7 +621,7 @@ int V4L2VideoDevice::requestBuffers(unsigned int count)
>  	rb.type = bufferType_;
>  	rb.memory = memoryType_;
>  
> -	ret = ioctl(fd_, VIDIOC_REQBUFS, &rb);
> +	ret = ioctl(VIDIOC_REQBUFS, &rb);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2, Error)
> @@ -695,7 +673,7 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
>  		buf.length = VIDEO_MAX_PLANES;
>  		buf.m.planes = planes;
>  
> -		ret = ioctl(fd_, VIDIOC_QUERYBUF, &buf);
> +		ret = ioctl(VIDIOC_QUERYBUF, &buf);
>  		if (ret < 0) {
>  			ret = -errno;
>  			LOG(V4L2, Error)
> @@ -748,7 +726,7 @@ int V4L2VideoDevice::createPlane(Buffer *buffer, unsigned int planeIndex,
>  	expbuf.plane = planeIndex;
>  	expbuf.flags = O_RDWR;
>  
> -	ret = ioctl(fd_, VIDIOC_EXPBUF, &expbuf);
> +	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2, Error)
> @@ -855,7 +833,7 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)
>  
>  	LOG(V4L2, Debug) << "Queueing buffer " << buf.index;
>  
> -	ret = ioctl(fd_, VIDIOC_QBUF, &buf);
> +	ret = ioctl(VIDIOC_QBUF, &buf);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2, Error)
> @@ -892,7 +870,7 @@ Buffer *V4L2VideoDevice::dequeueBuffer()
>  		buf.m.planes = planes;
>  	}
>  
> -	ret = ioctl(fd_, VIDIOC_DQBUF, &buf);
> +	ret = ioctl(VIDIOC_DQBUF, &buf);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2, Error)
> @@ -952,7 +930,7 @@ int V4L2VideoDevice::streamOn()
>  {
>  	int ret;
>  
> -	ret = ioctl(fd_, VIDIOC_STREAMON, &bufferType_);
> +	ret = ioctl(VIDIOC_STREAMON, &bufferType_);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2, Error)
> @@ -975,7 +953,7 @@ int V4L2VideoDevice::streamOff()
>  {
>  	int ret;
>  
> -	ret = ioctl(fd_, VIDIOC_STREAMOFF, &bufferType_);
> +	ret = ioctl(VIDIOC_STREAMOFF, &bufferType_);
>  	if (ret < 0) {
>  		ret = -errno;
>  		LOG(V4L2, Error)
> 

-- 
Regards
--
Kieran


More information about the libcamera-devel mailing list