[libcamera-devel] [PATCH v2 04/13] libcamera: controls: Improve the API towards applications
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Oct 3 21:20:22 CEST 2019
Hi Laurent,
Thanks for your work.
On 2019-09-29 22:02:45 +0300, Laurent Pinchart wrote:
> Rework the control-related classes to improve the API towards
> applications. The goal is to enable writing code similar to
>
> Request *req = ...;
> ControlList &controls = req->controls();
> controls->set(controls::AwbEnable, false);
> controls->set(controls::ManualExposure, 1000);
>
> ...
>
> int32_t exposure = controls->get(controls::ManualExposure);
>
> with the get and set operations ensuring type safety for the control
> values. This is achieved by creating the following classes:
>
> - Control defines controls and is the main way to reference a control.
> It is a template class to allow methods using it to refer to the
> control type.
>
> - ControlId is the base class of Control. It stores the control ID, name
> and type, and can be used in contexts where a control needs to be
> referenced regardless of its type (for instance in lists of controls).
> This class replaces ControlIdentifier.
>
> - ControlValue is kept as-is.
>
> The ControlList class now exposes two template get() and set() methods
> that replace the operator[]. They ensure type safety by infering the
> value type from the Control reference that they receive.
>
> The main way to refer to a control is now through the Control class, and
> optionally through its base ControlId class. The ControlId enumeration
> is removed, replaced by a list of global Control instances. Numerical
> control IDs are turned into macros, and are still exposed as they are
> required to communicate with IPAs (especially to deserialise control
> lists). They should however not be used by applications.
>
> Auto-generation of header and source files is removed for now to keep
> the change simple. It will be added back in the future in a more
> elaborate form.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> Changes since v1:
>
> - Reword comments
> - Remove libcamera::controls::None
> - Replace macros with an enum for control numerical IDs
> ---
> include/libcamera/control_ids.h | 44 ++--
> include/libcamera/controls.h | 108 +++++++---
> src/libcamera/control_ids.cpp | 52 +++++
> src/libcamera/controls.cpp | 318 ++++++++++++++--------------
> src/libcamera/gen-controls.awk | 106 ----------
> src/libcamera/meson.build | 11 +-
> src/libcamera/pipeline/uvcvideo.cpp | 40 ++--
> src/libcamera/pipeline/vimc.cpp | 28 +--
> test/controls/control_info.cpp | 25 +--
> test/controls/control_list.cpp | 66 +++---
> 10 files changed, 372 insertions(+), 426 deletions(-)
> create mode 100644 src/libcamera/control_ids.cpp
> delete mode 100755 src/libcamera/gen-controls.awk
>
> diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h
> index 75b6a2d5cafe..54235f1aea95 100644
> --- a/include/libcamera/control_ids.h
> +++ b/include/libcamera/control_ids.h
> @@ -8,34 +8,32 @@
> #ifndef __LIBCAMERA_CONTROL_IDS_H__
> #define __LIBCAMERA_CONTROL_IDS_H__
>
> -#include <functional>
> +#include <stdint.h>
> +
> +#include <libcamera/controls.h>
>
> namespace libcamera {
>
> -enum ControlId {
> - AwbEnable,
> - Brightness,
> - Contrast,
> - Saturation,
> - ManualExposure,
> - ManualGain,
> +namespace controls {
> +
> +enum {
> + AWB_ENABLE = 1,
> + BRIGHTNESS = 2,
> + CONTRAST = 3,
> + SATURATION = 4,
> + MANUAL_EXPOSURE = 5,
> + MANUAL_GAIN = 6,
> };
>
> +extern const Control<bool> AwbEnable;
> +extern const Control<int32_t> Brightness;
> +extern const Control<int32_t> Contrast;
> +extern const Control<int32_t> Saturation;
> +extern const Control<int32_t> ManualExposure;
> +extern const Control<int32_t> ManualGain;
> +
> +} /* namespace controls */
> +
> } /* namespace libcamera */
>
> -namespace std {
> -
> -template<>
> -struct hash<libcamera::ControlId> {
> - using argument_type = libcamera::ControlId;
> - using result_type = std::size_t;
> -
> - result_type operator()(const argument_type &key) const noexcept
> - {
> - return std::hash<std::underlying_type<argument_type>::type>()(key);
> - }
> -};
> -
> -} /* namespace std */
> -
> #endif // __LIBCAMERA_CONTROL_IDS_H__
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index a3089064c4b5..9698bd1dd383 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -8,12 +8,9 @@
> #ifndef __LIBCAMERA_CONTROLS_H__
> #define __LIBCAMERA_CONTROLS_H__
>
> -#include <stdint.h>
> #include <string>
> #include <unordered_map>
>
> -#include <libcamera/control_ids.h>
> -
> namespace libcamera {
>
> class Camera;
> @@ -53,55 +50,75 @@ private:
> };
> };
>
> -struct ControlIdentifier {
> - ControlId id;
> - const char *name;
> - ControlType type;
> +class ControlId
> +{
> +public:
> + unsigned int id() const { return id_; }
> + const char *name() const { return name_; }
> + ControlType type() const { return type_; }
> +
> +protected:
> + ControlId(unsigned int id, const char *name, ControlType type)
> + : id_(id), name_(name), type_(type)
> + {
> + }
> +
> +private:
> + ControlId(const ControlId &) = delete;
> + ControlId &operator=(const ControlId &) = delete;
> +
> + unsigned int id_;
> + const char *name_;
> + ControlType type_;
> +};
> +
> +static inline bool operator==(const ControlId &lhs, const ControlId &rhs)
> +{
> + return lhs.id() == rhs.id();
> +}
> +
> +static inline bool operator!=(const ControlId &lhs, const ControlId &rhs)
> +{
> + return !(lhs == rhs);
> +}
> +
> +template<typename T>
> +class Control : public ControlId
> +{
> +public:
> + using type = T;
> +
> + Control(unsigned int id, const char *name);
> +
> +private:
> + Control(const Control &) = delete;
> + Control &operator=(const Control &) = delete;
> };
>
> class ControlInfo
> {
> public:
> - explicit ControlInfo(ControlId id, const ControlValue &min = 0,
> + explicit ControlInfo(const ControlId &id, const ControlValue &min = 0,
> const ControlValue &max = 0);
>
> - ControlId id() const { return ident_->id; }
> - const char *name() const { return ident_->name; }
> - ControlType type() const { return ident_->type; }
> -
> + const ControlId &id() const { return id_; }
> const ControlValue &min() const { return min_; }
> const ControlValue &max() const { return max_; }
>
> std::string toString() const;
>
> private:
> - const struct ControlIdentifier *ident_;
> + const ControlId &id_;
> ControlValue min_;
> ControlValue max_;
> };
>
> -bool operator==(const ControlInfo &lhs, const ControlInfo &rhs);
> -bool operator==(const ControlId &lhs, const ControlInfo &rhs);
> -bool operator==(const ControlInfo &lhs, const ControlId &rhs);
> -static inline bool operator!=(const ControlInfo &lhs, const ControlInfo &rhs)
> -{
> - return !(lhs == rhs);
> -}
> -static inline bool operator!=(const ControlId &lhs, const ControlInfo &rhs)
> -{
> - return !(lhs == rhs);
> -}
> -static inline bool operator!=(const ControlInfo &lhs, const ControlId &rhs)
> -{
> - return !(lhs == rhs);
> -}
> -
> -using ControlInfoMap = std::unordered_map<ControlId, ControlInfo>;
> +using ControlInfoMap = std::unordered_map<const ControlId *, ControlInfo>;
>
> class ControlList
> {
> private:
> - using ControlListMap = std::unordered_map<const ControlInfo *, ControlValue>;
> + using ControlListMap = std::unordered_map<const ControlId *, ControlValue>;
>
> public:
> ControlList(Camera *camera);
> @@ -114,18 +131,39 @@ public:
> const_iterator begin() const { return controls_.begin(); }
> const_iterator end() const { return controls_.end(); }
>
> - bool contains(ControlId id) const;
> - bool contains(const ControlInfo *info) const;
> + bool contains(const ControlId &id) const;
> bool empty() const { return controls_.empty(); }
> std::size_t size() const { return controls_.size(); }
> void clear() { controls_.clear(); }
>
> - ControlValue &operator[](ControlId id);
> - ControlValue &operator[](const ControlInfo *info) { return controls_[info]; }
> + template<typename T>
> + const T &get(const Control<T> &ctrl) const
> + {
> + const ControlValue *val = find(ctrl);
> + if (!val) {
> + static T t(0);
> + return t;
> + }
> +
> + return val->get<T>();
> + }
> +
> + template<typename T>
> + void set(const Control<T> &ctrl, const T &value)
> + {
> + ControlValue *val = find(ctrl);
> + if (!val)
> + return;
> +
> + val->set<T>(value);
> + }
>
> void update(const ControlList &list);
>
> private:
> + const ControlValue *find(const ControlId &id) const;
> + ControlValue *find(const ControlId &id);
> +
> Camera *camera_;
> ControlListMap controls_;
> };
> diff --git a/src/libcamera/control_ids.cpp b/src/libcamera/control_ids.cpp
> new file mode 100644
> index 000000000000..3af23a458862
> --- /dev/null
> +++ b/src/libcamera/control_ids.cpp
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * control_ids.cpp : Control ID list
> + */
> +
> +#include <libcamera/control_ids.h>
> +
> +/**
> + * \file control_ids.h
> + * \brief Camera control identifiers
> + */
> +
> +namespace libcamera {
> +
> +namespace controls {
> +
> +/**
> + * \brief Enables or disables the AWB.
> + * \sa ManualGain
> + */
> +extern const Control<bool> AwbEnable(AWB_ENABLE, "AwbEnable");
> +
> +/**
> + * \brief Specify a fixed brightness parameter.
> + */
> +extern const Control<int32_t> Brightness(BRIGHTNESS, "Brightness");
> +
> +/**
> + * \brief Specify a fixed contrast parameter.
> + */
> +extern const Control<int32_t> Contrast(CONTRAST, "Contrast");
> +
> +/**
> + * \brief Specify a fixed saturation parameter.
> + */
> +extern const Control<int32_t> Saturation(SATURATION, "Saturation");
> +
> +/**
> + * \brief Specify a fixed exposure time in milli-seconds
> + */
> +extern const Control<int32_t> ManualExposure(MANUAL_EXPOSURE, "ManualExposure");
> +
> +/**
> + * \brief Specify a fixed gain parameter
> + */
> +extern const Control<int32_t> ManualGain(MANUAL_GAIN, "ManualGain");
> +
> +} /* namespace controls */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 295ccd55a622..a34af588fc7e 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -18,6 +18,29 @@
> /**
> * \file controls.h
> * \brief Describes control framework and controls supported by a camera
> + *
> + * A control is a mean to govern or influence the operation of a camera. Every
> + * control is defined by a unique numerical ID, a name string and the data type
> + * of the value it stores. The libcamera API defines a set of standard controls
> + * in the libcamera::controls namespace, as a set of instances of the Control
> + * class.
> + *
> + * The main way for applications to interact with controls is through the
> + * ControlList stored in the Request class:
> + *
> + * \code{.cpp}
> + * Request *req = ...;
> + * ControlList &controls = req->controls();
> + * controls->set(controls::AwbEnable, false);
> + * controls->set(controls::ManualExposure, 1000);
> + *
> + * ...
> + *
> + * int32_t exposure = controls->get(controls::ManualExposure);
> + * \endcode
> + *
> + * The ControlList::get() and ControlList::set() methods automatically deduce
> + * the data type based on the control.
> */
>
> namespace libcamera {
> @@ -173,76 +196,120 @@ std::string ControlValue::toString() const
> }
>
> /**
> - * \enum ControlId
> - * \brief Numerical control ID
> + * \class ControlId
> + * \brief Control static metadata
> + *
> + * The ControlId class stores a control ID, name and data type. It provides
> + * unique identification of a control, but without support for compile-time
> + * type deduction that the derived template Control class supports. See the
> + * Control class for more information.
> */
>
> /**
> - * \var AwbEnable
> - * ControlType: Bool
> - *
> - * Enables or disables the AWB. See also \a libcamera::ControlId::ManualGain
> + * \fn ControlId::ControlId(unsigned int id, const char *name, ControlType type)
> + * \brief Construct a ControlId instance
> + * \param[in] id The control numerical ID
> + * \param[in] name The control name
> + * \param[in] type The control data type
> */
>
> /**
> - * \var Brightness
> - * ControlType: Integer32
> - *
> - * Specify a fixed brightness parameter.
> + * \fn unsigned int ControlId::id() const
> + * \brief Retrieve the control numerical ID
> + * \return The control numerical ID
> */
>
> /**
> - * \var Contrast
> - * ControlType: Integer32
> - *
> - * Specify a fixed contrast parameter.
> + * \fn const char *ControlId::name() const
> + * \brief Retrieve the control name
> + * \return The control name
> */
>
> /**
> - * \var Saturation
> - * ControlType: Integer32
> - *
> - * Specify a fixed saturation parameter.
> + * \fn ControlType ControlId::type() const
> + * \brief Retrieve the control data type
> + * \return The control data type
> */
>
> /**
> - * \var ManualExposure
> - * ControlType: Integer32
> + * \fn bool operator==(const ControlId &lhs, const ControlId &rhs)
> + * \brief Compare two ControlId instances for equality
> + * \param[in] lhs Left-hand side ControlId
> + * \param[in] rhs Right-hand side ControlId
> + *
> + * ControlId instances are compared based on the numerical ControlId::id()
> + * only, as an object may not have two separate controls with the same
> + * numerical ID.
> *
> - * Specify a fixed exposure time in milli-seconds
> + * \return True if \a lhs and \a rhs have equal control IDs, false otherwise
> */
>
> /**
> - * \var ManualGain
> - * ControlType: Integer32
> + * \class Control
> + * \brief Describe a control and its intrinsic properties
> + *
> + * The Control class models a control exposed by a camera. Its template type
> + * name T refers to the control data type, and allows methods that operate on
> + * control values to be defined as template methods using the same type T for
> + * the control value. See for instance how the ControlList::get() method
> + * returns a value corresponding to the type of the requested control.
> + *
> + * While this class is the main mean to refer to a control, the control
> + * identifying information are stored in the non-template base ControlId class.
> + * This allows code that operates on a set of controls of different types to
> + * reference those controls through a ControlId instead of a Control. For
> + * instance, the list of controls supported by a camera is exposed as ControlId
> + * instead of Control.
> *
> - * Specify a fixed gain parameter
> + * Controls of any type can be defined through template specialisation, but
> + * libcamera only supports the bool, int32_t and int64_t types natively (this
> + * includes types that are equivalent to the supported types, such as int and
> + * long int).
> + *
> + * Controls IDs shall be unique. While nothing prevents multiple instances of
> + * the Control class to be created with the same ID, this may lead to undefined
> + * behaviour.
> */
>
> /**
> - * \struct ControlIdentifier
> - * \brief Describe a ControlId with control specific constant meta-data
> - *
> - * Defines a Control with a unique ID, a name, and a type.
> - * This structure is used as static part of the auto-generated control
> - * definitions, which are generated from the ControlId documentation.
> + * \fn Control::Control(unsigned int id, const char *name)
> + * \brief Construct a Control instance
> + * \param[in] id The control numerical ID
> + * \param[in] name The control name
> *
> - * \var ControlIdentifier::id
> - * The unique ID for a control
> - * \var ControlIdentifier::name
> - * The string representation of the control
> - * \var ControlIdentifier::type
> - * The ValueType required to represent the control value
> + * The control data type is automatically deduced from the template type T.
> */
>
> -/*
> - * The controlTypes are automatically generated to produce a control_types.cpp
> - * output. This file is not for public use, and so no suitable header exists
> - * for this sole usage of the controlTypes reference. As such the extern is
> - * only defined here for use during the ControlInfo constructor and should not
> - * be referenced directly elsewhere.
> +/**
> + * \typedef Control::type
> + * \brief The Control template type T
> */
> -extern const std::unordered_map<ControlId, ControlIdentifier> controlTypes;
> +
> +#ifndef __DOXYGEN__
> +template<>
> +Control<void>::Control(unsigned int id, const char *name)
> + : ControlId(id, name, ControlTypeNone)
> +{
> +}
> +
> +template<>
> +Control<bool>::Control(unsigned int id, const char *name)
> + : ControlId(id, name, ControlTypeBool)
> +{
> +}
> +
> +template<>
> +Control<int32_t>::Control(unsigned int id, const char *name)
> + : ControlId(id, name, ControlTypeInteger32)
> +{
> +}
> +
> +template<>
> +Control<int64_t>::Control(unsigned int id, const char *name)
> + : ControlId(id, name, ControlTypeInteger64)
> +{
> +}
> +#endif /* __DOXYGEN__ */
>
> /**
> * \class ControlInfo
> @@ -260,17 +327,10 @@ extern const std::unordered_map<ControlId, ControlIdentifier> controlTypes;
> * \param[in] min The control minimum value
> * \param[in] max The control maximum value
> */
> -ControlInfo::ControlInfo(ControlId id, const ControlValue &min,
> +ControlInfo::ControlInfo(const ControlId &id, const ControlValue &min,
> const ControlValue &max)
> - : min_(min), max_(max)
> + : id_(id), min_(min), max_(max)
> {
> - auto iter = controlTypes.find(id);
> - if (iter == controlTypes.end()) {
> - LOG(Controls, Fatal) << "Attempt to create invalid ControlInfo";
> - return;
> - }
> -
> - ident_ = &iter->second;
> }
>
> /**
> @@ -279,18 +339,6 @@ ControlInfo::ControlInfo(ControlId id, const ControlValue &min,
> * \return The control ID
> */
>
> -/**
> - * \fn ControlInfo::name()
> - * \brief Retrieve the control name string
> - * \return The control name string
> - */
> -
> -/**
> - * \fn ControlInfo::type()
> - * \brief Retrieve the control data type
> - * \return The control data type
> - */
> -
> /**
> * \fn ControlInfo::min()
> * \brief Retrieve the minimum value of the control
> @@ -310,56 +358,11 @@ std::string ControlInfo::toString() const
> {
> std::stringstream ss;
>
> - ss << name() << "[" << min_.toString() << ".." << max_.toString() << "]";
> + ss << id_.name() << "[" << min_.toString() << ".." << max_.toString() << "]";
>
> return ss.str();
> }
>
> -/**
> - * \brief Compare control information for equality
> - * \param[in] lhs Left-hand side control information
> - * \param[in] rhs Right-hand side control information
> - *
> - * Control information is compared based on the ID only, as a camera may not
> - * have two separate controls with the same ID.
> - *
> - * \return True if \a lhs and \a rhs are equal, false otherwise
> - */
> -bool operator==(const ControlInfo &lhs, const ControlInfo &rhs)
> -{
> - return lhs.id() == rhs.id();
> -}
> -
> -/**
> - * \brief Compare control ID and information for equality
> - * \param[in] lhs Left-hand side control identifier
> - * \param[in] rhs Right-hand side control information
> - *
> - * Control information is compared based on the ID only, as a camera may not
> - * have two separate controls with the same ID.
> - *
> - * \return True if \a lhs and \a rhs are equal, false otherwise
> - */
> -bool operator==(const ControlId &lhs, const ControlInfo &rhs)
> -{
> - return lhs == rhs.id();
> -}
> -
> -/**
> - * \brief Compare control information and ID for equality
> - * \param[in] lhs Left-hand side control information
> - * \param[in] rhs Right-hand side control identifier
> - *
> - * Control information is compared based on the ID only, as a camera may not
> - * have two separate controls with the same ID.
> - *
> - * \return True if \a lhs and \a rhs are equal, false otherwise
> - */
> -bool operator==(const ControlInfo &lhs, const ControlId &rhs)
> -{
> - return lhs.id() == rhs;
> -}
> -
> /**
> * \typedef ControlInfoMap
> * \brief A map of ControlId to ControlInfo
> @@ -431,29 +434,9 @@ ControlList::ControlList(Camera *camera)
> *
> * \return True if the list contains a matching control, false otherwise
> */
> -bool ControlList::contains(ControlId id) const
> +bool ControlList::contains(const ControlId &id) const
> {
> - const ControlInfoMap &controls = camera_->controls();
> - const auto iter = controls.find(id);
> - if (iter == controls.end()) {
> - LOG(Controls, Error)
> - << "Camera " << camera_->name()
> - << " does not support control " << id;
> -
> - return false;
> - }
> -
> - return controls_.find(&iter->second) != controls_.end();
> -}
> -
> -/**
> - * \brief Check if the list contains a control with the specified \a info
> - * \param[in] info The control info
> - * \return True if the list contains a matching control, false otherwise
> - */
> -bool ControlList::contains(const ControlInfo *info) const
> -{
> - return controls_.find(info) != controls_.end();
> + return controls_.find(&id) != controls_.end();
> }
>
> /**
> @@ -474,44 +457,61 @@ bool ControlList::contains(const ControlInfo *info) const
> */
>
> /**
> - * \brief Access or insert the control specified by \a id
> - * \param[in] id The control ID
> + * \fn template<typename T> const T &ControlList::get() const
> + * \brief Get the value of a control
> + * \param[in] ctrl The control
> *
> - * This method returns a reference to the control identified by \a id, inserting
> - * it in the list if the ID is not already present.
> + * The behaviour is undefined if the control \a ctrl is not present in the
> + * list. Use ControlList::contains() to test for the presence of a control in
> + * the list before retrieving its value.
> *
> - * The behaviour is undefined if the control \a id is not supported by the
> - * camera that the ControlList refers to.
> + * The control value type shall match the type T, otherwise the behaviour is
> + * undefined.
> *
> - * \return A reference to the value of the control identified by \a id
> + * \return The control value
> */
> -ControlValue &ControlList::operator[](ControlId id)
> +
> +/**
> + * \fn template<typename T> void ControlList::set()
> + * \brief Set the control value to \a value
> + * \param[in] ctrl The control
> + * \param[in] value The control value
> + *
> + * This method sets the value of a control in the control list. If the control
> + * is already present in the list, its value is updated, otherwise it is added
> + * to the list.
> + *
> + * The behaviour is undefined if the control \a ctrl is not supported by the
> + * camera that the list refers to.
> + */
> +
> +const ControlValue *ControlList::find(const ControlId &id) const
> +{
> + const auto iter = controls_.find(&id);
> + if (iter == controls_.end()) {
> + LOG(Controls, Error)
> + << "Control " << id.name() << " not found";
> +
> + return nullptr;
> + }
> +
> + return &iter->second;
> +}
> +
> +ControlValue *ControlList::find(const ControlId &id)
> {
> const ControlInfoMap &controls = camera_->controls();
> - const auto iter = controls.find(id);
> + const auto iter = controls.find(&id);
> if (iter == controls.end()) {
> LOG(Controls, Error)
> << "Camera " << camera_->name()
> - << " does not support control " << id;
> -
> - static ControlValue empty;
> - return empty;
> + << " does not support control " << id.name();
> + return nullptr;
> }
>
> - return controls_[&iter->second];
> + return &controls_[&id];
> }
>
> -/**
> - * \fn ControlList::operator[](const ControlInfo *info)
> - * \brief Access or insert the control specified by \a info
> - * \param[in] info The control info
> - *
> - * This method returns a reference to the control identified by \a info,
> - * inserting it in the list if the info is not already present.
> - *
> - * \return A reference to the value of the control identified by \a info
> - */
> -
> /**
> * \brief Update the list with a union of itself and \a other
> * \param other The other list
> @@ -533,10 +533,10 @@ void ControlList::update(const ControlList &other)
> }
>
> for (auto it : other) {
> - const ControlInfo *info = it.first;
> + const ControlId *id = it.first;
> const ControlValue &value = it.second;
>
> - controls_[info] = value;
> + controls_[id] = value;
> }
> }
>
> diff --git a/src/libcamera/gen-controls.awk b/src/libcamera/gen-controls.awk
> deleted file mode 100755
> index a3f291e7071c..000000000000
> --- a/src/libcamera/gen-controls.awk
> +++ /dev/null
> @@ -1,106 +0,0 @@
> -#!/usr/bin/awk -f
> -
> -# SPDX-License-Identifier: LGPL-2.1-or-later
> -
> -# Controls are documented using Doxygen in the main controls.cpp source.
> -#
> -# Generate control tables directly from the documentation, creating enumerations
> -# to support the IDs and static type information regarding each control.
> -
> -BEGIN {
> - id=0
> - input=ARGV[1]
> - mode=ARGV[2]
> - output=ARGV[3]
> -}
> -
> -# Detect Doxygen style comment blocks and ignore other lines
> -/^\/\*\*$/ { in_doxygen=1; first_line=1; next }
> -// { if (!in_doxygen) next }
> -
> -# Entry point for the Control Documentation
> -/ * \\enum ControlId$/ { in_controls=1; first_line=0; next }
> -// { if (!in_controls) next }
> -
> -# Extract control information
> -/ \* \\var/ { names[++id]=$3; first_line=0; next }
> -/ \* ControlType:/ { types[id] = $3 }
> -
> -# End of comment blocks
> -/^ \*\// { in_doxygen=0 }
> -
> -# Identify the end of controls
> -/^ \* \\/ { if (first_line) exit }
> -// { first_line=0 }
> -
> -################################
> -# Support output file generation
> -
> -function basename(file) {
> - sub(".*/", "", file)
> - return file
> -}
> -
> -function Header(file, description) {
> - print "/* SPDX-License-Identifier: LGPL-2.1-or-later */" > file
> - print "/*" > file
> - print " * Copyright (C) 2019, Google Inc." > file
> - print " *" > file
> - print " * " basename(file) " - " description > file
> - print " *" > file
> - print " * This file is auto-generated. Do not edit." > file
> - print " */" > file
> - print "" > file
> -}
> -
> -function EnterNameSpace(file) {
> - print "namespace libcamera {" > file
> - print "" > file
> -}
> -
> -function ExitNameSpace(file) {
> - print "" > file
> - print "} /* namespace libcamera */" > file
> -}
> -
> -function GenerateHeader(file) {
> - Header(file, "Control ID list")
> -
> - print "#ifndef __LIBCAMERA_CONTROL_IDS_H__" > file
> - print "#define __LIBCAMERA_CONTROL_IDS_H__" > file
> - print "" > file
> -
> - EnterNameSpace(file)
> - print "enum ControlId {" > file
> - for (i=1; i <= id; ++i) {
> - printf "\t%s,\n", names[i] > file
> - }
> - print "};" > file
> - ExitNameSpace(file)
> -
> - print "" > file
> - print "#endif // __LIBCAMERA_CONTROL_IDS_H__" > file
> -}
> -
> -function GenerateTable(file) {
> - Header(file, "Control types")
> - print "#include <libcamera/controls.h>" > file
> - print "" > file
> -
> - EnterNameSpace(file)
> -
> - print "extern const std::unordered_map<ControlId, ControlIdentifier>" > file
> - print "controlTypes {" > file
> - for (i=1; i <= id; ++i) {
> - printf "\t{ %s, { %s, \"%s\", ControlType%s } },\n", names[i], names[i], names[i], types[i] > file
> - }
> - print "};" > file
> - ExitNameSpace(file)
> -}
> -
> -END {
> - if (mode == "--header")
> - GenerateHeader(output)
> - else
> - GenerateTable(output)
> -}
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 755149c55c7b..8123d1d5bee9 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -5,6 +5,7 @@ libcamera_sources = files([
> 'camera_manager.cpp',
> 'camera_sensor.cpp',
> 'controls.cpp',
> + 'control_ids.cpp',
> 'device_enumerator.cpp',
> 'device_enumerator_sysfs.cpp',
> 'event_dispatcher.cpp',
> @@ -57,16 +58,6 @@ if libudev.found()
> ])
> endif
>
> -gen_controls = files('gen-controls.awk')
> -
> -control_types_cpp = custom_target('control_types_cpp',
> - input : files('controls.cpp'),
> - output : 'control_types.cpp',
> - depend_files : gen_controls,
> - command : [gen_controls, '@INPUT@', '--code', '@OUTPUT@'])
> -
> -libcamera_sources += control_types_cpp
> -
> gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
>
> version_cpp = vcs_tag(command : [gen_version, meson.build_root()],
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 0d56758e3e1d..d5d30932870a 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -10,6 +10,7 @@
> #include <tuple>
>
> #include <libcamera/camera.h>
> +#include <libcamera/control_ids.h>
> #include <libcamera/controls.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
> @@ -230,33 +231,20 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
> V4L2ControlList controls;
>
> for (auto it : request->controls()) {
> - const ControlInfo *ci = it.first;
> + const ControlId &id = *it.first;
> ControlValue &value = it.second;
>
> - switch (ci->id()) {
> - case Brightness:
> + if (id == controls::Brightness) {
> controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
> - break;
> -
> - case Contrast:
> + } else if (id == controls::Contrast) {
> controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
> - break;
> -
> - case Saturation:
> + } else if (id == controls::Saturation) {
> controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
> - break;
> -
> - case ManualExposure:
> + } else if (id == controls::ManualExposure) {
> controls.add(V4L2_CID_EXPOSURE_AUTO, 1);
> controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());
> - break;
> -
> - case ManualGain:
> + } else if (id == controls::ManualGain) {
> controls.add(V4L2_CID_GAIN, value.get<int32_t>());
> - break;
> -
> - default:
> - break;
> }
> }
>
> @@ -352,23 +340,23 @@ int UVCCameraData::init(MediaEntity *entity)
> for (const auto &ctrl : controls) {
> unsigned int v4l2Id = ctrl.first;
> const V4L2ControlInfo &info = ctrl.second;
> - ControlId id;
> + const ControlId *id;
>
> switch (v4l2Id) {
> case V4L2_CID_BRIGHTNESS:
> - id = Brightness;
> + id = &controls::Brightness;
> break;
> case V4L2_CID_CONTRAST:
> - id = Contrast;
> + id = &controls::Contrast;
> break;
> case V4L2_CID_SATURATION:
> - id = Saturation;
> + id = &controls::Saturation;
> break;
> case V4L2_CID_EXPOSURE_ABSOLUTE:
> - id = ManualExposure;
> + id = &controls::ManualExposure;
> break;
> case V4L2_CID_GAIN:
> - id = ManualGain;
> + id = &controls::ManualGain;
> break;
> default:
> continue;
> @@ -376,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity)
>
> controlInfo_.emplace(std::piecewise_construct,
> std::forward_as_tuple(id),
> - std::forward_as_tuple(id, info.min(), info.max()));
> + std::forward_as_tuple(*id, info.min(), info.max()));
> }
>
> return 0;
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index e549dc64b996..608a47aecf76 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -15,6 +15,7 @@
> #include <ipa/ipa_interface.h>
> #include <ipa/ipa_module_info.h>
> #include <libcamera/camera.h>
> +#include <libcamera/control_ids.h>
> #include <libcamera/controls.h>
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
> @@ -283,24 +284,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
> V4L2ControlList controls;
>
> for (auto it : request->controls()) {
> - const ControlInfo *ci = it.first;
> + const ControlId &id = *it.first;
> ControlValue &value = it.second;
>
> - switch (ci->id()) {
> - case Brightness:
> + if (id == controls::Brightness) {
> controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
> - break;
> -
> - case Contrast:
> + } else if (id == controls::Contrast) {
> controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
> - break;
> -
> - case Saturation:
> + } else if (id == controls::Saturation) {
> controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
> - break;
> -
> - default:
> - break;
> }
> }
>
> @@ -427,17 +419,17 @@ int VimcCameraData::init(MediaDevice *media)
> for (const auto &ctrl : controls) {
> unsigned int v4l2Id = ctrl.first;
> const V4L2ControlInfo &info = ctrl.second;
> - ControlId id;
> + const ControlId *id;
>
> switch (v4l2Id) {
> case V4L2_CID_BRIGHTNESS:
> - id = Brightness;
> + id = &controls::Brightness;
> break;
> case V4L2_CID_CONTRAST:
> - id = Contrast;
> + id = &controls::Contrast;
> break;
> case V4L2_CID_SATURATION:
> - id = Saturation;
> + id = &controls::Saturation;
> break;
> default:
> continue;
> @@ -445,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media)
>
> controlInfo_.emplace(std::piecewise_construct,
> std::forward_as_tuple(id),
> - std::forward_as_tuple(id, info.min(), info.max()));
> + std::forward_as_tuple(*id, info.min(), info.max()));
> }
>
> return 0;
> diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp
> index 2aba568a302e..dbc43df8e3d3 100644
> --- a/test/controls/control_info.cpp
> +++ b/test/controls/control_info.cpp
> @@ -7,6 +7,7 @@
>
> #include <iostream>
>
> +#include <libcamera/control_ids.h>
> #include <libcamera/controls.h>
>
> #include "test.h"
> @@ -23,17 +24,17 @@ protected:
> * Test information retrieval from a control with no minimum
> * and maximum.
> */
> - ControlInfo info(Brightness);
> + ControlInfo brightness(controls::Brightness);
>
> - if (info.id() != Brightness ||
> - info.type() != ControlTypeInteger32 ||
> - info.name() != std::string("Brightness")) {
> + if (brightness.id() != controls::Brightness ||
> + brightness.id().type() != ControlTypeInteger32 ||
> + brightness.id().name() != std::string("Brightness")) {
> cout << "Invalid control identification for Brightness" << endl;
> return TestFail;
> }
>
> - if (info.min().get<int32_t>() != 0 ||
> - info.max().get<int32_t>() != 0) {
> + if (brightness.min().get<int32_t>() != 0 ||
> + brightness.max().get<int32_t>() != 0) {
> cout << "Invalid control range for Brightness" << endl;
> return TestFail;
> }
> @@ -42,17 +43,17 @@ protected:
> * Test information retrieval from a control with a minimum and
> * a maximum value.
> */
> - info = ControlInfo(Contrast, 10, 200);
> + ControlInfo contrast(controls::Contrast, 10, 200);
>
> - if (info.id() != Contrast ||
> - info.type() != ControlTypeInteger32 ||
> - info.name() != std::string("Contrast")) {
> + if (contrast.id() != controls::Contrast ||
> + contrast.id().type() != ControlTypeInteger32 ||
> + contrast.id().name() != std::string("Contrast")) {
> cout << "Invalid control identification for Contrast" << endl;
> return TestFail;
> }
>
> - if (info.min().get<int32_t>() != 10 ||
> - info.max().get<int32_t>() != 200) {
> + if (contrast.min().get<int32_t>() != 10 ||
> + contrast.max().get<int32_t>() != 200) {
> cout << "Invalid control range for Contrast" << endl;
> return TestFail;
> }
> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> index 5215840b1c4e..053696814b67 100644
> --- a/test/controls/control_list.cpp
> +++ b/test/controls/control_list.cpp
> @@ -9,6 +9,7 @@
>
> #include <libcamera/camera.h>
> #include <libcamera/camera_manager.h>
> +#include <libcamera/control_ids.h>
> #include <libcamera/controls.h>
>
> #include "test.h"
> @@ -52,7 +53,7 @@ protected:
> return TestFail;
> }
>
> - if (list.contains(Brightness)) {
> + if (list.contains(controls::Brightness)) {
> cout << "List should not contain Brightness control" << endl;
> return TestFail;
> }
> @@ -70,7 +71,7 @@ protected:
> * Set a control, and verify that the list now contains it, and
> * nothing else.
> */
> - list[Brightness] = 255;
> + list.set(controls::Brightness, 255);
>
> if (list.empty()) {
> cout << "List should not be empty" << endl;
> @@ -82,7 +83,7 @@ protected:
> return TestFail;
> }
>
> - if (!list.contains(Brightness)) {
> + if (!list.contains(controls::Brightness)) {
> cout << "List should contain Brightness control" << endl;
> return TestFail;
> }
> @@ -96,54 +97,45 @@ protected:
> return TestFail;
> }
>
> - if (list[Brightness].get<int32_t>() != 255) {
> + if (list.get(controls::Brightness) != 255) {
> cout << "Incorrest Brightness control value" << endl;
> return TestFail;
> }
>
> - if (list.contains(Contrast)) {
> + if (list.contains(controls::Contrast)) {
> cout << "List should not contain Contract control" << endl;
> return TestFail;
> }
>
> - /*
> - * Set a second control through ControlInfo and retrieve it
> - * through both controlId and ControlInfo.
> - */
> - const ControlInfoMap &controls = camera_->controls();
> - const ControlInfo *brightness = &controls.find(Brightness)->second;
> - const ControlInfo *contrast = &controls.find(Contrast)->second;
> + /* Update the first control and set a second one. */
> + list.set(controls::Brightness, 64);
> + list.set(controls::Contrast, 128);
>
> - list[brightness] = 64;
> - list[contrast] = 128;
> -
> - if (!list.contains(Contrast) || !list.contains(contrast)) {
> + if (!list.contains(controls::Contrast) ||
> + !list.contains(controls::Contrast)) {
> cout << "List should contain Contrast control" << endl;
> return TestFail;
> }
>
> - /*
> - * Test control value retrieval and update through ControlInfo.
> - */
> - if (list[brightness].get<int32_t>() != 64 ||
> - list[contrast].get<int32_t>() != 128) {
> + if (list.get(controls::Brightness) != 64 ||
> + list.get(controls::Contrast) != 128) {
> cout << "Failed to retrieve control value" << endl;
> return TestFail;
> }
>
> - list[brightness] = 10;
> - list[contrast] = 20;
> + /*
> + * Update both controls and verify that the container doesn't
> + * grow.
> + */
> + list.set(controls::Brightness, 10);
> + list.set(controls::Contrast, 20);
>
> - if (list[brightness].get<int32_t>() != 10 ||
> - list[contrast].get<int32_t>() != 20) {
> + if (list.get(controls::Brightness) != 10 ||
> + list.get(controls::Contrast) != 20) {
> cout << "Failed to update control value" << endl;
> return TestFail;
> }
>
> - /*
> - * Assert that the container has not grown with the control
> - * updated.
> - */
> if (list.size() != 2) {
> cout << "List should contain two elements" << endl;
> return TestFail;
> @@ -157,8 +149,8 @@ protected:
> */
> ControlList newList(camera_.get());
>
> - newList[Brightness] = 128;
> - newList[Saturation] = 255;
> + newList.set(controls::Brightness, 128);
> + newList.set(controls::Saturation, 255);
> newList.update(list);
>
> list.clear();
> @@ -178,16 +170,16 @@ protected:
> return TestFail;
> }
>
> - if (!newList.contains(Brightness) ||
> - !newList.contains(Contrast) ||
> - !newList.contains(Saturation)) {
> + if (!newList.contains(controls::Brightness) ||
> + !newList.contains(controls::Contrast) ||
> + !newList.contains(controls::Saturation)) {
> cout << "New list contains incorrect items" << endl;
> return TestFail;
> }
>
> - if (newList[Brightness].get<int32_t>() != 10 ||
> - newList[Contrast].get<int32_t>() != 20 ||
> - newList[Saturation].get<int32_t>() != 255) {
> + if (newList.get(controls::Brightness) != 10 ||
> + newList.get(controls::Contrast) != 20 ||
> + newList.get(controls::Saturation) != 255) {
> cout << "New list contains incorrect values" << endl;
> return TestFail;
> }
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list