libcamera: controls: Make ControlValue get/set accessors template methods

The ControlValue get accessors are implemented with functions of
different names, whlie the set accessors use polymorphism to support
different control types. This isn't very consistent and intuitive. Make
the API clearer by using template methods. This will also have the added
advantage that support for the new types will only require adding
template specialisations, without adding new methods.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

include/libcamera/controls.h

@@ -36,13 +36,10 @@ public:
 	ControlType type() const { return type_; };
 	bool isNone() const { return type_ == ControlTypeNone; };
 
-	void set(bool value);
+	template<typename T>
-	void set(int value);
+	const T &get() const;
-	void set(int64_t value);
+	template<typename T>
-
+	void set(const T &value);
-	bool getBool() const;
-	int getInt() const;
-	int64_t getInt64() const;
 
 	std::string toString() const;
 

src/libcamera/controls.cpp

@@ -90,77 +90,68 @@ ControlValue::ControlValue(int64_t value)
  */
 
 /**
- * \brief Set the value with a boolean
+ * \fn template<typename T> const T &ControlValue::get() const
- * \param[in] value Boolean value to store
+ * \brief Get the control value
- */
-void ControlValue::set(bool value)
-{
-	type_ = ControlTypeBool;
-	bool_ = value;
-}
-
-/**
- * \brief Set the value with an integer
- * \param[in] value Integer value to store
- */
-void ControlValue::set(int value)
-{
-	type_ = ControlTypeInteger;
-	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_ = ControlTypeInteger64;
-	integer64_ = value;
-}
-
-/**
- * \brief Get the boolean value
  *
- * The value type must be Boolean.
+ * The control value type shall match the type T, otherwise the behaviour is
+ * undefined.
  *
- * \return The boolean value
+ * \return The control value
  */
-bool ControlValue::getBool() const
+
+/**
+ * \fn template<typename T> void ControlValue::set(const T &value)
+ * \brief Set the control value to \a value
+ * \param[in] value The control value
+ */
+
+#ifndef __DOXYGEN__
+template<>
+const bool &ControlValue::get<bool>() const
 {
 	ASSERT(type_ == ControlTypeBool);
 
 	return bool_;
 }
 
-/**
+template<>
- * \brief Get the integer value
+const int &ControlValue::get<int>() const
- *
- * The value type must be Integer or Integer64.
- *
- * \return The integer value
- */
-int ControlValue::getInt() const
 {
 	ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
 
 	return integer_;
 }
 
-/**
+template<>
- * \brief Get the 64-bit integer value
+const int64_t &ControlValue::get<int64_t>() const
- *
- * The value type must be Integer or Integer64.
- *
- * \return The 64-bit integer value
- */
-int64_t ControlValue::getInt64() const
 {
 	ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
 
 	return integer64_;
 }
 
