[PATCH v3 21/23] libtuning: lsc: rkisp1: Do not calculate ratios to green

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Wed, Jul 03, 2024 at 04:17:10PM +0200, Stefan Klug wrote:
> The current LSC algorithm for the rkisp1 just forwards the LSC tables to
> the hardware, so absolute factors are needed and not ratios compared to
> green. Therefore every channel needs to be calculated independently.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  utils/tuning/libtuning/modules/lsc/rkisp1.py | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/tuning/libtuning/modules/lsc/rkisp1.py b/utils/tuning/libtuning/modules/lsc/rkisp1.py
> index 512233aeae9d..56df23ec7b42 100644
> --- a/utils/tuning/libtuning/modules/lsc/rkisp1.py
> +++ b/utils/tuning/libtuning/modules/lsc/rkisp1.py
> @@ -38,8 +38,12 @@ class LSCRkISP1(LSC):
>  
>          # \todo Should these ratio against the average of both greens or just
>          # each green like we've done here?

I think this comment can be dropped.

> -        cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, gr)
> -        cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, gb)

gr and gb are not used anymore, so you can drop them from the lines
above this hunk.

> +
> +        # The LSC tables in the our rkisp1 algorithm represent gains on the
> +        # corresponding channel, not ratios with respect to green, so calculate
> +        # them independently
> +        cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, None)
> +        cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, None)

None is the default value for the last parameter so you can drop it.

I think all of this should become

        # Perform LSC on each colour channel independently. A future enhancement
	# worth investigating would be splitting the luminance and chrominance
	# LSC as done by Raspberry Pi.
        cgr, _ = self._lsc_single_channel(image.channels[lt.Color.GR], image)
        cgb, _ = self._lsc_single_channel(image.channels[lt.Color.GB], image)
        cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image)
        cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image)

You can drop the future enhancement sentence if desired.

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

>  
>          return image.color, cr.flatten(), cb.flatten(), cgr.flatten(), cgb.flatten()
>  

-- 
Regards,

Laurent Pinchart