[2/3] ipa: rkisp1: Add Lux algorithm module
diff mbox series

Message ID 20240412091700.1817754-3-paul.elder@ideasonboard.com
State New
Headers show
Series
  • ipa: rkisp1: Add lux estimation
Related show

Commit Message

Paul Elder April 12, 2024, 9:16 a.m. UTC
Add a lux algorithm module to rkisp1 IPA for estimating the lux level of
an image. This is reported in metadata, as well as saved in the frame
context so that other algorithms (mainly AGC) can use its value. It does
not set any controls.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/lux.cpp     | 76 +++++++++++++++++++++++++++
 src/ipa/rkisp1/algorithms/lux.h       | 39 ++++++++++++++
 src/ipa/rkisp1/algorithms/meson.build |  1 +
 src/ipa/rkisp1/ipa_context.h          |  1 +
 4 files changed, 117 insertions(+)
 create mode 100644 src/ipa/rkisp1/algorithms/lux.cpp
 create mode 100644 src/ipa/rkisp1/algorithms/lux.h

Comments

Kieran Bingham April 12, 2024, 10:57 a.m. UTC | #1
Quoting Paul Elder (2024-04-12 10:16:59)
> Add a lux algorithm module to rkisp1 IPA for estimating the lux level of
> an image. This is reported in metadata, as well as saved in the frame
> context so that other algorithms (mainly AGC) can use its value. It does
> not set any controls.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/lux.cpp     | 76 +++++++++++++++++++++++++++
>  src/ipa/rkisp1/algorithms/lux.h       | 39 ++++++++++++++
>  src/ipa/rkisp1/algorithms/meson.build |  1 +
>  src/ipa/rkisp1/ipa_context.h          |  1 +
>  4 files changed, 117 insertions(+)
>  create mode 100644 src/ipa/rkisp1/algorithms/lux.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/lux.h
> 
> diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp
> new file mode 100644
> index 00000000..1816ddb2
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/lux.cpp
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * lux.cpp - RkISP1 Lux control
> + */
> +
> +#include "lux.h"
> +
> +#include <algorithm>
> +#include <cmath>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/control_ids.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "libipa/histogram.h"
> +#include "libipa/lux.h"
> +
> +/**
> + * \file lux.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class Lux
> + * \brief RkISP1 Lux control
> + *
> + * The Lux algorithm is responsible for estimating the lux level of the image.
> + * It doesn't take or generate any controls, but it provides a lux level for
> + * other algorithms (such as AGC) to use.

I'm not yet convinced this is an 'algorithm', but could be put directly
in agc. It might be interesting to explore why RPi choose to keep this
separate though. Does the lux value get used by any algorithm /other/
than AGC?



> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Lux)
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Lux::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
> +{
> +       return lux_.readYaml(tuningData);
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::process
> + */
> +void Lux::process(IPAContext &context,
> +                 [[maybe_unused]] const uint32_t frame,
> +                 IPAFrameContext &frameContext,
> +                 const rkisp1_stat_buffer *stats,
> +                 ControlList &metadata)
> +{
> +       utils::Duration exposureTime = context.configuration.sensor.lineDuration
> +                                      * frameContext.sensor.exposure;
> +       double gain = frameContext.sensor.gain;
> +
> +       const rkisp1_cif_isp_stat *params = &stats->params;
> +       Histogram yHist = Histogram(Span<const uint32_t>(params->hist.hist_bins,
> +                                                        context.hw->numHistogramBins), 4);

Are we calculating/processing the histogram multiple times now by
keeping this out of the AGC module? Would that be optimised by havign
this as a part of agc directly ?

> +
> +       /* todo Update this when we support aperture */
> +       double lux = lux_.process(gain, exposureTime, 1.0, yHist);
> +       frameContext.agc.lux = lux;
> +       metadata.set(controls::Lux, lux);
> +}
> +
> +REGISTER_IPA_ALGORITHM(Lux, "Lux")
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/lux.h b/src/ipa/rkisp1/algorithms/lux.h
> new file mode 100644
> index 00000000..ea98c291
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/lux.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * lux.h - RkISP1 Lux control
> + */
> +
> +#pragma once
> +
> +#include <sys/types.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "libipa/lux.h"
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class Lux : public Algorithm
> +{
> +public:
> +       Lux() = default;
> +       ~Lux() = default;
> +
> +       int init(IPAContext &context, const YamlObject &tuningData) override;
> +       void process(IPAContext &context, const uint32_t frame,
> +                    IPAFrameContext &frameContext,
> +                    const rkisp1_stat_buffer *stats,
> +                    ControlList &metadata) override;
> +
> +private:
> +       ipa::Lux lux_;
> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index c9891e87..b0381d5f 100644
> --- a/src/ipa/rkisp1/algorithms/meson.build
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -11,4 +11,5 @@ rkisp1_ipa_algorithms = files([
>      'filter.cpp',
>      'gsl.cpp',
>      'lsc.cpp',
> +    'lux.cpp',
>  ])
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index dc876da0..c39e0e9b 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -118,6 +118,7 @@ struct IPAFrameContext : public FrameContext {
>                 int32_t exposureMode;
>                 int32_t constraintMode;
>                 utils::Duration maxShutterSpeed;
> +               double lux;

Especially as we're putting the lux in the agc component here!

--
Kieran


>         } agc;
>  
>         struct {
> -- 
> 2.39.2
>
Dan Scally May 21, 2024, 10:17 a.m. UTC | #2
Hi Paul

On 12/04/2024 10:16, Paul Elder wrote:
> Add a lux algorithm module to rkisp1 IPA for estimating the lux level of
> an image. This is reported in metadata, as well as saved in the frame
> context so that other algorithms (mainly AGC) can use its value. It does
> not set any controls.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>   src/ipa/rkisp1/algorithms/lux.cpp     | 76 +++++++++++++++++++++++++++
>   src/ipa/rkisp1/algorithms/lux.h       | 39 ++++++++++++++


Like Kieran, I also lean towards "this should just be a part of the Agc implementation" - it should 
only be a few lines added to ::process(). We could pass in the return value from estimateLuminance() 
rather than constructing a yHist too perhaps.

>   src/ipa/rkisp1/algorithms/meson.build |  1 +
>   src/ipa/rkisp1/ipa_context.h          |  1 +
>   4 files changed, 117 insertions(+)
>   create mode 100644 src/ipa/rkisp1/algorithms/lux.cpp
>   create mode 100644 src/ipa/rkisp1/algorithms/lux.h
>
> diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp
> new file mode 100644
> index 00000000..1816ddb2
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/lux.cpp
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * lux.cpp - RkISP1 Lux control
> + */
> +
> +#include "lux.h"
> +
> +#include <algorithm>
> +#include <cmath>
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/control_ids.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "libipa/histogram.h"
> +#include "libipa/lux.h"
> +
> +/**
> + * \file lux.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class Lux
> + * \brief RkISP1 Lux control
> + *
> + * The Lux algorithm is responsible for estimating the lux level of the image.
> + * It doesn't take or generate any controls, but it provides a lux level for
> + * other algorithms (such as AGC) to use.
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Lux)
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Lux::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
> +{
> +	return lux_.readYaml(tuningData);
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::process
> + */
> +void Lux::process(IPAContext &context,
> +		  [[maybe_unused]] const uint32_t frame,
> +		  IPAFrameContext &frameContext,
> +		  const rkisp1_stat_buffer *stats,
> +		  ControlList &metadata)
> +{
> +	utils::Duration exposureTime = context.configuration.sensor.lineDuration
> +				       * frameContext.sensor.exposure;
> +	double gain = frameContext.sensor.gain;
> +
> +	const rkisp1_cif_isp_stat *params = &stats->params;
> +	Histogram yHist = Histogram(Span<const uint32_t>(params->hist.hist_bins,
> +							 context.hw->numHistogramBins), 4);
> +
> +	/* todo Update this when we support aperture */
> +	double lux = lux_.process(gain, exposureTime, 1.0, yHist);
> +	frameContext.agc.lux = lux;
> +	metadata.set(controls::Lux, lux);
> +}
> +
> +REGISTER_IPA_ALGORITHM(Lux, "Lux")
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/lux.h b/src/ipa/rkisp1/algorithms/lux.h
> new file mode 100644
> index 00000000..ea98c291
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/lux.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * lux.h - RkISP1 Lux control
> + */
> +
> +#pragma once
> +
> +#include <sys/types.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "libipa/lux.h"
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class Lux : public Algorithm
> +{
> +public:
> +	Lux() = default;
> +	~Lux() = default;
> +
> +	int init(IPAContext &context, const YamlObject &tuningData) override;
> +	void process(IPAContext &context, const uint32_t frame,
> +		     IPAFrameContext &frameContext,
> +		     const rkisp1_stat_buffer *stats,
> +		     ControlList &metadata) override;
> +
> +private:
> +	ipa::Lux lux_;
> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index c9891e87..b0381d5f 100644
> --- a/src/ipa/rkisp1/algorithms/meson.build
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -11,4 +11,5 @@ rkisp1_ipa_algorithms = files([
>       'filter.cpp',
>       'gsl.cpp',
>       'lsc.cpp',
> +    'lux.cpp',
>   ])
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index dc876da0..c39e0e9b 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -118,6 +118,7 @@ struct IPAFrameContext : public FrameContext {
>   		int32_t exposureMode;
>   		int32_t constraintMode;
>   		utils::Duration maxShutterSpeed;
> +		double lux;
>   	} agc;
>   
>   	struct {

Patch
diff mbox series

diff --git a/src/ipa/rkisp1/algorithms/lux.cpp b/src/ipa/rkisp1/algorithms/lux.cpp
new file mode 100644
index 00000000..1816ddb2
--- /dev/null
+++ b/src/ipa/rkisp1/algorithms/lux.cpp
@@ -0,0 +1,76 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2024, Ideas On Board
+ *
+ * lux.cpp - RkISP1 Lux control
+ */
+
+#include "lux.h"
+
+#include <algorithm>
+#include <cmath>
+
+#include <libcamera/base/log.h>
+
+#include <libcamera/control_ids.h>
+
+#include "libcamera/internal/yaml_parser.h"
+
+#include "libipa/histogram.h"
+#include "libipa/lux.h"
+
+/**
+ * \file lux.h
+ */
+
+namespace libcamera {
+
+namespace ipa::rkisp1::algorithms {
+
+/**
+ * \class Lux
+ * \brief RkISP1 Lux control
+ *
+ * The Lux algorithm is responsible for estimating the lux level of the image.
+ * It doesn't take or generate any controls, but it provides a lux level for
+ * other algorithms (such as AGC) to use.
+ */
+
+LOG_DEFINE_CATEGORY(RkISP1Lux)
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::init
+ */
+int Lux::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
+{
+	return lux_.readYaml(tuningData);
+}
+
+/**
+ * \copydoc libcamera::ipa::Algorithm::process
+ */
+void Lux::process(IPAContext &context,
+		  [[maybe_unused]] const uint32_t frame,
+		  IPAFrameContext &frameContext,
+		  const rkisp1_stat_buffer *stats,
+		  ControlList &metadata)
+{
+	utils::Duration exposureTime = context.configuration.sensor.lineDuration
+				       * frameContext.sensor.exposure;
+	double gain = frameContext.sensor.gain;
+
+	const rkisp1_cif_isp_stat *params = &stats->params;
+	Histogram yHist = Histogram(Span<const uint32_t>(params->hist.hist_bins,
+							 context.hw->numHistogramBins), 4);
+
+	/* todo Update this when we support aperture */
+	double lux = lux_.process(gain, exposureTime, 1.0, yHist);
+	frameContext.agc.lux = lux;
+	metadata.set(controls::Lux, lux);
+}
+
+REGISTER_IPA_ALGORITHM(Lux, "Lux")
+
+} /* namespace ipa::rkisp1::algorithms */
+
+} /* namespace libcamera */
diff --git a/src/ipa/rkisp1/algorithms/lux.h b/src/ipa/rkisp1/algorithms/lux.h
new file mode 100644
index 00000000..ea98c291
--- /dev/null
+++ b/src/ipa/rkisp1/algorithms/lux.h
@@ -0,0 +1,39 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2024, Ideas On Board
+ *
+ * lux.h - RkISP1 Lux control
+ */
+
+#pragma once
+
+#include <sys/types.h>
+
+#include "libcamera/internal/yaml_parser.h"
+
+#include "libipa/lux.h"
+
+#include "algorithm.h"
+
+namespace libcamera {
+
+namespace ipa::rkisp1::algorithms {
+
+class Lux : public Algorithm
+{
+public:
+	Lux() = default;
+	~Lux() = default;
+
+	int init(IPAContext &context, const YamlObject &tuningData) override;
+	void process(IPAContext &context, const uint32_t frame,
+		     IPAFrameContext &frameContext,
+		     const rkisp1_stat_buffer *stats,
+		     ControlList &metadata) override;
+
+private:
+	ipa::Lux lux_;
+};
+
+} /* namespace ipa::rkisp1::algorithms */
+} /* namespace libcamera */
diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
index c9891e87..b0381d5f 100644
--- a/src/ipa/rkisp1/algorithms/meson.build
+++ b/src/ipa/rkisp1/algorithms/meson.build
@@ -11,4 +11,5 @@  rkisp1_ipa_algorithms = files([
     'filter.cpp',
     'gsl.cpp',
     'lsc.cpp',
+    'lux.cpp',
 ])
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index dc876da0..c39e0e9b 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -118,6 +118,7 @@  struct IPAFrameContext : public FrameContext {
 		int32_t exposureMode;
 		int32_t constraintMode;
 		utils::Duration maxShutterSpeed;
+		double lux;
 	} agc;
 
 	struct {