@@ -19,7 +19,7 @@ namespace libcamera {
class Camera;
-struct ControlIdentifier {
+struct ControlMetadata {
ControlId id;
const char *name;
DataType type;
@@ -30,28 +30,30 @@ class ControlInfo : public DataInfo
public:
explicit ControlInfo(ControlId id, const DataValue &min = 0,
const DataValue &max = 0);
- ControlId id() const { return ident_->id; }
- const char *name() const { return ident_->name; }
- DataType type() const { return ident_->type; }
+ ControlId id() const { return meta_->id; }
+ const char *name() const { return meta_->name; }
+ DataType type() const { return meta_->type; }
std::string toString() const;
private:
- const struct ControlIdentifier *ident_;
+ const struct ControlMetadata *meta_;
};
using ControlInfoMap = std::unordered_map<ControlId, ControlInfo>;
+using ControlValue = DataValue;
+
class ControlList
{
private:
- using ControlListMap = std::unordered_map<const ControlInfo *, DataValue>;
+ using ControlMap = std::unordered_map<const ControlInfo *, ControlValue>;
public:
ControlList(const ControlInfoMap &infoMap);
- using iterator = ControlListMap::iterator;
- using const_iterator = ControlListMap::const_iterator;
+ using iterator = ControlMap::iterator;
+ using const_iterator = ControlMap::const_iterator;
iterator begin() { return controls_.begin(); }
iterator end() { return controls_.end(); }
@@ -64,14 +66,14 @@ public:
std::size_t size() const { return controls_.size(); }
void clear() { controls_.clear(); }
- DataValue &operator[](ControlId id);
- DataValue &operator[](const ControlInfo *info) { return controls_[info]; }
+ ControlValue &operator[](ControlId id);
+ ControlValue &operator[](const ControlInfo *info) { return controls_[info]; }
- void update(const ControlList &list);
+ bool merge(const ControlList &list);
private:
const ControlInfoMap &infoMap_;
- ControlListMap controls_;
+ ControlMap controls_;
};
} /* namespace libcamera */
@@ -7,9 +7,6 @@
#include <libcamera/controls.h>
-#include <sstream>
-#include <string>
-
#include <libcamera/camera.h>
#include "log.h"
@@ -17,7 +14,7 @@
/**
* \file controls.h
- * \brief Describes control framework and controls supported by a camera
+ * \brief Control framework implementation
*/
namespace libcamera {
@@ -72,38 +69,52 @@ LOG_DEFINE_CATEGORY(Controls)
*/
/**
- * \struct ControlIdentifier
- * \brief Describe a ControlId with control specific constant meta-data
+ * \struct ControlMetadata
+ * \brief Describe the control static meta-data
+ *
+ * Defines the Control static meta-data information, auto-generated from the
+ * ControlId documentation.
*
- * 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.
+ * The control information is defined in the control_id.h file, parsed by
+ * the gen-controls.awk script to produce a control_metadata.cpp file that
+ * provides a static list of control id with an associated type and name. The
+ * file is not for public use, and so no suitable header exists as
+ * its sole usage is for the ControlMetadata definition.
*
- * \var ControlIdentifier::id
+ * \var ControlMetadata::id
* The unique ID for a control
- * \var ControlIdentifier::name
+ * \var ControlMetadata::name
* The string representation of the control
- * \var ControlIdentifier::type
+ * \var ControlMetadata::type
* The ValueType required to represent the control value
*/
/*
- * 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
+ * \todo Expand the control meta-data definition to support more complex
+ * control types and describe their characteristics (ie. Number of data elements
+ * for compound control types, versioning etc).
+ */
+
+/*
+ * The ControlTypes map is defined by the generated control_metadata.cpp file
+ * and only used here during the ControlInfo construction and should not
* be referenced directly elsewhere.
*/
-extern const std::unordered_map<ControlId, ControlIdentifier> controlTypes;
+extern const std::unordered_map<ControlId, ControlMetadata> controlTypes;
/**
* \class ControlInfo
* \brief Describe the information and capabilities of a Control
*
- * The ControlInfo represents control specific meta-data which is constant on a
- * per camera basis. ControlInfo classes are constructed by pipeline handlers
- * to expose the controls they support and the metadata needed to utilise those
- * controls.
+ * The ControlInfo class associates the control's constant metadata defined by
+ * libcamera and collected in the ControlMetadata class, with its static
+ * information, such the range of supported values and the control size,
+ * described by the DataInfo base class and defined by the capabilities of
+ * the Camera device that implements the control.
+ *
+ * ControlInfo instances are constructed by pipeline handlers and associated
+ * with the Camera devices they register to expose the list of controls they
+ * support and the static information required to use those controls.
*/
/**
@@ -118,28 +129,28 @@ ControlInfo::ControlInfo(ControlId id, const DataValue &min,
{
auto iter = controlTypes.find(id);
if (iter == controlTypes.end()) {
- LOG(Controls, Fatal) << "Attempt to create invalid ControlInfo";
+ LOG(Controls, Fatal) << "Control " << id << " not defined";
return;
}
- ident_ = &iter->second;
+ meta_ = &iter->second;
}
/**
* \fn ControlInfo::id()
- * \brief Retrieve the control ID
+ * \brief Retrieve the control ID from the control constant metadata
* \return The control ID
*/
/**
* \fn ControlInfo::name()
- * \brief Retrieve the control name string
+ * \brief Retrieve the control name string from the control constant metadata
* \return The control name string
*/
/**
* \fn ControlInfo::type()
- * \brief Retrieve the control data type
+ * \brief Retrieve the control data type from the constant metadata
* \return The control type
*/
@@ -161,15 +172,36 @@ std::string ControlInfo::toString() const
* \brief A map of ControlId to ControlInfo
*/
+/**
+ * \typedef ControlValue
+ * \brief A control value stored in the ControlList class
+ *
+ * The ControlValue type provides the value associated with a control, stored
+ * in a ControlList and there associated with a ControlInfo that provides
+ * the information for the control id, type and data size.
+ */
+
+/*
+ * \todo If more data and operations than the ones defined by DataValue
+ * will need to be implemented for controls, make this typedef a class that
+ * inherits from DataValue.
+ */
+
/**
* \class ControlList
- * \brief Associate a list of ControlId with their values for a camera
+ * \brief List of controls values
+ *
+ * A ControlList wraps a map of ControlId to ControlValue instances and provide
+ * additional validation against the control information reported by a Camera.
*
- * A ControlList wraps a map of ControlId to DataValue and provide
- * additional validation against the control information exposed by a Camera.
+ * ControlList instances are initially created empty and should be populated by
+ * assigning a value to each control added to the list. The added control is
+ * validated to verify it is supported by the Camera the list refers to, and
+ * the control value is validated against the static information provided by the
+ * ControlInfo instance associated with the control.
*
- * A list is only valid for as long as the camera it refers to is valid. After
- * that calling any method of the ControlList class other than its destructor
+ * A list is only valid as long as the camera it refers to is valid. After
+ * that, calling any method of the ControlList class other than its destructor
* will cause undefined behaviour.
*/
@@ -179,8 +211,9 @@ std::string ControlInfo::toString() const
*
* The provided \a infoMap collects the control id and the associated static
* information for each control that can be set on the list. \a infoMap is
- * provided by the Camera the list of controls refers to and it is used to make
- * sure only controls supported by the camera can be added to the list.
+ * provided by the Camera the list of controls refers to. It is used to make
+ * sure only controls supported by the camera can be added to the list and it is
+ * only valid as long as the camera that provided it stays valid.
*/
ControlList::ControlList(const ControlInfoMap &infoMap)
: infoMap_(infoMap)
@@ -199,14 +232,14 @@ ControlList::ControlList(const ControlInfoMap &infoMap)
/**
* \fn iterator ControlList::begin()
- * \brief Retrieve an iterator to the first Control in the list
- * \return An iterator to the first Control in the list
+ * \brief Retrieve an iterator to the first ControlValue in the list
+ * \return An iterator to the first ControlValue in the list
*/
/**
* \fn const_iterator ControlList::begin() const
- * \brief Retrieve a const_iterator to the first Control in the list
- * \return A const_iterator to the first Control in the list
+ * \brief Retrieve a const_iterator to the first ControlValue in the list
+ * \return A const_iterator to the first ControlValue in the list
*/
/**
@@ -227,8 +260,8 @@ ControlList::ControlList(const ControlInfoMap &infoMap)
* \brief Check if the list contains a control with the specified \a id
* \param[in] id The control ID
*
- * The behaviour is undefined if the control \a id is not supported by the
- * camera that the ControlList refers to.
+ * The behaviour is undefined if the camera the list refers to is not valid
+ * anymore.
*
* \return True if the list contains a matching control, false otherwise
*/
@@ -247,6 +280,10 @@ bool ControlList::contains(ControlId id) const
/**
* \brief Check if the list contains a control with the specified \a info
* \param[in] info The control info
+ *
+ * The behaviour is undefined if the camera the list refers to is not valid
+ * anymore.
+ *
* \return True if the list contains a matching control, false otherwise
*/
bool ControlList::contains(const ControlInfo *info) const
@@ -263,7 +300,7 @@ bool ControlList::contains(const ControlInfo *info) const
/**
* \fn ControlList::size()
* \brief Retrieve the number of controls in the list
- * \return The number of DataValue entries stored in the list
+ * \return The number of control entries stored in the list
*/
/**
@@ -278,18 +315,23 @@ bool ControlList::contains(const ControlInfo *info) const
* 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 id is not supported by the
- * camera that the ControlList refers to.
+ * If the control \a id is not supported by the camera the ControlList
+ * refers to, an empty control is returned and no control value is added to the
+ * list.
+ *
+ * The behaviour is undefined if the camera the list refers to is not valid
+ * anymore.
*
- * \return A reference to the value of the control identified by \a id
+ * \return A reference to the value of the control identified by \a id if the
+ * control is supported, an empty control otherwise
*/
-DataValue &ControlList::operator[](ControlId id)
+ControlValue &ControlList::operator[](ControlId id)
{
const auto info = infoMap_.find(id);
if (info == infoMap_.end()) {
LOG(Controls, Error) << "Control " << id << " not supported";
- static DataValue empty;
+ static ControlValue empty;
return empty;
}
@@ -304,22 +346,35 @@ DataValue &ControlList::operator[](ControlId id)
* 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
+ * If the control \a info is not supported by the camera the ControlList
+ * refers to, an empty control is returned and no control value is added to the
+ * list.
+ *
+ * The behaviour is undefined if the camera the list refers to is not valid
+ * anymore.
+ *
+ * \return A reference to the value of the control identified by \a info if the
+ * control is supported, an empty control otherwise
*/
/**
- * \brief Update the list with a union of itself and \a other
- * \param other The other list
+ * \brief Merge the control in the list with controls from \a other
+ * \param other The control list to merge with
*
* Update the control list to include all values from the \a other list.
- * Elements in the list whose control IDs are contained in \a other are updated
+ * Elements in the list whose control IDs are contained in \a other are merged
* with the value from \a other. Elements in the \a other list that have no
* corresponding element in the list are added to the list with their value.
*
- * The behaviour is undefined if the two lists refer to different Camera
- * instances.
+ * All controls in \a other should be supported by the list, otherwise no
+ * control is updated.
+ *
+ * The behaviour is undefined if the camera the list refers to is not valid
+ * anymore.
+ *
+ * \return True if control list have been merged, false otherwise
*/
-void ControlList::update(const ControlList &other)
+bool ControlList::merge(const ControlList &other)
{
/*
* Make sure all controls in the other list are supported by this
@@ -331,14 +386,16 @@ void ControlList::update(const ControlList &other)
if (infoMap_.find(id) == infoMap_.end()) {
LOG(Controls, Error)
- << "Failed to update control list with control: "
+ << "Failed to merge control list with control: "
<< id;
- return;
+ return false;
}
}
for (auto &control : other)
controls_[control.first] = control.second;
+
+ return true;
}
} /* namespace libcamera */
@@ -89,7 +89,7 @@ function GenerateTable(file) {
EnterNameSpace(file)
- print "extern const std::unordered_map<ControlId, ControlIdentifier>" > file
+ print "extern const std::unordered_map<ControlId, ControlMetadata>" > file
print "controlTypes {" > file
for (i=1; i <= id; ++i) {
printf "\t{ %s, { %s, \"%s\", DataType%s } },\n", names[i], names[i], names[i], types[i] > file
@@ -60,13 +60,13 @@ endif
gen_controls = files('gen-controls.awk')
-control_types_cpp = custom_target('control_types_cpp',
+control_metadata_cpp = custom_target('control_metadata_cpp',
input : files('controls.cpp'),
- output : 'control_types.cpp',
+ output : 'control_metadata.cpp',
depend_files : gen_controls,
command : [gen_controls, '@INPUT@', '--code', '@OUTPUT@'])
-libcamera_sources += control_types_cpp
+libcamera_sources += control_metadata_cpp
gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
@@ -231,7 +231,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
for (auto it : request->controls()) {
const ControlInfo *ci = it.first;
- DataValue &value = it.second;
+ ControlValue &value = it.second;
switch (ci->id()) {
case Brightness:
@@ -284,7 +284,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
for (auto it : request->controls()) {
const ControlInfo *ci = it.first;
- DataValue &value = it.second;
+ ControlValue &value = it.second;
switch (ci->id()) {
case Brightness:
@@ -159,7 +159,10 @@ protected:
newList[Brightness] = 128;
newList[Saturation] = 255;
- newList.update(list);
+ if (!newList.merge(list)) {
+ cout << "Failed to merge control lists" << endl;
+ return TestFail;
+ }
list.clear();
Now that the control data value and info storage have been unified with V4L2 control a refresh of naming and minor bits had to be performed. The control framework implementation uses the word 'control' in types and definition to refer to different things, making it harder to follow. Perform a rename of elements defined by the control framework to enforce the following concepts: - control metadata: static control metadata information such as id, type and name, defined by libcamera - control information: constant information associated with a control used for validation of control values, provided by pipeline handlers and defined by the capabilities of the device the control applies to - control: a control identifier with an associated value, provided by applications when setting a control to a specific value Update the framework documentation trying to use those concepts consistently. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/controls.h | 26 +++-- src/libcamera/controls.cpp | 165 +++++++++++++++++++--------- src/libcamera/gen-controls.awk | 2 +- src/libcamera/meson.build | 6 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- test/controls/control_list.cpp | 5 +- 7 files changed, 135 insertions(+), 73 deletions(-)