[v5,28/36] ipa: libipa: lsc_polynomial: Remove sensorSize_

27338 diff mbox series

The sensorSize_ field is only used during the init() function and is
provided to LscPolynomial using the LscDescriptor type.

There is no need to store it as a class member and to pass it in
at class construction time.

Remove it and move the \todo comment in the LscDescriptor type
documentation.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/ipa/libipa/lsc.cpp            | 6 +-----
 src/ipa/libipa/lsc_base.cpp       | 3 +++
 src/ipa/libipa/lsc_polynomial.cpp | 7 -------
 src/ipa/libipa/lsc_polynomial.h   | 6 ------
 4 files changed, 4 insertions(+), 18 deletions(-)

Hi Jacopo,

Quoting Jacopo Mondi (2026-07-08 17:51:10)
> The sensorSize_ field is only used during the init() function and is
> provided to LscPolynomial using the LscDescriptor type.
> 
> There is no need to store it as a class member and to pass it in
> at class construction time.
> 
> Remove it and move the \todo comment in the LscDescriptor type
> documentation.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/ipa/libipa/lsc.cpp            | 6 +-----
>  src/ipa/libipa/lsc_base.cpp       | 3 +++
>  src/ipa/libipa/lsc_polynomial.cpp | 7 -------
>  src/ipa/libipa/lsc_polynomial.h   | 6 ------
>  4 files changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp
> index 26c3e9968072..1d2f8eb1b41b 100644
> --- a/src/ipa/libipa/lsc.cpp
> +++ b/src/ipa/libipa/lsc.cpp
> @@ -215,11 +215,7 @@ int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &control
>                 impl_ = std::make_unique<LscTable>();
>                 LOG(Lsc, Debug) << "Using table-based Lsc";
>         } else if (type == "polynomial") {
> -               /*
> -                * \todo: Most likely the reference frame should be native_size.
> -                * Let's wait how the internal discussions progress.
> -                */
> -               impl_ = std::make_unique<LscPolynomial>(descriptor.sensorSize);
> +               impl_ = std::make_unique<LscPolynomial>();

I was looking for the code that added the new sensorSize in this patch
and then understood that the new sensorSize was silently introduced in
"[v5,26/36] ipa: libipa: lsc: Make Components a map<>".  Maybe that is
worth a comment on that patch.

The overall result is fine.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

Best regards,
Stefan

>                 polynomial_ = true;
>                 LOG(Lsc, Debug) << "Using polynomial Lsc";
>         } else {
> diff --git a/src/ipa/libipa/lsc_base.cpp b/src/ipa/libipa/lsc_base.cpp
> index 919622c6b951..adc09c15752a 100644
> --- a/src/ipa/libipa/lsc_base.cpp
> +++ b/src/ipa/libipa/lsc_base.cpp
> @@ -78,6 +78,9 @@ void Interpolator<lsc::Components>::
>   * \var LscDescriptor::sensorSize
>   * \brief The physical sensor size. This is the largest frame size used to
>   * generate the LSC table. Only used by the polynomial LSC algorithm
> + *
> + * \todo: Most likely the reference frame should be native_size.
> + * Let's wait how the internal discussions progress.
>   */
>  
>  /**
> diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> index e17ba85448be..c61c85fbf2b2 100644
> --- a/src/ipa/libipa/lsc_polynomial.cpp
> +++ b/src/ipa/libipa/lsc_polynomial.cpp
> @@ -123,13 +123,6 @@ void Polynomial::setReferenceImageSize(const Size &size)
>   * \sa LscImplementation
>   */
>  
> -/**
> - * \fn LscPolynomial::LscPolynomial
> - * \param[in] sensorSize The physical sensor size
> - *
> - * Construct an LscPolynomial
> - */
> -
>  /**
>   * \brief Parse polynomial LSC data
>   * \param[in] sets The tuning file content
> diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> index 2a26db499d68..6d19e46a50b7 100644
> --- a/src/ipa/libipa/lsc_polynomial.h
> +++ b/src/ipa/libipa/lsc_polynomial.h
> @@ -57,11 +57,6 @@ private:
>         using PolynomialComponentsMap = std::map<unsigned int, PolynomialComponents>;
>  
>  public:
> -       LscPolynomial(const Size &sensorSize)
> -               : sensorSize_(sensorSize)
> -       {
> -       }
> -
>         int parseLscData(const ValueNode &sets,
>                          const LscDescriptor &descriptor) override;
>  
> @@ -75,7 +70,6 @@ private:
>                                                Span<const double> yPositions,
>                                                const Rectangle &cropRectangle);
>         PolynomialComponentsMap lscData_;
> -       Size sensorSize_;
>  };
>  
>  } /* namespace ipa */
> 
> -- 
> 2.54.0
>

Hi Stefan

On Tue, Jul 14, 2026 at 09:11:25AM +0200, Stefan Klug wrote:
> Hi Jacopo,
>
> Quoting Jacopo Mondi (2026-07-08 17:51:10)
> > The sensorSize_ field is only used during the init() function and is
> > provided to LscPolynomial using the LscDescriptor type.
> >
> > There is no need to store it as a class member and to pass it in
> > at class construction time.
> >
> > Remove it and move the \todo comment in the LscDescriptor type
> > documentation.
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  src/ipa/libipa/lsc.cpp            | 6 +-----
> >  src/ipa/libipa/lsc_base.cpp       | 3 +++
> >  src/ipa/libipa/lsc_polynomial.cpp | 7 -------
> >  src/ipa/libipa/lsc_polynomial.h   | 6 ------
> >  4 files changed, 4 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp
> > index 26c3e9968072..1d2f8eb1b41b 100644
> > --- a/src/ipa/libipa/lsc.cpp
> > +++ b/src/ipa/libipa/lsc.cpp
> > @@ -215,11 +215,7 @@ int LscAlgorithm::init(const ValueNode &tuningData, ControlInfoMap::Map &control
> >                 impl_ = std::make_unique<LscTable>();
> >                 LOG(Lsc, Debug) << "Using table-based Lsc";
> >         } else if (type == "polynomial") {
> > -               /*
> > -                * \todo: Most likely the reference frame should be native_size.
> > -                * Let's wait how the internal discussions progress.
> > -                */
> > -               impl_ = std::make_unique<LscPolynomial>(descriptor.sensorSize);
> > +               impl_ = std::make_unique<LscPolynomial>();
>
> I was looking for the code that added the new sensorSize in this patch
> and then understood that the new sensorSize was silently introduced in
> "[v5,26/36] ipa: libipa: lsc: Make Components a map<>".  Maybe that is

The LscPolynomial class always had sensorSize provided to the class
constructor. It comes from the LscPolynomialLoader

       LscPolynomialLoader(const Size &sensorSize)
               : sensorSize_(sensorSize)
       {
       }

> worth a comment on that patch.
>
> The overall result is fine.
>
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

Thanks
  j
>
> Best regards,
> Stefan
>
> >                 polynomial_ = true;
> >                 LOG(Lsc, Debug) << "Using polynomial Lsc";
> >         } else {
> > diff --git a/src/ipa/libipa/lsc_base.cpp b/src/ipa/libipa/lsc_base.cpp
> > index 919622c6b951..adc09c15752a 100644
> > --- a/src/ipa/libipa/lsc_base.cpp
> > +++ b/src/ipa/libipa/lsc_base.cpp
> > @@ -78,6 +78,9 @@ void Interpolator<lsc::Components>::
> >   * \var LscDescriptor::sensorSize
> >   * \brief The physical sensor size. This is the largest frame size used to
> >   * generate the LSC table. Only used by the polynomial LSC algorithm
> > + *
> > + * \todo: Most likely the reference frame should be native_size.
> > + * Let's wait how the internal discussions progress.
> >   */
> >
> >  /**
> > diff --git a/src/ipa/libipa/lsc_polynomial.cpp b/src/ipa/libipa/lsc_polynomial.cpp
> > index e17ba85448be..c61c85fbf2b2 100644
> > --- a/src/ipa/libipa/lsc_polynomial.cpp
> > +++ b/src/ipa/libipa/lsc_polynomial.cpp
> > @@ -123,13 +123,6 @@ void Polynomial::setReferenceImageSize(const Size &size)
> >   * \sa LscImplementation
> >   */
> >
> > -/**
> > - * \fn LscPolynomial::LscPolynomial
> > - * \param[in] sensorSize The physical sensor size
> > - *
> > - * Construct an LscPolynomial
> > - */
> > -
> >  /**
> >   * \brief Parse polynomial LSC data
> >   * \param[in] sets The tuning file content
> > diff --git a/src/ipa/libipa/lsc_polynomial.h b/src/ipa/libipa/lsc_polynomial.h
> > index 2a26db499d68..6d19e46a50b7 100644
> > --- a/src/ipa/libipa/lsc_polynomial.h
> > +++ b/src/ipa/libipa/lsc_polynomial.h
> > @@ -57,11 +57,6 @@ private:
> >         using PolynomialComponentsMap = std::map<unsigned int, PolynomialComponents>;
> >
> >  public:
> > -       LscPolynomial(const Size &sensorSize)
> > -               : sensorSize_(sensorSize)
> > -       {
> > -       }
> > -
> >         int parseLscData(const ValueNode &sets,
> >                          const LscDescriptor &descriptor) override;
> >
> > @@ -75,7 +70,6 @@ private:
> >                                                Span<const double> yPositions,
> >                                                const Rectangle &cropRectangle);
> >         PolynomialComponentsMap lscData_;
> > -       Size sensorSize_;
> >  };
> >
> >  } /* namespace ipa */
> >
> > --
> > 2.54.0
> >