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

[Laurent Pinchart]

On Sun, Jun 23, 2019 at 05:22:03PM +0200, Niklas Söderlund wrote:
> 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.

And the second one should return max_.

> > +
> > +	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 ;-)

How did *I* miss that one ? :-)

> >      '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',

-- 
Regards,

Laurent Pinchart