[v2,21/25] libtuning: lsc: rkisp1: Do not calculate ratios to green
diff mbox series

Message ID 20240628104828.2928109-22-stefan.klug@ideasonboard.com
State Superseded
Headers show
Series
  • Add ccm calibration to libtuning
Related show

Commit Message

Stefan Klug June 28, 2024, 10:47 a.m. UTC
The hardware in the imx8mp treats the lsc tables as absolute factors and
not as ratios compared to green. Therefore every channel must calculated
independently.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 utils/tuning/libtuning/modules/lsc/rkisp1.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart June 29, 2024, 12:22 a.m. UTC | #1
Hi Stefan,

Thank you for the patch.

On Fri, Jun 28, 2024 at 12:47:14PM +0200, Stefan Klug wrote:
> The hardware in the imx8mp treats the lsc tables as absolute factors and

s/imx8mp/i.MX8MP/

but maybe you should mention rkisp1 here as that's not specific to the
version in the i.MX8MP.

s/lsc/LSC/

> not as ratios compared to green. Therefore every channel must calculated
> independently.

I don't think this is dictated by the hardware, but by the way the LSC
algorithm is implemented in the IPA. Raspberry Pi has a more advanced
LSC algorithm that splits processing in luminance LSC and chroma LSC,
and so needs to handle tuning accordingly. The IPA module calculates R,
G and B LSC tables at runtime, and the hardware operates on the colour
channels independently.

The RkISP1 implementation is (currently) much simpler and handles the
three colour channels independently. That's why, as far as I understand,
we shouldn't base the computation on ratios.

> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  utils/tuning/libtuning/modules/lsc/rkisp1.py | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/tuning/libtuning/modules/lsc/rkisp1.py b/utils/tuning/libtuning/modules/lsc/rkisp1.py
> index 5874f10c936f..a8ba454a7139 100644
> --- a/utils/tuning/libtuning/modules/lsc/rkisp1.py
> +++ b/utils/tuning/libtuning/modules/lsc/rkisp1.py
> @@ -38,8 +38,11 @@ class LSCRkISP1(LSC):
>  
>          # \todo Should these ratio against the average of both greens or just
>          # each green like we've done here?
> -        cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, gr)
> -        cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, gb)
> +
> +        # the lsc tables in the rkisp1 are gains on the corresponding channel,

s/the lsc/The LSC/

> +        # 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)
>  
>          return image.color, cr.flatten(), cb.flatten(), cgr.flatten(), cgb.flatten()
>
Stefan Klug July 2, 2024, 10:25 a.m. UTC | #2
Hi Laurent,

On Sat, Jun 29, 2024 at 03:22:33AM +0300, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thank you for the patch.
> 
> On Fri, Jun 28, 2024 at 12:47:14PM +0200, Stefan Klug wrote:
> > The hardware in the imx8mp treats the lsc tables as absolute factors and
> 
> s/imx8mp/i.MX8MP/
> 
> but maybe you should mention rkisp1 here as that's not specific to the
> version in the i.MX8MP.
> 
> s/lsc/LSC/
> 
> > not as ratios compared to green. Therefore every channel must calculated
> > independently.
> 
> I don't think this is dictated by the hardware, but by the way the LSC
> algorithm is implemented in the IPA. Raspberry Pi has a more advanced
> LSC algorithm that splits processing in luminance LSC and chroma LSC,
> and so needs to handle tuning accordingly. The IPA module calculates R,
> G and B LSC tables at runtime, and the hardware operates on the colour
> channels independently.
> 
> The RkISP1 implementation is (currently) much simpler and handles the
> three colour channels independently. That's why, as far as I understand,
> we shouldn't base the computation on ratios.

Thanks for the clarification. I've updated the comments/commit message
accordingly.

Regards,
Stefan

> 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >  utils/tuning/libtuning/modules/lsc/rkisp1.py | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/utils/tuning/libtuning/modules/lsc/rkisp1.py b/utils/tuning/libtuning/modules/lsc/rkisp1.py
> > index 5874f10c936f..a8ba454a7139 100644
> > --- a/utils/tuning/libtuning/modules/lsc/rkisp1.py
> > +++ b/utils/tuning/libtuning/modules/lsc/rkisp1.py
> > @@ -38,8 +38,11 @@ class LSCRkISP1(LSC):
> >  
> >          # \todo Should these ratio against the average of both greens or just
> >          # each green like we've done here?
> > -        cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, gr)
> > -        cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, gb)
> > +
> > +        # the lsc tables in the rkisp1 are gains on the corresponding channel,
> 
> s/the lsc/The LSC/
> 
> > +        # 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)
> >  
> >          return image.color, cr.flatten(), cb.flatten(), cgr.flatten(), cgb.flatten()
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series

diff --git a/utils/tuning/libtuning/modules/lsc/rkisp1.py b/utils/tuning/libtuning/modules/lsc/rkisp1.py
index 5874f10c936f..a8ba454a7139 100644
--- a/utils/tuning/libtuning/modules/lsc/rkisp1.py
+++ b/utils/tuning/libtuning/modules/lsc/rkisp1.py
@@ -38,8 +38,11 @@  class LSCRkISP1(LSC):
 
         # \todo Should these ratio against the average of both greens or just
         # each green like we've done here?
-        cr, _ = self._lsc_single_channel(image.channels[lt.Color.R], image, gr)
-        cb, _ = self._lsc_single_channel(image.channels[lt.Color.B], image, gb)
+
+        # the lsc tables in the rkisp1 are 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)
 
         return image.color, cr.flatten(), cb.flatten(), cgr.flatten(), cgb.flatten()