[libcamera-devel] [PATCH 1/4] libipa: camera_sensor_helper: Reorganize gain constants
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Mar 28 18:27:51 CEST 2022
On Mon, Mar 28, 2022 at 03:27:06PM +0200, Jacopo Mondi wrote:
> Hi Laurent
>
> On Mon, Mar 28, 2022 at 03:03:33PM +0300, Laurent Pinchart via libcamera-devel wrote:
> > To prepare for other gain models than the linear model, store the gain
> > constants in a union with per-model members. Due to the lack of
> > designated initializer support in gcc with C++17, initializing a single
> > complex structure that includes a union will be difficult. Split the
> > gain model type to a separate variable to work around this issue.
>
> Looking ahead, this makes sense.
>
> Too bad having something like
>
> struct {
> AnalogueGainType type_;
> AnalogueGainConstants constants_;
> } gain;
>
> makes initialization more complex, so separate fields are ok
It's the union in AnalogueGainConstants that messes it up :-S gcc has
partial support of designated initializers in C++, it only accepts them
when all the members are initialized by name (clang is happy with them,
I wish there would be a gcc switch to enable them even with older C++
standards). We would need to switch to C++20 to have proper designated
initializers support with gcc, I don't think that an option.
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/ipa/libipa/camera_sensor_helper.cpp | 102 ++++++++++++++----------
> > src/ipa/libipa/camera_sensor_helper.h | 10 ++-
> > 2 files changed, 65 insertions(+), 47 deletions(-)
> >
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > index c953def04fd7..714cd86f039f 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -58,11 +58,13 @@ namespace ipa {
> > */
> > uint32_t CameraSensorHelper::gainCode(double gain) const
> > {
> > - ASSERT(analogueGainConstants_.m0 == 0 || analogueGainConstants_.m1 == 0);
> > - ASSERT(analogueGainConstants_.type == AnalogueGainLinear);
> > + const AnalogueGainConstants &k = gainConstants_;
> >
> > - return (analogueGainConstants_.c0 - analogueGainConstants_.c1 * gain) /
> > - (analogueGainConstants_.m1 * gain - analogueGainConstants_.m0);
> > + ASSERT(gainType_ == AnalogueGainLinear);
> > + ASSERT(k.linear.m0 == 0 || k.linear.m1 == 0);
> > +
> > + return (k.linear.c0 - k.linear.c1 * gain) /
> > + (k.linear.m1 * gain - k.linear.m0);
> > }
> >
> > /**
> > @@ -80,11 +82,13 @@ uint32_t CameraSensorHelper::gainCode(double gain) const
> > */
> > double CameraSensorHelper::gain(uint32_t gainCode) const
> > {
> > - ASSERT(analogueGainConstants_.m0 == 0 || analogueGainConstants_.m1 == 0);
> > - ASSERT(analogueGainConstants_.type == AnalogueGainLinear);
> > + const AnalogueGainConstants &k = gainConstants_;
> >
> > - return (analogueGainConstants_.m0 * static_cast<double>(gainCode) + analogueGainConstants_.c0) /
> > - (analogueGainConstants_.m1 * static_cast<double>(gainCode) + analogueGainConstants_.c1);
> > + ASSERT(gainType_ == AnalogueGainLinear);
> > + ASSERT(k.linear.m0 == 0 || k.linear.m1 == 0);
> > +
> > + return (k.linear.m0 * static_cast<double>(gainCode) + k.linear.c0) /
> > + (k.linear.m1 * static_cast<double>(gainCode) + k.linear.c1);
> > }
> >
> > /**
> > @@ -127,42 +131,45 @@ double CameraSensorHelper::gain(uint32_t gainCode) const
> > * \todo not implemented in libipa
> > */
> >
> > +/**
> > + * \struct CameraSensorHelper::AnalogueGainLinearConstants
> > + * \brief Analogue gain constants for the linear gain model
> > + *
> > + * \var CameraSensorHelper::AnalogueGainLinearConstants::m0
> > + * \brief Constant used in the linear gain coding/decoding
> > + *
> > + * \note Either m0 or m1 shall be zero.
> > + *
> > + * \var CameraSensorHelper::AnalogueGainLinearConstants::c0
> > + * \brief Constant used in the linear gain coding/decoding
> > + *
> > + * \var CameraSensorHelper::AnalogueGainLinearConstants::m1
> > + * \brief Constant used in the linear gain coding/decoding
> > + *
> > + * \note Either m0 or m1 shall be zero.
> > + *
> > + * \var CameraSensorHelper::AnalogueGainLinearConstants::c1
> > + * \brief Constant used in the linear gain coding/decoding
> > + */
> > +
> > /**
> > * \struct CameraSensorHelper::AnalogueGainConstants
> > - * \brief Analogue gain constants used for gain calculation
> > - */
> > -
> > -/**
> > - * \var CameraSensorHelper::AnalogueGainConstants::type
> > - * \brief Analogue gain calculation mode
> > - */
> > -
> > -/**
> > - * \var CameraSensorHelper::AnalogueGainConstants::m0
> > - * \brief Constant used in the analogue Gain coding/decoding
> > + * \brief Analogue gain model constants
> > *
> > - * \note Either m0 or m1 shall be zero.
> > - */
> > -
> > -/**
> > - * \var CameraSensorHelper::AnalogueGainConstants::c0
> > - * \brief Constant used in the analogue gain coding/decoding
> > - */
> > -
> > -/**
> > - * \var CameraSensorHelper::AnalogueGainConstants::m1
> > - * \brief Constant used in the analogue gain coding/decoding
> > + * This union stores the constants used to calculate the analogue gain. The
> > + * CameraSensorHelper::gainType_ variable selects which union member is valid.
> > *
> > - * \note Either m0 or m1 shall be zero.
> > + * \var CameraSensorHelper::AnalogueGainConstants::linear
> > + * \brief Constants for the linear gain model
> > */
> >
> > /**
> > - * \var CameraSensorHelper::AnalogueGainConstants::c1
> > - * \brief Constant used in the analogue gain coding/decoding
> > + * \var CameraSensorHelper::gainType_
> > + * \brief The analogue gain model type
> > */
> >
> > /**
> > - * \var CameraSensorHelper::analogueGainConstants_
> > + * \var CameraSensorHelper::gainConstants_
> > * \brief The analogue gain parameters used for calculation
> > *
> > * The analogue gain is calculated through a formula, and its parameters are
> > @@ -290,7 +297,8 @@ class CameraSensorHelperImx219 : public CameraSensorHelper
> > public:
> > CameraSensorHelperImx219()
> > {
> > - analogueGainConstants_ = { AnalogueGainLinear, 0, 256, -1, 256 };
> > + gainType_ = AnalogueGainLinear;
> > + gainConstants_.linear = { 0, 256, -1, 256 };
> > }
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("imx219", CameraSensorHelperImx219)
> > @@ -298,10 +306,11 @@ REGISTER_CAMERA_SENSOR_HELPER("imx219", CameraSensorHelperImx219)
> > class CameraSensorHelperImx258 : public CameraSensorHelper
> > {
> > public:
> > - CameraSensorHelperImx258()
> > - {
> > - analogueGainConstants_ = { AnalogueGainLinear, 0, 512, -1, 512 };
> > - }
> > + CameraSensorHelperImx258()
> > + {
> > + gainType_ = AnalogueGainLinear;
> > + gainConstants_.linear = { 0, 512, -1, 512 };
> > + }
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("imx258", CameraSensorHelperImx258)
> >
> > @@ -310,7 +319,8 @@ class CameraSensorHelperOv2740 : public CameraSensorHelper
> > public:
> > CameraSensorHelperOv2740()
> > {
> > - analogueGainConstants_ = { AnalogueGainLinear, 1, 0, 0, 128 };
> > + gainType_ = AnalogueGainLinear;
> > + gainConstants_.linear = { 1, 0, 0, 128 };
> > }
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("ov2740", CameraSensorHelperOv2740)
> > @@ -320,7 +330,8 @@ class CameraSensorHelperOv5670 : public CameraSensorHelper
> > public:
> > CameraSensorHelperOv5670()
> > {
> > - analogueGainConstants_ = { AnalogueGainLinear, 1, 0, 0, 128 };
> > + gainType_ = AnalogueGainLinear;
> > + gainConstants_.linear = { 1, 0, 0, 128 };
> > }
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("ov5670", CameraSensorHelperOv5670)
> > @@ -330,7 +341,8 @@ class CameraSensorHelperOv5693 : public CameraSensorHelper
> > public:
> > CameraSensorHelperOv5693()
> > {
> > - analogueGainConstants_ = { AnalogueGainLinear, 1, 0, 0, 16 };
> > + gainType_ = AnalogueGainLinear;
> > + gainConstants_.linear = { 1, 0, 0, 16 };
> > }
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("ov5693", CameraSensorHelperOv5693)
> > @@ -340,7 +352,8 @@ class CameraSensorHelperOv8865 : public CameraSensorHelper
> > public:
> > CameraSensorHelperOv8865()
> > {
> > - analogueGainConstants_ = { AnalogueGainLinear, 1, 0, 0, 128 };
> > + gainType_ = AnalogueGainLinear;
> > + gainConstants_.linear = { 1, 0, 0, 128 };
> > }
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("ov8865", CameraSensorHelperOv8865)
> > @@ -350,7 +363,8 @@ class CameraSensorHelperOv13858 : public CameraSensorHelper
> > public:
> > CameraSensorHelperOv13858()
> > {
> > - analogueGainConstants_ = { AnalogueGainLinear, 1, 0, 0, 128 };
> > + gainType_ = AnalogueGainLinear;
> > + gainConstants_.linear = { 1, 0, 0, 128 };
> > }
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("ov13858", CameraSensorHelperOv13858)
> > diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h
> > index 26adfcb5955f..6b96520ba601 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.h
> > +++ b/src/ipa/libipa/camera_sensor_helper.h
> > @@ -34,15 +34,19 @@ protected:
> > AnalogueGainExponential,
> > };
> >
> > - struct AnalogueGainConstants {
> > - AnalogueGainType type;
> > + struct AnalogueGainLinearConstants {
> > int16_t m0;
> > int16_t c0;
> > int16_t m1;
> > int16_t c1;
> > };
> >
> > - AnalogueGainConstants analogueGainConstants_;
> > + union AnalogueGainConstants {
> > + AnalogueGainLinearConstants linear;
> > + };
> > +
> > + AnalogueGainType gainType_;
> > + AnalogueGainConstants gainConstants_;
> >
> > private:
> > LIBCAMERA_DISABLE_COPY_AND_MOVE(CameraSensorHelper)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list