[libcamera-devel] [PATCH] utils: ipc: Support types defined in core.mojom with skipHeader only

Umang Jain umang.jain at ideasonboard.com
Thu Apr 22 08:44:51 CEST 2021


Hi Paul

Thanks for the patch, I had a chance to test it this morning and it does 
solve the `Size` errors.

However, if I try to wrap CameraSensorInfo, there is a compile time error:
In file included from src/libcamera/proxy/ipu3_ipa_proxy.cpp:10:
In file included from include/libcamera/ipa/ipu3_ipa_proxy.h:14:
include/libcamera/ipa/ipu3_ipa_interface.h:101:19: error: field has 
incomplete type 'libcamera::CameraSensorInfo'
         CameraSensorInfo sensorInfo;
                          ^
../../../../../tmp/portage/media-libs/libcamera-9999/work/libcamera-9999/include/libcamera/ipa/ipa_interface.h:28:8: 
note: forward declaration of 'libcamera::CameraSensorInfo'
struct CameraSensorInfo;
        ^
1 error generated.

Is that something you are aware of? The only difference(with my limited 
knowledge about mojom) I can spot is `Size` comes from a header whereas 
CameraSensorInfo if forward-declared in ipa_interface.h

I have pushed a branch which can help you reproduce the issue on a ipu3 
device - https://github.com/uajain/libcamera/commits/uajain/mojom-namespace
Full build log : https://paste.gnome.org/pror88r8t

Please let me know if you need anything else, from my end.


On 4/21/21 3:56 PM, Paul Elder wrote:
> Previously, types that were defined in core.mojom with skipHeader only
> and no skipSerdes would cause serializer generation to fail due to the
> generated IPA namespace being used for mojom-defined types instead of
> the libcamera namespace. This doesn't work because skipHeader means that
> the type is defined in the libcamera (or some other) namespace
> elsewhere.
>
> Fix this by using "using namespace" in the generated serializer header,
> so that mojom-defined IPA types can be used without the full namespace
> qualifier.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>   .../module_ipa_serializer.h.tmpl              |  6 +++
>   .../libcamera_templates/serializer.tmpl       | 38 +++++++++----------
>   2 files changed, 24 insertions(+), 20 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..d0bac62d 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
> @@ -27,6 +27,12 @@
>   
>   namespace libcamera {
>   
> +{%- if has_namespace %}
> +{% for ns in namespace -%}
> +using namespace {{ns}};
> +{% endfor %}
> +{%- endif %}
> +
>   LOG_DECLARE_CATEGORY(IPADataSerializer)
>   {% for struct in structs_nonempty %}
>   template<>
> diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> index af35b9e3..afabd8e3 100644
> --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> @@ -63,12 +63,10 @@
>   	{%- else %}
>   		std::tie({{field.mojom_name}}, std::ignore) =
>   	{%- endif %}
> -	{%- if field|is_array or field|is_map %}
> -			IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}, cs);
> -	{%- elif field|is_str %}
> +	{%- if 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}}>::serialize(data.{{field.mojom_name}}, cs);
>   	{%- endif %}
>   		appendPOD<uint32_t>(retData, {{field.mojom_name}}.size());
>   	{%- if field|has_fd %}
> @@ -97,7 +95,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}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
>   		{%- endif %}
>   	{%- if not loop.last %}
>   		m += {{field_size}};
> @@ -150,11 +148,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}}>::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}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);
>   	{%- endif %}
>   	{%- if not loop.last %}
>   		m += {{field_size}};
> @@ -178,7 +176,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}} &data,
>   {%- if struct|needs_control_serializer %}
>   		  ControlSerializer *cs)
>   {%- else %}
> @@ -208,7 +206,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}}
>   	deserialize(std::vector<uint8_t> &data,
>   		    std::vector<int32_t> &fds,
>   {%- if struct|needs_control_serializer %}
> @@ -217,11 +215,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}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);
>   	}
>   
>   {# \todo Don't inline this function #}
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name}}
>   	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>   		    std::vector<uint8_t>::const_iterator dataEnd,
>   		    std::vector<int32_t>::const_iterator fdsBegin,
> @@ -232,7 +230,7 @@
>   		    [[maybe_unused]] ControlSerializer *cs = nullptr)
>   {%- endif %}
>   	{
> -		{{struct|name_full(namespace)}} ret;
> +		{{struct|name}} ret;
>   		std::vector<uint8_t>::const_iterator m = dataBegin;
>   		std::vector<int32_t>::const_iterator n = fdsBegin;
>   
> @@ -253,22 +251,22 @@
>    # deserializers with file descriptors.
>    #}
>   {%- macro deserializer_fd_simple(struct, namespace) %}
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name}}
>   	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}}>::deserialize(data.cbegin(), data.cend(), cs);
>   	}
>   
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name}}
>   	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}}>::deserialize(dataBegin, dataEnd, cs);
>   	}
>   {%- endmacro %}
>   
> @@ -280,7 +278,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}}
>   	deserialize(std::vector<uint8_t> &data,
>   {%- if struct|needs_control_serializer %}
>   		    ControlSerializer *cs)
> @@ -288,11 +286,11 @@
>   		    ControlSerializer *cs = nullptr)
>   {%- endif %}
>   	{
> -		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs);
> +		return IPADataSerializer<{{struct|name}}>::deserialize(data.cbegin(), data.cend(), cs);
>   	}
>   
>   {# \todo Don't inline this function #}
> -	static {{struct|name_full(namespace)}}
> +	static {{struct|name}}
>   	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
>   		    std::vector<uint8_t>::const_iterator dataEnd,
>   {%- if struct|needs_control_serializer %}
> @@ -301,7 +299,7 @@
>   		    [[maybe_unused]] ControlSerializer *cs = nullptr)
>   {%- endif %}
>   	{
> -		{{struct|name_full(namespace)}} ret;
> +		{{struct|name}} ret;
>   		std::vector<uint8_t>::const_iterator m = dataBegin;
>   
>   		size_t dataSize = std::distance(dataBegin, dataEnd);



More information about the libcamera-devel mailing list