[PATCH v3 1/6] ipa: libipa: Add colour helpers

[Laurent Pinchart]

Hi Dan,

Thank you for the patch.

On Fri, Nov 15, 2024 at 07:46:23AM +0000, Daniel Scally wrote:
> We start to have some functions relating to colour 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>
> ---
> Changes in v3:
> 
> 	- Named the file "colours.h" and "colours.cpp" instead of "helpers"
> 	- Made the rec601LuminanceFromRGB() arguments doubles
> 
> Changes in v2:
> 
> 	- Dropped the Q(m, n) format helpers until they're needed.
> 	- \return statements after long description
> 	- Switched rec601LuminanceFromRGB() to return a double
> 
>  src/ipa/libipa/colours.cpp | 77 ++++++++++++++++++++++++++++++++++++++
>  src/ipa/libipa/colours.h   | 21 +++++++++++
>  src/ipa/libipa/meson.build |  2 +
>  3 files changed, 100 insertions(+)
>  create mode 100644 src/ipa/libipa/colours.cpp
>  create mode 100644 src/ipa/libipa/colours.h
> 
> diff --git a/src/ipa/libipa/colours.cpp b/src/ipa/libipa/colours.cpp
> new file mode 100644
> index 00000000..9fcb53b0
> --- /dev/null
> +++ b/src/ipa/libipa/colours.cpp
> @@ -0,0 +1,77 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy
> + *
> + * libipa miscellaneous colour helpers
> + */
> +
> +#include "colours.h"
> +
> +#include <algorithm>
> +#include <cmath>
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +/**
> + * \file colours.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 value
> + *
> + * 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
> + *
> + * \return The estimated luminance value
> + */
> +double rec601LuminanceFromRGB(double r, double g, double b)
> +{
> +	return (r * .299) + (g * .587) + (b * .114);
> +}

I've sent "[RFC PATCH v2 00/12] Improve linear algebra helpers in
libipa" which would allow rewriting this function as

double rec601LuminanceFromRGB(RGB<double> rgb)
{
	static constexpr RGB<double> rgb2y{{ 0.299, 0.587, 0.114 }};

	return rgb.dot(rgb2y);
}

But I think we could take it one step further, and instead define RGB to
YUV conversion matrices (and their inverse matrices) in colour.h for
common colour spaces.

The Agc::estimateLuminance() function could then be written as

double Agc::estimateLuminance(double gain) const
{
	RGB<double> sum;

	for (const auto rgb : rgbTriples_) {
		RGB<double> rgbf = rgb;
		rgbf *= gain;
		sum += rgbf.min(255.0);
	}

	Vector<3, double> yuv = kBt601Rgb2YuvMatrix * (sum * gain_);
	return yuv[0] / (bdsGrid_.height * bdsGrid_.width) / 255;
}

The rgbf variable is needed as rgbTriples_ would store RGB<uint8_t>
values. We may need further improvements to the Vector class operators,
when multiplying a Vector<integer-type> by a double, we may want to
produce a Vector<double>.

We don't have to wait until "[RFC PATCH v2 00/12] Improve linear algebra
helpers in libipa" gets reviewed and merged, so

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Could you then improve the code on top ?

> +
> +/**
> + * \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
> + *
> + * 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
> + *
> + * \return The estimated color temperature
> + */
> +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;
> +}
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/colours.h b/src/ipa/libipa/colours.h
> new file mode 100644
> index 00000000..b42ed0ac
> --- /dev/null
> +++ b/src/ipa/libipa/colours.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2024, Ideas on Board Oy
> + *
> + * libipa miscellaneous colour helpers
> + */
> +
> +#pragma once
> +
> +#include <stdint.h>
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +double rec601LuminanceFromRGB(double r, double g, double b);
> +uint32_t estimateCCT(double red, double green, double blue);
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index e78cbcd6..788d037a 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -4,6 +4,7 @@ libipa_headers = files([
>      'agc_mean_luminance.h',
>      'algorithm.h',
>      'camera_sensor_helper.h',
> +    'colours.h',
>      'exposure_mode_helper.h',
>      'fc_queue.h',
>      'histogram.h',
> @@ -19,6 +20,7 @@ libipa_sources = files([
>      'agc_mean_luminance.cpp',
>      'algorithm.cpp',
>      'camera_sensor_helper.cpp',
> +    'colours.cpp',
>      'exposure_mode_helper.cpp',
>      'fc_queue.cpp',
>      'histogram.cpp',

-- 
Regards,

Laurent Pinchart