[RFC/PATCH] libcamera: libipa: camera_sensor: Add onsemi AR0144 sensor properties
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Jun 4 11:51:04 CEST 2024
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'.
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.
--
Kieran
> ---
> 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