[PATCH] libcamera: software_isp: Reset stored exposure in black level

[Kieran Bingham]

Quoting Milan Zamazal (2025-03-17 11:26:58)
> Automatic black level setting in software ISP updates the determined
> black level value when exposure or gain change.  It stores the last
> exposure and gain values to detect the change.
> 
> BlackLevel::configure() resets the stored black level value but not the
> exposure and gain values.  This can prevent updating the black value and
> cause bad image output e.g. after suspending and resuming a camera, if
> exposure and gain remain unchanged.
> 
> Let's reset the stored exposure and gain values in
> BlackLevel::configure() to fix the problem.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=259
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  src/ipa/simple/algorithms/blc.cpp | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp
> index 1d7d370b..14cf31bf 100644
> --- a/src/ipa/simple/algorithms/blc.cpp
> +++ b/src/ipa/simple/algorithms/blc.cpp
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
>  /*
> - * Copyright (C) 2024, Red Hat Inc.
> + * Copyright (C) 2024-2025, Red Hat Inc.
>   *
>   * Black level handling
>   */
> @@ -38,6 +38,9 @@ int BlackLevel::init([[maybe_unused]] IPAContext &context,
>  int BlackLevel::configure(IPAContext &context,
>                           [[maybe_unused]] const IPAConfigInfo &configInfo)
>  {
> +       exposure_ = 0;
> +       gain_ = 0;
> +

I wonder if we should try to make sure algorithms do not store private
state, and only use storage in the IPAContext so we can ensure we can
zero/reset the context on configure for all algos?

For now this seems reasonable to re-initialise.

Just to check though - is this sufficient for stop/start cycles?

If someone does:

 camera->configure()
 camera->start()
  ....
 camera->stop()
 camera->start()
   ... Will there be the same bug here? Or will it be ok ?

Or should these be re-initialised at start() ?

Perhaps it's ok - as the issue is that the black level was reset here in
configure, so the flag that was also checking when to recalibrate also
needs to be reset here.

A comment to state that clearly might be helpful here somewhere that the
exposure and gain are used to determine when to recalibrate the black
levels ...


>         if (definedLevel_.has_value())
>                 context.configuration.black.level = definedLevel_;
>         context.activeState.blc.level =
> -- 
> 2.48.1
>