+template<>
+void ControlValue::set<bool>(const bool &value)
+{
+	type_ = ControlTypeBool;
+	bool_ = value;
+}
+
+template<>
+void ControlValue::set<int>(const int &value)
+{
+	type_ = ControlTypeInteger;
+	integer_ = value;
+}
+
+template<>
+void ControlValue::set<int64_t>(const int64_t &value)
+{
+	type_ = ControlTypeInteger64;
+	integer64_ = value;
+}
+#endif /* __DOXYGEN__ */
+
 /**
  * \brief Assemble and return a string describing the value
  * \return A string describing the ControlValue

src/libcamera/pipeline/uvcvideo.cpp

@@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
 
 		switch (ci->id()) {
 		case Brightness:
-			controls.add(V4L2_CID_BRIGHTNESS, value.getInt());
+			controls.add(V4L2_CID_BRIGHTNESS, value.get<int>());
 			break;
 
 		case Contrast:
-			controls.add(V4L2_CID_CONTRAST, value.getInt());
+			controls.add(V4L2_CID_CONTRAST, value.get<int>());
 			break;
 
 		case Saturation:
-			controls.add(V4L2_CID_SATURATION, value.getInt());
+			controls.add(V4L2_CID_SATURATION, value.get<int>());
 			break;
 
 		case ManualExposure:
 			controls.add(V4L2_CID_EXPOSURE_AUTO, 1);
-			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.getInt());
+			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int>());
 			break;
 
 		case ManualGain:
-			controls.add(V4L2_CID_GAIN, value.getInt());
+			controls.add(V4L2_CID_GAIN, value.get<int>());
 			break;
 
 		default:

src/libcamera/pipeline/vimc.cpp

@@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
 
 		switch (ci->id()) {
 		case Brightness:
-			controls.add(V4L2_CID_BRIGHTNESS, value.getInt());
+			controls.add(V4L2_CID_BRIGHTNESS, value.get<int>());
 			break;
 
 		case Contrast:
-			controls.add(V4L2_CID_CONTRAST, value.getInt());
+			controls.add(V4L2_CID_CONTRAST, value.get<int>());
 			break;
 
 		case Saturation:
-			controls.add(V4L2_CID_SATURATION, value.getInt());
+			controls.add(V4L2_CID_SATURATION, value.get<int>());
 			break;
 
 		default:

test/controls/control_info.cpp

@@ -32,7 +32,7 @@ protected:
 			return TestFail;
 		}
 
-		if (info.min().getInt() != 0 || info.max().getInt() != 0) {
+		if (info.min().get<int>() != 0 || info.max().get<int>() != 0) {
 			cout << "Invalid control range for Brightness" << endl;
 			return TestFail;
 		}
@@ -50,7 +50,7 @@ protected:
 			return TestFail;
 		}
 
-		if (info.min().getInt() != 10 || info.max().getInt() != 200) {
+		if (info.min().get<int>() != 10 || info.max().get<int>() != 200) {
 			cout << "Invalid control range for Contrast" << endl;
 			return TestFail;
 		}

test/controls/control_list.cpp

@@ -96,7 +96,7 @@ protected:
 			return TestFail;
 		}
 
-		if (list[Brightness].getInt() != 255) {
+		if (list[Brightness].get<int>() != 255) {
 			cout << "Incorrest Brightness control value" << endl;
 			return TestFail;
 		}
@@ -125,8 +125,8 @@ protected:
 		/*
 		 * Test control value retrieval and update through ControlInfo.
 		 */
-		if (list[brightness].getInt() != 64 ||
+		if (list[brightness].get<int>() != 64 ||
-		    list[contrast].getInt() != 128) {
+		    list[contrast].get<int>() != 128) {
 			cout << "Failed to retrieve control value" << endl;
 			return TestFail;
 		}
@@ -134,8 +134,8 @@ protected:
 		list[brightness] = 10;
 		list[contrast] = 20;
 
-		if (list[brightness].getInt() != 10 ||
+		if (list[brightness].get<int>() != 10 ||
-		    list[contrast].getInt() != 20) {
+		    list[contrast].get<int>() != 20) {
 			cout << "Failed to update control value" << endl;
 			return TestFail;
 		}
@@ -185,9 +185,9 @@ protected:
 			return TestFail;
 		}
 
-		if (newList[Brightness].getInt() != 10 ||
+		if (newList[Brightness].get<int>() != 10 ||
-		    newList[Contrast].getInt() != 20 ||
+		    newList[Contrast].get<int>() != 20 ||
-		    newList[Saturation].getInt() != 255) {
+		    newList[Saturation].get<int>() != 255) {
 			cout << "New list contains incorrect values" << endl;
 			return TestFail;
 		}

test/controls/control_value.cpp

@@ -27,12 +27,12 @@ protected:
 		     << " Bool: " << boolean.toString()
 		     << endl;
 
-		if (integer.getInt() != 1234) {
+		if (integer.get<int>() != 1234) {
 			cerr << "Failed to get Integer" << endl;
 			return TestFail;
 		}
 
-		if (boolean.getBool() != true) {
+		if (boolean.get<bool>() != true) {
 			cerr << "Failed to get Boolean" << endl;
 			return TestFail;
 		}
@@ -45,19 +45,19 @@ protected:
 			return TestFail;
 		}
 
-		value.set(true);
+		value.set<bool>(true);
 		if (value.isNone()) {
 			cerr << "Failed to set an empty object" << endl;
 			return TestFail;
 		}
 
-		if (value.getBool() != true) {
+		if (value.get<bool>() != true) {
 			cerr << "Failed to get Booleans" << endl;
 			return TestFail;
 		}
 
-		value.set(10);
+		value.set<int>(10);
-		if (value.getInt() != 10) {
+		if (value.get<int>() != 10) {
 			cerr << "Failed to get Integer" << endl;
 			return TestFail;
 		}