[v3,03/19] libipa: camera_sensor_helper: Add quantizeGain() function
diff mbox series

Message ID 20250815102945.1602071-4-stefan.klug@ideasonboard.com
State Superseded
Headers show
Series
  • Implement WDR algorithm
Related show

Commit Message

Stefan Klug Aug. 15, 2025, 10:29 a.m. UTC
Add a small utility function that calculates the quantized gain that
gets applied by a sensor when the gain code is set to gainCode(gain).
This is needed by algorithms to calculate a digital correction gain that
gets applied to mitigate the error introduce by quantization.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

---

Changes in v3:
- Remove virtual from quantizeGain()
- Improved documentation
- Collected tags
---
 src/ipa/libipa/camera_sensor_helper.cpp | 23 +++++++++++++++++++++++
 src/ipa/libipa/camera_sensor_helper.h   |  1 +
 2 files changed, 24 insertions(+)

Comments

Paul Elder Sept. 5, 2025, 10:43 a.m. UTC | #1
Quoting Stefan Klug (2025-08-15 19:29:23)
> Add a small utility function that calculates the quantized gain that
> gets applied by a sensor when the gain code is set to gainCode(gain).
> This is needed by algorithms to calculate a digital correction gain that
> gets applied to mitigate the error introduce by quantization.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> 
> ---
> 
> Changes in v3:
> - Remove virtual from quantizeGain()
> - Improved documentation
> - Collected tags
> ---
>  src/ipa/libipa/camera_sensor_helper.cpp | 23 +++++++++++++++++++++++
>  src/ipa/libipa/camera_sensor_helper.h   |  1 +
>  2 files changed, 24 insertions(+)
> 
> diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> index dcd69d9f2bbb..a490161faf4c 100644
> --- a/src/ipa/libipa/camera_sensor_helper.cpp
> +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> @@ -131,6 +131,29 @@ double CameraSensorHelper::gain(uint32_t gainCode) const
>         }
>  }
>  
> +/**
> + * \brief Quantize the given gain value
> + * \param[in] _gain The real gain
> + * \param[out] quantizationGain The gain that is lost due to quantization
> + *
> + * This function returns the actual gain that is applied when the sensors gain

s/sensors/sensor's/

> + * is set to gainCode(gain).

s/(gain)/(_gain)/ I think might be clearer?

> + *
> + * It shall be guaranteed that gainCode(gain) == gainCode(quantizeGain(gain)).

Same here.

> + *
> + * If \a quantizationGain is provided it is filled with the gain that must be

Personally I'd prefer s/filled/populated/, since filling feels like there could
be an initial value that you "fill" until you reach the target value. It's just
a feeling though :)

> + * applied to correct the losses due to quantization.

s/applied to/applied to \a _gain/ and s/the/for the/ I think is slightly clearer.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> + *
> + * \return The quantized real gain
> + */
> +double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain) const
> +{
> +       double g = gain(gainCode(_gain));
> +       if (quantizationGain)
> +               *quantizationGain = _gain / g;
> +       return g;
> +}
> +
>  /**
>   * \struct CameraSensorHelper::AnalogueGainLinear
>   * \brief Analogue gain constants for the linear gain model
> diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h
> index a9300a64f1e7..bd3d0beec77f 100644
> --- a/src/ipa/libipa/camera_sensor_helper.h
> +++ b/src/ipa/libipa/camera_sensor_helper.h
> @@ -29,6 +29,7 @@ public:
>         std::optional<int16_t> blackLevel() const { return blackLevel_; }
>         virtual uint32_t gainCode(double gain) const;
>         virtual double gain(uint32_t gainCode) const;
> +       double quantizeGain(double gain, double *quantizationGain) const;
>  
>  protected:
>         struct AnalogueGainLinear {
> -- 
> 2.48.1
>
Stefan Klug Sept. 18, 2025, 1:16 p.m. UTC | #2
Hi Paul,

Thank you for the review.

Quoting Paul Elder (2025-09-05 12:43:02)
> Quoting Stefan Klug (2025-08-15 19:29:23)
> > Add a small utility function that calculates the quantized gain that
> > gets applied by a sensor when the gain code is set to gainCode(gain).
> > This is needed by algorithms to calculate a digital correction gain that
> > gets applied to mitigate the error introduce by quantization.
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> > 
> > ---
> > 
> > Changes in v3:
> > - Remove virtual from quantizeGain()
> > - Improved documentation
> > - Collected tags
> > ---
> >  src/ipa/libipa/camera_sensor_helper.cpp | 23 +++++++++++++++++++++++
> >  src/ipa/libipa/camera_sensor_helper.h   |  1 +
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > index dcd69d9f2bbb..a490161faf4c 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -131,6 +131,29 @@ double CameraSensorHelper::gain(uint32_t gainCode) const
> >         }
> >  }
> >  
> > +/**
> > + * \brief Quantize the given gain value
> > + * \param[in] _gain The real gain
> > + * \param[out] quantizationGain The gain that is lost due to quantization
> > + *
> > + * This function returns the actual gain that is applied when the sensors gain
> 
> s/sensors/sensor's/
> 
> > + * is set to gainCode(gain).
> 
> s/(gain)/(_gain)/ I think might be clearer?
> 
> > + *
> > + * It shall be guaranteed that gainCode(gain) == gainCode(quantizeGain(gain)).
> 
> Same here.
> 
> > + *
> > + * If \a quantizationGain is provided it is filled with the gain that must be
> 
> Personally I'd prefer s/filled/populated/, since filling feels like there could
> be an initial value that you "fill" until you reach the target value. It's just
> a feeling though :)
> 
> > + * applied to correct the losses due to quantization.
> 
> s/applied to/applied to \a _gain/ and s/the/for the/ I think is slightly clearer.

I applied the previous fixes. Here I think it is not exactly right as
quantizationGain is not applied to _gain but to the result of this
function. I went for "... the gain that must be applied on top to
correct for the losses...". This is also not ideal, but I already have
the next rewrite of this function in the pipeline so I guess it's ok for
now.

> 
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

Thanks,
Stefan

> 
> > + *
> > + * \return The quantized real gain
> > + */
> > +double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain) const
> > +{
> > +       double g = gain(gainCode(_gain));
> > +       if (quantizationGain)
> > +               *quantizationGain = _gain / g;
> > +       return g;
> > +}
> > +
> >  /**
> >   * \struct CameraSensorHelper::AnalogueGainLinear
> >   * \brief Analogue gain constants for the linear gain model
> > diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h
> > index a9300a64f1e7..bd3d0beec77f 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.h
> > +++ b/src/ipa/libipa/camera_sensor_helper.h
> > @@ -29,6 +29,7 @@ public:
> >         std::optional<int16_t> blackLevel() const { return blackLevel_; }
> >         virtual uint32_t gainCode(double gain) const;
> >         virtual double gain(uint32_t gainCode) const;
> > +       double quantizeGain(double gain, double *quantizationGain) const;
> >  
> >  protected:
> >         struct AnalogueGainLinear {
> > -- 
> > 2.48.1
> >
Paul Elder Sept. 18, 2025, 6:51 p.m. UTC | #3
Quoting Stefan Klug (2025-09-18 22:16:28)
> Hi Paul,
> 
> Thank you for the review.
> 
> Quoting Paul Elder (2025-09-05 12:43:02)
> > Quoting Stefan Klug (2025-08-15 19:29:23)
> > > Add a small utility function that calculates the quantized gain that
> > > gets applied by a sensor when the gain code is set to gainCode(gain).
> > > This is needed by algorithms to calculate a digital correction gain that
> > > gets applied to mitigate the error introduce by quantization.
> > > 
> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
> > > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> > > 
> > > ---
> > > 
> > > Changes in v3:
> > > - Remove virtual from quantizeGain()
> > > - Improved documentation
> > > - Collected tags
> > > ---
> > >  src/ipa/libipa/camera_sensor_helper.cpp | 23 +++++++++++++++++++++++
> > >  src/ipa/libipa/camera_sensor_helper.h   |  1 +
> > >  2 files changed, 24 insertions(+)
> > > 
> > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > > index dcd69d9f2bbb..a490161faf4c 100644
> > > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > > @@ -131,6 +131,29 @@ double CameraSensorHelper::gain(uint32_t gainCode) const
> > >         }
> > >  }
> > >  
> > > +/**
> > > + * \brief Quantize the given gain value
> > > + * \param[in] _gain The real gain
> > > + * \param[out] quantizationGain The gain that is lost due to quantization
> > > + *
> > > + * This function returns the actual gain that is applied when the sensors gain
> > 
> > s/sensors/sensor's/
> > 
> > > + * is set to gainCode(gain).
> > 
> > s/(gain)/(_gain)/ I think might be clearer?
> > 
> > > + *
> > > + * It shall be guaranteed that gainCode(gain) == gainCode(quantizeGain(gain)).
> > 
> > Same here.
> > 
> > > + *
> > > + * If \a quantizationGain is provided it is filled with the gain that must be
> > 
> > Personally I'd prefer s/filled/populated/, since filling feels like there could
> > be an initial value that you "fill" until you reach the target value. It's just
> > a feeling though :)
> > 
> > > + * applied to correct the losses due to quantization.
> > 
> > s/applied to/applied to \a _gain/ and s/the/for the/ I think is slightly clearer.
> 
> I applied the previous fixes. Here I think it is not exactly right as
> quantizationGain is not applied to _gain but to the result of this
> function. I went for "... the gain that must be applied on top to
> correct for the losses...". This is also not ideal, but I already have

Ok, that's good too, I'll take that.

Paul

> the next rewrite of this function in the pipeline so I guess it's ok for
> now.
> 
> > 
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> Thanks,
> Stefan
> 
> > 
> > > + *
> > > + * \return The quantized real gain
> > > + */
> > > +double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain) const
> > > +{
> > > +       double g = gain(gainCode(_gain));
> > > +       if (quantizationGain)
> > > +               *quantizationGain = _gain / g;
> > > +       return g;
> > > +}
> > > +
> > >  /**
> > >   * \struct CameraSensorHelper::AnalogueGainLinear
> > >   * \brief Analogue gain constants for the linear gain model
> > > diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h
> > > index a9300a64f1e7..bd3d0beec77f 100644
> > > --- a/src/ipa/libipa/camera_sensor_helper.h
> > > +++ b/src/ipa/libipa/camera_sensor_helper.h
> > > @@ -29,6 +29,7 @@ public:
> > >         std::optional<int16_t> blackLevel() const { return blackLevel_; }
> > >         virtual uint32_t gainCode(double gain) const;
> > >         virtual double gain(uint32_t gainCode) const;
> > > +       double quantizeGain(double gain, double *quantizationGain) const;
> > >  
> > >  protected:
> > >         struct AnalogueGainLinear {
> > > -- 
> > > 2.48.1
> > >

Patch
diff mbox series

diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
index dcd69d9f2bbb..a490161faf4c 100644
--- a/src/ipa/libipa/camera_sensor_helper.cpp
+++ b/src/ipa/libipa/camera_sensor_helper.cpp
@@ -131,6 +131,29 @@  double CameraSensorHelper::gain(uint32_t gainCode) const
 	}
 }
 
+/**
+ * \brief Quantize the given gain value
+ * \param[in] _gain The real gain
+ * \param[out] quantizationGain The gain that is lost due to quantization
+ *
+ * This function returns the actual gain that is applied when the sensors gain
+ * is set to gainCode(gain).
+ *
+ * It shall be guaranteed that gainCode(gain) == gainCode(quantizeGain(gain)).
+ *
+ * If \a quantizationGain is provided it is filled with the gain that must be
+ * applied to correct the losses due to quantization.
+ *
+ * \return The quantized real gain
+ */
+double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain) const
+{
+	double g = gain(gainCode(_gain));
+	if (quantizationGain)
+		*quantizationGain = _gain / g;
+	return g;
+}
+
 /**
  * \struct CameraSensorHelper::AnalogueGainLinear
  * \brief Analogue gain constants for the linear gain model
diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h
index a9300a64f1e7..bd3d0beec77f 100644
--- a/src/ipa/libipa/camera_sensor_helper.h
+++ b/src/ipa/libipa/camera_sensor_helper.h
@@ -29,6 +29,7 @@  public:
 	std::optional<int16_t> blackLevel() const { return blackLevel_; }
 	virtual uint32_t gainCode(double gain) const;
 	virtual double gain(uint32_t gainCode) const;
+	double quantizeGain(double gain, double *quantizationGain) const;
 
 protected:
 	struct AnalogueGainLinear {