[RFC/PATCH] libcamera: libipa: camera_sensor: Add onsemi AR0144 sensor properties
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Jun 4 22:14:31 CEST 2024
On Tue, Jun 04, 2024 at 10:51:04AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-06-04 00:03:00)
> > Provide the onsemi AR0144 camera sensor properties and registration with
> > libipa for the gain code helpers.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > I'm working on a driver for the AR0144 camera sensor, which implements
> > an exotic gain model. While I would normally post the kernel driver
> > before submitting the libcamera integration patch, I thought reversing
> > the order would be useful for discussion purpose, as we recently
> > resurected the topic of gain rounding (see
> > https://patchwork.libcamera.org/cover/20097/).
> >
> > The approach I've taken here is to ensure that the code -> gain -> code
> > roundtrip conversion always results to the same gain code, for all valid
> > gain codes. To achieve this, I had to increase the multiple m in the
> > gain() function by the machine epsilon. In theory increasing other
> > factors and terms in the formula may have been needed, but this proved
> > enough in practice by running the roundtrip conversion on all possible
> > code values.
> >
> > One challenge to test this in unit tests is that the valid gain code
> > ranges is not contiguous. The sensor rounds the fine gain differently
> > depending on the coarse gain, which creates holes in the range. It would
> > be feasible (and is considered) to extend the helpers to expose the
> > [min, max] range of valid codes, but exposing arbitrary holes is more
> > problematic. One option would be to specify that the gain() function
> > should return NaN when the gain code is invalid. To make this safe, we
> > would need to prove that the gainCode() function will never an invalid
> > gain code, ideally formally but possibly by brute force.
>
> I like this proposal. I'm very much in favour of us trying to validate
> each helper independently. And likely brute force (as it's relatively
> cheap in reality). Otherwise we won't be able to test 'all helpers'.
We can't brute-force the proof for the generic helpers unfortunately.
All we can brute-force is each subclass. That's fine as a first step,
but formally proving the generic helpers would be nicer.
> I think adding a min()/max() accessor to the helpers is reasonable to
> achieve this even if it means hardcoding additional properties of a
> sensor in the helpers.
Or a range() function that returns the min and max, as callers will
likely need them both. Sounds good to me.
> > ---
> > src/ipa/libipa/camera_sensor_helper.cpp | 87 +++++++++++++++++++
> > .../sensor/camera_sensor_properties.cpp | 9 ++
> > 2 files changed, 96 insertions(+)
> >
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp
> > index 782ff9904e81..c2be011300b1 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -8,6 +8,7 @@
> > #include "camera_sensor_helper.h"
> >
> > #include <cmath>
> > +#include <limits>
> >
> > #include <libcamera/base/log.h>
> >
> > @@ -366,6 +367,92 @@ static constexpr double expGainDb(double step)
> > return log2_10 * step / 20;
> > }
> >
> > +class CameraSensorHelperAr0144 : public CameraSensorHelper
> > +{
> > +public:
> > + uint32_t gainCode(double gain) const override
> > + {
> > + /* The recommended minimum gain is 1.6842 to avoid artifacts. */
> > + gain = std::clamp(gain, 1.0 / (1.0 - 13.0 / 32.0), 18.45);
> > +
> > + /*
> > + * The analogue gain is made of a coarse exponential gain in
> > + * the range [2^0, 2^4] and a fine inversely linear gain in the
> > + * range [1.0, 2.0[. There is an additional fixed 1.153125
> > + * multiplier when the coarse gain reaches 2^2.
> > + */
> > +
> > + if (gain > 4.0)
> > + gain /= 1.153125;
> > +
> > + unsigned int coarse = std::log2(gain);
> > + unsigned int fine = (1 - (1 << coarse) / gain) * 32;
> > +
> > + /* The fine gain rounding depends on the coarse gain. */
> > + if (coarse == 1 || coarse == 3)
> > + fine &= ~1;
> > + else if (coarse == 4)
> > + fine &= ~3;
> > +
> > + return (coarse << 4) | (fine & 0xf);
> > + }
> > +
> > + double gain(uint32_t gainCode) const override
> > + {
> > + unsigned int coarse = gainCode >> 4;
> > + unsigned int fine = gainCode & 0xf;
> > + unsigned int d1;
> > + double d2, m;
> > +
> > + switch (coarse) {
> > + case 0:
> > + d1 = 1;
> > + d2 = 32.0;
> > + m = 1.0;
> > + break;
> > + case 1:
> > + d1 = 2;
> > + d2 = 16.0;
> > + m = 1.0;
> > + break;
> > + case 2:
> > + d1 = 1;
> > + d2 = 32.0;
> > + m = 1.153125;
> > + break;
> > + case 3:
> > + d1 = 2;
> > + d2 = 16.0;
> > + m = 1.153125;
> > + break;
> > + case 4:
> > + d1 = 4;
> > + d2 = 8.0;
> > + m = 1.153125;
> > + break;
> > + }
> > +
> > + /*
> > + * With infinite precision, the calculated gain would be exact,
> > + * and the reverse conversion with gainCode() would produce the
> > + * same gain code. In the real world, rounding errors may cause
> > + * the calculated gain to be lower by an amount negligible for
> > + * all purposes, except for the reverse conversion. Converting
> > + * the gain to a gain code could then return the quantized value
> > + * just lower than the original gain code. To avoid this, tests
> > + * showed that adding the machine epsilon to the multiplier m is
> > + * sufficient.
> > + */
> > + m += std::numeric_limits<decltype(m)>::epsilon();
> > +
> > + return m * (1 << coarse) / (1.0 - (fine / d1) / d2);
> > + }
> > +
> > +private:
> > + static constexpr double kStep_ = 16;
> > +};
> > +REGISTER_CAMERA_SENSOR_HELPER("ar0144", CameraSensorHelperAr0144)
> > +
> > class CameraSensorHelperAr0521 : public CameraSensorHelper
> > {
> > public:
> > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp
> > index b18524d85b37..4e5217ab51ef 100644
> > --- a/src/libcamera/sensor/camera_sensor_properties.cpp
> > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp
> > @@ -52,6 +52,15 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)
> > const CameraSensorProperties *CameraSensorProperties::get(const std::string &sensor)
> > {
> > static const std::map<std::string, const CameraSensorProperties> sensorProps = {
> > + { "ar0144", {
> > + .unitCellSize = { 3000, 3000 },
> > + .testPatternModes = {
> > + { controls::draft::TestPatternModeOff, 0 },
> > + { controls::draft::TestPatternModeSolidColor, 1 },
> > + { controls::draft::TestPatternModeColorBars, 2 },
> > + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
> > + },
> > + } },
> > { "ar0521", {
> > .unitCellSize = { 2200, 2200 },
> > .testPatternModes = {
> >
> > base-commit: 6cd17515ffeb67fb38ffcc4d57aadf9732b54800
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list