Message ID | 20210923081625.60276-11-jeanmichel.hautbois@ideasonboard.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
On Thu, Sep 23, 2021 at 10:16:23AM +0200, Jean-Michel Hautbois wrote: > The pixels output by the camera normally include a black level, because > sensors do not always report a signal level of '0' for black. Pixels at > or below this level should be considered black and to achieve that, we > need to substract an offset to all the pixels. This can be taken into > account by reading the lowest value of a special region on sensors which > is not exposed to the lens. This provides a substracting factor to be > able to adjust the expected black levels in the resultant images. > > For a camera outputting 10-bit pixel values (in the range 0 to 1023) a > typical black level might be 64. It is a fixed value, obtained by > capturing a raw frame with minimum exposure and gain fixed to 1.0 while > covering the sensor (the darker the better). We consider it good enough > as a very first approximation, until we measure it during a tuning > process and include it in a configuration file > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/awb.cpp | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index a5391653..3013870b 100644 > --- a/src/ipa/ipu3/algorithms/awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -388,9 +388,17 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) > /* The CCM matrix may change when color temperature will be used */ > params->acc_param.ccm = imguCssCcmDefault; > > + /* The Optical Black Level correction values */ I think I'd like to see a clear todo highlighting that these values are ad-hoc: /* * \todo The correction values should come from sensor specific * tuning processes. This is a first approximation */ > + params->obgrid_param.gr = 64; > + params->obgrid_param.r = 64; > + params->obgrid_param.b = 64; > + params->obgrid_param.gb = 64; > + > params->use.acc_awb = 1; > params->use.acc_bnr = 1; > params->use.acc_ccm = 1; I'd be tempted by a blank line here to split out the obgrid to make it clear that it's a separate component. With those: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + params->use.obgrid = 1; > + params->use.obgrid_param = 1; > } > > } /* namespace ipa::ipu3::algorithms */ > -- > 2.30.2 >
On Tue, Sep 28, 2021 at 05:10:56PM +0100, Kieran Bingham wrote: > On Thu, Sep 23, 2021 at 10:16:23AM +0200, Jean-Michel Hautbois wrote: > > The pixels output by the camera normally include a black level, because > > sensors do not always report a signal level of '0' for black. Pixels at > > or below this level should be considered black and to achieve that, we > > need to substract an offset to all the pixels. This can be taken into > > account by reading the lowest value of a special region on sensors which > > is not exposed to the lens. This provides a substracting factor to be s/the lens/light/ > > able to adjust the expected black levels in the resultant images. > > > > For a camera outputting 10-bit pixel values (in the range 0 to 1023) a > > typical black level might be 64. It is a fixed value, obtained by > > capturing a raw frame with minimum exposure and gain fixed to 1.0 while > > covering the sensor (the darker the better). We consider it good enough > > as a very first approximation, until we measure it during a tuning > > process and include it in a configuration file > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > --- > > src/ipa/ipu3/algorithms/awb.cpp | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > > index a5391653..3013870b 100644 > > --- a/src/ipa/ipu3/algorithms/awb.cpp > > +++ b/src/ipa/ipu3/algorithms/awb.cpp > > @@ -388,9 +388,17 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) > > /* The CCM matrix may change when color temperature will be used */ > > params->acc_param.ccm = imguCssCcmDefault; > > > > + /* The Optical Black Level correction values */ > > I think I'd like to see a clear todo highlighting that these values are > ad-hoc: > > /* > * \todo The correction values should come from sensor specific > * tuning processes. This is a first approximation > */ Indeed, this is really a hack, and should be documented as such. > > + params->obgrid_param.gr = 64; > > + params->obgrid_param.r = 64; > > + params->obgrid_param.b = 64; > > + params->obgrid_param.gb = 64; > > + > > params->use.acc_awb = 1; > > params->use.acc_bnr = 1; > > params->use.acc_ccm = 1; > > I'd be tempted by a blank line here to split out the obgrid to make it > clear that it's a separate component. Why do we set the optical black level compensation parameters in AWB though ? Shouldn't it be a separate algorithm ? > With those: > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + params->use.obgrid = 1; > > + params->use.obgrid_param = 1; > > } > > > > } /* namespace ipa::ipu3::algorithms */
diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index a5391653..3013870b 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -388,9 +388,17 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) /* The CCM matrix may change when color temperature will be used */ params->acc_param.ccm = imguCssCcmDefault; + /* The Optical Black Level correction values */ + params->obgrid_param.gr = 64; + params->obgrid_param.r = 64; + params->obgrid_param.b = 64; + params->obgrid_param.gb = 64; + params->use.acc_awb = 1; params->use.acc_bnr = 1; params->use.acc_ccm = 1; + params->use.obgrid = 1; + params->use.obgrid_param = 1; } } /* namespace ipa::ipu3::algorithms */
The pixels output by the camera normally include a black level, because sensors do not always report a signal level of '0' for black. Pixels at or below this level should be considered black and to achieve that, we need to substract an offset to all the pixels. This can be taken into account by reading the lowest value of a special region on sensors which is not exposed to the lens. This provides a substracting factor to be able to adjust the expected black levels in the resultant images. For a camera outputting 10-bit pixel values (in the range 0 to 1023) a typical black level might be 64. It is a fixed value, obtained by capturing a raw frame with minimum exposure and gain fixed to 1.0 while covering the sensor (the darker the better). We consider it good enough as a very first approximation, until we measure it during a tuning process and include it in a configuration file Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/ipu3/algorithms/awb.cpp | 8 ++++++++ 1 file changed, 8 insertions(+)