[{"id":24455,"web_url":"https://patchwork.libcamera.org/comment/24455/","msgid":"","date":"2022-08-09T01:59:03","subject":"Re: [libcamera-devel] [PATCH 6/9] utils: ipc: Add support for Flags","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Wed, Aug 03, 2022 at 08:21:47PM +0900, Paul Elder via libcamera-devel wrote:\n> Add Flags as a supported type in the IPA interface.\n> \n> It is defined and used in mojom as \"Flags_E_t\", and the code generator\n> will convert it to \"Flags\". It is usable and has been tested in\n> struct members, function input and output parameters, and Signal\n> parameters.\n\nHmmm... mojom can parse the \"<...>\" syntax to some extend, as it\nsupports \"array\" for instance. Would it be possible to extend that\nwith flags ?\n\n> This does not add support for returning Flags as direct return values.\n> \n> Signed-off-by: Paul Elder \n> ---\n> .../core_ipa_interface.h.tmpl | 4 +++\n> .../definition_functions.tmpl | 13 +++++++\n> .../module_ipa_interface.h.tmpl | 4 +++\n> .../libcamera_templates/proxy_functions.tmpl | 10 ++++--\n> .../libcamera_templates/serializer.tmpl | 4 +++\n> .../generators/mojom_libcamera_generator.py | 35 ++++++++++++++++++-\n> 6 files changed, 66 insertions(+), 4 deletions(-)\n> \n> diff --git a/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> index a565b59a..2fd55119 100644\n> --- a/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl\n> @@ -30,6 +30,10 @@ static const {{const.kind|name}} {{const.mojom_name}} = {{const.value}};\n> {{funcs.define_enum(enum)}}\n> {% endfor %}\n> \n> +{% for flag in flags %}\n> +{{funcs.define_flags(flag)}}\n> +{% endfor %}\n> +\n> {%- for struct in structs_gen_header %}\n> {{funcs.define_struct(struct)}}\n> {% endfor %}\n> diff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> index 94bb4918..3dbcfca0 100644\n> --- a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> @@ -16,6 +16,19 @@ enum {{enum.mojom_name}} {\n> };\n> {%- endmacro -%}\n> \n> +{#\n> + # \\brief Generate Flags definition\n> + #\n> + # \\param flags Enum object from which a Flags definition is to be generated\n> + #}\n> +{%- macro define_flags(flags) -%}\n> +enum class {{flags|flags_name}} {\n> +{%- for field in flags.fields %}\n> +\t{{field.mojom_name}} = {{field.numeric_value}},\n> +{%- endfor %}\n> +};\n> +{%- endmacro -%}\n> +\n> {#\n> # \\brief Generate struct definition\n> #\n> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> index 415ec283..5e003849 100644\n> --- a/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl\n> @@ -48,6 +48,10 @@ enum class {{cmd_event_enum_name}} {\n> {{funcs.define_enum(enum)}}\n> {% endfor %}\n> \n> +{% for flag in flags %}\n> +{{funcs.define_flags(flag)}}\n> +{% endfor %}\n> +\n> {%- for struct in structs_nonempty %}\n> {{funcs.define_struct(struct)}}\n> {% endfor %}\n> diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> index dc35620f..c5e24323 100644\n> --- a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> @@ -59,7 +59,9 @@\n> {%- else %}\n> \tstd::tie({{param.mojom_name}}Buf, std::ignore) =\n> {%- endif %}\n> -{%- if param|is_enum %}\n> +{%- if param|is_flags %}\n> +\t\tIPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}\n> +{%- elif param|is_enum %}\n> \t\tIPADataSerializer::serialize(static_cast({{param.mojom_name}})\n> {%- else %}\n> \t\tIPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}\n> @@ -102,7 +104,9 @@\n> #}\n> {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}\n> {{\"*\" if pointer}}{{param.mojom_name}} =\n> -{%- if param|is_enum %}\n> +{%- if param|is_flags %}\n> +IPADataSerializer<{{param|name_full}}>::deserialize(\n> +{%- elif param|is_enum %}\n> static_cast<{{param|name_full}}>(IPADataSerializer::deserialize(\n> {%- else %}\n> IPADataSerializer<{{param|name}}>::deserialize(\n> @@ -130,7 +134,7 @@ IPADataSerializer<{{param|name}}>::deserialize(\n> {%- if param|needs_control_serializer %}\n> \t&controlSerializer_\n> {%- endif -%}\n> -){{\")\" if param|is_enum}};\n> +){{\")\" if param|is_enum and not param|is_flags}};\n> {%- endmacro -%}\n> \n> \n> diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> index 77bae36f..eec75211 100644\n> --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl\n> @@ -34,6 +34,8 @@\n> \t\tstd::tie({{field.mojom_name}}, std::ignore) =\n> \t{%- if field|is_pod %}\n> \t\t\tIPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});\n> +\t{%- elif field|is_flags %}\n> +\t\t\tIPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}});\n> \t{%- elif field|is_enum %}\n> \t\t\tIPADataSerializer::serialize(data.{{field.mojom_name}});\n> \t{%- endif %}\n> @@ -96,6 +98,8 @@\n> \t\t{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}\n> \t\t{%- if field|is_pod %}\n> \t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}});\n> +\t\t{%- elif field|is_flags %}\n> +\t\tret.{{field.mojom_name}} = IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field_size}});\n> \t\t{%- else %}\n> \t\tret.{{field.mojom_name}} = static_cast<{{field|name_full}}>(IPADataSerializer::deserialize(m, m + {{field_size}}));\n> \t\t{%- endif %}\n> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py\n> index 753bfc73..22de6fdd 100644\n> --- a/utils/ipc/generators/mojom_libcamera_generator.py\n> +++ b/utils/ipc/generators/mojom_libcamera_generator.py\n> @@ -47,6 +47,8 @@ _bit_widths = {\n> mojom.DOUBLE: '64',\n> }\n> \n> +_flags_re = r'^Flags_(.*)_t$'\n> +\n> def ModuleName(path):\n> return path.split('/')[-1].split('.')[0]\n> \n> @@ -74,6 +76,8 @@ def GetDefaultValue(element):\n> return element.default\n> if type(element.kind) == mojom.Kind:\n> return '0'\n> + if IsFlags(element):\n> + return ''\n> if mojom.IsEnumKind(element.kind):\n> return f'static_cast<{element.kind.mojom_name}>(0)'\n> if isinstance(element.kind, mojom.Struct) and \\\n> @@ -223,6 +227,16 @@ def IsEnum(element):\n> def IsFd(element):\n> return mojom.IsStructKind(element.kind) and element.kind.mojom_name == \"SharedFD\"\n> \n> +def IsFlags(element):\n> + if not hasattr(element, 'mojom_name'):\n> + return False\n> + if re.match(_flags_re, element.mojom_name):\n> + return True\n> + if hasattr(element, 'kind'):\n> + if hasattr(element.kind, 'mojom_name'):\n> + return True if re.match(_flags_re, element.kind.mojom_name) else False\n> + return False\n> +\n> def IsMap(element):\n> return mojom.IsMapKind(element.kind)\n> \n> @@ -251,9 +265,20 @@ def ByteWidthFromCppType(t):\n> raise Exception('invalid type')\n> return str(int(_bit_widths[key]) // 8)\n> \n> +def FlagsName(element):\n> + if hasattr(element, 'mojom_name'):\n> + if re.match(_flags_re, element.mojom_name):\n> + return re.sub(_flags_re, lambda match: match.group(1), element.mojom_name)\n> + if hasattr(element, 'kind'):\n> + if hasattr(element.kind, 'mojom_name'):\n> + return re.sub(_flags_re, lambda match: match.group(1), element.kind.mojom_name)\n> + return None\n> \n> # Get the type name for a given element\n> def GetNameForElement(element):\n> + # Flags\n> + if IsFlags(element):\n> + return f'Flags<{FlagsName(element)}>'\n> # structs\n> if (mojom.IsEnumKind(element) or\n> mojom.IsInterfaceKind(element) or\n> @@ -311,6 +336,11 @@ def GetFullNameForElement(element):\n> \n> if namespace_str == '':\n> return name\n> +\n> + if IsFlags(element):\n> + name = re.match(r'^Flags<(.*)>$', name).group(1)\n> + return f'Flags<{namespace_str}::{name}>'\n> +\n> return f'{namespace_str}::{name}'\n> \n> def ValidateZeroLength(l, s, cap=True):\n> @@ -401,6 +431,7 @@ class Generator(generator.Generator):\n> 'choose': Choose,\n> 'comma_sep': CommaSep,\n> 'default_value': GetDefaultValue,\n> + 'flags_name': FlagsName,\n> 'has_default_fields': HasDefaultFields,\n> 'has_fd': HasFd,\n> 'is_async': IsAsync,\n> @@ -408,6 +439,7 @@ class Generator(generator.Generator):\n> 'is_controls': IsControls,\n> 'is_enum': IsEnum,\n> 'is_fd': IsFd,\n> + 'is_flags': IsFlags,\n> 'is_map': IsMap,\n> 'is_plain_struct': IsPlainStruct,\n> 'is_pod': IsPod,\n> @@ -433,7 +465,8 @@ class Generator(generator.Generator):\n> 'cmd_enum_name': '_%sCmd' % self.module_name,\n> 'cmd_event_enum_name': '_%sEventCmd' % self.module_name,\n> 'consts': self.module.constants,\n> - 'enums': self.module.enums,\n> + 'enums': [enum for enum in self.module.enums if not IsFlags(enum)],\n> + 'flags': [enum for enum in self.module.enums if IsFlags(enum)],\n> 'has_array': len([x for x in self.module.kinds.keys() if x[0] == 'a']) > 0,\n> 'has_map': len([x for x in self.module.kinds.keys() if x[0] == 'm']) > 0,\n> 'has_namespace': self.module.mojom_namespace != '',","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 64927BE173\n\tfor ;\n\tTue, 9 Aug 2022 01:59:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE1EB6330E;\n\tTue, 9 Aug 2022 03:59:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 271CC6330E\n\tfor ;\n\tTue, 9 Aug 2022 03:59:15 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8E52B481;\n\tTue, 9 Aug 2022 03:59:14 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660010356;\n\tbh=XanuUsJPeoLdBmujFi1ZupGHbAolvKP52hoh/Xq7Zis=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=FjIZQR0tBf6HdIPH9ikk5sxEJPYvq/wqMwUPdgkge5Bsq1QDmH++DsoyI/PEXH0J/\n\tbo4Rb0HMKKKECvNfc8KaJJ8IHApMWPscqM6k22VgAyIlQ6fA4ybfsKJ1LdMGV45Dns\n\t/6CW3GsUVyxptu+hNSLodluPuS3wg9JpiKQJ8wSyRF+GQYF6Y/Jm5mcZr8z6IDFhJW\n\t1hYU//wkiQy0qPwc0TK14xFsq2NY9Ft4wuo8qRdIVNT0mRYwjvRGcLIsKu2bvhXzfh\n\tqeGCDSJQ27KGInwJoL7WaTEOc6uu4ksAtBnSENTwby8vcciC3YjNdRtvo5ezkxzFsL\n\tVoImzOfeOw5kw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660010354;\n\tbh=XanuUsJPeoLdBmujFi1ZupGHbAolvKP52hoh/Xq7Zis=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YVAgzwy+zzIR0Rg/IYGi5hGFgZ0cIx1rXgzSd5CXCU1MRC1v+F41X3CW97j4anM/X\n\tvKWAbp2c2r+KUMA9K+DALFT0kYm8HlocfegSzORPmJkBYMcaQxHgaqRK6Z/Vpeio8F\n\t/MPPxk32BzMjp8L7vAF/4dSKA3GH5/8JeA8+5Szk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YVAgzwy+\"; dkim-atps=neutral","Date":"Tue, 9 Aug 2022 04:59:03 +0300","To":"Paul Elder ","Message-ID":"","References":"<20220803112150.3040287-1-paul.elder@ideasonboard.com>\n\t<20220803112150.3040287-7-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220803112150.3040287-7-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 6/9] 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":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]