| Message ID | 20250414173244.118560-1-mzamazal@redhat.com |
|---|---|
| State | Accepted |
| Commit | 026ed6273969d931d38876345e01de626def8b07 |
| Headers | show |
| Series |
|
| Related | show |
Hi Milan, Thank you for the patch. On Mon, Apr 14, 2025 at 07:32:44PM +0200, Milan Zamazal wrote: > A colour correction matrix (CCM) is applied like this to an RGB pixel > vector P: > > CCM * P > > White balance must be applied before CCM. If CCM is used, software ISP > makes a combined matrix by multiplying the CCM by a white balance gains > CCM-like matrix (WBG). The multiplication should be as follows to do it > in the correct order: > > CCM * (WBG * P) = (CCM * WBG) * P > > The multiplication order in Lut software ISP algorithm is reversed, > resulting in colour casts. Let's fix the order. Fixes: e2b4000dc9cc ("libcamera: software_isp: Apply CCM in debayering") > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/simple/algorithms/lut.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp > index e8638f27..d1d5f727 100644 > --- a/src/ipa/simple/algorithms/lut.cpp > +++ b/src/ipa/simple/algorithms/lut.cpp > @@ -122,7 +122,7 @@ void Lut::prepare(IPAContext &context, > Matrix<float, 3, 3> gainCcm = { { gains.r(), 0, 0, > 0, gains.g(), 0, > 0, 0, gains.b() } }; > - auto ccm = gainCcm * context.activeState.ccm.ccm; > + auto ccm = context.activeState.ccm.ccm * gainCcm; > auto &red = params->redCcm; > auto &green = params->greenCcm; > auto &blue = params->blueCcm;
Quoting Laurent Pinchart (2025-04-14 19:20:57) > Hi Milan, > > Thank you for the patch. > > On Mon, Apr 14, 2025 at 07:32:44PM +0200, Milan Zamazal wrote: > > A colour correction matrix (CCM) is applied like this to an RGB pixel > > vector P: > > > > CCM * P > > > > White balance must be applied before CCM. If CCM is used, software ISP > > makes a combined matrix by multiplying the CCM by a white balance gains > > CCM-like matrix (WBG). The multiplication should be as follows to do it > > in the correct order: > > > > CCM * (WBG * P) = (CCM * WBG) * P > > > > The multiplication order in Lut software ISP algorithm is reversed, > > resulting in colour casts. Let's fix the order. > > Fixes: e2b4000dc9cc ("libcamera: software_isp: Apply CCM in debayering") > > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> This looks great to me. Fixes the issue I reported with testing the Saturation control which now gives clear greyscale values on 0.0 saturation. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > src/ipa/simple/algorithms/lut.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp > > index e8638f27..d1d5f727 100644 > > --- a/src/ipa/simple/algorithms/lut.cpp > > +++ b/src/ipa/simple/algorithms/lut.cpp > > @@ -122,7 +122,7 @@ void Lut::prepare(IPAContext &context, > > Matrix<float, 3, 3> gainCcm = { { gains.r(), 0, 0, > > 0, gains.g(), 0, > > 0, 0, gains.b() } }; > > - auto ccm = gainCcm * context.activeState.ccm.ccm; > > + auto ccm = context.activeState.ccm.ccm * gainCcm; > > auto &red = params->redCcm; > > auto &green = params->greenCcm; > > auto &blue = params->blueCcm; > > -- > Regards, > > Laurent Pinchart
diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp index e8638f27..d1d5f727 100644 --- a/src/ipa/simple/algorithms/lut.cpp +++ b/src/ipa/simple/algorithms/lut.cpp @@ -122,7 +122,7 @@ void Lut::prepare(IPAContext &context, Matrix<float, 3, 3> gainCcm = { { gains.r(), 0, 0, 0, gains.g(), 0, 0, 0, gains.b() } }; - auto ccm = gainCcm * context.activeState.ccm.ccm; + auto ccm = context.activeState.ccm.ccm * gainCcm; auto &red = params->redCcm; auto &green = params->greenCcm; auto &blue = params->blueCcm;
A colour correction matrix (CCM) is applied like this to an RGB pixel vector P: CCM * P White balance must be applied before CCM. If CCM is used, software ISP makes a combined matrix by multiplying the CCM by a white balance gains CCM-like matrix (WBG). The multiplication should be as follows to do it in the correct order: CCM * (WBG * P) = (CCM * WBG) * P The multiplication order in Lut software ISP algorithm is reversed, resulting in colour casts. Let's fix the order. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/ipa/simple/algorithms/lut.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)