| Message ID | 20240703104004.184783-2-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Wed, Jul 03, 2024 at 12:39:48PM +0200, Stefan Klug wrote: > For a proper tuning process we need to know the sensor black levels. In > most cases these are fixed and not reported by the kernel driver. Store > them inside the sensor helpers for later retrieval by the algorithms. > > Add black level value corresponding to the data pedestal for three > initial sensors as documented in the datasheets. More should be added, > eventually filling the gaps for all supported sensors. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/ipa/libipa/camera_sensor_helper.cpp | 27 +++++++++++++++++++++++++ > src/ipa/libipa/camera_sensor_helper.h | 3 +++ > 2 files changed, 30 insertions(+) > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp > index 782ff9904e81..3d0e756927f0 100644 > --- a/src/ipa/libipa/camera_sensor_helper.cpp > +++ b/src/ipa/libipa/camera_sensor_helper.cpp > @@ -47,6 +47,30 @@ namespace ipa { > * function. > */ > > +/** > + * \fn CameraSensorHelper::blackLevel() > + * \brief Fetch the black level of the sensor > + * > + * This function returns the black level of the sensor scaled to a 16bit pixel > + * width. If it is unknown an empty optional is returned. > + * > + * Black levels are typically the result of the following phenomena: > + * - Pedestal added by the sensor to pixel values. They are typically fixed, > + * sometimes programmable and should be reported in datasheets (but > + * documentation is not always available). > + * - Dark currents and other physical effects that add charge to pixels in the > + * absence of light. Those can depend on the integration time and the sensor > + * die temperature, and their contribution to pixel values depend on the > + * sensor gains. > + * > + * The pedestal is usually the value with the biggest contribution to the > + * overall black level. In most cases it is either known before or in rare cases > + * (There is not a single driver with such a control in the linux kernel) can be > + * queried from the sensor. This function shall provide that fixed, known value. > + * > + * \return The black level of the sensor, or std::nullopt if not known > + */ > + > /** > * \brief Compute gain code from the analogue gain absolute value > * \param[in] gain The real gain to pass > @@ -396,6 +420,7 @@ class CameraSensorHelperImx219 : public CameraSensorHelper > public: > CameraSensorHelperImx219() > { > + blackLevel_ = 4096; > gainType_ = AnalogueGainLinear; > gainConstants_.linear = { 0, 256, -1, 256 }; > } > @@ -407,6 +432,7 @@ class CameraSensorHelperImx258 : public CameraSensorHelper > public: > CameraSensorHelperImx258() > { > + blackLevel_ = 4096; > gainType_ = AnalogueGainLinear; > gainConstants_.linear = { 0, 512, -1, 512 }; > } > @@ -456,6 +482,7 @@ class CameraSensorHelperImx335 : public CameraSensorHelper > public: > CameraSensorHelperImx335() > { > + blackLevel_ = 3200; > gainType_ = AnalogueGainExponential; > gainConstants_.exp = { 1.0, expGainDb(0.3) }; > } > diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h > index 0d99073bea82..ac276e27f523 100644 > --- a/src/ipa/libipa/camera_sensor_helper.h > +++ b/src/ipa/libipa/camera_sensor_helper.h > @@ -10,6 +10,7 @@ > #include <stdint.h> > > #include <memory> > +#include <optional> > #include <string> > #include <vector> > > @@ -25,6 +26,7 @@ public: > CameraSensorHelper() = default; > virtual ~CameraSensorHelper() = default; > > + std::optional<int16_t> blackLevel() const { return blackLevel_; } > virtual uint32_t gainCode(double gain) const; > virtual double gain(uint32_t gainCode) const; > > @@ -51,6 +53,7 @@ protected: > AnalogueGainExpConstants exp; > }; > > + std::optional<int16_t> blackLevel_; > AnalogueGainType gainType_; > AnalogueGainConstants gainConstants_; > > -- > 2.43.0 >
Quoting Stefan Klug (2024-07-03 11:39:48) > For a proper tuning process we need to know the sensor black levels. In > most cases these are fixed and not reported by the kernel driver. Store > them inside the sensor helpers for later retrieval by the algorithms. > > Add black level value corresponding to the data pedestal for three > initial sensors as documented in the datasheets. More should be added, > eventually filling the gaps for all supported sensors. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/ipa/libipa/camera_sensor_helper.cpp | 27 +++++++++++++++++++++++++ > src/ipa/libipa/camera_sensor_helper.h | 3 +++ > 2 files changed, 30 insertions(+) > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp > index 782ff9904e81..3d0e756927f0 100644 > --- a/src/ipa/libipa/camera_sensor_helper.cpp > +++ b/src/ipa/libipa/camera_sensor_helper.cpp > @@ -47,6 +47,30 @@ namespace ipa { > * function. > */ > > +/** > + * \fn CameraSensorHelper::blackLevel() > + * \brief Fetch the black level of the sensor > + * > + * This function returns the black level of the sensor scaled to a 16bit pixel > + * width. If it is unknown an empty optional is returned. > + * > + * Black levels are typically the result of the following phenomena: > + * - Pedestal added by the sensor to pixel values. They are typically fixed, > + * sometimes programmable and should be reported in datasheets (but > + * documentation is not always available). > + * - Dark currents and other physical effects that add charge to pixels in the > + * absence of light. Those can depend on the integration time and the sensor > + * die temperature, and their contribution to pixel values depend on the > + * sensor gains. > + * > + * The pedestal is usually the value with the biggest contribution to the > + * overall black level. In most cases it is either known before or in rare cases > + * (There is not a single driver with such a control in the linux kernel) can be > + * queried from the sensor. This function shall provide that fixed, known value. > + * > + * \return The black level of the sensor, or std::nullopt if not known > + */ > + > /** > * \brief Compute gain code from the analogue gain absolute value > * \param[in] gain The real gain to pass > @@ -396,6 +420,7 @@ class CameraSensorHelperImx219 : public CameraSensorHelper > public: > CameraSensorHelperImx219() > { > + blackLevel_ = 4096; > gainType_ = AnalogueGainLinear; > gainConstants_.linear = { 0, 256, -1, 256 }; > } > @@ -407,6 +432,7 @@ class CameraSensorHelperImx258 : public CameraSensorHelper > public: > CameraSensorHelperImx258() > { > + blackLevel_ = 4096; > gainType_ = AnalogueGainLinear; > gainConstants_.linear = { 0, 512, -1, 512 }; > } > @@ -456,6 +482,7 @@ class CameraSensorHelperImx335 : public CameraSensorHelper > public: > CameraSensorHelperImx335() > { > + blackLevel_ = 3200; > gainType_ = AnalogueGainExponential; > gainConstants_.exp = { 1.0, expGainDb(0.3) }; > } > diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h > index 0d99073bea82..ac276e27f523 100644 > --- a/src/ipa/libipa/camera_sensor_helper.h > +++ b/src/ipa/libipa/camera_sensor_helper.h > @@ -10,6 +10,7 @@ > #include <stdint.h> > > #include <memory> > +#include <optional> > #include <string> > #include <vector> > > @@ -25,6 +26,7 @@ public: > CameraSensorHelper() = default; > virtual ~CameraSensorHelper() = default; > > + std::optional<int16_t> blackLevel() const { return blackLevel_; } > virtual uint32_t gainCode(double gain) const; > virtual double gain(uint32_t gainCode) const; > > @@ -51,6 +53,7 @@ protected: > AnalogueGainExpConstants exp; > }; > > + std::optional<int16_t> blackLevel_; Shouldn't the black level be a uint16_t ? Otherwise, this would then be limited to 15 bit max ? Can we expect a blacklevel above 32767? > AnalogueGainType gainType_; > AnalogueGainConstants gainConstants_; > > -- > 2.43.0 >
Quoting Kieran Bingham (2024-07-03 13:05:13) > Quoting Stefan Klug (2024-07-03 11:39:48) > > For a proper tuning process we need to know the sensor black levels. In > > most cases these are fixed and not reported by the kernel driver. Store > > them inside the sensor helpers for later retrieval by the algorithms. > > > > Add black level value corresponding to the data pedestal for three > > initial sensors as documented in the datasheets. More should be added, > > eventually filling the gaps for all supported sensors. > > > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > > --- > > src/ipa/libipa/camera_sensor_helper.cpp | 27 +++++++++++++++++++++++++ > > src/ipa/libipa/camera_sensor_helper.h | 3 +++ > > 2 files changed, 30 insertions(+) > > > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp > > index 782ff9904e81..3d0e756927f0 100644 > > --- a/src/ipa/libipa/camera_sensor_helper.cpp > > +++ b/src/ipa/libipa/camera_sensor_helper.cpp > > @@ -47,6 +47,30 @@ namespace ipa { > > * function. > > */ > > > > +/** > > + * \fn CameraSensorHelper::blackLevel() > > + * \brief Fetch the black level of the sensor > > + * > > + * This function returns the black level of the sensor scaled to a 16bit pixel > > + * width. If it is unknown an empty optional is returned. > > + * > > + * Black levels are typically the result of the following phenomena: > > + * - Pedestal added by the sensor to pixel values. They are typically fixed, > > + * sometimes programmable and should be reported in datasheets (but > > + * documentation is not always available). > > + * - Dark currents and other physical effects that add charge to pixels in the > > + * absence of light. Those can depend on the integration time and the sensor > > + * die temperature, and their contribution to pixel values depend on the > > + * sensor gains. > > + * > > + * The pedestal is usually the value with the biggest contribution to the > > + * overall black level. In most cases it is either known before or in rare cases > > + * (There is not a single driver with such a control in the linux kernel) can be > > + * queried from the sensor. This function shall provide that fixed, known value. > > + * > > + * \return The black level of the sensor, or std::nullopt if not known > > + */ > > + > > /** > > * \brief Compute gain code from the analogue gain absolute value > > * \param[in] gain The real gain to pass > > @@ -396,6 +420,7 @@ class CameraSensorHelperImx219 : public CameraSensorHelper > > public: > > CameraSensorHelperImx219() > > { > > + blackLevel_ = 4096; > > gainType_ = AnalogueGainLinear; > > gainConstants_.linear = { 0, 256, -1, 256 }; > > } > > @@ -407,6 +432,7 @@ class CameraSensorHelperImx258 : public CameraSensorHelper > > public: > > CameraSensorHelperImx258() > > { > > + blackLevel_ = 4096; > > gainType_ = AnalogueGainLinear; > > gainConstants_.linear = { 0, 512, -1, 512 }; > > } > > @@ -456,6 +482,7 @@ class CameraSensorHelperImx335 : public CameraSensorHelper > > public: > > CameraSensorHelperImx335() > > { > > + blackLevel_ = 3200; > > gainType_ = AnalogueGainExponential; > > gainConstants_.exp = { 1.0, expGainDb(0.3) }; > > } > > diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h > > index 0d99073bea82..ac276e27f523 100644 > > --- a/src/ipa/libipa/camera_sensor_helper.h > > +++ b/src/ipa/libipa/camera_sensor_helper.h > > @@ -10,6 +10,7 @@ > > #include <stdint.h> > > > > #include <memory> > > +#include <optional> > > #include <string> > > #include <vector> > > > > @@ -25,6 +26,7 @@ public: > > CameraSensorHelper() = default; > > virtual ~CameraSensorHelper() = default; > > > > + std::optional<int16_t> blackLevel() const { return blackLevel_; } > > virtual uint32_t gainCode(double gain) const; > > virtual double gain(uint32_t gainCode) const; > > > > @@ -51,6 +53,7 @@ protected: > > AnalogueGainExpConstants exp; > > }; > > > > + std::optional<int16_t> blackLevel_; > > Shouldn't the black level be a uint16_t ? Otherwise, this would then be > limited to 15 bit max ? Can we expect a blacklevel above 32767? > Given it's a known stipulation that we're using int16_t to read from the yaml files currently: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > AnalogueGainType gainType_; > > AnalogueGainConstants gainConstants_; > > > > -- > > 2.43.0 > >
Hi Stefan, Thank you for the patch. On Wed, Jul 03, 2024 at 12:39:48PM +0200, Stefan Klug wrote: > For a proper tuning process we need to know the sensor black levels. In > most cases these are fixed and not reported by the kernel driver. Store > them inside the sensor helpers for later retrieval by the algorithms. > > Add black level value corresponding to the data pedestal for three > initial sensors as documented in the datasheets. More should be added, > eventually filling the gaps for all supported sensors. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/ipa/libipa/camera_sensor_helper.cpp | 27 +++++++++++++++++++++++++ > src/ipa/libipa/camera_sensor_helper.h | 3 +++ > 2 files changed, 30 insertions(+) > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp > index 782ff9904e81..3d0e756927f0 100644 > --- a/src/ipa/libipa/camera_sensor_helper.cpp > +++ b/src/ipa/libipa/camera_sensor_helper.cpp > @@ -47,6 +47,30 @@ namespace ipa { > * function. > */ > > +/** > + * \fn CameraSensorHelper::blackLevel() > + * \brief Fetch the black level of the sensor > + * > + * This function returns the black level of the sensor scaled to a 16bit pixel > + * width. If it is unknown an empty optional is returned. Let's add * \todo Fill the blanks and add pedestal values for all supported sensors. Once * done, drop the std::optional<>. > + * > + * Black levels are typically the result of the following phenomena: > + * - Pedestal added by the sensor to pixel values. They are typically fixed, > + * sometimes programmable and should be reported in datasheets (but > + * documentation is not always available). > + * - Dark currents and other physical effects that add charge to pixels in the > + * absence of light. Those can depend on the integration time and the sensor > + * die temperature, and their contribution to pixel values depend on the > + * sensor gains. > + * > + * The pedestal is usually the value with the biggest contribution to the > + * overall black level. In most cases it is either known before or in rare cases > + * (There is not a single driver with such a control in the linux kernel) can be s/There/there/ > + * queried from the sensor. This function shall provide that fixed, known value. s/shall provide/provides/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + * > + * \return The black level of the sensor, or std::nullopt if not known > + */ > + > /** > * \brief Compute gain code from the analogue gain absolute value > * \param[in] gain The real gain to pass > @@ -396,6 +420,7 @@ class CameraSensorHelperImx219 : public CameraSensorHelper > public: > CameraSensorHelperImx219() > { > + blackLevel_ = 4096; > gainType_ = AnalogueGainLinear; > gainConstants_.linear = { 0, 256, -1, 256 }; > } > @@ -407,6 +432,7 @@ class CameraSensorHelperImx258 : public CameraSensorHelper > public: > CameraSensorHelperImx258() > { > + blackLevel_ = 4096; > gainType_ = AnalogueGainLinear; > gainConstants_.linear = { 0, 512, -1, 512 }; > } > @@ -456,6 +482,7 @@ class CameraSensorHelperImx335 : public CameraSensorHelper > public: > CameraSensorHelperImx335() > { > + blackLevel_ = 3200; > gainType_ = AnalogueGainExponential; > gainConstants_.exp = { 1.0, expGainDb(0.3) }; > } > diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h > index 0d99073bea82..ac276e27f523 100644 > --- a/src/ipa/libipa/camera_sensor_helper.h > +++ b/src/ipa/libipa/camera_sensor_helper.h > @@ -10,6 +10,7 @@ > #include <stdint.h> > > #include <memory> > +#include <optional> > #include <string> > #include <vector> > > @@ -25,6 +26,7 @@ public: > CameraSensorHelper() = default; > virtual ~CameraSensorHelper() = default; > > + std::optional<int16_t> blackLevel() const { return blackLevel_; } > virtual uint32_t gainCode(double gain) const; > virtual double gain(uint32_t gainCode) const; > > @@ -51,6 +53,7 @@ protected: > AnalogueGainExpConstants exp; > }; > > + std::optional<int16_t> blackLevel_; > AnalogueGainType gainType_; > AnalogueGainConstants gainConstants_; >
diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp index 782ff9904e81..3d0e756927f0 100644 --- a/src/ipa/libipa/camera_sensor_helper.cpp +++ b/src/ipa/libipa/camera_sensor_helper.cpp @@ -47,6 +47,30 @@ namespace ipa { * function. */ +/** + * \fn CameraSensorHelper::blackLevel() + * \brief Fetch the black level of the sensor + * + * This function returns the black level of the sensor scaled to a 16bit pixel + * width. If it is unknown an empty optional is returned. + * + * Black levels are typically the result of the following phenomena: + * - Pedestal added by the sensor to pixel values. They are typically fixed, + * sometimes programmable and should be reported in datasheets (but + * documentation is not always available). + * - Dark currents and other physical effects that add charge to pixels in the + * absence of light. Those can depend on the integration time and the sensor + * die temperature, and their contribution to pixel values depend on the + * sensor gains. + * + * The pedestal is usually the value with the biggest contribution to the + * overall black level. In most cases it is either known before or in rare cases + * (There is not a single driver with such a control in the linux kernel) can be + * queried from the sensor. This function shall provide that fixed, known value. + * + * \return The black level of the sensor, or std::nullopt if not known + */ + /** * \brief Compute gain code from the analogue gain absolute value * \param[in] gain The real gain to pass @@ -396,6 +420,7 @@ class CameraSensorHelperImx219 : public CameraSensorHelper public: CameraSensorHelperImx219() { + blackLevel_ = 4096; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 0, 256, -1, 256 }; } @@ -407,6 +432,7 @@ class CameraSensorHelperImx258 : public CameraSensorHelper public: CameraSensorHelperImx258() { + blackLevel_ = 4096; gainType_ = AnalogueGainLinear; gainConstants_.linear = { 0, 512, -1, 512 }; } @@ -456,6 +482,7 @@ class CameraSensorHelperImx335 : public CameraSensorHelper public: CameraSensorHelperImx335() { + blackLevel_ = 3200; gainType_ = AnalogueGainExponential; gainConstants_.exp = { 1.0, expGainDb(0.3) }; } diff --git a/src/ipa/libipa/camera_sensor_helper.h b/src/ipa/libipa/camera_sensor_helper.h index 0d99073bea82..ac276e27f523 100644 --- a/src/ipa/libipa/camera_sensor_helper.h +++ b/src/ipa/libipa/camera_sensor_helper.h @@ -10,6 +10,7 @@ #include <stdint.h> #include <memory> +#include <optional> #include <string> #include <vector> @@ -25,6 +26,7 @@ public: CameraSensorHelper() = default; virtual ~CameraSensorHelper() = default; + std::optional<int16_t> blackLevel() const { return blackLevel_; } virtual uint32_t gainCode(double gain) const; virtual double gain(uint32_t gainCode) const; @@ -51,6 +53,7 @@ protected: AnalogueGainExpConstants exp; }; + std::optional<int16_t> blackLevel_; AnalogueGainType gainType_; AnalogueGainConstants gainConstants_;
For a proper tuning process we need to know the sensor black levels. In most cases these are fixed and not reported by the kernel driver. Store them inside the sensor helpers for later retrieval by the algorithms. Add black level value corresponding to the data pedestal for three initial sensors as documented in the datasheets. More should be added, eventually filling the gaps for all supported sensors. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/ipa/libipa/camera_sensor_helper.cpp | 27 +++++++++++++++++++++++++ src/ipa/libipa/camera_sensor_helper.h | 3 +++ 2 files changed, 30 insertions(+)