[PATCH v6 18/18] libcamera: software_isp: Update black level only on exposure changes

[Kieran Bingham]

Quoting Milan Zamazal (2024-09-06 13:09:27)
> The black level is likely to get updated, if ever, only after exposure
> or gain changes.  Don't compute its possible updates if exposure and
> gain are unchanged.
> 
> It's probably not worth trying to implement something more
> sophisticated.  Better to spend the effort on supporting tuning files.
> 

Fine with me. Indeed I think we just go with static/constant black
levels when we know more information about the sensor.

I'll be curious to see how close this implementation measures against
the 'known' black level values, or measured blacks if we do calibration
of this.

My worry on this automatic detection of black levels is what impact
there would be if the scene is always bright ... but it hasn't been an
issue yet - and I'm sure it will continue to develop so I won't impeded
here:


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>  src/ipa/simple/algorithms/blc.cpp | 9 ++++++++-
>  src/ipa/simple/algorithms/blc.h   | 4 ++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp
> index 1ceae85d..5e0a36d9 100644
> --- a/src/ipa/simple/algorithms/blc.cpp
> +++ b/src/ipa/simple/algorithms/blc.cpp
> @@ -30,10 +30,15 @@ int BlackLevel::init(IPAContext &context,
>  
>  void BlackLevel::process(IPAContext &context,
>                          [[maybe_unused]] const uint32_t frame,
> -                        [[maybe_unused]] IPAFrameContext &frameContext,
> +                        IPAFrameContext &frameContext,
>                          const SwIspStats *stats,
>                          [[maybe_unused]] ControlList &metadata)
>  {
> +       if (frameContext.sensor.exposure == exposure_ &&
> +           frameContext.sensor.gain == gain_) {
> +               return;
> +       }
> +
>         const SwIspStats::Histogram &histogram = stats->yHistogram;
>  
>         /*
> @@ -54,6 +59,8 @@ void BlackLevel::process(IPAContext &context,
>                 if (seen >= pixelThreshold) {
>                         context.activeState.blc.level =
>                                 static_cast<double>(i) / SwIspStats::kYHistogramSize;
> +                       exposure_ = frameContext.sensor.exposure;
> +                       gain_ = frameContext.sensor.gain;
>                         LOG(IPASoftBL, Debug)
>                                 << "Auto-set black level: "
>                                 << i << "/" << SwIspStats::kYHistogramSize
> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h
> index c2140b4b..fc38375e 100644
> --- a/src/ipa/simple/algorithms/blc.h
> +++ b/src/ipa/simple/algorithms/blc.h
> @@ -25,6 +25,10 @@ public:
>                      IPAFrameContext &frameContext,
>                      const SwIspStats *stats,
>                      ControlList &metadata) override;
> +
> +private:
> +       uint32_t exposure_;
> +       double gain_;
>  };
>  
>  } /* namespace ipa::soft::algorithms */
> -- 
> 2.44.1
>