[v8,07/18] libcamera: software_isp: Add SwStatsCpu class
diff mbox series

Message ID 20240416091357.211951-8-mzamazal@redhat.com
State Accepted
Headers show
Series
  • libcamera: introduce Software ISP and Software IPA
Related show

Commit Message

Milan Zamazal April 16, 2024, 9:13 a.m. UTC
From: Hans de Goede <hdegoede@redhat.com>

Add a CPU based SwStats implementation for SoftwareISP / SoftIPA use.

This implementation offers a configure function + functions to gather
statistics on a line by line basis. This allows CPU based software
debayering to call into interleave debayering and statistics gathering
on a line by line basis while the input data is still hot in the cache.

This implementation also allows specifying a window over which to gather
statistics instead of processing the whole frame.

Doxygen documentation by Dennis Bonke.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s
Tested-by: Pavel Machek <pavel@ucw.cz>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Co-developed-by: Andrey Konovalov <andrey.konovalov@linaro.org>
Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
Co-developed-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Co-developed-by: Dennis Bonke <admin@dennisbonke.com>
Signed-off-by: Dennis Bonke <admin@dennisbonke.com>
Co-developed-by: Marttico <g.martti@gmail.com>
Signed-off-by: Marttico <g.martti@gmail.com>
Co-developed-by: Toon Langendam <t.langendam@gmail.com>
Signed-off-by: Toon Langendam <t.langendam@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/libcamera/internal/meson.build        |   1 +
 .../internal/software_isp/meson.build         |   5 +
 .../internal/software_isp/swisp_stats.h       |  45 +++
 src/libcamera/meson.build                     |   1 +
 src/libcamera/software_isp/TODO               |  71 ++++
 src/libcamera/software_isp/meson.build        |  12 +
 src/libcamera/software_isp/swstats_cpu.cpp    | 304 ++++++++++++++++++
 src/libcamera/software_isp/swstats_cpu.h      |  88 +++++
 8 files changed, 527 insertions(+)
 create mode 100644 include/libcamera/internal/software_isp/meson.build
 create mode 100644 include/libcamera/internal/software_isp/swisp_stats.h
 create mode 100644 src/libcamera/software_isp/TODO
 create mode 100644 src/libcamera/software_isp/meson.build
 create mode 100644 src/libcamera/software_isp/swstats_cpu.cpp
 create mode 100644 src/libcamera/software_isp/swstats_cpu.h

Patch
diff mbox series

diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
index 5807dfd9..160fdc37 100644
--- a/include/libcamera/internal/meson.build
+++ b/include/libcamera/internal/meson.build
@@ -50,3 +50,4 @@  libcamera_internal_headers = files([
 ])
 
 subdir('converter')
+subdir('software_isp')
diff --git a/include/libcamera/internal/software_isp/meson.build b/include/libcamera/internal/software_isp/meson.build
new file mode 100644
index 00000000..66c9c3fb
--- /dev/null
+++ b/include/libcamera/internal/software_isp/meson.build
@@ -0,0 +1,5 @@ 
+# SPDX-License-Identifier: CC0-1.0
+
+libcamera_internal_headers += files([
+    'swisp_stats.h',
+])
diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h
new file mode 100644
index 00000000..dd5e207d
--- /dev/null
+++ b/include/libcamera/internal/software_isp/swisp_stats.h
@@ -0,0 +1,45 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2023, Linaro Ltd
+ *
+ * swisp_stats.h - Statistics data format used by the software ISP and software IPA
+ */
+
+#pragma once
+
+#include <array>
+#include <stdint.h>
+
+namespace libcamera {
+
+/**
+ * \brief Struct that holds the statistics for the Software ISP
+ *
+ * The struct value types are large enough to not overflow.
+ * Should they still overflow for some reason, no check is performed and they
+ * wrap around.
+ */
+struct SwIspStats {
+	/**
+	 * \brief Holds the sum of all sampled red pixels
+	 */
+	uint64_t sumR_;
+	/**
+	 * \brief Holds the sum of all sampled green pixels
+	 */
+	uint64_t sumG_;
+	/**
+	 * \brief Holds the sum of all sampled blue pixels
+	 */
+	uint64_t sumB_;
+	/**
+	 * \brief Number of bins in the yHistogram
+	 */
+	static constexpr unsigned int kYHistogramSize = 16;
+	/**
+	 * \brief A histogram of luminance values
+	 */
+	std::array<uint32_t, kYHistogramSize> yHistogram;
+};
+
+} /* namespace libcamera */
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
index ce31180b..a3b12bc1 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -70,6 +70,7 @@  subdir('ipa')
 subdir('pipeline')
 subdir('proxy')
 subdir('sensor')
