| Message ID | 20250808141315.413839-4-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2025-08-08 15:12:41) > 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> > --- > 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..0a7b66cd003a 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 get's 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 is lost > + * 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..42bdb3c5550f 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; > + virtual double quantizeGain(double gain, double *quantizationGain) const; Should this be virtual? do you expect it to be overridden by anything? -- Kieran > > protected: > struct AnalogueGainLinear { > -- > 2.48.1 >
Hi Stefan - thanks for the patches On 08/08/2025 15:12, Stefan Klug wrote: > 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> > --- > 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..0a7b66cd003a 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 get's applied when the sensors s/get's/gets and s/sensors/sensor's > + * 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 is lost > + * due to quantization. Maybe "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 Why the underscore prefix for _gain? > +{ > + 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..42bdb3c5550f 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; > + virtual double quantizeGain(double gain, double *quantizationGain) const; Does this need to be virtual? The calculation seems pretty universal. Thanks Dan > > protected: > struct AnalogueGainLinear {
Hi Kieran, Thank you for the review. Quoting Kieran Bingham (2025-08-08 19:45:36) > Quoting Stefan Klug (2025-08-08 15:12:41) > > 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> > > --- > > 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..0a7b66cd003a 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 get's 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 is lost > > + * 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..42bdb3c5550f 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; > > + virtual double quantizeGain(double gain, double *quantizationGain) const; > > Should this be virtual? do you expect it to be overridden by anything? No, I will remove that virtual. I can't really foresee a use case for overwriting that. Best regards, Stefan > -- > Kieran > > > > > protected: > > struct AnalogueGainLinear { > > -- > > 2.48.1 > >
Hi Dan, Thank you for the review. Quoting Dan Scally (2025-08-08 23:07:16) > Hi Stefan - thanks for the patches > > On 08/08/2025 15:12, Stefan Klug wrote: > > 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> > > --- > > 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..0a7b66cd003a 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 get's applied when the sensors > > s/get's/gets and s/sensors/sensor's Thanks. > > > + * 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 is lost > > + * due to quantization. > > Maybe "gain that must be applied to correct the losses due to quantization"? Yes, that sounds better. > > > + * > > + * \return The quantized real gain > > + */ > > +double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain) const > > Why the underscore prefix for _gain? If I call it gain, there is a name clash with the gain() function. I thought I saw that pattern elsewhere. Do you have a preferred name? > > > +{ > > + 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..42bdb3c5550f 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; > > + virtual double quantizeGain(double gain, double *quantizationGain) const; > > Does this need to be virtual? The calculation seems pretty universal. Yes, right. I'll remove that. Thanks, Stefan > > > Thanks > > Dan > > > > > protected: > > struct AnalogueGainLinear {
Hi Stefan On 11/08/2025 06:26, Stefan Klug wrote: > Hi Dan, > > Thank you for the review. > > Quoting Dan Scally (2025-08-08 23:07:16) >> Hi Stefan - thanks for the patches >> >> On 08/08/2025 15:12, Stefan Klug wrote: >>> 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> >>> --- >>> 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..0a7b66cd003a 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 get's applied when the sensors >> >> s/get's/gets and s/sensors/sensor's > > Thanks. > >> >>> + * 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 is lost >>> + * due to quantization. >> >> Maybe "gain that must be applied to correct the losses due to quantization"? > > Yes, that sounds better. > >> >>> + * >>> + * \return The quantized real gain >>> + */ >>> +double CameraSensorHelper::quantizeGain(double _gain, double *quantizationGain) const >> >> Why the underscore prefix for _gain? > > If I call it gain, there is a name clash with the gain() function. I > thought I saw that pattern elsewhere. Do you have a preferred name? No not particularly; I just hadn't spotted the name clash - that's fine. > >> >>> +{ >>> + 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..42bdb3c5550f 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; >>> + virtual double quantizeGain(double gain, double *quantizationGain) const; >> >> Does this need to be virtual? The calculation seems pretty universal. > > Yes, right. I'll remove that. With that change then: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > > Thanks, > Stefan > >> >> >> Thanks >> >> Dan >> >>> >>> protected: >>> struct AnalogueGainLinear {
diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp index dcd69d9f2bbb..0a7b66cd003a 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 get's 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 is lost + * 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..42bdb3c5550f 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; + virtual double quantizeGain(double gain, double *quantizationGain) const; protected: struct AnalogueGainLinear {
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> --- src/ipa/libipa/camera_sensor_helper.cpp | 23 +++++++++++++++++++++++ src/ipa/libipa/camera_sensor_helper.h | 1 + 2 files changed, 24 insertions(+)