{"id":13715,"url":"https://patchwork.libcamera.org/api/1.1/patches/13715/?format=json","web_url":"https://patchwork.libcamera.org/patch/13715/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20210907111038.739104-4-jacopo@jmondi.org>","date":"2021-09-07T11:10:36","name":"[libcamera-devel,v2,3/5] libcamera: control_serializer: Use the right idmap","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"47faf05740467ae7980959810454b6cdaef9149a","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/1.1/people/3/?format=json","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/13715/mbox/","series":[{"id":2487,"url":"https://patchwork.libcamera.org/api/1.1/series/2487/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2487","date":"2021-09-07T11:10:33","name":"libcamera: control serializer fixes","version":2,"mbox":"https://patchwork.libcamera.org/series/2487/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/13715/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/13715/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 60AA6BEFBE\n\tfor ;\n\tTue, 7 Sep 2021 11:10:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0950C69177;\n\tTue, 7 Sep 2021 13:10:00 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A43A60251\n\tfor ;\n\tTue, 7 Sep 2021 13:09:58 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 925AB60002;\n\tTue, 7 Sep 2021 11:09:57 +0000 (UTC)"],"From":"Jacopo Mondi ","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 7 Sep 2021 13:10:36 +0200","Message-Id":"<20210907111038.739104-4-jacopo@jmondi.org>","X-Mailer":"git-send-email 2.32.0","In-Reply-To":"<20210907111038.739104-1-jacopo@jmondi.org>","References":"<20210907111038.739104-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 3/5] libcamera: control_serializer: Use\n\tthe right idmap","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"When a ControlList is deserialized, the code searches for a valid\nControlInfoMap in the local cache and use its id map to initialize the\nlist. If no valid ControlInfoMap is found, as it's usually the case\nfor lists transporting libcamera controls and properties, the globally\ndefined controls::controls id map is used unconditionally.\n\nThis breaks the deserialization of libcamera properties, for which a\nwrong idmap is used at construction time.\n\nAs the serialization header now transports an id_map_type field, store\nthe idmap type at serialization time, and re-use it at\ndeserialization time to identify the correct id map.\n\nAlso make the validation stricter by imposing to list of V4L2 controls to\nhave an associated ControlInfoMap available, as there is no globally\ndefined idmap for such controls.\n\nTo be able to retrieve the idmap associated with a ControlList, add an\naccessor function to the ControlList class.\n\nIt might be worth in future using a ControlInfoMap to initialize the\ndeserialized ControlList to implement controls validation against their\nlimit. As such validation is not implemented at the moment, maintain the\ncurrent behaviour and initialize the control list with an idmap.\n\nSigned-off-by: Jacopo Mondi \nReviewed-by: Paul Elder \nReviewed-by: Kieran Bingham \n---\n include/libcamera/controls.h | 1 +\n src/libcamera/control_serializer.cpp | 51 +++++++++++++++++++++++-----\n src/libcamera/controls.cpp | 8 +++++\n 3 files changed, 51 insertions(+), 9 deletions(-)","diff":"diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex 6668e4bb1053..8e5bc8b70b94 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -407,6 +407,7 @@ public:\n \tvoid set(unsigned int id, const ControlValue &value);\n \n \tconst ControlInfoMap *infoMap() const { return infoMap_; }\n+\tconst ControlIdMap *idMap() const { return idmap_; }\n \n private:\n \tconst ControlValue *find(unsigned int id) const;\ndiff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\nindex 27360526f9eb..d42840cddecb 100644\n--- a/src/libcamera/control_serializer.cpp\n+++ b/src/libcamera/control_serializer.cpp\n@@ -275,6 +275,15 @@ int ControlSerializer::serialize(const ControlList &list,\n \t\tinfoMapHandle = 0;\n \t}\n \n+\tconst ControlIdMap *idmap = list.idMap();\n+\tenum ipa_controls_id_map_type idMapType;\n+\tif (idmap == &controls::controls)\n+\t\tidMapType = IPA_CONTROL_ID_MAP_CONTROLS;\n+\telse if (idmap == &properties::properties)\n+\t\tidMapType = IPA_CONTROL_ID_MAP_PROPERTIES;\n+\telse\n+\t\tidMapType = IPA_CONTROL_ID_MAP_V4L2;\n+\n \tsize_t entriesSize = list.size() * sizeof(struct ipa_control_value_entry);\n \tsize_t valuesSize = 0;\n \tfor (const auto &ctrl : list)\n@@ -287,6 +296,7 @@ int ControlSerializer::serialize(const ControlList &list,\n \thdr.entries = list.size();\n \thdr.size = sizeof(hdr) + entriesSize + valuesSize;\n \thdr.data_offset = sizeof(hdr) + entriesSize;\n+\thdr.id_map_type = idMapType;\n \n \tbuffer.write(&hdr);\n \n@@ -496,13 +506,15 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n \t}\n \n \t/*\n-\t * Retrieve the ControlInfoMap associated with the ControlList based on\n-\t * its ID. The mapping between infoMap and ID is set up when serializing\n-\t * or deserializing ControlInfoMap. If no mapping is found (which is\n-\t * currently the case for ControlList related to libcamera controls),\n-\t * use the global control::control idmap.\n+\t * Retrieve the ControlIdMap associated with the ControlList.\n+\t *\n+\t * The idmap is either retrieved from the list's ControlInfoMap when\n+\t * a valid handle has been initialized at serialization time, or by\n+\t * using the header's id_map_type field for lists that refer to the\n+\t * globally defined libcamera controls and properties, for which no\n+\t * ControlInfoMap is available.\n \t */\n-\tconst ControlInfoMap *infoMap;\n+\tconst ControlIdMap *idMap;\n \tif (hdr->handle) {\n \t\tauto iter = std::find_if(infoMapHandles_.begin(), infoMapHandles_.end(),\n \t\t\t\t\t [&](decltype(infoMapHandles_)::value_type &entry) {\n@@ -514,12 +526,33 @@ ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer\n \t\t\treturn {};\n \t\t}\n \n-\t\tinfoMap = iter->first;\n+\t\tconst ControlInfoMap *infoMap = iter->first;\n+\t\tidMap = &infoMap->idmap();\n+\t\tASSERT(idMap);\n \t} else {\n-\t\tinfoMap = nullptr;\n+\t\tswitch (hdr->id_map_type) {\n+\t\tcase IPA_CONTROL_ID_MAP_CONTROLS:\n+\t\t\tidMap = &controls::controls;\n+\t\t\tbreak;\n+\n+\t\tcase IPA_CONTROL_ID_MAP_PROPERTIES:\n+\t\t\tidMap = &properties::properties;\n+\t\t\tbreak;\n+\n+\t\tcase IPA_CONTROL_ID_MAP_V4L2:\n+\t\t\tLOG(Serializer, Fatal)\n+\t\t\t\t<< \"A list of V4L2 controls requires an ControlInfoMap\";\n+\t\t\treturn {};\n+\t\t}\n \t}\n \n-\tControlList ctrls(infoMap ? infoMap->idmap() : controls::controls);\n+\t/*\n+\t * \\todo When available, initialize the list with the ControlInfoMap\n+\t * so that controls can be validated against their limits.\n+\t * Currently no validation is performed, so it's fine relying on the\n+\t * idmap only.\n+\t */\n+\tControlList ctrls(*idMap);\n \n \tfor (unsigned int i = 0; i < hdr->entries; ++i) {\n \t\tconst struct ipa_control_value_entry *entry =\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 5c05ba4a7cd0..96625edb1380 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -1038,6 +1038,14 @@ void ControlList::set(unsigned int id, const ControlValue &value)\n * associated ControlInfoMap, nullptr is returned in that case.\n */\n \n+/**\n+ * \\fn ControlList::idMap()\n+ * \\brief Retrieve the ControlId map used to construct the ControlList\n+ * \\return The ControlId map used to construct the ControlList. ControlsList\n+ * instances constructed with ControlList() have no associated idmap, nullptr is\n+ * returned in that case.\n+ */\n+\n const ControlValue *ControlList::find(unsigned int id) const\n {\n \tconst auto iter = controls_.find(id);\n","prefixes":["libcamera-devel","v2","3/5"]}