| Message ID | 20260918120949.191668-5-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2026-09-18 13:09:32) > Log the rgb means immediately upon enter `process()`, and also log > whether it is valid or not. And also log the determined colour temperature.... Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/libipa/awb.cpp | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp > index 0e312690b8..8d465a4f89 100644 > --- a/src/ipa/libipa/awb.cpp > +++ b/src/ipa/libipa/awb.cpp > @@ -367,7 +367,13 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, > const AwbStats &stats, unsigned int lux, > ControlList &metadata) > { > - if (!stats.valid()) > + const bool valid = stats.valid(); > + > + LOG(Awb, Debug) << std::showpoint > + << "means: " << stats.rgbMeans() > + << " (" << (!valid ? "in" : "") << "valid)"; > + > + if (!valid) > return; > > auto awbResult = impl_->calculateAwb(stats, lux, { currentMode_->ctLo, > @@ -394,8 +400,8 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, > static_cast<float>(frameContext.gains.b()) }); > metadata.set(controls::ColourTemperature, frameContext.colourTemperature); > > - LOG(Awb, Debug) << std::showpoint << "Means " << stats.rgbMeans() > - << ", gains " << state.automatic.gains > + LOG(Awb, Debug) << std::showpoint > + << "gains " << state.automatic.gains > << ", temp " << state.automatic.colourTemperature << "K"; > } > > -- > 2.55.0 >
Hi Barnabás On Fri, Sep 18, 2026 at 02:09:32PM +0200, Barnabás Pőcze wrote: > Log the rgb means immediately upon enter `process()`, and also log > whether it is valid or not. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/libipa/awb.cpp | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp > index 0e312690b8..8d465a4f89 100644 > --- a/src/ipa/libipa/awb.cpp > +++ b/src/ipa/libipa/awb.cpp > @@ -367,7 +367,13 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, > const AwbStats &stats, unsigned int lux, > ControlList &metadata) > { > - if (!stats.valid()) > + const bool valid = stats.valid(); > + > + LOG(Awb, Debug) << std::showpoint > + << "means: " << stats.rgbMeans() > + << " (" << (!valid ? "in" : "") << "valid)"; > + > + if (!valid) > return; Is there any value in logging invalid means ? If not, you can move the printout after the if (!valid) check and add a warning if the condition evaluates to true. > > auto awbResult = impl_->calculateAwb(stats, lux, { currentMode_->ctLo, > @@ -394,8 +400,8 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, > static_cast<float>(frameContext.gains.b()) }); > metadata.set(controls::ColourTemperature, frameContext.colourTemperature); > > - LOG(Awb, Debug) << std::showpoint << "Means " << stats.rgbMeans() > - << ", gains " << state.automatic.gains > + LOG(Awb, Debug) << std::showpoint > + << "gains " << state.automatic.gains > << ", temp " << state.automatic.colourTemperature << "K"; > } > > -- > 2.55.0 >
2026. 09. 23. 17:05 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Fri, Sep 18, 2026 at 02:09:32PM +0200, Barnabás Pőcze wrote: >> Log the rgb means immediately upon enter `process()`, and also log >> whether it is valid or not. >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/ipa/libipa/awb.cpp | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp >> index 0e312690b8..8d465a4f89 100644 >> --- a/src/ipa/libipa/awb.cpp >> +++ b/src/ipa/libipa/awb.cpp >> @@ -367,7 +367,13 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, >> const AwbStats &stats, unsigned int lux, >> ControlList &metadata) >> { >> - if (!stats.valid()) >> + const bool valid = stats.valid(); >> + >> + LOG(Awb, Debug) << std::showpoint >> + << "means: " << stats.rgbMeans() >> + << " (" << (!valid ? "in" : "") << "valid)"; >> + >> + if (!valid) >> return; > > Is there any value in logging invalid means ? > > If not, you can move the printout after the if (!valid) check and add > a warning if the condition evaluates to true. I found it useful during debugging. I think there are "valid" scenes where the rgb means is invalid (e.g. going from very dark to very bright), so I believe a warning could potentially generate a lot of noise, unless the warning is only printed e.g. after N frames without valid data. > >> >> auto awbResult = impl_->calculateAwb(stats, lux, { currentMode_->ctLo, >> @@ -394,8 +400,8 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, >> static_cast<float>(frameContext.gains.b()) }); >> metadata.set(controls::ColourTemperature, frameContext.colourTemperature); >> >> - LOG(Awb, Debug) << std::showpoint << "Means " << stats.rgbMeans() >> - << ", gains " << state.automatic.gains >> + LOG(Awb, Debug) << std::showpoint >> + << "gains " << state.automatic.gains >> << ", temp " << state.automatic.colourTemperature << "K"; >> } >> >> -- >> 2.55.0 >>
Hi Barnabás On Wed, Sep 23, 2026 at 05:34:53PM +0200, Barnabás Pőcze wrote: > 2026. 09. 23. 17:05 keltezéssel, Jacopo Mondi írta: > > Hi Barnabás > > > > On Fri, Sep 18, 2026 at 02:09:32PM +0200, Barnabás Pőcze wrote: > > > Log the rgb means immediately upon enter `process()`, and also log > > > whether it is valid or not. > > > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > --- > > > src/ipa/libipa/awb.cpp | 12 +++++++++--- > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp > > > index 0e312690b8..8d465a4f89 100644 > > > --- a/src/ipa/libipa/awb.cpp > > > +++ b/src/ipa/libipa/awb.cpp > > > @@ -367,7 +367,13 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, > > > const AwbStats &stats, unsigned int lux, > > > ControlList &metadata) > > > { > > > - if (!stats.valid()) > > > + const bool valid = stats.valid(); > > > + > > > + LOG(Awb, Debug) << std::showpoint > > > + << "means: " << stats.rgbMeans() > > > + << " (" << (!valid ? "in" : "") << "valid)"; > > > + > > > + if (!valid) > > > return; > > > > Is there any value in logging invalid means ? > > > > If not, you can move the printout after the if (!valid) check and add > > a warning if the condition evaluates to true. > > I found it useful during debugging. I think there are "valid" scenes where > the rgb means is invalid (e.g. going from very dark to very bright), so I > believe a warning could potentially generate a lot of noise, unless the warning > is only printed e.g. after N frames without valid data. > Ok, I don't have a strong opinion here but the distinction between "invalid"/"valid" might be hard to read. You might consider only printing "invalid" and omitting "valid". A minor Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > > > > auto awbResult = impl_->calculateAwb(stats, lux, { currentMode_->ctLo, > > > @@ -394,8 +400,8 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, > > > static_cast<float>(frameContext.gains.b()) }); > > > metadata.set(controls::ColourTemperature, frameContext.colourTemperature); > > > > > > - LOG(Awb, Debug) << std::showpoint << "Means " << stats.rgbMeans() > > > - << ", gains " << state.automatic.gains > > > + LOG(Awb, Debug) << std::showpoint > > > + << "gains " << state.automatic.gains > > > << ", temp " << state.automatic.colourTemperature << "K"; > > > } > > > > > > -- > > > 2.55.0 > > > >
diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp index 0e312690b8..8d465a4f89 100644 --- a/src/ipa/libipa/awb.cpp +++ b/src/ipa/libipa/awb.cpp @@ -367,7 +367,13 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, const AwbStats &stats, unsigned int lux, ControlList &metadata) { - if (!stats.valid()) + const bool valid = stats.valid(); + + LOG(Awb, Debug) << std::showpoint + << "means: " << stats.rgbMeans() + << " (" << (!valid ? "in" : "") << "valid)"; + + if (!valid) return; auto awbResult = impl_->calculateAwb(stats, lux, { currentMode_->ctLo, @@ -394,8 +400,8 @@ void AwbAlgorithmBase::process(awb::ActiveState &state, static_cast<float>(frameContext.gains.b()) }); metadata.set(controls::ColourTemperature, frameContext.colourTemperature); - LOG(Awb, Debug) << std::showpoint << "Means " << stats.rgbMeans() - << ", gains " << state.automatic.gains + LOG(Awb, Debug) << std::showpoint + << "gains " << state.automatic.gains << ", temp " << state.automatic.colourTemperature << "K"; }
Log the rgb means immediately upon enter `process()`, and also log whether it is valid or not. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/libipa/awb.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)