| Message ID | 20240628104828.2928109-22-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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() >
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
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()
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(-)