[libcamera-devel] [PATCH v2 2/2] libcamera: V4L2Control: remove V4L2Control classes
Hirokazu Honda
hiroh at chromium.org
Thu May 20 11:03:30 CEST 2021
Gentle ping for review, or could the patch series be merged?
On Mon, May 10, 2021 at 2:42 PM Hirokazu Honda <hiroh at chromium.org> wrote:
> V4L2ControlId and V4L2ControlInfo are just convenience classes to
> create ControlId and ControlInfo from v4l2_query_ext_control.
> Therefore, there is no need of being a class. It is used only
> from V4L2Device. This removes the classes and put the equivalent
> functions of creating ControlId and ControlInfo in
> v4l2_device.cpp.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/internal/meson.build | 1 -
> include/libcamera/internal/v4l2_controls.h | 31 -----
> include/libcamera/internal/v4l2_device.h | 4 +-
> src/libcamera/meson.build | 1 -
> src/libcamera/v4l2_controls.cpp | 151 ---------------------
> src/libcamera/v4l2_device.cpp | 72 +++++++++-
> 6 files changed, 71 insertions(+), 189 deletions(-)
> delete mode 100644 include/libcamera/internal/v4l2_controls.h
> delete mode 100644 src/libcamera/v4l2_controls.cpp
>
> diff --git a/include/libcamera/internal/meson.build
> b/include/libcamera/internal/meson.build
> index 1fe3918c..3fb0216d 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -42,7 +42,6 @@ libcamera_internal_headers = files([
> 'thread.h',
> 'timer.h',
> 'utils.h',
> - 'v4l2_controls.h',
> 'v4l2_device.h',
> 'v4l2_pixelformat.h',
> 'v4l2_subdevice.h',
> diff --git a/include/libcamera/internal/v4l2_controls.h
> b/include/libcamera/internal/v4l2_controls.h
> deleted file mode 100644
> index 0851b8dd..00000000
> --- a/include/libcamera/internal/v4l2_controls.h
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * v4l2_controls.h - V4L2 Controls Support
> - */
> -
> -#ifndef __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__
> -#define __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__
> -
> -#include <linux/videodev2.h>
> -
> -#include <libcamera/controls.h>
> -
> -namespace libcamera {
> -
> -class V4L2ControlId : public ControlId
> -{
> -public:
> - V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl);
> -};
> -
> -class V4L2ControlInfo : public ControlInfo
> -{
> -public:
> - V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
> -};
> -
> -} /* namespace libcamera */
> -
> -#endif /* __LIBCAMERA_INTERNAL_V4L2_CONTROLS_H__ */
> diff --git a/include/libcamera/internal/v4l2_device.h
> b/include/libcamera/internal/v4l2_device.h
> index 087f07e7..116bbbaf 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -13,11 +13,11 @@
>
> #include <linux/videodev2.h>
>
> +#include <libcamera/controls.h>
> #include <libcamera/signal.h>
> #include <libcamera/span.h>
>
> #include "libcamera/internal/log.h"
> -#include "libcamera/internal/v4l2_controls.h"
>
> namespace libcamera {
>
> @@ -61,7 +61,7 @@ private:
> void eventAvailable(EventNotifier *notifier);
>
> std::map<unsigned int, struct v4l2_query_ext_ctrl> controlInfo_;
> - std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
> + std::vector<std::unique_ptr<ControlId>> controlIds_;
> ControlInfoMap controls_;
> std::string deviceNode_;
> int fd_;
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index e0a48aa2..62251a64 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -51,7 +51,6 @@ libcamera_sources = files([
> 'timer.cpp',
> 'transform.cpp',
> 'utils.cpp',
> - 'v4l2_controls.cpp',
> 'v4l2_device.cpp',
> 'v4l2_pixelformat.cpp',
> 'v4l2_subdevice.cpp',
> diff --git a/src/libcamera/v4l2_controls.cpp
> b/src/libcamera/v4l2_controls.cpp
> deleted file mode 100644
> index 3f8ec6ca..00000000
> --- a/src/libcamera/v4l2_controls.cpp
> +++ /dev/null
> @@ -1,151 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2019, Google Inc.
> - *
> - * v4l2_controls.cpp - V4L2 Controls Support
> - */
> -
> -#include "libcamera/internal/v4l2_controls.h"
> -
> -#include <string.h>
> -
> -/**
> - * \file v4l2_controls.h
> - * \brief Support for V4L2 Controls using the V4L2 Extended Controls APIs
> - *
> - * The V4L2 Control API allows application to inspect and modify sets of
> - * configurable parameters on a video device or subdevice. The nature of
> the
> - * parameters an application can modify using the control framework
> depends on
> - * what the driver implements support for, and on the characteristics of
> the
> - * underlying hardware platform. Generally controls are used to modify
> user
> - * visible settings, such as the image brightness and exposure time, or
> - * non-standard parameters which cannot be controlled through the V4L2
> format
> - * negotiation API.
> - *
> - * Controls are identified by a numerical ID, defined by the V4L2 kernel
> headers
> - * and have an associated type. Each control has a value, which is the
> data that
> - * can be modified with V4L2Device::setControls() or retrieved with
> - * V4L2Device::getControls().
> - *
> - * The control's type along with the control's flags define the type of
> the
> - * control's value content. Controls can transport a single data value
> stored in
> - * variable inside the control, or they might as well deal with more
> complex
> - * data types, such as arrays of matrices, stored in a contiguous memory
> - * locations associated with the control and called 'the payload'. Such
> controls
> - * are called 'compound controls' and are currently not supported by the
> - * libcamera V4L2 control framework.
> - *
> - * libcamera implements support for controls using the V4L2 Extended
> Control
> - * API, which allows future handling of controls with payloads of
> arbitrary
> - * sizes.
> - *
> - * The libcamera V4L2 Controls framework operates on lists of controls,
> wrapped
> - * by the ControlList class, to match the V4L2 extended controls API. The
> - * interface to set and get control is implemented by the V4L2Device
> class, and
> - * this file only provides the data type definitions.
> - *
> - * \todo Add support for compound controls
> - */
> -
> -namespace libcamera {
> -
> -namespace {
> -
> -std::string v4l2_ctrl_name(const struct v4l2_query_ext_ctrl &ctrl)
> -{
> - size_t len = strnlen(ctrl.name, sizeof(ctrl.name));
> - return std::string(static_cast<const char *>(ctrl.name), len);
> -}
> -
> -ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
> -{
> - switch (ctrl.type) {
> - case V4L2_CTRL_TYPE_U8:
> - return ControlTypeByte;
> -
> - case V4L2_CTRL_TYPE_BOOLEAN:
> - return ControlTypeBool;
> -
> - case V4L2_CTRL_TYPE_INTEGER:
> - return ControlTypeInteger32;
> -
> - case V4L2_CTRL_TYPE_INTEGER64:
> - return ControlTypeInteger64;
> -
> - case V4L2_CTRL_TYPE_MENU:
> - case V4L2_CTRL_TYPE_BUTTON:
> - case V4L2_CTRL_TYPE_BITMASK:
> - case V4L2_CTRL_TYPE_INTEGER_MENU:
> - /*
> - * More precise types may be needed, for now use a 32-bit
> - * integer type.
> - */
> - return ControlTypeInteger32;
> -
> - default:
> - return ControlTypeNone;
> - }
> -}
> -
> -} /* namespace */
> -
> -/**
> - * \class V4L2ControlId
> - * \brief V4L2 control static metadata
> - *
> - * The V4L2ControlId class is a specialisation of the ControlId for V4L2
> - * controls.
> - */
> -
> -/**
> - * \brief Construct a V4L2ControlId from a struct v4l2_query_ext_ctrl
> - * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the
> kernel
> - */
> -V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl)
> - : ControlId(ctrl.id, v4l2_ctrl_name(ctrl), v4l2_ctrl_type(ctrl))
> -{
> -}
> -
> -/**
> - * \class V4L2ControlInfo
> - * \brief Convenience specialisation of ControlInfo for V4L2 controls
> - *
> - * The V4L2ControlInfo class is a specialisation of the ControlInfo for
> V4L2
> - * controls. It offers a convenience constructor from a struct
> - * v4l2_query_ext_ctrl, and is otherwise equivalent to the ControlInfo
> class.
> - */
> -
> -/**
> - * \brief Construct a V4L2ControlInfo from a struct v4l2_query_ext_ctrl
> - * \param[in] ctrl The struct v4l2_query_ext_ctrl as returned by the
> kernel
> - */
> -V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> -{
> - switch (ctrl.type) {
> - case V4L2_CTRL_TYPE_U8:
> -
> ControlInfo::operator=(ControlInfo(static_cast<uint8_t>(ctrl.minimum),
> -
> static_cast<uint8_t>(ctrl.maximum),
> -
> static_cast<uint8_t>(ctrl.default_value)));
> - break;
> -
> - case V4L2_CTRL_TYPE_BOOLEAN:
> -
> ControlInfo::operator=(ControlInfo(static_cast<bool>(ctrl.minimum),
> -
> static_cast<bool>(ctrl.maximum),
> -
> static_cast<bool>(ctrl.default_value)));
> - break;
> -
> - case V4L2_CTRL_TYPE_INTEGER64:
> -
> ControlInfo::operator=(ControlInfo(static_cast<int64_t>(ctrl.minimum),
> -
> static_cast<int64_t>(ctrl.maximum),
> -
> static_cast<int64_t>(ctrl.default_value)));
> - break;
> -
> - default:
> -
> ControlInfo::operator=(ControlInfo(static_cast<int32_t>(ctrl.minimum),
> -
> static_cast<int32_t>(ctrl.maximum),
> -
> static_cast<int32_t>(ctrl.default_value)));
> - break;
> - }
> -}
> -
> -} /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 397029ac..f9414043 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -20,7 +20,6 @@
> #include "libcamera/internal/log.h"
> #include "libcamera/internal/sysfs.h"
> #include "libcamera/internal/utils.h"
> -#include "libcamera/internal/v4l2_controls.h"
>
> /**
> * \file v4l2_device.h
> @@ -31,6 +30,73 @@ namespace libcamera {
>
> LOG_DEFINE_CATEGORY(V4L2)
>
> +namespace {
> +
> +ControlType v4l2CtrlType(uint32_t ctrlType)
> +{
> + switch (ctrlType) {
> + case V4L2_CTRL_TYPE_U8:
> + return ControlTypeByte;
> +
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + return ControlTypeBool;
> +
> + case V4L2_CTRL_TYPE_INTEGER:
> + return ControlTypeInteger32;
> +
> + case V4L2_CTRL_TYPE_INTEGER64:
> + return ControlTypeInteger64;
> +
> + case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_BUTTON:
> + case V4L2_CTRL_TYPE_BITMASK:
> + case V4L2_CTRL_TYPE_INTEGER_MENU:
> + /*
> + * More precise types may be needed, for now use a 32-bit
> + * integer type.
> + */
> + return ControlTypeInteger32;
> +
> + default:
> + return ControlTypeNone;
> + }
> +}
> +
> +std::unique_ptr<ControlId> v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)
> +{
> + const size_t len = strnlen(ctrl.name, sizeof(ctrl.name));
> + const std::string name(static_cast<const char *>(ctrl.name), len);
> + const ControlType type = v4l2CtrlType(ctrl.type);
> +
> + return std::make_unique<ControlId>(ctrl.id, name, type);
> +}
> +
> +ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)
> +{
> + switch (ctrl.type) {
> + case V4L2_CTRL_TYPE_U8:
> + return ControlInfo(static_cast<uint8_t>(ctrl.minimum),
> + static_cast<uint8_t>(ctrl.maximum),
> +
> static_cast<uint8_t>(ctrl.default_value));
> +
> + case V4L2_CTRL_TYPE_BOOLEAN:
> + return ControlInfo(static_cast<bool>(ctrl.minimum),
> + static_cast<bool>(ctrl.maximum),
> + static_cast<bool>(ctrl.default_value));
> +
> + case V4L2_CTRL_TYPE_INTEGER64:
> + return ControlInfo(static_cast<int64_t>(ctrl.minimum),
> + static_cast<int64_t>(ctrl.maximum),
> +
> static_cast<int64_t>(ctrl.default_value));
> +
> + default:
> + return ControlInfo(static_cast<int32_t>(ctrl.minimum),
> + static_cast<int32_t>(ctrl.maximum),
> +
> static_cast<int32_t>(ctrl.default_value));
> + }
> +}
> +} /* namespace */
> +
> /**
> * \class V4L2Device
> * \brief Base class for V4L2VideoDevice and V4L2Subdevice
> @@ -502,10 +568,10 @@ void V4L2Device::listControls()
> continue;
> }
>
> -
> controlIds_.emplace_back(std::make_unique<V4L2ControlId>(ctrl));
> + controlIds_.emplace_back(v4l2ControlId(ctrl));
> controlInfo_.emplace(ctrl.id, ctrl);
>
> - ctrls.emplace(controlIds_.back().get(),
> V4L2ControlInfo(ctrl));
> + ctrls.emplace(controlIds_.back().get(),
> v4l2ControlInfo(ctrl));
> }
>
> controls_ = std::move(ctrls);
> --
> 2.31.1.607.g51e8a6a459-goog
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210520/95edb16e/attachment-0001.htm>
More information about the libcamera-devel
mailing list