{"id":1997,"url":"https://patchwork.libcamera.org/api/patches/1997/?format=json","web_url":"https://patchwork.libcamera.org/patch/1997/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20190924171440.29758-5-jacopo@jmondi.org>","date":"2019-09-24T17:14:39","name":"[libcamera-devel,v2,4/5] libcamera: controls: De-couple Controls from Camera","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"5cce792d760e952f0e8fa8f12ebb1302864bd87b","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/?format=json","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/1997/mbox/","series":[{"id":505,"url":"https://patchwork.libcamera.org/api/series/505/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=505","date":"2019-09-24T17:14:35","name":"libcamera: Control framework backend rewor","version":2,"mbox":"https://patchwork.libcamera.org/series/505/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/1997/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/1997/checks/","tags":{},"headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D272F62382\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Sep 2019 19:13:08 +0200 (CEST)","from uno.homenet.telecomitalia.it\n\t(host89-248-dynamic.45-213-r.retail.telecomitalia.it [213.45.248.89])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 5CBA92000D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Sep 2019 17:13:08 +0000 (UTC)"],"X-Originating-IP":"213.45.248.89","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 24 Sep 2019 19:14:39 +0200","Message-Id":"<20190924171440.29758-5-jacopo@jmondi.org>","X-Mailer":"git-send-email 2.23.0","In-Reply-To":"<20190924171440.29758-1-jacopo@jmondi.org>","References":"<20190924171440.29758-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 4/5] libcamera: controls: De-couple\n\tControls from Camera","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":"Tue, 24 Sep 2019 17:13:09 -0000"},"content":"ControlList requires a Camera class instance at construction time,\npreventing it from being re-constructed from serialized binary data.\n\nDe-couple ControlList from Camera by internally storing a reference to\nthe Camera's ControlInfoList.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nSigned-off-by: Jacopo Mondi <jacopo@jmondi.org>\n---\n include/libcamera/controls.h   |  4 +--\n src/libcamera/controls.cpp     | 61 ++++++++++++++++++----------------\n src/libcamera/request.cpp      |  4 +--\n test/controls/control_list.cpp |  4 +--\n 4 files changed, 39 insertions(+), 34 deletions(-)","diff":"diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex e46cd8a78679..d3065c0f3f16 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -48,7 +48,7 @@ private:\n \tusing ControlListMap = std::unordered_map<const ControlInfo *, DataValue>;\n \n public:\n-\tControlList(Camera *camera);\n+\tControlList(const ControlInfoMap &infoMap);\n \n \tusing iterator = ControlListMap::iterator;\n \tusing const_iterator = ControlListMap::const_iterator;\n@@ -70,7 +70,7 @@ public:\n \tvoid update(const ControlList &list);\n \n private:\n-\tCamera *camera_;\n+\tconst ControlInfoMap &infoMap_;\n \tControlListMap controls_;\n };\n \ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 154d9512f660..3c2a8dc50a16 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -174,11 +174,16 @@ std::string ControlInfo::toString() const\n  */\n \n /**\n- * \\brief Construct a ControlList with a reference to the Camera it applies on\n- * \\param[in] camera The camera\n+ * \\brief Construct a ControlList with a map of control information\n+ * \\param[in] infoMap The ControlInfoMap of the camera the control list refers to\n+ *\n+ * The provided \\a infoMap collects the control id and the associated static\n+ * information for each control that can be set on the list. \\a infoMap is\n+ * provided by the Camera the list of controls refers to and it is used to make\n+ * sure only controls supported by the camera can be added to the list.\n  */\n-ControlList::ControlList(Camera *camera)\n-\t: camera_(camera)\n+ControlList::ControlList(const ControlInfoMap &infoMap)\n+\t: infoMap_(infoMap)\n {\n }\n \n@@ -229,17 +234,14 @@ ControlList::ControlList(Camera *camera)\n  */\n bool ControlList::contains(ControlId id) const\n {\n-\tconst ControlInfoMap &controls = camera_->controls();\n-\tconst auto iter = controls.find(id);\n-\tif (iter == controls.end()) {\n-\t\tLOG(Controls, Error)\n-\t\t\t<< \"Camera \" << camera_->name()\n-\t\t\t<< \" does not support control \" << id;\n+\tconst auto info = infoMap_.find(id);\n+\tif (info == infoMap_.end()) {\n+\t\tLOG(Controls, Error) << \"Control \" << id << \" not supported\";\n \n \t\treturn false;\n \t}\n \n-\treturn controls_.find(&iter->second) != controls_.end();\n+\treturn controls_.find(&info->second) != controls_.end();\n }\n \n /**\n@@ -283,18 +285,15 @@ bool ControlList::contains(const ControlInfo *info) const\n  */\n DataValue &ControlList::operator[](ControlId id)\n {\n-\tconst ControlInfoMap &controls = camera_->controls();\n-\tconst auto iter = controls.find(id);\n-\tif (iter == controls.end()) {\n-\t\tLOG(Controls, Error)\n-\t\t\t<< \"Camera \" << camera_->name()\n-\t\t\t<< \" does not support control \" << id;\n+\tconst auto info = infoMap_.find(id);\n+\tif (info == infoMap_.end()) {\n+\t\tLOG(Controls, Error) << \"Control \" << id << \" not supported\";\n \n \t\tstatic DataValue empty;\n \t\treturn empty;\n \t}\n \n-\treturn controls_[&iter->second];\n+\treturn controls_[&info->second];\n }\n \n /**\n@@ -322,18 +321,24 @@ DataValue &ControlList::operator[](ControlId id)\n  */\n void ControlList::update(const ControlList &other)\n {\n-\tif (other.camera_ != camera_) {\n-\t\tLOG(Controls, Error)\n-\t\t\t<< \"Can't update ControlList from a different camera\";\n-\t\treturn;\n+\t/*\n+\t * Make sure all controls in the other list are supported by this\n+\t * Camera. This is guaranteed to be true if the two lists refer to\n+\t * the same Camera.\n+\t */\n+\tfor (auto &control : other) {\n+\t\tControlId id = control.first->id();\n+\n+\t\tif (infoMap_.find(id) == infoMap_.end()) {\n+\t\t\tLOG(Controls, Error)\n+\t\t\t\t<< \"Failed to update control list with control: \"\n+\t\t\t\t<< id;\n+\t\t\treturn;\n+\t\t}\n \t}\n \n-\tfor (auto it : other) {\n-\t\tconst ControlInfo *info = it.first;\n-\t\tconst DataValue &value = it.second;\n-\n-\t\tcontrols_[info] = value;\n-\t}\n+\tfor (auto &control : other)\n+\t\tcontrols_[control.first] = control.second;\n }\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\nindex ee2158fc7a9c..2b3e1870094e 100644\n--- a/src/libcamera/request.cpp\n+++ b/src/libcamera/request.cpp\n@@ -55,8 +55,8 @@ LOG_DEFINE_CATEGORY(Request)\n  *\n  */\n Request::Request(Camera *camera, uint64_t cookie)\n-\t: camera_(camera), controls_(camera), cookie_(cookie),\n-\t  status_(RequestPending), cancelled_(false)\n+\t: camera_(camera), controls_(camera->controls()),\n+\t  cookie_(cookie), status_(RequestPending), cancelled_(false)\n {\n }\n \ndiff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp\nindex f1d79ff8fcfd..3c6eb40c091b 100644\n--- a/test/controls/control_list.cpp\n+++ b/test/controls/control_list.cpp\n@@ -39,7 +39,7 @@ protected:\n \n \tint run()\n \t{\n-\t\tControlList list(camera_.get());\n+\t\tControlList list(camera_->controls());\n \n \t\t/* Test that the list is initially empty. */\n \t\tif (!list.empty()) {\n@@ -155,7 +155,7 @@ protected:\n \t\t * the old list. Verify that the new list is empty and that the\n \t\t * new list contains the expected items and values.\n \t\t */\n-\t\tControlList newList(camera_.get());\n+\t\tControlList newList(camera_->controls());\n \n \t\tnewList[Brightness] = 128;\n \t\tnewList[Saturation] = 255;\n","prefixes":["libcamera-devel","v2","4/5"]}