[PATCH 1/1] libcamera: software_isp: Actually apply black level from tuning data

[Robert Mader]

Wait, sorry, take that back. This commit is *not* needed any more. 
Blacklevels from tuning files now get applied on master.

On 30.11.24 00:07, Robert Mader wrote:
> Quickly retested on top of latest master, including "libcamera: 
> software_isp: Initialize exposure+gain before agc calculations"
>
> Tested-by: Robert Mader <robert.mader at collabora.com>
>
> On 08.11.24 10:43, Milan Zamazal wrote:
>> The black level obtained from the tuning file in software ISP is
>> retrieved in init (because this is the standard algorithm method with
>> access to tuning data) and stored into context.  But the context gets
>> reset in configure and the black level is lost and never applied.
>>
>> Let's store the black level from the tuning file into an algorithm
>> instance variable and put it into the context only later in configure.
>> This is similar to what rkisp1 IPA does with the values obtained from
>> the tuning file.
>>
>> Fixes: 41e3d61c745153779ed5a38634d7266bffd60d61 ("libcamera: 
>> software_isp: Clear IPA context on configure and stop")
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>   src/ipa/simple/algorithms/blc.cpp | 7 +++++--
>>   src/ipa/simple/algorithms/blc.h   | 4 ++++
>>   src/ipa/simple/soft_simple.cpp    | 3 +--
>>   3 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/ipa/simple/algorithms/blc.cpp 
>> b/src/ipa/simple/algorithms/blc.cpp
>> index b4e32fe1c..1d7d370b5 100644
>> --- a/src/ipa/simple/algorithms/blc.cpp
>> +++ b/src/ipa/simple/algorithms/blc.cpp
>> @@ -21,7 +21,8 @@ BlackLevel::BlackLevel()
>>   {
>>   }
>>   -int BlackLevel::init(IPAContext &context, const YamlObject 
>> &tuningData)
>> +int BlackLevel::init([[maybe_unused]] IPAContext &context,
>> +             const YamlObject &tuningData)
>>   {
>>       auto blackLevel = tuningData["blackLevel"].get<int16_t>();
>>       if (blackLevel.has_value()) {
>> @@ -29,7 +30,7 @@ int BlackLevel::init(IPAContext &context, const 
>> YamlObject &tuningData)
>>            * Convert 16 bit values from the tuning file to 8 bit black
>>            * level for the SoftISP.
>>            */
>> -        context.configuration.black.level = blackLevel.value() >> 8;
>> +        definedLevel_ = blackLevel.value() >> 8;
>>       }
>>       return 0;
>>   }
>> @@ -37,6 +38,8 @@ int BlackLevel::init(IPAContext &context, const 
>> YamlObject &tuningData)
>>   int BlackLevel::configure(IPAContext &context,
>>                 [[maybe_unused]] const IPAConfigInfo &configInfo)
>>   {
>> +    if (definedLevel_.has_value())
>> +        context.configuration.black.level = definedLevel_;
>>       context.activeState.blc.level =
>>           context.configuration.black.level.value_or(255);
>>       return 0;
>> diff --git a/src/ipa/simple/algorithms/blc.h 
>> b/src/ipa/simple/algorithms/blc.h
>> index 2cf2a8774..453123d27 100644
>> --- a/src/ipa/simple/algorithms/blc.h
>> +++ b/src/ipa/simple/algorithms/blc.h
>> @@ -7,6 +7,9 @@
>>     #pragma once
>>   +#include <optional>
>> +#include <stdint.h>
>> +
>>   #include "algorithm.h"
>>     namespace libcamera {
>> @@ -29,6 +32,7 @@ public:
>>   private:
>>       uint32_t exposure_;
>>       double gain_;
>> +    std::optional<uint8_t> definedLevel_;
>>   };
>>     } /* namespace ipa::soft::algorithms */
>> diff --git a/src/ipa/simple/soft_simple.cpp 
>> b/src/ipa/simple/soft_simple.cpp
>> index c8ad55a21..825c06757 100644
>> --- a/src/ipa/simple/soft_simple.cpp
>> +++ b/src/ipa/simple/soft_simple.cpp
>> @@ -206,8 +206,7 @@ int IPASoftSimple::configure(const IPAConfigInfo 
>> &configInfo)
>>               (context_.configuration.agc.againMax -
>>                context_.configuration.agc.againMin) /
>>               100.0;
>> -        if (!context_.configuration.black.level.has_value() &&
>> -            camHelper_->blackLevel().has_value()) {
>> +        if (camHelper_->blackLevel().has_value()) {
>>               /*
>>                * The black level from camHelper_ is a 16 bit value, 
>> software ISP
>>                * works with 8 bit pixel values, both regardless of 
>> the actual