summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLaurent Pinchart <laurent.pinchart@ideasonboard.com>2019-09-28 02:45:49 +0300
committerLaurent Pinchart <laurent.pinchart@ideasonboard.com>2019-10-05 20:02:51 +0300
commitecf1c2e57b357f1b843796fd9ac4c77da940a26a (patch)
treec955705f0641f26b9e53749f40756509cb7e2d16
parentf671d84ceb491fdb07ad39d1fe20950fd6482e4f (diff)
libcamera: controls: Use ControlValidator to validate ControlList
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>
-rw-r--r--include/libcamera/controls.h6
-rw-r--r--include/libcamera/request.h7
-rw-r--r--src/libcamera/controls.cpp27
-rw-r--r--src/libcamera/request.cpp14
-rw-r--r--test/controls/control_list.cpp15
5 files changed, 43 insertions, 26 deletions
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index d3eea643..90426753 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 9edf1ced..e3db5243 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 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_;
}
/**
diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
index 8469ecf0..1bcfecc4 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;
}