[RFC,v1] utils: codegen: ipc: Split proxy types

24056 diff mbox series

Even though there is an abstract class to represent the interface of an IPA,
the threaded and IPC versions are still multiplexed using the same type,
which uses a boolean to actually dispatch to the right function.

Instead of doing that, split the classes into a "local" and "remote" variant,
and make `IPAManager` choose accordingly.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/internal/ipa_manager.h      |   8 +-
 .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
 .../module_ipa_proxy.h.tmpl                   |  56 +++++--
 3 files changed, 118 insertions(+), 95 deletions(-)

Hi Barnabás,

Thanks for the patch.

Quoting Barnabás Pőcze (2025-08-05 01:52:26)
> Even though there is an abstract class to represent the interface of an IPA,
> the threaded and IPC versions are still multiplexed using the same type,
> which uses a boolean to actually dispatch to the right function.
> 
> Instead of doing that, split the classes into a "local" and "remote" variant,
> and make `IPAManager` choose accordingly.

Wow, that is a cool upgrade!

> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  include/libcamera/internal/ipa_manager.h      |   8 +-
>  .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
>  .../module_ipa_proxy.h.tmpl                   |  56 +++++--
>  3 files changed, 118 insertions(+), 95 deletions(-)
> 
> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> index a0d448cf9..847a69066 100644
> --- a/include/libcamera/internal/ipa_manager.h
> +++ b/include/libcamera/internal/ipa_manager.h
> @@ -42,7 +42,13 @@ public:
>                 if (!m)
>                         return nullptr;
>  
> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));
> +               auto proxy = [&]() -> std::unique_ptr<T> {
> +                       if (self->isSignatureValid(m))
> +                               return std::make_unique<typename T::Local>(m);
> +                       else
> +                               return std::make_unique<typename T::Remote>(m);
> +               } ();
> +
>                 if (!proxy->isValid()) {
>                         LOG(IPAManager, Error) << "Failed to load proxy";
>                         return nullptr;
> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> index beb646e2d..8792b3b9e 100644
> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> @@ -45,36 +45,13 @@ namespace {{ns}} {
>  {% endfor %}
>  {%- endif %}
>  
> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
> -       : IPAProxy(ipam), isolate_(isolate),
> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
> +{{proxy_name}}Local::{{proxy_name}}Local(IPAModule *ipam)
> +       : {{proxy_name}}(ipam)
>  {
>         LOG(IPAProxy, Debug)
>                 << "initializing {{module_name}} proxy: loading IPA from "
>                 << ipam->path();

Might it be worth it to also print information about isolation? Same for the
constructor below.

>  
> -       if (isolate_) {
> -               const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> -               if (proxyWorkerPath.empty()) {
> -                       LOG(IPAProxy, Error)
> -                               << "Failed to get proxy worker path";
> -                       return;
> -               }
> -
> -               auto 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);
> -
> -               ipc_ = std::move(ipc);
> -               valid_ = true;
> -               return;
> -       }
> -
>         if (!ipam->load())
>                 return;
>  
> @@ -89,59 +66,16 @@ namespace {{ns}} {
>         proxy_.setIPA(ipa_.get());
>  
>  {% for method in interface_event.methods %}
> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Local::{{method.mojom_name}}Handler);
>  {%- endfor %}
>  
>         valid_ = true;
>  }
>  
> -{{proxy_name}}::~{{proxy_name}}()
> -{
> -       if (ipc_) {
> -               IPCMessage::Header header =
> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
> -               IPCMessage msg(header);
> -               ipc_->sendAsync(msg);
> -       }
> -}
> -
> -{% if interface_event.methods|length > 0 %}
> -void {{proxy_name}}::recvMessage(const IPCMessage &data)
> -{
> -       size_t dataSize = data.data().size();
> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
> -
> -       switch (_cmd) {
> -{%- for method in interface_event.methods %}
> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
> -               break;
> -       }
> -{%- endfor %}
> -       default:
> -               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> -       }
> -}
> -{%- endif %}
> +{{proxy_name}}Local::~{{proxy_name}}Local() = default;
>  
>  {% for method in interface_main.methods %}
> -{{proxy_funcs.func_sig(proxy_name, method)}}
> -{
> -       if (isolate_)
> -               return {{method.mojom_name}}IPC(
> -{%- for param in method|method_param_names -%}
> -               {{param}}{{- ", " if not loop.last}}
> -{%- endfor -%}
> -);
> -       else
> -               return {{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")}}
> +{{proxy_funcs.func_sig(proxy_name + "Local", method)}}
>  {
>  {%- if method.mojom_name == "stop" %}
>         {{proxy_funcs.stop_thread_body()}}
> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>  );
>  {%- endif %}
>  }
> +{% endfor %}
>  
> -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
> +{% for method in interface_event.methods %}
> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler")}}
> +{
> +       ASSERT(state_ != ProxyStopped);
> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> +}
> +{% endfor %}
> +
> +/* ========================================================================== */
> +
> +{{proxy_name}}Remote::{{proxy_name}}Remote(IPAModule *ipam)
> +       : {{proxy_name}}(ipam),
> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
> +{
> +       LOG(IPAProxy, Debug)
> +               << "initializing {{module_name}} proxy: loading IPA from "
> +               << ipam->path();

Here ^

> +
> +       const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> +       if (proxyWorkerPath.empty()) {
> +               LOG(IPAProxy, Error)
> +                       << "Failed to get proxy worker path";
> +               return;
> +       }
> +
> +       auto 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}}Remote::recvMessage);
> +
> +       ipc_ = std::move(ipc);
> +       valid_ = true;
> +}
> +
> +{{proxy_name}}Remote::~{{proxy_name}}Remote()
> +{
> +       if (ipc_) {
> +               IPCMessage::Header header =
> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
> +               IPCMessage msg(header);
> +               ipc_->sendAsync(msg);
> +       }
> +}
> +
> +{% for method in interface_main.methods %}
> +{{proxy_funcs.func_sig(proxy_name + "Remote", method)}}
>  {
>  {%- if method.mojom_name == "configure" %}
>         controlSerializer_.reset();
> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>  
>  {% endfor %}
>  
> -{% for method in interface_event.methods %}
> -{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
> +void {{proxy_name}}Remote::recvMessage(const IPCMessage &data)
>  {
> -       ASSERT(state_ != ProxyStopped);
> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> +       size_t dataSize = data.data().size();
> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
> +
> +       switch (_cmd) {
> +{%- for method in interface_event.methods %}
> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());
> +               break;
> +       }
> +{%- endfor %}
> +       default:
> +               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> +       }
>  }
>  
> -void {{proxy_name}}::{{method.mojom_name}}IPC(
> +{% for method in interface_event.methods %}
> +void {{proxy_name}}Remote::{{method.mojom_name}}Handler(
>         [[maybe_unused]] std::vector<uint8_t>::const_iterator data,
>         [[maybe_unused]] size_t dataSize,
>         [[maybe_unused]] const std::vector<SharedFD> &fds)
> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> index a0312a7c1..0c4ef985b 100644
> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> @@ -34,29 +34,32 @@ namespace {{ns}} {
>  {% endfor %}
>  {%- endif %}
>  
> +class {{proxy_name}}Local;
> +class {{proxy_name}}Remote;
> +
>  class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
>  {
>  public:
> -       {{proxy_name}}(IPAModule *ipam, bool isolate);
> -       ~{{proxy_name}}();
> +       using Local = {{proxy_name}}Local;
> +       using Remote = {{proxy_name}}Remote;
>  
> -{% for method in interface_main.methods %}
> -{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
> -{% endfor %}
> +protected:
> +       using IPAProxy::IPAProxy;
> +};
>  
> -private:
> -       void recvMessage(const IPCMessage &data);
> +class {{proxy_name}}Local : public {{proxy_name}}
> +{
> +public:
> +       {{proxy_name}}Local(IPAModule *ipam);
> +       ~{{proxy_name}}Local();
>  
>  {% 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)}};
> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
>  {% endfor %}
> +
> +private:
>  {% 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<SharedFD> &fds);
> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler", false)|indent(8, true)}};
>  {% endfor %}
>  
>         /* Helper class to invoke async functions in another thread. */
> @@ -79,12 +82,12 @@ private:
>                 }
>  {% for method in interface_main.methods %}
>  {%- if method|is_async %}
> -               {{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
> +               {{proxy_funcs.func_sig(proxy_name + "Local", 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)}}
> +               {{proxy_funcs.func_sig(proxy_name + "Local", method, "", false)|indent(16)}}
>                 {
>  {%- if method|method_return_value != "void" %}
>                         return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
> @@ -104,8 +107,27 @@ private:
>         Thread thread_;
>         ThreadProxy proxy_;
>         std::unique_ptr<{{interface_name}}> ipa_;
> +};
>  
> -       const bool isolate_;
> +class {{proxy_name}}Remote : public {{proxy_name}}
> +{
> +public:
> +       {{proxy_name}}Remote(IPAModule *ipam);
> +       ~{{proxy_name}}Remote();
> +
> +{% for method in interface_main.methods %}
> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};

s/Local/Remote/ ?


Other than that, looks good to me. This is exciting!

Paul

> +{% endfor %}
> +
> +private:
> +       void recvMessage(const IPCMessage &data);
> +
> +{% for method in interface_event.methods %}
> +       void {{method.mojom_name}}Handler(
> +               std::vector<uint8_t>::const_iterator data,
> +               size_t dataSize,
> +               const std::vector<SharedFD> &fds);
> +{% endfor %}
>  
>         std::unique_ptr<IPCPipeUnixSocket> ipc_;
>  
> -- 
> 2.50.1
>

Hi

2025. 08. 14. 10:59 keltezéssel, Paul Elder írta:
> Hi Barnabás,
> 
> Thanks for the patch.
> 
> Quoting Barnabás Pőcze (2025-08-05 01:52:26)
>> Even though there is an abstract class to represent the interface of an IPA,
>> the threaded and IPC versions are still multiplexed using the same type,
>> which uses a boolean to actually dispatch to the right function.
>>
>> Instead of doing that, split the classes into a "local" and "remote" variant,
>> and make `IPAManager` choose accordingly.
> 
> Wow, that is a cool upgrade!
> 
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   include/libcamera/internal/ipa_manager.h      |   8 +-
>>   .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
>>   .../module_ipa_proxy.h.tmpl                   |  56 +++++--
>>   3 files changed, 118 insertions(+), 95 deletions(-)
>>
>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
>> index a0d448cf9..847a69066 100644
>> --- a/include/libcamera/internal/ipa_manager.h
>> +++ b/include/libcamera/internal/ipa_manager.h
>> @@ -42,7 +42,13 @@ public:
>>                  if (!m)
>>                          return nullptr;
>>   
>> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));
>> +               auto proxy = [&]() -> std::unique_ptr<T> {
>> +                       if (self->isSignatureValid(m))
>> +                               return std::make_unique<typename T::Local>(m);
>> +                       else
>> +                               return std::make_unique<typename T::Remote>(m);
>> +               } ();
>> +
>>                  if (!proxy->isValid()) {
>>                          LOG(IPAManager, Error) << "Failed to load proxy";
>>                          return nullptr;
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> index beb646e2d..8792b3b9e 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> @@ -45,36 +45,13 @@ namespace {{ns}} {
>>   {% endfor %}
>>   {%- endif %}
>>   
>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
>> -       : IPAProxy(ipam), isolate_(isolate),
>> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
>> +{{proxy_name}}Local::{{proxy_name}}Local(IPAModule *ipam)
>> +       : {{proxy_name}}(ipam)
>>   {
>>          LOG(IPAProxy, Debug)
>>                  << "initializing {{module_name}} proxy: loading IPA from "
>>                  << ipam->path();
> 
> Might it be worth it to also print information about isolation? Same for the
> constructor below.
> 
>>   
>> -       if (isolate_) {
>> -               const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
>> -               if (proxyWorkerPath.empty()) {
>> -                       LOG(IPAProxy, Error)
>> -                               << "Failed to get proxy worker path";
>> -                       return;
>> -               }
>> -
>> -               auto 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);
>> -
>> -               ipc_ = std::move(ipc);
>> -               valid_ = true;
>> -               return;
>> -       }
>> -
>>          if (!ipam->load())
>>                  return;
>>   
>> @@ -89,59 +66,16 @@ namespace {{ns}} {
>>          proxy_.setIPA(ipa_.get());
>>   
>>   {% for method in interface_event.methods %}
>> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
>> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Local::{{method.mojom_name}}Handler);
>>   {%- endfor %}
>>   
>>          valid_ = true;
>>   }
>>   
>> -{{proxy_name}}::~{{proxy_name}}()
>> -{
>> -       if (ipc_) {
>> -               IPCMessage::Header header =
>> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
>> -               IPCMessage msg(header);
>> -               ipc_->sendAsync(msg);
>> -       }
>> -}
>> -
>> -{% if interface_event.methods|length > 0 %}
>> -void {{proxy_name}}::recvMessage(const IPCMessage &data)
>> -{
>> -       size_t dataSize = data.data().size();
>> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>> -
>> -       switch (_cmd) {
>> -{%- for method in interface_event.methods %}
>> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
>> -               break;
>> -       }
>> -{%- endfor %}
>> -       default:
>> -               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
>> -       }
>> -}
>> -{%- endif %}
>> +{{proxy_name}}Local::~{{proxy_name}}Local() = default;
>>   
>>   {% for method in interface_main.methods %}
>> -{{proxy_funcs.func_sig(proxy_name, method)}}
>> -{
>> -       if (isolate_)
>> -               return {{method.mojom_name}}IPC(
>> -{%- for param in method|method_param_names -%}
>> -               {{param}}{{- ", " if not loop.last}}
>> -{%- endfor -%}
>> -);
>> -       else
>> -               return {{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")}}
>> +{{proxy_funcs.func_sig(proxy_name + "Local", method)}}
>>   {
>>   {%- if method.mojom_name == "stop" %}
>>          {{proxy_funcs.stop_thread_body()}}
>> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>   );
>>   {%- endif %}
>>   }
>> +{% endfor %}
>>   
>> -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
>> +{% for method in interface_event.methods %}
>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler")}}
>> +{
>> +       ASSERT(state_ != ProxyStopped);
>> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>> +}
>> +{% endfor %}
>> +
>> +/* ========================================================================== */
>> +
>> +{{proxy_name}}Remote::{{proxy_name}}Remote(IPAModule *ipam)
>> +       : {{proxy_name}}(ipam),
>> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
>> +{
>> +       LOG(IPAProxy, Debug)
>> +               << "initializing {{module_name}} proxy: loading IPA from "
>> +               << ipam->path();
> 
> Here ^

I'll add "locally" and "remotely" to the messages. Or should it be more concrete?


> 
>> +
>> +       const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
>> +       if (proxyWorkerPath.empty()) {
>> +               LOG(IPAProxy, Error)
>> +                       << "Failed to get proxy worker path";
>> +               return;
>> +       }
>> +
>> +       auto 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}}Remote::recvMessage);
>> +
>> +       ipc_ = std::move(ipc);
>> +       valid_ = true;
>> +}
>> +
>> +{{proxy_name}}Remote::~{{proxy_name}}Remote()
>> +{
>> +       if (ipc_) {
>> +               IPCMessage::Header header =
>> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
>> +               IPCMessage msg(header);
>> +               ipc_->sendAsync(msg);
>> +       }
>> +}
>> +
>> +{% for method in interface_main.methods %}
>> +{{proxy_funcs.func_sig(proxy_name + "Remote", method)}}
>>   {
>>   {%- if method.mojom_name == "configure" %}
>>          controlSerializer_.reset();
>> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>   
>>   {% endfor %}
>>   
>> -{% for method in interface_event.methods %}
>> -{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
>> +void {{proxy_name}}Remote::recvMessage(const IPCMessage &data)
>>   {
>> -       ASSERT(state_ != ProxyStopped);
>> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>> +       size_t dataSize = data.data().size();
>> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>> +
>> +       switch (_cmd) {
>> +{%- for method in interface_event.methods %}
>> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());
>> +               break;
>> +       }
>> +{%- endfor %}
>> +       default:
>> +               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
>> +       }
>>   }
>>   
>> -void {{proxy_name}}::{{method.mojom_name}}IPC(
>> +{% for method in interface_event.methods %}
>> +void {{proxy_name}}Remote::{{method.mojom_name}}Handler(
>>          [[maybe_unused]] std::vector<uint8_t>::const_iterator data,
>>          [[maybe_unused]] size_t dataSize,
>>          [[maybe_unused]] const std::vector<SharedFD> &fds)
>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>> index a0312a7c1..0c4ef985b 100644
>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>> @@ -34,29 +34,32 @@ namespace {{ns}} {
>>   {% endfor %}
>>   {%- endif %}
>>   
>> +class {{proxy_name}}Local;
>> +class {{proxy_name}}Remote;
>> +
>>   class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
>>   {
>>   public:
>> -       {{proxy_name}}(IPAModule *ipam, bool isolate);
>> -       ~{{proxy_name}}();
>> +       using Local = {{proxy_name}}Local;
>> +       using Remote = {{proxy_name}}Remote;
>>   
>> -{% for method in interface_main.methods %}
>> -{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
>> -{% endfor %}
>> +protected:
>> +       using IPAProxy::IPAProxy;
>> +};
>>   
>> -private:
>> -       void recvMessage(const IPCMessage &data);
>> +class {{proxy_name}}Local : public {{proxy_name}}
>> +{
>> +public:
>> +       {{proxy_name}}Local(IPAModule *ipam);
>> +       ~{{proxy_name}}Local();
>>   
>>   {% 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)}};
>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
>>   {% endfor %}
>> +
>> +private:
>>   {% 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<SharedFD> &fds);
>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler", false)|indent(8, true)}};
>>   {% endfor %}
>>   
>>          /* Helper class to invoke async functions in another thread. */
>> @@ -79,12 +82,12 @@ private:
>>                  }
>>   {% for method in interface_main.methods %}
>>   {%- if method|is_async %}
>> -               {{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
>> +               {{proxy_funcs.func_sig(proxy_name + "Local", 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)}}
>> +               {{proxy_funcs.func_sig(proxy_name + "Local", method, "", false)|indent(16)}}
>>                  {
>>   {%- if method|method_return_value != "void" %}
>>                          return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
>> @@ -104,8 +107,27 @@ private:
>>          Thread thread_;
>>          ThreadProxy proxy_;
>>          std::unique_ptr<{{interface_name}}> ipa_;
>> +};
>>   
>> -       const bool isolate_;
>> +class {{proxy_name}}Remote : public {{proxy_name}}
>> +{
>> +public:
>> +       {{proxy_name}}Remote(IPAModule *ipam);
>> +       ~{{proxy_name}}Remote();
>> +
>> +{% for method in interface_main.methods %}
>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
> 
> s/Local/Remote/ ?
> 
> 
> Other than that, looks good to me. This is exciting!

Oops, fixed. I was wondering why it did not cause any issues... of course
the name of the class is not used due to the fourth argument being false.


Regards,
Barnabás Pőcze


> 
> Paul
> 
>> +{% endfor %}
>> +
>> +private:
>> +       void recvMessage(const IPCMessage &data);
>> +
>> +{% for method in interface_event.methods %}
>> +       void {{method.mojom_name}}Handler(
>> +               std::vector<uint8_t>::const_iterator data,
>> +               size_t dataSize,
>> +               const std::vector<SharedFD> &fds);
>> +{% endfor %}
>>   
>>          std::unique_ptr<IPCPipeUnixSocket> ipc_;
>>   
>> -- 
>> 2.50.1
>>

On Thu, Aug 14, 2025 at 11:42:39AM +0200, Barnabás Pőcze wrote:
> 2025. 08. 14. 10:59 keltezéssel, Paul Elder írta:
> > Quoting Barnabás Pőcze (2025-08-05 01:52:26)
> >> Even though there is an abstract class to represent the interface of an IPA,
> >> the threaded and IPC versions are still multiplexed using the same type,
> >> which uses a boolean to actually dispatch to the right function.
> >>
> >> Instead of doing that, split the classes into a "local" and "remote" variant,
> >> and make `IPAManager` choose accordingly.

"Local" and "Remote" are not the clearest names. Would "Threaded" and
"Isolated" be better ? I'm sure there are even better option.

> > Wow, that is a cool upgrade!
> > 
> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >> ---
> >>   include/libcamera/internal/ipa_manager.h      |   8 +-
> >>   .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
> >>   .../module_ipa_proxy.h.tmpl                   |  56 +++++--
> >>   3 files changed, 118 insertions(+), 95 deletions(-)
> >>
> >> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> >> index a0d448cf9..847a69066 100644
> >> --- a/include/libcamera/internal/ipa_manager.h
> >> +++ b/include/libcamera/internal/ipa_manager.h
> >> @@ -42,7 +42,13 @@ public:
> >>                  if (!m)
> >>                          return nullptr;
> >>   
> >> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));
> >> +               auto proxy = [&]() -> std::unique_ptr<T> {
> >> +                       if (self->isSignatureValid(m))
> >> +                               return std::make_unique<typename T::Local>(m);
> >> +                       else
> >> +                               return std::make_unique<typename T::Remote>(m);
> >> +               } ();

Is this using a lambda just for the sake of it ? :-)

		std::unique_ptr<T proxy = self->isSignatureValid(m)
					? std::make_unique<typename T::Local>(m)
					: std::make_unique<typename T::Remote>(m);

> >> +
> >>                  if (!proxy->isValid()) {
> >>                          LOG(IPAManager, Error) << "Failed to load proxy";
> >>                          return nullptr;
> >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >> index beb646e2d..8792b3b9e 100644
> >> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >> @@ -45,36 +45,13 @@ namespace {{ns}} {
> >>   {% endfor %}
> >>   {%- endif %}
> >>   
> >> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
> >> -       : IPAProxy(ipam), isolate_(isolate),
> >> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
> >> +{{proxy_name}}Local::{{proxy_name}}Local(IPAModule *ipam)
> >> +       : {{proxy_name}}(ipam)
> >>   {
> >>          LOG(IPAProxy, Debug)
> >>                  << "initializing {{module_name}} proxy: loading IPA from "
> >>                  << ipam->path();
> > 
> > Might it be worth it to also print information about isolation? Same for the
> > constructor below.
> > 
> >>   
> >> -       if (isolate_) {
> >> -               const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> >> -               if (proxyWorkerPath.empty()) {
> >> -                       LOG(IPAProxy, Error)
> >> -                               << "Failed to get proxy worker path";
> >> -                       return;
> >> -               }
> >> -
> >> -               auto 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);
> >> -
> >> -               ipc_ = std::move(ipc);
> >> -               valid_ = true;
> >> -               return;
> >> -       }
> >> -
> >>          if (!ipam->load())
> >>                  return;
> >>   
> >> @@ -89,59 +66,16 @@ namespace {{ns}} {
> >>          proxy_.setIPA(ipa_.get());
> >>   
> >>   {% for method in interface_event.methods %}
> >> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
> >> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Local::{{method.mojom_name}}Handler);
> >>   {%- endfor %}
> >>   
> >>          valid_ = true;
> >>   }
> >>   
> >> -{{proxy_name}}::~{{proxy_name}}()
> >> -{
> >> -       if (ipc_) {
> >> -               IPCMessage::Header header =
> >> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
> >> -               IPCMessage msg(header);
> >> -               ipc_->sendAsync(msg);
> >> -       }
> >> -}
> >> -
> >> -{% if interface_event.methods|length > 0 %}
> >> -void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >> -{
> >> -       size_t dataSize = data.data().size();
> >> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
> >> -
> >> -       switch (_cmd) {
> >> -{%- for method in interface_event.methods %}
> >> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> >> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
> >> -               break;
> >> -       }
> >> -{%- endfor %}
> >> -       default:
> >> -               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> >> -       }
> >> -}
> >> -{%- endif %}
> >> +{{proxy_name}}Local::~{{proxy_name}}Local() = default;
> >>   
> >>   {% for method in interface_main.methods %}
> >> -{{proxy_funcs.func_sig(proxy_name, method)}}
> >> -{
> >> -       if (isolate_)
> >> -               return {{method.mojom_name}}IPC(
> >> -{%- for param in method|method_param_names -%}
> >> -               {{param}}{{- ", " if not loop.last}}
> >> -{%- endfor -%}
> >> -);
> >> -       else
> >> -               return {{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")}}
> >> +{{proxy_funcs.func_sig(proxy_name + "Local", method)}}
> >>   {
> >>   {%- if method.mojom_name == "stop" %}
> >>          {{proxy_funcs.stop_thread_body()}}
> >> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >>   );
> >>   {%- endif %}
> >>   }
> >> +{% endfor %}
> >>   
> >> -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
> >> +{% for method in interface_event.methods %}
> >> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler")}}
> >> +{
> >> +       ASSERT(state_ != ProxyStopped);
> >> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> >> +}
> >> +{% endfor %}
> >> +
> >> +/* ========================================================================== */
> >> +
> >> +{{proxy_name}}Remote::{{proxy_name}}Remote(IPAModule *ipam)
> >> +       : {{proxy_name}}(ipam),
> >> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
> >> +{
> >> +       LOG(IPAProxy, Debug)
> >> +               << "initializing {{module_name}} proxy: loading IPA from "
> >> +               << ipam->path();
> > 
> > Here ^
> 
> I'll add "locally" and "remotely" to the messages. Or should it be more concrete?
> 
> >> +
> >> +       const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> >> +       if (proxyWorkerPath.empty()) {
> >> +               LOG(IPAProxy, Error)
> >> +                       << "Failed to get proxy worker path";
> >> +               return;
> >> +       }
> >> +
> >> +       auto 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}}Remote::recvMessage);
> >> +
> >> +       ipc_ = std::move(ipc);
> >> +       valid_ = true;
> >> +}
> >> +
> >> +{{proxy_name}}Remote::~{{proxy_name}}Remote()
> >> +{
> >> +       if (ipc_) {
> >> +               IPCMessage::Header header =
> >> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
> >> +               IPCMessage msg(header);
> >> +               ipc_->sendAsync(msg);
> >> +       }
> >> +}
> >> +
> >> +{% for method in interface_main.methods %}
> >> +{{proxy_funcs.func_sig(proxy_name + "Remote", method)}}
> >>   {
> >>   {%- if method.mojom_name == "configure" %}
> >>          controlSerializer_.reset();
> >> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >>   
> >>   {% endfor %}
> >>   
> >> -{% for method in interface_event.methods %}
> >> -{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
> >> +void {{proxy_name}}Remote::recvMessage(const IPCMessage &data)
> >>   {
> >> -       ASSERT(state_ != ProxyStopped);
> >> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> >> +       size_t dataSize = data.data().size();
> >> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
> >> +
> >> +       switch (_cmd) {
> >> +{%- for method in interface_event.methods %}
> >> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> >> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());
> >> +               break;
> >> +       }
> >> +{%- endfor %}
> >> +       default:
> >> +               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> >> +       }
> >>   }
> >>   
> >> -void {{proxy_name}}::{{method.mojom_name}}IPC(
> >> +{% for method in interface_event.methods %}
> >> +void {{proxy_name}}Remote::{{method.mojom_name}}Handler(
> >>          [[maybe_unused]] std::vector<uint8_t>::const_iterator data,
> >>          [[maybe_unused]] size_t dataSize,
> >>          [[maybe_unused]] const std::vector<SharedFD> &fds)
> >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >> index a0312a7c1..0c4ef985b 100644
> >> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >> @@ -34,29 +34,32 @@ namespace {{ns}} {
> >>   {% endfor %}
> >>   {%- endif %}
> >>   
> >> +class {{proxy_name}}Local;
> >> +class {{proxy_name}}Remote;
> >> +
> >>   class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
> >>   {
> >>   public:
> >> -       {{proxy_name}}(IPAModule *ipam, bool isolate);
> >> -       ~{{proxy_name}}();
> >> +       using Local = {{proxy_name}}Local;
> >> +       using Remote = {{proxy_name}}Remote;
> >>   
> >> -{% for method in interface_main.methods %}
> >> -{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
> >> -{% endfor %}
> >> +protected:
> >> +       using IPAProxy::IPAProxy;
> >> +};
> >>   
> >> -private:
> >> -       void recvMessage(const IPCMessage &data);
> >> +class {{proxy_name}}Local : public {{proxy_name}}
> >> +{
> >> +public:
> >> +       {{proxy_name}}Local(IPAModule *ipam);
> >> +       ~{{proxy_name}}Local();
> >>   
> >>   {% 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)}};
> >> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};

As this is Python code I *think* you could also write

{{proxy_funcs.func_sig(f"{proxy_name}Local", method, "", false, true)|indent(8, true)}};

if you think it's clearer (sae elsewhere in the patch).

> >>   {% endfor %}
> >> +
> >> +private:
> >>   {% 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<SharedFD> &fds);
> >> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler", false)|indent(8, true)}};
> >>   {% endfor %}
> >>   
> >>          /* Helper class to invoke async functions in another thread. */
> >> @@ -79,12 +82,12 @@ private:
> >>                  }
> >>   {% for method in interface_main.methods %}
> >>   {%- if method|is_async %}
> >> -               {{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
> >> +               {{proxy_funcs.func_sig(proxy_name + "Local", 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)}}
> >> +               {{proxy_funcs.func_sig(proxy_name + "Local", method, "", false)|indent(16)}}
> >>                  {
> >>   {%- if method|method_return_value != "void" %}
> >>                          return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
> >> @@ -104,8 +107,27 @@ private:
> >>          Thread thread_;
> >>          ThreadProxy proxy_;
> >>          std::unique_ptr<{{interface_name}}> ipa_;
> >> +};
> >>   
> >> -       const bool isolate_;
> >> +class {{proxy_name}}Remote : public {{proxy_name}}
> >> +{
> >> +public:
> >> +       {{proxy_name}}Remote(IPAModule *ipam);
> >> +       ~{{proxy_name}}Remote();
> >> +
> >> +{% for method in interface_main.methods %}
> >> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
> > 
> > s/Local/Remote/ ?
> > 
> > Other than that, looks good to me. This is exciting!
> 
> Oops, fixed. I was wondering why it did not cause any issues... of course
> the name of the class is not used due to the fourth argument being false.
> 
> >> +{% endfor %}
> >> +
> >> +private:
> >> +       void recvMessage(const IPCMessage &data);
> >> +
> >> +{% for method in interface_event.methods %}
> >> +       void {{method.mojom_name}}Handler(
> >> +               std::vector<uint8_t>::const_iterator data,
> >> +               size_t dataSize,
> >> +               const std::vector<SharedFD> &fds);
> >> +{% endfor %}
> >>   
> >>          std::unique_ptr<IPCPipeUnixSocket> ipc_;
> >>

2025. 08. 15. 0:24 keltezéssel, Laurent Pinchart írta:
> On Thu, Aug 14, 2025 at 11:42:39AM +0200, Barnabás Pőcze wrote:
>> 2025. 08. 14. 10:59 keltezéssel, Paul Elder írta:
>>> Quoting Barnabás Pőcze (2025-08-05 01:52:26)
>>>> Even though there is an abstract class to represent the interface of an IPA,
>>>> the threaded and IPC versions are still multiplexed using the same type,
>>>> which uses a boolean to actually dispatch to the right function.
>>>>
>>>> Instead of doing that, split the classes into a "local" and "remote" variant,
>>>> and make `IPAManager` choose accordingly.
> 
> "Local" and "Remote" are not the clearest names. Would "Threaded" and
> "Isolated" be better ? I'm sure there are even better option.

I'm sure there are endless opportunities. I will rename them.


> 
>>> Wow, that is a cool upgrade!
>>>
>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>> ---
>>>>    include/libcamera/internal/ipa_manager.h      |   8 +-
>>>>    .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
>>>>    .../module_ipa_proxy.h.tmpl                   |  56 +++++--
>>>>    3 files changed, 118 insertions(+), 95 deletions(-)
>>>>
>>>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
>>>> index a0d448cf9..847a69066 100644
>>>> --- a/include/libcamera/internal/ipa_manager.h
>>>> +++ b/include/libcamera/internal/ipa_manager.h
>>>> @@ -42,7 +42,13 @@ public:
>>>>                   if (!m)
>>>>                           return nullptr;
>>>>    
>>>> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));
>>>> +               auto proxy = [&]() -> std::unique_ptr<T> {
>>>> +                       if (self->isSignatureValid(m))
>>>> +                               return std::make_unique<typename T::Local>(m);
>>>> +                       else
>>>> +                               return std::make_unique<typename T::Remote>(m);
>>>> +               } ();
> 
> Is this using a lambda just for the sake of it ? :-)
> 
> 		std::unique_ptr<T proxy = self->isSignatureValid(m)
> 					? std::make_unique<typename T::Local>(m)
> 					: std::make_unique<typename T::Remote>(m);

