[RFC,v1,7/8] libcamera: ipa_data_serializer: Add specialization for enums
diff mbox series

Message ID 20250515120012.3127231-8-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • utils: codegen: ipc: Simplify deserialization
Related show

Commit Message

Barnabás Pőcze May 15, 2025, noon UTC 
Instead of handling enums specially in the code generation templates,
create a specialization of `IPADataSerializer` that handles enums.

To stay compatible, encode every enum as a `uint32_t`, and also
static_assert that the given enum type is smaller.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 .../libcamera/internal/ipa_data_serializer.h  | 48 ++++++++++++++++++-
 .../libcamera_templates/proxy_functions.tmpl  | 17 +------
 .../libcamera_templates/serializer.tmpl       | 14 ------
 3 files changed, 48 insertions(+), 31 deletions(-)

Comments

Paul Elder May 15, 2025, 1:39 p.m. UTC | #1 
Quoting Barnabás Pőcze (2025-05-15 14:00:11)
> Instead of handling enums specially in the code generation templates,
> create a specialization of `IPADataSerializer` that handles enums.
> 
> To stay compatible, encode every enum as a `uint32_t`, and also
> static_assert that the given enum type is smaller.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  .../libcamera/internal/ipa_data_serializer.h  | 48 ++++++++++++++++++-
>  .../libcamera_templates/proxy_functions.tmpl  | 17 +------
>  .../libcamera_templates/serializer.tmpl       | 14 ------

That is a nice cleanup.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

