From patchwork Thu Aug 18 06:49:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 17150 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 D11A6C3272 for ; Thu, 18 Aug 2022 06:49:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 97BF561FCB; Thu, 18 Aug 2022 08:49:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660805385; bh=FgXZX85vMNdhkD2BbjNvYNq4SD2K4AepZVEriwSojt4=; 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=bCRZpya7jF3CDiCcpQWPB0dASfRYY8eQYDQH2rOncpdbCu6+KZYnYWFYEkzsJA/Gf rUSzHKYY72motXGtcRpYAXDi5ICUjxBiu5vEVCSb5zH08UINPCM63jWtDmEZatmyMF q/lHOKb2r6wLhil7KNc++WPkc8z3czycIN2ka/S6jze1frPN5M9NDb5ZFcBqExRFJO pzONUYywxcVOiMEE70EDcHQQyM1lQZ+BZx125A/lC3/VS3fibxuohVjuf3dlM/egg0 XCzIuiA2UWd3KXO//JWM3iAPBMFabqcotICC3Tom5/aQhuikHCn3s9Bfg8pbDB/UJ0 0kp43woXmEnbg== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3609F61FCB for ; Thu, 18 Aug 2022 08:49:44 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="WpNZliSQ"; dkim-atps=neutral Received: from pyrite.rasen.tech (KD027085204050.au-net.ne.jp [27.85.204.50]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B8DE34A8; Thu, 18 Aug 2022 08:49:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660805384; bh=FgXZX85vMNdhkD2BbjNvYNq4SD2K4AepZVEriwSojt4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WpNZliSQEWNEU9R/7hxZmwOSyMVP26WiKPuj8txTIEKyH6xvjoMMJMncbmuHRxAIf bMW7URUMuXYWqoQAmP5S5zks51lSURAzQtHhq8MSgwgS7HDBnEMzUgQ1G1MCptSa2S Fn6Hc6ZqxuKE7MAh1zi/cymi6h/AtD3OOc2NXGOk= To: libcamera-devel@lists.libcamera.org Date: Thu, 18 Aug 2022 15:49:19 +0900 Message-Id: <20220818064923.2573060-5-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220818064923.2573060-1-paul.elder@ideasonboard.com> References: <20220818064923.2573060-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 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. The [Flags] attribute can also 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 --- Question for reviewers: Should we use a different attribute name to specify that an enum should be an enum class? My rationale here was that if you want to use an enum for Flags then you put the [Flags] attribute on it, then it'll be generated as an enum class, and just like how in C++ you have to wrap E in Flags<>, the analogous form in mojom is the [Flags] attribute. Also, I did try converting all enums to enum classes; it broke a lot of raspberrypi stuff because they expect enums to be usable as both flags and ints (as far as I could understand)... 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/ipa_interface.h | 1 + .../definition_functions.tmpl | 2 +- .../libcamera_templates/proxy_functions.tmpl | 10 +++++--- .../libcamera_templates/serializer.tmpl | 4 ++++ .../generators/mojom_libcamera_generator.py | 23 ++++++++++++++++++- 5 files changed, 35 insertions(+), 5 deletions(-) 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..fa185177 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_flags}} {{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..5534b24e 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 \ @@ -223,6 +225,15 @@ def IsEnum(element): def IsFd(element): return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "SharedFD" +def IsFlags(element): + if not hasattr(element, 'attributes'): + return False + if element.attributes is None: + return False + if 'Flags' in element.attributes: + return True + return False + def IsMap(element): return mojom.IsMapKind(element.kind) @@ -251,9 +262,13 @@ 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): + if mojom.IsEnumKind(element): + return element.mojom_name + return f'Flags<{GetFullNameForElement(element.kind)}>' # structs if (mojom.IsEnumKind(element) or mojom.IsInterfaceKind(element) or @@ -311,6 +326,11 @@ def GetFullNameForElement(element): if namespace_str == '': return name + + if IsFlags(element): + name = re.match(r'^Flags<(.*)>$', name).group(1) + return f'Flags<{namespace_str}::{name}>' + return f'{namespace_str}::{name}' def ValidateZeroLength(l, s, cap=True): @@ -408,6 +428,7 @@ 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,