The lambda is there because the ternary doesn't work.


> 
>>>> +
>>>>                   if (!proxy->isValid()) {
>>>>                           LOG(IPAManager, Error) << "Failed to load proxy";
>>>>                           return nullptr;
>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>> index beb646e2d..8792b3b9e 100644
>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>> @@ -45,36 +45,13 @@ namespace {{ns}} {
>>>>    {% endfor %}
>>>>    {%- endif %}
>>>>    
>>>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
>>>> -       : IPAProxy(ipam), isolate_(isolate),
>>>> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
>>>> +{{proxy_name}}Local::{{proxy_name}}Local(IPAModule *ipam)
>>>> +       : {{proxy_name}}(ipam)
>>>>    {
>>>>           LOG(IPAProxy, Debug)
>>>>                   << "initializing {{module_name}} proxy: loading IPA from "
>>>>                   << ipam->path();
>>>
>>> Might it be worth it to also print information about isolation? Same for the
>>> constructor below.
>>>
>>>>    
>>>> -       if (isolate_) {
>>>> -               const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
>>>> -               if (proxyWorkerPath.empty()) {
>>>> -                       LOG(IPAProxy, Error)
>>>> -                               << "Failed to get proxy worker path";
>>>> -                       return;
>>>> -               }
>>>> -
>>>> -               auto 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);
>>>> -
>>>> -               ipc_ = std::move(ipc);
>>>> -               valid_ = true;
>>>> -               return;
>>>> -       }
>>>> -
>>>>           if (!ipam->load())
>>>>                   return;
>>>>    
>>>> @@ -89,59 +66,16 @@ namespace {{ns}} {
>>>>           proxy_.setIPA(ipa_.get());
>>>>    
>>>>    {% for method in interface_event.methods %}
>>>> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
>>>> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Local::{{method.mojom_name}}Handler);
>>>>    {%- endfor %}
>>>>    
>>>>           valid_ = true;
>>>>    }
>>>>    
>>>> -{{proxy_name}}::~{{proxy_name}}()
>>>> -{
>>>> -       if (ipc_) {
>>>> -               IPCMessage::Header header =
>>>> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
>>>> -               IPCMessage msg(header);
>>>> -               ipc_->sendAsync(msg);
>>>> -       }
>>>> -}
>>>> -
>>>> -{% if interface_event.methods|length > 0 %}
>>>> -void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>> -{
>>>> -       size_t dataSize = data.data().size();
>>>> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>>>> -
>>>> -       switch (_cmd) {
>>>> -{%- for method in interface_event.methods %}
>>>> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>>>> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
>>>> -               break;
>>>> -       }
>>>> -{%- endfor %}
>>>> -       default:
>>>> -               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
>>>> -       }
>>>> -}
>>>> -{%- endif %}
>>>> +{{proxy_name}}Local::~{{proxy_name}}Local() = default;
>>>>    
>>>>    {% for method in interface_main.methods %}
>>>> -{{proxy_funcs.func_sig(proxy_name, method)}}
>>>> -{
>>>> -       if (isolate_)
>>>> -               return {{method.mojom_name}}IPC(
>>>> -{%- for param in method|method_param_names -%}
>>>> -               {{param}}{{- ", " if not loop.last}}
>>>> -{%- endfor -%}
>>>> -);
>>>> -       else
>>>> -               return {{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")}}
>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method)}}
>>>>    {
>>>>    {%- if method.mojom_name == "stop" %}
>>>>           {{proxy_funcs.stop_thread_body()}}
>>>> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>>    );
>>>>    {%- endif %}
>>>>    }
>>>> +{% endfor %}
>>>>    
>>>> -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
>>>> +{% for method in interface_event.methods %}
>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler")}}
>>>> +{
>>>> +       ASSERT(state_ != ProxyStopped);
>>>> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>>>> +}
>>>> +{% endfor %}
>>>> +
>>>> +/* ========================================================================== */
>>>> +
>>>> +{{proxy_name}}Remote::{{proxy_name}}Remote(IPAModule *ipam)
>>>> +       : {{proxy_name}}(ipam),
>>>> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
>>>> +{
>>>> +       LOG(IPAProxy, Debug)
>>>> +               << "initializing {{module_name}} proxy: loading IPA from "
>>>> +               << ipam->path();
>>>
>>> Here ^
>>
>> I'll add "locally" and "remotely" to the messages. Or should it be more concrete?
>>
>>>> +
>>>> +       const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
>>>> +       if (proxyWorkerPath.empty()) {
>>>> +               LOG(IPAProxy, Error)
>>>> +                       << "Failed to get proxy worker path";
>>>> +               return;
>>>> +       }
>>>> +
>>>> +       auto 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}}Remote::recvMessage);
>>>> +
>>>> +       ipc_ = std::move(ipc);
>>>> +       valid_ = true;
>>>> +}
>>>> +
>>>> +{{proxy_name}}Remote::~{{proxy_name}}Remote()
>>>> +{
>>>> +       if (ipc_) {
>>>> +               IPCMessage::Header header =
>>>> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
>>>> +               IPCMessage msg(header);
>>>> +               ipc_->sendAsync(msg);
>>>> +       }
>>>> +}
>>>> +
>>>> +{% for method in interface_main.methods %}
>>>> +{{proxy_funcs.func_sig(proxy_name + "Remote", method)}}
>>>>    {
>>>>    {%- if method.mojom_name == "configure" %}
>>>>           controlSerializer_.reset();
>>>> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>>    
>>>>    {% endfor %}
>>>>    
>>>> -{% for method in interface_event.methods %}
>>>> -{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
>>>> +void {{proxy_name}}Remote::recvMessage(const IPCMessage &data)
>>>>    {
>>>> -       ASSERT(state_ != ProxyStopped);
>>>> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>>>> +       size_t dataSize = data.data().size();
>>>> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>>>> +
>>>> +       switch (_cmd) {
>>>> +{%- for method in interface_event.methods %}
>>>> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>>>> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());
>>>> +               break;
>>>> +       }
>>>> +{%- endfor %}
>>>> +       default:
>>>> +               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
>>>> +       }
>>>>    }
>>>>    
>>>> -void {{proxy_name}}::{{method.mojom_name}}IPC(
>>>> +{% for method in interface_event.methods %}
>>>> +void {{proxy_name}}Remote::{{method.mojom_name}}Handler(
>>>>           [[maybe_unused]] std::vector<uint8_t>::const_iterator data,
>>>>           [[maybe_unused]] size_t dataSize,
>>>>           [[maybe_unused]] const std::vector<SharedFD> &fds)
>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>> index a0312a7c1..0c4ef985b 100644
>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>> @@ -34,29 +34,32 @@ namespace {{ns}} {
>>>>    {% endfor %}
>>>>    {%- endif %}
>>>>    
>>>> +class {{proxy_name}}Local;
>>>> +class {{proxy_name}}Remote;
>>>> +
>>>>    class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
>>>>    {
>>>>    public:
>>>> -       {{proxy_name}}(IPAModule *ipam, bool isolate);
>>>> -       ~{{proxy_name}}();
>>>> +       using Local = {{proxy_name}}Local;
>>>> +       using Remote = {{proxy_name}}Remote;
>>>>    
>>>> -{% for method in interface_main.methods %}
>>>> -{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
>>>> -{% endfor %}
>>>> +protected:
>>>> +       using IPAProxy::IPAProxy;
>>>> +};
>>>>    
>>>> -private:
>>>> -       void recvMessage(const IPCMessage &data);
>>>> +class {{proxy_name}}Local : public {{proxy_name}}
>>>> +{
>>>> +public:
>>>> +       {{proxy_name}}Local(IPAModule *ipam);
>>>> +       ~{{proxy_name}}Local();
>>>>    
>>>>    {% 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)}};
>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
> 
> As this is Python code I *think* you could also write
> 
> {{proxy_funcs.func_sig(f"{proxy_name}Local", method, "", false, true)|indent(8, true)}};
> 
> if you think it's clearer (sae elsewhere in the patch).

I'll check.


Regards,
Barnabás Pőcze


> 
>>>>    {% endfor %}
>>>> +
>>>> +private:
>>>>    {% 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<SharedFD> &fds);
>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler", false)|indent(8, true)}};
>>>>    {% endfor %}
>>>>    
>>>>           /* Helper class to invoke async functions in another thread. */
>>>> @@ -79,12 +82,12 @@ private:
>>>>                   }
>>>>    {% for method in interface_main.methods %}
>>>>    {%- if method|is_async %}
>>>> -               {{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
>>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", 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)}}
>>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", method, "", false)|indent(16)}}
>>>>                   {
>>>>    {%- if method|method_return_value != "void" %}
>>>>                           return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
>>>> @@ -104,8 +107,27 @@ private:
>>>>           Thread thread_;
>>>>           ThreadProxy proxy_;
>>>>           std::unique_ptr<{{interface_name}}> ipa_;
>>>> +};
>>>>    
>>>> -       const bool isolate_;
>>>> +class {{proxy_name}}Remote : public {{proxy_name}}
>>>> +{
>>>> +public:
>>>> +       {{proxy_name}}Remote(IPAModule *ipam);
>>>> +       ~{{proxy_name}}Remote();
>>>> +
>>>> +{% for method in interface_main.methods %}
>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
>>>
>>> s/Local/Remote/ ?
>>>
>>> Other than that, looks good to me. This is exciting!
>>
>> Oops, fixed. I was wondering why it did not cause any issues... of course
>> the name of the class is not used due to the fourth argument being false.
>>
>>>> +{% endfor %}
>>>> +
>>>> +private:
>>>> +       void recvMessage(const IPCMessage &data);
>>>> +
>>>> +{% for method in interface_event.methods %}
>>>> +       void {{method.mojom_name}}Handler(
>>>> +               std::vector<uint8_t>::const_iterator data,
>>>> +               size_t dataSize,
>>>> +               const std::vector<SharedFD> &fds);
>>>> +{% endfor %}
>>>>    
>>>>           std::unique_ptr<IPCPipeUnixSocket> ipc_;
>>>>    
>

On Fri, Aug 15, 2025 at 09:10:02AM +0200, Barnabás Pőcze wrote:
> 2025. 08. 15. 0:24 keltezéssel, Laurent Pinchart írta:
> > On Thu, Aug 14, 2025 at 11:42:39AM +0200, Barnabás Pőcze wrote:
> >> 2025. 08. 14. 10:59 keltezéssel, Paul Elder írta:
> >>> Quoting Barnabás Pőcze (2025-08-05 01:52:26)
> >>>> Even though there is an abstract class to represent the interface of an IPA,
> >>>> the threaded and IPC versions are still multiplexed using the same type,
> >>>> which uses a boolean to actually dispatch to the right function.
> >>>>
> >>>> Instead of doing that, split the classes into a "local" and "remote" variant,
> >>>> and make `IPAManager` choose accordingly.
> > 
> > "Local" and "Remote" are not the clearest names. Would "Threaded" and
> > "Isolated" be better ? I'm sure there are even better option.
> 
> I'm sure there are endless opportunities. I will rename them.
> 
> >>> Wow, that is a cool upgrade!
> >>>
> >>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >>>> ---
> >>>>    include/libcamera/internal/ipa_manager.h      |   8 +-
> >>>>    .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
> >>>>    .../module_ipa_proxy.h.tmpl                   |  56 +++++--
> >>>>    3 files changed, 118 insertions(+), 95 deletions(-)
> >>>>
> >>>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> >>>> index a0d448cf9..847a69066 100644
> >>>> --- a/include/libcamera/internal/ipa_manager.h
> >>>> +++ b/include/libcamera/internal/ipa_manager.h
> >>>> @@ -42,7 +42,13 @@ public:
> >>>>                   if (!m)
> >>>>                           return nullptr;
> >>>>    
> >>>> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));
> >>>> +               auto proxy = [&]() -> std::unique_ptr<T> {
> >>>> +                       if (self->isSignatureValid(m))
> >>>> +                               return std::make_unique<typename T::Local>(m);
> >>>> +                       else
> >>>> +                               return std::make_unique<typename T::Remote>(m);
> >>>> +               } ();
> > 
> > Is this using a lambda just for the sake of it ? :-)
> > 
> > 		std::unique_ptr<T proxy = self->isSignatureValid(m)
> > 					? std::make_unique<typename T::Local>(m)
> > 					: std::make_unique<typename T::Remote>(m);
> 
> The lambda is there because the ternary doesn't work.

Ah yes, they're different types. We can also do

		std::unique_ptr<T> proxy;

	       	if (self->isSignatureValid(m))
			proxy = std::make_unique<typename T::Local>(m);
		else
			proxy = std::make_unique<typename T::Remote>(m);

I see the appeal in initializing the variable when declaring it though.

> >>>> +
> >>>>                   if (!proxy->isValid()) {
> >>>>                           LOG(IPAManager, Error) << "Failed to load proxy";
> >>>>                           return nullptr;
> >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >>>> index beb646e2d..8792b3b9e 100644
> >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >>>> @@ -45,36 +45,13 @@ namespace {{ns}} {
> >>>>    {% endfor %}
> >>>>    {%- endif %}
> >>>>    
> >>>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
> >>>> -       : IPAProxy(ipam), isolate_(isolate),
> >>>> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
> >>>> +{{proxy_name}}Local::{{proxy_name}}Local(IPAModule *ipam)
> >>>> +       : {{proxy_name}}(ipam)
> >>>>    {
> >>>>           LOG(IPAProxy, Debug)
> >>>>                   << "initializing {{module_name}} proxy: loading IPA from "
> >>>>                   << ipam->path();
> >>>
> >>> Might it be worth it to also print information about isolation? Same for the
> >>> constructor below.
> >>>
> >>>>    
> >>>> -       if (isolate_) {
> >>>> -               const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> >>>> -               if (proxyWorkerPath.empty()) {
> >>>> -                       LOG(IPAProxy, Error)
> >>>> -                               << "Failed to get proxy worker path";
> >>>> -                       return;
> >>>> -               }
> >>>> -
> >>>> -               auto 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);
> >>>> -
> >>>> -               ipc_ = std::move(ipc);
> >>>> -               valid_ = true;
> >>>> -               return;
> >>>> -       }
> >>>> -
> >>>>           if (!ipam->load())
> >>>>                   return;
> >>>>    
> >>>> @@ -89,59 +66,16 @@ namespace {{ns}} {
> >>>>           proxy_.setIPA(ipa_.get());
> >>>>    
> >>>>    {% for method in interface_event.methods %}
> >>>> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
> >>>> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Local::{{method.mojom_name}}Handler);
> >>>>    {%- endfor %}
> >>>>    
> >>>>           valid_ = true;
> >>>>    }
> >>>>    
> >>>> -{{proxy_name}}::~{{proxy_name}}()
> >>>> -{
> >>>> -       if (ipc_) {
> >>>> -               IPCMessage::Header header =
> >>>> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
> >>>> -               IPCMessage msg(header);
> >>>> -               ipc_->sendAsync(msg);
> >>>> -       }
> >>>> -}
> >>>> -
> >>>> -{% if interface_event.methods|length > 0 %}
> >>>> -void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >>>> -{
> >>>> -       size_t dataSize = data.data().size();
> >>>> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
> >>>> -
> >>>> -       switch (_cmd) {
> >>>> -{%- for method in interface_event.methods %}
> >>>> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> >>>> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
> >>>> -               break;
> >>>> -       }
> >>>> -{%- endfor %}
> >>>> -       default:
> >>>> -               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> >>>> -       }
> >>>> -}
> >>>> -{%- endif %}
> >>>> +{{proxy_name}}Local::~{{proxy_name}}Local() = default;
> >>>>    
> >>>>    {% for method in interface_main.methods %}
> >>>> -{{proxy_funcs.func_sig(proxy_name, method)}}
> >>>> -{
> >>>> -       if (isolate_)
> >>>> -               return {{method.mojom_name}}IPC(
> >>>> -{%- for param in method|method_param_names -%}
> >>>> -               {{param}}{{- ", " if not loop.last}}
> >>>> -{%- endfor -%}
> >>>> -);
> >>>> -       else
> >>>> -               return {{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")}}
> >>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method)}}
> >>>>    {
> >>>>    {%- if method.mojom_name == "stop" %}
> >>>>           {{proxy_funcs.stop_thread_body()}}
> >>>> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >>>>    );
> >>>>    {%- endif %}
> >>>>    }
> >>>> +{% endfor %}
> >>>>    
> >>>> -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
> >>>> +{% for method in interface_event.methods %}
> >>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler")}}
> >>>> +{
> >>>> +       ASSERT(state_ != ProxyStopped);
> >>>> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> >>>> +}
> >>>> +{% endfor %}
> >>>> +
> >>>> +/* ========================================================================== */
> >>>> +
> >>>> +{{proxy_name}}Remote::{{proxy_name}}Remote(IPAModule *ipam)
> >>>> +       : {{proxy_name}}(ipam),
> >>>> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
> >>>> +{
> >>>> +       LOG(IPAProxy, Debug)
> >>>> +               << "initializing {{module_name}} proxy: loading IPA from "
> >>>> +               << ipam->path();
> >>>
> >>> Here ^
> >>
> >> I'll add "locally" and "remotely" to the messages. Or should it be more concrete?
> >>
> >>>> +
> >>>> +       const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> >>>> +       if (proxyWorkerPath.empty()) {
> >>>> +               LOG(IPAProxy, Error)
> >>>> +                       << "Failed to get proxy worker path";
> >>>> +               return;
> >>>> +       }
> >>>> +
> >>>> +       auto 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}}Remote::recvMessage);
> >>>> +
> >>>> +       ipc_ = std::move(ipc);
> >>>> +       valid_ = true;
> >>>> +}
> >>>> +
> >>>> +{{proxy_name}}Remote::~{{proxy_name}}Remote()
> >>>> +{
> >>>> +       if (ipc_) {
> >>>> +               IPCMessage::Header header =
> >>>> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
> >>>> +               IPCMessage msg(header);
> >>>> +               ipc_->sendAsync(msg);
> >>>> +       }
> >>>> +}
> >>>> +
> >>>> +{% for method in interface_main.methods %}
> >>>> +{{proxy_funcs.func_sig(proxy_name + "Remote", method)}}
> >>>>    {
> >>>>    {%- if method.mojom_name == "configure" %}
> >>>>           controlSerializer_.reset();
> >>>> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >>>>    
> >>>>    {% endfor %}
> >>>>    
> >>>> -{% for method in interface_event.methods %}
> >>>> -{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
> >>>> +void {{proxy_name}}Remote::recvMessage(const IPCMessage &data)
> >>>>    {
> >>>> -       ASSERT(state_ != ProxyStopped);
> >>>> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> >>>> +       size_t dataSize = data.data().size();
> >>>> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
> >>>> +
> >>>> +       switch (_cmd) {
> >>>> +{%- for method in interface_event.methods %}
> >>>> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> >>>> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());
> >>>> +               break;
> >>>> +       }
> >>>> +{%- endfor %}
> >>>> +       default:
> >>>> +               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> >>>> +       }
> >>>>    }
> >>>>    
> >>>> -void {{proxy_name}}::{{method.mojom_name}}IPC(
> >>>> +{% for method in interface_event.methods %}
> >>>> +void {{proxy_name}}Remote::{{method.mojom_name}}Handler(
> >>>>           [[maybe_unused]] std::vector<uint8_t>::const_iterator data,
> >>>>           [[maybe_unused]] size_t dataSize,
> >>>>           [[maybe_unused]] const std::vector<SharedFD> &fds)
> >>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >>>> index a0312a7c1..0c4ef985b 100644
> >>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >>>> @@ -34,29 +34,32 @@ namespace {{ns}} {
> >>>>    {% endfor %}
> >>>>    {%- endif %}
> >>>>    
> >>>> +class {{proxy_name}}Local;
> >>>> +class {{proxy_name}}Remote;
> >>>> +
> >>>>    class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
> >>>>    {
> >>>>    public:
> >>>> -       {{proxy_name}}(IPAModule *ipam, bool isolate);
> >>>> -       ~{{proxy_name}}();
> >>>> +       using Local = {{proxy_name}}Local;
> >>>> +       using Remote = {{proxy_name}}Remote;
> >>>>    
> >>>> -{% for method in interface_main.methods %}
> >>>> -{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
> >>>> -{% endfor %}
> >>>> +protected:
> >>>> +       using IPAProxy::IPAProxy;
> >>>> +};
> >>>>    
> >>>> -private:
> >>>> -       void recvMessage(const IPCMessage &data);
> >>>> +class {{proxy_name}}Local : public {{proxy_name}}
> >>>> +{
> >>>> +public:
> >>>> +       {{proxy_name}}Local(IPAModule *ipam);
> >>>> +       ~{{proxy_name}}Local();
> >>>>    
> >>>>    {% 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)}};
> >>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
> > 
> > As this is Python code I *think* you could also write
> > 
> > {{proxy_funcs.func_sig(f"{proxy_name}Local", method, "", false, true)|indent(8, true)}};
> > 
> > if you think it's clearer (sae elsewhere in the patch).
> 
> I'll check.
> 
> >>>>    {% endfor %}
> >>>> +
> >>>> +private:
> >>>>    {% 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<SharedFD> &fds);
> >>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler", false)|indent(8, true)}};
> >>>>    {% endfor %}
> >>>>    
> >>>>           /* Helper class to invoke async functions in another thread. */
> >>>> @@ -79,12 +82,12 @@ private:
> >>>>                   }
> >>>>    {% for method in interface_main.methods %}
> >>>>    {%- if method|is_async %}
> >>>> -               {{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
> >>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", 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)}}
> >>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", method, "", false)|indent(16)}}
> >>>>                   {
> >>>>    {%- if method|method_return_value != "void" %}
> >>>>                           return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
> >>>> @@ -104,8 +107,27 @@ private:
> >>>>           Thread thread_;
> >>>>           ThreadProxy proxy_;
> >>>>           std::unique_ptr<{{interface_name}}> ipa_;
> >>>> +};
> >>>>    
> >>>> -       const bool isolate_;
> >>>> +class {{proxy_name}}Remote : public {{proxy_name}}
> >>>> +{
> >>>> +public:
> >>>> +       {{proxy_name}}Remote(IPAModule *ipam);
> >>>> +       ~{{proxy_name}}Remote();
> >>>> +
> >>>> +{% for method in interface_main.methods %}
> >>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
> >>>
> >>> s/Local/Remote/ ?
> >>>
> >>> Other than that, looks good to me. This is exciting!
> >>
> >> Oops, fixed. I was wondering why it did not cause any issues... of course
> >> the name of the class is not used due to the fourth argument being false.
> >>
> >>>> +{% endfor %}
> >>>> +
> >>>> +private:
> >>>> +       void recvMessage(const IPCMessage &data);
> >>>> +
> >>>> +{% for method in interface_event.methods %}
> >>>> +       void {{method.mojom_name}}Handler(
> >>>> +               std::vector<uint8_t>::const_iterator data,
> >>>> +               size_t dataSize,
> >>>> +               const std::vector<SharedFD> &fds);
> >>>> +{% endfor %}
> >>>>    
> >>>>           std::unique_ptr<IPCPipeUnixSocket> ipc_;
> >>>>

2025. 08. 15. 10:26 keltezéssel, Laurent Pinchart írta:
> On Fri, Aug 15, 2025 at 09:10:02AM +0200, Barnabás Pőcze wrote:
>> 2025. 08. 15. 0:24 keltezéssel, Laurent Pinchart írta:
>>> On Thu, Aug 14, 2025 at 11:42:39AM +0200, Barnabás Pőcze wrote:
>>>> 2025. 08. 14. 10:59 keltezéssel, Paul Elder írta:
>>>>> Quoting Barnabás Pőcze (2025-08-05 01:52:26)
>>>>>> Even though there is an abstract class to represent the interface of an IPA,
>>>>>> the threaded and IPC versions are still multiplexed using the same type,
>>>>>> which uses a boolean to actually dispatch to the right function.
>>>>>>
>>>>>> Instead of doing that, split the classes into a "local" and "remote" variant,
>>>>>> and make `IPAManager` choose accordingly.
>>>
>>> "Local" and "Remote" are not the clearest names. Would "Threaded" and
>>> "Isolated" be better ? I'm sure there are even better option.
>>
>> I'm sure there are endless opportunities. I will rename them.
>>
>>>>> Wow, that is a cool upgrade!
>>>>>
>>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>>>> ---
>>>>>>     include/libcamera/internal/ipa_manager.h      |   8 +-
>>>>>>     .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
>>>>>>     .../module_ipa_proxy.h.tmpl                   |  56 +++++--
>>>>>>     3 files changed, 118 insertions(+), 95 deletions(-)
>>>>>>
>>>>>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
>>>>>> index a0d448cf9..847a69066 100644
>>>>>> --- a/include/libcamera/internal/ipa_manager.h
>>>>>> +++ b/include/libcamera/internal/ipa_manager.h
>>>>>> @@ -42,7 +42,13 @@ public:
>>>>>>                    if (!m)
>>>>>>                            return nullptr;
>>>>>>     
>>>>>> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));
>>>>>> +               auto proxy = [&]() -> std::unique_ptr<T> {
>>>>>> +                       if (self->isSignatureValid(m))
>>>>>> +                               return std::make_unique<typename T::Local>(m);
>>>>>> +                       else
>>>>>> +                               return std::make_unique<typename T::Remote>(m);
>>>>>> +               } ();
>>>
>>> Is this using a lambda just for the sake of it ? :-)
>>>
>>> 		std::unique_ptr<T proxy = self->isSignatureValid(m)
>>> 					? std::make_unique<typename T::Local>(m)
>>> 					: std::make_unique<typename T::Remote>(m);
>>
>> The lambda is there because the ternary doesn't work.
> 
> Ah yes, they're different types. We can also do
> 
> 		std::unique_ptr<T> proxy;
> 
> 	       	if (self->isSignatureValid(m))
> 			proxy = std::make_unique<typename T::Local>(m);
> 		else
> 			proxy = std::make_unique<typename T::Remote>(m);
> 
> I see the appeal in initializing the variable when declaring it though.

Yes, I prefer the "immediately invoked function expression" approach. In any case
I don't think it matters much, so which one should it be?


> 
>>>>>> +
>>>>>>                    if (!proxy->isValid()) {
>>>>>>                            LOG(IPAManager, Error) << "Failed to load proxy";
>>>>>>                            return nullptr;
>>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>>>> index beb646e2d..8792b3b9e 100644
>>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>>>> @@ -45,36 +45,13 @@ namespace {{ns}} {
>>>>>>     {% endfor %}
>>>>>>     {%- endif %}
>>>>>>     
>>>>>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
>>>>>> -       : IPAProxy(ipam), isolate_(isolate),
>>>>>> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
>>>>>> +{{proxy_name}}Local::{{proxy_name}}Local(IPAModule *ipam)
>>>>>> +       : {{proxy_name}}(ipam)
>>>>>>     {
>>>>>>            LOG(IPAProxy, Debug)
>>>>>>                    << "initializing {{module_name}} proxy: loading IPA from "
>>>>>>                    << ipam->path();
>>>>>
>>>>> Might it be worth it to also print information about isolation? Same for the
>>>>> constructor below.
>>>>>
>>>>>>     
>>>>>> -       if (isolate_) {
>>>>>> -               const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
>>>>>> -               if (proxyWorkerPath.empty()) {
>>>>>> -                       LOG(IPAProxy, Error)
>>>>>> -                               << "Failed to get proxy worker path";
>>>>>> -                       return;
>>>>>> -               }
>>>>>> -
>>>>>> -               auto 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);
>>>>>> -
>>>>>> -               ipc_ = std::move(ipc);
>>>>>> -               valid_ = true;
>>>>>> -               return;
>>>>>> -       }
>>>>>> -
>>>>>>            if (!ipam->load())
>>>>>>                    return;
>>>>>>     
>>>>>> @@ -89,59 +66,16 @@ namespace {{ns}} {
>>>>>>            proxy_.setIPA(ipa_.get());
>>>>>>     
>>>>>>     {% for method in interface_event.methods %}
>>>>>> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
>>>>>> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Local::{{method.mojom_name}}Handler);
>>>>>>     {%- endfor %}
>>>>>>     
>>>>>>            valid_ = true;
>>>>>>     }
>>>>>>     
>>>>>> -{{proxy_name}}::~{{proxy_name}}()
>>>>>> -{
>>>>>> -       if (ipc_) {
>>>>>> -               IPCMessage::Header header =
>>>>>> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
>>>>>> -               IPCMessage msg(header);
>>>>>> -               ipc_->sendAsync(msg);
>>>>>> -       }
>>>>>> -}
>>>>>> -
>>>>>> -{% if interface_event.methods|length > 0 %}
>>>>>> -void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>>>> -{
>>>>>> -       size_t dataSize = data.data().size();
>>>>>> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>>>>>> -
>>>>>> -       switch (_cmd) {
>>>>>> -{%- for method in interface_event.methods %}
>>>>>> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>>>>>> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
>>>>>> -               break;
>>>>>> -       }
>>>>>> -{%- endfor %}
>>>>>> -       default:
>>>>>> -               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
>>>>>> -       }
>>>>>> -}
>>>>>> -{%- endif %}
>>>>>> +{{proxy_name}}Local::~{{proxy_name}}Local() = default;
>>>>>>     
>>>>>>     {% for method in interface_main.methods %}
>>>>>> -{{proxy_funcs.func_sig(proxy_name, method)}}
>>>>>> -{
>>>>>> -       if (isolate_)
>>>>>> -               return {{method.mojom_name}}IPC(
>>>>>> -{%- for param in method|method_param_names -%}
>>>>>> -               {{param}}{{- ", " if not loop.last}}
>>>>>> -{%- endfor -%}
>>>>>> -);
>>>>>> -       else
>>>>>> -               return {{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")}}
>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method)}}
>>>>>>     {
>>>>>>     {%- if method.mojom_name == "stop" %}
>>>>>>            {{proxy_funcs.stop_thread_body()}}
>>>>>> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>>>>     );
>>>>>>     {%- endif %}
>>>>>>     }
>>>>>> +{% endfor %}
>>>>>>     
>>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
>>>>>> +{% for method in interface_event.methods %}
>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler")}}
>>>>>> +{
>>>>>> +       ASSERT(state_ != ProxyStopped);
>>>>>> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>>>>>> +}
>>>>>> +{% endfor %}
>>>>>> +
>>>>>> +/* ========================================================================== */
>>>>>> +
>>>>>> +{{proxy_name}}Remote::{{proxy_name}}Remote(IPAModule *ipam)
>>>>>> +       : {{proxy_name}}(ipam),
>>>>>> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
>>>>>> +{
>>>>>> +       LOG(IPAProxy, Debug)
>>>>>> +               << "initializing {{module_name}} proxy: loading IPA from "
>>>>>> +               << ipam->path();
>>>>>
>>>>> Here ^
>>>>
>>>> I'll add "locally" and "remotely" to the messages. Or should it be more concrete?
>>>>
>>>>>> +
>>>>>> +       const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
>>>>>> +       if (proxyWorkerPath.empty()) {
>>>>>> +               LOG(IPAProxy, Error)
>>>>>> +                       << "Failed to get proxy worker path";
>>>>>> +               return;
>>>>>> +       }
>>>>>> +
>>>>>> +       auto 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}}Remote::recvMessage);
>>>>>> +
>>>>>> +       ipc_ = std::move(ipc);
>>>>>> +       valid_ = true;
>>>>>> +}
>>>>>> +
>>>>>> +{{proxy_name}}Remote::~{{proxy_name}}Remote()
>>>>>> +{
>>>>>> +       if (ipc_) {
>>>>>> +               IPCMessage::Header header =
>>>>>> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
>>>>>> +               IPCMessage msg(header);
>>>>>> +               ipc_->sendAsync(msg);
>>>>>> +       }
>>>>>> +}
>>>>>> +
>>>>>> +{% for method in interface_main.methods %}
>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Remote", method)}}
>>>>>>     {
>>>>>>     {%- if method.mojom_name == "configure" %}
>>>>>>            controlSerializer_.reset();
>>>>>> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>>>>     
>>>>>>     {% endfor %}
>>>>>>     
>>>>>> -{% for method in interface_event.methods %}
>>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
>>>>>> +void {{proxy_name}}Remote::recvMessage(const IPCMessage &data)
>>>>>>     {
>>>>>> -       ASSERT(state_ != ProxyStopped);
>>>>>> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>>>>>> +       size_t dataSize = data.data().size();
>>>>>> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>>>>>> +
>>>>>> +       switch (_cmd) {
>>>>>> +{%- for method in interface_event.methods %}
>>>>>> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>>>>>> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());
>>>>>> +               break;
>>>>>> +       }
>>>>>> +{%- endfor %}
>>>>>> +       default:
>>>>>> +               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
>>>>>> +       }
>>>>>>     }
>>>>>>     
>>>>>> -void {{proxy_name}}::{{method.mojom_name}}IPC(
>>>>>> +{% for method in interface_event.methods %}
>>>>>> +void {{proxy_name}}Remote::{{method.mojom_name}}Handler(
>>>>>>            [[maybe_unused]] std::vector<uint8_t>::const_iterator data,
>>>>>>            [[maybe_unused]] size_t dataSize,
>>>>>>            [[maybe_unused]] const std::vector<SharedFD> &fds)
>>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>>>> index a0312a7c1..0c4ef985b 100644
>>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>>>> @@ -34,29 +34,32 @@ namespace {{ns}} {
>>>>>>     {% endfor %}
>>>>>>     {%- endif %}
>>>>>>     
>>>>>> +class {{proxy_name}}Local;
>>>>>> +class {{proxy_name}}Remote;
>>>>>> +
>>>>>>     class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
>>>>>>     {
>>>>>>     public:
>>>>>> -       {{proxy_name}}(IPAModule *ipam, bool isolate);
>>>>>> -       ~{{proxy_name}}();
>>>>>> +       using Local = {{proxy_name}}Local;
>>>>>> +       using Remote = {{proxy_name}}Remote;
>>>>>>     
>>>>>> -{% for method in interface_main.methods %}
>>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
>>>>>> -{% endfor %}
>>>>>> +protected:
>>>>>> +       using IPAProxy::IPAProxy;
>>>>>> +};
>>>>>>     
>>>>>> -private:
>>>>>> -       void recvMessage(const IPCMessage &data);
>>>>>> +class {{proxy_name}}Local : public {{proxy_name}}
>>>>>> +{
>>>>>> +public:
>>>>>> +       {{proxy_name}}Local(IPAModule *ipam);
>>>>>> +       ~{{proxy_name}}Local();
>>>>>>     
>>>>>>     {% 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)}};
>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
>>>
>>> As this is Python code I *think* you could also write
>>>
>>> {{proxy_funcs.func_sig(f"{proxy_name}Local", method, "", false, true)|indent(8, true)}};
>>>
>>> if you think it's clearer (sae elsewhere in the patch).
>>
>> I'll check.

Apparently f-strings do not work in jinja, so I kept the "+".


>>
>>>>>>     {% endfor %}
>>>>>> +
>>>>>> +private:
>>>>>>     {% 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<SharedFD> &fds);
>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler", false)|indent(8, true)}};
>>>>>>     {% endfor %}
>>>>>>     
>>>>>>            /* Helper class to invoke async functions in another thread. */
>>>>>> @@ -79,12 +82,12 @@ private:
>>>>>>                    }
>>>>>>     {% for method in interface_main.methods %}
>>>>>>     {%- if method|is_async %}
>>>>>> -               {{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
>>>>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", 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)}}
>>>>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", method, "", false)|indent(16)}}
>>>>>>                    {
>>>>>>     {%- if method|method_return_value != "void" %}
>>>>>>                            return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
>>>>>> @@ -104,8 +107,27 @@ private:
>>>>>>            Thread thread_;
>>>>>>            ThreadProxy proxy_;
>>>>>>            std::unique_ptr<{{interface_name}}> ipa_;
>>>>>> +};
>>>>>>     
>>>>>> -       const bool isolate_;
>>>>>> +class {{proxy_name}}Remote : public {{proxy_name}}
>>>>>> +{
>>>>>> +public:
>>>>>> +       {{proxy_name}}Remote(IPAModule *ipam);
>>>>>> +       ~{{proxy_name}}Remote();
>>>>>> +
>>>>>> +{% for method in interface_main.methods %}
>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
>>>>>
>>>>> s/Local/Remote/ ?
>>>>>
>>>>> Other than that, looks good to me. This is exciting!
>>>>
>>>> Oops, fixed. I was wondering why it did not cause any issues... of course
>>>> the name of the class is not used due to the fourth argument being false.
>>>>
>>>>>> +{% endfor %}
>>>>>> +
>>>>>> +private:
>>>>>> +       void recvMessage(const IPCMessage &data);
>>>>>> +
>>>>>> +{% for method in interface_event.methods %}
>>>>>> +       void {{method.mojom_name}}Handler(
>>>>>> +               std::vector<uint8_t>::const_iterator data,
>>>>>> +               size_t dataSize,
>>>>>> +               const std::vector<SharedFD> &fds);
>>>>>> +{% endfor %}
>>>>>>     
>>>>>>            std::unique_ptr<IPCPipeUnixSocket> ipc_;
>>>>>>     
>

On Fri, Aug 15, 2025 at 10:40:49AM +0200, Barnabás Pőcze wrote:
> 2025. 08. 15. 10:26 keltezéssel, Laurent Pinchart írta:
> > On Fri, Aug 15, 2025 at 09:10:02AM +0200, Barnabás Pőcze wrote:
> >> 2025. 08. 15. 0:24 keltezéssel, Laurent Pinchart írta:
> >>> On Thu, Aug 14, 2025 at 11:42:39AM +0200, Barnabás Pőcze wrote:
> >>>> 2025. 08. 14. 10:59 keltezéssel, Paul Elder írta:
> >>>>> Quoting Barnabás Pőcze (2025-08-05 01:52:26)
> >>>>>> Even though there is an abstract class to represent the interface of an IPA,
> >>>>>> the threaded and IPC versions are still multiplexed using the same type,
> >>>>>> which uses a boolean to actually dispatch to the right function.
> >>>>>>
> >>>>>> Instead of doing that, split the classes into a "local" and "remote" variant,
> >>>>>> and make `IPAManager` choose accordingly.
> >>>
> >>> "Local" and "Remote" are not the clearest names. Would "Threaded" and
> >>> "Isolated" be better ? I'm sure there are even better option.
> >>
> >> I'm sure there are endless opportunities. I will rename them.
> >>
> >>>>> Wow, that is a cool upgrade!
> >>>>>
> >>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> >>>>>> ---
> >>>>>>     include/libcamera/internal/ipa_manager.h      |   8 +-
> >>>>>>     .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
> >>>>>>     .../module_ipa_proxy.h.tmpl                   |  56 +++++--
> >>>>>>     3 files changed, 118 insertions(+), 95 deletions(-)
> >>>>>>
> >>>>>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> >>>>>> index a0d448cf9..847a69066 100644
> >>>>>> --- a/include/libcamera/internal/ipa_manager.h
> >>>>>> +++ b/include/libcamera/internal/ipa_manager.h
> >>>>>> @@ -42,7 +42,13 @@ public:
> >>>>>>                    if (!m)
> >>>>>>                            return nullptr;
> >>>>>>     
> >>>>>> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));
> >>>>>> +               auto proxy = [&]() -> std::unique_ptr<T> {
> >>>>>> +                       if (self->isSignatureValid(m))
> >>>>>> +                               return std::make_unique<typename T::Local>(m);
> >>>>>> +                       else
> >>>>>> +                               return std::make_unique<typename T::Remote>(m);
> >>>>>> +               } ();
> >>>
> >>> Is this using a lambda just for the sake of it ? :-)
> >>>
> >>> 		std::unique_ptr<T proxy = self->isSignatureValid(m)
> >>> 					? std::make_unique<typename T::Local>(m)
> >>> 					: std::make_unique<typename T::Remote>(m);
> >>
> >> The lambda is there because the ternary doesn't work.
> > 
> > Ah yes, they're different types. We can also do
> > 
> > 		std::unique_ptr<T> proxy;
> > 
> > 	       	if (self->isSignatureValid(m))
> > 			proxy = std::make_unique<typename T::Local>(m);
> > 		else
> > 			proxy = std::make_unique<typename T::Remote>(m);
> > 
> > I see the appeal in initializing the variable when declaring it though.
> 
> Yes, I prefer the "immediately invoked function expression" approach. In any case
> I don't think it matters much, so which one should it be?

