[libcamera-devel] [PATCH v6 2/9] utils: ipc: add templates for code generation for IPC mechanism

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue Feb 2 01:33:43 CET 2021


Hi Paul,

Thank you for the patch.

On Thu, Dec 24, 2020 at 05:15:27PM +0900, Paul Elder wrote:
> Add templates to mojo to generate code for the IPC mechanism. These
> templates generate:
> - module header
> - module serializer
> - IPA proxy cpp, header, and worker
> 
> Given an input data definition mojom file for a pipeline.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Acked-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> ---
> Changes in v6:
> - add templates for core_ipa_interface.h and core_ipa_serializer.h
>   - for libcamera types defined in mojom
> - rename everything to {{module_name}}_ipa_{interface,proxy,proxy_worker}.{c,h}
> - remove #include <libcamera/ipa/{{module_name}}.h
> - support customizable start()
> - remove the need for per-pipeline ControlInfoMap
> - add todo for avoiding intermediate vectors
> - remove postfix underscore for generated struct fields
> - support structs that are members of vectors/maps that aren't defined
>   in mojom (in mojom)
> - fix has_fd detection
> - namespacing is now required in mojom, in the form of ^ipa\.[0-9A-Za-z_]+
> - support consts in mojom
> - make the pseudo-switch-case in the python generator nicer
> 
> Changes in v5:
> - add a usage output to the proxy worker, to document the interface for
>   executing the proxy worker
> - in the mojom generator python script:
>   - removed unused things (imports, functions, jinja exports)
>   - document GetNameForElement
>   - rename everything cb -> event
>   - refactor Method{Input,Output}HasFd with a helper MethodParamsHaveFd
>   - add Get{Main,Event}Interface to fix the interface_{main,event} jinja
>     exports
>   - add copyright
>   - require that event interfaces have at least one event
> - expand copyright for templates
> - use new sendSync/sendAsync API (with IPCMessage)
> - rename a bunch of things
> 
> Changes in v4:
> For the non-template files:
> - rename IPA{pipeline_name}CallbackInterface to
>   IPA{pipeline_name}EventInterface
>   - to avoid the notion of "callback" and emphasize that it's an event
> - add support for strings in custom structs
> - add validation, that async methods must not have return values
>   - it throws exception and isn't very clear though...?
> - rename controls to libcamera::{pipeline_name}::controls (controls is
>   now lowercase)
> - rename {pipeline_name}_generated.h to {pipeline_name}_ipa_interface.h,
>   and {pipeline_name}_serializer.h to {pipeline_name}_ipa_serializer.h
>   - same for their corresponding template files
> For the template files:
> - fix spacing (now it's all {{var}} instead of some {{ var }})
>   - except if it's code, so code is still {{ code }}
> - move inclusion of corresponding header to first in the inclusion list
> - fix copy&paste errors
> - change snake_case to camelCase in the generated code
>   - template code still uses snake_case
> - change the generated command enums to an enum class, and make it
>   capitalized (instead of allcaps)
> - add length checks to recvIPC (in proxy)
> - fix some template spacing
> - don't use const for PODs in function/signal parameters
> - add the proper length checks to readPOD/appendPOD
>   - the helper functions for reading and writing PODs to and from
>     serialized data
> - rename readUInt/appendUInt to readPOD/appendPOD
> - add support for strings in custom structs
> 
> Changes in v3:
> - add support for namespaces
> - fix enum assignment (used to have +1 for CMD applied to all enums)
> - use readHeader, writeHeader, and eraseHeader as static class functions
>   of IPAIPCUnixSocket (in the proxy worker)
> - add requirement that base controls *must* be defined in
>   libcamera::{pipeline_name}::Controls
> 
> Changes in v2:
> - mandate the main and callback interfaces, and init(), start(), stop()
>   and their parameters
> - fix returning single pod value from IPC-called function
> - add licenses
> - improve auto-generated message
> - other fixes related to serdes
> ---
>  .../core_ipa_interface.h.tmpl                 |  37 ++
>  .../core_ipa_serializer.h.tmpl                |  47 ++
>  .../definition_functions.tmpl                 |  53 ++
>  .../libcamera_templates/meson.build           |  14 +
>  .../module_ipa_interface.h.tmpl               |  87 +++
>  .../module_ipa_proxy.cpp.tmpl                 | 232 ++++++++
>  .../module_ipa_proxy.h.tmpl                   | 126 +++++
>  .../module_ipa_proxy_worker.cpp.tmpl          | 224 ++++++++
>  .../module_ipa_serializer.h.tmpl              |  47 ++
>  .../libcamera_templates/proxy_functions.tmpl  | 192 +++++++
>  .../libcamera_templates/serializer.tmpl       | 313 +++++++++++
>  .../generators/mojom_libcamera_generator.py   | 511 ++++++++++++++++++
>  12 files changed, 1883 insertions(+)
>  create mode 100644 utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/core_ipa_serializer.h.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/definition_functions.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/meson.build
>  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
>  create mode 100644 utils/ipc/generators/libcamera_templates/serializer.tmpl
>  create mode 100644 utils/ipc/generators/mojom_libcamera_generator.py
> 
> diff --git a/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
> new file mode 100644
> index 00000000..f11d56fb
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/core_ipa_interface.h.tmpl
> @@ -0,0 +1,37 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{%- import "definition_functions.tmpl" as funcs -%}
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * core_ipa_interface.h - libcamera core definitions for Image Processing Algorithms
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +#ifndef __LIBCAMERA_IPA_INTERFACE_CORE_GENERATED_H__
> +#define __LIBCAMERA_IPA_INTERFACE_CORE_GENERATED_H__
> +
> +{% if has_map %}#include <map>{% endif %}
> +{% if has_array %}#include <vector>{% endif %}
> +
> +namespace libcamera {
> +
> +{% for const in consts %}
> +const {{const.kind|name}} {{const.mojom_name}} = {{const.value}};

s/const/static const/ ?

Same for module_ipa_interface.h.tmpl.

It would be best to use constexpr instead of const, but that won't work
with strings as the std::string constructor only becomes constexpr in
C++20 :-S Switching strings to char * won't be easy. Could you add a
\todo comment about this ?

> +{% endfor %}
> +
> +{% for enum in enums %}
> +{{funcs.define_enum(enum)}}
> +{% endfor %}
> +
> +{%- for struct in structs_gen_header %}
> +{{funcs.define_struct(struct)}}
> +{% endfor %}
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPA_INTERFACE_CORE_GENERATED_H__ */
> diff --git a/utils/ipc/generators/libcamera_templates/core_ipa_serializer.h.tmpl b/utils/ipc/generators/libcamera_templates/core_ipa_serializer.h.tmpl
> new file mode 100644
> index 00000000..37a784f1
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/core_ipa_serializer.h.tmpl
> @@ -0,0 +1,47 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{%- import "serializer.tmpl" as serializer -%}
> +
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * core_ipa_serializer.h - Data serializer for core libcamera definitions for IPA
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +#ifndef __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_CORE_H__
> +#define __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_CORE_H__
> +
> +#include <tuple>
> +#include <vector>
> +
> +#include <libcamera/ipa/core_ipa_interface.h>
> +
> +#include "libcamera/internal/control_serializer.h"
> +#include "libcamera/internal/ipa_data_serializer.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(IPADataSerializer)
> +{% for struct in structs_gen_serializer %}
> +template<>
> +class IPADataSerializer<{{struct|name}}>
> +{
> +public:
> +{{- serializer.serializer(struct, "")}}
> +{%- if struct|has_fd %}
> +{{serializer.deserializer_fd(struct, "")}}
> +{%- else %}
> +{{serializer.deserializer_no_fd(struct, "")}}
> +{{serializer.deserializer_fd_simple(struct, "")}}
> +{%- endif %}
> +};
> +{% endfor %}
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_CORE_H__ */
> diff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
> new file mode 100644
> index 00000000..cdd75f89
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
> @@ -0,0 +1,53 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +
> +{#
> + # \brief Generate enum definition
> + #
> + # \param enum Enum object whose definition is to be generated
> + #}
> +{%- macro define_enum(enum) -%}
> +enum {{enum.mojom_name}} {
> +{%- for field in enum.fields %}
> +	{{field.mojom_name}} = {{field.numeric_value}},
> +{%- endfor %}
> +};
> +{%- endmacro -%}
> +
> +{#
> + # \brief Generate struct definition
> + #
> + # \param struct Struct object whose definition is to be generated
> + #}
> +{%- macro define_struct(struct) -%}
> +struct {{struct.mojom_name}}
> +{
> +public:
> +	{{struct.mojom_name}}() {%- if struct|has_default_fields %}
> +		:{% endif %}
> +{%- for field in struct.fields|with_default_values -%}
> +{{" " if loop.first}}{{field.mojom_name}}({{field|default_value}}){{", " if not loop.last}}
> +{%- endfor %}
> +	{
> +	}
> +
> +	{{struct.mojom_name}}(
> +{%- for field in struct.fields -%}
> +{{"const " if not field|is_pod}}{{field|name}} {{"&" if not field|is_pod}}_{{field.mojom_name}}{{", " if not loop.last}}
> +{%- endfor -%}
> +)
> +		:
> +{%- for field in struct.fields -%}
> +{{" " if loop.first}}{{field.mojom_name}}(_{{field.mojom_name}}){{", " if not loop.last}}
> +{%- endfor %}
> +	{
> +	}
> +{% for field in struct.fields %}
> +	{{field|name}} {{field.mojom_name}};
> +{%- endfor %}
> +};
> +{%- endmacro -%}
> +
> +
> diff --git a/utils/ipc/generators/libcamera_templates/meson.build b/utils/ipc/generators/libcamera_templates/meson.build
> new file mode 100644
> index 00000000..70664eab
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/meson.build
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +mojom_template_files = files([
> +    'core_ipa_interface.h.tmpl',
> +    'core_ipa_serializer.h.tmpl',
> +    'definition_functions.tmpl',
> +    'module_ipa_interface.h.tmpl',
> +    'module_ipa_proxy.cpp.tmpl',
> +    'module_ipa_proxy.h.tmpl',
> +    'module_ipa_proxy_worker.cpp.tmpl',
> +    'module_ipa_serializer.h.tmpl',
> +    'proxy_functions.tmpl',
> +    'serializer.tmpl',
> +])
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
> new file mode 100644
> index 00000000..afbcb1b1
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
> @@ -0,0 +1,87 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{%- import "definition_functions.tmpl" as funcs -%}
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * {{module_name}}_ipa_interface.h - Image Processing Algorithm interface for {{module_name}}
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +#ifndef __LIBCAMERA_IPA_INTERFACE_{{module_name|upper}}_GENERATED_H__
> +#define __LIBCAMERA_IPA_INTERFACE_{{module_name|upper}}_GENERATED_H__
> +
> +#include <libcamera/ipa/ipa_interface.h>
> +#include <libcamera/ipa/core_ipa_interface.h>

Wrong alphabetical order.

> +
> +{% if has_map %}#include <map>{% endif %}
> +{% if has_array %}#include <vector>{% endif %}
> +
> +namespace libcamera {
> +{%- if has_namespace %}
> +{% for ns in namespace %}
> +namespace {{ns}} {
> +{% endfor %}
> +{%- endif %}
> +
> +{% for const in consts %}
> +const {{const.kind|name}} {{const.mojom_name}} = {{const.value}};
> +{% endfor %}
> +
> +enum class {{cmd_enum_name}} {
> +	Exit = 0,
> +{%- for method in interface_main.methods %}
> +	{{method.mojom_name|cap}} = {{loop.index}},
> +{%- endfor %}
> +};
> +
> +enum class {{cmd_event_enum_name}} {
> +{%- for method in interface_event.methods %}
> +	{{method.mojom_name|cap}} = {{loop.index}},
> +{%- endfor %}
> +};
> +
> +{% for enum in enums %}
> +{{funcs.define_enum(enum)}}
> +{% endfor %}
> +
> +{%- for struct in structs_nonempty %}
> +{{funcs.define_struct(struct)}}
> +{% endfor %}
> +
> +{#-
> +Any consts or #defines should be moved to the mojom file.
> +#}
> +class {{interface_name}} : public IPAInterface
> +{
> +public:
> +{% for method in interface_main.methods %}
> +	virtual {{method|method_return_value}} {{method.mojom_name}}(
> +{%- for param in method|method_parameters %}
> +		{{param}}{{- "," if not loop.last}}
> +{%- endfor -%}
> +) = 0;
> +{% endfor %}
> +
> +{%- for method in interface_event.methods %}
> +	Signal<
> +{%- for param in method.parameters -%}
> +		{{"const " if not param|is_pod}}{{param|name}}{{" &" if not param|is_pod}}
> +		{{- ", " if not loop.last}}
> +{%- endfor -%}
> +> {{method.mojom_name}};
> +{% endfor -%}
> +};
> +
> +{%- if has_namespace %}
> +{% for ns in namespace|reverse %}
> +} /* namespace {{ns}} */
> +{% endfor %}
> +{%- endif %}
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPA_INTERFACE_{{module_name|upper}}_GENERATED_H__ */
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> new file mode 100644
> index 00000000..a181cc84
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> @@ -0,0 +1,232 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{%- import "proxy_functions.tmpl" as proxy_funcs -%}
> +
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * {{module_name}}_ipa_proxy.cpp - Image Processing Algorithm proxy for {{module_name}}
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +#include <libcamera/ipa/{{module_name}}_ipa_proxy.h>
> +

#include <memory>
#include <string>

(for std::unique_ptr and std::string)

> +#include <vector>
> +
> +#include <libcamera/ipa/ipa_module_info.h>
> +#include <libcamera/ipa/{{module_name}}_ipa_interface.h>
> +#include <libcamera/ipa/{{module_name}}_ipa_serializer.h>
> +
> +#include "libcamera/internal/control_serializer.h"
> +#include "libcamera/internal/ipa_data_serializer.h"
> +#include "libcamera/internal/ipa_module.h"
> +#include "libcamera/internal/ipa_proxy.h"
> +#include "libcamera/internal/ipc_pipe.h"
> +#include "libcamera/internal/ipc_pipe_unixsocket.h"
> +#include "libcamera/internal/ipc_unixsocket.h"
> +#include "libcamera/internal/log.h"
> +#include "libcamera/internal/process.h"
> +#include "libcamera/internal/thread.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(IPAProxy)
> +
> +{%- if has_namespace %}
> +{% for ns in namespace %}
> +namespace {{ns}} {
> +{% endfor %}
> +{%- endif %}
> +
> +{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
> +	: IPAProxy(ipam), running_(false),
> +	  isolate_(isolate)
> +{
> +	LOG(IPAProxy, Debug)
> +		<< "initializing {{module_name}} proxy: loading IPA from "
> +		<< ipam->path();
> +
> +	if (isolate_) {
> +		const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> +		if (proxyWorkerPath.empty()) {
> +			LOG(IPAProxy, Error)
> +				<< "Failed to get proxy worker path";
> +			return;
> +		}
> +
> +		ipc_ = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(),
> +							   proxyWorkerPath.c_str());
> +		if (!ipc_->isConnected()) {
> +			LOG(IPAProxy, Error) << "Failed to create IPCPipe";
> +			return;
> +		}
> +
> +		ipc_->recv.connect(this, &{{proxy_name}}::recvMessage);
> +
> +		valid_ = true;
> +		return;
> +	}
> +
> +	if (!ipam->load())
> +		return;
> +
> +	IPAInterface *ipai = ipam->createInterface();
> +	if (!ipai) {
> +		LOG(IPAProxy, Error)
> +			<< "Failed to create IPA context for " << ipam->path();
> +		return;
> +	}
> +
> +	ipa_ = std::unique_ptr<{{interface_name}}>(static_cast<{{interface_name}} *>(ipai));
> +	proxy_.setIPA(ipa_.get());
> +
> +{% for method in interface_event.methods %}
> +	ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
> +{%- endfor %}
> +
> +	valid_ = true;
> +}
> +
> +{{proxy_name}}::~{{proxy_name}}()
> +{
> +	if (isolate_)
> +		ipc_->sendAsync(static_cast<uint32_t>({{cmd_enum_name}}::Exit), {});
> +}
> +
> +{% if interface_event.methods|length > 0 %}
> +void {{proxy_name}}::recvMessage(uint32_t cmd, const IPCMessage &data)
> +{
> +	size_t dataSize = data.cdata().size();
> +	{{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(cmd);
> +
> +	switch (_cmd) {
> +{%- for method in interface_event.methods %}
> +	case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> +		{{method.mojom_name}}IPC(data.cdata().cbegin(), dataSize, data.cfds());
> +		break;
> +	}
> +{%- endfor %}
> +	default:
> +		LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> +	}
> +}
> +{%- endif %}
> +
> +{% for method in interface_main.methods %}
> +{{proxy_funcs.func_sig(proxy_name, method)}}
> +{
> +	if (isolate_)
> +		{{"return " if method|method_return_value != "void"}}{{method.mojom_name}}IPC(
> +{%- for param in method|method_param_names -%}
> +		{{param}}{{- ", " if not loop.last}}
> +{%- endfor -%}
> +);
> +	else
> +		{{"return " if method|method_return_value != "void"}}{{method.mojom_name}}Thread(
> +{%- for param in method|method_param_names -%}
> +		{{param}}{{- ", " if not loop.last}}
> +{%- endfor -%}
> +);
> +}
> +
> +{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
> +{
> +{%- if method.mojom_name == "init" %}
> +	{{proxy_funcs.init_thread_body()}}
> +{%- elif method.mojom_name == "stop" %}
> +	{{proxy_funcs.stop_thread_body()}}
> +{%- elif method.mojom_name == "start" %}
> +	running_ = true;
> +	thread_.start();
> +
> +	{{ "return " if method|method_return_value != "void" -}}
> +	proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking
> +	{{- ", " if method|method_param_names}}
> +	{%- for param in method|method_param_names -%}
> +		{{param}}{{- ", " if not loop.last}}
> +	{%- endfor -%}
> +);
> +{%- elif not method|is_async %}
> +	{{ "return " if method|method_return_value != "void" -}}
> +	ipa_->{{method.mojom_name}}(
> +	{%- for param in method|method_param_names -%}
> +		{{param}}{{- ", " if not loop.last}}
> +	{%- endfor -%}
> +);
> +{% elif method|is_async %}
> +	proxy_.invokeMethod(&ThreadProxy::{{method.mojom_name}}, ConnectionTypeQueued,
> +	{%- for param in method|method_param_names -%}
> +		{{param}}{{- ", " if not loop.last}}
> +	{%- endfor -%}
> +);
> +{%- endif %}
> +}
> +
> +{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
> +{
> +{%- set has_input = true if method|method_param_inputs|length > 0 %}
> +{%- set has_output = true if method|method_param_outputs|length > 0 or method|method_return_value != "void" %}
> +{%- if has_input %}
> +	IPCMessage _ipcInputBuf;
> +{%- endif %}
> +{%- if has_output %}
> +	IPCMessage _ipcOutputBuf;
> +{%- endif %}
> +
> +{{proxy_funcs.serialize_call(method|method_param_inputs, '_ipcInputBuf.data()', '_ipcInputBuf.fds()')}}
> +
> +{%- set input_buf = "_ipcInputBuf" if has_input else "{}" %}
> +{%- set cmd = cmd_enum_name + "::" + method.mojom_name|cap %}
> +{% if method|is_async %}
> +	int _ret = ipc_->sendAsync(static_cast<uint32_t>({{cmd}}), {{input_buf}});
> +{%- else %}
> +	int _ret = ipc_->sendSync(static_cast<uint32_t>({{cmd}}), {{input_buf}}
> +{{- ", &_ipcOutputBuf" if has_output -}}
> +);
> +{%- endif %}
> +	if (_ret < 0) {
> +		LOG(IPAProxy, Error) << "Failed to call {{method.mojom_name}}";
> +{%- if method|method_return_value != "void" %}
> +		return static_cast<{{method|method_return_value}}>(_ret);
> +{%- else %}
> +		return;
> +{%- endif %}
> +	}
> +{% if method|method_return_value != "void" %}
> +	return IPADataSerializer<{{method.response_parameters|first|name}}>::deserialize(_ipcOutputBuf.data(), 0);
> +{% elif method|method_param_outputs|length > 0 %}
> +{{proxy_funcs.deserialize_call(method|method_param_outputs, '_ipcOutputBuf.data()', '_ipcOutputBuf.fds()')}}
> +{% endif -%}
> +}
> +
> +{% endfor %}
> +
> +{% for method in interface_event.methods %}
> +{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
> +{
> +	{{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> +}
> +
> +void {{proxy_name}}::{{method.mojom_name}}IPC(
> +	std::vector<uint8_t>::const_iterator data,
> +	size_t dataSize,
> +	[[maybe_unused]] const std::vector<int32_t> &fds)
> +{
> +{%- for param in method.parameters %}
> +	{{param|name}} {{param.mojom_name}};
> +{%- endfor %}
> +{{proxy_funcs.deserialize_call(method.parameters, 'data', 'fds', false, false, true, 'dataSize')}}
> +	{{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> +}
> +{% endfor %}
> +
> +{%- if has_namespace %}
> +{% for ns in namespace|reverse %}
> +} /* namespace {{ns}} */
> +{% endfor %}
> +{%- endif %}
> +} /* namespace libcamera */
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> new file mode 100644
> index 00000000..b0f04bf6
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> @@ -0,0 +1,126 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{%- import "proxy_functions.tmpl" as proxy_funcs -%}
> +
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * {{module_name}}_ipa_proxy.h - Image Processing Algorithm proxy for {{module_name}}
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +#ifndef __LIBCAMERA_INTERNAL_IPA_PROXY_{{module_name|upper}}_H__
> +#define __LIBCAMERA_INTERNAL_IPA_PROXY_{{module_name|upper}}_H__
> +
> +#include <libcamera/ipa/ipa_interface.h>
> +#include <libcamera/ipa/{{module_name}}_ipa_interface.h>
> +
> +#include "libcamera/internal/control_serializer.h"
> +#include "libcamera/internal/ipc_pipe.h"
> +#include "libcamera/internal/ipc_pipe_unixsocket.h"
> +#include "libcamera/internal/ipa_proxy.h"

This one goes before ipc_pipe.h.

> +#include "libcamera/internal/ipc_unixsocket.h"
> +#include "libcamera/internal/thread.h"
> +
> +namespace libcamera {
> +{%- if has_namespace %}
> +{% for ns in namespace %}
> +namespace {{ns}} {
> +{% endfor %}
> +{%- endif %}
> +
> +class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
> +{
> +public:
> +	{{proxy_name}}(IPAModule *ipam, bool isolate);
> +	~{{proxy_name}}();
> +
> +{% for method in interface_main.methods %}
> +{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
> +{% endfor %}
> +
> +{%- for method in interface_event.methods %}
> +	Signal<
> +{%- for param in method.parameters -%}
> +		{{"const " if not param|is_pod}}{{param|name}}{{" &" if not param|is_pod}}
> +		{{- ", " if not loop.last}}
> +{%- endfor -%}
> +> {{method.mojom_name}};
> +{% endfor %}
> +
> +private:
> +	void recvMessage(uint32_t cmd, const IPCMessage &data);
> +
> +{% for method in interface_main.methods %}
> +{{proxy_funcs.func_sig(proxy_name, method, "Thread", false)|indent(8, true)}};
> +{{proxy_funcs.func_sig(proxy_name, method, "IPC", false)|indent(8, true)}};
> +{% endfor %}
> +{% for method in interface_event.methods %}
> +{{proxy_funcs.func_sig(proxy_name, method, "Thread", false)|indent(8, true)}};
> +	void {{method.mojom_name}}IPC(
> +		std::vector<uint8_t>::const_iterator data,
> +		size_t dataSize,
> +		const std::vector<int32_t> &fds);

Really nice work on the constification of the API, and the usage of
IPCMessage !

> +{% endfor %}
> +
> +	/* Helper class to invoke async functions in another thread. */
> +	class ThreadProxy : public Object
> +	{
> +	public:
> +		void setIPA({{interface_name}} *ipa)
> +		{
> +			ipa_ = ipa;
> +		}
> +
> +		void stop()
> +		{
> +			ipa_->stop();
> +		}
> +{% for method in interface_main.methods %}
> +{%- if method|is_async %}
> +		{{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
> +		{
> +			ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
> +		}
> +{%- elif method.mojom_name == "start" %}
> +		{{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
> +		{
> +{%- if method|method_return_value != "void" %}
> +			return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
> +{%- else %}
> +			ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}}
> +	{{- ", " if method|method_param_outputs|params_comma_sep -}}
> +	{{- method|method_param_outputs|params_comma_sep}});
> +{%- endif %}
> +		}
> +{%- endif %}
> +{%- endfor %}
> +
> +	private:
> +		{{interface_name}} *ipa_;
> +	};
> +
> +	bool running_;
> +	Thread thread_;
> +	ThreadProxy proxy_;
> +	std::unique_ptr<{{interface_name}}> ipa_;
> +
> +	const bool isolate_;
> +
> +	std::unique_ptr<IPCPipeUnixSocket> ipc_;
> +
> +	ControlSerializer controlSerializer_;
> +};
> +
> +{%- if has_namespace %}
> +{% for ns in namespace|reverse %}
> +} /* namespace {{ns}} */
> +{% endfor %}
> +{%- endif %}
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_IPA_PROXY_{{module_name|upper}}_H__ */
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> new file mode 100644
> index 00000000..cb3df4eb
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> @@ -0,0 +1,224 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{%- import "proxy_functions.tmpl" as proxy_funcs -%}
> +
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * {{module_name}}_ipa_proxy_worker.cpp - Image Processing Algorithm proxy worker for {{module_name}}
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +{#- \todo Split proxy worker into IPC worker and proxy worker. #}
> +
> +#include <algorithm>
> +#include <iostream>
> +#include <sys/types.h>
> +#include <tuple>
> +#include <unistd.h>
> +
> +#include <libcamera/ipa/ipa_interface.h>
> +#include <libcamera/ipa/{{module_name}}_ipa_interface.h>
> +#include <libcamera/ipa/{{module_name}}_ipa_serializer.h>
> +#include <libcamera/logging.h>
> +
> +#include "libcamera/internal/camera_sensor.h"
> +#include "libcamera/internal/control_serializer.h"
> +#include "libcamera/internal/event_dispatcher.h"
> +#include "libcamera/internal/ipa_data_serializer.h"
> +#include "libcamera/internal/ipc_pipe.h"
> +#include "libcamera/internal/ipc_pipe_unixsocket.h"
> +#include "libcamera/internal/ipa_module.h"
> +#include "libcamera/internal/ipa_proxy.h"

These two go before ipc_pipe.h.

> +#include "libcamera/internal/ipc_unixsocket.h"
> +#include "libcamera/internal/log.h"
> +#include "libcamera/internal/thread.h"
> +
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY({{proxy_worker_name}})
> +
> +{%- if has_namespace %}
> +{% for ns in namespace -%}
> +using namespace {{ns}};
> +{% endfor %}
> +{%- endif %}
> +
> +class {{proxy_worker_name}}
> +{
> +public:
> +	{{proxy_worker_name}}()
> +		: ipa_(nullptr), exit_(false) {}
> +
> +	~{{proxy_worker_name}}() {}
> +
> +	void readyRead(IPCUnixSocket *socket)
> +	{
> +		IPCUnixSocket::Payload _message;
> +		int _retRecv = socket->receive(&_message);
> +		if (_retRecv) {
> +			LOG({{proxy_worker_name}}, Error)
> +				<< "Receive message failed" << _retRecv;

s/failed/failed: /

> +			return;
> +		}
> +
> +		IPCMessage _ipcMessage(_message);
> +
> +		{{cmd_enum_name}} _cmd = static_cast<{{cmd_enum_name}}>(_ipcMessage.header().cmd);
> +
> +		switch (_cmd) {
> +		case {{cmd_enum_name}}::Exit: {
> +			exit_ = true;
> +			break;
> +		}
> +
> +{% for method in interface_main.methods %}
> +		case {{cmd_enum_name}}::{{method.mojom_name|cap}}: {
> +		{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}}
> +{% for param in method|method_param_outputs %}
> +			{{param|name}} {{param.mojom_name}};
> +{% endfor %}
> +{%- if method|method_return_value != "void" %}
> +			{{method|method_return_value}} _callRet = ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
> +{%- else %}
> +			ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}}
> +{{- ", " if method|method_param_outputs|params_comma_sep -}}
> +{%- for param in method|method_param_outputs -%}
> +&{{param.mojom_name}}{{", " if not loop.last}}
> +{%- endfor -%}
> +);
> +{%- endif %}
> +{% if not method|is_async %}
> +			IPCMessage::Header header = { _ipcMessage.header().cmd, _ipcMessage.header().cookie };
> +			IPCMessage _response(header);
> +{%- if method|method_return_value != "void" %}
> +			std::vector<uint8_t> _callRetBuf;
> +			std::tie(_callRetBuf, std::ignore) =
> +				IPADataSerializer<{{method|method_return_value}}>::serialize(_callRet);
> +			_response.data().insert(_response.data().end(), _callRetBuf.cbegin(), _callRetBuf.cend());
> +{%- else %}
> +		{{proxy_funcs.serialize_call(method|method_param_outputs, "_response.data()", "_response.fds()")|indent(16, true)}}
> +{%- endif %}
> +			int _ret = socket_.send(_response.payload());
> +			if (_ret < 0) {
> +				LOG({{proxy_worker_name}}, Error)
> +					<< "Reply to {{method.mojom_name}}() failed" << _ret;

s/failed/failed: /

> +			}
> +			LOG({{proxy_worker_name}}, Debug) << "Done replying to {{method.mojom_name}}()";
> +{%- endif %}
> +			break;
> +		}
> +{% endfor %}
> +		default:
> +			LOG({{proxy_worker_name}}, Error) << "Unknown command " << _ipcMessage.header().cmd;
> +		}
> +	}
> +
> +	int init(std::unique_ptr<IPAModule> &ipam, int socketfd)
> +	{
> +		if (socket_.bind(socketfd) < 0) {
> +			LOG({{proxy_worker_name}}, Error)
> +				<< "IPC socket binding failed";
> +			return EXIT_FAILURE;
> +		}
> +		socket_.readyRead.connect(this, &{{proxy_worker_name}}::readyRead);
> +
> +		ipa_ = dynamic_cast<{{interface_name}} *>(ipam->createInterface());
> +		if (!ipa_) {
> +			LOG({{proxy_worker_name}}, Error)
> +				<< "Failed to create IPA interface instance";
> +			return EXIT_FAILURE;
> +		}
> +{% for method in interface_event.methods %}
> +		ipa_->{{method.mojom_name}}.connect(this, &{{proxy_worker_name}}::{{method.mojom_name}});
> +{%- endfor %}
> +		return 0;
> +	}
> +
> +	void run()
> +	{
> +		EventDispatcher *dispatcher = Thread::current()->eventDispatcher();
> +		while (!exit_)
> +			dispatcher->processEvents();
> +	}
> +
> +	void cleanup()
> +	{
> +		delete ipa_;
> +		socket_.close();
> +	}
> +
> +private:
> +
> +{% for method in interface_event.methods %}
> +{{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(8, true)}}
> +	{
> +		IPCMessage::Header header = {
> +			static_cast<uint32_t>({{cmd_event_enum_name}}::{{method.mojom_name|cap}}),
> +			0
> +		};
> +		IPCMessage _message(header);
> +
> +		{{proxy_funcs.serialize_call(method|method_param_inputs, "_message.data()", "_message.fds()")}}
> +
> +		socket_.send(_message.payload());
> +
> +		LOG({{proxy_worker_name}}, Debug) << "{{method.mojom_name}} done";
> +	}
> +{% endfor %}
> +
> +	{{interface_name}} *ipa_;
> +	IPCUnixSocket socket_;
> +
> +	ControlSerializer controlSerializer_;
> +
> +	bool exit_;
> +};
> +
> +int main(int argc, char **argv)
> +{
> +{#- \todo Handle enabling debugging more dynamically. #}
> +	/* Uncomment this for debugging. */

It's not commented :-)

> +	std::string logPath = "/tmp/libcamera.worker." +
> +			      std::to_string(getpid()) + ".log";
> +	logSetFile(logPath.c_str());
> +
> +	if (argc < 3) {
> +		LOG({{proxy_worker_name}}, Error)
> +			<< "Tried to start worker with no args: "
> +			<< "expected <path to IPA so> <fd to bind unix socket>";
> +		return EXIT_FAILURE;
> +	}
> +
> +	int fd = std::stoi(argv[2]);
> +	LOG({{proxy_worker_name}}, Info)
> +		<< "Starting worker for IPA module " << argv[1]
> +		<< " with IPC fd = " << fd;
> +
> +	std::unique_ptr<IPAModule> ipam = std::make_unique<IPAModule>(argv[1]);
> +	if (!ipam->isValid() || !ipam->load()) {
> +		LOG({{proxy_worker_name}}, Error)
> +			<< "IPAModule " << argv[1] << " isn't valid";
> +		return EXIT_FAILURE;
> +	}
> +
> +	{{proxy_worker_name}} proxyWorker;
> +	int ret = proxyWorker.init(ipam, fd);
> +	if (ret < 0) {
> +		LOG({{proxy_worker_name}}, Error)
> +			<< "Failed to initialize proxy worker";
> +		return ret;
> +	}
> +
> +	LOG({{proxy_worker_name}}, Debug) << "Proxy worker successfully started";

s/started/initialized/ ?

> +
> +	proxyWorker.run();
> +
> +	proxyWorker.cleanup();
> +
> +	return 0;
> +}
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
> new file mode 100644
> index 00000000..ad522964
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
> @@ -0,0 +1,47 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{%- import "serializer.tmpl" as serializer -%}
> +
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Google Inc.
> + *
> + * {{module_name}}_ipa_serializer.h - Image Processing Algorithm data serializer for {{module_name}}
> + *
> + * This file is auto-generated. Do not edit.
> + */
> +
> +#ifndef __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_{{module_name|upper}}_H__
> +#define __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_{{module_name|upper}}_H__
> +
> +#include <tuple>
> +#include <vector>
> +
> +#include <libcamera/ipa/{{module_name}}_ipa_interface.h>
> +#include <libcamera/ipa/core_ipa_serializer.h>
> +
> +#include "libcamera/internal/control_serializer.h"
> +#include "libcamera/internal/ipa_data_serializer.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(IPADataSerializer)
> +{% for struct in structs_nonempty %}
> +template<>
> +class IPADataSerializer<{{struct|name_full(namespace_str)}}>
> +{
> +public:
> +{{- serializer.serializer(struct, namespace_str)}}
> +{%- if struct|has_fd %}
> +{{serializer.deserializer_fd(struct, namespace_str)}}
> +{%- else %}
> +{{serializer.deserializer_no_fd(struct, namespace_str)}}
> +{%- endif %}
> +};
> +{% endfor %}
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_INTERNAL_IPA_DATA_SERIALIZER_{{module_name|upper}}_H__ */
> diff --git a/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> new file mode 100644
> index 00000000..3f2143b1
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> @@ -0,0 +1,192 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{#
> + # \brief Generate fuction prototype

s/fuction/function/

> + #
> + # \param class Class name
> + # \param method mojom Method object
> + # \param suffix Suffix to append to \a method function name
> + # \param need_class_name If true, generate class name with function
> + # \param override If true, generate override tag after the function prototype
> + #}
> +{%- macro func_sig(class, method, suffix = "", need_class_name = true, override = false) -%}
> +{{method|method_return_value}} {{class + "::" if need_class_name}}{{method.mojom_name}}{{suffix}}(
> +{%- for param in method|method_parameters %}
> +	{{param}}{{- "," if not loop.last}}
> +{%- endfor -%}
> +){{" override" if override}}
> +{%- endmacro -%}
> +
> +{#
> + # \brief Generate function body for IPA init() function for thread
> + #}
> +{%- macro init_thread_body() -%}
> +	int ret = ipa_->init(settings);
> +	if (ret)
> +		return ret;
> +
> +	proxy_.moveToThread(&thread_);
> +
> +	return 0;
> +{%- endmacro -%}
> +
> +{#
> + # \brief Generate function body for IPA stop() function for thread
> + #}
> +{%- macro stop_thread_body() -%}
> +	if (!running_)
> +		return;
> +
> +	running_ = false;
> +
> +	proxy_.invokeMethod(&ThreadProxy::stop, ConnectionTypeBlocking);
> +
> +	thread_.exit();
> +	thread_.wait();
> +{%- endmacro -%}
> +
> +
> +{#
> + # \brief Serialize multiple objects into data buffer and fd vector
> + #
> + # Generate code to serialize multiple objects, as specified in \a params
> + # (which are the parameters to some function), into \a buf data buffer and
> + # \a fds fd vector.
> + # This code is meant to be used by the proxy, for serializing prior to IPC calls.
> + #}
> +{%- macro serialize_call(params, buf, fds) %}
> +{%- for param in params %}
> +	std::vector<uint8_t> {{param.mojom_name}}Buf;

There's room for optimization here too,

 # \todo Avoid intermediate vectors

I'm not sure how such optimization would work, but given that we know
what we're serializing, I'm confident we could do something. One useful
task would be to see if using the same binary format as mojo could help
(I assume it has been designed with performance in mind).

> +{%- if param|has_fd %}
> +	std::vector<int32_t> {{param.mojom_name}}Fds;
> +	std::tie({{param.mojom_name}}Buf, {{param.mojom_name}}Fds) =
> +{%- else %}
> +	std::tie({{param.mojom_name}}Buf, std::ignore) =
> +{%- endif %}
> +		IPADataSerializer<{{param|name}}>::serialize({{param.mojom_name}}
> +{{- ", &controlSerializer_" if param|needs_control_serializer -}}
> +);
> +{%- endfor %}
> +
> +{%- if params|length > 1 %}
> +{%- for param in params %}
> +	appendPOD<uint32_t>({{buf}}, {{param.mojom_name}}Buf.size());
> +{%- if param|has_fd %}
> +	appendPOD<uint32_t>({{buf}}, {{param.mojom_name}}Fds.size());
> +{%- endif %}
> +{%- endfor %}
> +{%- endif %}
> +
> +{%- for param in params %}
> +	{{buf}}.insert({{buf}}.end(), {{param.mojom_name}}Buf.begin(), {{param.mojom_name}}Buf.end());
> +{%- endfor %}
> +
> +{%- for param in params %}
> +{%- if param|has_fd %}
> +	{{fds}}.insert({{fds}}.end(), {{param.mojom_name}}Fds.begin(), {{param.mojom_name}}Fds.end());
> +{%- endif %}
> +{%- endfor %}
> +{%- endmacro -%}
> +
> +
> +{#
> + # \brief Deserialize a single object from data buffer and fd vector
> + #
> + # \param pointer If true, deserialize the object into a dereferenced pointer
> + # \param iter If true, treat \a buf as an iterator instead of a vector
> + # \param data_size Variable that holds the size of the vector referenced by \a buf
> + #
> + # Generate code to deserialize a single object, as specified in \a param,
> + # from \a buf data buffer and \a fds fd vector.
> + # This code is meant to be used by macro deserialize_call.
> + #}
> +{%- macro deserialize_param(param, pointer, loop, buf, fds, iter, data_size) -%}
> +{{"*" if pointer}}{{param.mojom_name}} = IPADataSerializer<{{param|name}}>::deserialize(
> +	{{buf}}{{- ".cbegin()" if not iter}} + {{param.mojom_name}}Start,
> +{%- if loop.last and not iter %}
> +	{{buf}}.cend()
> +{%- elif not iter %}
> +	{{buf}}.cbegin() + {{param.mojom_name}}Start + {{param.mojom_name}}BufSize
> +{%- elif iter and loop.length == 1 %}
> +	{{buf}} + {{data_size}}
> +{%- else %}
> +	{{buf}} + {{param.mojom_name}}Start + {{param.mojom_name}}BufSize
> +{%- endif -%}
> +{{- "," if param|has_fd}}
> +{%- if param|has_fd %}
> +	{{fds}}.cbegin() + {{param.mojom_name}}FdStart,
> +{%- if loop.last %}
> +	{{fds}}.cend()
> +{%- else %}
> +	{{fds}}.cbegin() + {{param.mojom_name}}FdStart + {{param.mojom_name}}FdsSize
> +{%- endif -%}
> +{%- endif -%}
> +{{- "," if param|needs_control_serializer}}
> +{%- if param|needs_control_serializer %}
> +	&controlSerializer_
> +{%- endif -%}
> +);
> +{%- endmacro -%}
> +
> +
> +{#
> + # \brief Deserialize multiple objects from data buffer and fd vector
> + #
> + # \param pointer If true, deserialize objects into pointers, and adds a null check.
> + # \param declare If true, declare the objects in addition to deserialization.
> + # \param iter if true, treat \a buf as an iterator instead of a vector
> + # \param data_size Variable that holds the size of the vector referenced by \a buf
> + #
> + # Generate code to deserialize multiple objects, as specified in \a params
> + # (which are the parameters to some function), from \a buf data buffer and
> + # \a fds fd vector.
> + # This code is meant to be used by the proxy, for deserializing after IPC calls.
> + #
> + # \todo Avoid intermediate vectors
> + #}
> +{%- macro deserialize_call(params, buf, fds, pointer = true, declare = false, iter = false, data_size = '') -%}
> +{% set ns = namespace(size_offset = 0) %}
> +{%- if params|length > 1 %}
> +{%- for param in params %}
> +	[[maybe_unused]]  const size_t {{param.mojom_name}}BufSize = readPOD<uint32_t>({{buf}}, {{ns.size_offset}}

Extra space before const.

> +{%- if iter -%}
> +, {{buf}} + {{data_size}}
> +{%- endif -%}
> +);
> +	{%- set ns.size_offset = ns.size_offset + 4 %}
> +{%- if param|has_fd %}
> +	[[maybe_unused]] const size_t {{param.mojom_name}}FdsSize = readPOD<uint32_t>({{buf}}, {{ns.size_offset}}
> +{%- if iter -%}
> +, {{buf}} + {{data_size}}
> +{%- endif -%}
> +);
> +	{%- set ns.size_offset = ns.size_offset + 4 %}
> +{%- endif %}
> +{%- endfor %}
> +{%- endif %}
> +{% for param in params %}
> +{%- if loop.first %}
> +	const size_t {{param.mojom_name}}Start = {{ns.size_offset}};
> +{%- else %}
> +	const size_t {{param.mojom_name}}Start = {{loop.previtem.mojom_name}}Start + {{loop.previtem.mojom_name}}BufSize;
> +{%- endif %}
> +{%- endfor %}
> +{% for param in params|with_fds %}
> +{%- if loop.first %}
> +	const size_t {{param.mojom_name}}FdStart = 0;
> +{%- elif not loop.last %}
> +	const size_t {{param.mojom_name}}FdStart = {{loop.previtem.mojom_name}}FdStart + {{loop.previtem.mojom_name}}FdsSize;
> +{%- endif %}
> +{%- endfor %}
> +{% for param in params %}
> +	{%- if pointer %}
> +	if ({{param.mojom_name}}) {
> +{{deserialize_param(param, pointer, loop, buf, fds, iter, data_size)|indent(16, True)}}
> +	}
> +	{%- else %}
> +	{{param|name + " " if declare}}{{deserialize_param(param, pointer, loop, buf, fds, iter, data_size)|indent(8)}}
> +	{%- endif %}
> +{% endfor %}
> +{%- endmacro -%}
> diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> new file mode 100644
> index 00000000..af4800bf
> --- /dev/null
> +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl
> @@ -0,0 +1,313 @@
> +{#-
> + # SPDX-License-Identifier: LGPL-2.1-or-later
> + # Copyright (C) 2020, Google Inc.
> +-#}
> +{# Turn this into a C macro? #}
> +{#
> + # \brief Verify that there is enough bytes to deserialize
> + #
> + # Generate code that verifies that \a size is not greater than \a dataSize.
> + # Otherwise log an error with \a name and \a typename.
> + #}
> +{%- macro check_data_size(size, dataSize, name, typename) %}
> +		if ({{dataSize}} < {{size}}) {
> +			LOG(IPADataSerializer, Error)
> +				<< "Failed to deserialize " << "{{name}}"
> +				<< ": not enough {{typename}}, expected "
> +				<< ({{size}}) << ", got " << ({{dataSize}});
> +			return ret;
> +		}
> +{%- endmacro %}
> +
> +
> +{#
> + # \brief Serialize some field into return vector
> + #
> + # Generate code to serialize \a field into retData, including size of the
> + # field and fds (where appropriate).
> + # This code is meant to be used by the IPADataSerializer specialization.
> + #
> + # \todo Avoid intermediate vectors
> + #}
> +{%- macro serializer_field(field, namespace, loop) %}
> +{%- 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_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}};
> +		std::vector<int32_t> {{field.mojom_name}}Fds;
> +		std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) =
> +			IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});
> +		retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
> +		retFds.insert(retFds.end(), {{field.mojom_name}}Fds.begin(), {{field.mojom_name}}Fds.end());
> +{%- elif field|is_controls %}
> +		if (data.{{field.mojom_name}}.size() > 0) {
> +			std::vector<uint8_t> {{field.mojom_name}};
> +			std::tie({{field.mojom_name}}, std::ignore) =
> +				IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}, cs);
> +			appendPOD<uint32_t>(retData, {{field.mojom_name}}.size());
> +			retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
> +		} else {
> +			appendPOD<uint32_t>(retData, 0);
> +		}
> +{%- elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}
> +		std::vector<uint8_t> {{field.mojom_name}};
> +	{%- if field|has_fd %}
> +		std::vector<int32_t> {{field.mojom_name}}Fds;
> +		std::tie({{field.mojom_name}}, {{field.mojom_name}}Fds) =
> +	{%- 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 %}
> +			IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}});
> +	{%- else %}
> +			IPADataSerializer<{{field|name_full(namespace)}}>::serialize(data.{{field.mojom_name}}, cs);
> +	{%- endif %}
> +		appendPOD<uint32_t>(retData, {{field.mojom_name}}.size());
> +	{%- if field|has_fd %}
> +		appendPOD<uint32_t>(retData, {{field.mojom_name}}Fds.size());
> +	{%- endif %}
> +		retData.insert(retData.end(), {{field.mojom_name}}.begin(), {{field.mojom_name}}.end());
> +	{%- if field|has_fd %}
> +		retFds.insert(retFds.end(), {{field.mojom_name}}Fds.begin(), {{field.mojom_name}}Fds.end());
> +	{%- endif %}
> +{%- else %}
> +		/* Unknown serialization for {{field.mojom_name}}. */
> +{%- endif %}
> +{%- endmacro %}
> +
> +
> +{#
> + # \brief Deserialize some field into return struct
> + #
> + # Generate code to deserialize \a field into object ret.
> + # This code is meant to be used by the IPADataSerializer specialization.
> + #}
> +{%- macro deserializer_field(field, namespace, loop) %}
> +{% 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}});
> +		{%- else %}
> +		ret.{{field.mojom_name}} = static_cast<{{field|name_full(namespace)}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}}));
> +		{%- endif %}
> +	{%- if not loop.last %}
> +		m += {{field_size}};
> +		dataSize -= {{field_size}};
> +	{%- endif %}
> +{% elif field|is_fd %}
> +	{%- set field_size = 1 %}
> +		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
> +		ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + 1, n, n + 1, cs);
> +	{%- if not loop.last %}
> +		m += {{field_size}};
> +		dataSize -= {{field_size}};
> +		n += ret.{{field.mojom_name}}.isValid() ? 1 : 0;
> +		fdsSize -= ret.{{field.mojom_name}}.isValid() ? 1 : 0;
> +	{%- endif %}
> +{% elif field|is_controls %}
> +	{%- set field_size = 4 %}
> +		{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'Size', 'data')}}
> +		const size_t {{field.mojom_name}}Size = readPOD<uint32_t>(m, 0, dataEnd);
> +		m += {{field_size}};
> +		dataSize -= {{field_size}};
> +	{%- set field_size = field.mojom_name + 'Size' -%}
> +		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
> +		if ({{field.mojom_name}}Size > 0)
> +			ret.{{field.mojom_name}} =
> +				IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, cs);
> +	{%- if not loop.last %}
> +		m += {{field_size}};
> +		dataSize -= {{field_size}};
> +	{%- endif %}
> +{% elif field|is_plain_struct or field|is_array or field|is_map or field|is_str %}
> +	{%- set field_size = 4 %}
> +		{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'Size', 'data')}}
> +		const size_t {{field.mojom_name}}Size = readPOD<uint32_t>(m, 0, dataEnd);
> +		m += {{field_size}};
> +		dataSize -= {{field_size}};
> +	{%- if field|has_fd %}
> +	{%- set field_size = 4 %}
> +		{{- check_data_size(field_size, 'dataSize', field.mojom_name + 'FdsSize', 'data')}}
> +		const size_t {{field.mojom_name}}FdsSize = readPOD<uint32_t>(m, 0, dataEnd);
> +		m += {{field_size}};
> +		dataSize -= {{field_size}};
> +		{{- check_data_size(field.mojom_name + 'FdsSize', 'fdsSize', field.mojom_name, 'fds')}}
> +	{%- endif %}
> +	{%- set field_size = field.mojom_name + 'Size' -%}
> +		{{- check_data_size(field_size, 'dataSize', field.mojom_name, 'data')}}
> +		ret.{{field.mojom_name}} =
> +	{%- if field|is_str %}
> +			IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size);
> +	{%- 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);
> +	{%- 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);
> +	{%- endif %}
> +	{%- if not loop.last %}
> +		m += {{field_size}};
> +		dataSize -= {{field_size}};
> +	{%- if field|has_fd %}
> +		n += {{field.mojom_name}}FdsSize;
> +		fdsSize -= {{field.mojom_name}}FdsSize;
> +	{%- endif %}
> +	{%- endif %}
> +{% else %}
> +		/* Unknown deserialization for {{field.mojom_name}}. */
> +{%- endif %}
> +{%- endmacro %}
> +
> +
> +{#
> + # \brief Serialize a struct
> + #
> + # Generate code for IPADataSerializer specialization, for serializing
> + # \a struct.
> + #}
> +{%- macro serializer(struct, namespace) %}
> +	static std::tuple<std::vector<uint8_t>, std::vector<int32_t>>
> +	serialize(const {{struct|name_full(namespace)}} &data,
> +{%- if struct|needs_control_serializer %}
> +		  ControlSerializer *cs)
> +{%- else %}
> +		  [[maybe_unused]] ControlSerializer *cs = nullptr)
> +{%- endif %}
> +	{
> +		std::vector<uint8_t> retData;
> +{%- if struct|has_fd %}
> +		std::vector<int32_t> retFds;
> +{%- endif %}
> +{%- for field in struct.fields %}
> +{{serializer_field(field, namespace, loop)}}
> +{%- endfor %}
> +{% if struct|has_fd %}
> +		return {retData, retFds};
> +{%- else %}
> +		return {retData, {}};
> +{%- endif %}
> +	}
> +{%- endmacro %}
> +
> +
> +{#
> + # \brief Deserialize a struct that has fds
> + #
> + # Generate code for IPADataSerializer specialization, for deserializing
> + # \a struct, in the case that \a struct has file descriptors.
> + #           fd parameters

Leftover ?

> + #}
> +{%- macro deserializer_fd(struct, namespace) %}
> +	static {{struct|name_full(namespace)}}
> +	deserialize(std::vector<uint8_t> &data,
> +		    std::vector<int32_t> &fds,
> +{%- if struct|needs_control_serializer %}
> +		    ControlSerializer *cs)
> +{%- else %}
> +		    [[maybe_unused]] ControlSerializer *cs = nullptr)
> +{%- endif %}
> +	{
> +		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs);
> +	}
> +

The next function can be quite large, inlining it isn't great. You don't
have to fix this now, but a "\todo Don't inline this function" would be
Agood. Same for the "no fds" version.

> +	static {{struct|name_full(namespace)}}
> +	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +		    std::vector<uint8_t>::const_iterator dataEnd,
> +		    std::vector<int32_t>::const_iterator fdsBegin,
> +		    std::vector<int32_t>::const_iterator fdsEnd,
> +{%- if struct|needs_control_serializer %}
> +		    ControlSerializer *cs)
> +{%- else %}
> +		    [[maybe_unused]] ControlSerializer *cs = nullptr)
> +{%- endif %}
> +	{
> +		{{struct|name_full(namespace)}} ret;
> +		std::vector<uint8_t>::const_iterator m = dataBegin;
> +		std::vector<int32_t>::const_iterator n = fdsBegin;
> +
> +		size_t dataSize = std::distance(dataBegin, dataEnd);
> +		[[maybe_unused]] size_t fdsSize = std::distance(fdsBegin, fdsEnd);
> +{%- for field in struct.fields -%}
> +{{deserializer_field(field, namespace, loop)}}
> +{%- endfor %}
> +		return ret;
> +	}
> +{%- endmacro %}
> +
> +{#
> + # \brief Deserialize a struct that has fds, using non-fd
> + #
> + # Generate code for IPADataSerializer specialization, for deserializing
> + # \a struct, in the case that \a struct has no file descriptors but requires
> + # deserializers with file descriptors.
> + #}
> +{%- macro deserializer_fd_simple(struct, namespace) %}
> +	static {{struct|name_full(namespace)}}
> +	deserialize(std::vector<uint8_t> &data,
> +		    [[maybe_unused]] std::vector<int32_t> &fds,
> +		    [[maybe_unused]] ControlSerializer *cs = nullptr)

Given that you're passing cs to the deserialize() call on the next line,
do you need [[maybe_unused]] here ? Same below, and in a few other
locations.

> +	{
> +		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs);
> +	}
> +
> +	static {{struct|name_full(namespace)}}
> +	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,
> +		    [[maybe_unused]] ControlSerializer *cs = nullptr)
> +	{
> +		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(dataBegin, dataEnd, cs);
> +	}
> +{%- endmacro %}
> +
> +
> +{#
> + # \brief Deserialize a struct that has no fds
> + #
> + # Generate code for IPADataSerializer specialization, for deserializing
> + # \a struct, in the case that \a struct does not have file descriptors.
> + #}
> +{%- macro deserializer_no_fd(struct, namespace) %}
> +	static {{struct|name_full(namespace)}}
> +	deserialize(std::vector<uint8_t> &data,
> +{%- if struct|needs_control_serializer %}
> +		    ControlSerializer *cs)
> +{%- else %}
> +		    [[maybe_unused]] ControlSerializer *cs = nullptr)
> +{%- endif %}
> +	{
> +		return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs);
> +	}
> +
> +	static {{struct|name_full(namespace)}}
> +	deserialize(std::vector<uint8_t>::const_iterator dataBegin,
> +		    std::vector<uint8_t>::const_iterator dataEnd,
> +{%- if struct|needs_control_serializer %}
> +		    ControlSerializer *cs)
> +{%- else %}
> +		    [[maybe_unused]] ControlSerializer *cs = nullptr)
> +{%- endif %}
> +	{
> +		{{struct|name_full(namespace)}} ret;
> +		std::vector<uint8_t>::const_iterator m = dataBegin;
> +
> +		size_t dataSize = std::distance(dataBegin, dataEnd);
> +{%- for field in struct.fields -%}
> +{{deserializer_field(field, namespace, loop)}}
> +{%- endfor %}
> +		return ret;
> +	}
> +{%- endmacro %}
> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py
> new file mode 100644
> index 00000000..5c4ad4fe
> --- /dev/null
> +++ b/utils/ipc/generators/mojom_libcamera_generator.py
> @@ -0,0 +1,511 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2020, Google Inc.
> +#
> +# Author: Paul Elder <paul.elder at ideasonboard.com>
> +#
> +# mojom_libcamera_generator.py - Generates libcamera files from a mojom.Module.
> +
> +import argparse
> +import datetime
> +import os
> +import re
> +
> +import mojom.fileutil as fileutil
> +import mojom.generate.generator as generator
> +import mojom.generate.module as mojom
> +from mojom.generate.template_expander import UseJinja
> +
> +
> +GENERATOR_PREFIX = 'libcamera'
> +
> +_kind_to_cpp_type = {
> +    mojom.BOOL:   'bool',
> +    mojom.INT8:   'int8_t',
> +    mojom.UINT8:  'uint8_t',
> +    mojom.INT16:  'int16_t',
> +    mojom.UINT16: 'uint16_t',
> +    mojom.INT32:  'int32_t',
> +    mojom.UINT32: 'uint32_t',
> +    mojom.FLOAT:  'float',
> +    mojom.INT64:  'int64_t',
> +    mojom.UINT64: 'uint64_t',
> +    mojom.DOUBLE: 'double',
> +}
> +
> +_bit_widths = {
> +    mojom.BOOL:   '8',
> +    mojom.INT8:   '8',
> +    mojom.UINT8:  '8',
> +    mojom.INT16:  '16',
> +    mojom.UINT16: '16',
> +    mojom.INT32:  '32',
> +    mojom.UINT32: '32',
> +    mojom.FLOAT:  '32',
> +    mojom.INT64:  '64',
> +    mojom.UINT64: '64',
> +    mojom.DOUBLE: '64',
> +}
> +
> +def ModuleName(path):
> +    return path.split('/')[-1].split('.')[0]
> +
> +def ModuleClassName(module):
> +    return re.sub(r'^IPA(.*)Interface$', lambda match: match.group(1),
> +                  module.interfaces[0].mojom_name)
> +
> +def Capitalize(name):
> +    return name[0].upper() + name[1:]
> +
> +def ConstantStyle(name):
> +    return generator.ToUpperSnakeCase(name)
> +
> +def Choose(cond, t, f):
> +    return t if cond else f
> +
> +def CommaSep(l):
> +    return ', '.join([m for m in l])
> +
> +def ParamsCommaSep(l):
> +    return ', '.join([m.mojom_name for m in l])
> +
> +def GetDefaultValue(element):
> +    if element.default is not None:
> +        return element.default
> +    if type(element.kind) == mojom.Kind:
> +        return '0'
> +    if mojom.IsEnumKind(element.kind):
> +        return f'static_cast<{element.kind.mojom_name}>(0)'
> +    if isinstance(element.kind, mojom.Struct) and \
> +       element.kind.mojom_name == 'FileDescriptor':
> +        return '-1'
> +    return ''
> +
> +def HasDefaultValue(element):
> +    return GetDefaultValue(element) != ''
> +
> +def HasDefaultFields(element):
> +    return True in [HasDefaultValue(x) for x in element.fields]
> +
> +def GetAllTypes(element):
> +    if mojom.IsArrayKind(element):
> +        return GetAllTypes(element.kind)
> +    if mojom.IsMapKind(element):
> +        return GetAllTypes(element.key_kind) + GetAllTypes(element.value_kind)
> +    if isinstance(element, mojom.Parameter):
> +        return GetAllTypes(element.kind)
> +    if mojom.IsEnumKind(element):
> +        return [element.mojom_name]
> +    if not mojom.IsStructKind(element):
> +        return [element.spec]
> +    if len(element.fields) == 0:
> +        return [element.mojom_name]
> +    ret = [GetAllTypes(x.kind) for x in element.fields]
> +    ret = [x for sublist in ret for x in sublist]
> +    return list(set(ret))
> +
> +def GetAllAttrs(element):
> +    if mojom.IsArrayKind(element):
> +        return GetAllAttrs(element.kind)
> +    if mojom.IsMapKind(element):
> +        return {**GetAllAttrs(element.key_kind), **GetAllAttrs(element.value_kind)}
> +    if isinstance(element, mojom.Parameter):
> +        return GetAllAttrs(element.kind)
> +    if mojom.IsEnumKind(element):
> +        return element.attributes if element.attributes is not None else {}
> +    if mojom.IsStructKind(element) and len(element.fields) == 0:
> +        return element.attributes if element.attributes is not None else {}
> +    if not mojom.IsStructKind(element):
> +        if hasattr(element, 'attributes'):
> +            return element.attributes or {}
> +        return {}
> +    attrs = [(x.attributes) for x in element.fields]
> +    ret = {}
> +    for d in attrs:
> +        ret.update(d or {})
> +    if hasattr(element, 'attributes'):
> +        ret.update(element.attributes or {})
> +    return ret
> +
> +def NeedsControlSerializer(element):
> +    types = GetAllTypes(element)
> +    return "ControlList" in types or "ControlInfoMap" in types
> +
> +def HasFd(element):
> +    attrs = GetAllAttrs(element)
> +    if isinstance(element, mojom.Kind):
> +        types = GetAllTypes(element)
> +    else:
> +        types = GetAllTypes(element.kind)
> +    return "FileDescriptor" in types or (attrs is not None and "hasFd" in attrs)
> +
> +def WithDefaultValues(element):
> +    return [x for x in element if HasDefaultValue(x)]
> +
> +def WithFds(element):
> +    return [x for x in element if HasFd(x)]
> +
> +def MethodParamInputs(method):
> +    return method.parameters
> +
> +def MethodParamOutputs(method):
> +    if (MethodReturnValue(method) != 'void' or
> +        method.response_parameters is None):

