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

[Milan Zamazal]

Kieran Bingham <kieran.bingham at ideasonboard.com> writes:

> 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.

In my environment, it's accurate, within the 256 levels resolution.
Sometimes it doesn't reach the right value immediately but adjusts to it
very soon.  This isn't representative but I haven't heard complaints
from anybody so far.  And the aim is not to substitute datasheets or
calibration but to serve as a reasonable fallback.

> My worry on this automatic detection of black levels is what impact
> there would be if the scene is always bright ... 

Sessions start with minimum exposure, so hardly anything can be too
bright initially.  If this wasn't the case then I guess the effect would
be similar to autoexposure adjustments -- bringing the bright scene
towards middle brightness but with more contrast.

> 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
>>