It took me some time when reading the patch to notice the "()" after the
lambda. That's why I commented on this. It may be a matter of getting
used to it though. You can decide.

> >>>>>> +
> >>>>>>                    if (!proxy->isValid()) {
> >>>>>>                            LOG(IPAManager, Error) << "Failed to load proxy";
> >>>>>>                            return nullptr;
> >>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >>>>>> index beb646e2d..8792b3b9e 100644
> >>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >>>>>> @@ -45,36 +45,13 @@ namespace {{ns}} {
> >>>>>>     {% endfor %}
> >>>>>>     {%- endif %}
> >>>>>>     
> >>>>>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
> >>>>>> -       : IPAProxy(ipam), isolate_(isolate),
> >>>>>> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
> >>>>>> +{{proxy_name}}Local::{{proxy_name}}Local(IPAModule *ipam)
> >>>>>> +       : {{proxy_name}}(ipam)
> >>>>>>     {
> >>>>>>            LOG(IPAProxy, Debug)
> >>>>>>                    << "initializing {{module_name}} proxy: loading IPA from "
> >>>>>>                    << ipam->path();
> >>>>>
> >>>>> Might it be worth it to also print information about isolation? Same for the
> >>>>> constructor below.
> >>>>>
> >>>>>>     
> >>>>>> -       if (isolate_) {
> >>>>>> -               const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> >>>>>> -               if (proxyWorkerPath.empty()) {
> >>>>>> -                       LOG(IPAProxy, Error)
> >>>>>> -                               << "Failed to get proxy worker path";
> >>>>>> -                       return;
> >>>>>> -               }
> >>>>>> -
> >>>>>> -               auto 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);
> >>>>>> -
> >>>>>> -               ipc_ = std::move(ipc);
> >>>>>> -               valid_ = true;
> >>>>>> -               return;
> >>>>>> -       }
> >>>>>> -
> >>>>>>            if (!ipam->load())
> >>>>>>                    return;
> >>>>>>     
> >>>>>> @@ -89,59 +66,16 @@ namespace {{ns}} {
> >>>>>>            proxy_.setIPA(ipa_.get());
> >>>>>>     
> >>>>>>     {% for method in interface_event.methods %}
> >>>>>> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
> >>>>>> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Local::{{method.mojom_name}}Handler);
> >>>>>>     {%- endfor %}
> >>>>>>     
> >>>>>>            valid_ = true;
> >>>>>>     }
> >>>>>>     
> >>>>>> -{{proxy_name}}::~{{proxy_name}}()
> >>>>>> -{
> >>>>>> -       if (ipc_) {
> >>>>>> -               IPCMessage::Header header =
> >>>>>> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
> >>>>>> -               IPCMessage msg(header);
> >>>>>> -               ipc_->sendAsync(msg);
> >>>>>> -       }
> >>>>>> -}
> >>>>>> -
> >>>>>> -{% if interface_event.methods|length > 0 %}
> >>>>>> -void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >>>>>> -{
> >>>>>> -       size_t dataSize = data.data().size();
> >>>>>> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
> >>>>>> -
> >>>>>> -       switch (_cmd) {
> >>>>>> -{%- for method in interface_event.methods %}
> >>>>>> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> >>>>>> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
> >>>>>> -               break;
> >>>>>> -       }
> >>>>>> -{%- endfor %}
> >>>>>> -       default:
> >>>>>> -               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> >>>>>> -       }
> >>>>>> -}
> >>>>>> -{%- endif %}
> >>>>>> +{{proxy_name}}Local::~{{proxy_name}}Local() = default;
> >>>>>>     
> >>>>>>     {% for method in interface_main.methods %}
> >>>>>> -{{proxy_funcs.func_sig(proxy_name, method)}}
> >>>>>> -{
> >>>>>> -       if (isolate_)
> >>>>>> -               return {{method.mojom_name}}IPC(
> >>>>>> -{%- for param in method|method_param_names -%}
> >>>>>> -               {{param}}{{- ", " if not loop.last}}
> >>>>>> -{%- endfor -%}
> >>>>>> -);
> >>>>>> -       else
> >>>>>> -               return {{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")}}
> >>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method)}}
> >>>>>>     {
> >>>>>>     {%- if method.mojom_name == "stop" %}
> >>>>>>            {{proxy_funcs.stop_thread_body()}}
> >>>>>> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >>>>>>     );
> >>>>>>     {%- endif %}
> >>>>>>     }
> >>>>>> +{% endfor %}
> >>>>>>     
> >>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
> >>>>>> +{% for method in interface_event.methods %}
> >>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler")}}
> >>>>>> +{
> >>>>>> +       ASSERT(state_ != ProxyStopped);
> >>>>>> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> >>>>>> +}
> >>>>>> +{% endfor %}
> >>>>>> +
> >>>>>> +/* ========================================================================== */
> >>>>>> +
> >>>>>> +{{proxy_name}}Remote::{{proxy_name}}Remote(IPAModule *ipam)
> >>>>>> +       : {{proxy_name}}(ipam),
> >>>>>> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
> >>>>>> +{
> >>>>>> +       LOG(IPAProxy, Debug)
> >>>>>> +               << "initializing {{module_name}} proxy: loading IPA from "
> >>>>>> +               << ipam->path();
> >>>>>
> >>>>> Here ^
> >>>>
> >>>> I'll add "locally" and "remotely" to the messages. Or should it be more concrete?
> >>>>
> >>>>>> +
> >>>>>> +       const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
> >>>>>> +       if (proxyWorkerPath.empty()) {
> >>>>>> +               LOG(IPAProxy, Error)
> >>>>>> +                       << "Failed to get proxy worker path";
> >>>>>> +               return;
> >>>>>> +       }
> >>>>>> +
> >>>>>> +       auto 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}}Remote::recvMessage);
> >>>>>> +
> >>>>>> +       ipc_ = std::move(ipc);
> >>>>>> +       valid_ = true;
> >>>>>> +}
> >>>>>> +
> >>>>>> +{{proxy_name}}Remote::~{{proxy_name}}Remote()
> >>>>>> +{
> >>>>>> +       if (ipc_) {
> >>>>>> +               IPCMessage::Header header =
> >>>>>> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
> >>>>>> +               IPCMessage msg(header);
> >>>>>> +               ipc_->sendAsync(msg);
> >>>>>> +       }
> >>>>>> +}
> >>>>>> +
> >>>>>> +{% for method in interface_main.methods %}
> >>>>>> +{{proxy_funcs.func_sig(proxy_name + "Remote", method)}}
> >>>>>>     {
> >>>>>>     {%- if method.mojom_name == "configure" %}
> >>>>>>            controlSerializer_.reset();
> >>>>>> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
> >>>>>>     
> >>>>>>     {% endfor %}
> >>>>>>     
> >>>>>> -{% for method in interface_event.methods %}
> >>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
> >>>>>> +void {{proxy_name}}Remote::recvMessage(const IPCMessage &data)
> >>>>>>     {
> >>>>>> -       ASSERT(state_ != ProxyStopped);
> >>>>>> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
> >>>>>> +       size_t dataSize = data.data().size();
> >>>>>> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
> >>>>>> +
> >>>>>> +       switch (_cmd) {
> >>>>>> +{%- for method in interface_event.methods %}
> >>>>>> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
> >>>>>> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());
> >>>>>> +               break;
> >>>>>> +       }
> >>>>>> +{%- endfor %}
> >>>>>> +       default:
> >>>>>> +               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
> >>>>>> +       }
> >>>>>>     }
> >>>>>>     
> >>>>>> -void {{proxy_name}}::{{method.mojom_name}}IPC(
> >>>>>> +{% for method in interface_event.methods %}
> >>>>>> +void {{proxy_name}}Remote::{{method.mojom_name}}Handler(
> >>>>>>            [[maybe_unused]] std::vector<uint8_t>::const_iterator data,
> >>>>>>            [[maybe_unused]] size_t dataSize,
> >>>>>>            [[maybe_unused]] const std::vector<SharedFD> &fds)
> >>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >>>>>> index a0312a7c1..0c4ef985b 100644
> >>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >>>>>> @@ -34,29 +34,32 @@ namespace {{ns}} {
> >>>>>>     {% endfor %}
> >>>>>>     {%- endif %}
> >>>>>>     
> >>>>>> +class {{proxy_name}}Local;
> >>>>>> +class {{proxy_name}}Remote;
> >>>>>> +
> >>>>>>     class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
> >>>>>>     {
> >>>>>>     public:
> >>>>>> -       {{proxy_name}}(IPAModule *ipam, bool isolate);
> >>>>>> -       ~{{proxy_name}}();
> >>>>>> +       using Local = {{proxy_name}}Local;
> >>>>>> +       using Remote = {{proxy_name}}Remote;
> >>>>>>     
> >>>>>> -{% for method in interface_main.methods %}
> >>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
> >>>>>> -{% endfor %}
> >>>>>> +protected:
> >>>>>> +       using IPAProxy::IPAProxy;
> >>>>>> +};
> >>>>>>     
> >>>>>> -private:
> >>>>>> -       void recvMessage(const IPCMessage &data);
> >>>>>> +class {{proxy_name}}Local : public {{proxy_name}}
> >>>>>> +{
> >>>>>> +public:
> >>>>>> +       {{proxy_name}}Local(IPAModule *ipam);
> >>>>>> +       ~{{proxy_name}}Local();
> >>>>>>     
> >>>>>>     {% 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)}};
> >>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
> >>>
> >>> As this is Python code I *think* you could also write
> >>>
> >>> {{proxy_funcs.func_sig(f"{proxy_name}Local", method, "", false, true)|indent(8, true)}};
> >>>
> >>> if you think it's clearer (sae elsewhere in the patch).
> >>
> >> I'll check.
> 
> Apparently f-strings do not work in jinja, so I kept the "+".

Good to know, thanks for trying it out.

> >>>>>>     {% endfor %}
> >>>>>> +
> >>>>>> +private:
> >>>>>>     {% 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<SharedFD> &fds);
> >>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler", false)|indent(8, true)}};
> >>>>>>     {% endfor %}
> >>>>>>     
> >>>>>>            /* Helper class to invoke async functions in another thread. */
> >>>>>> @@ -79,12 +82,12 @@ private:
> >>>>>>                    }
> >>>>>>     {% for method in interface_main.methods %}
> >>>>>>     {%- if method|is_async %}
> >>>>>> -               {{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
> >>>>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", 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)}}
> >>>>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", method, "", false)|indent(16)}}
> >>>>>>                    {
> >>>>>>     {%- if method|method_return_value != "void" %}
> >>>>>>                            return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
> >>>>>> @@ -104,8 +107,27 @@ private:
> >>>>>>            Thread thread_;
> >>>>>>            ThreadProxy proxy_;
> >>>>>>            std::unique_ptr<{{interface_name}}> ipa_;
> >>>>>> +};
> >>>>>>     
> >>>>>> -       const bool isolate_;
> >>>>>> +class {{proxy_name}}Remote : public {{proxy_name}}
> >>>>>> +{
> >>>>>> +public:
> >>>>>> +       {{proxy_name}}Remote(IPAModule *ipam);
> >>>>>> +       ~{{proxy_name}}Remote();
> >>>>>> +
> >>>>>> +{% for method in interface_main.methods %}
> >>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
> >>>>>
> >>>>> s/Local/Remote/ ?
> >>>>>
> >>>>> Other than that, looks good to me. This is exciting!
> >>>>
> >>>> Oops, fixed. I was wondering why it did not cause any issues... of course
> >>>> the name of the class is not used due to the fourth argument being false.
> >>>>
> >>>>>> +{% endfor %}
> >>>>>> +
> >>>>>> +private:
> >>>>>> +       void recvMessage(const IPCMessage &data);
> >>>>>> +
> >>>>>> +{% for method in interface_event.methods %}
> >>>>>> +       void {{method.mojom_name}}Handler(
> >>>>>> +               std::vector<uint8_t>::const_iterator data,
> >>>>>> +               size_t dataSize,
> >>>>>> +               const std::vector<SharedFD> &fds);
> >>>>>> +{% endfor %}
> >>>>>>     
> >>>>>>            std::unique_ptr<IPCPipeUnixSocket> ipc_;
> >>>>>>

2025. 08. 15. 10:47 keltezéssel, Laurent Pinchart írta:
> On Fri, Aug 15, 2025 at 10:40:49AM +0200, Barnabás Pőcze wrote:
>> 2025. 08. 15. 10:26 keltezéssel, Laurent Pinchart írta:
>>> On Fri, Aug 15, 2025 at 09:10:02AM +0200, Barnabás Pőcze wrote:
>>>> 2025. 08. 15. 0:24 keltezéssel, Laurent Pinchart írta:
>>>>> On Thu, Aug 14, 2025 at 11:42:39AM +0200, Barnabás Pőcze wrote:
>>>>>> 2025. 08. 14. 10:59 keltezéssel, Paul Elder írta:
>>>>>>> Quoting Barnabás Pőcze (2025-08-05 01:52:26)
>>>>>>>> Even though there is an abstract class to represent the interface of an IPA,
>>>>>>>> the threaded and IPC versions are still multiplexed using the same type,
>>>>>>>> which uses a boolean to actually dispatch to the right function.
>>>>>>>>
>>>>>>>> Instead of doing that, split the classes into a "local" and "remote" variant,
>>>>>>>> and make `IPAManager` choose accordingly.
>>>>>
>>>>> "Local" and "Remote" are not the clearest names. Would "Threaded" and
>>>>> "Isolated" be better ? I'm sure there are even better option.
>>>>
>>>> I'm sure there are endless opportunities. I will rename them.
>>>>
>>>>>>> Wow, that is a cool upgrade!
>>>>>>>
>>>>>>>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>>>>>>>> ---
>>>>>>>>      include/libcamera/internal/ipa_manager.h      |   8 +-
>>>>>>>>      .../module_ipa_proxy.cpp.tmpl                 | 149 +++++++++---------
>>>>>>>>      .../module_ipa_proxy.h.tmpl                   |  56 +++++--
>>>>>>>>      3 files changed, 118 insertions(+), 95 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
>>>>>>>> index a0d448cf9..847a69066 100644
>>>>>>>> --- a/include/libcamera/internal/ipa_manager.h
>>>>>>>> +++ b/include/libcamera/internal/ipa_manager.h
>>>>>>>> @@ -42,7 +42,13 @@ public:
>>>>>>>>                     if (!m)
>>>>>>>>                             return nullptr;
>>>>>>>>      
>>>>>>>> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));
>>>>>>>> +               auto proxy = [&]() -> std::unique_ptr<T> {
>>>>>>>> +                       if (self->isSignatureValid(m))
>>>>>>>> +                               return std::make_unique<typename T::Local>(m);
>>>>>>>> +                       else
>>>>>>>> +                               return std::make_unique<typename T::Remote>(m);
>>>>>>>> +               } ();
>>>>>
>>>>> Is this using a lambda just for the sake of it ? :-)
>>>>>
>>>>> 		std::unique_ptr<T proxy = self->isSignatureValid(m)
>>>>> 					? std::make_unique<typename T::Local>(m)
>>>>> 					: std::make_unique<typename T::Remote>(m);
>>>>
>>>> The lambda is there because the ternary doesn't work.
>>>
>>> Ah yes, they're different types. We can also do
>>>
>>> 		std::unique_ptr<T> proxy;
>>>
>>> 	       	if (self->isSignatureValid(m))
>>> 			proxy = std::make_unique<typename T::Local>(m);
>>> 		else
>>> 			proxy = std::make_unique<typename T::Remote>(m);
>>>
>>> I see the appeal in initializing the variable when declaring it though.
>>
>> Yes, I prefer the "immediately invoked function expression" approach. In any case
>> I don't think it matters much, so which one should it be?
> 
> It took me some time when reading the patch to notice the "()" after the
> lambda. That's why I commented on this. It may be a matter of getting
> used to it though. You can decide.