+subdir('software_isp')
 
 null_dep = dependency('', required : false)
 
diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO
new file mode 100644
index 00000000..1c28fc36
--- /dev/null
+++ b/src/libcamera/software_isp/TODO
@@ -0,0 +1,71 @@ 
+1. Setting F_SEAL_SHRINK and F_SEAL_GROW after ftruncate()
+
+>> SharedMem::SharedMem(const std::string &name, std::size_t size)
+>> 	: name_(name), size_(size), mem_(nullptr)
+>>
+>> ...
+>>
+>> 	if (ftruncate(fd_.get(), size_) < 0)
+>> 		return;
+>
+> Should we set the GROW and SHRINK seals (in a separate patch) ?
+
+Yes, this can be done.
+Setting F_SEAL_SHRINK and F_SEAL_GROW after the ftruncate() call above could catch
+some potential errors related to improper access to the shared memory allocated by
+the SharedMemObject.
+
+---
+
+2. Reconsider stats sharing
+
+>>> +void SwStatsCpu::finishFrame(void)
+>>> +{
+>>> +	*sharedStats_ = stats_;
+>> 
+>> Is it more efficient to copy the stats instead of operating directly on
+>> the shared memory ?
+>
+> I inherited doing things this way from Andrey. I kept this because
+> we don't really have any synchronization with the IPA reading this.
+>
+> So the idea is to only touch this when the next set of statistics
+> is ready since we don't know when the IPA is done with accessing
+> the previous set of statistics ...
+>
+> This is both something which seems mostly a theoretic problem,
+> yet also definitely something which I think we need to fix.
+>
+> Maybe use a ringbuffer of stats buffers and pass the index into
+> the ringbuffer to the emit signal ?
+
+That would match how we deal with hardware ISPs, and I think that's a
+good idea. It will help decoupling the processing side from the IPA.
+
+---
+
+3. Remove statsReady signal
+
+> class SwStatsCpu
+> {
+> 	/**
+> 	 * \brief Signals that the statistics are ready
+> 	 */
+> 	Signal<> statsReady;
+
+But better, I wonder if the signal could be dropped completely. The
+SwStatsCpu class does not operate asynchronously. Shouldn't whoever
+calls the finishFrame() function then handle emitting the signal ?
+
+Now, the trouble is that this would be the DebayerCpu class, whose name
+doesn't indicate as a prime candidate to handle stats. However, it
+already exposes a getStatsFD() function, so we're already calling for
+trouble :-) Either that should be moved to somewhere else, or the class
+should be renamed. Considering that the class applies colour gains in
+addition to performing the interpolation, it may be more of a naming
+issue.
+
+Removing the signal and refactoring those classes doesn't have to be
+addressed now, I think it would be part of a larger refactoring
+(possibly also considering platforms that have no ISP but can produce
+stats in hardware, such as the i.MX7), but please keep it on your radar.
diff --git a/src/libcamera/software_isp/meson.build b/src/libcamera/software_isp/meson.build
new file mode 100644
index 00000000..281bbf0e
--- /dev/null
+++ b/src/libcamera/software_isp/meson.build
@@ -0,0 +1,12 @@ 
+# SPDX-License-Identifier: CC0-1.0
+
+softisp_enabled = pipelines.contains('simple')
+summary({'SoftISP support' : softisp_enabled}, section : 'Configuration')
+
+if not softisp_enabled
+    subdir_done()
+endif
+
+libcamera_sources += files([
+    'swstats_cpu.cpp',
+])
diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
new file mode 100644
index 00000000..24ae0b16
--- /dev/null
+++ b/src/libcamera/software_isp/swstats_cpu.cpp
@@ -0,0 +1,304 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2023, Linaro Ltd
+ * Copyright (C) 2023, Red Hat Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ *
+ * swstats_cpu.cpp - CPU based software statistics implementation
+ */
+
+#include "swstats_cpu.h"
+
+#include <libcamera/base/log.h>
+
+#include <libcamera/stream.h>
+
+#include "libcamera/internal/bayer_format.h"
+
+namespace libcamera {
+
+/**
+ * \class SwStatsCpu
+ * \brief Class for gathering statistics on the CPU
+ *
+ * CPU based software ISP statistics implementation.
+ *
+ * This class offers a configure function + functions to gather statistics on a
+ * line by line basis. This allows CPU based software debayering to interleave
+ * debayering and statistics gathering on a line by line basis while the input
+ * data is still hot in the cache.
+ *
+ * It is also possible to specify a window over which to gather statistics
+ * instead of processing the whole frame.
+ */
+
+/**
+ * \fn bool SwStatsCpu::isValid() const
+ * \brief Gets whether the statistics object is valid
+ *
+ * \return True if it's valid, false otherwise
+ */
+
+/**
+ * \fn const SharedFD &SwStatsCpu::getStatsFD()
+ * \brief Get the file descriptor for the statistics
+ *
+ * \return The file descriptor
+ */
+
+/**
+ * \fn const Size &SwStatsCpu::patternSize()
+ * \brief Get the pattern size
+ *
+ * For some input-formats, e.g. Bayer data, processing is done multiple lines
+ * and/or columns at a time. Get width and height at which the (bayer) pattern
+ * repeats. Window values are rounded down to a multiple of this and the height
+ * also indicates if processLine2() should be called or not.
+ * This may only be called after a successful configure() call.
+ *
+ * \return The pattern size
+ */
+
+/**
+ * \fn void SwStatsCpu::processLine0(unsigned int y, const uint8_t *src[])
+ * \brief Process line 0
+ * \param[in] y The y coordinate.
+ * \param[in] src The input data.
+ *
+ * This function processes line 0 for input formats with
+ * patternSize height == 1.
+ * It'll process line 0 and 1 for input formats with patternSize height >= 2.
+ * This function may only be called after a successful setWindow() call.
+ */
+
+/**
+ * \fn void SwStatsCpu::processLine2(unsigned int y, const uint8_t *src[])
+ * \brief Process line 2 and 3
+ * \param[in] y The y coordinate.
+ * \param[in] src The input data.
+ *
+ * This function processes line 2 and 3 for input formats with
+ * patternSize height == 4.
+ * This function may only be called after a successful setWindow() call.
+ */
+
+/**
+ * \var Signal<> SwStatsCpu::statsReady
+ * \brief Signals that the statistics are ready
+ */
+
+/**
+ * \typedef SwStatsCpu::statsProcessFn
+ * \brief Called when there is data to get statistics from
+ * \param[in] src The input data
+ *
+ * These functions take an array of (patternSize_.height + 1) src
+ * pointers each pointing to a line in the source image. The middle
+ * element of the array will point to the actual line being processed.
+ * Earlier element(s) will point to the previous line(s) and later
+ * element(s) to the next line(s).
+ *
+ * See the documentation of DebayerCpu::debayerFn for more details.
+ */
+
+/**
+ * \var unsigned int SwStatsCpu::ySkipMask_
+ * \brief Skip lines where this bitmask is set in y
+ */
+
+/**
+ * \var Rectangle SwStatsCpu::window_
+ * \brief Statistics window, set by setWindow(), used every line
+ */
+
+/**
+ * \var Size SwStatsCpu::patternSize_
+ * \brief The size of the bayer pattern
+ *
+ * Valid sizes are: 2x2, 4x2 or 4x4.
+ */
+
+/**
+ * \var unsigned int SwStatsCpu::xShift_
+ * \brief The offset of x, applied to window_.x for bayer variants
+ *
+ * This can either be 0 or 1.
+ */
+
+LOG_DEFINE_CATEGORY(SwStatsCpu)
+
+SwStatsCpu::SwStatsCpu()
+	: sharedStats_("softIsp_stats")
+{
+	if (!sharedStats_)
+		LOG(SwStatsCpu, Error)
+			<< "Failed to create shared memory for statistics";
+}
+
+static constexpr unsigned int kRedYMul = 77; /* 0.299 * 256 */
+static constexpr unsigned int kGreenYMul = 150; /* 0.587 * 256 */
+static constexpr unsigned int kBlueYMul = 29; /* 0.114 * 256 */
+
+#define SWSTATS_START_LINE_STATS(pixel_t) \
+	pixel_t r, g, g2, b;              \
+	uint64_t yVal;                    \
+                                          \
+	uint64_t sumR = 0;                \
+	uint64_t sumG = 0;                \
+	uint64_t sumB = 0;
+
+#define SWSTATS_ACCUMULATE_LINE_STATS(div) \
+	sumR += r;                         \
+	sumG += g;                         \
+	sumB += b;                         \
+                                           \
+	yVal = r * kRedYMul;               \
+	yVal += g * kGreenYMul;            \
+	yVal += b * kBlueYMul;             \
+	stats_.yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;
+
+#define SWSTATS_FINISH_LINE_STATS() \
+	stats_.sumR_ += sumR;       \
+	stats_.sumG_ += sumG;       \
+	stats_.sumB_ += sumB;
+
+void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])
+{
+	const uint8_t *src0 = src[1] + window_.x * 5 / 4;
+	const uint8_t *src1 = src[2] + window_.x * 5 / 4;
+	const int widthInBytes = window_.width * 5 / 4;
+
+	if (swapLines_)
+		std::swap(src0, src1);
+
+	SWSTATS_START_LINE_STATS(uint8_t)
+
+	/* x += 5 sample every other 2x2 block */
+	for (int x = 0; x < widthInBytes; x += 5) {
+		/* BGGR */
+		b = src0[x];
+		g = src0[x + 1];
+		g2 = src1[x];
+		r = src1[x + 1];
+		g = (g + g2) / 2;
+		/* Data is already 8 bits, divide by 1 */
+		SWSTATS_ACCUMULATE_LINE_STATS(1)
+	}
+
+	SWSTATS_FINISH_LINE_STATS()
+}
+
+void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
+{
+	const uint8_t *src0 = src[1] + window_.x * 5 / 4;
+	const uint8_t *src1 = src[2] + window_.x * 5 / 4;
+	const int widthInBytes = window_.width * 5 / 4;
+
+	if (swapLines_)
+		std::swap(src0, src1);
+
+	SWSTATS_START_LINE_STATS(uint8_t)
+
+	/* x += 5 sample every other 2x2 block */
+	for (int x = 0; x < widthInBytes; x += 5) {
+		/* GBRG */
+		g = src0[x];
+		b = src0[x + 1];
+		r = src1[x];
+		g2 = src1[x + 1];
+		g = (g + g2) / 2;
+		/* Data is already 8 bits, divide by 1 */
+		SWSTATS_ACCUMULATE_LINE_STATS(1)
+	}
+
+	SWSTATS_FINISH_LINE_STATS()
+}
+
+/**
+ * \brief Reset state to start statistics gathering for a new frame
+ *
+ * This may only be called after a successful setWindow() call.
+ */
+void SwStatsCpu::startFrame(void)
+{
+	if (window_.width == 0)
+		LOG(SwStatsCpu, Error) << "Calling startFrame() without setWindow()";
+
+	stats_.sumR_ = 0;
+	stats_.sumB_ = 0;
+	stats_.sumG_ = 0;
+	stats_.yHistogram.fill(0);
+}
+
+/**
+ * \brief Finish statistics calculation for the current frame
+ *
+ * This may only be called after a successful setWindow() call.
+ */
+void SwStatsCpu::finishFrame(void)
+{
+	*sharedStats_ = stats_;
+	statsReady.emit();
+}
+
+/**
+ * \brief Configure the statistics object for the passed in input format
+ * \param[in] inputCfg The input format
+ *
+ * \return 0 on success, a negative errno value on failure
+ */
+int SwStatsCpu::configure(const StreamConfiguration &inputCfg)
+{
+	BayerFormat bayerFormat =
+		BayerFormat::fromPixelFormat(inputCfg.pixelFormat);
+
+	if (bayerFormat.bitDepth == 10 &&
+	    bayerFormat.packing == BayerFormat::Packing::CSI2) {
+		patternSize_.height = 2;
+		patternSize_.width = 4; /* 5 bytes per *4* pixels */
+		/* Skip every 3th and 4th line, sample every other 2x2 block */
+		ySkipMask_ = 0x02;
+		xShift_ = 0;
+
+		switch (bayerFormat.order) {
+		case BayerFormat::BGGR:
+		case BayerFormat::GRBG:
+			stats0_ = &SwStatsCpu::statsBGGR10PLine0;
+			swapLines_ = bayerFormat.order == BayerFormat::GRBG;
+			return 0;
+		case BayerFormat::GBRG:
+		case BayerFormat::RGGB:
+			stats0_ = &SwStatsCpu::statsGBRG10PLine0;
+			swapLines_ = bayerFormat.order == BayerFormat::RGGB;
+			return 0;
+		default:
+			break;
+		}
+	}
+
+	LOG(SwStatsCpu, Info)
+		<< "Unsupported input format " << inputCfg.pixelFormat.toString();
+	return -EINVAL;
+}
+
+/**
+ * \brief Specify window coordinates over which to gather statistics
+ * \param[in] window The window object.
+ */
+void SwStatsCpu::setWindow(const Rectangle &window)
+{
+	window_ = window;
+
+	window_.x &= ~(patternSize_.width - 1);
+	window_.x += xShift_;
+	window_.y &= ~(patternSize_.height - 1);
+
+	/* width_ - xShift_ to make sure the window fits */
+	window_.width -= xShift_;
+	window_.width &= ~(patternSize_.width - 1);
+	window_.height &= ~(patternSize_.height - 1);
+}
+
+} /* namespace libcamera */
diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h
new file mode 100644
index 00000000..27b15756
--- /dev/null
+++ b/src/libcamera/software_isp/swstats_cpu.h
@@ -0,0 +1,88 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2023, Linaro Ltd
+ * Copyright (C) 2023, Red Hat Inc.
+ *
+ * Authors:
+ * Hans de Goede <hdegoede@redhat.com>
+ *
+ * swstats_cpu.h - CPU based software statistics implementation
+ */
+
+#pragma once
+
+#include <stdint.h>
+
+#include <libcamera/base/signal.h>
+
+#include <libcamera/geometry.h>
+
+#include "libcamera/internal/shared_mem_object.h"
+#include "libcamera/internal/software_isp/swisp_stats.h"
+
+namespace libcamera {
+
+class PixelFormat;
+struct StreamConfiguration;
+
+class SwStatsCpu
+{
+public:
+	SwStatsCpu();
+	~SwStatsCpu() = default;
+
+	bool isValid() const { return sharedStats_.fd().isValid(); }
+
+	const SharedFD &getStatsFD() { return sharedStats_.fd(); }
+
+	const Size &patternSize() { return patternSize_; }
+
+	int configure(const StreamConfiguration &inputCfg);
+	void setWindow(const Rectangle &window);
+	void startFrame();
+	void finishFrame();
+
+	void processLine0(unsigned int y, const uint8_t *src[])
+	{
+		if ((y & ySkipMask_) || y < static_cast<unsigned int>(window_.y) ||
+		    y >= (window_.y + window_.height))
+			return;
+
+		(this->*stats0_)(src);
+	}
+
+	void processLine2(unsigned int y, const uint8_t *src[])
+	{
+		if ((y & ySkipMask_) || y < static_cast<unsigned int>(window_.y) ||
+		    y >= (window_.y + window_.height))
+			return;
+
+		(this->*stats2_)(src);
+	}
+
+	Signal<> statsReady;
+
+private:
+	using statsProcessFn = void (SwStatsCpu::*)(const uint8_t *src[]);
+
+	void statsBGGR10PLine0(const uint8_t *src[]);
+	void statsGBRG10PLine0(const uint8_t *src[]);
+
+	/* Variables set by configure(), used every line */
+	statsProcessFn stats0_;
+	statsProcessFn stats2_;
+	bool swapLines_;
+
+	unsigned int ySkipMask_;
+
+	Rectangle window_;
+
+	Size patternSize_;
+
+	unsigned int xShift_;
+
+	SharedMemObject<SwIspStats> sharedStats_;
+	SwIspStats stats_;
+};
+
+} /* namespace libcamera */