[PATCH 1/6] ipa: libipa: Add miscellaneous helpers

[Dan Scally]

Hi Jacopo - thanks for the review

On 04/11/2024 11:07, Jacopo Mondi wrote:
> Hi Dan
>
> On Thu, Oct 31, 2024 at 04:07:36PM +0000, Daniel Scally wrote:
>> We start to have functions that are effectively identical crop up
>> across the IPA modules. Add a file allowing those to be centralised
>> within libipa so that a single implementation can be used in all of
>> the IPAs.
>>
>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
>> ---
>>   src/ipa/libipa/helpers.cpp | 103 +++++++++++++++++++++++++++++++++++++
>>   src/ipa/libipa/helpers.h   |  23 +++++++++
>>   src/ipa/libipa/meson.build |   2 +
>>   3 files changed, 128 insertions(+)
>>   create mode 100644 src/ipa/libipa/helpers.cpp
>>   create mode 100644 src/ipa/libipa/helpers.h
>>
>> diff --git a/src/ipa/libipa/helpers.cpp b/src/ipa/libipa/helpers.cpp
>> new file mode 100644
>> index 00000000..cc0cd586
>> --- /dev/null
>> +++ b/src/ipa/libipa/helpers.cpp
> Not a huge fan of a collection of C functions, but I guess
> alternatives could be considered over-design
That was my thinking basically.
>
>> @@ -0,0 +1,103 @@
>> +/* SPDX-License-Identifier: BSD-2-Clause */
>> +/*
>> + * Copyright (C) 2024, Ideas on Board Oy
>> + *
>> + * libipa miscellaneous helpers
>> + */
>> +
>> +#include "helpers.h"
>> +
>> +#include <algorithm>
>> +#include <cmath>
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa {
>> +
>> +/**
>> + * \file helpers.h
>> + * \brief Functions to reduce code duplication between IPA modules
>> + */
>> +
>> +/**
>> + * \brief Estimate luminance from RGB values following ITU-R BT.601
>> + * \param[in] r The red value
>> + * \param[in] g The green value
>> + * \param[in] b The blue valye
>> + * \return The estimated luminance value
> \return statements usually go after the longer function documentation


Ack

>
>
>> + *
>> + * This function estimates a luminance value from a triplet of Red, Green and
>> + * Blue values, following the formula defined by ITU-R Recommendation BT.601-7
>> + * which can be found at https://www.itu.int/rec/R-REC-BT.601
>> + */
>> +unsigned int rec601LuminanceFromRGB(unsigned int r, unsigned int g, unsigned int b)
> In example, in the RPi IPA, the return value is assigned to a double,
> and the calculation suggests it might be worth not casing the result to
> unsigned int.
You're right, that's wrong. Thanks.
>
>> +{
>> +	return (r * .299) + (g * .587) + (b * .114);
>> +}
>> +
>> +/**
>> + * \brief Estimate correlated colour temperature from RGB color space input
>> + * \param[in] red The input red value
>> + * \param[in] green The input green value
>> + * \param[in] blue The input blue value
>> + * \return The estimated color temperature
> same here
>
>> + *
>> + * This function estimates the correlated color temperature RGB color space
>> + * input. In physics and color science, the Planckian locus or black body locus
>> + * is the path or locus that the color of an incandescent black body would take
>> + * in a particular chromaticity space as the blackbody temperature changes.
>> + *
>> + * If a narrow range of color temperatures is considered (those encapsulating
>> + * daylight being the most practical case) one can approximate the Planckian
>> + * locus in order to calculate the CCT in terms of chromaticity coordinates.
>> + *
>> + * More detailed information can be found in:
>> + * https://en.wikipedia.org/wiki/Color_temperature#Approximation
>> + */
>> +uint32_t estimateCCT(double red, double green, double blue)
>> +{
>> +	/* Convert the RGB values to CIE tristimulus values (XYZ) */
>> +	double X = (-0.14282) * (red) + (1.54924) * (green) + (-0.95641) * (blue);
>> +	double Y = (-0.32466) * (red) + (1.57837) * (green) + (-0.73191) * (blue);
>> +	double Z = (-0.68202) * (red) + (0.77073) * (green) + (0.56332) * (blue);
>> +
>> +	/* Calculate the normalized chromaticity values */
>> +	double x = X / (X + Y + Z);
>> +	double y = Y / (X + Y + Z);
>> +
>> +	/* Calculate CCT */
>> +	double n = (x - 0.3320) / (0.1858 - y);
>> +	return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33;
>> +}
> This seems to come straight from the IPU3 IPA, so ack to the text and
> implementation
>
>> +
>> +/**
>> + * \brief Convert double to Q(m.n) format
> Note these two helpers seems to be un-used.
Ah yes, they're used in the C55 series which will be based on top of this...I can introduce them 
there if that's better?
> What Q formats are we referring to ? Wikipedia lists two (TI version
> and ARM version).
> https://en.wikipedia.org/wiki/Q_(number_format)


So far I'm only using numbers specified in documentation as being unsigned, and so it's the TI 
version, but the "UQ" format rather than just "Q"...possibly renaming the function 
"toUnsignedQFormat()" would be clearer?

>
>> + * \param[in] value The value to convert
>> + * \param[in] m The number of bits used to represent the integer part
>> + * \param[in] n The number of bits used to represent the fraction part
>> + *
>> + * This function converts a double into an unsigned Q format, clamping at the
>> + * largest possible value given m and n.
> \return ?
>
>> + */
>> +unsigned int toQFormat(double value, unsigned int m, unsigned int n)
>> +{
>> +	double maxVal = (std::pow(2, m + n) - 1) / std::pow(2, n);
>> +
>> +	return std::clamp(value, 0.0, maxVal) * std::pow(2, n);
>> +}
>> +
>> +/**
>> + * \brief Convert Q(m.n) formatted number to double
>> + * \param[in] value The value to convert
>> + * \param[in] n The number of bits used to represent the fraction part
>> + *
>> + * This function converts an unsigned Q formatted value into a double.
>> + */
>> +double fromQFormat(unsigned int value, unsigned int n)
>> +{
>> +	return value / std::pow(2, n);
>> +}
> Let's discuss these once the above is clarified
>
>> +
>> +} /* namespace ipa */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/libipa/helpers.h b/src/ipa/libipa/helpers.h
>> new file mode 100644
>> index 00000000..361b63bd
>> --- /dev/null
>> +++ b/src/ipa/libipa/helpers.h
>> @@ -0,0 +1,23 @@
>> +/* SPDX-License-Identifier: BSD-2-Clause */
>> +/*
>> + * Copyright (C) 2024, Ideas on Board Oy
>> + *
>> + * libipa miscellaneous helpers
>> + */
>> +
>> +#pragma once
>> +
>> +#include <stdint.h>
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa {
>> +
>> +unsigned int rec601LuminanceFromRGB(unsigned int r, unsigned int g, unsigned int b);
>> +uint32_t estimateCCT(double red, double green, double blue);
>> +unsigned int toQFormat(double value, unsigned int m, unsigned int n);
>> +double fromQFormat(unsigned int value, unsigned int n);
>> +
>> +} /* namespace ipa */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
>> index e78cbcd6..ca4f3e28 100644
>> --- a/src/ipa/libipa/meson.build
>> +++ b/src/ipa/libipa/meson.build
>> @@ -6,6 +6,7 @@ libipa_headers = files([
>>       'camera_sensor_helper.h',
>>       'exposure_mode_helper.h',
>>       'fc_queue.h',
>> +    'helpers.h',
>>       'histogram.h',
>>       'interpolator.h',
>>       'lsc_polynomial.h',
>> @@ -21,6 +22,7 @@ libipa_sources = files([
>>       'camera_sensor_helper.cpp',
>>       'exposure_mode_helper.cpp',
>>       'fc_queue.cpp',
>> +    'helpers.cpp',
>>       'histogram.cpp',
>>       'interpolator.cpp',
>>       'lsc_polynomial.cpp',
>> --
>> 2.30.2
>>