[libcamera-devel,v2,02/11] utils: libtuning: modules: Add ALSC module

17653 diff mbox series

Add an ALSC module to libtuning's collection of modules. It is based on
raspberrypi's ctt's ALSC, but customizable for different lens shading
table sizes, among other things.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v2:
- fix python errors
- fix style
- add SPDX and copyright
- don't call super().validateConfig()
- fix sector splitting
  - i forgot that we have to half the image size for each color channel
- make the base ALSC module into an abstract module, which can be easily
  used by other platforms to derive their own ALSC modules (see rkisp1's
  module later in this series; i think it's quite nice)
---
 .../tuning/libtuning/modules/alsc/__init__.py |  5 ++
 utils/tuning/libtuning/modules/alsc/alsc.py   | 78 +++++++++++++++++++
 2 files changed, 83 insertions(+)
 create mode 100644 utils/tuning/libtuning/modules/alsc/__init__.py
 create mode 100644 utils/tuning/libtuning/modules/alsc/alsc.py

Hi Paul,

Thank you for the patch.

On Sat, Oct 22, 2022 at 03:23:01PM +0900, Paul Elder via libcamera-devel wrote:
> Add an ALSC module to libtuning's collection of modules. It is based on
> raspberrypi's ctt's ALSC, but customizable for different lens shading
> table sizes, among other things.

I think I would name the module LSC, not ALSC, as there's nothing
adaptative here.

> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> Changes in v2:
> - fix python errors
> - fix style
> - add SPDX and copyright
> - don't call super().validateConfig()
> - fix sector splitting
>   - i forgot that we have to half the image size for each color channel
> - make the base ALSC module into an abstract module, which can be easily
>   used by other platforms to derive their own ALSC modules (see rkisp1's
>   module later in this series; i think it's quite nice)
> ---
>  .../tuning/libtuning/modules/alsc/__init__.py |  5 ++
>  utils/tuning/libtuning/modules/alsc/alsc.py   | 78 +++++++++++++++++++
>  2 files changed, 83 insertions(+)
>  create mode 100644 utils/tuning/libtuning/modules/alsc/__init__.py
>  create mode 100644 utils/tuning/libtuning/modules/alsc/alsc.py
> 
> diff --git a/utils/tuning/libtuning/modules/alsc/__init__.py b/utils/tuning/libtuning/modules/alsc/__init__.py
> new file mode 100644
> index 00000000..a8f28923
> --- /dev/null
> +++ b/utils/tuning/libtuning/modules/alsc/__init__.py
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +#
> +# Copyright (C) 2022, Paul Elder <paul.elder@ideasonboard.com>
> +
> +from libtuning.modules.alsc.alsc import ALSC
> diff --git a/utils/tuning/libtuning/modules/alsc/alsc.py b/utils/tuning/libtuning/modules/alsc/alsc.py
> new file mode 100644
> index 00000000..b708b28d
> --- /dev/null
> +++ b/utils/tuning/libtuning/modules/alsc/alsc.py
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: BSD-2-Clause
> +#
> +# Copyright (C) 2019, Raspberry Pi Ltd
> +# Copyright (C) 2022, Paul Elder <paul.elder@ideasonboard.com>
> +
> +from ..module import Module
> +
> +import libtuning as lt
> +import libtuning.utils as utils
> +
> +import numpy as np
> +
> +
> +class ALSC(Module):
> +    type = 'alsc'
> +    hr_name = 'ALSC (Base)'
> +    out_name = 'GenericALSC'
> +
> +    def __init__(self, *,
> +                 debug: list,
> +                 sector_shape: tuple,
> +                 sector_x_gradient: lt.Gradient,
> +                 sector_y_gradient: lt.Gradient,
> +                 sector_average_function: lt.Average,
> +                 smoothing_function: lt.Smoothing):
> +        super().__init__()
> +
> +        if (sector_x_gradient is lt.gradient.Linear and sector_x_remainder is None):

No need for parentheses. Same below.

> +            raise ValueError('sector_x_remainder must be specified if sector_x_gradient is Linear')

sector_x_remainder is not defined. Same for y. The "is
lt.gradient.Linear" test is always false, which is something you
probably want to fix too.

I assume all this will be revisited (I don't think that generic gradient
types will be useful), for now,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +        if (sector_y_gradient is lt.gradient.Linear and sector_y_remainder is None):
> +            raise ValueError('sector_y_remainder must be specified if sector_y_gradient is Linear')
> +
> +        self.debug = debug
> +
> +        self.sector_shape = sector_shape
> +        self.sector_x_gradient = sector_x_gradient
> +        self.sector_y_gradient = sector_y_gradient
> +        self.sector_average_function = sector_average_function
> +
> +        self.smoothing_function = smoothing_function
> +
> +    def _enumerate_alsc_images(self, images):
> +        for image in images:
> +            if image.alsc_only:
> +                yield image
> +
> +    def _get_grid(self, channel, img_w, img_h):
> +        # List of number of pixels in each sector
> +        sectors_x = self.sector_x_gradient.distribute(img_w / 2, self.sector_shape[0])
> +        sectors_y = self.sector_y_gradient.distribute(img_h / 2, self.sector_shape[1])
> +
> +        grid = []
> +
> +        r = 0
> +        for y in sectors_y:
> +            c = 0
> +            for x in sectors_x:
> +                grid.append(self.sector_average_function.average(channel[r:r + y, c:c + x]))
> +                c += x
> +            r += y
> +
> +        return np.array(grid)
> +
> +    def _lsc_single_channel(self, channel: np.array,
> +                            image: lt.Image, green_grid: np.array = None):
> +        grid = self._get_grid(channel, image.w, image.h)
> +        grid -= image.blacklevel_16
> +        if green_grid is None:
> +            table = np.reshape(1 / grid, self.sector_shape[::-1])
> +        else:
> +            table = np.reshape(green_grid / grid, self.sector_shape[::-1])
> +        table = self.smoothing_function.smoothing(table)
> +
> +        if green_grid is None:
> +            table = table / np.min(table)
> +
> +        return table, grid

On Wed, Nov 09, 2022 at 12:29:50AM +0200, Laurent Pinchart wrote:
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Sat, Oct 22, 2022 at 03:23:01PM +0900, Paul Elder via libcamera-devel wrote:
> > Add an ALSC module to libtuning's collection of modules. It is based on
> > raspberrypi's ctt's ALSC, but customizable for different lens shading
> > table sizes, among other things.
> 
> I think I would name the module LSC, not ALSC, as there's nothing
> adaptative here.
> 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > 
> > ---
> > Changes in v2:
> > - fix python errors
> > - fix style
> > - add SPDX and copyright
> > - don't call super().validateConfig()
> > - fix sector splitting
> >   - i forgot that we have to half the image size for each color channel
> > - make the base ALSC module into an abstract module, which can be easily
> >   used by other platforms to derive their own ALSC modules (see rkisp1's
> >   module later in this series; i think it's quite nice)
> > ---
> >  .../tuning/libtuning/modules/alsc/__init__.py |  5 ++
> >  utils/tuning/libtuning/modules/alsc/alsc.py   | 78 +++++++++++++++++++
> >  2 files changed, 83 insertions(+)
> >  create mode 100644 utils/tuning/libtuning/modules/alsc/__init__.py
> >  create mode 100644 utils/tuning/libtuning/modules/alsc/alsc.py
> > 
> > diff --git a/utils/tuning/libtuning/modules/alsc/__init__.py b/utils/tuning/libtuning/modules/alsc/__init__.py
> > new file mode 100644
> > index 00000000..a8f28923
> > --- /dev/null
> > +++ b/utils/tuning/libtuning/modules/alsc/__init__.py
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +#
> > +# Copyright (C) 2022, Paul Elder <paul.elder@ideasonboard.com>
> > +
> > +from libtuning.modules.alsc.alsc import ALSC
> > diff --git a/utils/tuning/libtuning/modules/alsc/alsc.py b/utils/tuning/libtuning/modules/alsc/alsc.py
> > new file mode 100644
> > index 00000000..b708b28d
> > --- /dev/null
> > +++ b/utils/tuning/libtuning/modules/alsc/alsc.py
> > @@ -0,0 +1,78 @@
> > +# SPDX-License-Identifier: BSD-2-Clause
> > +#
> > +# Copyright (C) 2019, Raspberry Pi Ltd
> > +# Copyright (C) 2022, Paul Elder <paul.elder@ideasonboard.com>
> > +
> > +from ..module import Module
> > +
> > +import libtuning as lt
> > +import libtuning.utils as utils
> > +
> > +import numpy as np
> > +
> > +
> > +class ALSC(Module):
> > +    type = 'alsc'
> > +    hr_name = 'ALSC (Base)'
> > +    out_name = 'GenericALSC'
> > +
> > +    def __init__(self, *,
> > +                 debug: list,
> > +                 sector_shape: tuple,
> > +                 sector_x_gradient: lt.Gradient,
> > +                 sector_y_gradient: lt.Gradient,
> > +                 sector_average_function: lt.Average,
> > +                 smoothing_function: lt.Smoothing):
> > +        super().__init__()
> > +
> > +        if (sector_x_gradient is lt.gradient.Linear and sector_x_remainder is None):
> 
> No need for parentheses. Same below.
> 
> > +            raise ValueError('sector_x_remainder must be specified if sector_x_gradient is Linear')
> 
> sector_x_remainder is not defined. Same for y. The "is
> lt.gradient.Linear" test is always false, which is something you
> probably want to fix too.

Actually this (and the next) can be dropped because the remainder is
passed to the Linear constructor, if that's what's passed.

> I assume all this will be revisited (I don't think that generic gradient
> types will be useful), for now,

(I think it might, but I guess we'll see)

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


Thanks,

Paul

> 
> > +
> > +        if (sector_y_gradient is lt.gradient.Linear and sector_y_remainder is None):
> > +            raise ValueError('sector_y_remainder must be specified if sector_y_gradient is Linear')
> > +
> > +        self.debug = debug
> > +
> > +        self.sector_shape = sector_shape
> > +        self.sector_x_gradient = sector_x_gradient
> > +        self.sector_y_gradient = sector_y_gradient
> > +        self.sector_average_function = sector_average_function
> > +
> > +        self.smoothing_function = smoothing_function
> > +
> > +    def _enumerate_alsc_images(self, images):
> > +        for image in images:
> > +            if image.alsc_only:
> > +                yield image
> > +
> > +    def _get_grid(self, channel, img_w, img_h):
> > +        # List of number of pixels in each sector
> > +        sectors_x = self.sector_x_gradient.distribute(img_w / 2, self.sector_shape[0])
> > +        sectors_y = self.sector_y_gradient.distribute(img_h / 2, self.sector_shape[1])
> > +
> > +        grid = []
> > +
> > +        r = 0
> > +        for y in sectors_y:
> > +            c = 0
> > +            for x in sectors_x:
> > +                grid.append(self.sector_average_function.average(channel[r:r + y, c:c + x]))
> > +                c += x
> > +            r += y
> > +
> > +        return np.array(grid)
> > +
> > +    def _lsc_single_channel(self, channel: np.array,
> > +                            image: lt.Image, green_grid: np.array = None):
> > +        grid = self._get_grid(channel, image.w, image.h)
> > +        grid -= image.blacklevel_16
> > +        if green_grid is None:
> > +            table = np.reshape(1 / grid, self.sector_shape[::-1])
> > +        else:
> > +            table = np.reshape(green_grid / grid, self.sector_shape[::-1])
> > +        table = self.smoothing_function.smoothing(table)
> > +
> > +        if green_grid is None:
> > +            table = table / np.min(table)
> > +
> > +        return table, grid
> 
> -- 
> Regards,
> 
> Laurent Pinchart