From ecf1c2e57b357f1b843796fd9ac4c77da940a26a Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Sat, 28 Sep 2019 02:45:49 +0300
Subject: libcamera: controls: Use ControlValidator to validate ControlList
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace the manual validation of controls against a Camera with usage of
the new ControlValidator interface.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/controls.cpp | 27 ++++++++++-----------------
 src/libcamera/request.cpp  | 14 ++++++++++++--
 2 files changed, 22 insertions(+), 19 deletions(-)

(limited to 'src')

diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index a7e9d069..f3260edc 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -10,8 +10,7 @@
 #include <sstream>
 #include <string>
 
-#include <libcamera/camera.h>
-
+#include "control_validator.h"
 #include "log.h"
 #include "utils.h"
 
@@ -365,20 +364,16 @@ std::string ControlRange::toString() const
  * \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
- * 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
- * that calling any method of the ControlList class other than its destructor
- * will cause undefined behaviour.
+ * A ControlList wraps a map of ControlId to ControlValue and optionally
+ * validates controls against a ControlValidator.
  */
 
 /**
- * \brief Construct a ControlList with a reference to the Camera it applies on
- * \param[in] camera The camera
+ * \brief Construct a ControlList with an optional control validator
+ * \param[in] validator The validator (may be null)
  */
-ControlList::ControlList(Camera *camera)
-	: camera_(camera)
+ControlList::ControlList(ControlValidator *validator)
+	: validator_(validator)
 {
 }
 
@@ -493,12 +488,10 @@ const ControlValue *ControlList::find(const ControlId &id) const
 
 ControlValue *ControlList::find(const ControlId &id)
 {
-	const ControlInfoMap &controls = camera_->controls();
-	const auto iter = controls.find(&id);
-	if (iter == controls.end()) {
+	if (validator_ && !validator_->validate(id)) {
 		LOG(Controls, Error)
-			<< "Camera " << camera_->name()
-			<< " does not support control " << id.name();
+			<< "Control " << id.name()
+			<< " is not valid for " << validator_->name();
 		return nullptr;
 	}
 
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index ee2158fc..19f6d0b9 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -13,6 +13,7 @@
 #include <libcamera/camera.h>
 #include <libcamera/stream.h>
 
+#include "camera_controls.h"
 #include "log.h"
 
 /**
@@ -55,9 +56,15 @@ LOG_DEFINE_CATEGORY(Request)
  *
  */
 Request::Request(Camera *camera, uint64_t cookie)
-	: camera_(camera), controls_(camera), cookie_(cookie),
-	  status_(RequestPending), cancelled_(false)
+	: camera_(camera), cookie_(cookie), status_(RequestPending),
+	  cancelled_(false)
 {
+	/**
+	 * \todo Should the Camera expose a validator instance, to avoid
+	 * creating a new instance for each request?
+	 */
+	validator_ = new CameraControlValidator(camera);
+	controls_ = new ControlList(validator_);
 }
 
 Request::~Request()
@@ -66,6 +73,9 @@ Request::~Request()
 		Buffer *buffer = it.second;
 		delete buffer;
 	}
+
+	delete controls_;
+	delete validator_;
 }
 
 /**
-- 
cgit v1.2.1