From patchwork Thu Dec 21 15:01:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 19335 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 CA881C3293 for ; Thu, 21 Dec 2023 15:02:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 69A5C62B3D; Thu, 21 Dec 2023 16:02:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1703170931; bh=O+B6mOjskYYZR6uLvDjUPqWaqQVl60N5IL2TOHCyQKM=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=KuWLKO1CKF0DJPfrxALNO5TwJo006vqG/KpIVsafQMhCouv3ITU6BSLYeUM5piczu IRF0oMNZJPPhO33TG1v1gw5k/IkzYFqQxXTuJw15lCgrsM834rfnvMq/pNgdZXxAVR A6tMcEBvL+9DEN6lIqfh1EQavXYYZzlyvftJ9KzOFPO4I5sF0jMWGUbMPG5j2CCrrY HQpFxnShUBam6Gf8UTVtaPtBmQN9EsTct1+r2GeHsYI4gpCDXA+AYnyAuGi7gh5WGM w33QDjgW9yJSdUoNlmjpboVsgvb2ybW9mvWtfzLwkAjmBNwJ8zChJDUqbdXS/aSErx bfvRlv3I1pMbw== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4014C62B31 for ; Thu, 21 Dec 2023 16:02:08 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="eWu9+BVW"; dkim-atps=neutral Received: from localhost.localdomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1DE15833; Thu, 21 Dec 2023 16:01:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1703170877; bh=O+B6mOjskYYZR6uLvDjUPqWaqQVl60N5IL2TOHCyQKM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eWu9+BVW3nF+oExdhyG9cCVssRRYOUtDi4scNVBTw8VvtVlp6xTohFNT+fAAX/JRr oXPm4A4yBbGUn/LrnVq2aQc+Irq1ZWONWGn6ZZ8niC1Z/a1L8cmsGLCu1EWvlJu6nH mSoFQyluH1kicLNSa/yC7wGEvUjqRQntiOZe22Bg= To: libcamera-devel@lists.libcamera.org Date: Thu, 21 Dec 2023 16:01:56 +0100 Message-ID: <20231221150157.584264-2-jacopo.mondi@ideasonboard.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20231221150157.584264-1-jacopo.mondi@ideasonboard.com> References: <20231221150157.584264-1-jacopo.mondi@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] libcamera: Add 'required' property to controls 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: , X-Patchwork-Original-From: Jacopo Mondi via libcamera-devel From: Jacopo Mondi Reply-To: Jacopo Mondi Cc: Jacopo Mondi Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add to the ControlId class a 'required' boolean flag that determine if the control (or property) is mandatory to be supported by a Camera in order to comply with the libcamera API specification. Add support for a 'required' field to the controls and properties yaml file definition and control generation scripts. Also plumb support for the flag in the control serializer component to allow pass the information across IPC borders. Signed-off-by: Jacopo Mondi --- Documentation/guides/pipeline-handler.rst | 13 ++++++++---- include/libcamera/controls.h | 10 ++++++---- include/libcamera/ipa/ipa_controls.h | 3 ++- src/libcamera/control_serializer.cpp | 4 +++- src/libcamera/controls.cpp | 24 +++++++++++++++++------ src/libcamera/ipa_controls.cpp | 2 ++ src/libcamera/v4l2_device.cpp | 2 +- utils/gen-controls.py | 8 +++++++- 8 files changed, 48 insertions(+), 18 deletions(-) diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst index 26dc93589382..ad2a5beb26bc 100644 --- a/Documentation/guides/pipeline-handler.rst +++ b/Documentation/guides/pipeline-handler.rst @@ -610,10 +610,15 @@ information can be found in the `ControlInfoMap`_ class documentation. .. _ControlInfoMap: https://libcamera.org/api-html/classlibcamera_1_1ControlInfoMap.html Pipeline handlers register controls to expose the tunable device and IPA -parameters to applications. Our example pipeline handler only exposes trivial -controls of the video device, by registering a ``ControlId`` instance with -associated values for each supported V4L2 control but demonstrates the mapping -of V4L2 Controls to libcamera ControlIDs. +parameters to applications and register properties to expose the Camera +immutable characteristics. Controls and properties which have the ``required`` +field specified in the YAML definition are mandatory to be supported by a Camera +in order for it to comply with the libcamera API specification. + +Our example pipeline handler only exposes trivial controls of the video device, +by registering a ``ControlId`` instance with associated values for each +supported V4L2 control but demonstrates the mapping of V4L2 Controls to +libcamera ControlIDs. Complete the initialization of the ``VividCameraData`` class by adding the following code to the ``VividCameraData::init()`` function to initialise the diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index cf94205577a5..0182119119f0 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -213,14 +213,15 @@ private: class ControlId { public: - ControlId(unsigned int id, const std::string &name, ControlType type) - : id_(id), name_(name), type_(type) + ControlId(unsigned int id, const std::string &name, ControlType type, bool required) + : id_(id), name_(name), type_(type), required_(required) { } unsigned int id() const { return id_; } const std::string &name() const { return name_; } ControlType type() const { return type_; } + bool required() const { return required_; } private: LIBCAMERA_DISABLE_COPY_AND_MOVE(ControlId) @@ -228,6 +229,7 @@ private: unsigned int id_; std::string name_; ControlType type_; + bool required_; }; static inline bool operator==(unsigned int lhs, const ControlId &rhs) @@ -256,8 +258,8 @@ class Control : public ControlId public: using type = T; - Control(unsigned int id, const char *name) - : ControlId(id, name, details::control_type>::value) + Control(unsigned int id, const char *name, bool required) + : ControlId(id, name, details::control_type>::value, required) { } diff --git a/include/libcamera/ipa/ipa_controls.h b/include/libcamera/ipa/ipa_controls.h index e5da1946ce1d..5268b0a8f659 100644 --- a/include/libcamera/ipa/ipa_controls.h +++ b/include/libcamera/ipa/ipa_controls.h @@ -46,7 +46,8 @@ struct ipa_control_info_entry { uint32_t id; uint32_t type; uint32_t offset; - uint32_t padding[1]; + uint8_t required; + uint8_t padding[3]; }; #ifdef __cplusplus diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp index 0cf719bde798..b9ed1eea6552 100644 --- a/src/libcamera/control_serializer.cpp +++ b/src/libcamera/control_serializer.cpp @@ -281,6 +281,7 @@ int ControlSerializer::serialize(const ControlInfoMap &infoMap, entry.id = id->id(); entry.type = id->type(); entry.offset = values.offset(); + entry.required = id->required(); entries.write(&entry); store(info, values); @@ -498,7 +499,8 @@ ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer & * debugging purpose. */ controlIds_.emplace_back(std::make_unique(entry->id, - "", type)); + "", type, + entry->required)); (*localIdMap)[entry->id] = controlIds_.back().get(); } diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index b808116c01e5..25f309530eca 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -378,18 +378,19 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen * \class ControlId * \brief Control static metadata * - * The ControlId class stores a control ID, name and data type. It provides - * unique identification of a control, but without support for compile-time - * type deduction that the derived template Control class supports. See the - * Control class for more information. + * The ControlId class stores a control ID, name, data type and a boolean + * 'required' flag. It provides unique identification of a control, but without + * support for compile-time type deduction that the derived template Control + * class supports. See the Control class for more information. */ /** - * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type) + * \fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type, bool required) * \brief Construct a ControlId instance * \param[in] id The control numerical ID * \param[in] name The control name * \param[in] type The control data type + * \param[in] required Boolean flag that determine if the control is required */ /** @@ -410,6 +411,16 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen * \return The control data type */ +/** + * \fn bool ControlId::required() const + * \brief Determine if the control is required or not + * + * A control is 'required' if it is mandatory for a Camera to support it to + * comply with the libcamera API specification. + * + * \return True if the control is required, false otherwise + */ + /** * \fn bool operator==(unsigned int lhs, const ControlId &rhs) * \brief Compare a ControlId with a control numerical ID @@ -456,10 +467,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen */ /** - * \fn Control::Control(unsigned int id, const char *name) + * \fn Control::Control(unsigned int id, const char *name, bool required) * \brief Construct a Control instance * \param[in] id The control numerical ID * \param[in] name The control name + * \param[in] required Boolean flag that determine if a control is required * * The control data type is automatically deduced from the template type T. */ diff --git a/src/libcamera/ipa_controls.cpp b/src/libcamera/ipa_controls.cpp index 870a443b0f38..2376cbd9df60 100644 --- a/src/libcamera/ipa_controls.cpp +++ b/src/libcamera/ipa_controls.cpp @@ -220,6 +220,8 @@ static_assert(sizeof(ipa_control_value_entry) == 16, * \var ipa_control_info_entry::offset * The offset in bytes from the beginning of the data section to the control * info data (shall be a multiple of 8 bytes) + * \var ipa_control_info_entry::required + * Boolean flag that determines if the controls is required or not * \var ipa_control_info_entry::padding * Padding bytes (shall be set to 0) */ diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 24d208ef77dc..b055d14c2e51 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -522,7 +522,7 @@ std::unique_ptr V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl & const std::string name(static_cast(ctrl.name), len); const ControlType type = v4l2CtrlType(ctrl.type); - return std::make_unique(ctrl.id, name, type); + return std::make_unique(ctrl.id, name, type, false); } /** diff --git a/utils/gen-controls.py b/utils/gen-controls.py index 6cd5e362c66f..1e997708b10d 100755 --- a/utils/gen-controls.py +++ b/utils/gen-controls.py @@ -112,6 +112,11 @@ class Control(object): else: return f"Span" + @property + def required(self): + """Is the control required""" + return self.__data.get('required') + def snake_case(s): return ''.join([c.isupper() and ('_' + c) or c for c in s]).strip('_') @@ -133,7 +138,7 @@ ${description}''') * \\var ${name} ${description} */''') - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, "${name}");') + def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, "${name}", ${required});') enum_values_doc = string.Template('''/** * \\var ${name}Values * \\brief List of all $name supported values @@ -158,6 +163,7 @@ ${description} 'type': ctrl.type, 'description': format_description(ctrl.description), 'id_name': id_name, + 'required': "true" if ctrl.required else "false" } target_doc = ctrls_doc[vendor]