From patchwork Fri Aug 15 08:51:14 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: 24108 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 9164DBDCC1 for ; Fri, 15 Aug 2025 08:51:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8802B69257; Fri, 15 Aug 2025 10:51:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="A/0558Fj"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7DB9969244 for ; Fri, 15 Aug 2025 10:51:17 +0200 (CEST) Received: from pb-laptop.local (185.221.141.188.nat.pool.zt.hu [185.221.141.188]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6469763B; Fri, 15 Aug 2025 10:50:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1755247822; bh=PiBJW8Z8JkkVJLTGvi0ov/pj9JvYcU9JJzTpmMybaGk=; h=From:To:Subject:Date:From; b=A/0558FjA2BMlIKydR93vkoLfXWCfr8r3m9n6i9f0SSs5l3GdXwhBoTRyhWjTi75M wDgRdlCryDzSHC0N1521QBmfZt2yF1kv/JbweewkiDuPPVyigcA+crXuOpRE4Aw2UT DQmTX2S3ACjWKOU2bt6nr9l1QNWAJmn0c8M6RnkA= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org, Paul Elder Subject: [RFC PATCH v2] utils: codegen: ipc: Split proxy types Date: Fri, 15 Aug 2025 10:51:14 +0200 Message-ID: <20250815085114.2188604-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 "threaded" and "isolated" variants, and make `IPAManager` choose accordingly. Signed-off-by: Barnabás Pőcze --- changes in v2: * fix typo * rename local -> threaded, remote -> isolated * have different log messages in the two variants v1: https://patchwork.libcamera.org/patch/24056/ --- include/libcamera/internal/ipa_manager.h | 8 +- .../module_ipa_proxy.cpp.tmpl | 151 +++++++++--------- .../module_ipa_proxy.h.tmpl | 56 +++++-- 3 files changed, 119 insertions(+), 96 deletions(-) -- 2.50.1 diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h index a0d448cf9..ae832b645 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..7ccde1d56 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}}Threaded::{{proxy_name}}Threaded(IPAModule *ipam) + : {{proxy_name}}(ipam) { LOG(IPAProxy, Debug) - << "initializing {{module_name}} proxy: loading IPA from " + << "initializing {{module_name}} proxy in thread: 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}}Threaded::{{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}}Threaded::~{{proxy_name}}Threaded() = 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 + "Threaded", method)}} { {%- if method.mojom_name == "stop" %} {{proxy_funcs.stop_thread_body()}} @@ -181,8 +115,58 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) ); {%- endif %} } +{% endfor %} + +{% for method in interface_event.methods %} +{{proxy_funcs.func_sig(proxy_name + "Threaded", method, "Handler")}} +{ + ASSERT(state_ != ProxyStopped); + {{method.mojom_name}}.emit({{method.parameters|params_comma_sep}}); +} +{% endfor %} + +/* ========================================================================== */ + +{{proxy_name}}Isolated::{{proxy_name}}Isolated(IPAModule *ipam) + : {{proxy_name}}(ipam), + controlSerializer_(ControlSerializer::Role::Proxy), seq_(0) +{ + LOG(IPAProxy, Debug) + << "initializing {{module_name}} proxy in isolation: 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}}Isolated::recvMessage); + + ipc_ = std::move(ipc); + valid_ = true; +} -{{proxy_funcs.func_sig(proxy_name, method, "IPC")}} +{{proxy_name}}Isolated::~{{proxy_name}}Isolated() +{ + 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 + "Isolated", 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}}Isolated::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}}Isolated::{{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..caf8392da 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}}Threaded; +class {{proxy_name}}Isolated; + class {{proxy_name}} : public IPAProxy, public {{interface_name}}, public Object { public: - {{proxy_name}}(IPAModule *ipam, bool isolate); - ~{{proxy_name}}(); + using Threaded = {{proxy_name}}Threaded; + using Isolated = {{proxy_name}}Isolated; -{% 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}}Threaded : public {{proxy_name}} +{ +public: + {{proxy_name}}Threaded(IPAModule *ipam); + ~{{proxy_name}}Threaded(); {% 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 + "Threaded", 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 + "Threaded", 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 + "Threaded", 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 + "Threaded", 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}}Isolated : public {{proxy_name}} +{ +public: + {{proxy_name}}Isolated(IPAModule *ipam); + ~{{proxy_name}}Isolated(); + +{% for method in interface_main.methods %} +{{proxy_funcs.func_sig(proxy_name + "Isolated", 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_;