No need for outer parentheses.

> +        return []
> +    return method.response_parameters
> +
> +def MethodParamsHaveFd(parameters):
> +    return len([x for x in parameters if HasFd(x)]) > 0
> +
> +def MethodInputHasFd(method):
> +    return MethodParamsHaveFd(method.parameters)
> +
> +def MethodOutputHasFd(method):
> +    if (MethodReturnValue(method) != 'void' or
> +        method.response_parameters is None):

Same here.

> +        return False
> +    return MethodParamsHaveFd(method.response_parameters)

Could this be written

    return MethodParamHasFd(MethodParamOutputs(method))

?

> +
> +def MethodParamNames(method):
> +    params = []
> +    for param in method.parameters:
> +        params.append(param.mojom_name)
> +    if MethodReturnValue(method) == 'void':
> +        if method.response_parameters is None:
> +            return params
> +        for param in method.response_parameters:
> +            params.append(param.mojom_name)
> +    return params
> +
> +def MethodParameters(method):
> +    params = []
> +    for param in method.parameters:
> +        params.append('const %s %s%s' % (GetNameForElement(param),
> +                                         '&' if not IsPod(param) else '',
> +                                         param.mojom_name))
> +    if MethodReturnValue(method) == 'void':
> +        if method.response_parameters is None:
> +            return params
> +        for param in method.response_parameters:
> +            params.append(f'{GetNameForElement(param)} *{param.mojom_name}')
> +    return params
> +
> +def MethodReturnValue(method):
> +    if method.response_parameters is None:
> +        return 'void'
> +    if len(method.response_parameters) == 1 and IsPod(method.response_parameters[0]):
> +        return GetNameForElement(method.response_parameters[0])
> +    return 'void'
> +
> +def IsAsync(method):
> +    # Events are always async
> +    if re.match("^IPA.*EventInterface$", method.interface.mojom_name):
> +        return True
> +    elif re.match("^IPA.*Interface$", method.interface.mojom_name):
> +        if method.attributes is None:
> +            return False
> +        elif 'async' in method.attributes and method.attributes['async']:
> +            return True
> +    return False
> +
> +def IsArray(element):
> +    return mojom.IsArrayKind(element.kind)
> +
> +def IsControls(element):
> +    return mojom.IsStructKind(element.kind) and (element.kind.mojom_name == "ControlList" or
> +                                                 element.kind.mojom_name == "ControlInfoMap")
> +
> +def IsEnum(element):
> +    return mojom.IsEnumKind(element.kind)
> +
> +def IsFd(element):
> +    return mojom.IsStructKind(element.kind) and element.kind.mojom_name == "FileDescriptor"
> +
> +def IsMap(element):
> +    return mojom.IsMapKind(element.kind)
> +
> +def IsPlainStruct(element):
> +    return mojom.IsStructKind(element.kind) and not IsControls(element) and not IsFd(element)
> +
> +def IsPod(element):
> +    return element.kind in _kind_to_cpp_type
> +
> +def IsStr(element):
> +    return element.kind.spec == 's'
> +
> +def BitWidth(element):
> +    if element.kind in _bit_widths:
> +        return _bit_widths[element.kind]
> +    if mojom.IsEnumKind(element.kind):
> +        return '32'
> +    return ''
> +
> +# Get the type name for a given element
> +def GetNameForElement(element):
> +    # structs
> +    if (mojom.IsEnumKind(element) or
> +        mojom.IsInterfaceKind(element) or
> +        mojom.IsStructKind(element)):

