[libcamera-devel] [PATCH 3/4] libipa: camera_sensor_helper: Add IMX290 helper

[Jacopo Mondi]

Hi Laurent

On Mon, Mar 28, 2022 at 07:18:27PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Mon, Mar 28, 2022 at 03:52:18PM +0200, Jacopo Mondi wrote:
> > On Mon, Mar 28, 2022 at 03:03:35PM +0300, Laurent Pinchart via libcamera-devel wrote:
> > > The IMX290 is a Sony sensor that expresses its gain in 0.3dB units. It
> > > thus maps to the exponential gain model.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >  src/ipa/libipa/camera_sensor_helper.cpp | 31 +++++++++++++++++++++++++
> > >  1 file changed, 31 insertions(+)
> > >
> > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > > index 7bb999e19102..136b9f6bc3c5 100644
> > > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > > @@ -332,6 +332,26 @@ std::vector<CameraSensorHelperFactory *> &CameraSensorHelperFactory::factories()
> > >
> > >  #ifndef __DOXYGEN__
> > >
> > > +/*
> > > + * Helper function to compute the m parameter of the exponential gain model
> > > + * when the gain code is expressed in dB.
> > > + */
> > > +static constexpr double expGainDb(double step)
> > > +{
> > > +	constexpr double log2_10 = 3.321928094887362;
> > > +
> > > +	/*
> > > +	 * The gain code is expressed in step * dB (e.g. in 0.1 dB steps):
> > > +	 *
> > > +	 * G_code = G_dB/step = 20/step*log10(G_linear)
> >
> > Why was I expecting
> >                                 10/step*log10(G_linear)
> >
> > As my understanding is that
> >
> >                 G_db = 10*log10(G_linear) ?
>
> The joys of dB as a power ratio or an amplitude ratio :-) Here the
> linear gain is an amplitude gain, so G_dB = 20*log10(G_linear). Quoting
> https://en.wikipedia.org/wiki/Decibel,
>
> "Two principal types of scaling of the decibel are in common use. When
> expressing a power ratio, it is defined as ten times the logarithm in
> base 10.[5] That is, a change in power by a factor of 10 corresponds to
> a 10 dB change in level. When expressing root-power quantities, a change
> in amplitude by a factor of 10 corresponds to a 20 dB change in level.
> The decibel scales differ by a factor of two, so that the related power
> and root-power levels change by the same value in linear systems, where
> power is proportional to the square of amplitude."
>
> A 20dB increment will multiple the amplitude by 10 and the power by 100.
>

Thanks, I was missing that part!

> > (nit: isn't it more readable with spaces between operands ?)
>
> I can add spaces. The line below may get long though:
>
> 	 * G_linear = 10 ^ (step / 20 * G_code) = 2 ^ (log2(10) * step / 20 * G_code)
>
> What do you prefer ? I can also switch to Latex notation:
>
> 	 * G_{linear} = 10^{frac{step*G_{code}}{20}} = 2^{log_{2}(10) * frac{step*G_{code}}{20}}
>
> but that's likely not better :-)

Likely not, yeah :)

It gets a bit longer you're right. Whatever works

Thanks for the clarification
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j
>
> > > +	 *
> > > +	 * Inverting the formula, we get
> > > +	 *
> > > +	 * G_linear = 10^(step/20*G_code) = 2^(log2(10)*step/20*G_code)
> > > +	 */
> > > +	return log2_10 * step / 20;
> > > +}
> > > +
> > >  class CameraSensorHelperImx219 : public CameraSensorHelper
> > >  {
> > >  public:
> > > @@ -354,6 +374,17 @@ public:
> > >  };
> > >  REGISTER_CAMERA_SENSOR_HELPER("imx258", CameraSensorHelperImx258)
> > >
> > > +class CameraSensorHelperImx290 : public CameraSensorHelper
> > > +{
> > > +public:
> > > +	CameraSensorHelperImx290()
> > > +	{
> > > +		gainType_ = AnalogueGainExponential;
> > > +		gainConstants_.exp = { 1.0, expGainDb(0.3) };
> > > +	}
> > > +};
> > > +REGISTER_CAMERA_SENSOR_HELPER("imx290", CameraSensorHelperImx290)
> > > +
> > >  class CameraSensorHelperOv2740 : public CameraSensorHelper
> > >  {
> > >  public:
>
> --
> Regards,
>
> Laurent Pinchart