[libcamera-devel] [PATCH 2/2] utils: ipc: proxy: Always reset ControlSerializer during IPA configure

[Umang Jain]

Hi Paul,

On 7/8/21 3:35 PM, paul.elder at ideasonboard.com wrote:
> Hi Umang,
>
> On Thu, Jul 08, 2021 at 01:51:45PM +0530, Umang Jain wrote:
>> ControlSerializer should be reset during IPA (re)configuration,
>> so that it doesn't look up stale deserialized cache built from
>> consecutive previous runs. This is already recommended in
>> ControlSerializer docs but the implementation seems missing.
>>
>> The stale cache lookup seems to the core issue with Bug #58.
> Oh, I didn't notice that this was the issue. If it is, then this patch
> ought to fix the issue indeed.
>
>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=58
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>>   .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl  | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> index 5a64fe9c..3d27067a 100644
>> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
>> @@ -125,6 +125,10 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data)
>>   {% for method in interface_main.methods %}
>>   {{proxy_funcs.func_sig(proxy_name, method)}}
>>   {
>> +{%- if method.mojom_name == "configure" %}
>> +	controlSerializer_.reset();
>> +{%- endif %}
>> +
> I don't think we need to run this when we're not isolated, so probably
> moving this to below
>
> {{proxy_funcs.func_sig(proxy_name, method, "IPC")}}
>
> is better.

Okay, as I understand from broad level, no 
serialization/de-serialization happens during non-isolated mode,

so ControlSerializer is entirely out the picture in that case.

>
> I haven't tested it, but with the suggested change, looks good to me.
>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
>
>>   	if (isolate_)
>>   		{{"return " if method|method_return_value != "void"}}{{method.mojom_name}}IPC(
>>   {%- for param in method|method_param_names -%}
>> -- 
>> 2.31.1
>>