[libcamera-devel] [PATCH v2 2/5] libcamera: controls: Use DataType and DataValue
Jacopo Mondi
jacopo at jmondi.org
Tue Sep 24 19:14:37 CEST 2019
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 at 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);
-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
--
2.23.0
More information about the libcamera-devel
mailing list