Okay, then I'll keep the current version and we'll see what others think.


> 
>>>>>>>> +
>>>>>>>>                     if (!proxy->isValid()) {
>>>>>>>>                             LOG(IPAManager, Error) << "Failed to load proxy";
>>>>>>>>                             return nullptr;
>>>>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>>>>>> index beb646e2d..8792b3b9e 100644
>>>>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>>>>>>>> @@ -45,36 +45,13 @@ namespace {{ns}} {
>>>>>>>>      {% endfor %}
>>>>>>>>      {%- endif %}
>>>>>>>>      
>>>>>>>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)
>>>>>>>> -       : IPAProxy(ipam), isolate_(isolate),
>>>>>>>> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
>>>>>>>> +{{proxy_name}}Local::{{proxy_name}}Local(IPAModule *ipam)
>>>>>>>> +       : {{proxy_name}}(ipam)
>>>>>>>>      {
>>>>>>>>             LOG(IPAProxy, Debug)
>>>>>>>>                     << "initializing {{module_name}} proxy: loading IPA from "
>>>>>>>>                     << ipam->path();
>>>>>>>
>>>>>>> Might it be worth it to also print information about isolation? Same for the
>>>>>>> constructor below.
>>>>>>>
>>>>>>>>      
>>>>>>>> -       if (isolate_) {
>>>>>>>> -               const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
>>>>>>>> -               if (proxyWorkerPath.empty()) {
>>>>>>>> -                       LOG(IPAProxy, Error)
>>>>>>>> -                               << "Failed to get proxy worker path";
>>>>>>>> -                       return;
>>>>>>>> -               }
>>>>>>>> -
>>>>>>>> -               auto 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);
>>>>>>>> -
>>>>>>>> -               ipc_ = std::move(ipc);
>>>>>>>> -               valid_ = true;
>>>>>>>> -               return;
>>>>>>>> -       }
>>>>>>>> -
>>>>>>>>             if (!ipam->load())
>>>>>>>>                     return;
>>>>>>>>      
>>>>>>>> @@ -89,59 +66,16 @@ namespace {{ns}} {
>>>>>>>>             proxy_.setIPA(ipa_.get());
>>>>>>>>      
>>>>>>>>      {% for method in interface_event.methods %}
>>>>>>>> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);
>>>>>>>> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Local::{{method.mojom_name}}Handler);
>>>>>>>>      {%- endfor %}
>>>>>>>>      
>>>>>>>>             valid_ = true;
>>>>>>>>      }
>>>>>>>>      
>>>>>>>> -{{proxy_name}}::~{{proxy_name}}()
>>>>>>>> -{
>>>>>>>> -       if (ipc_) {
>>>>>>>> -               IPCMessage::Header header =
>>>>>>>> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
>>>>>>>> -               IPCMessage msg(header);
>>>>>>>> -               ipc_->sendAsync(msg);
>>>>>>>> -       }
>>>>>>>> -}
>>>>>>>> -
>>>>>>>> -{% if interface_event.methods|length > 0 %}
>>>>>>>> -void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>>>>>> -{
>>>>>>>> -       size_t dataSize = data.data().size();
>>>>>>>> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>>>>>>>> -
>>>>>>>> -       switch (_cmd) {
>>>>>>>> -{%- for method in interface_event.methods %}
>>>>>>>> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>>>>>>>> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());
>>>>>>>> -               break;
>>>>>>>> -       }
>>>>>>>> -{%- endfor %}
>>>>>>>> -       default:
>>>>>>>> -               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
>>>>>>>> -       }
>>>>>>>> -}
>>>>>>>> -{%- endif %}
>>>>>>>> +{{proxy_name}}Local::~{{proxy_name}}Local() = default;
>>>>>>>>      
>>>>>>>>      {% for method in interface_main.methods %}
>>>>>>>> -{{proxy_funcs.func_sig(proxy_name, method)}}
>>>>>>>> -{
>>>>>>>> -       if (isolate_)
>>>>>>>> -               return {{method.mojom_name}}IPC(
>>>>>>>> -{%- for param in method|method_param_names -%}
>>>>>>>> -               {{param}}{{- ", " if not loop.last}}
>>>>>>>> -{%- endfor -%}
>>>>>>>> -);
>>>>>>>> -       else
>>>>>>>> -               return {{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")}}
>>>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method)}}
>>>>>>>>      {
>>>>>>>>      {%- if method.mojom_name == "stop" %}
>>>>>>>>             {{proxy_funcs.stop_thread_body()}}
>>>>>>>> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>>>>>>      );
>>>>>>>>      {%- endif %}
>>>>>>>>      }
>>>>>>>> +{% endfor %}
>>>>>>>>      
>>>>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
>>>>>>>> +{% for method in interface_event.methods %}
>>>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler")}}
>>>>>>>> +{
>>>>>>>> +       ASSERT(state_ != ProxyStopped);
>>>>>>>> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>>>>>>>> +}
>>>>>>>> +{% endfor %}
>>>>>>>> +
>>>>>>>> +/* ========================================================================== */
>>>>>>>> +
>>>>>>>> +{{proxy_name}}Remote::{{proxy_name}}Remote(IPAModule *ipam)
>>>>>>>> +       : {{proxy_name}}(ipam),
>>>>>>>> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)
>>>>>>>> +{
>>>>>>>> +       LOG(IPAProxy, Debug)
>>>>>>>> +               << "initializing {{module_name}} proxy: loading IPA from "
>>>>>>>> +               << ipam->path();
>>>>>>>
>>>>>>> Here ^
>>>>>>
>>>>>> I'll add "locally" and "remotely" to the messages. Or should it be more concrete?
>>>>>>
>>>>>>>> +
>>>>>>>> +       const std::string proxyWorkerPath = resolvePath("{{module_name}}_ipa_proxy");
>>>>>>>> +       if (proxyWorkerPath.empty()) {
>>>>>>>> +               LOG(IPAProxy, Error)
>>>>>>>> +                       << "Failed to get proxy worker path";
>>>>>>>> +               return;
>>>>>>>> +       }
>>>>>>>> +
>>>>>>>> +       auto 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}}Remote::recvMessage);
>>>>>>>> +
>>>>>>>> +       ipc_ = std::move(ipc);
>>>>>>>> +       valid_ = true;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +{{proxy_name}}Remote::~{{proxy_name}}Remote()
>>>>>>>> +{
>>>>>>>> +       if (ipc_) {
>>>>>>>> +               IPCMessage::Header header =
>>>>>>>> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };
>>>>>>>> +               IPCMessage msg(header);
>>>>>>>> +               ipc_->sendAsync(msg);
>>>>>>>> +       }
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +{% for method in interface_main.methods %}
>>>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Remote", method)}}
>>>>>>>>      {
>>>>>>>>      {%- if method.mojom_name == "configure" %}
>>>>>>>>             controlSerializer_.reset();
>>>>>>>> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>>>>>>>      
>>>>>>>>      {% endfor %}
>>>>>>>>      
>>>>>>>> -{% for method in interface_event.methods %}
>>>>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "Thread")}}
>>>>>>>> +void {{proxy_name}}Remote::recvMessage(const IPCMessage &data)
>>>>>>>>      {
>>>>>>>> -       ASSERT(state_ != ProxyStopped);
>>>>>>>> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>>>>>>>> +       size_t dataSize = data.data().size();
>>>>>>>> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);
>>>>>>>> +
>>>>>>>> +       switch (_cmd) {
>>>>>>>> +{%- for method in interface_event.methods %}
>>>>>>>> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {
>>>>>>>> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());
>>>>>>>> +               break;
>>>>>>>> +       }
>>>>>>>> +{%- endfor %}
>>>>>>>> +       default:
>>>>>>>> +               LOG(IPAProxy, Error) << "Unknown command " << static_cast<uint32_t>(_cmd);
>>>>>>>> +       }
>>>>>>>>      }
>>>>>>>>      
>>>>>>>> -void {{proxy_name}}::{{method.mojom_name}}IPC(
>>>>>>>> +{% for method in interface_event.methods %}
>>>>>>>> +void {{proxy_name}}Remote::{{method.mojom_name}}Handler(
>>>>>>>>             [[maybe_unused]] std::vector<uint8_t>::const_iterator data,
>>>>>>>>             [[maybe_unused]] size_t dataSize,
>>>>>>>>             [[maybe_unused]] const std::vector<SharedFD> &fds)
>>>>>>>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>>>>>> index a0312a7c1..0c4ef985b 100644
>>>>>>>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>>>>>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
>>>>>>>> @@ -34,29 +34,32 @@ namespace {{ns}} {
>>>>>>>>      {% endfor %}
>>>>>>>>      {%- endif %}
>>>>>>>>      
>>>>>>>> +class {{proxy_name}}Local;
>>>>>>>> +class {{proxy_name}}Remote;
>>>>>>>> +
>>>>>>>>      class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object
>>>>>>>>      {
>>>>>>>>      public:
>>>>>>>> -       {{proxy_name}}(IPAModule *ipam, bool isolate);
>>>>>>>> -       ~{{proxy_name}}();
>>>>>>>> +       using Local = {{proxy_name}}Local;
>>>>>>>> +       using Remote = {{proxy_name}}Remote;
>>>>>>>>      
>>>>>>>> -{% for method in interface_main.methods %}
>>>>>>>> -{{proxy_funcs.func_sig(proxy_name, method, "", false, true)|indent(8, true)}};
>>>>>>>> -{% endfor %}
>>>>>>>> +protected:
>>>>>>>> +       using IPAProxy::IPAProxy;
>>>>>>>> +};
>>>>>>>>      
>>>>>>>> -private:
>>>>>>>> -       void recvMessage(const IPCMessage &data);
>>>>>>>> +class {{proxy_name}}Local : public {{proxy_name}}
>>>>>>>> +{
>>>>>>>> +public:
>>>>>>>> +       {{proxy_name}}Local(IPAModule *ipam);
>>>>>>>> +       ~{{proxy_name}}Local();
>>>>>>>>      
>>>>>>>>      {% 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)}};
>>>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
>>>>>
>>>>> As this is Python code I *think* you could also write
>>>>>
>>>>> {{proxy_funcs.func_sig(f"{proxy_name}Local", method, "", false, true)|indent(8, true)}};
>>>>>
>>>>> if you think it's clearer (sae elsewhere in the patch).
>>>>
>>>> I'll check.
>>
>> Apparently f-strings do not work in jinja, so I kept the "+".
> 
> Good to know, thanks for trying it out.
> 
>>>>>>>>      {% endfor %}
>>>>>>>> +
>>>>>>>> +private:
>>>>>>>>      {% 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<SharedFD> &fds);
>>>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "Handler", false)|indent(8, true)}};
>>>>>>>>      {% endfor %}
>>>>>>>>      
>>>>>>>>             /* Helper class to invoke async functions in another thread. */
>>>>>>>> @@ -79,12 +82,12 @@ private:
>>>>>>>>                     }
>>>>>>>>      {% for method in interface_main.methods %}
>>>>>>>>      {%- if method|is_async %}
>>>>>>>> -               {{proxy_funcs.func_sig(proxy_name, method, "", false)|indent(16)}}
>>>>>>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", 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)}}
>>>>>>>> +               {{proxy_funcs.func_sig(proxy_name + "Local", method, "", false)|indent(16)}}
>>>>>>>>                     {
>>>>>>>>      {%- if method|method_return_value != "void" %}
>>>>>>>>                             return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});
>>>>>>>> @@ -104,8 +107,27 @@ private:
>>>>>>>>             Thread thread_;
>>>>>>>>             ThreadProxy proxy_;
>>>>>>>>             std::unique_ptr<{{interface_name}}> ipa_;
>>>>>>>> +};
>>>>>>>>      
>>>>>>>> -       const bool isolate_;
>>>>>>>> +class {{proxy_name}}Remote : public {{proxy_name}}
>>>>>>>> +{
>>>>>>>> +public:
>>>>>>>> +       {{proxy_name}}Remote(IPAModule *ipam);
>>>>>>>> +       ~{{proxy_name}}Remote();
>>>>>>>> +
>>>>>>>> +{% for method in interface_main.methods %}
>>>>>>>> +{{proxy_funcs.func_sig(proxy_name + "Local", method, "", false, true)|indent(8, true)}};
>>>>>>>
>>>>>>> s/Local/Remote/ ?
>>>>>>>
>>>>>>> Other than that, looks good to me. This is exciting!
>>>>>>
>>>>>> Oops, fixed. I was wondering why it did not cause any issues... of course
>>>>>> the name of the class is not used due to the fourth argument being false.
>>>>>>
>>>>>>>> +{% endfor %}
>>>>>>>> +
>>>>>>>> +private:
>>>>>>>> +       void recvMessage(const IPCMessage &data);
>>>>>>>> +
>>>>>>>> +{% for method in interface_event.methods %}
>>>>>>>> +       void {{method.mojom_name}}Handler(
>>>>>>>> +               std::vector<uint8_t>::const_iterator data,
>>>>>>>> +               size_t dataSize,
>>>>>>>> +               const std::vector<SharedFD> &fds);
>>>>>>>> +{% endfor %}
>>>>>>>>      
>>>>>>>>             std::unique_ptr<IPCPipeUnixSocket> ipc_;
>>>>>>>>      
>