| Message ID | 20240626072100.55497-6-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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... Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > +} > + > } /* namespace ipa::soft */ > > /*
Hi Umang, thank you for your reviews. Umang Jain <umang.jain@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@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@ideasonboard.com> >> +} >> + >> } /* namespace ipa::soft */ >> /*
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"; +} + } /* namespace ipa::soft */ /*
The Module class is a base class for All IPA modules. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/ipa/simple/soft_simple.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)