From patchwork Wed Sep 18 10:34:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 1986 X-Patchwork-Delegate: jacopo@jmondi.org Return-Path: Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AB99A60BB0 for ; Wed, 18 Sep 2019 12:32:51 +0200 (CEST) X-Originating-IP: 2.224.242.101 Received: from uno.lan (2-224-242-101.ip172.fastwebnet.it [2.224.242.101]) (Authenticated sender: jacopo@jmondi.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 5B1D3E0003 for ; Wed, 18 Sep 2019 10:32:51 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Wed, 18 Sep 2019 12:34:21 +0200 Message-Id: <20190918103424.14536-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20190918103424.14536-1-jacopo@jmondi.org> References: <20190918103424.14536-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/5] libcamera: controls: Use DataType and DataValue X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 18 Sep 2019 10:32:51 -0000 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 --- include/libcamera/controls.h | 77 ++------- src/libcamera/controls.cpp | 237 ++-------------------------- 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, 26 insertions(+), 368 deletions(-) delete mode 100644 test/controls/control_value.cpp -- 2.23.0 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 #include +#include 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; class ControlList { private: - using ControlListMap = std::unordered_map; + using ControlListMap = std::unordered_map; 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..2d8adde5688e 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 ""; - case ControlValueBool: - return bool_ ? "True" : "False"; - case ControlValueInteger: - return std::to_string(integer_); - case ControlValueInteger64: - return std::to_string(integer64_); - } - - return ""; -} - /** * \enum ControlId * \brief Numerical control ID @@ -269,9 +112,9 @@ extern const std::unordered_map 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()) { @@ -296,79 +139,23 @@ 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 + * \brief Retrieve the control designated type + * \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" > 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 - -#include - -#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