[libcamera-devel] [PATCH 4/5] libcamera: controls: De-couple Controls from Camera

[Jacopo Mondi]

ControlList requires a Camera class instance at construction time,
preventing it from being re-constructed from serialized binary data.

De-couple ControlList from Camera by internally storing a reference to
the Camera's ControlInfoList.

Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 include/libcamera/controls.h   |  4 +--
 src/libcamera/controls.cpp     | 54 +++++++++++++++++-----------------
 src/libcamera/request.cpp      |  4 +--
 test/controls/control_list.cpp |  4 +--
 4 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index e46cd8a78679..d3065c0f3f16 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -48,7 +48,7 @@ private:
 	using ControlListMap = std::unordered_map<const ControlInfo *, DataValue>;
 
 public:
-	ControlList(Camera *camera);
+	ControlList(const ControlInfoMap &infoMap);
 
 	using iterator = ControlListMap::iterator;
 	using const_iterator = ControlListMap::const_iterator;
@@ -70,7 +70,7 @@ public:
 	void update(const ControlList &list);
 
 private:
-	Camera *camera_;
+	const ControlInfoMap &infoMap_;
 	ControlListMap controls_;
 };
 
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 2d8adde5688e..184d544c05bc 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -175,10 +175,10 @@ std::string ControlInfo::toString() const
 
 /**
  * \brief Construct a ControlList with a reference to the Camera it applies on
- * \param[in] camera The camera
+ * \param[in] infoMap The ControlInfoMap of the camera the control list refers to
  */
-ControlList::ControlList(Camera *camera)
-	: camera_(camera)
+ControlList::ControlList(const ControlInfoMap &infoMap)
+	: infoMap_(infoMap)
 {
 }
 
@@ -229,17 +229,14 @@ ControlList::ControlList(Camera *camera)
  */
 bool ControlList::contains(ControlId id) const
 {
-	const ControlInfoMap &controls = camera_->controls();
-	const auto iter = controls.find(id);
-	if (iter == controls.end()) {
-		LOG(Controls, Error)
-			<< "Camera " << camera_->name()
-			<< " does not support control " << id;
+	const auto info = infoMap_.find(id);
+	if (info == infoMap_.end()) {
+		LOG(Controls, Error) << "Control " << id << " not supported";
 
 		return false;
 	}
 
-	return controls_.find(&iter->second) != controls_.end();
+	return controls_.find(&info->second) != controls_.end();
 }
 
 /**
@@ -283,18 +280,15 @@ bool ControlList::contains(const ControlInfo *info) const
  */
 DataValue &ControlList::operator[](ControlId id)
 {
-	const ControlInfoMap &controls = camera_->controls();
-	const auto iter = controls.find(id);
-	if (iter == controls.end()) {
-		LOG(Controls, Error)
-			<< "Camera " << camera_->name()
-			<< " does not support control " << id;
+	const auto info = infoMap_.find(id);
+	if (info == infoMap_.end()) {
+		LOG(Controls, Error) << "Control " << id << " not supported";
 
 		static DataValue empty;
 		return empty;
 	}
 
-	return controls_[&iter->second];
+	return controls_[&info->second];
 }
 
 /**
@@ -322,18 +316,24 @@ DataValue &ControlList::operator[](ControlId id)
  */
 void ControlList::update(const ControlList &other)
 {
-	if (other.camera_ != camera_) {
-		LOG(Controls, Error)
-			<< "Can't update ControlList from a different camera";
-		return;
+	/*
+	 * Make sure if all controls in other's list are supported by this
+	 * Camera. This is guaranteed to be true if the two lists refer to
+	 * the same Camera.
+	 */
+	for (auto &control : other) {
+		ControlId id = control.first->id();
+
+		if (infoMap_.find(id) == infoMap_.end()) {
+			LOG(Controls, Error)
+				<< "Failed to update control list with control: "
+				<< id;
+			return;
+		}
 	}
 
-	for (auto it : other) {
-		const ControlInfo *info = it.first;
-		const DataValue &value = it.second;
-
-		controls_[info] = value;
-	}
+	for (auto &control : other)
+		controls_[control.first] = control.second;
 }
 
 } /* namespace libcamera */
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index ee2158fc7a9c..2b3e1870094e 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -55,8 +55,8 @@ LOG_DEFINE_CATEGORY(Request)
  *
  */
 Request::Request(Camera *camera, uint64_t cookie)
-	: camera_(camera), controls_(camera), cookie_(cookie),
-	  status_(RequestPending), cancelled_(false)
+	: camera_(camera), controls_(camera->controls()),
+	  cookie_(cookie), status_(RequestPending), cancelled_(false)
 {
 }
 
diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
index f1d79ff8fcfd..3c6eb40c091b 100644
--- a/test/controls/control_list.cpp
+++ b/test/controls/control_list.cpp
@@ -39,7 +39,7 @@ protected:
 
 	int run()
 	{
-		ControlList list(camera_.get());
+		ControlList list(camera_->controls());
 
 		/* Test that the list is initially empty. */
 		if (!list.empty()) {
@@ -155,7 +155,7 @@ protected:
 		 * the old list. Verify that the new list is empty and that the
 		 * new list contains the expected items and values.
 		 */
-		ControlList newList(camera_.get());
+		ControlList newList(camera_->controls());
 
 		newList[Brightness] = 128;
 		newList[Saturation] = 255;
-- 
2.23.0