| Message ID | 20250217100203.297894-10-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2025-02-17 10:01:50) > Apparently AWB measurements are taken after the CCM. So the estimated Are we more sure than 'apparantly' ? What do we need to do to be more confident or certain of this ? > colour temperature and the corresponding CCM also lead to changed > rgbMeans. This is another source of oscillations. Fix that by applying > the inverse transform on the rgbMeans. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/awb.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > index 12934771c69c..66e6aecedc4c 100644 > --- a/src/ipa/rkisp1/algorithms/awb.cpp > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > @@ -305,6 +305,8 @@ void Awb::process(IPAContext &context, > rgbMeans = rgbMeans.max(0.0); > } > I would definitely put a block comment here describing /why/ we are doing this - likely matching closely to what you have in the commit message. > + rgbMeans = frameContext.ccm.ccm.cast<double>().inverse() * rgbMeans; > + > /* > * The ISP computes the AWB means after applying the colour gains, > * divide by the gains that were used to get the raw means from the > -- > 2.43.0 >
Hi Kieran, Thank you for the review. On Mon, Feb 17, 2025 at 12:26:36PM +0000, Kieran Bingham wrote: > Quoting Stefan Klug (2025-02-17 10:01:50) > > Apparently AWB measurements are taken after the CCM. So the estimated > > Are we more sure than 'apparantly' ? What do we need to do to be more > confident or certain of this ? You are right. I will explain how to test that in v2. Basically, you can comment the gains from the manual colour temperture code path and enable AWB debug logs. Then you can see the measurements change, when you modify the colour temperature. (I just realize that we can't manually set the CCM on rkisp1. That would have been an even easier test) > > > colour temperature and the corresponding CCM also lead to changed > > rgbMeans. This is another source of oscillations. Fix that by applying > > the inverse transform on the rgbMeans. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > src/ipa/rkisp1/algorithms/awb.cpp | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp > > index 12934771c69c..66e6aecedc4c 100644 > > --- a/src/ipa/rkisp1/algorithms/awb.cpp > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp > > @@ -305,6 +305,8 @@ void Awb::process(IPAContext &context, > > rgbMeans = rgbMeans.max(0.0); > > } > > > > I would definitely put a block comment here describing /why/ we are > doing this - likely matching closely to what you have in the commit > message. Right. Will add one. Best regards, Stefan > > > + rgbMeans = frameContext.ccm.ccm.cast<double>().inverse() * rgbMeans; > > + > > /* > > * The ISP computes the AWB means after applying the colour gains, > > * divide by the gains that were used to get the raw means from the > > -- > > 2.43.0 > >
diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 12934771c69c..66e6aecedc4c 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -305,6 +305,8 @@ void Awb::process(IPAContext &context, rgbMeans = rgbMeans.max(0.0); } + rgbMeans = frameContext.ccm.ccm.cast<double>().inverse() * rgbMeans; + /* * The ISP computes the AWB means after applying the colour gains, * divide by the gains that were used to get the raw means from the
Apparently AWB measurements are taken after the CCM. So the estimated colour temperature and the corresponding CCM also lead to changed rgbMeans. This is another source of oscillations. Fix that by applying the inverse transform on the rgbMeans. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/ipa/rkisp1/algorithms/awb.cpp | 2 ++ 1 file changed, 2 insertions(+)