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

[Laurent Pinchart]

Hi Paul,

On Wed, Mar 30, 2022 at 05:50:03PM +0900, paul.elder at ideasonboard.com 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)
> > +	 *
> > +	 * Inverting the formula, we get
> > +	 *
> > +	 * G_linear = 10^(step/20*G_code) = 2^(log2(10)*step/20*G_code)
> > +	 */
> > +	return log2_10 * step / 20;
> > +}
> > +
> 
> Would this be better in a separate patch?

I've thought about it, but the compiler then warns that the function is
unused. It could be worked around by adding a [[maybe_unused]] and then
dropping it in the next patch, but I didn't think it was worth it.

> >  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:
> > -- 
> 
> The rest looks good.
> 
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

-- 
Regards,

Laurent Pinchart