[libcamera-devel] [PATCH v2 13/13] libcamera: controls: Use ControlValidator to validate ControlList
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Sep 29 21:02:54 CEST 2019
Replace the manual validation of controls against a Camera with usage of
the new ControlValidator interface.
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
include/libcamera/controls.h | 6 +++---
include/libcamera/request.h | 7 ++++---
src/libcamera/controls.cpp | 27 ++++++++++-----------------
src/libcamera/request.cpp | 14 ++++++++++++--
test/controls/control_list.cpp | 15 ++++++++++++++-
5 files changed, 43 insertions(+), 26 deletions(-)
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index d3eea643c0ec..90426753f40f 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -13,7 +13,7 @@
namespace libcamera {
-class Camera;
+class ControlValidator;
enum ControlType {
ControlTypeNone,
@@ -119,7 +119,7 @@ private:
using ControlListMap = std::unordered_map<const ControlId *, ControlValue>;
public:
- ControlList(Camera *camera);
+ ControlList(ControlValidator *validator);
using iterator = ControlListMap::iterator;
using const_iterator = ControlListMap::const_iterator;
@@ -160,7 +160,7 @@ private:
const ControlValue *find(const ControlId &id) const;
ControlValue *find(const ControlId &id);
- Camera *camera_;
+ ControlValidator *validator_;
ControlListMap controls_;
};
diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 9edf1cedc054..e3db5243aaf3 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -19,9 +19,9 @@ namespace libcamera {
class Buffer;
class Camera;
+class CameraControlValidator;
class Stream;
-
class Request
{
public:
@@ -36,7 +36,7 @@ public:
Request &operator=(const Request &) = delete;
~Request();
- ControlList &controls() { return controls_; }
+ ControlList &controls() { return *controls_; }
const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
int addBuffer(std::unique_ptr<Buffer> buffer);
Buffer *findBuffer(Stream *stream) const;
@@ -56,7 +56,8 @@ private:
bool completeBuffer(Buffer *buffer);
Camera *camera_;
- ControlList controls_;
+ CameraControlValidator *validator_;
+ ControlList *controls_;
std::map<Stream *, Buffer *> bufferMap_;
std::unordered_set<Buffer *> pending_;
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index a7e9d069b31a..f3260edce0bc 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 ee2158fc7a9c..19f6d0b9a0ae 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_;
}
/**
diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
index 8469ecf09439..1bcfecc467b5 100644
--- a/test/controls/control_list.cpp
+++ b/test/controls/control_list.cpp
@@ -12,6 +12,7 @@
#include <libcamera/control_ids.h>
#include <libcamera/controls.h>
+#include "camera_controls.h"
#include "test.h"
using namespace std;
@@ -40,7 +41,8 @@ protected:
int run()
{
- ControlList list(camera_.get());
+ CameraControlValidator validator(camera_.get());
+ ControlList list(&validator);
/* Test that the list is initially empty. */
if (!list.empty()) {
@@ -141,6 +143,17 @@ protected:
return TestFail;
}
+ /*
+ * Attempt to set an invalid control and verify that the
+ * operation failed.
+ */
+ list.set(controls::AwbEnable, true);
+
+ if (list.contains(controls::AwbEnable)) {
+ cout << "List shouldn't contain AwbEnable control" << endl;
+ return TestFail;
+ }
+
return TestPass;
}
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list