[v5,29/36] ipa: libipa: lsc: Document LscDescriptor::sensorSize usage

27336 diff mbox series

Now that sensorSize_ has been removed from LscPolynomial and is
passed to the Lsc algorithm through LscDescriptor document its intended
purpose.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/libipa/lsc.cpp | 4 ++++
 1 file changed, 4 insertions(+)

Hi Jacopo,

Quoting Jacopo Mondi (2026-07-08 17:51:11)
> Now that sensorSize_ has been removed from LscPolynomial and is
> passed to the Lsc algorithm through LscDescriptor document its intended
> purpose.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  src/ipa/libipa/lsc.cpp | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp
> index 1d2f8eb1b41b..13b1b738106b 100644
> --- a/src/ipa/libipa/lsc.cpp
> +++ b/src/ipa/libipa/lsc.cpp
> @@ -179,6 +179,10 @@ namespace lsc {
>   *          k4: 0.0
>   * \endcode
>   *
> + * The LSC tables or the polynomial definition are generated at tuning time
> + * using an image of known resolution which needs to be specified in
> + * LscDescriptor::sensorSize.
> + *

This is not entirely correct (But also depends on the rectangle
discussion). The generation might happen with something that is smaller
than sensor size. The values are then normalized to the physical
sensor size in the tuning process (we could also carry the tuning crop
rectangle in the tuning file and do the normalization here, but that is
an implementation detail).

What about:
* The LSC polynomial definition is generated at tuning time in relation
* to the full sensor pixel array. This needs to be specified in
* LscDescriptor::sensorSize.

That is also not perfect, but breaks the assumption that sensorSize
equals the tuning image size.

Writing that I realize that it would be more transparent to include the
source crop information in the tuning file. But that should be done on
top.

With the docs change:
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

Best regards,
Stefan


>   * At LscAlgorithm::configure() time the LSC tables are re-sampled on the
>   * sensor's crop rectangle in use to adapt them to the configuration in use for
>   * a streaming session. Polynomial LSC tables support re-sampling and can be
> 
> -- 
> 2.54.0
>

Quoting Stefan Klug (2026-07-14 11:58:26)
> Hi Jacopo,
> 
> Quoting Jacopo Mondi (2026-07-08 17:51:11)
> > Now that sensorSize_ has been removed from LscPolynomial and is
> > passed to the Lsc algorithm through LscDescriptor document its intended
> > purpose.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> >  src/ipa/libipa/lsc.cpp | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/ipa/libipa/lsc.cpp b/src/ipa/libipa/lsc.cpp
> > index 1d2f8eb1b41b..13b1b738106b 100644
> > --- a/src/ipa/libipa/lsc.cpp
> > +++ b/src/ipa/libipa/lsc.cpp
> > @@ -179,6 +179,10 @@ namespace lsc {
> >   *          k4: 0.0
> >   * \endcode
> >   *
> > + * The LSC tables or the polynomial definition are generated at tuning time
> > + * using an image of known resolution which needs to be specified in
> > + * LscDescriptor::sensorSize.
> > + *
> 
> This is not entirely correct (But also depends on the rectangle
> discussion). The generation might happen with something that is smaller
> than sensor size. The values are then normalized to the physical
> sensor size in the tuning process (we could also carry the tuning crop
> rectangle in the tuning file and do the normalization here, but that is
> an implementation detail).
> 
> What about:
> * The LSC polynomial definition is generated at tuning time in relation
> * to the full sensor pixel array. This needs to be specified in
> * LscDescriptor::sensorSize.
> 
> That is also not perfect, but breaks the assumption that sensorSize
> equals the tuning image size.
> 
> Writing that I realize that it would be more transparent to include the
> source crop information in the tuning file. But that should be done on
> top.

I had wondered how we convey the 'range' (or scale?) of the polynomial.
I guess there is currently an implicit assumption it's from "the active
area" or such - but as these definitions seem to be difficult, and so
yes I think adding more detail in the tuning file entries makes sense -
but that's not there currently so not part of this series:


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

> 
> With the docs change:
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> Best regards,
> Stefan
> 
> 
> >   * At LscAlgorithm::configure() time the LSC tables are re-sampled on the
> >   * sensor's crop rectangle in use to adapt them to the configuration in use for
> >   * a streaming session. Polynomial LSC tables support re-sampling and can be
> > 
> > -- 
> > 2.54.0
> >