[libcamera-devel] [PATCH 2/6] ipa: proxy_worker: Reset ControlSerializer on worker

[paul.elder at ideasonboard.com]

Hi Jacopo,

On Wed, Sep 01, 2021 at 04:37:56PM +0200, Jacopo Mondi wrote:
> When running the IPA in isolated mode, each side of the IPC boundary
> has an instance of the ControlSerializer class which is used to
> serializer/deserialize controls before transmitting them on the wire.
> 
> The IPAProxyWorker, which creates and manages the process the IPA runs into,

s/into/in/

> does not reset its ControlSerializer upon an IPA::configure() call, while
> the IPAProxy does so, effectively creating a misalignment between the

s/ so//

> two sides of the fence.
> 
> This obviously creates issues as one side of the IPC runs with a
> populated and possibly stale cache of ControlInfoMap references, while the
> other side gets reset every time a new configuration is applied to the
> Camera.
> 
> Fix that by resetting the IPAProxyWorker ControlSerializer on an
> IPA configure() call.
> 
> This change fixes an issue which is easily triggered  by running two

s/  / /

> consecutive capture sessions with the IPA running in isolated mode:
> ERROR Serializer control_serializer.cpp:520 Can't deserialize ControlList: unknown ControlInfoMap
> 
> Fixes: 7832e19a599e ("utils: ipc: add templates for code generation for IPC mechanism")
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

Looks good.

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> ---
>  .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl      | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> index 8a88bd467da7..22cbc15c5eff 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> @@ -79,6 +79,10 @@ public:
>  
>  {% for method in interface_main.methods %}
>  		case {{cmd_enum_name}}::{{method.mojom_name|cap}}: {
> +
> +{%- if method.mojom_name == "configure" %}
> +		controlSerializer_.reset();
> +{%- endif %}
>  		{{proxy_funcs.deserialize_call(method|method_param_inputs, '_ipcMessage.data()', '_ipcMessage.fds()', false, true)|indent(8, true)}}
>  {% for param in method|method_param_outputs %}
>  			{{param|name}} {{param.mojom_name}};
> -- 
> 2.32.0
>