[{"id":35555,"web_url":"https://patchwork.libcamera.org/comment/35555/","msgid":"<175617809646.607151.8412527477107317174@neptunite.rasen.tech>","date":"2025-08-26T03:14:56","subject":"Re: [RFC PATCH v2] utils: codegen: ipc: Split proxy types","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Barnabás,\n\nThanks for the patch.\n\nQuoting Barnabás Pőcze (2025-08-15 17:51:14)\n> Even though there is an abstract class to represent the interface of an IPA,\n> the threaded and IPC versions are still multiplexed using the same type,\n> which uses a boolean to actually dispatch to the right function.\n> \n> Instead of doing that, split the classes into \"threaded\" and \"isolated\"\n> variants, and make `IPAManager` choose accordingly.\n> \n> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> ---\n> changes in v2:\n>   * fix typo\n>   * rename local -> threaded, remote -> isolated\n>   * have different log messages in the two variants\n> \n> v1: https://patchwork.libcamera.org/patch/24056/\n> ---\n>  include/libcamera/internal/ipa_manager.h      |   8 +-\n>  .../module_ipa_proxy.cpp.tmpl                 | 151 +++++++++---------\n>  .../module_ipa_proxy.h.tmpl                   |  56 +++++--\n>  3 files changed, 119 insertions(+), 96 deletions(-)\n> \n> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n> index a0d448cf9..ae832b645 100644\n> --- a/include/libcamera/internal/ipa_manager.h\n> +++ b/include/libcamera/internal/ipa_manager.h\n> @@ -42,7 +42,13 @@ public:\n>                 if (!m)\n>                         return nullptr;\n> \n> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));\n> +               auto proxy = [&]() -> std::unique_ptr<T> {\n> +                       if (self->isSignatureValid(m))\n> +                               return std::make_unique<typename T::Threaded>(m);\n> +                       else\n> +                               return std::make_unique<typename T::Isolated>(m);\n> +               }();\n> +\n>                 if (!proxy->isValid()) {\n>                         LOG(IPAManager, Error) << \"Failed to load proxy\";\n>                         return nullptr;\n> 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\n> index beb646e2d..7ccde1d56 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n> @@ -45,36 +45,13 @@ namespace {{ns}} {\n>  {% endfor %}\n>  {%- endif %}\n> \n> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)\n> -       : IPAProxy(ipam), isolate_(isolate),\n> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)\n> +{{proxy_name}}Threaded::{{proxy_name}}Threaded(IPAModule *ipam)\n> +       : {{proxy_name}}(ipam)\n>  {\n>         LOG(IPAProxy, Debug)\n> -               << \"initializing {{module_name}} proxy: loading IPA from \"\n> +               << \"initializing {{module_name}} proxy in thread: loading IPA from \"\n>                 << ipam->path();\n> \n> -       if (isolate_) {\n> -               const std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\");\n> -               if (proxyWorkerPath.empty()) {\n> -                       LOG(IPAProxy, Error)\n> -                               << \"Failed to get proxy worker path\";\n> -                       return;\n> -               }\n> -\n> -               auto ipc = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(),\n> -                                                              proxyWorkerPath.c_str());\n> -               if (!ipc->isConnected()) {\n> -                       LOG(IPAProxy, Error) << \"Failed to create IPCPipe\";\n> -                       return;\n> -               }\n> -\n> -               ipc->recv.connect(this, &{{proxy_name}}::recvMessage);\n> -\n> -               ipc_ = std::move(ipc);\n> -               valid_ = true;\n> -               return;\n> -       }\n> -\n>         if (!ipam->load())\n>                 return;\n> \n> @@ -89,59 +66,16 @@ namespace {{ns}} {\n>         proxy_.setIPA(ipa_.get());\n> \n>  {% for method in interface_event.methods %}\n> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);\n> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Threaded::{{method.mojom_name}}Handler);\n>  {%- endfor %}\n> \n>         valid_ = true;\n>  }\n> \n> -{{proxy_name}}::~{{proxy_name}}()\n> -{\n> -       if (ipc_) {\n> -               IPCMessage::Header header =\n> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };\n> -               IPCMessage msg(header);\n> -               ipc_->sendAsync(msg);\n> -       }\n> -}\n> -\n> -{% if interface_event.methods|length > 0 %}\n> -void {{proxy_name}}::recvMessage(const IPCMessage &data)\n> -{\n> -       size_t dataSize = data.data().size();\n> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);\n> -\n> -       switch (_cmd) {\n> -{%- for method in interface_event.methods %}\n> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {\n> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());\n> -               break;\n> -       }\n> -{%- endfor %}\n> -       default:\n> -               LOG(IPAProxy, Error) << \"Unknown command \" << static_cast<uint32_t>(_cmd);\n> -       }\n> -}\n> -{%- endif %}\n> +{{proxy_name}}Threaded::~{{proxy_name}}Threaded() = default;\n> \n>  {% for method in interface_main.methods %}\n> -{{proxy_funcs.func_sig(proxy_name, method)}}\n> -{\n> -       if (isolate_)\n> -               return {{method.mojom_name}}IPC(\n> -{%- for param in method|method_param_names -%}\n> -               {{param}}{{- \", \" if not loop.last}}\n> -{%- endfor -%}\n> -);\n> -       else\n> -               return {{method.mojom_name}}Thread(\n> -{%- for param in method|method_param_names -%}\n> -               {{param}}{{- \", \" if not loop.last}}\n> -{%- endfor -%}\n> -);\n> -}\n> -\n> -{{proxy_funcs.func_sig(proxy_name, method, \"Thread\")}}\n> +{{proxy_funcs.func_sig(proxy_name + \"Threaded\", method)}}\n>  {\n>  {%- if method.mojom_name == \"stop\" %}\n>         {{proxy_funcs.stop_thread_body()}}\n> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)\n>  );\n>  {%- endif %}\n>  }\n> +{% endfor %}\n> +\n> +{% for method in interface_event.methods %}\n> +{{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"Handler\")}}\n> +{\n> +       ASSERT(state_ != ProxyStopped);\n> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});\n> +}\n> +{% endfor %}\n> +\n> +/* ========================================================================== */\n> +\n> +{{proxy_name}}Isolated::{{proxy_name}}Isolated(IPAModule *ipam)\n> +       : {{proxy_name}}(ipam),\n> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)\n> +{\n> +       LOG(IPAProxy, Debug)\n> +               << \"initializing {{module_name}} proxy in isolation: loading IPA from \"\n> +               << ipam->path();\n> +\n> +       const std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\");\n> +       if (proxyWorkerPath.empty()) {\n> +               LOG(IPAProxy, Error)\n> +                       << \"Failed to get proxy worker path\";\n> +               return;\n> +       }\n> +\n> +       auto ipc = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(),\n> +                                                      proxyWorkerPath.c_str());\n> +       if (!ipc->isConnected()) {\n> +               LOG(IPAProxy, Error) << \"Failed to create IPCPipe\";\n> +               return;\n> +       }\n> +\n> +       ipc->recv.connect(this, &{{proxy_name}}Isolated::recvMessage);\n> +\n> +       ipc_ = std::move(ipc);\n> +       valid_ = true;\n> +}\n> \n> -{{proxy_funcs.func_sig(proxy_name, method, \"IPC\")}}\n> +{{proxy_name}}Isolated::~{{proxy_name}}Isolated()\n> +{\n> +       if (ipc_) {\n> +               IPCMessage::Header header =\n> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };\n> +               IPCMessage msg(header);\n> +               ipc_->sendAsync(msg);\n> +       }\n> +}\n> +\n> +{% for method in interface_main.methods %}\n> +{{proxy_funcs.func_sig(proxy_name + \"Isolated\", method)}}\n>  {\n>  {%- if method.mojom_name == \"configure\" %}\n>         controlSerializer_.reset();\n> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)\n> \n>  {% endfor %}\n> \n> -{% for method in interface_event.methods %}\n> -{{proxy_funcs.func_sig(proxy_name, method, \"Thread\")}}\n> +void {{proxy_name}}Isolated::recvMessage(const IPCMessage &data)\n>  {\n> -       ASSERT(state_ != ProxyStopped);\n> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});\n> +       size_t dataSize = data.data().size();\n> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);\n> +\n> +       switch (_cmd) {\n> +{%- for method in interface_event.methods %}\n\nI just realized that you removed the {% if interface_event.methods|length > 0\n%} guard that was around recvMessage (and that I didn't have the same guard in\nthe header file). Would the compiler complain here that the switch has no cases here?\n\nOr maybe we don't have to worry about that scenario since IPAs will never not\nhave events, since an IPA that doesn't return anything to the pipeline handler\nis the same as no IPA. So maybe it's fine.\n\nOk, looks good to me.\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {\n> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());\n> +               break;\n> +       }\n> +{%- endfor %}\n> +       default:\n> +               LOG(IPAProxy, Error) << \"Unknown command \" << static_cast<uint32_t>(_cmd);\n> +       }\n>  }\n> \n> -void {{proxy_name}}::{{method.mojom_name}}IPC(\n> +{% for method in interface_event.methods %}\n> +void {{proxy_name}}Isolated::{{method.mojom_name}}Handler(\n>         [[maybe_unused]] std::vector<uint8_t>::const_iterator data,\n>         [[maybe_unused]] size_t dataSize,\n>         [[maybe_unused]] const std::vector<SharedFD> &fds)\n> 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\n> index a0312a7c1..caf8392da 100644\n> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n> @@ -34,29 +34,32 @@ namespace {{ns}} {\n>  {% endfor %}\n>  {%- endif %}\n> \n> +class {{proxy_name}}Threaded;\n> +class {{proxy_name}}Isolated;\n> +\n>  class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object\n>  {\n>  public:\n> -       {{proxy_name}}(IPAModule *ipam, bool isolate);\n> -       ~{{proxy_name}}();\n> +       using Threaded = {{proxy_name}}Threaded;\n> +       using Isolated = {{proxy_name}}Isolated;\n> \n> -{% for method in interface_main.methods %}\n> -{{proxy_funcs.func_sig(proxy_name, method, \"\", false, true)|indent(8, true)}};\n> -{% endfor %}\n> +protected:\n> +       using IPAProxy::IPAProxy;\n> +};\n> \n> -private:\n> -       void recvMessage(const IPCMessage &data);\n> +class {{proxy_name}}Threaded : public {{proxy_name}}\n> +{\n> +public:\n> +       {{proxy_name}}Threaded(IPAModule *ipam);\n> +       ~{{proxy_name}}Threaded();\n> \n>  {% for method in interface_main.methods %}\n> -{{proxy_funcs.func_sig(proxy_name, method, \"Thread\", false)|indent(8, true)}};\n> -{{proxy_funcs.func_sig(proxy_name, method, \"IPC\", false)|indent(8, true)}};\n> +{{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"\", false, true)|indent(8, true)}};\n>  {% endfor %}\n> +\n> +private:\n>  {% for method in interface_event.methods %}\n> -{{proxy_funcs.func_sig(proxy_name, method, \"Thread\", false)|indent(8, true)}};\n> -       void {{method.mojom_name}}IPC(\n> -               std::vector<uint8_t>::const_iterator data,\n> -               size_t dataSize,\n> -               const std::vector<SharedFD> &fds);\n> +{{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"Handler\", false)|indent(8, true)}};\n>  {% endfor %}\n> \n>         /* Helper class to invoke async functions in another thread. */\n> @@ -79,12 +82,12 @@ private:\n>                 }\n>  {% for method in interface_main.methods %}\n>  {%- if method|is_async %}\n> -               {{proxy_funcs.func_sig(proxy_name, method, \"\", false)|indent(16)}}\n> +               {{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"\", false)|indent(16)}}\n>                 {\n>                         ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});\n>                 }\n>  {%- elif method.mojom_name == \"start\" %}\n> -               {{proxy_funcs.func_sig(proxy_name, method, \"\", false)|indent(16)}}\n> +               {{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"\", false)|indent(16)}}\n>                 {\n>  {%- if method|method_return_value != \"void\" %}\n>                         return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});\n> @@ -104,8 +107,27 @@ private:\n>         Thread thread_;\n>         ThreadProxy proxy_;\n>         std::unique_ptr<{{interface_name}}> ipa_;\n> +};\n> \n> -       const bool isolate_;\n> +class {{proxy_name}}Isolated : public {{proxy_name}}\n> +{\n> +public:\n> +       {{proxy_name}}Isolated(IPAModule *ipam);\n> +       ~{{proxy_name}}Isolated();\n> +\n> +{% for method in interface_main.methods %}\n> +{{proxy_funcs.func_sig(proxy_name + \"Isolated\", method, \"\", false, true)|indent(8, true)}};\n> +{% endfor %}\n> +\n> +private:\n> +       void recvMessage(const IPCMessage &data);\n> +\n> +{% for method in interface_event.methods %}\n> +       void {{method.mojom_name}}Handler(\n> +               std::vector<uint8_t>::const_iterator data,\n> +               size_t dataSize,\n> +               const std::vector<SharedFD> &fds);\n> +{% endfor %}\n> \n>         std::unique_ptr<IPCPipeUnixSocket> ipc_;\n> \n> --\n> 2.50.1","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9D0E6BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Aug 2025 03:15:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2796692E9;\n\tTue, 26 Aug 2025 05:15:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87282613BA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Aug 2025 05:15:04 +0200 (CEST)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:5d3f:5a62:a50a:b707])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 1DA7419F6; \n\tTue, 26 Aug 2025 05:14:00 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"GZnIp1Y1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1756178041;\n\tbh=PJY0dpvVuu/dzPxD4zE5t3jYaZFFrHI5WUA5JGav9F0=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=GZnIp1Y1GVugZRb7eHMyOKzUq2o/VkyYQ+5VBPTpsk6R1CSUdN3WKbkp9mz89aU7U\n\tR9/UVjqAXMNX8y8220j8YYTmvU0qtsqO5Z71Q55RlexaxKtEUwfMVsbWXeTPpbnKz4\n\tXGehqrZzjU4uozCUQ9MxlozMJ/NK173lyw1fdBNY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250815085114.2188604-1-barnabas.pocze@ideasonboard.com>","References":"<20250815085114.2188604-1-barnabas.pocze@ideasonboard.com>","Subject":"Re: [RFC PATCH v2] utils: codegen: ipc: Split proxy types","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 26 Aug 2025 12:14:56 +0900","Message-ID":"<175617809646.607151.8412527477107317174@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35569,"web_url":"https://patchwork.libcamera.org/comment/35569/","msgid":"<e105d378-6b43-401f-8421-7ba2a82bbb07@ideasonboard.com>","date":"2025-08-26T10:03:25","subject":"Re: [RFC PATCH v2] utils: codegen: ipc: Split proxy types","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 08. 26. 5:14 keltezéssel, Paul Elder írta:\n> Hi Barnabás,\n> \n> Thanks for the patch.\n> \n> Quoting Barnabás Pőcze (2025-08-15 17:51:14)\n>> Even though there is an abstract class to represent the interface of an IPA,\n>> the threaded and IPC versions are still multiplexed using the same type,\n>> which uses a boolean to actually dispatch to the right function.\n>>\n>> Instead of doing that, split the classes into \"threaded\" and \"isolated\"\n>> variants, and make `IPAManager` choose accordingly.\n>>\n>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n>> ---\n>> changes in v2:\n>>    * fix typo\n>>    * rename local -> threaded, remote -> isolated\n>>    * have different log messages in the two variants\n>>\n>> v1: https://patchwork.libcamera.org/patch/24056/\n>> ---\n>>   include/libcamera/internal/ipa_manager.h      |   8 +-\n>>   .../module_ipa_proxy.cpp.tmpl                 | 151 +++++++++---------\n>>   .../module_ipa_proxy.h.tmpl                   |  56 +++++--\n>>   3 files changed, 119 insertions(+), 96 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n>> index a0d448cf9..ae832b645 100644\n>> --- a/include/libcamera/internal/ipa_manager.h\n>> +++ b/include/libcamera/internal/ipa_manager.h\n>> @@ -42,7 +42,13 @@ public:\n>>                  if (!m)\n>>                          return nullptr;\n>>\n>> -               std::unique_ptr<T> proxy = std::make_unique<T>(m, !self->isSignatureValid(m));\n>> +               auto proxy = [&]() -> std::unique_ptr<T> {\n>> +                       if (self->isSignatureValid(m))\n>> +                               return std::make_unique<typename T::Threaded>(m);\n>> +                       else\n>> +                               return std::make_unique<typename T::Isolated>(m);\n>> +               }();\n>> +\n>>                  if (!proxy->isValid()) {\n>>                          LOG(IPAManager, Error) << \"Failed to load proxy\";\n>>                          return nullptr;\n>> 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\n>> index beb646e2d..7ccde1d56 100644\n>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl\n>> @@ -45,36 +45,13 @@ namespace {{ns}} {\n>>   {% endfor %}\n>>   {%- endif %}\n>>\n>> -{{proxy_name}}::{{proxy_name}}(IPAModule *ipam, bool isolate)\n>> -       : IPAProxy(ipam), isolate_(isolate),\n>> -         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)\n>> +{{proxy_name}}Threaded::{{proxy_name}}Threaded(IPAModule *ipam)\n>> +       : {{proxy_name}}(ipam)\n>>   {\n>>          LOG(IPAProxy, Debug)\n>> -               << \"initializing {{module_name}} proxy: loading IPA from \"\n>> +               << \"initializing {{module_name}} proxy in thread: loading IPA from \"\n>>                  << ipam->path();\n>>\n>> -       if (isolate_) {\n>> -               const std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\");\n>> -               if (proxyWorkerPath.empty()) {\n>> -                       LOG(IPAProxy, Error)\n>> -                               << \"Failed to get proxy worker path\";\n>> -                       return;\n>> -               }\n>> -\n>> -               auto ipc = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(),\n>> -                                                              proxyWorkerPath.c_str());\n>> -               if (!ipc->isConnected()) {\n>> -                       LOG(IPAProxy, Error) << \"Failed to create IPCPipe\";\n>> -                       return;\n>> -               }\n>> -\n>> -               ipc->recv.connect(this, &{{proxy_name}}::recvMessage);\n>> -\n>> -               ipc_ = std::move(ipc);\n>> -               valid_ = true;\n>> -               return;\n>> -       }\n>> -\n>>          if (!ipam->load())\n>>                  return;\n>>\n>> @@ -89,59 +66,16 @@ namespace {{ns}} {\n>>          proxy_.setIPA(ipa_.get());\n>>\n>>   {% for method in interface_event.methods %}\n>> -       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}::{{method.mojom_name}}Thread);\n>> +       ipa_->{{method.mojom_name}}.connect(this, &{{proxy_name}}Threaded::{{method.mojom_name}}Handler);\n>>   {%- endfor %}\n>>\n>>          valid_ = true;\n>>   }\n>>\n>> -{{proxy_name}}::~{{proxy_name}}()\n>> -{\n>> -       if (ipc_) {\n>> -               IPCMessage::Header header =\n>> -                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };\n>> -               IPCMessage msg(header);\n>> -               ipc_->sendAsync(msg);\n>> -       }\n>> -}\n>> -\n>> -{% if interface_event.methods|length > 0 %}\n>> -void {{proxy_name}}::recvMessage(const IPCMessage &data)\n>> -{\n>> -       size_t dataSize = data.data().size();\n>> -       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);\n>> -\n>> -       switch (_cmd) {\n>> -{%- for method in interface_event.methods %}\n>> -       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {\n>> -               {{method.mojom_name}}IPC(data.data().cbegin(), dataSize, data.fds());\n>> -               break;\n>> -       }\n>> -{%- endfor %}\n>> -       default:\n>> -               LOG(IPAProxy, Error) << \"Unknown command \" << static_cast<uint32_t>(_cmd);\n>> -       }\n>> -}\n>> -{%- endif %}\n>> +{{proxy_name}}Threaded::~{{proxy_name}}Threaded() = default;\n>>\n>>   {% for method in interface_main.methods %}\n>> -{{proxy_funcs.func_sig(proxy_name, method)}}\n>> -{\n>> -       if (isolate_)\n>> -               return {{method.mojom_name}}IPC(\n>> -{%- for param in method|method_param_names -%}\n>> -               {{param}}{{- \", \" if not loop.last}}\n>> -{%- endfor -%}\n>> -);\n>> -       else\n>> -               return {{method.mojom_name}}Thread(\n>> -{%- for param in method|method_param_names -%}\n>> -               {{param}}{{- \", \" if not loop.last}}\n>> -{%- endfor -%}\n>> -);\n>> -}\n>> -\n>> -{{proxy_funcs.func_sig(proxy_name, method, \"Thread\")}}\n>> +{{proxy_funcs.func_sig(proxy_name + \"Threaded\", method)}}\n>>   {\n>>   {%- if method.mojom_name == \"stop\" %}\n>>          {{proxy_funcs.stop_thread_body()}}\n>> @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)\n>>   );\n>>   {%- endif %}\n>>   }\n>> +{% endfor %}\n>> +\n>> +{% for method in interface_event.methods %}\n>> +{{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"Handler\")}}\n>> +{\n>> +       ASSERT(state_ != ProxyStopped);\n>> +       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});\n>> +}\n>> +{% endfor %}\n>> +\n>> +/* ========================================================================== */\n>> +\n>> +{{proxy_name}}Isolated::{{proxy_name}}Isolated(IPAModule *ipam)\n>> +       : {{proxy_name}}(ipam),\n>> +         controlSerializer_(ControlSerializer::Role::Proxy), seq_(0)\n>> +{\n>> +       LOG(IPAProxy, Debug)\n>> +               << \"initializing {{module_name}} proxy in isolation: loading IPA from \"\n>> +               << ipam->path();\n>> +\n>> +       const std::string proxyWorkerPath = resolvePath(\"{{module_name}}_ipa_proxy\");\n>> +       if (proxyWorkerPath.empty()) {\n>> +               LOG(IPAProxy, Error)\n>> +                       << \"Failed to get proxy worker path\";\n>> +               return;\n>> +       }\n>> +\n>> +       auto ipc = std::make_unique<IPCPipeUnixSocket>(ipam->path().c_str(),\n>> +                                                      proxyWorkerPath.c_str());\n>> +       if (!ipc->isConnected()) {\n>> +               LOG(IPAProxy, Error) << \"Failed to create IPCPipe\";\n>> +               return;\n>> +       }\n>> +\n>> +       ipc->recv.connect(this, &{{proxy_name}}Isolated::recvMessage);\n>> +\n>> +       ipc_ = std::move(ipc);\n>> +       valid_ = true;\n>> +}\n>>\n>> -{{proxy_funcs.func_sig(proxy_name, method, \"IPC\")}}\n>> +{{proxy_name}}Isolated::~{{proxy_name}}Isolated()\n>> +{\n>> +       if (ipc_) {\n>> +               IPCMessage::Header header =\n>> +                       { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ };\n>> +               IPCMessage msg(header);\n>> +               ipc_->sendAsync(msg);\n>> +       }\n>> +}\n>> +\n>> +{% for method in interface_main.methods %}\n>> +{{proxy_funcs.func_sig(proxy_name + \"Isolated\", method)}}\n>>   {\n>>   {%- if method.mojom_name == \"configure\" %}\n>>          controlSerializer_.reset();\n>> @@ -226,14 +210,25 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)\n>>\n>>   {% endfor %}\n>>\n>> -{% for method in interface_event.methods %}\n>> -{{proxy_funcs.func_sig(proxy_name, method, \"Thread\")}}\n>> +void {{proxy_name}}Isolated::recvMessage(const IPCMessage &data)\n>>   {\n>> -       ASSERT(state_ != ProxyStopped);\n>> -       {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});\n>> +       size_t dataSize = data.data().size();\n>> +       {{cmd_event_enum_name}} _cmd = static_cast<{{cmd_event_enum_name}}>(data.header().cmd);\n>> +\n>> +       switch (_cmd) {\n>> +{%- for method in interface_event.methods %}\n> \n> I just realized that you removed the {% if interface_event.methods|length > 0\n> %} guard that was around recvMessage (and that I didn't have the same guard in\n> the header file). Would the compiler complain here that the switch has no cases here?\n> \n> Or maybe we don't have to worry about that scenario since IPAs will never not\n> have events, since an IPA that doesn't return anything to the pipeline handler\n> is the same as no IPA. So maybe it's fine.\n\nThe enum type seems to always be generated, so I am pretty sure it should be ok.\nI don't expect any compiler to complain about the lack of `case` labels in any case.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> Ok, looks good to me.\n> \n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n>> +       case {{cmd_event_enum_name}}::{{method.mojom_name|cap}}: {\n>> +               {{method.mojom_name}}Handler(data.data().cbegin(), dataSize, data.fds());\n>> +               break;\n>> +       }\n>> +{%- endfor %}\n>> +       default:\n>> +               LOG(IPAProxy, Error) << \"Unknown command \" << static_cast<uint32_t>(_cmd);\n>> +       }\n>>   }\n>>\n>> -void {{proxy_name}}::{{method.mojom_name}}IPC(\n>> +{% for method in interface_event.methods %}\n>> +void {{proxy_name}}Isolated::{{method.mojom_name}}Handler(\n>>          [[maybe_unused]] std::vector<uint8_t>::const_iterator data,\n>>          [[maybe_unused]] size_t dataSize,\n>>          [[maybe_unused]] const std::vector<SharedFD> &fds)\n>> 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\n>> index a0312a7c1..caf8392da 100644\n>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl\n>> @@ -34,29 +34,32 @@ namespace {{ns}} {\n>>   {% endfor %}\n>>   {%- endif %}\n>>\n>> +class {{proxy_name}}Threaded;\n>> +class {{proxy_name}}Isolated;\n>> +\n>>   class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object\n>>   {\n>>   public:\n>> -       {{proxy_name}}(IPAModule *ipam, bool isolate);\n>> -       ~{{proxy_name}}();\n>> +       using Threaded = {{proxy_name}}Threaded;\n>> +       using Isolated = {{proxy_name}}Isolated;\n>>\n>> -{% for method in interface_main.methods %}\n>> -{{proxy_funcs.func_sig(proxy_name, method, \"\", false, true)|indent(8, true)}};\n>> -{% endfor %}\n>> +protected:\n>> +       using IPAProxy::IPAProxy;\n>> +};\n>>\n>> -private:\n>> -       void recvMessage(const IPCMessage &data);\n>> +class {{proxy_name}}Threaded : public {{proxy_name}}\n>> +{\n>> +public:\n>> +       {{proxy_name}}Threaded(IPAModule *ipam);\n>> +       ~{{proxy_name}}Threaded();\n>>\n>>   {% for method in interface_main.methods %}\n>> -{{proxy_funcs.func_sig(proxy_name, method, \"Thread\", false)|indent(8, true)}};\n>> -{{proxy_funcs.func_sig(proxy_name, method, \"IPC\", false)|indent(8, true)}};\n>> +{{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"\", false, true)|indent(8, true)}};\n>>   {% endfor %}\n>> +\n>> +private:\n>>   {% for method in interface_event.methods %}\n>> -{{proxy_funcs.func_sig(proxy_name, method, \"Thread\", false)|indent(8, true)}};\n>> -       void {{method.mojom_name}}IPC(\n>> -               std::vector<uint8_t>::const_iterator data,\n>> -               size_t dataSize,\n>> -               const std::vector<SharedFD> &fds);\n>> +{{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"Handler\", false)|indent(8, true)}};\n>>   {% endfor %}\n>>\n>>          /* Helper class to invoke async functions in another thread. */\n>> @@ -79,12 +82,12 @@ private:\n>>                  }\n>>   {% for method in interface_main.methods %}\n>>   {%- if method|is_async %}\n>> -               {{proxy_funcs.func_sig(proxy_name, method, \"\", false)|indent(16)}}\n>> +               {{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"\", false)|indent(16)}}\n>>                  {\n>>                          ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});\n>>                  }\n>>   {%- elif method.mojom_name == \"start\" %}\n>> -               {{proxy_funcs.func_sig(proxy_name, method, \"\", false)|indent(16)}}\n>> +               {{proxy_funcs.func_sig(proxy_name + \"Threaded\", method, \"\", false)|indent(16)}}\n>>                  {\n>>   {%- if method|method_return_value != \"void\" %}\n>>                          return ipa_->{{method.mojom_name}}({{method.parameters|params_comma_sep}});\n>> @@ -104,8 +107,27 @@ private:\n>>          Thread thread_;\n>>          ThreadProxy proxy_;\n>>          std::unique_ptr<{{interface_name}}> ipa_;\n>> +};\n>>\n>> -       const bool isolate_;\n>> +class {{proxy_name}}Isolated : public {{proxy_name}}\n>> +{\n>> +public:\n>> +       {{proxy_name}}Isolated(IPAModule *ipam);\n>> +       ~{{proxy_name}}Isolated();\n>> +\n>> +{% for method in interface_main.methods %}\n>> +{{proxy_funcs.func_sig(proxy_name + \"Isolated\", method, \"\", false, true)|indent(8, true)}};\n>> +{% endfor %}\n>> +\n>> +private:\n>> +       void recvMessage(const IPCMessage &data);\n>> +\n>> +{% for method in interface_event.methods %}\n>> +       void {{method.mojom_name}}Handler(\n>> +               std::vector<uint8_t>::const_iterator data,\n>> +               size_t dataSize,\n>> +               const std::vector<SharedFD> &fds);\n>> +{% endfor %}\n>>\n>>          std::unique_ptr<IPCPipeUnixSocket> ipc_;\n>>\n>> --\n>> 2.50.1","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4C670BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 26 Aug 2025 10:03:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BAD55692E9;\n\tTue, 26 Aug 2025 12:03:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C985F692D1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Aug 2025 12:03:30 +0200 (CEST)","from [192.168.33.22] (185.182.215.11.nat.pool.zt.hu\n\t[185.182.215.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2C60FE45;\n\tTue, 26 Aug 2025 12:02:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Kcjdkarr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1756202547;\n\tbh=ivxKuOJQmU1lEs+VnRI5o7JZMr+sPnPYlLg/SUauFFE=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=KcjdkarrmvrkpDlWfRD4C0lpIzOY1I9WwsRm4x2X6f7yrSVlvx2I4SqMEJzxTwRvq\n\tK/C2UILPT0iO6RIB/3jBu+y0pBCpmd8Wyv43xz8T8+5qLEaYLF9/nccX7d8F8NUwzh\n\tJ+kPnAR1zdibZeJr+D9Mjdj443X3K+ieR/I6LTig=","Message-ID":"<e105d378-6b43-401f-8421-7ba2a82bbb07@ideasonboard.com>","Date":"Tue, 26 Aug 2025 12:03:25 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [RFC PATCH v2] utils: codegen: ipc: Split proxy types","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250815085114.2188604-1-barnabas.pocze@ideasonboard.com>\n\t<175617809646.607151.8412527477107317174@neptunite.rasen.tech>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<175617809646.607151.8412527477107317174@neptunite.rasen.tech>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]