diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index d3065c0f3f16..c299ed94d9e2 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -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; +using ControlValue = DataValue; + class ControlList { private: - using ControlListMap = std::unordered_map; + using ControlMap = std::unordered_map; 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 */ diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 3c2a8dc50a16..65001259a3df 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -7,9 +7,6 @@ #include -#include -#include - #include #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 controlTypes; +extern const std::unordered_map 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 */ diff --git a/src/libcamera/gen-controls.awk b/src/libcamera/gen-controls.awk index abeb0218546b..b7f6532dfd43 100755 --- a/src/libcamera/gen-controls.awk +++ b/src/libcamera/gen-controls.awk @@ -89,7 +89,7 @@ function GenerateTable(file) { EnterNameSpace(file) - print "extern const std::unordered_map" > file + print "extern const std::unordered_map" > 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 diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index c3100a1709e0..c4fcd0569bd7 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -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') diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index dd253f94ca3d..8965210550d2 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -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: diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 3df239bdb277..f26a91f86ec1 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -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: diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 3c6eb40c091b..f04f441c018a 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -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();