>  3 files changed, 48 insertions(+), 31 deletions(-)
> 
> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
> index b1fefba58..564f59e25 100644
> --- a/include/libcamera/internal/ipa_data_serializer.h
> +++ b/include/libcamera/internal/ipa_data_serializer.h
> @@ -61,7 +61,7 @@ T readPOD(std::vector<uint8_t> &vec, size_t pos)
>  
>  } /* namespace */
>  
> -template<typename T>
> +template<typename T, typename = void>
>  class IPADataSerializer
>  {
>  public:
> @@ -344,6 +344,52 @@ public:
>         }
>  };
>  
> +template<typename E>
> +class IPADataSerializer<E, std::enable_if_t<std::is_enum_v<E>>>
> +{
> +       using U = uint32_t;
> +       static_assert(sizeof(E) <= sizeof(U));
> +
> +public:
> +       static std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
> +       serialize(const E &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> +       {
> +               std::vector<uint8_t> dataVec;
> +               appendPOD<U>(dataVec, static_cast<U>(data));
> +
> +               return { dataVec, {} };
> +       }
> +
> +       static E deserialize(std::vector<uint8_t> &data,
> +                            [[maybe_unused]] ControlSerializer *cs = nullptr)
> +       {
> +               return deserialize(data.cbegin(), data.cend());
> +       }
> +
> +       static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +                            std::vector<uint8_t>::const_iterator dataEnd,
> +                            [[maybe_unused]] ControlSerializer *cs = nullptr)
> +       {
> +               return static_cast<E>(readPOD<U>(dataBegin, 0, dataEnd));
> +       }
> +
> +       static E deserialize(std::vector<uint8_t> &data,
> +                           [[maybe_unused]] std::vector<SharedFD> &fds,
> +                           [[maybe_unused]] ControlSerializer *cs = nullptr)
> +       {
> +               return deserialize(data.cbegin(), data.cend());
> +       }
> +
> +       static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +                            std::vector<uint8_t>::const_iterator dataEnd,
> +                            [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
> +                            [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
> +                            [[maybe_unused]] ControlSerializer *cs = nullptr)
> +       {
> +               return deserialize(dataBegin, dataEnd);
> +       }
> +};
> +
>  #endif /* __DOXYGEN__ */
>  
>  } /* namespace libcamera */
> diff --git a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
> index 25476990e..01e2567ca 100644
> --- a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
> +++ b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
> @@ -52,9 +52,6 @@
>   #}
>  {%- macro serialize_call(params, buf, fds) %}
>  {%- for param in params %}
> -{%- if param|is_enum %}
> -       static_assert(sizeof({{param|name_full}}) <= 4);
> -{%- endif %}
>         std::vector<uint8_t> {{param.mojom_name}}Buf;
>  {%- if param|has_fd %}
>         std::vector<SharedFD> {{param.mojom_name}}Fds;
> @@ -62,13 +59,7 @@
>  {%- else %}
>         std::tie({{param.mojom_name}}Buf, std::ignore) =
>  {%- endif %}
> -{%- if param|is_flags %}
>                 IPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}
> -{%- elif param|is_enum %}
> -               IPADataSerializer<uint32_t>::serialize(static_cast<uint32_t>({{param.mojom_name}})
> -{%- else %}
> -               IPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}
> -{% endif -%}
>  {{- ", &controlSerializer_" if param|needs_control_serializer -}}
>  );
>  {%- endfor %}
> @@ -107,13 +98,7 @@
>   #}
>  {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}
>  {{"*" if pointer}}{{param.mojom_name}} =
> -{%- if param|is_flags %}
>  IPADataSerializer<{{param|name_full}}>::deserialize(
> -{%- elif param|is_enum %}
> -static_cast<{{param|name_full}}>(IPADataSerializer<uint32_t>::deserialize(
> -{%- else %}
> -IPADataSerializer<{{param|name}}>::deserialize(
> -{%- endif %}
>         {{buf}}{{- ".cbegin()" if not iter}} + {{param.mojom_name}}Start,
>  {%- if loop.last and not iter %}
>         {{buf}}.cend()
> @@ -137,7 +122,7 @@ IPADataSerializer<{{param|name}}>::deserialize(
>  {%- if param|needs_control_serializer %}
>         &controlSerializer_
>  {%- endif -%}
> -){{")" if param|is_enum and not param|is_flags}};
> +);
>  {%- endmacro -%}
>  
>  
> diff --git a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
> index d07836cc1..e316dd88a 100644
> --- a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
> +++ b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
> @@ -32,15 +32,7 @@
>  {%- if field|is_pod or field|is_enum %}
>                 std::vector<uint8_t> {{field.mojom_name}};
>                 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_scoped %}
> -                       IPADataSerializer<uint{{field|bit_width}}_t>::serialize(static_cast<uint{{field|bit_width}}_t>(data.{{field.mojom_name}}));
> -       {%- elif field|is_enum %}
> -                       IPADataSerializer<uint{{field|bit_width}}_t>::serialize(data.{{field.mojom_name}});
> -       {%- endif %}
>                 retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
>  {%- elif field|is_fd %}
>                 std::vector<uint8_t> {{field.mojom_name}};
> @@ -98,13 +90,7 @@
>  {% if field|is_pod or field|is_enum %}
>         {%- set field_size = (field|bit_width|int / 8)|int %}
>                 {{- 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<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
> -               {%- endif %}
>         {%- if not loop.last %}
>                 m += {{field_size}};
>                 dataSize -= {{field_size}};
> -- 
> 2.49.0
>
Jacopo Mondi May 20, 2025, 1:11 p.m. UTC | #2 
Hi Barnabás

On Thu, May 15, 2025 at 02:00:11PM +0200, Barnabás Pőcze wrote:
> Instead of handling enums specially in the code generation templates,
> create a specialization of `IPADataSerializer` that handles enums.
>
> To stay compatible, encode every enum as a `uint32_t`, and also
> static_assert that the given enum type is smaller.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  .../libcamera/internal/ipa_data_serializer.h  | 48 ++++++++++++++++++-
>  .../libcamera_templates/proxy_functions.tmpl  | 17 +------
>  .../libcamera_templates/serializer.tmpl       | 14 ------
>  3 files changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
> index b1fefba58..564f59e25 100644
> --- a/include/libcamera/internal/ipa_data_serializer.h
> +++ b/include/libcamera/internal/ipa_data_serializer.h
> @@ -61,7 +61,7 @@ T readPOD(std::vector<uint8_t> &vec, size_t pos)
>
>  } /* namespace */
>
> -template<typename T>
> +template<typename T, typename = void>

It took me a bit of time to understand why, give the above

>  class IPADataSerializer
>  {
>  public:
> @@ -344,6 +344,52 @@ public:
>  	}
>  };
>
> +template<typename E>
> +class IPADataSerializer<E, std::enable_if_t<std::is_enum_v<E>>>

This could work for other E that are not enums, or in other words, I
was expecting that we need to have an overload for !std::is_enum<>

Later I realized that all other specializations here are for containers
like vector<E> map<K, E> and Flag<E>, and all other types will have a
generated specialization.

Do you know how many copies of this function will be generated by the
compiler ? One for each enum type specified in the interface file ?

Anyway, looks good to me
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>


> +{
> +	using U = uint32_t;
> +	static_assert(sizeof(E) <= sizeof(U));
> +
> +public:
> +	static std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
> +	serialize(const E &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		std::vector<uint8_t> dataVec;
> +		appendPOD<U>(dataVec, static_cast<U>(data));
> +
> +		return { dataVec, {} };
> +	}
> +
> +	static E deserialize(std::vector<uint8_t> &data,
> +			     [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.cend());
> +	}
> +
> +	static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +			     std::vector<uint8_t>::const_iterator dataEnd,
> +			     [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return static_cast<E>(readPOD<U>(dataBegin, 0, dataEnd));
> +	}
> +
> +	static E deserialize(std::vector<uint8_t> &data,
> +			    [[maybe_unused]] std::vector<SharedFD> &fds,
> +			    [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(data.cbegin(), data.cend());
> +	}
> +
> +	static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +			     std::vector<uint8_t>::const_iterator dataEnd,
> +			     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
> +			     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
> +			     [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return deserialize(dataBegin, dataEnd);
> +	}
> +};
> +
>  #endif /* __DOXYGEN__ */
>
>  } /* namespace libcamera */
> diff --git a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
> index 25476990e..01e2567ca 100644
> --- a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
> +++ b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
> @@ -52,9 +52,6 @@
>   #}
>  {%- macro serialize_call(params, buf, fds) %}
>  {%- for param in params %}
> -{%- if param|is_enum %}
> -	static_assert(sizeof({{param|name_full}}) <= 4);
> -{%- endif %}
>  	std::vector<uint8_t> {{param.mojom_name}}Buf;
>  {%- if param|has_fd %}
>  	std::vector<SharedFD> {{param.mojom_name}}Fds;
> @@ -62,13 +59,7 @@
>  {%- else %}
>  	std::tie({{param.mojom_name}}Buf, std::ignore) =
>  {%- endif %}
> -{%- if param|is_flags %}
>  		IPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}
> -{%- elif param|is_enum %}
> -		IPADataSerializer<uint32_t>::serialize(static_cast<uint32_t>({{param.mojom_name}})
> -{%- else %}
> -		IPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}
> -{% endif -%}
>  {{- ", &controlSerializer_" if param|needs_control_serializer -}}
>  );
>  {%- endfor %}
> @@ -107,13 +98,7 @@
>   #}
>  {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}
>  {{"*" if pointer}}{{param.mojom_name}} =
> -{%- if param|is_flags %}
>  IPADataSerializer<{{param|name_full}}>::deserialize(
> -{%- elif param|is_enum %}
> -static_cast<{{param|name_full}}>(IPADataSerializer<uint32_t>::deserialize(
> -{%- else %}
> -IPADataSerializer<{{param|name}}>::deserialize(
> -{%- endif %}
>  	{{buf}}{{- ".cbegin()" if not iter}} + {{param.mojom_name}}Start,
>  {%- if loop.last and not iter %}
>  	{{buf}}.cend()
> @@ -137,7 +122,7 @@ IPADataSerializer<{{param|name}}>::deserialize(
>  {%- if param|needs_control_serializer %}
>  	&controlSerializer_
>  {%- endif -%}
> -){{")" if param|is_enum and not param|is_flags}};
> +);
>  {%- endmacro -%}
>
>
> diff --git a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
> index d07836cc1..e316dd88a 100644
> --- a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
> +++ b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
> @@ -32,15 +32,7 @@
>  {%- if field|is_pod or field|is_enum %}
>  		std::vector<uint8_t> {{field.mojom_name}};
>  		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_scoped %}
> -			IPADataSerializer<uint{{field|bit_width}}_t>::serialize(static_cast<uint{{field|bit_width}}_t>(data.{{field.mojom_name}}));
> -	{%- elif field|is_enum %}
> -			IPADataSerializer<uint{{field|bit_width}}_t>::serialize(data.{{field.mojom_name}});
> -	{%- endif %}
>  		retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
>  {%- elif field|is_fd %}
>  		std::vector<uint8_t> {{field.mojom_name}};
> @@ -98,13 +90,7 @@
>  {% if field|is_pod or field|is_enum %}
>  	{%- set field_size = (field|bit_width|int / 8)|int %}
>  		{{- 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<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
> -		{%- endif %}
>  	{%- if not loop.last %}
>  		m += {{field_size}};
>  		dataSize -= {{field_size}};
> --
> 2.49.0
>
Barnabás Pőcze May 20, 2025, 1:56 p.m. UTC | #3 
Hi


2025. 05. 20. 15:11 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Thu, May 15, 2025 at 02:00:11PM +0200, Barnabás Pőcze wrote:
>> Instead of handling enums specially in the code generation templates,
>> create a specialization of `IPADataSerializer` that handles enums.
>>
>> To stay compatible, encode every enum as a `uint32_t`, and also
>> static_assert that the given enum type is smaller.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   .../libcamera/internal/ipa_data_serializer.h  | 48 ++++++++++++++++++-
>>   .../libcamera_templates/proxy_functions.tmpl  | 17 +------
>>   .../libcamera_templates/serializer.tmpl       | 14 ------
>>   3 files changed, 48 insertions(+), 31 deletions(-)
>>
>> diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
>> index b1fefba58..564f59e25 100644
>> --- a/include/libcamera/internal/ipa_data_serializer.h
>> +++ b/include/libcamera/internal/ipa_data_serializer.h
>> @@ -61,7 +61,7 @@ T readPOD(std::vector<uint8_t> &vec, size_t pos)
>>
>>   } /* namespace */
>>
>> -template<typename T>
>> +template<typename T, typename = void>
> 
> It took me a bit of time to understand why, give the above

Yes, this could be better in C++20:

   template<typename E>
   	requires (std::is_enum_v<E>)
   class IPADataSerializer<E> { ... };

which is easier to understand and does not require modification of the main template.


> 
>>   class IPADataSerializer
>>   {
>>   public:
>> @@ -344,6 +344,52 @@ public:
>>   	}
>>   };
>>
>> +template<typename E>
>> +class IPADataSerializer<E, std::enable_if_t<std::is_enum_v<E>>>
> 
> This could work for other E that are not enums, or in other words, I
> was expecting that we need to have an overload for !std::is_enum<>
> 
> Later I realized that all other specializations here are for containers
> like vector<E> map<K, E> and Flag<E>, and all other types will have a
> generated specialization.
> 
> Do you know how many copies of this function will be generated by the
> compiler ? One for each enum type specified in the interface file ?

This specialization will be instantiated for every enum type that goes
through (de)serialization. (At the moment such enums are only used in vimc,
as far as I remember.) But I generally expect the functions to be inlined.


Regards,
Barnabás Pőcze


> 
> Anyway, looks good to me
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> 
>> +{
>> +	using U = uint32_t;
>> +	static_assert(sizeof(E) <= sizeof(U));
>> +
>> +public:
>> +	static std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
>> +	serialize(const E &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
>> +	{
>> +		std::vector<uint8_t> dataVec;
>> +		appendPOD<U>(dataVec, static_cast<U>(data));
>> +
>> +		return { dataVec, {} };
>> +	}
>> +
>> +	static E deserialize(std::vector<uint8_t> &data,
>> +			     [[maybe_unused]] ControlSerializer *cs = nullptr)
>> +	{
>> +		return deserialize(data.cbegin(), data.cend());
>> +	}
>> +
>> +	static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> +			     std::vector<uint8_t>::const_iterator dataEnd,
>> +			     [[maybe_unused]] ControlSerializer *cs = nullptr)
>> +	{
>> +		return static_cast<E>(readPOD<U>(dataBegin, 0, dataEnd));
>> +	}
>> +
>> +	static E deserialize(std::vector<uint8_t> &data,
>> +			    [[maybe_unused]] std::vector<SharedFD> &fds,
>> +			    [[maybe_unused]] ControlSerializer *cs = nullptr)
>> +	{
>> +		return deserialize(data.cbegin(), data.cend());
>> +	}
>> +
>> +	static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>> +			     std::vector<uint8_t>::const_iterator dataEnd,
>> +			     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
>> +			     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
>> +			     [[maybe_unused]] ControlSerializer *cs = nullptr)
>> +	{
>> +		return deserialize(dataBegin, dataEnd);
>> +	}
>> +};
>> +
>>   #endif /* __DOXYGEN__ */
>>
>>   } /* namespace libcamera */
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
>> index 25476990e..01e2567ca 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
>> @@ -52,9 +52,6 @@
>>    #}
>>   {%- macro serialize_call(params, buf, fds) %}
>>   {%- for param in params %}
>> -{%- if param|is_enum %}
>> -	static_assert(sizeof({{param|name_full}}) <= 4);
>> -{%- endif %}
>>   	std::vector<uint8_t> {{param.mojom_name}}Buf;
>>   {%- if param|has_fd %}
>>   	std::vector<SharedFD> {{param.mojom_name}}Fds;
>> @@ -62,13 +59,7 @@
>>   {%- else %}
>>   	std::tie({{param.mojom_name}}Buf, std::ignore) =
>>   {%- endif %}
>> -{%- if param|is_flags %}
>>   		IPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}
>> -{%- elif param|is_enum %}
>> -		IPADataSerializer<uint32_t>::serialize(static_cast<uint32_t>({{param.mojom_name}})
>> -{%- else %}
>> -		IPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}
>> -{% endif -%}
>>   {{- ", &controlSerializer_" if param|needs_control_serializer -}}
>>   );
>>   {%- endfor %}
>> @@ -107,13 +98,7 @@
>>    #}
>>   {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}
>>   {{"*" if pointer}}{{param.mojom_name}} =
>> -{%- if param|is_flags %}
>>   IPADataSerializer<{{param|name_full}}>::deserialize(
>> -{%- elif param|is_enum %}
>> -static_cast<{{param|name_full}}>(IPADataSerializer<uint32_t>::deserialize(
>> -{%- else %}
>> -IPADataSerializer<{{param|name}}>::deserialize(
>> -{%- endif %}
>>   	{{buf}}{{- ".cbegin()" if not iter}} + {{param.mojom_name}}Start,
>>   {%- if loop.last and not iter %}
>>   	{{buf}}.cend()
>> @@ -137,7 +122,7 @@ IPADataSerializer<{{param|name}}>::deserialize(
>>   {%- if param|needs_control_serializer %}
>>   	&controlSerializer_
>>   {%- endif -%}
>> -){{")" if param|is_enum and not param|is_flags}};
>> +);
>>   {%- endmacro -%}
>>
>>
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
>> index d07836cc1..e316dd88a 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
>> @@ -32,15 +32,7 @@
>>   {%- if field|is_pod or field|is_enum %}
>>   		std::vector<uint8_t> {{field.mojom_name}};
>>   		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_scoped %}
>> -			IPADataSerializer<uint{{field|bit_width}}_t>::serialize(static_cast<uint{{field|bit_width}}_t>(data.{{field.mojom_name}}));
>> -	{%- elif field|is_enum %}
>> -			IPADataSerializer<uint{{field|bit_width}}_t>::serialize(data.{{field.mojom_name}});
>> -	{%- endif %}
>>   		retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
>>   {%- elif field|is_fd %}
>>   		std::vector<uint8_t> {{field.mojom_name}};
>> @@ -98,13 +90,7 @@
>>   {% if field|is_pod or field|is_enum %}
>>   	{%- set field_size = (field|bit_width|int / 8)|int %}
>>   		{{- 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<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
>> -		{%- endif %}
>>   	{%- if not loop.last %}
>>   		m += {{field_size}};
>>   		dataSize -= {{field_size}};
>> --
>> 2.49.0
>>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/ipa_data_serializer.h b/include/libcamera/internal/ipa_data_serializer.h
index b1fefba58..564f59e25 100644
--- a/include/libcamera/internal/ipa_data_serializer.h
+++ b/include/libcamera/internal/ipa_data_serializer.h
@@ -61,7 +61,7 @@  T readPOD(std::vector<uint8_t> &vec, size_t pos)
 
 } /* namespace */
 
-template<typename T>
+template<typename T, typename = void>
 class IPADataSerializer
 {
 public:
@@ -344,6 +344,52 @@  public:
 	}
 };
 
+template<typename E>
+class IPADataSerializer<E, std::enable_if_t<std::is_enum_v<E>>>
+{
+	using U = uint32_t;
+	static_assert(sizeof(E) <= sizeof(U));
+
+public:
+	static std::tuple<std::vector<uint8_t>, std::vector<SharedFD>>
+	serialize(const E &data, [[maybe_unused]] ControlSerializer *cs = nullptr)
+	{
+		std::vector<uint8_t> dataVec;
+		appendPOD<U>(dataVec, static_cast<U>(data));
+
+		return { dataVec, {} };
+	}
+
+	static E deserialize(std::vector<uint8_t> &data,
+			     [[maybe_unused]] ControlSerializer *cs = nullptr)
+	{
+		return deserialize(data.cbegin(), data.cend());
+	}
+
+	static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
+			     std::vector<uint8_t>::const_iterator dataEnd,
+			     [[maybe_unused]] ControlSerializer *cs = nullptr)
+	{
+		return static_cast<E>(readPOD<U>(dataBegin, 0, dataEnd));
+	}
+
+	static E deserialize(std::vector<uint8_t> &data,
+			    [[maybe_unused]] std::vector<SharedFD> &fds,
+			    [[maybe_unused]] ControlSerializer *cs = nullptr)
+	{
+		return deserialize(data.cbegin(), data.cend());
+	}
+
+	static E deserialize(std::vector<uint8_t>::const_iterator dataBegin,
+			     std::vector<uint8_t>::const_iterator dataEnd,
+			     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsBegin,
+			     [[maybe_unused]] std::vector<SharedFD>::const_iterator fdsEnd,
+			     [[maybe_unused]] ControlSerializer *cs = nullptr)
+	{
+		return deserialize(dataBegin, dataEnd);
+	}
+};
+
 #endif /* __DOXYGEN__ */
 
 } /* namespace libcamera */
