[libcamera-devel] [PATCH v3 2/3] utils: ipc: Use the proper namespace for mojom structs

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Apr 26 01:35:06 CEST 2021


Hi Paul,

Thank you for the patch.

On Fri, Apr 23, 2021 at 07:47:10PM +0900, Paul Elder wrote:
> Structs defined in mojom previously used the namespace of the mojom file
> that was being used as the source. This is obviously not the correct
> namespace for structs that are defined in core.mojom. Fix the jinja
> function for getting the element type including namespace, and use it.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>

Nice cleanup.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
> New in v3
> ---
>  .../module_ipa_serializer.h.tmpl              |  2 +-
>  .../libcamera_templates/serializer.tmpl       | 34 +++++++++----------
>  .../generators/mojom_libcamera_generator.py   | 16 ++++++---
>  3 files changed, 30 insertions(+), 22 deletions(-)
> 
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
> index 64ae99dc..779d2114 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
> @@ -30,7 +30,7 @@ namespace libcamera {
>  LOG_DECLARE_CATEGORY(IPADataSerializer)
>  {% for struct in structs_nonempty %}
>  template<>
> -class IPADataSerializer<{{struct|name_full(namespace_str)}}>
> +class IPADataSerializer<{{struct|name_full}}>
>  {
>  public:
>  {{- serializer.serializer(struct, namespace_str)}}
> diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> index af35b9e3..d8d55807 100644
> --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> @@ -68,7 +68,7 @@
>  	{%- elif field|is_str %}
>  			IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});
>  	{%- else %}
> -			IPADataSerializer<{{field|name_full(namespace)}}>::serialize(data.{{field.mojom_name}}, cs);
> +			IPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}}, cs);
>  	{%- endif %}
>  		appendPOD<uint32_t>(retData, {{field.mojom_name}}.size());
>  	{%- if field|has_fd %}
> @@ -97,7 +97,7 @@
>  		{%- if field|is_pod %}
>  		ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}});
>  		{%- else %}
> -		ret.{{field.mojom_name}} = static_cast<{{field|name_full(namespace)}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
> +		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}};
> @@ -150,11 +150,11 @@
>  	{%- elif field|has_fd and (field|is_array or field|is_map) %}
>  			IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs);
>  	{%- elif field|has_fd and (not (field|is_array or field|is_map)) %}
> -			IPADataSerializer<{{field|name_full(namespace)}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs);
> +			IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs);
>  	{%- elif (not field|has_fd) and (field|is_array or field|is_map) %}
>  			IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);
>  	{%- else %}
> -			IPADataSerializer<{{field|name_full(namespace)}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);
> +			IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);
>  	{%- endif %}
>  	{%- if not loop.last %}
>  		m += {{field_size}};
> @@ -178,7 +178,7 @@
>   #}
>  {%- macro serializer(struct, namespace) %}
>  	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> -	serialize(const {{struct|name_full(namespace)}} &data,
> +	serialize(const {{struct|name_full}} &data,
>  {%- if struct|needs_control_serializer %}
>  		  ControlSerializer *cs)
>  {%- else %}
> @@ -208,7 +208,7 @@
>   # \a struct, in the case that \a struct has file descriptors.
>   #}
>  {%- macro deserializer_fd(struct, namespace) %}
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name_full}}
>  	deserialize(std::vector<uint8_t> &data,
>  		    std::vector<int32_t> &fds,
>  {%- if struct|needs_control_serializer %}
> @@ -217,11 +217,11 @@
>  		    ControlSerializer *cs = nullptr)
>  {%- endif %}
>  	{
> -		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);
> +		return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);
>  	}
>  
>  {# \todo Don't inline this function #}
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name_full}}
>  	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>  		    std::vector<uint8_t>::const_iterator dataEnd,
>  		    std::vector<int32_t>::const_iterator fdsBegin,
> @@ -232,7 +232,7 @@
>  		    [[maybe_unused]] ControlSerializer *cs = nullptr)
>  {%- endif %}
>  	{
> -		{{struct|name_full(namespace)}} ret;
> +		{{struct|name_full}} ret;
>  		std::vector<uint8_t>::const_iterator m = dataBegin;
>  		std::vector<int32_t>::const_iterator n = fdsBegin;
>  
> @@ -253,22 +253,22 @@
>   # deserializers with file descriptors.
>   #}
>  {%- macro deserializer_fd_simple(struct, namespace) %}
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name_full}}
>  	deserialize(std::vector<uint8_t> &data,
>  		    [[maybe_unused]] std::vector<int32_t> &fds,
>  		    ControlSerializer *cs = nullptr)
>  	{
> -		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs);
> +		return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);
>  	}
>  
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name_full}}
>  	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>  		    std::vector<uint8_t>::const_iterator dataEnd,
>  		    [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin,
>  		    [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd,
>  		    ControlSerializer *cs = nullptr)
>  	{
> -		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(dataBegin, dataEnd, cs);
> +		return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs);
>  	}
>  {%- endmacro %}
>  
> @@ -280,7 +280,7 @@
>   # \a struct, in the case that \a struct does not have file descriptors.
>   #}
>  {%- macro deserializer_no_fd(struct, namespace) %}
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name_full}}
>  	deserialize(std::vector<uint8_t> &data,
>  {%- if struct|needs_control_serializer %}
>  		    ControlSerializer *cs)
> @@ -288,11 +288,11 @@
>  		    ControlSerializer *cs = nullptr)
>  {%- endif %}
>  	{
> -		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs);
> +		return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs);
>  	}
>  
>  {# \todo Don't inline this function #}
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name_full}}
>  	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>  		    std::vector<uint8_t>::const_iterator dataEnd,
>  {%- if struct|needs_control_serializer %}
> @@ -301,7 +301,7 @@
>  		    [[maybe_unused]] ControlSerializer *cs = nullptr)
>  {%- endif %}
>  	{
> -		{{struct|name_full(namespace)}} ret;
> +		{{struct|name_full}} ret;
>  		std::vector<uint8_t>::const_iterator m = dataBegin;
>  
>  		size_t dataSize = std::distance(dataBegin, dataEnd);
> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> index db9e28a6..effdfed6 100644
> --- a/utils/ipc/generators/mojom_libcamera_generator.py
> +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> @@ -258,12 +258,12 @@ def GetNameForElement(element):
>          return element.mojom_name
>      # vectors
>      if (mojom.IsArrayKind(element)):
> -        elem_name = GetNameForElement(element.kind)
> +        elem_name = GetFullNameForElement(element.kind)
>          return f'std::vector<{elem_name}>'
>      # maps
>      if (mojom.IsMapKind(element)):
> -        key_name = GetNameForElement(element.key_kind)
> -        value_name = GetNameForElement(element.value_kind)
> +        key_name = GetFullNameForElement(element.key_kind)
> +        value_name = GetFullNameForElement(element.value_kind)
>          return f'std::map<{key_name}, {value_name}>'
>      # struct fields and function parameters
>      if isinstance(element, (mojom.Field, mojom.Method, mojom.Parameter)):
> @@ -296,8 +296,16 @@ def GetNameForElement(element):
>          raise Exception('Unsupported element: %s' % element)
>      raise Exception('Unexpected element: %s' % element)
>  
> -def GetFullNameForElement(element, namespace_str):
> +def GetFullNameForElement(element):
>      name = GetNameForElement(element)
> +    namespace_str = ''
> +    if mojom.IsStructKind(element):
> +        namespace_str = element.module.mojom_namespace.replace('.', '::')
> +    elif (hasattr(element, 'kind') and
> +             (mojom.IsStructKind(element.kind) or
> +              mojom.IsEnumKind(element.kind))):
> +        namespace_str = element.kind.module.mojom_namespace.replace('.', '::')
> +
>      if namespace_str == '':
>          return name
>      return f'{namespace_str}::{name}'

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list