[RFC PATCH v1] utils: ipc: Do not define variables in signal handler upfront

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Wed, Mar 26, 2025 at 11:13:24AM +0100, Barnabás Pőcze wrote:
> Defining the variables at the beginning of the function forces the types
> to be default constructible, which may not be desirable; furthermore, it
> also forces the move/copy assignment operator to be used when the
> deserialized value is retrieved.
> 
> Having `T val = f()` has the advantage of benefitting from potential RVO
> as well as not requiring `T` to be default constructible, so generate
> code in that form by calling `deserialize_call()` with `declare=true`.

I looked at the diff in generated files:

diff -Nur a/src/libcamera/proxy/ipu3_ipa_proxy.cpp b/src/libcamera/proxy/ipu3_ipa_proxy.cpp
--- a/src/libcamera/proxy/ipu3_ipa_proxy.cpp	2025-03-20 17:00:30.998772793 +0200
+++ b/src/libcamera/proxy/ipu3_ipa_proxy.cpp	2025-04-22 18:26:58.982280071 +0300
@@ -609,9 +609,6 @@
 	[[maybe_unused]] size_t dataSize,
 	[[maybe_unused]] const std::vector<SharedFD> &fds)
 {
-	uint32_t frame;
-	ControlList sensorControls;
-	ControlList lensControls;
 
 	[[maybe_unused]] const size_t frameBufSize = readPOD<uint32_t>(data, 0, data + dataSize);
 	[[maybe_unused]] const size_t sensorControlsBufSize = readPOD<uint32_t>(data, 4, data + dataSize);
@@ -622,18 +619,18 @@
 	const size_t lensControlsStart = sensorControlsStart + sensorControlsBufSize;
 
 
-	frame =
+	uint32_t frame =
         IPADataSerializer<uint32_t>::deserialize(
         	data + frameStart,
         	data + frameStart + frameBufSize);
 
-	sensorControls =
+	ControlList sensorControls =
         IPADataSerializer<ControlList>::deserialize(
         	data + sensorControlsStart,
         	data + sensorControlsStart + sensorControlsBufSize,
         	&controlSerializer_);
 
-	lensControls =
+	ControlList lensControls =
         IPADataSerializer<ControlList>::deserialize(
         	data + lensControlsStart,
         	data + lensControlsStart + lensControlsBufSize,
@@ -654,13 +651,12 @@
 	[[maybe_unused]] size_t dataSize,
 	[[maybe_unused]] const std::vector<SharedFD> &fds)
 {
-	uint32_t frame;
 
 
 	const size_t frameStart = 0;
 
 
-	frame =
+	uint32_t frame =
         IPADataSerializer<uint32_t>::deserialize(
         	data + frameStart,
         	data + dataSize);
@@ -681,8 +677,6 @@
 	[[maybe_unused]] size_t dataSize,
 	[[maybe_unused]] const std::vector<SharedFD> &fds)
 {
-	uint32_t frame;
-	ControlList metadata;
 
 	[[maybe_unused]] const size_t frameBufSize = readPOD<uint32_t>(data, 0, data + dataSize);
 	[[maybe_unused]] const size_t metadataBufSize = readPOD<uint32_t>(data, 4, data + dataSize);
@@ -691,12 +685,12 @@
 	const size_t metadataStart = frameStart + frameBufSize;
 
 
-	frame =
+	uint32_t frame =
         IPADataSerializer<uint32_t>::deserialize(
         	data + frameStart,
         	data + frameStart + frameBufSize);
 
-	metadata =
+	ControlList metadata =
         IPADataSerializer<ControlList>::deserialize(
         	data + metadataStart,
         	data + metadataStart + metadataBufSize,

This looks right.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> 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 ce3cc5ab6..07165821e 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
> @@ -239,10 +239,7 @@ void {{proxy_name}}::{{method.mojom_name}}IPC(
>  	[[maybe_unused]] size_t dataSize,
>  	[[maybe_unused]] const std::vector<SharedFD> &fds)
>  {
> -{%- for param in method.parameters %}
> -	{{param|name}} {{param.mojom_name}};
> -{%- endfor %}
> -{{proxy_funcs.deserialize_call(method.parameters, 'data', 'fds', false, false, true, 'dataSize')}}
> +{{proxy_funcs.deserialize_call(method.parameters, 'data', 'fds', false, true, true, 'dataSize')}}
>  	{{method.mojom_name}}.emit({{method.parameters|params_comma_sep}});
>  }
>  {% endfor %}

-- 
Regards,

Laurent Pinchart