[RFC,v2] utils: codegen: ipc: Split proxy types
diff mbox series

Message ID 20250815085114.2188604-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [RFC,v2] utils: codegen: ipc: Split proxy types
Related show

Commit Message

Barnabás Pőcze Aug. 15, 2025, 8:51 a.m. UTC 
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 <barnabas.pocze@ideasonboard.com>
---
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

Patch
diff mbox series 

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<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::Threaded>(m);
+			else
+				return std::make_unique<typename T::Isolated>(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<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}}Threaded::{{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}}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<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}}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<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 + "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<uint32_t>(_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<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..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<uint8_t>::const_iterator data,
-		size_t dataSize,
-		const std::vector<SharedFD> &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<uint8_t>::const_iterator data,
+		size_t dataSize,
+		const std::vector<SharedFD> &fds);
+{% endfor %}

 	std::unique_ptr<IPCPipeUnixSocket> ipc_;