| Message ID | 20220616091148.67870-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi, JM,
For BNR parameters WB gain factor for the three channels [Ggr, Ggb, Gb, Gr].
Their precision is U3.13 and the range is (0, 8) and the actual gain is
Gx + 1, it is typically Gx = 1.
Pout = {Pin * (1 + Gx)}.
So that means for wb_gains.Ggr/gb/g/r = 0.5, the actual gain is 1.5.
It seems not correct to changing ratio from 8192 to 4096 as its
precision is U3.13. I am not familiar with ipa implementation in
libcamera, not sure other details.
Hi Bingbu, On 17/06/2022 08:58, Cao, Bingbu wrote: > Hi, JM, > > For BNR parameters WB gain factor for the three channels [Ggr, Ggb, Gb, Gr]. > Their precision is U3.13 and the range is (0, 8) and the actual gain is > Gx + 1, it is typically Gx = 1. > > Pout = {Pin * (1 + Gx)}. That is very interesting, as I thought it would be Pout = {Pin * Gx} ! It means that, applying a gain of 1 for green channel is really forcing Ggr and Ggb to be 0 ! > > So that means for wb_gains.Ggr/gb/g/r = 0.5, the actual gain is 1.5. > > It seems not correct to changing ratio from 8192 to 4096 as its > precision is U3.13. I am not familiar with ipa implementation in > libcamera, not sure other details. Nope, indeed, this is not the correct ratio ! But now I know what is expected, I think. Thanks ! JM > > > ________________________ > BRs, > Bingbu Cao > >> -----Original Message----- >> From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> Sent: Thursday, June 16, 2022 5:12 PM >> To: libcamera-devel@lists.libcamera.org >> Cc: Cao, Bingbu <bingbu.cao@intel.com>; sakari.ailus@linux.intel.com; >> Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> Subject: [RFC PATCH 0/1] IPU3 AWB gains correction >> >> Hi there, >> >> Bingbu, Sakari, I cc'ed you because this patch is not aligned with the >> current documentation in the intel-ipu3.h file, could you have a look >> please ? >> >> The part which worries me is that the green gains should be 0 for the >> white balance to be correct which is not stated in the documentation at >> all. >> >> In order to demonstrate it I captured two frames, [1] is the actual SGo2 >> image quality, and [2] with this patch applied. >> >> [1]: https://pasteboard.co/hUcL4U6eyyAH.png >> [2]: https://pasteboard.co/2XIbkkL8kitl.png >> >> There is a very clear benefit, that's why I submit this RFC as I think it >> needs to be clarified :-). >> >> Thanks ! >> JM >> >> Jean-Michel Hautbois (1): >> ipa: ipu3: awb: Correct the coefficient factor >> >> src/ipa/ipu3/algorithms/awb.cpp | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> -- >> 2.34.1 >
Hi Bingbu, Thank you very much for your quick reply. On Fri, Jun 17, 2022 at 06:58:36AM +0000, Cao, Bingbu via libcamera-devel wrote: > Hi, JM, > > For BNR parameters WB gain factor for the three channels [Ggr, Ggb, Gb, Gr]. > Their precision is U3.13 and the range is (0, 8) and the actual gain is > Gx + 1, it is typically Gx = 1. > > Pout = {Pin * (1 + Gx)}. > > So that means for wb_gains.Ggr/gb/g/r = 0.5, the actual gain is 1.5. Just to make sure my understanding is correct, does this mean the following ? wb_gains.gr = 0 -> actual gain = 1.0 wb_gains.gr = 4096 -> actual gain = 1.5 wb_gains.gr = 8192 -> actual gain = 2.0 > It seems not correct to changing ratio from 8192 to 4096 as its > precision is U3.13. I am not familiar with ipa implementation in > libcamera, not sure other details. > > > -----Original Message----- > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > Sent: Thursday, June 16, 2022 5:12 PM > > To: libcamera-devel@lists.libcamera.org > > Cc: Cao, Bingbu <bingbu.cao@intel.com>; sakari.ailus@linux.intel.com; > > Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > Subject: [RFC PATCH 0/1] IPU3 AWB gains correction > > > > Hi there, > > > > Bingbu, Sakari, I cc'ed you because this patch is not aligned with the > > current documentation in the intel-ipu3.h file, could you have a look > > please ? > > > > The part which worries me is that the green gains should be 0 for the > > white balance to be correct which is not stated in the documentation at > > all. > > > > In order to demonstrate it I captured two frames, [1] is the actual SGo2 > > image quality, and [2] with this patch applied. > > > > [1]: https://pasteboard.co/hUcL4U6eyyAH.png > > [2]: https://pasteboard.co/2XIbkkL8kitl.png > > > > There is a very clear benefit, that's why I submit this RFC as I think it > > needs to be clarified :-). > > > > Thanks ! > > JM > > > > Jean-Michel Hautbois (1): > > ipa: ipu3: awb: Correct the coefficient factor > > > > src/ipa/ipu3/algorithms/awb.cpp | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-)
Laurent and JM, Give me some time, let me confirm with our algo team.
Thank you. FYI, I tested the patch I sent with this fixed [1], and I obtain [2] while it is [3] on master. [1]: https://patchwork.libcamera.org/patch/16256/ [2]: https://pasteboard.co/sPXz45GjbpAg.png [3]: https://pasteboard.co/DLfD69BzsFuu.png Waiting for your confirmation ;-). JM On 17/06/2022 15:30, Cao, Bingbu via libcamera-devel wrote: > Laurent and JM, > > Give me some time, let me confirm with our algo team. > > ________________________ > BRs, > Bingbu Cao > >> -----Original Message----- >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Sent: Friday, June 17, 2022 4:08 PM >> To: Cao, Bingbu <bingbu.cao@intel.com> >> Cc: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>; >> libcamera-devel@lists.libcamera.org; sakari.ailus@linux.intel.com >> Subject: Re: [libcamera-devel] [RFC PATCH 0/1] IPU3 AWB gains correction >> >> Hi Bingbu, >> >> Thank you very much for your quick reply. >> >> On Fri, Jun 17, 2022 at 06:58:36AM +0000, Cao, Bingbu via libcamera-devel >> wrote: >>> Hi, JM, >>> >>> For BNR parameters WB gain factor for the three channels [Ggr, Ggb, Gb, >> Gr]. >>> Their precision is U3.13 and the range is (0, 8) and the actual gain >>> is Gx + 1, it is typically Gx = 1. >>> >>> Pout = {Pin * (1 + Gx)}. >>> >>> So that means for wb_gains.Ggr/gb/g/r = 0.5, the actual gain is 1.5. >> >> Just to make sure my understanding is correct, does this mean the >> following ? >> >> wb_gains.gr = 0 -> actual gain = 1.0 >> wb_gains.gr = 4096 -> actual gain = 1.5 >> wb_gains.gr = 8192 -> actual gain = 2.0 >> >>> It seems not correct to changing ratio from 8192 to 4096 as its >>> precision is U3.13. I am not familiar with ipa implementation in >>> libcamera, not sure other details. >>> >>>> -----Original Message----- >>>> From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >>>> Sent: Thursday, June 16, 2022 5:12 PM >>>> To: libcamera-devel@lists.libcamera.org >>>> Cc: Cao, Bingbu <bingbu.cao@intel.com>; >>>> sakari.ailus@linux.intel.com; Jean-Michel Hautbois >>>> <jeanmichel.hautbois@ideasonboard.com> >>>> Subject: [RFC PATCH 0/1] IPU3 AWB gains correction >>>> >>>> Hi there, >>>> >>>> Bingbu, Sakari, I cc'ed you because this patch is not aligned with >>>> the current documentation in the intel-ipu3.h file, could you have a >>>> look please ? >>>> >>>> The part which worries me is that the green gains should be 0 for >>>> the white balance to be correct which is not stated in the >>>> documentation at all. >>>> >>>> In order to demonstrate it I captured two frames, [1] is the actual >>>> SGo2 image quality, and [2] with this patch applied. >>>> >>>> [1]: https://pasteboard.co/hUcL4U6eyyAH.png >>>> [2]: https://pasteboard.co/2XIbkkL8kitl.png >>>> >>>> There is a very clear benefit, that's why I submit this RFC as I >>>> think it needs to be clarified :-). >>>> >>>> Thanks ! >>>> JM >>>> >>>> Jean-Michel Hautbois (1): >>>> ipa: ipu3: awb: Correct the coefficient factor >>>> >>>> src/ipa/ipu3/algorithms/awb.cpp | 8 ++++---- >>>> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> -- >> Regards, >> >> Laurent Pinchart
JM and Laurent,
I get some pseudo code from Algo which to calculate the gain
settings from awb results, I hope it can help you.
--------------------------------------------------------
inline int Real2fix(unsigned N, double x)
{
return static_cast<int>(x * (1<<N) +
(x > 0 ? 0.5 : -0.5));
}
final_b_per_g=awb_results->final_b_per_g;
final_r_per_g=awb_results->final_r_per_g;
float max_ratio = MAX(MAX(final_b_per_g, final_r_per_g),
1.0f);
wb_gains->r = static_cast<unsigned short>(
Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>(
max_ratio / final_r_per_g - 1)));
wb_gains->b = static_cast<unsigned short>(
Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>(
max_ratio / final_b_per_g - 1)));
wb_gains->gr = wb_gains->gb = static_cast<unsigned short>(
Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>(
max_ratio - 1)));
--------------------------------------------------------
BTW, I checked the 2nd picture, it is better than 3, but it is a
little pinkish.
Hi Bingbu, On Mon, Jun 20, 2022 at 12:03:52PM +0000, Cao, Bingbu wrote: > JM and Laurent, > > I get some pseudo code from Algo which to calculate the gain > settings from awb results, I hope it can help you. > > -------------------------------------------------------- > > inline int Real2fix(unsigned N, double x) I suppose this was meant to be template<unsigned N> inline int Real2fix(double x) based on how it's called below. > { > return static_cast<int>(x * (1<<N) + > (x > 0 ? 0.5 : -0.5)); > } > > final_b_per_g=awb_results->final_b_per_g; > final_r_per_g=awb_results->final_r_per_g; > > float max_ratio = MAX(MAX(final_b_per_g, final_r_per_g), > 1.0f); > > wb_gains->r = static_cast<unsigned short>( > Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>( > max_ratio / final_r_per_g - 1))); > > wb_gains->b = static_cast<unsigned short>( > Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>( > max_ratio / final_b_per_g - 1))); > > wb_gains->gr = wb_gains->gb = static_cast<unsigned short>( > Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>( > max_ratio - 1))); Thank you, that clarifies it. > -------------------------------------------------------- > BTW, I checked the 2nd picture, it is better than 3, but it is a > little pinkish. Yes, further improvements are needed. The image is also a bit too dark now. Still, it's much better than before :-) > > -----Original Message----- > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > Sent: Friday, June 17, 2022 10:40 PM > > To: Cao, Bingbu <bingbu.cao@intel.com>; Laurent Pinchart > > <laurent.pinchart@ideasonboard.com> > > Cc: libcamera-devel@lists.libcamera.org; sakari.ailus@linux.intel.com > > Subject: Re: [libcamera-devel] [RFC PATCH 0/1] IPU3 AWB gains correction > > > > Thank you. > > FYI, I tested the patch I sent with this fixed [1], and I obtain [2] > > while it is [3] on master. > > > > [1]: https://patchwork.libcamera.org/patch/16256/ > > [2]: https://pasteboard.co/sPXz45GjbpAg.png > > [3]: https://pasteboard.co/DLfD69BzsFuu.png > > > > Waiting for your confirmation ;-). > > JM > > > > On 17/06/2022 15:30, Cao, Bingbu via libcamera-devel wrote: > > > Laurent and JM, > > > > > > Give me some time, let me confirm with our algo team. > > > > > > ________________________ > > > BRs, > > > Bingbu Cao > > > > > >> -----Original Message----- > > >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > >> Sent: Friday, June 17, 2022 4:08 PM > > >> To: Cao, Bingbu <bingbu.cao@intel.com> > > >> Cc: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>; > > >> libcamera-devel@lists.libcamera.org; sakari.ailus@linux.intel.com > > >> Subject: Re: [libcamera-devel] [RFC PATCH 0/1] IPU3 AWB gains > > >> correction > > >> > > >> Hi Bingbu, > > >> > > >> Thank you very much for your quick reply. > > >> > > >> On Fri, Jun 17, 2022 at 06:58:36AM +0000, Cao, Bingbu via libcamera-devel wrote: > > >>> Hi, JM, > > >>> > > >>> For BNR parameters WB gain factor for the three channels [Ggr, Ggb, Gb, Gr]. > > >>> Their precision is U3.13 and the range is (0, 8) and the actual gain > > >>> is Gx + 1, it is typically Gx = 1. > > >>> > > >>> Pout = {Pin * (1 + Gx)}. > > >>> > > >>> So that means for wb_gains.Ggr/gb/g/r = 0.5, the actual gain is 1.5. > > >> > > >> Just to make sure my understanding is correct, does this mean the > > >> following ? > > >> > > >> wb_gains.gr = 0 -> actual gain = 1.0 > > >> wb_gains.gr = 4096 -> actual gain = 1.5 wb_gains.gr = 8192 -> actual > > >> gain = 2.0 > > >> > > >>> It seems not correct to changing ratio from 8192 to 4096 as its > > >>> precision is U3.13. I am not familiar with ipa implementation in > > >>> libcamera, not sure other details. > > >>> > > >>>> -----Original Message----- > > >>>> From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > >>>> Sent: Thursday, June 16, 2022 5:12 PM > > >>>> To: libcamera-devel@lists.libcamera.org > > >>>> Cc: Cao, Bingbu <bingbu.cao@intel.com>; > > >>>> sakari.ailus@linux.intel.com; Jean-Michel Hautbois > > >>>> <jeanmichel.hautbois@ideasonboard.com> > > >>>> Subject: [RFC PATCH 0/1] IPU3 AWB gains correction > > >>>> > > >>>> Hi there, > > >>>> > > >>>> Bingbu, Sakari, I cc'ed you because this patch is not aligned with > > >>>> the current documentation in the intel-ipu3.h file, could you have > > >>>> a look please ? > > >>>> > > >>>> The part which worries me is that the green gains should be 0 for > > >>>> the white balance to be correct which is not stated in the > > >>>> documentation at all. > > >>>> > > >>>> In order to demonstrate it I captured two frames, [1] is the actual > > >>>> SGo2 image quality, and [2] with this patch applied. > > >>>> > > >>>> [1]: https://pasteboard.co/hUcL4U6eyyAH.png > > >>>> [2]: https://pasteboard.co/2XIbkkL8kitl.png > > >>>> > > >>>> There is a very clear benefit, that's why I submit this RFC as I > > >>>> think it needs to be clarified :-). > > >>>> > > >>>> Thanks ! > > >>>> JM > > >>>> > > >>>> Jean-Michel Hautbois (1): > > >>>> ipa: ipu3: awb: Correct the coefficient factor > > >>>> > > >>>> src/ipa/ipu3/algorithms/awb.cpp | 8 ++++---- > > >>>> 1 file changed, 4 insertions(+), 4 deletions(-)
Laurent, > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Monday, June 20, 2022 8:26 PM > To: Cao, Bingbu <bingbu.cao@intel.com> > Cc: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>; > libcamera-devel@lists.libcamera.org; sakari.ailus@linux.intel.com > Subject: Re: [libcamera-devel] [RFC PATCH 0/1] IPU3 AWB gains correction > > Hi Bingbu, > > On Mon, Jun 20, 2022 at 12:03:52PM +0000, Cao, Bingbu wrote: > > JM and Laurent, > > > > I get some pseudo code from Algo which to calculate the gain settings > > from awb results, I hope it can help you. > > > > -------------------------------------------------------- > > > > inline int Real2fix(unsigned N, double x) > > I suppose this was meant to be > > template<unsigned N> > inline int Real2fix(double x) > > based on how it's called below. Yes, you are right, my bad. > > > { > > return static_cast<int>(x * (1<<N) + > > (x > 0 ? 0.5 : -0.5)); > > } > > > > final_b_per_g=awb_results->final_b_per_g; > > final_r_per_g=awb_results->final_r_per_g; > > > > float max_ratio = MAX(MAX(final_b_per_g, final_r_per_g), > > 1.0f); > > > > wb_gains->r = static_cast<unsigned short>( > > Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>( > > max_ratio / final_r_per_g - 1))); > > > > wb_gains->b = static_cast<unsigned short>( > > Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>( > > max_ratio / final_b_per_g - 1))); > > > > wb_gains->gr = wb_gains->gb = static_cast<unsigned short>( > > Clamp<wb_apply_min_, wb_apply_max_>(Real2fix<13>( > > max_ratio - 1))); > > Thank you, that clarifies it. Glad to see it can help you. 😊 > > > -------------------------------------------------------- > > BTW, I checked the 2nd picture, it is better than 3, but it is a > > little pinkish. > > Yes, further improvements are needed. The image is also a bit too dark > now. Still, it's much better than before :-) > > > > -----Original Message----- > > > From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > Sent: Friday, June 17, 2022 10:40 PM > > > To: Cao, Bingbu <bingbu.cao@intel.com>; Laurent Pinchart > > > <laurent.pinchart@ideasonboard.com> > > > Cc: libcamera-devel@lists.libcamera.org; > > > sakari.ailus@linux.intel.com > > > Subject: Re: [libcamera-devel] [RFC PATCH 0/1] IPU3 AWB gains > > > correction > > > > > > Thank you. > > > FYI, I tested the patch I sent with this fixed [1], and I obtain [2] > > > while it is [3] on master. > > > > > > [1]: https://patchwork.libcamera.org/patch/16256/ > > > [2]: https://pasteboard.co/sPXz45GjbpAg.png > > > [3]: https://pasteboard.co/DLfD69BzsFuu.png > > > > > > Waiting for your confirmation ;-). > > > JM > > > > > > On 17/06/2022 15:30, Cao, Bingbu via libcamera-devel wrote: > > > > Laurent and JM, > > > > > > > > Give me some time, let me confirm with our algo team. > > > > > > > > ________________________ > > > > BRs, > > > > Bingbu Cao > > > > > > > >> -----Original Message----- > > > >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > >> Sent: Friday, June 17, 2022 4:08 PM > > > >> To: Cao, Bingbu <bingbu.cao@intel.com> > > > >> Cc: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>; > > > >> libcamera-devel@lists.libcamera.org; sakari.ailus@linux.intel.com > > > >> Subject: Re: [libcamera-devel] [RFC PATCH 0/1] IPU3 AWB gains > > > >> correction > > > >> > > > >> Hi Bingbu, > > > >> > > > >> Thank you very much for your quick reply. > > > >> > > > >> On Fri, Jun 17, 2022 at 06:58:36AM +0000, Cao, Bingbu via > libcamera-devel wrote: > > > >>> Hi, JM, > > > >>> > > > >>> For BNR parameters WB gain factor for the three channels [Ggr, > Ggb, Gb, Gr]. > > > >>> Their precision is U3.13 and the range is (0, 8) and the actual > > > >>> gain is Gx + 1, it is typically Gx = 1. > > > >>> > > > >>> Pout = {Pin * (1 + Gx)}. > > > >>> > > > >>> So that means for wb_gains.Ggr/gb/g/r = 0.5, the actual gain is > 1.5. > > > >> > > > >> Just to make sure my understanding is correct, does this mean the > > > >> following ? > > > >> > > > >> wb_gains.gr = 0 -> actual gain = 1.0 > > > >> wb_gains.gr = 4096 -> actual gain = 1.5 wb_gains.gr = 8192 -> > > > >> actual gain = 2.0 > > > >> > > > >>> It seems not correct to changing ratio from 8192 to 4096 as its > > > >>> precision is U3.13. I am not familiar with ipa implementation > > > >>> in libcamera, not sure other details. > > > >>> > > > >>>> -----Original Message----- > > > >>>> From: Jean-Michel Hautbois > > > >>>> <jeanmichel.hautbois@ideasonboard.com> > > > >>>> Sent: Thursday, June 16, 2022 5:12 PM > > > >>>> To: libcamera-devel@lists.libcamera.org > > > >>>> Cc: Cao, Bingbu <bingbu.cao@intel.com>; > > > >>>> sakari.ailus@linux.intel.com; Jean-Michel Hautbois > > > >>>> <jeanmichel.hautbois@ideasonboard.com> > > > >>>> Subject: [RFC PATCH 0/1] IPU3 AWB gains correction > > > >>>> > > > >>>> Hi there, > > > >>>> > > > >>>> Bingbu, Sakari, I cc'ed you because this patch is not aligned > > > >>>> with the current documentation in the intel-ipu3.h file, could > > > >>>> you have a look please ? > > > >>>> > > > >>>> The part which worries me is that the green gains should be 0 > > > >>>> for the white balance to be correct which is not stated in the > > > >>>> documentation at all. > > > >>>> > > > >>>> In order to demonstrate it I captured two frames, [1] is the > > > >>>> actual > > > >>>> SGo2 image quality, and [2] with this patch applied. > > > >>>> > > > >>>> [1]: https://pasteboard.co/hUcL4U6eyyAH.png > > > >>>> [2]: https://pasteboard.co/2XIbkkL8kitl.png > > > >>>> > > > >>>> There is a very clear benefit, that's why I submit this RFC as > > > >>>> I think it needs to be clarified :-). > > > >>>> > > > >>>> Thanks ! > > > >>>> JM > > > >>>> > > > >>>> Jean-Michel Hautbois (1): > > > >>>> ipa: ipu3: awb: Correct the coefficient factor > > > >>>> > > > >>>> src/ipa/ipu3/algorithms/awb.cpp | 8 ++++---- > > > >>>> 1 file changed, 4 insertions(+), 4 deletions(-) > > -- > Regards, > > Laurent Pinchart