Patch Detail
Show a patch.
GET /api/patches/2168/?format=api
{ "id": 2168, "url": "https://patchwork.libcamera.org/api/patches/2168/?format=api", "web_url": "https://patchwork.libcamera.org/patch/2168/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20191012184407.31684-8-laurent.pinchart@ideasonboard.com>", "date": "2019-10-12T18:44:00", "name": "[libcamera-devel,v2,07/14] libcamera: controls: Support accessing controls by numerical ID", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "2fcf851af76acb69f2b9c0879b233abcd3d27e5e", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/2168/mbox/", "series": [ { "id": 531, "url": "https://patchwork.libcamera.org/api/series/531/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=531", "date": "2019-10-12T18:43:53", "name": "Use ControlList for both libcamera and V4L2 controls", "version": 2, "mbox": "https://patchwork.libcamera.org/series/531/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/2168/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/2168/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E54161978\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 12 Oct 2019 20:44:21 +0200 (CEST)", "from pendragon.bb.dnainternet.fi\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9728B9C0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 12 Oct 2019 20:44:20 +0200 (CEST)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570905860;\n\tbh=d0p6DvIdtTbSRMT48oSbkp/Qaj5k5oL64j2QfRmA5RQ=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=cAD3EgUjHiE4hUWB7OfDyKNG65EyX0ZQ4D5/JL2SyQF7b58+mNT90qNmfojvM6vQb\n\tgDCZzhhTUt1ose6X5beR9vGDMMotkHHH4L5bcH/CFv+F2hmCvDuXb6c9fFEERjq25P\n\t9h+sQae2T20/9MARK03XTbWdiH0kSox2hVmix0ZA=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 12 Oct 2019 21:44:00 +0300", "Message-Id": "<20191012184407.31684-8-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.21.0", "In-Reply-To": "<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>", "References": "<20191012184407.31684-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 07/14] libcamera: controls: Support\n\taccessing controls by numerical ID", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Sat, 12 Oct 2019 18:44:21 -0000" }, "content": "The ControlList class has template get() and set() methods to get and\nset control values. The methods require a reference to a Control\ninstance, which is only available when calling them with a hardcoded\ncontrol. In order to support usage of ControlList for V4L2 controls, as\nwell as serialisation and deserialisation of ControlList, we need a way\nto get and set control values based on a control numerical ID. Add new\ncontains(), get() and set() overload methods to do so.\n\nAs this change prepares the ControlList to be used for other objects\nthan camera, update its documentation accordingly.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/controls.h | 10 ++-\n src/ipa/rkisp1/rkisp1.cpp | 2 +-\n src/libcamera/controls.cpp | 143 ++++++++++++++++++++++++++-------\n src/libcamera/request.cpp | 5 +-\n test/controls/control_list.cpp | 2 +-\n 5 files changed, 125 insertions(+), 37 deletions(-)", "diff": "diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex 999fcf7a3a62..5e6708fe570b 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -126,7 +126,7 @@ private:\n \tusing ControlListMap = std::unordered_map<const ControlId *, ControlValue>;\n \n public:\n-\tControlList(ControlValidator *validator = nullptr);\n+\tControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr);\n \n \tusing iterator = ControlListMap::iterator;\n \tusing const_iterator = ControlListMap::const_iterator;\n@@ -136,11 +136,13 @@ public:\n \tconst_iterator begin() const { return controls_.begin(); }\n \tconst_iterator end() const { return controls_.end(); }\n \n-\tbool contains(const ControlId &id) const;\n \tbool empty() const { return controls_.empty(); }\n \tstd::size_t size() const { return controls_.size(); }\n \tvoid clear() { controls_.clear(); }\n \n+\tbool contains(const ControlId &id) const;\n+\tbool contains(unsigned int id) const;\n+\n \ttemplate<typename T>\n \tconst T &get(const Control<T> &ctrl) const\n \t{\n@@ -163,11 +165,15 @@ public:\n \t\tval->set<T>(value);\n \t}\n \n+\tconst ControlValue &get(unsigned int id) const;\n+\tvoid set(unsigned int id, const ControlValue &value);\n+\n private:\n \tconst ControlValue *find(const ControlId &id) const;\n \tControlValue *find(const ControlId &id);\n \n \tControlValidator *validator_;\n+\tconst ControlIdMap *idmap_;\n \tControlListMap controls_;\n };\n \ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 80138f196184..b0d23dd154be 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -220,7 +220,7 @@ void IPARkISP1::setControls(unsigned int frame)\n \n void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n {\n-\tControlList ctrls;\n+\tControlList ctrls(controls::controls);\n \n \tif (aeState)\n \t\tctrls.set(controls::AeLocked, aeState == 2);\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 292e48cd6d25..ddd4e6680ce2 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -7,6 +7,7 @@\n \n #include <libcamera/controls.h>\n \n+#include <iomanip>\n #include <sstream>\n #include <string>\n \n@@ -16,13 +17,13 @@\n \n /**\n * \\file controls.h\n- * \\brief Describes control framework and controls supported by a camera\n+ * \\brief Framework to handle manage controls related to an object\n *\n- * A control is a mean to govern or influence the operation of a camera. Every\n- * control is defined by a unique numerical ID, a name string and the data type\n- * of the value it stores. The libcamera API defines a set of standard controls\n- * in the libcamera::controls namespace, as a set of instances of the Control\n- * class.\n+ * A control is a mean to govern or influence the operation of an object, and in\n+ * particular of a camera. Every control is defined by a unique numerical ID, a\n+ * name string and the data type of the value it stores. The libcamera API\n+ * defines a set of standard controls in the libcamera::controls namespace, as\n+ * a set of instances of the Control class.\n *\n * The main way for applications to interact with controls is through the\n * ControlList stored in the Request class:\n@@ -274,7 +275,7 @@ bool ControlValue::operator==(const ControlValue &other) const\n * \\class Control\n * \\brief Describe a control and its intrinsic properties\n *\n- * The Control class models a control exposed by a camera. Its template type\n+ * The Control class models a control exposed by an object. Its template type\n * name T refers to the control data type, and allows methods that operate on\n * control values to be defined as template methods using the same type T for\n * the control value. See for instance how the ControlList::get() method\n@@ -293,8 +294,8 @@ bool ControlValue::operator==(const ControlValue &other) const\n * long int).\n *\n * Controls IDs shall be unique. While nothing prevents multiple instances of\n- * the Control class to be created with the same ID, this may lead to undefined\n- * behaviour.\n+ * the Control class to be created with the same ID for the same object, doing\n+ * so may cause undefined behaviour.\n */\n \n /**\n@@ -398,18 +399,28 @@ std::string ControlRange::toString() const\n \n /**\n * \\class ControlList\n- * \\brief Associate a list of ControlId with their values for a camera\n+ * \\brief Associate a list of ControlId with their values for an object\n *\n- * A ControlList wraps a map of ControlId to ControlValue and optionally\n- * validates controls against a ControlValidator.\n+ * The ControlList class stores values of controls exposed by an object. The\n+ * lists returned by the Request::controls() and Request::metadata() methods\n+ * refer to the camera that the request belongs to.\n+ *\n+ * Control lists are constructed with a map of all the controls supported by\n+ * their object, and an optional ControlValidator to further validate the\n+ * controls.\n */\n \n /**\n * \\brief Construct a ControlList with an optional control validator\n+ * \\param[in] idmap The ControlId map for the control list target object\n * \\param[in] validator The validator (may be null)\n+ *\n+ * For ControlList containing libcamera controls, a global map of all libcamera\n+ * controls is provided by controls::controls and can be used as the \\a idmap\n+ * argument.\n */\n-ControlList::ControlList(ControlValidator *validator)\n-\t: validator_(validator)\n+ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator)\n+\t: validator_(validator), idmap_(&idmap)\n {\n }\n \n@@ -449,20 +460,6 @@ ControlList::ControlList(ControlValidator *validator)\n * list\n */\n \n-/**\n- * \\brief Check if the list contains a control with the specified \\a id\n- * \\param[in] id The control ID\n- *\n- * The behaviour is undefined if the control \\a id is not supported by the\n- * camera that the ControlList refers to.\n- *\n- * \\return True if the list contains a matching control, false otherwise\n- */\n-bool ControlList::contains(const ControlId &id) const\n-{\n-\treturn controls_.find(&id) != controls_.end();\n-}\n-\n /**\n * \\fn ControlList::empty()\n * \\brief Identify if the list is empty\n@@ -481,7 +478,33 @@ bool ControlList::contains(const ControlId &id) const\n */\n \n /**\n- * \\fn template<typename T> const T &ControlList::get() const\n+ * \\brief Check if the list contains a control with the specified \\a id\n+ * \\param[in] id The control ID\n+ *\n+ * \\return True if the list contains a matching control, false otherwise\n+ */\n+bool ControlList::contains(const ControlId &id) const\n+{\n+\treturn controls_.find(&id) != controls_.end();\n+}\n+\n+/**\n+ * \\brief Check if the list contains a control with the specified \\a id\n+ * \\param[in] id The control numerical ID\n+ *\n+ * \\return True if the list contains a matching control, false otherwise\n+ */\n+bool ControlList::contains(unsigned int id) const\n+{\n+\tconst auto iter = idmap_->find(id);\n+\tif (iter == idmap_->end())\n+\t\treturn false;\n+\n+\treturn contains(*iter->second);\n+}\n+\n+/**\n+ * \\fn template<typename T> const T &ControlList::get(const Control<T> &ctrl) const\n * \\brief Get the value of a control\n * \\param[in] ctrl The control\n *\n@@ -496,7 +519,7 @@ bool ControlList::contains(const ControlId &id) const\n */\n \n /**\n- * \\fn template<typename T> void ControlList::set()\n+ * \\fn template<typename T> void ControlList::set(const Control<T> &ctrl, const T &value)\n * \\brief Set the control value to \\a value\n * \\param[in] ctrl The control\n * \\param[in] value The control value\n@@ -506,9 +529,67 @@ bool ControlList::contains(const ControlId &id) const\n * to the list.\n *\n * The behaviour is undefined if the control \\a ctrl is not supported by the\n- * camera that the list refers to.\n+ * object that the list refers to.\n */\n \n+/**\n+ * \\brief Get the value of control \\a id\n+ * \\param[in] id The control numerical ID\n+ *\n+ * The behaviour is undefined if the control \\a id is not present in the list.\n+ * Use ControlList::contains() to test for the presence of a control in the\n+ * list before retrieving its value.\n+ *\n+ * \\return The control value\n+ */\n+const ControlValue &ControlList::get(unsigned int id) const\n+{\n+\tstatic ControlValue zero;\n+\n+\tconst auto ctrl = idmap_->find(id);\n+\tif (ctrl == idmap_->end()) {\n+\t\tLOG(Controls, Error)\n+\t\t\t<< std::hex << std::setfill('0')\n+\t\t\t<< \"Control 0x\" << std::setw(8) << id << \" is not valid\";\n+\t\treturn zero;\n+\t}\n+\n+\tconst ControlValue *val = find(*ctrl->second);\n+\tif (!val)\n+\t\treturn zero;\n+\n+\treturn *val;\n+}\n+\n+/**\n+ * \\brief Set the value of control \\a id to \\a value\n+ * \\param[in] id The control ID\n+ * \\param[in] value The control value\n+ *\n+ * This method sets the value of a control in the control list. If the control\n+ * is already present in the list, its value is updated, otherwise it is added\n+ * to the list.\n+ *\n+ * The behaviour is undefined if the control \\a id is not supported by the\n+ * object that the list refers to.\n+ */\n+void ControlList::set(unsigned int id, const ControlValue &value)\n+{\n+\tconst auto ctrl = idmap_->find(id);\n+\tif (ctrl == idmap_->end()) {\n+\t\tLOG(Controls, Error)\n+\t\t\t<< std::hex << std::setfill('0')\n+\t\t\t<< \"Control 0x\" << std::setw(8) << id << \" is not valid\";\n+\t\treturn;\n+\t}\n+\n+\tControlValue *val = find(*ctrl->second);\n+\tif (!val)\n+\t\treturn;\n+\n+\t*val = value;\n+}\n+\n const ControlValue *ControlList::find(const ControlId &id) const\n {\n \tconst auto iter = controls_.find(&id);\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex e800f1449888..c14ed1a4d3ce 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -11,6 +11,7 @@\n \n #include <libcamera/buffer.h>\n #include <libcamera/camera.h>\n+#include <libcamera/control_ids.h>\n #include <libcamera/stream.h>\n \n #include \"camera_controls.h\"\n@@ -64,12 +65,12 @@ Request::Request(Camera *camera, uint64_t cookie)\n \t * creating a new instance for each request?\n \t */\n \tvalidator_ = new CameraControlValidator(camera);\n-\tcontrols_ = new ControlList(validator_);\n+\tcontrols_ = new ControlList(controls::controls, validator_);\n \n \t/**\n \t * \\todo: Add a validator for metadata controls.\n \t */\n-\tmetadata_ = new ControlList();\n+\tmetadata_ = new ControlList(controls::controls);\n }\n \n Request::~Request()\ndiff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp\nindex 1bcfecc467b5..5af53f64bb6c 100644\n--- a/test/controls/control_list.cpp\n+++ b/test/controls/control_list.cpp\n@@ -42,7 +42,7 @@ protected:\n \tint run()\n \t{\n \t\tCameraControlValidator validator(camera_.get());\n-\t\tControlList list(&validator);\n+\t\tControlList list(controls::controls, &validator);\n \n \t\t/* Test that the list is initially empty. */\n \t\tif (!list.empty()) {\n", "prefixes": [ "libcamera-devel", "v2", "07/14" ] }