[{"id":36587,"web_url":"https://patchwork.libcamera.org/comment/36587/","msgid":"","date":"2025-11-01T09:00:31","subject":"Re: [RFC PATCH v2 1/2] libcamera: ipa_data_serializer: Add\n\tspecialization for enums","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Tue, Oct 14, 2025 at 01:57:51PM +0200, Barnabás Pőcze wrote:\n> Instead of handling enums specially in the code generation templates,\n> create a specialization of `IPADataSerializer` that handles enums.\n>\n> Every enum is serialized as a `uint32_t`, with `static_assert` to\n> ensure that every possible value fits. Previously, enums were\n> (de)serialized in `(de)serializer_field()` based on the size of\n> their underlying types. Afer this change, every enum is uniformly\n> handled as a `uint32_t`.\n>\n> Signed-off-by: Barnabás Pőcze \n> Reviewed-by: Paul Elder \n> Reviewed-by: Jacopo Mondi \n> ---\n> changes in v2:\n> * rebase\n>\n> v1: https://patchwork.libcamera.org/patch/23372/\n> ---\n> .../libcamera/internal/ipa_data_serializer.h | 48 ++++++++++++++++++-\n> .../libcamera_templates/proxy_functions.tmpl | 17 +------\n> .../libcamera_templates/serializer.tmpl | 14 ------\n> 3 files changed, 48 insertions(+), 31 deletions(-)\n>\n> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h\n> index b1fefba58..927f5c4c7 100644\n> --- a/include/libcamera/internal/ipa_data_serializer.h\n> +++ b/include/libcamera/internal/ipa_data_serializer.h\n> @@ -61,7 +61,7 @@ T readPOD(std::vector &vec, size_t pos)\n>\n> } /* namespace */\n>\n> -template\n> +template\n> class IPADataSerializer\n> {\n> public:\n> @@ -344,6 +344,52 @@ public:\n> \t}\n> };\n>\n> +template\n> +class IPADataSerializer>>\n> +{\n> +\tusing U = uint32_t;\n> +\tstatic_assert(sizeof(E) <= sizeof(U));\n> +\n> +public:\n> +\tstatic std::tuple, std::vector>\n> +\tserialize(const E &data, [[maybe_unused]] ControlSerializer *cs = nullptr)\n> +\t{\n> +\t\tstd::vector dataVec;\n> +\t\tappendPOD(dataVec, static_cast(data));\n> +\n> +\t\treturn { dataVec, {} };\n> +\t}\n> +\n> +\tstatic E deserialize(std::vector &data,\n> +\t\t\t [[maybe_unused]] ControlSerializer *cs = nullptr)\n> +\t{\n> +\t\treturn deserialize(data.cbegin(), data.cend());\n> +\t}\n> +\n> +\tstatic E deserialize(std::vector::const_iterator dataBegin,\n> +\t\t\t std::vector::const_iterator dataEnd,\n> +\t\t\t [[maybe_unused]] ControlSerializer *cs = nullptr)\n> +\t{\n> +\t\treturn static_cast(readPOD(dataBegin, 0, dataEnd));\n> +\t}\n> +\n> +\tstatic E deserialize(std::vector &data,\n> +\t\t\t [[maybe_unused]] std::vector &fds,\n> +\t\t\t [[maybe_unused]] ControlSerializer *cs = nullptr)\n> +\t{\n> +\t\treturn deserialize(data.cbegin(), data.cend());\n> +\t}\n> +\n> +\tstatic E deserialize(std::vector::const_iterator dataBegin,\n> +\t\t\t std::vector::const_iterator dataEnd,\n> +\t\t\t [[maybe_unused]] std::vector::const_iterator fdsBegin,\n> +\t\t\t [[maybe_unused]] std::vector::const_iterator fdsEnd,\n> +\t\t\t [[maybe_unused]] ControlSerializer *cs = nullptr)\n> +\t{\n> +\t\treturn deserialize(dataBegin, dataEnd);\n> +\t}\n> +};\n> +\n> #endif /* __DOXYGEN__ */\n>\n> } /* namespace libcamera */\n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> index 25476990e..01e2567ca 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n> @@ -52,9 +52,6 @@\n> #}\n> {%- macro serialize_call(params, buf, fds) %}\n> {%- for param in params %}\n> -{%- if param|is_enum %}\n> -\tstatic_assert(sizeof({{param|name_full}}) <= 4);\n> -{%- endif %}\n> \tstd::vector {{param.mojom_name}}Buf;\n> {%- if param|has_fd %}\n> \tstd::vector {{param.mojom_name}}Fds;\n> @@ -62,13 +59,7 @@\n> {%- else %}\n> \tstd::tie({{param.mojom_name}}Buf, std::ignore) =\n> {%- endif %}\n> -{%- if param|is_flags %}\n> \t\tIPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}\n\nOut of curiosity, what's the difference between:\n \t\tIPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}\n\t\tIPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}\n\n\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> -{% endif -%}\n> {{- \", &controlSerializer_\" if param|needs_control_serializer -}}\n> );\n> {%- endfor %}\n> @@ -107,13 +98,7 @@\n> #}\n> {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}\n> {{\"*\" if pointer}}{{param.mojom_name}} =\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\nEspecially becuase the !is_flag and !is_enum case seems to use the\n{param|name} version\n\n> -{%- endif %}\n> \t{{buf}}{{- \".cbegin()\" if not iter}} + {{param.mojom_name}}Start,\n> {%- if loop.last and not iter %}\n> \t{{buf}}.cend()\n> @@ -137,7 +122,7 @@ IPADataSerializer<{{param|name}}>::deserialize(\n> {%- if param|needs_control_serializer %}\n> \t&controlSerializer_\n> {%- endif -%}\n> -){{\")\" if param|is_enum and not param|is_flags}};\n> +);\n> {%- endmacro -%}\n>\n>\n> diff --git a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl\n> index d07836cc1..e316dd88a 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl\n> @@ -32,15 +32,7 @@\n> {%- if field|is_pod or field|is_enum %}\n> \t\tstd::vector {{field.mojom_name}};\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_scoped %}\n> -\t\t\tIPADataSerializer::serialize(static_cast(data.{{field.mojom_name}}));\n> -\t{%- elif field|is_enum %}\n> -\t\t\tIPADataSerializer::serialize(data.{{field.mojom_name}});\n> -\t{%- endif %}\n> \t\tretData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());\n> {%- elif field|is_fd %}\n> \t\tstd::vector {{field.mojom_name}};\n> @@ -98,13 +90,7 @@\n> {% if field|is_pod or field|is_enum %}\n> \t{%- set field_size = (field|bit_width|int / 8)|int %}\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> \t{%- if not loop.last %}\n> \t\tm += {{field_size}};\n> \t\tdataSize -= {{field_size}};\n> --\n> 2.51.0","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 27AC1BDE4C\n\tfor ;\n\tSat, 1 Nov 2025 09:00:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5C476609DE;\n\tSat, 1 Nov 2025 10:00:37 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73A76606A0\n\tfor ;\n\tSat, 1 Nov 2025 10:00:35 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:6462:5de2:153:f9b8:5024:faa2])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E83D31440;\n\tSat, 1 Nov 2025 09:58:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PV1I1mMY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761987524;\n\tbh=/c//mHyXQFFzASMnIuGnP3tMMpwaGE+BOhA1vEDCVZA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PV1I1mMYnGc6gbbuuV7D+letcIOzefi+gxlQgfSsjU4MDwy1Zuw6z/nYnnf3g1br+\n\t4mEFjghrEJDVx4vgEcTaLODww8Zb+kGmGai4cWdL4rnJa0vAKSeGh2pt2400qGnlYN\n\ttF0B5526jakb+KbXvNwT7FzvxvmUehbHIpmFs5L0=","Date":"Sat, 1 Nov 2025 10:00:31 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, \n\tPaul Elder ,\n\tJacopo Mondi ","Subject":"Re: [RFC PATCH v2 1/2] libcamera: ipa_data_serializer: Add\n\tspecialization for enums","Message-ID":"","References":"<20251014115752.466522-1-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20251014115752.466522-1-barnabas.pocze@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":36648,"web_url":"https://patchwork.libcamera.org/comment/36648/","msgid":"","date":"2025-11-03T11:41:22","subject":"Re: [RFC PATCH v2 1/2] libcamera: ipa_data_serializer: Add\n\tspecialization for enums","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 11. 01. 10:00 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Tue, Oct 14, 2025 at 01:57:51PM +0200, Barnabás Pőcze wrote:\n>> Instead of handling enums specially in the code generation templates,\n>> create a specialization of `IPADataSerializer` that handles enums.\n>>\n>> Every enum is serialized as a `uint32_t`, with `static_assert` to\n>> ensure that every possible value fits. Previously, enums were\n>> (de)serialized in `(de)serializer_field()` based on the size of\n>> their underlying types. Afer this change, every enum is uniformly\n>> handled as a `uint32_t`.\n>>\n>> Signed-off-by: Barnabás Pőcze \n>> Reviewed-by: Paul Elder \n>> Reviewed-by: Jacopo Mondi \n>> ---\n>> changes in v2:\n>> * rebase\n>>\n>> v1: https://patchwork.libcamera.org/patch/23372/\n>> ---\n>> .../libcamera/internal/ipa_data_serializer.h | 48 ++++++++++++++++++-\n>> .../libcamera_templates/proxy_functions.tmpl | 17 +------\n>> .../libcamera_templates/serializer.tmpl | 14 ------\n>> 3 files changed, 48 insertions(+), 31 deletions(-)\n>>\n> [...]\n>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n>> index 25476990e..01e2567ca 100644\n>> --- a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n>> +++ b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl\n>> @@ -52,9 +52,6 @@\n>> #}\n>> {%- macro serialize_call(params, buf, fds) %}\n>> {%- for param in params %}\n>> -{%- if param|is_enum %}\n>> -\tstatic_assert(sizeof({{param|name_full}}) <= 4);\n>> -{%- endif %}\n>> \tstd::vector {{param.mojom_name}}Buf;\n>> {%- if param|has_fd %}\n>> \tstd::vector {{param.mojom_name}}Fds;\n>> @@ -62,13 +59,7 @@\n>> {%- else %}\n>> \tstd::tie({{param.mojom_name}}Buf, std::ignore) =\n>> {%- endif %}\n>> -{%- if param|is_flags %}\n>> \t\tIPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}\n> \n> Out of curiosity, what's the difference between:\n> \t\tIPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}\n> \t\tIPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}\n> \n\n(see below)\n\n\n> \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>> -{% endif -%}\n>> {{- \", &controlSerializer_\" if param|needs_control_serializer -}}\n>> );\n>> {%- endfor %}\n>> @@ -107,13 +98,7 @@\n>> #}\n>> {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}\n>> {{\"*\" if pointer}}{{param.mojom_name}} =\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> \n> Especially becuase the !is_flag and !is_enum case seems to use the\n> {param|name} version\n\n`name_full` includes the namespace of the type. So using `name_full`\nshould always be usable instead of `name`, that's why I decided to\ngo with the fully qualified name everywhere.\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n>> -{%- endif %}\n>> \t{{buf}}{{- \".cbegin()\" if not iter}} + {{param.mojom_name}}Start,\n>> {%- if loop.last and not iter %}\n>> \t{{buf}}.cend()\n>> @@ -137,7 +122,7 @@ IPADataSerializer<{{param|name}}>::deserialize(\n>> {%- if param|needs_control_serializer %}\n>> \t&controlSerializer_\n>> {%- endif -%}\n>> -){{\")\" if param|is_enum and not param|is_flags}};\n>> +);\n>> {%- endmacro -%}\n> [...]","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 DD845C3241\n\tfor ;\n\tMon, 3 Nov 2025 11:41:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4C5ED609D8;\n\tMon, 3 Nov 2025 12:41:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 768EB606A0\n\tfor ;\n\tMon, 3 Nov 2025 12:41:26 +0100 (CET)","from [192.168.33.39] (185.221.140.239.nat.pool.zt.hu\n\t[185.221.140.239])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 59805111D;\n\tMon, 3 Nov 2025 12:39:33 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dv4t6DsL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1762169973;\n\tbh=TOrH4dN5il85hck4GK4S6/EPGaFk3wySFC59cDxNUWU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=dv4t6DsLJLtLMB6ISGzuJqtBWlafQCtm296HWVUPZZdfMMCAvPrZtqJInx1wwiITX\n\tv/QXbaPaqh/J+zHofWtBnOVyKcvlOyidTpGZqpTuvrgwoVX0N4s7SUDD5hpoflFH2M\n\tujCe6BQTiXC/nH4+rUDxJ/eZp/FPqCW/cXW2XWSg=","Message-ID":"","Date":"Mon, 3 Nov 2025 12:41:22 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2 1/2] libcamera: ipa_data_serializer: Add\n\tspecialization for enums","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder ","References":"<20251014115752.466522-1-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]