From patchwork Mon Aug 4 16:52:26 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 24056 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 3A8DCBE086 for ; Mon, 4 Aug 2025 16:52:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D84C869220; Mon, 4 Aug 2025 18:52:30 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="DJcMx/QJ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DDFE769217 for ; Mon, 4 Aug 2025 18:52:28 +0200 (CEST) Received: from pb-laptop.local (185.182.215.213.nat.pool.zt.hu [185.182.215.213]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B4BABC6D; Mon, 4 Aug 2025 18:51:41 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1754326301; bh=sbUHwJ4Js68DkKRnjeyalheTvQoaHJ60FTiPfEWksYo=; h=From:To:Subject:Date:From; b=DJcMx/QJ4vzVRRbWxWz4KP7JEwQyp5FrdRjIAhEmtWyBY8W/iZfMHQzPyOZYBcBGX MMiHEq9AqaEphHb5CKBlRj5ej9n6XCdn+nSqIvElONOBrOmegI4Tf/QCTjqr/LMgij meykkChbBbSRwMFuxlydZat7i9kSH/0JnpaC5Dvc= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org, Paul Elder Subject: [RFC PATCH v1] utils: codegen: ipc: Split proxy types Date: Mon, 4 Aug 2025 18:52:26 +0200 Message-ID: <20250804165226.63825-1-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.50.1 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- 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 proxy = std::make_unique(m, !self->isSignatureValid(m)); + auto proxy = [&]() -> std::unique_ptr { + if (self->isSignatureValid(m)) + return std::make_unique(m); + else + return std::make_unique(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(); - 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(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({{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(_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(); + + 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(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({{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(_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::const_iterator data, [[maybe_unused]] size_t dataSize, [[maybe_unused]] const std::vector &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::const_iterator data, - size_t dataSize, - const std::vector &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)}}; +{% endfor %} + +private: + void recvMessage(const IPCMessage &data); + +{% for method in interface_event.methods %} + void {{method.mojom_name}}Handler( + std::vector::const_iterator data, + size_t dataSize, + const std::vector &fds); +{% endfor %} std::unique_ptr ipc_;