[libcamera-devel,2/5] libcamera: controls: Use DataType and DataValue

Message ID 20190918103133.14296-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Control framework backend rework
Related show

Commit Message

Jacopo Mondi Sept. 18, 2019, 10:31 a.m. UTC
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          | 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

Patch

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..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 "<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()) {
@@ -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<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