[PATCH 05/19] libcamera: software_isp: Let IPASoftSimple inherit Module

[Milan Zamazal]

Hi Umang,

thank you for your reviews.

Umang Jain <umang.jain at ideasonboard.com> writes:

> Hi Milan
>
> Thank you for the patch.
>
> On 26/06/24 12:50 pm, Milan Zamazal wrote:
>> The Module class is a base class for All IPA modules.
>
> nit: "In addition, implement logPrefix() for the softIPA."
>>
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>   src/ipa/simple/soft_simple.cpp | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
>> index b7746ce0..04f283c8 100644
>> --- a/src/ipa/simple/soft_simple.cpp
>> +++ b/src/ipa/simple/soft_simple.cpp
>> @@ -30,6 +30,7 @@
>>   #include "libipa/camera_sensor_helper.h"
>>     #include "black_level.h"
>> +#include "module.h"
>>     namespace libcamera {
>>   LOG_DEFINE_CATEGORY(IPASoft)
>> @@ -54,7 +55,7 @@ static constexpr float kExposureOptimal = kExposureBinsCount / 2.0;
>>    */
>>   static constexpr float kExposureSatisfactory = 0.2;
>>   -class IPASoftSimple : public ipa::soft::IPASoftInterface
>> +class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module
>>   {
>>   public:
>>   	IPASoftSimple()
>> @@ -76,6 +77,9 @@ public:
>>     	void processStats(const ControlList &sensorControls) override;
>>   +protected:
>> +	std::string logPrefix() const override;
>> +
>>   private:
>>   	void updateExposure(double exposureMSV);
>>   @@ -421,6 +425,11 @@ void IPASoftSimple::updateExposure(double exposureMSV)
>>   	again_ = std::clamp(again_, againMin_, againMax_);
>>   }
>>   +std::string IPASoftSimple::logPrefix() const
>> +{
>> +	return "soft";
>
> my preference here would "IPASoft" but let's see what others think...

No other opinions and I think "IPASoft" is indeed clearer, although the
other pipelines use non-"IPA" prefixes, so I'll use it.

> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>> +}
>> +
>>   } /* namespace ipa::soft */
>>     /*