[libcamera-devel] [PATCH] libcamera: controls: Use template in the id-based interface

[Jacopo Mondi]

The ControlList Control<> and id based interfaces to set or get controls
differ in the way they return or receive values. The Control<> based
interface allows the usage of raw values, while the id-based interface
always goes through ControlValue, resulting in one additional function
call in the caller when accessing controls by numerical id.

Align the two implementations by providing access to raw values for the
id-based interface.

-       controls.get(V4L2_CID_CONTRAST).get<int32_t>()
+	controls.get<int32_t>(V4L2_CID_CONTRAST)

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 include/libcamera/controls.h       | 23 ++++++++++++++++++-
 src/libcamera/controls.cpp         | 37 +++++++++++++++++-------------
 test/ipa/ipa_wrappers_test.cpp     |  6 ++---
 test/v4l2_videodevice/controls.cpp | 12 +++++-----
 4 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index b1b73367e874..0b55359890e6 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -220,6 +220,18 @@ public:
 		return val->get<T>();
 	}
 
+	template<typename T>
+	const T &get(unsigned int id) const
+	{
+		const ControlValue *val = find(id);
+		if (!val) {
+			static T t(0);
+			return t;
+		}
+
+		return val->get<T>();
+	}
+
 	template<typename T>
 	void set(const Control<T> &ctrl, const T &value)
 	{
@@ -230,7 +242,16 @@ public:
 		val->set<T>(value);
 	}
 
-	const ControlValue &get(unsigned int id) const;
+	template<typename T>
+	void set(unsigned int id, const T &value)
+	{
+		ControlValue *val = find(id);
+		if (!val)
+			return;
+
+		val->set<T>(value);
+	}
+
 	void set(unsigned int id, const ControlValue &value);
 
 	const ControlInfoMap *infoMap() const { return infoMap_; }
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 7d8a0e97ee3a..78729d55ee29 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -726,6 +726,18 @@ bool ControlList::contains(unsigned int id) const
  * \return The control value
  */
 
+/**
+ * \fn template<typename T> const T &ControlList::get(unsigned int id) const
+ * \brief Get the value of control \a id
+ * \param[in] id The control numerical ID
+ *
+ * The behaviour is undefined if the control \a id is not present in the list.
+ * Use ControlList::contains() to test for the presence of a control in the
+ * list before retrieving its value.
+ *
+ * \return The control value
+ */
+
 /**
  * \fn template<typename T> void ControlList::set(const Control<T> &ctrl, const T &value)
  * \brief Set the control \a ctrl value to \a value
@@ -741,25 +753,18 @@ bool ControlList::contains(unsigned int id) const
  */
 
 /**
- * \brief Get the value of control \a id
- * \param[in] id The control numerical ID
+ * \fn template<typename T> void ControlList::set(unsigned int id, const T &value)
+ * \brief Set the value of control \a id to \a value
+ * \param[in] id The control ID
+ * \param[in] value The control value
  *
- * The behaviour is undefined if the control \a id is not present in the list.
- * Use ControlList::contains() to test for the presence of a control in the
- * list before retrieving its value.
+ * This method sets the value of a control in the control list. If the control
+ * is already present in the list, its value is updated, otherwise it is added
+ * to the list.
  *
- * \return The control value
+ * The behaviour is undefined if the control \a id is not supported by the
+ * object that the list refers to.
  */
-const ControlValue &ControlList::get(unsigned int id) const
-{
-	static ControlValue zero;
-
-	const ControlValue *val = find(id);
-	if (!val)
-		return zero;
-
-	return *val;
-}
 
 /**
  * \brief Set the value of control \a id to \a value
diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
index a1e34ad52317..11fea2ccc506 100644
--- a/test/ipa/ipa_wrappers_test.cpp
+++ b/test/ipa/ipa_wrappers_test.cpp
@@ -181,9 +181,9 @@ public:
 		}
 
 		const ControlList &controls = data.controls[0];
-		if (controls.get(V4L2_CID_BRIGHTNESS).get<int32_t>() != 10 ||
-		    controls.get(V4L2_CID_CONTRAST).get<int32_t>() != 20 ||
-		    controls.get(V4L2_CID_SATURATION).get<int32_t>() != 30) {
+		if (controls.get<int32_t>(V4L2_CID_BRIGHTNESS) != 10 ||
+		    controls.get<int32_t>(V4L2_CID_CONTRAST) != 20 ||
+		    controls.get<int32_t>(V4L2_CID_SATURATION) != 30) {
 			cerr << "processEvent(): Invalid controls" << endl;
 			return report(Op_processEvent, TestFail);
 		}
diff --git a/test/v4l2_videodevice/controls.cpp b/test/v4l2_videodevice/controls.cpp
index 42c653d4435a..e6ab1b4c11bc 100644
--- a/test/v4l2_videodevice/controls.cpp
+++ b/test/v4l2_videodevice/controls.cpp
@@ -57,9 +57,9 @@ protected:
 			return TestFail;
 		}
 
-		if (ctrls.get(V4L2_CID_BRIGHTNESS).get<int32_t>() == -1 ||
-		    ctrls.get(V4L2_CID_CONTRAST).get<int32_t>() == -1 ||
-		    ctrls.get(V4L2_CID_SATURATION).get<int32_t>() == -1) {
+		if (ctrls.get<int32_t>(V4L2_CID_BRIGHTNESS) == -1 ||
+		    ctrls.get<int32_t>(V4L2_CID_CONTRAST) == -1 ||
+		    ctrls.get<int32_t>(V4L2_CID_SATURATION) == -1) {
 			cerr << "Incorrect value for retrieved controls" << endl;
 			return TestFail;
 		}
@@ -86,9 +86,9 @@ protected:
 			return TestFail;
 		}
 
-		if (ctrls.get(V4L2_CID_BRIGHTNESS) != brightness.min() ||
-		    ctrls.get(V4L2_CID_CONTRAST) != contrast.max() ||
-		    ctrls.get(V4L2_CID_SATURATION) != saturation.min().get<int32_t>() + 1) {
+		if (ctrls.get<int32_t>(V4L2_CID_BRIGHTNESS) != brightness.min().get<int32_t>() ||
+		    ctrls.get<int32_t>(V4L2_CID_CONTRAST) != contrast.max().get<int32_t>() ||
+		    ctrls.get<int32_t>(V4L2_CID_SATURATION) != saturation.min().get<int32_t>() + 1) {
 			cerr << "Controls not updated when set" << endl;
 			return TestFail;
 		}
-- 
2.24.0