Message ID | 20220328120336.10834-2-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
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 Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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 >
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@jmondi.org> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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)
Hi Laurent Thank you for the patch On 3/28/22 17:33, 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. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Umang Jain <umang.jain@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)
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. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@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 >
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)
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. Signed-off-by: Laurent Pinchart <laurent.pinchart@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(-)