[libcamera-devel] [PATCH v7 02/10] utils: ipc: add templates for code generation for IPC mechanism
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Feb 12 03:01:27 CET 2021
Hi Paul,
Another comment.
On Fri, Feb 12, 2021 at 02:25:00AM +0200, Laurent Pinchart wrote:
> On Thu, Feb 11, 2021 at 04:17:57PM +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>
> > Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > ---
> > Changes in v7:
> > - cosmetic changes, add a few todos
> > - use the new sendSync/sendAsync IPCPipe API
> > - save the sequence number in the proxy
> > - construct the header before calling sendSync/sendAsync (from the
> > proxy)
> > - replace genHeader and genSerdes with skipHeader and skipSerdes in
> > core.mojom
> >
> > 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 | 40 ++
> > .../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 | 236 ++++++++
> > .../module_ipa_proxy.h.tmpl | 128 +++++
> > .../module_ipa_proxy_worker.cpp.tmpl | 226 ++++++++
> > .../module_ipa_serializer.h.tmpl | 48 ++
> > .../libcamera_templates/proxy_functions.tmpl | 194 +++++++
> > .../libcamera_templates/serializer.tmpl | 313 +++++++++++
> > .../generators/mojom_libcamera_generator.py | 508 ++++++++++++++++++
> > 12 files changed, 1894 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
[snip]
> > 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..2bc187f2
> > --- /dev/null
> > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> > @@ -0,0 +1,128 @@
> > +{#-
> > + # 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/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/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(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);
> > +{% 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_;
> > +
> > + uint32_t seq_;
I don't think this belongs here, the sequence number should be an
internal property of the IPCPipe implementation. As it won't be
straightforward to solve, you can just add a todo comment here.
> > +};
> > +
> > +{%- if has_namespace %}
> > +{% for ns in namespace|reverse %}
> > +} /* namespace {{ns}} */
> > +{% endfor %}
> > +{%- endif %}
> > +} /* namespace libcamera */
> > +
[snip]
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list