Message ID | 20190924171440.29758-3-jacopo@jmondi.org |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Jacopo, On 24/09/2019 18:14, Jacopo Mondi wrote: > Replace the use of ControlValue with DataValue and make ControlInfo a > DataInfo derived class to maximize the code reuse with V4L2Control which > will be modified in the next patch. > > Remove the now unused control_value test, replaced in the previous patch > by data_value test. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/controls.h | 77 ++------- > src/libcamera/controls.cpp | 235 ++-------------------------- > src/libcamera/gen-controls.awk | 2 +- > src/libcamera/pipeline/uvcvideo.cpp | 2 +- > src/libcamera/pipeline/vimc.cpp | 2 +- > test/controls/control_info.cpp | 4 +- > test/controls/control_value.cpp | 69 -------- > test/controls/meson.build | 1 - > 8 files changed, 25 insertions(+), 367 deletions(-) > delete mode 100644 test/controls/control_value.cpp > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index fbb3a62274c6..e46cd8a78679 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -13,98 +13,39 @@ > #include <unordered_map> > > #include <libcamera/control_ids.h> > +#include <libcamera/data_value.h> > > namespace libcamera { > > class Camera; > > -enum ControlValueType { > - ControlValueNone, > - ControlValueBool, > - ControlValueInteger, > - ControlValueInteger64, > -}; > - > -class ControlValue > -{ > -public: > - ControlValue(); > - ControlValue(bool value); > - ControlValue(int value); > - ControlValue(int64_t value); > - > - ControlValueType type() const { return type_; }; > - bool isNone() const { return type_ == ControlValueNone; }; > - > - void set(bool value); > - void set(int value); > - void set(int64_t value); > - > - bool getBool() const; > - int getInt() const; > - int64_t getInt64() const; > - > - std::string toString() const; > - > -private: > - ControlValueType type_; > - > - union { > - bool bool_; > - int integer_; > - int64_t integer64_; > - }; > -}; > - > struct ControlIdentifier { > ControlId id; > const char *name; > - ControlValueType type; > + DataType type; > }; > > -class ControlInfo > +class ControlInfo : public DataInfo > { > public: > - explicit ControlInfo(ControlId id, const ControlValue &min = 0, > - const ControlValue &max = 0); > - > + 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; } > - ControlValueType type() const { return ident_->type; } > - > - const ControlValue &min() const { return min_; } > - const ControlValue &max() const { return max_; } > + DataType type() const { return ident_->type; } > > std::string toString() const; > > private: > const struct ControlIdentifier *ident_; > - 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); I believe at least one of these was to allow the direct comparison between a ControlInfo and a ControlId so that ControlLists can be indexed by ControlInfo or ControlId with the operator[]s. Clearly we did not add a test to support this yet, or the removal of these should have broken something from within the control_list tests. I'll let you and Laurent decide whether it should be kept. I'm not particularly attached to this otherwise. -- Kieran > -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>; > > class ControlList > { > private: > - using ControlListMap = std::unordered_map<const ControlInfo *, ControlValue>; > + using ControlListMap = std::unordered_map<const ControlInfo *, DataValue>; > > public: > ControlList(Camera *camera); > @@ -123,8 +64,8 @@ public: > std::size_t size() const { return controls_.size(); } > void clear() { controls_.clear(); } > > - ControlValue &operator[](ControlId id); > - ControlValue &operator[](const ControlInfo *info) { return controls_[info]; } > + DataValue &operator[](ControlId id); > + DataValue &operator[](const ControlInfo *info) { return controls_[info]; } > > void update(const ControlList &list); > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 727fdbd9450d..154d9512f660 100644 > --- a/src/libcamera/controls.cpp > +++ b/src/libcamera/controls.cpp > @@ -24,163 +24,6 @@ namespace libcamera { > > LOG_DEFINE_CATEGORY(Controls) > > -/** > - * \enum ControlValueType > - * \brief Define the data type of value represented by a ControlValue > - * \var ControlValueNone > - * Identifies an unset control value > - * \var ControlValueBool > - * Identifies controls storing a boolean value > - * \var ControlValueInteger > - * Identifies controls storing an integer value > - * \var ControlValueInteger64 > - * Identifies controls storing a 64-bit integer value > - */ > - > -/** > - * \class ControlValue > - * \brief Abstract type representing the value of a control > - */ > - > -/** > - * \brief Construct an empty ControlValue. > - */ > -ControlValue::ControlValue() > - : type_(ControlValueNone) > -{ > -} > - > -/** > - * \brief Construct a Boolean ControlValue > - * \param[in] value Boolean value to store > - */ > -ControlValue::ControlValue(bool value) > - : type_(ControlValueBool), bool_(value) > -{ > -} > - > -/** > - * \brief Construct an integer ControlValue > - * \param[in] value Integer value to store > - */ > -ControlValue::ControlValue(int value) > - : type_(ControlValueInteger), integer_(value) > -{ > -} > - > -/** > - * \brief Construct a 64 bit integer ControlValue > - * \param[in] value Integer value to store > - */ > -ControlValue::ControlValue(int64_t value) > - : type_(ControlValueInteger64), integer64_(value) > -{ > -} > - > -/** > - * \fn ControlValue::type() > - * \brief Retrieve the data type of the value > - * \return The value data type > - */ > - > -/** > - * \fn ControlValue::isNone() > - * \brief Determine if the value is not initialised > - * \return True if the value type is ControlValueNone, false otherwise > - */ > - > -/** > - * \brief Set the value with a boolean > - * \param[in] value Boolean value to store > - */ > -void ControlValue::set(bool value) > -{ > - type_ = ControlValueBool; > - bool_ = value; > -} > - > -/** > - * \brief Set the value with an integer > - * \param[in] value Integer value to store > - */ > -void ControlValue::set(int value) > -{ > - type_ = ControlValueInteger; > - integer_ = value; > -} > - > -/** > - * \brief Set the value with a 64 bit integer > - * \param[in] value 64 bit integer value to store > - */ > -void ControlValue::set(int64_t value) > -{ > - type_ = ControlValueInteger64; > - integer64_ = value; > -} > - > -/** > - * \brief Get the boolean value > - * > - * The value type must be Boolean. > - * > - * \return The boolean value > - */ > -bool ControlValue::getBool() const > -{ > - ASSERT(type_ == ControlValueBool); > - > - return bool_; > -} > - > -/** > - * \brief Get the integer value > - * > - * The value type must be Integer or Integer64. > - * > - * \return The integer value > - */ > -int ControlValue::getInt() const > -{ > - ASSERT(type_ == ControlValueInteger || type_ == ControlValueInteger64); > - > - return integer_; > -} > - > -/** > - * \brief Get the 64-bit integer value > - * > - * The value type must be Integer or Integer64. > - * > - * \return The 64-bit integer value > - */ > -int64_t ControlValue::getInt64() const > -{ > - ASSERT(type_ == ControlValueInteger || type_ == ControlValueInteger64); > - > - return integer64_; > -} > - > -/** > - * \brief Assemble and return a string describing the value > - * \return A string describing the ControlValue > - */ > -std::string ControlValue::toString() const > -{ > - switch (type_) { > - case ControlValueNone: > - return "<None>"; > - case ControlValueBool: > - return bool_ ? "True" : "False"; > - case ControlValueInteger: > - return std::to_string(integer_); > - case ControlValueInteger64: > - return std::to_string(integer64_); > - } > - > - return "<ValueType Error>"; > -} > - > /** > * \enum ControlId > * \brief Numerical control ID > @@ -269,9 +112,9 @@ 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, > - const ControlValue &max) > - : min_(min), max_(max) > +ControlInfo::ControlInfo(ControlId id, const DataValue &min, > + const DataValue &max) > + : DataInfo(min, max) > { > auto iter = controlTypes.find(id); > if (iter == controlTypes.end()) { > @@ -297,78 +140,22 @@ ControlInfo::ControlInfo(ControlId id, const ControlValue &min, > /** > * \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 > - * \return A ControlValue with the minimum value for the control > - */ > - > -/** > - * \fn ControlInfo::max() > - * \brief Retrieve the maximum value of the control > - * \return A ControlValue with the maximum value for the control > + * \return The control type > */ > > /** > * \brief Provide a string representation of the ControlInfo > + * \return The control name > */ > std::string ControlInfo::toString() const > { > std::stringstream ss; > > - ss << name() << "[" << min_.toString() << ".." << max_.toString() << "]"; > + ss << 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 > @@ -378,7 +165,7 @@ bool operator==(const ControlInfo &lhs, const ControlId &rhs) > * \class ControlList > * \brief Associate a list of ControlId with their values for a camera > * > - * A ControlList wraps a map of ControlId to ControlValue and provide > + * A ControlList wraps a map of ControlId to DataValue and provide > * additional validation against the control information exposed by a Camera. > * > * A list is only valid for as long as the camera it refers to is valid. After > @@ -474,7 +261,7 @@ bool ControlList::contains(const ControlInfo *info) const > /** > * \fn ControlList::size() > * \brief Retrieve the number of controls in the list > - * \return The number of Control entries stored in the list > + * \return The number of DataValue entries stored in the list > */ > > /** > @@ -494,7 +281,7 @@ bool ControlList::contains(const ControlInfo *info) const > * > * \return A reference to the value of the control identified by \a id > */ > -ControlValue &ControlList::operator[](ControlId id) > +DataValue &ControlList::operator[](ControlId id) > { > const ControlInfoMap &controls = camera_->controls(); > const auto iter = controls.find(id); > @@ -503,7 +290,7 @@ ControlValue &ControlList::operator[](ControlId id) > << "Camera " << camera_->name() > << " does not support control " << id; > > - static ControlValue empty; > + static DataValue empty; > return empty; > } > > @@ -543,7 +330,7 @@ void ControlList::update(const ControlList &other) > > for (auto it : other) { > const ControlInfo *info = it.first; > - const ControlValue &value = it.second; > + const DataValue &value = it.second; > > controls_[info] = value; > } > diff --git a/src/libcamera/gen-controls.awk b/src/libcamera/gen-controls.awk > index f3d068123012..abeb0218546b 100755 > --- a/src/libcamera/gen-controls.awk > +++ b/src/libcamera/gen-controls.awk > @@ -92,7 +92,7 @@ function GenerateTable(file) { > print "extern const std::unordered_map<ControlId, ControlIdentifier>" > file > print "controlTypes {" > file > for (i=1; i <= id; ++i) { > - printf "\t{ %s, { %s, \"%s\", ControlValue%s } },\n", names[i], names[i], names[i], types[i] > file > + printf "\t{ %s, { %s, \"%s\", DataType%s } },\n", names[i], names[i], names[i], types[i] > file > } > print "};" > file > ExitNameSpace(file) > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 8965210550d2..dd253f94ca3d 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; > - ControlValue &value = it.second; > + DataValue &value = it.second; > > switch (ci->id()) { > case Brightness: > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index f26a91f86ec1..3df239bdb277 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; > - ControlValue &value = it.second; > + DataValue &value = it.second; > > switch (ci->id()) { > case Brightness: > diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp > index aa3a65b1e5ef..c3f9b85580a7 100644 > --- a/test/controls/control_info.cpp > +++ b/test/controls/control_info.cpp > @@ -26,7 +26,7 @@ protected: > ControlInfo info(Brightness); > > if (info.id() != Brightness || > - info.type() != ControlValueInteger || > + info.type() != DataTypeInteger || > info.name() != std::string("Brightness")) { > cout << "Invalid control identification for Brightness" << endl; > return TestFail; > @@ -44,7 +44,7 @@ protected: > info = ControlInfo(Contrast, 10, 200); > > if (info.id() != Contrast || > - info.type() != ControlValueInteger || > + info.type() != DataTypeInteger || > info.name() != std::string("Contrast")) { > cout << "Invalid control identification for Contrast" << endl; > return TestFail; > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp > deleted file mode 100644 > index 778efe5c115f..000000000000 > --- a/test/controls/control_value.cpp > +++ /dev/null > @@ -1,69 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0-or-later */ > -/* > - * Copyright (C) 2019, Google Inc. > - * > - * control_value.cpp - ControlValue tests > - */ > - > -#include <iostream> > - > -#include <libcamera/controls.h> > - > -#include "test.h" > - > -using namespace std; > -using namespace libcamera; > - > -class ControlValueTest : public Test > -{ > -protected: > - int run() > - { > - ControlValue integer(1234); > - ControlValue boolean(true); > - > - /* Just a string conversion output test... no validation */ > - cout << "Int: " << integer.toString() > - << " Bool: " << boolean.toString() > - << endl; > - > - if (integer.getInt() != 1234) { > - cerr << "Failed to get Integer" << endl; > - return TestFail; > - } > - > - if (boolean.getBool() != true) { > - cerr << "Failed to get Boolean" << endl; > - return TestFail; > - } > - > - /* Test an uninitialised value, and updating it. */ > - > - ControlValue value; > - if (!value.isNone()) { > - cerr << "Empty value is non-null" << endl; > - return TestFail; > - } > - > - value.set(true); > - if (value.isNone()) { > - cerr << "Failed to set an empty object" << endl; > - return TestFail; > - } > - > - if (value.getBool() != true) { > - cerr << "Failed to get Booleans" << endl; > - return TestFail; > - } > - > - value.set(10); > - if (value.getInt() != 10) { > - cerr << "Failed to get Integer" << endl; > - return TestFail; > - } > - > - return TestPass; > - } > -}; > - > -TEST_REGISTER(ControlValueTest) > diff --git a/test/controls/meson.build b/test/controls/meson.build > index f4fc7b947dd9..9070be85718b 100644 > --- a/test/controls/meson.build > +++ b/test/controls/meson.build > @@ -1,7 +1,6 @@ > control_tests = [ > [ 'control_info', 'control_info.cpp' ], > [ 'control_list', 'control_list.cpp' ], > - [ 'control_value', 'control_value.cpp' ], > ] > > foreach t : control_tests >
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index fbb3a62274c6..e46cd8a78679 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -13,98 +13,39 @@ #include <unordered_map> #include <libcamera/control_ids.h> +#include <libcamera/data_value.h> namespace libcamera { class Camera; -enum ControlValueType { - ControlValueNone, - ControlValueBool, - ControlValueInteger, - ControlValueInteger64, -}; - -class ControlValue -{ -public: - ControlValue(); - ControlValue(bool value); - ControlValue(int value); - ControlValue(int64_t value); - - ControlValueType type() const { return type_; }; - bool isNone() const { return type_ == ControlValueNone; }; - - void set(bool value); - void set(int value); - void set(int64_t value); - - bool getBool() const; - int getInt() const; - int64_t getInt64() const; - - std::string toString() const; - -private: - ControlValueType type_; - - union { - bool bool_; - int integer_; - int64_t integer64_; - }; -}; - struct ControlIdentifier { ControlId id; const char *name; - ControlValueType type; + DataType type; }; -class ControlInfo +class ControlInfo : public DataInfo { public: - explicit ControlInfo(ControlId id, const ControlValue &min = 0, - const ControlValue &max = 0); - + 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; } - ControlValueType type() const { return ident_->type; } - - const ControlValue &min() const { return min_; } - const ControlValue &max() const { return max_; } + DataType type() const { return ident_->type; } std::string toString() const; private: const struct ControlIdentifier *ident_; - 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>; class ControlList { private: - using ControlListMap = std::unordered_map<const ControlInfo *, ControlValue>; + using ControlListMap = std::unordered_map<const ControlInfo *, DataValue>; public: ControlList(Camera *camera); @@ -123,8 +64,8 @@ public: std::size_t size() const { return controls_.size(); } void clear() { controls_.clear(); } - ControlValue &operator[](ControlId id); - ControlValue &operator[](const ControlInfo *info) { return controls_[info]; } + DataValue &operator[](ControlId id); + DataValue &operator[](const ControlInfo *info) { return controls_[info]; } void update(const ControlList &list); diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 727fdbd9450d..154d9512f660 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -24,163 +24,6 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Controls) -/** - * \enum ControlValueType - * \brief Define the data type of value represented by a ControlValue - * \var ControlValueNone - * Identifies an unset control value - * \var ControlValueBool - * Identifies controls storing a boolean value - * \var ControlValueInteger - * Identifies controls storing an integer value - * \var ControlValueInteger64 - * Identifies controls storing a 64-bit integer value - */ - -/** - * \class ControlValue - * \brief Abstract type representing the value of a control - */ - -/** - * \brief Construct an empty ControlValue. - */ -ControlValue::ControlValue() - : type_(ControlValueNone) -{ -} - -/** - * \brief Construct a Boolean ControlValue - * \param[in] value Boolean value to store - */ -ControlValue::ControlValue(bool value) - : type_(ControlValueBool), bool_(value) -{ -} - -/** - * \brief Construct an integer ControlValue - * \param[in] value Integer value to store - */ -ControlValue::ControlValue(int value) - : type_(ControlValueInteger), integer_(value) -{ -} - -/** - * \brief Construct a 64 bit integer ControlValue - * \param[in] value Integer value to store - */ -ControlValue::ControlValue(int64_t value) - : type_(ControlValueInteger64), integer64_(value) -{ -} - -/** - * \fn ControlValue::type() - * \brief Retrieve the data type of the value - * \return The value data type - */ - -/** - * \fn ControlValue::isNone() - * \brief Determine if the value is not initialised - * \return True if the value type is ControlValueNone, false otherwise - */ - -/** - * \brief Set the value with a boolean - * \param[in] value Boolean value to store - */ -void ControlValue::set(bool value) -{ - type_ = ControlValueBool; - bool_ = value; -} - -/** - * \brief Set the value with an integer - * \param[in] value Integer value to store - */ -void ControlValue::set(int value) -{ - type_ = ControlValueInteger; - integer_ = value; -} - -/** - * \brief Set the value with a 64 bit integer - * \param[in] value 64 bit integer value to store - */ -void ControlValue::set(int64_t value) -{ - type_ = ControlValueInteger64; - integer64_ = value; -} - -/** - * \brief Get the boolean value - * - * The value type must be Boolean. - * - * \return The boolean value - */ -bool ControlValue::getBool() const -{ - ASSERT(type_ == ControlValueBool); - - return bool_; -} - -/** - * \brief Get the integer value - * - * The value type must be Integer or Integer64. - * - * \return The integer value - */ -int ControlValue::getInt() const -{ - ASSERT(type_ == ControlValueInteger || type_ == ControlValueInteger64); - - return integer_; -} - -/** - * \brief Get the 64-bit integer value - * - * The value type must be Integer or Integer64. - * - * \return The 64-bit integer value - */ -int64_t ControlValue::getInt64() const -{ - ASSERT(type_ == ControlValueInteger || type_ == ControlValueInteger64); - - return integer64_; -} - -/** - * \brief Assemble and return a string describing the value - * \return A string describing the ControlValue - */ -std::string ControlValue::toString() const -{ - switch (type_) { - case ControlValueNone: - return "<None>"; - case ControlValueBool: - return bool_ ? "True" : "False"; - case ControlValueInteger: - return std::to_string(integer_); - case ControlValueInteger64: - return std::to_string(integer64_); - } - - return "<ValueType Error>"; -} - /** * \enum ControlId * \brief Numerical control ID @@ -269,9 +112,9 @@ 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, - const ControlValue &max) - : min_(min), max_(max) +ControlInfo::ControlInfo(ControlId id, const DataValue &min, + const DataValue &max) + : DataInfo(min, max) { auto iter = controlTypes.find(id); if (iter == controlTypes.end()) { @@ -297,78 +140,22 @@ ControlInfo::ControlInfo(ControlId id, const ControlValue &min, /** * \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 - * \return A ControlValue with the minimum value for the control - */ - -/** - * \fn ControlInfo::max() - * \brief Retrieve the maximum value of the control - * \return A ControlValue with the maximum value for the control + * \return The control type */ /** * \brief Provide a string representation of the ControlInfo + * \return The control name */ std::string ControlInfo::toString() const { std::stringstream ss; - ss << name() << "[" << min_.toString() << ".." << max_.toString() << "]"; + ss << 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 @@ -378,7 +165,7 @@ bool operator==(const ControlInfo &lhs, const ControlId &rhs) * \class ControlList * \brief Associate a list of ControlId with their values for a camera * - * A ControlList wraps a map of ControlId to ControlValue and provide + * A ControlList wraps a map of ControlId to DataValue and provide * additional validation against the control information exposed by a Camera. * * A list is only valid for as long as the camera it refers to is valid. After @@ -474,7 +261,7 @@ bool ControlList::contains(const ControlInfo *info) const /** * \fn ControlList::size() * \brief Retrieve the number of controls in the list - * \return The number of Control entries stored in the list + * \return The number of DataValue entries stored in the list */ /** @@ -494,7 +281,7 @@ bool ControlList::contains(const ControlInfo *info) const * * \return A reference to the value of the control identified by \a id */ -ControlValue &ControlList::operator[](ControlId id) +DataValue &ControlList::operator[](ControlId id) { const ControlInfoMap &controls = camera_->controls(); const auto iter = controls.find(id); @@ -503,7 +290,7 @@ ControlValue &ControlList::operator[](ControlId id) << "Camera " << camera_->name() << " does not support control " << id; - static ControlValue empty; + static DataValue empty; return empty; } @@ -543,7 +330,7 @@ void ControlList::update(const ControlList &other) for (auto it : other) { const ControlInfo *info = it.first; - const ControlValue &value = it.second; + const DataValue &value = it.second; controls_[info] = value; } diff --git a/src/libcamera/gen-controls.awk b/src/libcamera/gen-controls.awk index f3d068123012..abeb0218546b 100755 --- a/src/libcamera/gen-controls.awk +++ b/src/libcamera/gen-controls.awk @@ -92,7 +92,7 @@ function GenerateTable(file) { print "extern const std::unordered_map<ControlId, ControlIdentifier>" > file print "controlTypes {" > file for (i=1; i <= id; ++i) { - printf "\t{ %s, { %s, \"%s\", ControlValue%s } },\n", names[i], names[i], names[i], types[i] > file + printf "\t{ %s, { %s, \"%s\", DataType%s } },\n", names[i], names[i], names[i], types[i] > file } print "};" > file ExitNameSpace(file) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 8965210550d2..dd253f94ca3d 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; - ControlValue &value = it.second; + DataValue &value = it.second; switch (ci->id()) { case Brightness: diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index f26a91f86ec1..3df239bdb277 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; - ControlValue &value = it.second; + DataValue &value = it.second; switch (ci->id()) { case Brightness: diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp index aa3a65b1e5ef..c3f9b85580a7 100644 --- a/test/controls/control_info.cpp +++ b/test/controls/control_info.cpp @@ -26,7 +26,7 @@ protected: ControlInfo info(Brightness); if (info.id() != Brightness || - info.type() != ControlValueInteger || + info.type() != DataTypeInteger || info.name() != std::string("Brightness")) { cout << "Invalid control identification for Brightness" << endl; return TestFail; @@ -44,7 +44,7 @@ protected: info = ControlInfo(Contrast, 10, 200); if (info.id() != Contrast || - info.type() != ControlValueInteger || + info.type() != DataTypeInteger || info.name() != std::string("Contrast")) { cout << "Invalid control identification for Contrast" << endl; return TestFail; diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp deleted file mode 100644 index 778efe5c115f..000000000000 --- a/test/controls/control_value.cpp +++ /dev/null @@ -1,69 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * control_value.cpp - ControlValue tests - */ - -#include <iostream> - -#include <libcamera/controls.h> - -#include "test.h" - -using namespace std; -using namespace libcamera; - -class ControlValueTest : public Test -{ -protected: - int run() - { - ControlValue integer(1234); - ControlValue boolean(true); - - /* Just a string conversion output test... no validation */ - cout << "Int: " << integer.toString() - << " Bool: " << boolean.toString() - << endl; - - if (integer.getInt() != 1234) { - cerr << "Failed to get Integer" << endl; - return TestFail; - } - - if (boolean.getBool() != true) { - cerr << "Failed to get Boolean" << endl; - return TestFail; - } - - /* Test an uninitialised value, and updating it. */ - - ControlValue value; - if (!value.isNone()) { - cerr << "Empty value is non-null" << endl; - return TestFail; - } - - value.set(true); - if (value.isNone()) { - cerr << "Failed to set an empty object" << endl; - return TestFail; - } - - if (value.getBool() != true) { - cerr << "Failed to get Booleans" << endl; - return TestFail; - } - - value.set(10); - if (value.getInt() != 10) { - cerr << "Failed to get Integer" << endl; - return TestFail; - } - - return TestPass; - } -}; - -TEST_REGISTER(ControlValueTest) diff --git a/test/controls/meson.build b/test/controls/meson.build index f4fc7b947dd9..9070be85718b 100644 --- a/test/controls/meson.build +++ b/test/controls/meson.build @@ -1,7 +1,6 @@ control_tests = [ [ 'control_info', 'control_info.cpp' ], [ 'control_list', 'control_list.cpp' ], - [ 'control_value', 'control_value.cpp' ], ] foreach t : control_tests
Replace the use of ControlValue with DataValue and make ControlInfo a DataInfo derived class to maximize the code reuse with V4L2Control which will be modified in the next patch. Remove the now unused control_value test, replaced in the previous patch by data_value test. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/controls.h | 77 ++------- src/libcamera/controls.cpp | 235 ++-------------------------- src/libcamera/gen-controls.awk | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- test/controls/control_info.cpp | 4 +- test/controls/control_value.cpp | 69 -------- test/controls/meson.build | 1 - 8 files changed, 25 insertions(+), 367 deletions(-) delete mode 100644 test/controls/control_value.cpp