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

[paul.elder at ideasonboard.com]

Hi Laurent,

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?

>  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>