[v2] ipa: rpi: awb: Remove "fast" parameter

23296 diff mbox

The "fast" parameter has not been used since it first appeared in the
source code. And not only is it not used, but its retrieval from
the configuration since c1597f989654 ("ipa: raspberrypi: Use YamlParser
to replace dependency on boost") has been incorrect. So remove it.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
changes in v2:
  * remove it altogether

v1: https://patchwork.libcamera.org/patch/20076/
---
 src/ipa/rpi/controller/rpi/awb.cpp | 1 -
 src/ipa/rpi/controller/rpi/awb.h   | 1 -
 2 files changed, 2 deletions(-)

--
2.49.0

Quoting Barnabás Pőcze (2025-04-29 08:53:21)
> The "fast" parameter has not been used since it first appeared in the
> source code. And not only is it not used, but its retrieval from
> the configuration since c1597f989654 ("ipa: raspberrypi: Use YamlParser
> to replace dependency on boost") has been incorrect. So remove it.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
> changes in v2:
>   * remove it altogether
> 
> v1: https://patchwork.libcamera.org/patch/20076/
> ---
>  src/ipa/rpi/controller/rpi/awb.cpp | 1 -
>  src/ipa/rpi/controller/rpi/awb.h   | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
> index 8479ae409..365b595ff 100644
> --- a/src/ipa/rpi/controller/rpi/awb.cpp
> +++ b/src/ipa/rpi/controller/rpi/awb.cpp
> @@ -165,7 +165,6 @@ int AwbConfig::read(const libcamera::YamlObject &params)
>                         bayes = false;
>                 }
>         }
> -       fast = params[fast].get<int>(bayes); /* default to fast for Bayesian, otherwise slow */
>         whitepointR = params["whitepoint_r"].get<double>(0.0);
>         whitepointB = params["whitepoint_b"].get<double>(0.0);
>         if (bayes == false)
> diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
> index 86640f8f8..2fb912541 100644
> --- a/src/ipa/rpi/controller/rpi/awb.h
> +++ b/src/ipa/rpi/controller/rpi/awb.h
> @@ -43,7 +43,6 @@ struct AwbConfig {
>         uint16_t startupFrames;
>         unsigned int convergenceFrames; /* approx number of frames to converge */
>         double speed; /* IIR filter speed applied to algorithm results */
> -       bool fast; /* "fast" mode uses a 16x16 rather than 32x32 grid */

Even better..

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>         libcamera::ipa::Pwl ctR; /* function maps CT to r (= R/G) */
>         libcamera::ipa::Pwl ctB; /* function maps CT to b (= B/G) */
>         libcamera::ipa::Pwl ctRInverse; /* inverse of ctR */
> --
> 2.49.0

Hi Barnabas

Thanks for the patch.

On Tue, 29 Apr 2025 at 08:59, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting Barnabás Pőcze (2025-04-29 08:53:21)
> > The "fast" parameter has not been used since it first appeared in the
> > source code. And not only is it not used, but its retrieval from
> > the configuration since c1597f989654 ("ipa: raspberrypi: Use YamlParser
> > to replace dependency on boost") has been incorrect. So remove it.
> >
> > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > ---
> > changes in v2:
> >   * remove it altogether
> >
> > v1: https://patchwork.libcamera.org/patch/20076/
> > ---
> >  src/ipa/rpi/controller/rpi/awb.cpp | 1 -
> >  src/ipa/rpi/controller/rpi/awb.h   | 1 -
> >  2 files changed, 2 deletions(-)
> >
> > diff --git a/src/ipa/rpi/controller/rpi/awb.cpp b/src/ipa/rpi/controller/rpi/awb.cpp
> > index 8479ae409..365b595ff 100644
> > --- a/src/ipa/rpi/controller/rpi/awb.cpp
> > +++ b/src/ipa/rpi/controller/rpi/awb.cpp
> > @@ -165,7 +165,6 @@ int AwbConfig::read(const libcamera::YamlObject &params)
> >                         bayes = false;
> >                 }
> >         }
> > -       fast = params[fast].get<int>(bayes); /* default to fast for Bayesian, otherwise slow */
> >         whitepointR = params["whitepoint_r"].get<double>(0.0);
> >         whitepointB = params["whitepoint_b"].get<double>(0.0);
> >         if (bayes == false)
> > diff --git a/src/ipa/rpi/controller/rpi/awb.h b/src/ipa/rpi/controller/rpi/awb.h
> > index 86640f8f8..2fb912541 100644
> > --- a/src/ipa/rpi/controller/rpi/awb.h
> > +++ b/src/ipa/rpi/controller/rpi/awb.h
> > @@ -43,7 +43,6 @@ struct AwbConfig {
> >         uint16_t startupFrames;
> >         unsigned int convergenceFrames; /* approx number of frames to converge */
> >         double speed; /* IIR filter speed applied to algorithm results */
> > -       bool fast; /* "fast" mode uses a 16x16 rather than 32x32 grid */
>
> Even better..
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Well, I honestly have no recollection that we ever turned the 32x32
grid into a 16x16 one, but I suppose the evidence is there and it
would have saved some computation. for whatever that was worth (which
can't have been much, because the code was obviously deleted a long
time ago).

So yes, let's remove it.

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks for spotting it!

David

>
> >         libcamera::ipa::Pwl ctR; /* function maps CT to r (= R/G) */
> >         libcamera::ipa::Pwl ctB; /* function maps CT to b (= B/G) */
> >         libcamera::ipa::Pwl ctRInverse; /* inverse of ctR */
> > --
> > 2.49.0