You really like your outer parentheses :-) Same below.

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

> +        return element.mojom_name
> +    # vectors
> +    if (mojom.IsArrayKind(element)):
> +        elem_name = GetNameForElement(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)
> +        return f'std::map<{key_name}, {value_name}>'
> +    # struct fields and function parameters
> +    if isinstance(element, (mojom.Field, mojom.Method, mojom.Parameter)):
> +        # maps and vectors
> +        if (mojom.IsArrayKind(element.kind) or mojom.IsMapKind(element.kind)):
> +            return GetNameForElement(element.kind)
> +        # strings
> +        if (mojom.IsReferenceKind(element.kind) and element.kind.spec == 's'):
> +            return 'std::string'
> +        # PODs
> +        if element.kind in _kind_to_cpp_type:
> +            return _kind_to_cpp_type[element.kind]
> +        # structs and enums
> +        return element.kind.mojom_name
> +    # PODs that are members of vectors/maps
> +    if (hasattr(element, '__hash__') and element in _kind_to_cpp_type):
> +        return _kind_to_cpp_type[element]
> +    if (hasattr(element, 'spec')):
> +        # strings that are members of vectors/maps
> +        if (element.spec == 's'):
> +            return 'std::string'
> +        # structs that aren't defined in mojom that are members of vectors/maps
> +        if (element.spec[0] == 'x'):
> +            return element.spec.replace('x:', '').replace('.', '::')
> +    if (mojom.IsInterfaceRequestKind(element) or
> +        mojom.IsAssociatedKind(element) or
> +        mojom.IsPendingRemoteKind(element) or
> +        mojom.IsPendingReceiverKind(element) or
> +        mojom.IsUnionKind(element)):
> +        raise Exception('Unsupported element: %s' % element)
> +    raise Exception('Unexpected element: %s' % element)
> +
> +def GetFullNameForElement(element, namespace_str):
> +    name = GetNameForElement(element)
> +    if namespace_str == '':
> +        return name
> +    return f'{namespace_str}::{name}'
> +
> +def ValidateZeroLength(l, s, cap=True):
> +    if l is None:
> +        return
> +    if len(l) > 0:
> +        raise Exception(f'{s.capitalize() if cap else s} should be empty')
> +
> +def ValidateSingleLength(l, s, cap=True):
> +    if len(l) > 1:
> +        raise Exception(f'Only one {s} allowed')
> +    if len(l) < 1:
> +        raise Exception(f'{s.capitalize() if cap else s} is required')
> +
> +def GetMainInterface(interfaces):
> +    intf = [x for x in interfaces
> +            if re.match("^IPA.*Interface", x.mojom_name) and
> +               not re.match("^IPA.*EventInterface", x.mojom_name)]
> +    ValidateSingleLength(intf, 'main interface')
> +    return None if len(intf) == 0 else intf[0]
> +
> +def GetEventInterface(interfaces):
> +    event = [x for x in interfaces if re.match("^IPA.*EventInterface", x.mojom_name)]
> +    ValidateSingleLength(event, 'event interface')
> +    return None if len(event) == 0 else event[0]
> +
> +def ValidateNamespace(namespace):
> +    if namespace == '':
> +        raise Exception('Must have a namespace')
> +
> +    if not re.match('^ipa\.[0-9A-Za-z_]+', namespace):
> +        raise Exception('Namespace must be of the form "ipa.{pipeline_name}"')
> +
> +def ValidateInterfaces(interfaces):
> +    # Validate presence of main interface
> +    intf = GetMainInterface(interfaces)
> +    if intf is None:
> +        raise Exception('Must have main IPA interface')
> +
> +    # Validate presence of event interface
> +    event = GetEventInterface(interfaces)
> +    if intf is None:
> +        raise Exception('Must have event IPA interface')
> +
> +    # Validate required main interface functions
> +    f_init  = [x for x in intf.methods if x.mojom_name == 'init']
> +    f_start = [x for x in intf.methods if x.mojom_name == 'start']
> +    f_stop  = [x for x in intf.methods if x.mojom_name == 'stop']
> +
> +    ValidateSingleLength(f_init, 'init()', False)
> +    ValidateSingleLength(f_start, 'start()', False)
> +    ValidateSingleLength(f_stop, 'stop()', False)
> +
> +    f_init  = f_init[0]
> +    f_start = f_start[0]
> +    f_stop  = f_stop[0]
> +
> +    # Validate parameters to init()
> +    ValidateSingleLength(f_init.parameters, 'input parameter to init()')
> +    ValidateSingleLength(f_init.response_parameters, 'output parameter from init()')
> +    if f_init.parameters[0].kind.mojom_name != 'IPASettings':
> +        raise Exception('init() must have single IPASettings input parameter')
> +    if f_init.response_parameters[0].kind.spec != 'i32':
> +        raise Exception('init() must have single int32 output parameter')
> +
> +    # No need to validate start() as it is customizable
> +
> +    # Validate parameters to stop()
> +    ValidateZeroLength(f_stop.parameters, 'input parameter to stop()')
> +    ValidateZeroLength(f_stop.parameters, 'output parameter from stop()')
> +
> +    # Validate that event interface has at least one event
> +    if len(event.methods) < 1:
> +        raise Exception('Event interface must have at least one event')
> +
> +    # Validate that all async methods don't have return values
> +    intf_methods_async = [x for x in intf.methods if IsAsync(x)]
> +    for method in intf_methods_async:
> +        ValidateZeroLength(method.response_parameters,
> +                           f'{method.mojom_name} response parameters', False)
> +
> +    event_methods_async = [x for x in event.methods if IsAsync(x)]
> +    for method in event_methods_async:
> +        ValidateZeroLength(method.response_parameters,
> +                           f'{method.mojom_name} response parameters', False)
> +
> +class Generator(generator.Generator):
> +    @staticmethod
> +    def GetTemplatePrefix():
> +        return 'libcamera_templates'
> +
> +    def GetFilters(self):
> +        libcamera_filters = {
> +            'all_types': GetAllTypes,
> +            'bit_width': BitWidth,
> +            'cap': Capitalize,
> +            'choose': Choose,
> +            'comma_sep': CommaSep,
> +            'default_value': GetDefaultValue,
> +            'has_default_fields': HasDefaultFields,
> +            'has_fd': HasFd,
> +            'is_async': IsAsync,
> +            'is_array': IsArray,
> +            'is_controls': IsControls,
> +            'is_enum': IsEnum,
> +            'is_fd': IsFd,
> +            'is_map': IsMap,
> +            'is_plain_struct': IsPlainStruct,
> +            'is_pod': IsPod,
> +            'is_str': IsStr,
> +            'method_input_has_fd': MethodInputHasFd,
> +            'method_output_has_fd': MethodOutputHasFd,
> +            'method_param_names': MethodParamNames,
> +            'method_param_inputs': MethodParamInputs,
> +            'method_param_outputs': MethodParamOutputs,
> +            'method_parameters': MethodParameters,
> +            'method_return_value': MethodReturnValue,
> +            'name': GetNameForElement,
> +            'name_full': GetFullNameForElement,
> +            'needs_control_serializer': NeedsControlSerializer,
> +            'params_comma_sep': ParamsCommaSep,
> +            'with_default_values': WithDefaultValues,
> +            'with_fds': WithFds,
> +        }
> +        return libcamera_filters
> +
> +    def _GetJinjaExports(self):
> +        return {
> +            'cmd_enum_name': '_%sCmd' % self.module_name,
> +            'cmd_event_enum_name': '_%sEventCmd' % self.module_name,
> +            'consts': self.module.constants,
> +            'enums': self.module.enums,
> +            'has_array': len([x for x in self.module.kinds.keys() if x[0] == 'a']) > 0,
> +            'has_map': len([x for x in self.module.kinds.keys() if x[0] == 'm']) > 0,
> +            'has_namespace': self.module.mojom_namespace != '',
> +            'interface_event': GetEventInterface(self.module.interfaces),
> +            'interface_main': GetMainInterface(self.module.interfaces),
> +            'interface_name': 'IPA%sInterface' % self.module_name,
> +            'module_name': ModuleName(self.module.path),
> +            'namespace': self.module.mojom_namespace.split('.'),
> +            'namespace_str': self.module.mojom_namespace.replace('.', '::') if
> +                             self.module.mojom_namespace is not None else '',
> +            'proxy_name': 'IPAProxy%s' % self.module_name,
> +            'proxy_worker_name': 'IPAProxy%sWorker' % self.module_name,
> +            'structs_nonempty': [x for x in self.module.structs if len(x.fields) > 0],
> +        }
> +
> +    def _GetJinjaExportsForCore(self):
> +        return {
> +            'consts': self.module.constants,
> +            'enums': self.module.enums,
> +            'has_array': len([x for x in self.module.kinds.keys() if x[0] == 'a']) > 0,
> +            'has_map': len([x for x in self.module.kinds.keys() if x[0] == 'm']) > 0,
> +            'structs_gen_header': [x for x in self.module.structs if x.attributes is not None and 'genHeader' in x.attributes],
> +            'structs_gen_serializer': [x for x in self.module.structs if x.attributes is not None and 'genSerdes' in x.attributes],
> +        }
> +
> +    @UseJinja('core_ipa_interface.h.tmpl')
> +    def _GenerateCoreHeader(self):
> +        return self._GetJinjaExportsForCore()
> +
> +    @UseJinja('core_ipa_serializer.h.tmpl')
> +    def _GenerateCoreSerializer(self):
> +        return self._GetJinjaExportsForCore()
> +
> +    @UseJinja('module_ipa_interface.h.tmpl')
> +    def _GenerateDataHeader(self):
> +        return self._GetJinjaExports()
> +
> +    @UseJinja('module_ipa_serializer.h.tmpl')
> +    def _GenerateSerializer(self):
> +        return self._GetJinjaExports()
> +
> +    @UseJinja('module_ipa_proxy.cpp.tmpl')
> +    def _GenerateProxyCpp(self):
> +        return self._GetJinjaExports()
> +
> +    @UseJinja('module_ipa_proxy.h.tmpl')
> +    def _GenerateProxyHeader(self):
> +        return self._GetJinjaExports()
> +
> +    @UseJinja('module_ipa_proxy_worker.cpp.tmpl')
> +    def _GenerateProxyWorker(self):
> +        return self._GetJinjaExports()
> +
> +    def GenerateFiles(self, unparsed_args):
> +        parser = argparse.ArgumentParser()
> +        parser.add_argument('--libcamera_generate_core_header',     action='store_true')
> +        parser.add_argument('--libcamera_generate_core_serializer', action='store_true')
> +        parser.add_argument('--libcamera_generate_header',          action='store_true')
> +        parser.add_argument('--libcamera_generate_serializer',      action='store_true')
> +        parser.add_argument('--libcamera_generate_proxy_cpp',       action='store_true')
> +        parser.add_argument('--libcamera_generate_proxy_h',         action='store_true')
> +        parser.add_argument('--libcamera_generate_proxy_worker',    action='store_true')
> +        parser.add_argument('--libcamera_output_path')
> +        args = parser.parse_args(unparsed_args)
> +
> +        if not args.libcamera_generate_core_header and \
> +           not args.libcamera_generate_core_serializer:
> +            ValidateNamespace(self.module.mojom_namespace)
> +            ValidateInterfaces(self.module.interfaces)
> +            self.module_name = ModuleClassName(self.module)
> +
> +        fileutil.EnsureDirectoryExists(os.path.dirname(args.libcamera_output_path))
> +
> +        gen_funcs = [
> +                [args.libcamera_generate_core_header,     self._GenerateCoreHeader],
> +                [args.libcamera_generate_core_serializer, self._GenerateCoreSerializer],
> +                [args.libcamera_generate_header,          self._GenerateDataHeader],
> +                [args.libcamera_generate_serializer,      self._GenerateSerializer],
> +                [args.libcamera_generate_proxy_cpp,       self._GenerateProxyCpp],
> +                [args.libcamera_generate_proxy_h,         self._GenerateProxyHeader],
> +                [args.libcamera_generate_proxy_worker,    self._GenerateProxyWorker],
> +        ]
> +
> +        for pair in gen_funcs:
> +            if pair[0]:
> +                self.Write(pair[1](), args.libcamera_output_path)

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list