From patchwork Tue Aug 10 15:12:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 13287 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2BB59C3241 for ; Tue, 10 Aug 2021 15:11:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5161468888; Tue, 10 Aug 2021 17:11:30 +0200 (CEST) Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 781D6687F0 for ; Tue, 10 Aug 2021 17:11:28 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id C7FA9200008; Tue, 10 Aug 2021 15:11:27 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Tue, 10 Aug 2021 17:12:09 +0200 Message-Id: <20210810151211.56702-4-jacopo@jmondi.org> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20210810151211.56702-1-jacopo@jmondi.org> References: <20210810151211.56702-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/5] libcamera: controls: Use ControlIdMap in deserialization X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Introduce a new field in the controls serialization protocol to allow discerning which ControlIdMap a ControlInfoMap refers to. The newly introduced IdMapType enumeration describes the possible info maps: - Either the globally available controls::controls and properties::properties maps, which are valid across IPC boundaries - A ControlIdMap created locally by the V4L2 device, which is not valid across the IPC boundaries At de-serialization time the idMapType field is inspected and - If the idmap is a globally defined one, there's no need to create new ControlId instances when populating the de-serialized ControlInfoMap. Use the globally available map to retrieve the ControlId reference and use it. - If the idmap is a map only available locally, create a new ControlId as it used to happen before this patch. As a direct consequence, this change allows us to perform lookup by ControlId reference on de-serialized ControlIdMap that refers to the libcamera defined controls::controls and properties::properties. Signed-off-by: Jacopo Mondi Reviewed-by: Paul Elder Reviewed-by: Laurent Pinchart --- include/libcamera/ipa/ipa_controls.h | 9 +++- src/libcamera/control_serializer.cpp | 65 +++++++++++++++++++++++----- src/libcamera/ipa_controls.cpp | 29 +++++++++++++ 3 files changed, 90 insertions(+), 13 deletions(-) diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h index 6d3bf279c22d..eec34d9267af 100644 --- a/include/libcamera/ipa/ipa_controls.h +++ b/include/libcamera/ipa/ipa_controls.h @@ -15,13 +15,20 @@ extern "C" { #define IPA_CONTROLS_FORMAT_VERSION 1 +enum ipa_controls_id_map_type { + IPA_CONTROL_ID_MAP_CONTROLS, + IPA_CONTROL_ID_MAP_PROPERTIES, + IPA_CONTROL_ID_MAP_V4L2, +}; + struct ipa_controls_header { uint32_t version; uint32_t handle; uint32_t entries; uint32_t size; uint32_t data_offset; - uint32_t reserved[3]; + enum ipa_controls_id_map_type id_map_type; + uint32_t reserved[2]; }; struct ipa_control_value_entry { diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp index ed456fd445a0..ba277393d517 100644 --- a/src/libcamera/control_serializer.cpp +++ b/src/libcamera/control_serializer.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include "libcamera/internal/byte_stream_buffer.h" @@ -188,6 +189,15 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap, for (const auto &ctrl : infoMap) valuesSize += binarySize(ctrl.second); + const ControlIdMap *idmap = &infoMap.idmap(); + enum ipa_controls_id_map_type idMapType; + if (idmap == &controls::controls) + idMapType = IPA_CONTROL_ID_MAP_CONTROLS; + else if (idmap == &properties::properties) + idMapType = IPA_CONTROL_ID_MAP_PROPERTIES; + else + idMapType = IPA_CONTROL_ID_MAP_V4L2; + /* Prepare the packet header, assign a handle to the ControlInfoMap. */ struct ipa_controls_header hdr; hdr.version = IPA_CONTROLS_FORMAT_VERSION; @@ -195,6 +205,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap, hdr.entries = infoMap.size(); hdr.size = sizeof(hdr) + entriesSize + valuesSize; hdr.data_offset = sizeof(hdr) + entriesSize; + hdr.id_map_type = idMapType; buffer.write(&hdr); @@ -368,6 +379,33 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & return {}; } + /* + * Use the ControlIdMap corresponding to the id map type. If the type + * references a globally defined id map (such as controls::controls + * or properties::properties), use it. Otherwise, create a local id map + * that will be populated with dynamically created ControlId instances + * when deserializing individual ControlInfoMap entries. + */ + const ControlIdMap *idMap = nullptr; + ControlIdMap *localIdMap = nullptr; + switch (hdr->id_map_type) { + case IPA_CONTROL_ID_MAP_CONTROLS: + idMap = &controls::controls; + break; + case IPA_CONTROL_ID_MAP_PROPERTIES: + idMap = &properties::properties; + break; + case IPA_CONTROL_ID_MAP_V4L2: + controlIdMaps_.emplace_back(std::make_unique()); + localIdMap = controlIdMaps_.back().get(); + idMap = localIdMap; + break; + default: + LOG(Serializer, Error) + << "Unknown id map type: " << hdr->id_map_type; + return {}; + } + ByteStreamBuffer entries = buffer.carveOut(hdr->data_offset - sizeof(*hdr)); ByteStreamBuffer values = buffer.carveOut(hdr->size - hdr->data_offset); @@ -377,9 +415,6 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & } ControlInfoMap::Map ctrls; - controlIdMaps_.emplace_back(std::make_unique()); - ControlIdMap *localIdMap = controlIdMaps_.back().get(); - for (unsigned int i = 0; i < hdr->entries; ++i) { const struct ipa_control_info_entry *entry = entries.read(); @@ -388,15 +423,21 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & return {}; } - /* Create and cache the individual ControlId. */ ControlType type = static_cast(entry->type); - /** - * \todo Find a way to preserve the control name for debugging - * purpose. - */ - controlIds_.emplace_back(std::make_unique(entry->id, "", type)); - ControlId *controlId = controlIds_.back().get(); - (*localIdMap)[entry->id] = controlId; + + /* If we're using a local id map, populate it. */ + if (localIdMap) { + /** + * \todo Find a way to preserve the control name for + * debugging purpose. + */ + controlIds_.emplace_back(std::make_unique(entry->id, + "", type)); + (*localIdMap)[entry->id] = controlIds_.back().get(); + } + + const ControlId *controlId = idMap->at(entry->id); + ASSERT(controlId); if (entry->offset != values.offset()) { LOG(Serializer, Error) @@ -413,7 +454,7 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & * Create the ControlInfoMap in the cache, and store the map to handle * association. */ - infoMaps_[hdr->handle] = ControlInfoMap(std::move(ctrls), *localIdMap); + infoMaps_[hdr->handle] = ControlInfoMap(std::move(ctrls), *idMap); ControlInfoMap &map = infoMaps_[hdr->handle]; infoMapHandles_[&map] = hdr->handle; diff --git a/src/libcamera/ipa_controls.cpp b/src/libcamera/ipa_controls.cpp index 8fd726513182..fb98cda30ede 100644 --- a/src/libcamera/ipa_controls.cpp +++ b/src/libcamera/ipa_controls.cpp @@ -134,6 +134,33 @@ * \brief The current control serialization format version */ +/** + * \var ipa_controls_id_map_type + * \brief Enumerates the different control id map types + * + * Each ControlInfoMap and ControlList refers to a control id map that + * associates the ControlId references to a numerical identifier. + * During the serialization procedure the raw pointers to the ControlId + * instances cannot be transported on the wire, hence their numerical id is + * used to identify them in the serialized data buffer. At deserialization time + * it is required to associate back to the numerical id the ControlId instance + * it represents. This enumeration describes which ControlIdMap should be + * used to perform such operation. + * + * \var ipa_controls_id_map_type::IPA_CONTROL_ID_MAP_CONTROLS + * \brief The numerical control identifier are resolved to a ControlId * using + * the global controls::controls id map + * \var ipa_controls_id_map_type::IPA_CONTROL_ID_MAP_PROPERTIES + * \brief The numerical control identifier are resolved to a ControlId * using + * the global properties::properties id map + * \var ipa_controls_id_map_type::IPA_CONTROL_ID_MAP_V4L2 + * \brief ControlId for V4L2 defined controls are created by the video device + * that enumerates them, and are not available across the IPC boundaries. The + * deserializer shall create new ControlId instances for them as well as store + * them in a dedicated ControlIdMap. Only lookup by numerical id can be + * performed on de-serialized ControlInfoMap that represents V4L2 controls. + */ + /** * \struct ipa_controls_header * \brief Serialized control packet header @@ -149,6 +176,8 @@ * The total packet size in bytes * \var ipa_controls_header::data_offset * Offset in bytes from the beginning of the packet of the data section start + * \var ipa_controls_header::id_map_type + * The id map type as defined by the ipa_controls_id_map_type enumeration * \var ipa_controls_header::reserved * Reserved for future extensions */