diff --git a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
index 25476990e..01e2567ca 100644
--- a/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
+++ b/utils/codegen/ipc/generators/libcamera_templates/proxy_functions.tmpl
@@ -52,9 +52,6 @@ 
  #}
 {%- macro serialize_call(params, buf, fds) %}
 {%- for param in params %}
-{%- if param|is_enum %}
-	static_assert(sizeof({{param|name_full}}) <= 4);
-{%- endif %}
 	std::vector<uint8_t> {{param.mojom_name}}Buf;
 {%- if param|has_fd %}
 	std::vector<SharedFD> {{param.mojom_name}}Fds;
@@ -62,13 +59,7 @@ 
 {%- else %}
 	std::tie({{param.mojom_name}}Buf, std::ignore) =
 {%- endif %}
-{%- if param|is_flags %}
 		IPADataSerializer<{{param|name_full}}>::serialize({{param.mojom_name}}
-{%- elif param|is_enum %}
-		IPADataSerializer<uint32_t>::serialize(static_cast<uint32_t>({{param.mojom_name}})
-{%- else %}
-		IPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}
-{% endif -%}
 {{- ", &controlSerializer_" if param|needs_control_serializer -}}
 );
 {%- endfor %}
@@ -107,13 +98,7 @@ 
  #}
 {%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}
 {{"*" if pointer}}{{param.mojom_name}} =
