[libcamera-devel] [PATCH v2 4/7] utils: ipc: Add support for Flags
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Thu Aug 18 08:56:26 CEST 2022
On Thu, Aug 18, 2022 at 03:49:19PM +0900, Paul Elder wrote:
> Add Flags<E> 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<E>" and appropriate
> serialization/deserialization code for Flags<E> 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 <paul.elder at ideasonboard.com>
>
> ---
> 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.
It turns out that since you can make Flags out of an enum that isn't a
class, maybe this isn't even necessary? I just noticed in the
generated_serializer test (next patch) I forgot to put [Flags] in the
enum definition but in the [TEST] patch (at the end of the series) I
did.
Not sure which is better.
Paul
>
> 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<E>,
> 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 <map>
> #include <vector>
>
> +#include <libcamera/base/flags.h>
> #include <libcamera/base/signal.h>
>
> #include <libcamera/controls.h>
> 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<uint32_t>::serialize(static_cast<uint32_t>({{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<uint32_t>::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<uint{{field|bit_width}}_t>::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<uint{{field|bit_width}}_t>::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,
> --
> 2.30.2
>
More information about the libcamera-devel
mailing list