[libcamera-devel] [PATCH] libcamera: v4l2_device: Group base operations in V4L2Base
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Jan 25 12:18:36 CET 2019
Hi Jacopo,
Thank you for the patch.
On Thu, Jan 24, 2019 at 04:31:35PM +0100, Jacopo Mondi wrote:
> Group operations commont to V4L2Device and the forthcoming V4L2Subdevice
> in a common V4L2Base class.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> As I started implementing V4L2Subdevice support, I realized I was going to
> re-implement what is now part of V4L2Base.
As I mentioned before, I think the amount of code currently in your
V4L2Base class isn't worth it. It would be a different story if we had
more code to share, it would then be easier to convince me :-)
Furthemore, the code in V4L2Base isn't V4L2-specific, so V4L2Base isn't
a proper name in my opinion and I don't think we want to create a custom
File class just to encapsulate open and close.
> I would like to develop V4L2Subdevice on top of this.
> Patch to be applied on top of my latest:
> [PATCH v2 0/2] Add mplane support to V4L2 device
>
> ---
> src/libcamera/include/v4l2_device.h | 38 ++++++---
> src/libcamera/v4l2_device.cpp | 127 ++++++++++++++++++----------
> 2 files changed, 112 insertions(+), 53 deletions(-)
>
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index 2da30d1..08bfe9f 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -54,30 +54,48 @@ struct V4L2Capability final : v4l2_capability {
> };
>
> class MediaEntity;
> -class V4L2Device
> +class V4L2Base
> {
> public:
> - explicit V4L2Device(const std::string &deviceNode);
> - explicit V4L2Device(const MediaEntity &entity);
> + virtual ~V4L2Base() { close(); }
> +
> + virtual int open();
> + bool isOpen() const { return fd_ != -1; }
> + virtual void close();
> +
> +protected:
> + V4L2Base(const std::string &deviceNode);
> + V4L2Base(const MediaEntity &entity);
> + V4L2Base(const V4L2Base &) = delete;
> + bool operator=(const V4L2Base &) = delete;
> +
> + std::string deviceNode_;
> + int fd_;
> +};
> +
> +class V4L2Device : public V4L2Base
> +{
> +public:
> + explicit V4L2Device(const std::string &deviceNode)
> + : V4L2Base(deviceNode) {}
> + explicit V4L2Device(const MediaEntity &entity)
> + : V4L2Base(entity) {}
> V4L2Device(const V4L2Device &) = delete;
> - ~V4L2Device();
>
> - void operator=(const V4L2Device &) = delete;
> + ~V4L2Device() {}
>
> - int open();
> - bool isOpen() const;
> - void close();
> + void operator=(const V4L2Device &) = delete;
>
> const char *driverName() const { return caps_.driver(); }
> const char *deviceName() const { return caps_.card(); }
> const char *busName() const { return caps_.bus_info(); }
>
> + int open();
> +
> int format();
> int setFormat();
>
> private:
> - std::string deviceNode_;
> - int fd_;
> V4L2Capability caps_;
>
> int getFormatSingleplane();
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 2ddb800..f505240 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -81,16 +81,15 @@ LOG_DEFINE_CATEGORY(V4L2)
> */
>
> /**
> - * \class V4L2Device
> - * \brief V4L2Device object and API
> + * \class V4L2Base
> + * \brief Group methods and field common to V4L2 devices and subdevices
> *
> - * The V4L2 Device API class models an instance of a V4L2 device node.
> - * It is constructed with the path to a V4L2 video device node. The device node
> - * is only opened upon a call to open() which must be checked for success.
> + * Represents the base class for V4L2 devices and sub-devices.
> + * It is constructed with the path to a V4L2 video (sub-)device node or
> + * with a MediaEntity, provided its deviceNode() path has been set.
> *
> - * The device capabilities are validated when the device is opened and the
> - * device is rejected if it is not a suitable V4L2 capture or output device, or
> - * if the device does not support streaming I/O.
> + * The device node is only opened upon a call to open() which must be
> + * checked for success.
> *
> * No API call other than open(), isOpen() and close() shall be called on an
> * unopened device instance.
> @@ -100,35 +99,32 @@ LOG_DEFINE_CATEGORY(V4L2)
> */
>
> /**
> - * \brief Construct a V4L2Device
> - * \param deviceNode The file-system path to the video device node
> + * \fn V4L2Base::V4L2Base(const std::string &deviceNode)
> + * \brief Construct a V4L2 base instance
> + * \param deviceNode The file-system path to the video (sub-)device node
> */
> -V4L2Device::V4L2Device(const std::string &deviceNode)
> +V4L2Base::V4L2Base(const std::string &deviceNode)
> : deviceNode_(deviceNode), fd_(-1)
> {
> }
>
> /**
> - * \brief Construct a V4L2Device from a MediaEntity
> + * \fn V4L2Base::V4L2Base(const MediaEntity &entity)
> + * \brief Construct a V4L2 base instance from a MediaEntity
> * \param entity The MediaEntity to build the device from
> *
> - * Construct a V4L2Device from a MediaEntity's device node path.
> + * Construct a V4L2 base instance from a MediaEntity's device node path.
> */
> -V4L2Device::V4L2Device(const MediaEntity &entity)
> - : V4L2Device(entity.deviceNode())
> -{
> -}
> -
> -V4L2Device::~V4L2Device()
> +V4L2Base::V4L2Base(const MediaEntity &entity)
> + : V4L2Base(entity.deviceNode())
> {
> - close();
> }
>
> /**
> - * \brief Open a V4L2 device and query its capabilities
> + * \brief Open a device node associated with a V4L2 device or subdevice
> * \return 0 on success, or a negative error code otherwise
> */
> -int V4L2Device::open()
> +int V4L2Base::open()
> {
> int ret;
>
> @@ -147,6 +143,72 @@ int V4L2Device::open()
> }
> fd_ = ret;
>
> + return 0;
> +}
> +
> +/**
> + * \fn V4L2Base::isOpen()
> + * \brief Check if device is successfully opened
> + * \return True if the device is open, false otherwise
> + */
> +
> +/**
> + * \brief Close the device node, releasing any resources acquired by open()
> + */
> +void V4L2Base::close()
> +{
> + if (fd_ < 0)
> + return;
> +
> + ::close(fd_);
> + fd_ = -1;
> +}
> +
> +/**
> + * \var V4L2Base::deviceNode_
> + * \brief The device node path associated with the V4L2 instance.
> + */
> +
> +/**
> + * \var V4L2Base::fd_
> + * \brief The file descriptor associated with an open device node
> + */
> +
> +/**
> + * \class V4L2Device
> + * \brief V4L2Device object and API
> + *
> + * The V4L2 Device API class models an instance of a V4L2 device node.
> + *
> + * The device capabilities are validated when the device is opened and the
> + * device is rejected if it is not a suitable V4L2 capture or output device, or
> + * if the device does not support streaming I/O.
> + */
> +
> +/**
> + * \fn V4L2Device::V4L2Device(const std::string &deviceNode)
> + * \brief Construct a V4L2Device
> + * \param deviceNode The file-system path to the video (sub)device node
> + */
> +
> +/**
> + * \fn V4L2Device::V4L2Device(const MediaEntity &entity)
> + * \brief Construct a V4L2Device from a MediaEntity
> + * \param entity The MediaEntity to build the device from
> + *
> + * Construct a V4L2Device from a MediaEntity's device node path.
> + */
> +
> +/**
> + * \brief Open a V4L2 device and query its capabilities
> + * \return 0 on success, or a negative error code otherwise
> + */
> +int V4L2Device::open()
> +{
> + int ret = V4L2Base::open();
> + if (ret)
> + return ret;
> +
> ret = ioctl(fd_, VIDIOC_QUERYCAP, &caps_);
> if (ret < 0) {
> ret = -errno;
> @@ -174,27 +236,6 @@ int V4L2Device::open()
> return 0;
> }
>
> -/**
> - * \brief Check if device is successfully opened
> - * \return True if the device is open, false otherwise
> - */
> -bool V4L2Device::isOpen() const
> -{
> - return fd_ != -1;
> -}
> -
> -/**
> - * \brief Close the device, releasing any resources acquired by open()
> - */
> -void V4L2Device::close()
> -{
> - if (fd_ < 0)
> - return;
> -
> - ::close(fd_);
> - fd_ = -1;
> -}
> -
> /**
> * \fn const char *V4L2Device::driverName()
> * \brief Retrieve the name of the V4L2 device driver
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list