-{%- if param|is_flags %}
 IPADataSerializer<{{param|name_full}}>::deserialize(
-{%- elif param|is_enum %}
-static_cast<{{param|name_full}}>(IPADataSerializer<uint32_t>::deserialize(
-{%- else %}
-IPADataSerializer<{{param|name}}>::deserialize(
-{%- endif %}
 	{{buf}}{{- ".cbegin()" if not iter}} + {{param.mojom_name}}Start,
 {%- if loop.last and not iter %}
 	{{buf}}.cend()
@@ -137,7 +122,7 @@  IPADataSerializer<{{param|name}}>::deserialize(
 {%- if param|needs_control_serializer %}
 	&controlSerializer_
 {%- endif -%}
-){{")" if param|is_enum and not param|is_flags}};
+);
 {%- endmacro -%}
 
 
diff --git a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
index d07836cc1..e316dd88a 100644
--- a/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
+++ b/utils/codegen/ipc/generators/libcamera_templates/serializer.tmpl
@@ -32,15 +32,7 @@ 
 {%- if field|is_pod or field|is_enum %}
 		std::vector<uint8_t> {{field.mojom_name}};
 		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_scoped %}
-			IPADataSerializer<uint{{field|bit_width}}_t>::serialize(static_cast<uint{{field|bit_width}}_t>(data.{{field.mojom_name}}));
-	{%- elif field|is_enum %}
-			IPADataSerializer<uint{{field|bit_width}}_t>::serialize(data.{{field.mojom_name}});
-	{%- endif %}
 		retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
 {%- elif field|is_fd %}
 		std::vector<uint8_t> {{field.mojom_name}};
@@ -98,13 +90,7 @@ 
 {% if field|is_pod or field|is_enum %}
 	{%- set field_size = (field|bit_width|int / 8)|int %}
 		{{- 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<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
-		{%- endif %}
 	{%- if not loop.last %}
 		m += {{field_size}};
 		dataSize -= {{field_size}};