From patchwork Fri Aug 26 16:54:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 17214 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 8B0BCC3272 for ; Fri, 26 Aug 2022 16:55:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BBE4461FC0; Fri, 26 Aug 2022 18:55:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1661532909; bh=veOn7iDaOLD8SZlBQLblTsSQq33gn/a3psXtcym7ZrA=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=nYV5HBaxlHb6Lnq0ddxTbFxAnd7Q+Tux4tE5T50s0GvuERke3rqC6ROWW9VPSapk6 SvgQk/oOd1hxid6rvGmwdjwmk1QlDZd968iX9MBKjlitugZB/n+CHSa8+kth+/rPmb SHFCMPSVkKvTCSH7d8jksyp+KqEdlrvwUaiiM3RhVGZ8VDcr7vehGUgaxeCVboRUtc Ud0NYywfmXuMQERdtkWgmm2KJJhU7q8w+81weXsYDx56pMl5eI9wbym7m3d0mKUWkN 9+RI2Rtu9Ng749bTrt2Efgk8g5mPC3fBJY18OrbVByYGiKMIWuRX3RiuOYWf0RGUkS yAKv7NC0pNLow== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CF96D61FA2 for ; Fri, 26 Aug 2022 18:55:07 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vzoXuTU3"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E680F2B3; Fri, 26 Aug 2022 18:55:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1661532907; bh=veOn7iDaOLD8SZlBQLblTsSQq33gn/a3psXtcym7ZrA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vzoXuTU3p4y+JzNX7ejcvVU5aJQ0Dtj1yK7SgMMHuzCdpjAyTBo1mTL9spTOKJbld FjoxoMnMyXEce+z0PMW2bKWR5vSUSJuarocqyOHF9Kpsq0apesvYWWNcaU6tcO0cPK iCT0cr+FRRO+8qT3aX0cNqNRBddbDAyK1YABXjbI= To: libcamera-devel@lists.libcamera.org Date: Fri, 26 Aug 2022 11:54:53 -0500 Message-Id: <20220826165453.135113-1-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220826154419.4099372-5-paul.elder@ideasonboard.com> References: <20220826154419.4099372-5-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3.1 4/7] utils: ipc: Add support for Flags 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: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add Flags as a supported type in the IPA interface. It is used in mojom with the [flags] attribute. Any field or parameter type E that is prefixed with the [flags] attribute will direct the code generator to generate the type name "Flags" and appropriate serialization/deserialization code for Flags instead of for E. It is usable and has been tested in struct members, function input and output parameters, and Signal parameters. This does not add support for returning Flags as direct return values. Additionally, the [scopedEnum] attribute can be used on enum definitions, which will instruct the code generator to convert it to an enum class instead of a raw enum. Signed-off-by: Paul Elder Reviewed-by: Laurent Pinchart --- Changes in v3.1: - add documentation of [scopedEnum] and [flags] to core.mojom - it's not the best place, but better somewhere than nowhere for now Changes in v3: - change flags attribute from [Flags] to [flags] - make it so that enum input parameters are passed directly (as opposed to const references) - to turn enum definitions in mojom to enum classes, use the [scopedEnum] attribute (as opposed to the [Flags] attribute in v2) - correspondingly, the function in the mojom generator python script is separated out from IsFlags() into IsScoped() - clean up IsFlags() - simplify GetFullNameForElement() for flags Question for reviewers (in v2): Should we use a different attribute name to specify that an enum should be an enum class? Answer (in v3): Yes; that attribute name is [scopedEnum] Changes in v2: - Use mojom attribute to specify that a field should be Flags, instead of using a magic type name format --- include/libcamera/ipa/core.mojom | 9 ++++++ include/libcamera/ipa/ipa_interface.h | 1 + .../definition_functions.tmpl | 2 +- .../libcamera_templates/proxy_functions.tmpl | 10 +++++-- .../libcamera_templates/serializer.tmpl | 4 +++ .../generators/mojom_libcamera_generator.py | 29 +++++++++++++++++-- 6 files changed, 48 insertions(+), 7 deletions(-) diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom index 74f3339e..ef28ff2d 100644 --- a/include/libcamera/ipa/core.mojom +++ b/include/libcamera/ipa/core.mojom @@ -33,6 +33,15 @@ module libcamera; * available for the type and there's no need to generate one * - hasFd - struct fields or empty structs only * - Designate that this field or empty struct contains a SharedFD + * - scopedEnum - enum definitions + * - Designate that this enum should be an enum class, as opposed to a pure + * enum + * - flags - struct fields or function parameters that are enums + * - Designate that this enum type E should be Flags in the generated C++ + * code + * - For example, if a struct field is defined as `[flags] ErrorFlag f;` + * (where ErrorFlag is defined as an enum elsewhere in mojom), then the + * generated code for this field will be `Flags f` * * Rules: * - If the type is defined in a libcamera C++ header *and* a (de)serializer is diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h index 50ca0e7b..8afcfe21 100644 --- a/include/libcamera/ipa/ipa_interface.h +++ b/include/libcamera/ipa/ipa_interface.h @@ -13,6 +13,7 @@ #include #include +#include #include #include diff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl index 94bb4918..8b8509f3 100644 --- a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl +++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl @@ -9,7 +9,7 @@ # \param enum Enum object whose definition is to be generated #} {%- macro define_enum(enum) -%} -enum {{enum.mojom_name}} { +enum{{" class" if enum|is_scoped}} {{enum.mojom_name}} { {%- for field in enum.fields %} {{field.mojom_name}} = {{field.numeric_value}}, {%- endfor %} diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl index cbcfb64a..2be65d43 100644 --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl @@ -62,7 +62,9 @@ {%- else %} std::tie({{param.mojom_name}}Buf, std::ignore) = {%- endif %} -{%- if param|is_enum %} +{%- if param|is_flags %} + IPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}} +{%- elif param|is_enum %} IPADataSerializer::serialize(static_cast({{param.mojom_name}}) {%- else %} IPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}} @@ -105,7 +107,9 @@ #} {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%} {{"*" if pointer}}{{param.mojom_name}} = -{%- if param|is_enum %} +{%- if param|is_flags %} +IPADataSerializer<{{param|name_full}}>::deserialize( +{%- elif param|is_enum %} static_cast<{{param|name_full}}>(IPADataSerializer::deserialize( {%- else %} IPADataSerializer<{{param|name}}>::deserialize( @@ -133,7 +137,7 @@ IPADataSerializer<{{param|name}}>::deserialize( {%- if param|needs_control_serializer %} &controlSerializer_ {%- endif -%} -){{")" if param|is_enum}}; +){{")" if param|is_enum and not param|is_flags}}; {%- endmacro -%} diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl index 77bae36f..eec75211 100644 --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl @@ -34,6 +34,8 @@ std::tie({{field.mojom_name}}, std::ignore) = {%- if field|is_pod %} IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}); + {%- elif field|is_flags %} + IPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}}); {%- elif field|is_enum %} IPADataSerializer::serialize(data.{{field.mojom_name}}); {%- endif %} @@ -96,6 +98,8 @@ {{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}} {%- if field|is_pod %} ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}); + {%- elif field|is_flags %} + ret.{{field.mojom_name}} = IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field_size}}); {%- else %} ret.{{field.mojom_name}} = static_cast<{{field|name_full}}>(IPADataSerializer::deserialize(m, m + {{field_size}})); {%- endif %} diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py index 753bfc73..d7fe2031 100644 --- a/utils/ipc/generators/mojom_libcamera_generator.py +++ b/utils/ipc/generators/mojom_libcamera_generator.py @@ -74,6 +74,8 @@ def GetDefaultValue(element): return element.default if type(element.kind) == mojom.Kind: return '0' + if IsFlags(element): + return '' if mojom.IsEnumKind(element.kind): return f'static_cast<{element.kind.mojom_name}>(0)' if isinstance(element.kind, mojom.Struct) and \ @@ -184,7 +186,7 @@ def MethodParameters(method): params = [] for param in method.parameters: params.append('const %s %s%s' % (GetNameForElement(param), - '&' if not IsPod(param) else '', + '' if IsPod(param) or (IsEnum(param) and not IsFlags(param)) else '&', param.mojom_name)) for param in MethodParamOutputs(method): params.append(f'{GetNameForElement(param)} *{param.mojom_name}') @@ -220,9 +222,21 @@ def IsControls(element): def IsEnum(element): return mojom.IsEnumKind(element.kind) +def IsScoped(element): + attributes = getattr(element, 'attributes', None) + if not attributes: + return False + return 'scopedEnum' in attributes + def IsFd(element): return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "SharedFD" +def IsFlags(element): + attributes = getattr(element, 'attributes', None) + if not attributes: + return False + return 'flags' in attributes + def IsMap(element): return mojom.IsMapKind(element.kind) @@ -251,9 +265,11 @@ def ByteWidthFromCppType(t): raise Exception('invalid type') return str(int(_bit_widths[key]) // 8) - # Get the type name for a given element def GetNameForElement(element): + # Flags + if IsFlags(element): + return f'Flags<{GetFullNameForElement(element.kind)}>' # structs if (mojom.IsEnumKind(element) or mojom.IsInterfaceKind(element) or @@ -302,7 +318,8 @@ def GetNameForElement(element): def GetFullNameForElement(element): name = GetNameForElement(element) namespace_str = '' - if mojom.IsStructKind(element): + if (mojom.IsStructKind(element) or + mojom.IsEnumKind(element)): namespace_str = element.module.mojom_namespace.replace('.', '::') elif (hasattr(element, 'kind') and (mojom.IsStructKind(element.kind) or @@ -311,6 +328,10 @@ def GetFullNameForElement(element): if namespace_str == '': return name + + if IsFlags(element): + return GetNameForElement(element) + return f'{namespace_str}::{name}' def ValidateZeroLength(l, s, cap=True): @@ -408,9 +429,11 @@ class Generator(generator.Generator): 'is_controls': IsControls, 'is_enum': IsEnum, 'is_fd': IsFd, + 'is_flags': IsFlags, 'is_map': IsMap, 'is_plain_struct': IsPlainStruct, 'is_pod': IsPod, + 'is_scoped': IsScoped, 'is_str': IsStr, 'method_input_has_fd': MethodInputHasFd, 'method_output_has_fd': MethodOutputHasFd,