[PATCH 1/3] ipa: libipa: Add Lux helper

[Kieran Bingham]

Hi Paul,

Quoting Paul Elder (2024-04-12 10:16:58)
> Add a Lux helper to libipa that does the estimation of the lux level
> given gain, exposure, aperture, and luminance histogram. The helper also
> handles reading the reference values from the tuning file. These are
> expected to be common operations of lux algorithm modules in IPAs, and
> is modeled/copied from Raspberry Pi.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/ipa/libipa/lux.cpp     | 119 +++++++++++++++++++++++++++++++++++++
>  src/ipa/libipa/lux.h       |  45 ++++++++++++++
>  src/ipa/libipa/meson.build |   2 +
>  3 files changed, 166 insertions(+)
>  create mode 100644 src/ipa/libipa/lux.cpp
>  create mode 100644 src/ipa/libipa/lux.h
> 
> diff --git a/src/ipa/libipa/lux.cpp b/src/ipa/libipa/lux.cpp
> new file mode 100644
> index 00000000..756cd3c4
> --- /dev/null
> +++ b/src/ipa/libipa/lux.cpp
> @@ -0,0 +1,119 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2019, Raspberry Pi Ltd
> + * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> + *
> + * lux.h - Helper class that implements lux estimation
> + */
> +#include "lux.h"
> +
> +#include <algorithm>
> +#include <chrono>
> +
> +#include <libcamera/base/log.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "histogram.h"
> +
> +/**
> + * \file lux.h
> + * \brief Helper class that implements lux estimation
> + *
> + * As estimating the lux level of an image is expected to be a common
> + * operation, it is implemented in a helper in libipa.

I think the brief should explain what it is and how to use it (or in
this case what it uses?) - not where it lives.

> + */
> +
> +namespace libcamera {
> +
> +using namespace std::literals::chrono_literals;
> +
> +LOG_DEFINE_CATEGORY(Lux)
> +
> +namespace ipa {
> +
> +/**
> + * \class Lux
> + * \brief Class that implements lux estimation
> + */
> +
> +/**
> + * \var Lux::referenceExposureTime_
> + * \brief The exposure time of the reference image, in microseconds.
> + */
> +
> +/**
> + * \var Lux::referenceGain_
> + * \brief The analogue gain of the reference image.
> + */
> +
> +/**
> + * \var Lux::referenceAperture_
> + * \brief The aperture of the reference image in units of 1/f.
> + */
> +
> +/**
> + * \var Lux::referenceY_
> + * \brief The measured luminance of the reference image, out of 65536.

Why 65536? Would it be better/more explicit to say that it's a 16 bit
value? Except of course that is BIT(16) so it's a 16-bit-value plus one?
Is 65536 a permitted value?

> + */
> +
> +/**
> + * \var Lux::referenceLux_
> + * \brief The estimated lux level of the reference image.
> + */
> +
> +int Lux::readYaml(const YamlObject &tuningData)
> +{
> +       auto value = tuningData["reference_exposure_time"].get<double>();
> +       if (!value) {
> +               LOG(Lux, Error) << "Missing tuning parameter: 'reference_exposure_time'";
> +               return -EINVAL;
> +       }
> +       referenceExposureTime_ = *value * 1.0us;
> +
> +       value = tuningData["reference_gain"].get<double>();
> +       if (!value) {
> +               LOG(Lux, Error) << "Missing tuning parameter: 'reference_gain'";
> +               return -EINVAL;
> +       }
> +       referenceGain_ = *value;
> +
> +       referenceAperture_ = tuningData["reference_aperture"].get<double>(1.0);
> +
> +       value = tuningData["reference_Y"].get<double>();
> +       if (!value) {
> +               LOG(Lux, Error) << "Missing tuning parameter: 'reference_Y'";
> +               return -EINVAL;
> +       }
> +       referenceY_ = *value;
> +
> +       value = tuningData["reference_lux"].get<double>();
> +       if (!value) {
> +               LOG(Lux, Error) << "Missing tuning parameter: 'reference_lux'";
> +               return -EINVAL;
> +       }
> +       referenceLux_ = *value;
> +
> +       return 0;
> +}
> +
> +double Lux::process(double gain, utils::Duration exposureTime, double aperture,
> +                   const Histogram &yHist) const

As this isn't an algorithm itself, it's a helper I wonder if this
function should be named to match what it /does/. I.e.
  double Lux::estimate() ?

> +{
> +       double currentY = yHist.interQuantileMean(0, 1);
> +       double gainRatio = referenceGain_ / gain;
> +       double exposureTimeRatio = referenceExposureTime_ / exposureTime;
> +       double apertureRatio = referenceAperture_ / aperture;
> +       double yRatio = currentY * (65536 / yHist.bins()) / referenceY_;
> +
> +       double estimatedLux = exposureTimeRatio * gainRatio *
> +                             apertureRatio * apertureRatio *
> +                             yRatio * referenceLux_;

Does the tuning process only generate a single reference point? I guess
it's expected to be closely linear here? Or maybe it's close enough as
an estimate anyway.



> +
> +       LOG(Lux, Debug) << "Estimated lux " << estimatedLux;
> +       return estimatedLux;
> +}
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/lux.h b/src/ipa/libipa/lux.h
> new file mode 100644
> index 00000000..6bc9cf9f
> --- /dev/null
> +++ b/src/ipa/libipa/lux.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2019, Raspberry Pi Ltd
> + * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> + *
> + * lux.h - Helper class that implements lux estimation
> + */
> +
> +#pragma once
> +
> +#include <algorithm>
> +#include <tuple>
> +#include <vector>
> +
> +#include <libcamera/base/utils.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "histogram.h"
> +
> +namespace libcamera {
> +
> +namespace ipa {
> +
> +class Lux
> +{
> +public:
> +       Lux() = default;
> +       ~Lux() = default;
> +
> +       int readYaml(const YamlObject &tuningData);
> +       double process(double gain, utils::Duration exposureTime,
> +                      double aperture, const Histogram &yHist) const;
> +
> +private:
> +       utils::Duration referenceExposureTime_;
> +       double referenceGain_;
> +       double referenceAperture_;
> +       double referenceY_;
> +       double referenceLux_;
> +};
> +
> +} /* namespace ipa */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 0796982e..b6d58900 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -8,6 +8,7 @@ libipa_headers = files([
>      'exposure_mode_helper.h',
>      'fc_queue.h',
>      'histogram.h',
> +    'lux.h',
>      'matrix.h',
>      'module.h',
>      'pwl.h',
> @@ -21,6 +22,7 @@ libipa_sources = files([
>      'exposure_mode_helper.cpp',
>      'fc_queue.cpp',
>      'histogram.cpp',
> +    'lux.cpp',
>      'matrix.cpp',
>      'module.cpp',
>      'pwl.cpp'
> -- 
> 2.39.2
>