[v2] libcamera: ipa: simple: Fix multiplication order in Awb
diff mbox series

Message ID 20260203115326.10421-1-mzamazal@redhat.com
State Accepted
Commit bb2e6d0833adad9a1678c169fef74e9f7c211c73
Headers show
Series
  • [v2] libcamera: ipa: simple: Fix multiplication order in Awb
Related show

Commit Message

Milan Zamazal Feb. 3, 2026, 11:53 a.m. UTC
The matrix multiplication in Awb is swapped: the gains should be applied
after combinedMatrix, i.e. on the left side.  The mistake happened when
`ccm' was replaced with combinedMatrix and gainMatrix multiplication was
moved to Awb.  While CCM must be applied after gains, the gains must be
applied after the combined matrix, which no longer corresponds to CCM in
Awb.

Since there is currently no algorithm modifying combinedMatrix before
Awb, combinedMatrix is an identity matrix there and the wrong order
doesn't influence the output at the moment.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/ipa/simple/algorithms/awb.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Barnabás Pőcze Feb. 5, 2026, 10:29 a.m. UTC | #1
2026. 02. 03. 12:53 keltezéssel, Milan Zamazal írta:
> The matrix multiplication in Awb is swapped: the gains should be applied
> after combinedMatrix, i.e. on the left side.  The mistake happened when
> `ccm' was replaced with combinedMatrix and gainMatrix multiplication was
> moved to Awb.  While CCM must be applied after gains, the gains must be
> applied after the combined matrix, which no longer corresponds to CCM in
> Awb.
> 
> Since there is currently no algorithm modifying combinedMatrix before
> Awb, combinedMatrix is an identity matrix there and the wrong order
> doesn't influence the output at the moment.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---

Looks ok to me.

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/ipa/simple/algorithms/awb.cpp | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp
> index 6fdaacaba..4ed1be289 100644
> --- a/src/ipa/simple/algorithms/awb.cpp
> +++ b/src/ipa/simple/algorithms/awb.cpp
> @@ -44,7 +44,7 @@ void Awb::prepare(IPAContext &context,
>   					     0, gains.g(), 0,
>   					     0, 0, gains.b() } };
>   	context.activeState.combinedMatrix =
> -		context.activeState.combinedMatrix * gainMatrix;
> +		gainMatrix * context.activeState.combinedMatrix;
>   
>   	frameContext.gains.red = gains.r();
>   	frameContext.gains.blue = gains.b();
Laurent Pinchart Feb. 5, 2026, 10:25 p.m. UTC | #2
On Thu, Feb 05, 2026 at 11:29:45AM +0100, Barnabás Pőcze wrote:
> 2026. 02. 03. 12:53 keltezéssel, Milan Zamazal írta:
> > The matrix multiplication in Awb is swapped: the gains should be applied
> > after combinedMatrix, i.e. on the left side.  The mistake happened when
> > `ccm' was replaced with combinedMatrix and gainMatrix multiplication was
> > moved to Awb.  While CCM must be applied after gains, the gains must be
> > applied after the combined matrix, which no longer corresponds to CCM in
> > Awb.
> > 
> > Since there is currently no algorithm modifying combinedMatrix before
> > Awb, combinedMatrix is an identity matrix there and the wrong order
> > doesn't influence the output at the moment.
> > 
> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> > ---
> 
> Looks ok to me.
> 
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

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

> >   src/ipa/simple/algorithms/awb.cpp | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp
> > index 6fdaacaba..4ed1be289 100644
> > --- a/src/ipa/simple/algorithms/awb.cpp
> > +++ b/src/ipa/simple/algorithms/awb.cpp
> > @@ -44,7 +44,7 @@ void Awb::prepare(IPAContext &context,
> >   					     0, gains.g(), 0,
> >   					     0, 0, gains.b() } };
> >   	context.activeState.combinedMatrix =
> > -		context.activeState.combinedMatrix * gainMatrix;
> > +		gainMatrix * context.activeState.combinedMatrix;
> >   
> >   	frameContext.gains.red = gains.r();
> >   	frameContext.gains.blue = gains.b();

Patch
diff mbox series

diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp
index 6fdaacaba..4ed1be289 100644
--- a/src/ipa/simple/algorithms/awb.cpp
+++ b/src/ipa/simple/algorithms/awb.cpp
@@ -44,7 +44,7 @@  void Awb::prepare(IPAContext &context,
 					     0, gains.g(), 0,
 					     0, 0, gains.b() } };
 	context.activeState.combinedMatrix =
-		context.activeState.combinedMatrix * gainMatrix;
+		gainMatrix * context.activeState.combinedMatrix;
 
 	frameContext.gains.red = gains.r();
 	frameContext.gains.blue = gains.b();