[libcamera-devel] [RFC PATCH v2 3/9] libcamera: controls: Introduce Control structures

[Niklas Söderlund]

Hi Kieran,

Thanks for your work.

On 2019-06-21 17:13:55 +0100, Kieran Bingham wrote:
> ControlIdentifiers declare the types of a control, and map their names,
> and the ControlInfo class allows runtime state to be represented.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  include/libcamera/controls.h  |  58 ++++++++++++
>  include/libcamera/meson.build |   1 +
>  src/libcamera/controls.cpp    | 160 ++++++++++++++++++++++++++++++++++
>  src/libcamera/meson.build     |   1 +
>  4 files changed, 220 insertions(+)
>  create mode 100644 include/libcamera/controls.h
>  create mode 100644 src/libcamera/controls.cpp
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> new file mode 100644
> index 000000000000..95198d41c4cf
> --- /dev/null
> +++ b/include/libcamera/controls.h
> @@ -0,0 +1,58 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * controls.h - Control handling
> + */
> +#ifndef __LIBCAMERA_CONTROLS_H__
> +#define __LIBCAMERA_CONTROLS_H__
> +
> +#include <libcamera/value.h>
> +
> +namespace libcamera {
> +
> +enum ControlId : uint32_t {
> +	/* IPA Controls */
> +	IpaAwbEnable,
> +
> +	/* Manual Controls */
> +	ManualBrightness,
> +	ManualExposure,
> +	ManualGain,
> +};
> +
> +struct ControlIdentifier {
> +	ControlId id;
> +	const char *name;
> +	ValueType type;
> +};
> +
> +class ControlInfo
> +{
> +public:
> +	ControlInfo(ControlId id, Value min = 0, Value max = 0);
> +
> +	ControlId id() const { return ident_->id; }
> +	const char *name() const { return ident_->name; }
> +	ValueType type() const { return ident_->type; }
> +
> +	const Value &min() { return min_; }
> +	const Value &max() { return min_; }

These can be const functions.

> +
> +	std::string toString() const;
> +
> +	bool operator==(const ControlInfo &rhs) const { return id() == rhs.id(); }
> +	bool operator==(const ControlId rhs) const { return id() == rhs; }
> +
> +private:
> +	struct ControlIdentifier const *ident_;
> +	Value min_;
> +	Value max_;
> +
> +};
> +
> +std::ostream &operator<<(std::ostream &stream, const ControlInfo &value);
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_CONTROLS_H__ */
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index eb2211ae1fc3..06e3feebd23d 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -1,5 +1,6 @@
>  libcamera_api = files([
>      'buffer.h',
> +    'controls.h',

Not inserted in alphabetical order ;-)

>      'camera.h',
>      'camera_manager.h',
>      'event_dispatcher.h',
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> new file mode 100644
> index 000000000000..b1be46ddb55e
> --- /dev/null
> +++ b/src/libcamera/controls.cpp
> @@ -0,0 +1,160 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * controls.cpp - Control handling
> + */
> +
> +#include <limits.h>
> +#include <sstream>
> +#include <string>
> +
> +#include <libcamera/controls.h>

You should have this as the first include statement in the file.

> +
> +#include "log.h"
> +#include "utils.h"
> +
> +/**
> + * \file controls.h
> + * \brief Describes control framework and controls supported by a camera
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(Controls)
> +
> +/**
> + * \enum ControlId
> + * Control Identifiers
> + * \var IpaAwbEnable
> + * bool: Enables or disables the AWB
> + * \var ManualExposure
> + * Manually control the exposure time in milli-seconds
> + * \var ManualGain
> + * Controls the value of the manual gain setting
> + */

ManualBrightness missing.

> +
> +/**
> + * \struct ControlIdentifier
> + * Defines a Control with a unique ID, a name, and a type.
> + * \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
> + */
> +
> +/*
> + * Two sets of tables are generated for the static control definitions.
> + *
> + * An enum to declare the ID (in controls.h), and a type to establish its
> + * representation.
> + *
> + * Todo: Automate the generation of both tables from a single input table.
> + * Todo: Consider if this should be a static std::map created at init instead.

I think a static std::map (or maybe unordered_map) is a good idea.

> + */
> +static const struct ControlIdentifier ControlTypes[] = {
> +#define CONTROL_TYPE(_id, _type) { _id, #_id, _type }
> +
> +	CONTROL_TYPE(IpaAwbEnable,		ValueBool),
> +	CONTROL_TYPE(ManualBrightness,		ValueInteger),
> +	CONTROL_TYPE(ManualExposure,		ValueInteger),
> +	CONTROL_TYPE(ManualGain,		ValueInteger),
> +
> +#undef CONTROL_TYPE
> +};
> +
> +static struct ControlIdentifier const *FindControlType(ControlId id)
> +{
> +	struct ControlIdentifier const *ident;
> +
> +	for (ident = ControlTypes;
> +	     ident != &ControlTypes[ARRAY_SIZE(ControlTypes)];
> +	     ++ident) {
> +		if (ident->id == id)
> +			return ident;
> +	}
> +
> +	LOG(Controls, Fatal) << "Failed to find a ControlType.";
> +
> +	/* Unreachable. */
> +	return nullptr;
> +}
> +
> +/**
> + * \class ControlInfo
> + * \brief Describes the information and capabilities of a Control
> + */
> +
> +/**
> + * \brief Construct a ControlInfo with minimum and maximum range parameters.
> + */
> +ControlInfo::ControlInfo(ControlId id, Value min, Value max)
> +	: ident_(FindControlType(id)), min_(min), max_(max)
> +{
> +}
> +
> +/**
> + * \fn ControlInfo::id()
> + * \brief Return the ID of the control information descriptor
> + * \return the ControlId
> + */
> +
> +/**
> + * \fn ControlInfo::name()
> + * \brief Return the string name of the control information descriptor
> + * \return A string name for the Control
> + */
> +
> +/**
> + * \fn ControlInfo::type()
> + * \brief Return the ValueType of the control information descriptor
> + * \return the ControlId
> + */
> +
> +/**
> + * \fn ControlInfo::min()
> + * \brief Reports the minimum value of the control
> + * \return a Value with the minimum setting for the control
> + */
> +
> +/**
> + * \fn ControlInfo::max()
> + * \brief Reports the maximum value of the control
> + * \return a Value with the maximum setting for the control
> + */
> +
> +/**
> + * \brief Provide a string representation of the ControlInfo
> + */
> +std::string ControlInfo::toString() const
> +{
> +	std::stringstream ss;
> +
> +	ss << "Control: " << name()
> +	   << " : Min(" << min_ << ") Max(" << max_ << ")";
> +
> +	return ss.str();
> +}
> +
> +/**
> + * \fn ControlInfo::operator==(const ControlInfo &rhs) const
> + * \brief Establish equivalence of ControlInfo. Only the IDs are considered.
> + */
> +
> +/**
> + * \fn ControlInfo::operator==(const ControlId rhs) const
> + * \brief Establish equivalence of ControlInfo, against a ControlID.
> + */
> +
> +/**
> + * \brief Provide a string stream representation of the ControlInfo \a info to
> + * the \a stream.
> + */
> +std::ostream &operator<<(std::ostream &stream, const ControlInfo &info)
> +{
> +	return stream << info.toString();
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 8e68373118df..e2c07d79bfb5 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -3,6 +3,7 @@ libcamera_sources = files([
>      'camera.cpp',
>      'camera_manager.cpp',
>      'camera_sensor.cpp',
> +    'controls.cpp',
>      'device_enumerator.cpp',
>      'device_enumerator_sysfs.cpp',
>      'event_dispatcher.cpp',
> -- 
> 2.20.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund