[PATCH 1/2] libcamera: software_isp: Stop clearing context config and state again

[Milan Zamazal]

Hi Robert,

thank you for the patch.

Robert Mader <robert.mader at collabora.com> writes:

> This partly reverts commit 41e3d61c, removing parts that had unintended
> side effects as the intention for the commit was purely to fix crashes.
>
> Clearing the configuration turned out to be problematic as some values such
> as configuration.black.level only get on initialization and thus were never
> used.

Oh, indeed, I must have done some mistake when testing this. :-(

But rather than not clearing the configuration, a better approach should
be to store the black level from a tuning file to a class variable
rather than to the context.

> Clearing the activeState resulted in additional, arguably undesired churn,
> very noticable when switching back and forth between cameras. 

Do you know what causes this?  Is it related to black level reset or
something else?

> Whether this is desirable is AFAIK a matter of taste/policy and
> shouldn't have been done as part of a crash fix.
>
> Fixes: 41e3d61c ("libcamera: software_isp: Clear IPA context on configure and stop")
> Signed-off-by: Robert Mader <robert.mader at collabora.com>
> ---
>  src/ipa/simple/soft_simple.cpp | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index c8ad55a2..065673dc 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -185,8 +185,6 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo)
>  	const ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second;
>  
>  	/* Clear the IPA context before the streaming session. */
> -	context_.configuration = {};
> -	context_.activeState = {};
>  	context_.frameContexts.clear();
>  
>  	context_.configuration.agc.exposureMin = exposureInfo.min().get<int32_t>();