[PATCH v2 07/18] libcamera: software_isp: Add SwStats base class

Hans de Goede hdegoede at redhat.com
Wed Feb 7 16:21:52 CET 2024


Hi,

On 1/23/24 16:37, Milan Zamazal wrote:
> Hans de Goede <hdegoede at redhat.com> writes:
> 
>> Add a virtual base class for CPU based software statistics gathering
>> implementations.
>>
>> The idea is for the implementations to offer a configure function +
>> functions to gather statistics on a line by line basis. This allows
>> CPU based software debayering to call into interlace debayering and
>> statistics gathering on a line by line bases while the input data
>> is still hot in the cache.
>>
>> This base class also allows the user of an implementation to specify
>> a window over which to gather statistics instead of processing the
>> whole frame; and it allows the implementation to choose to only
>> process 1/2, 1/4th, etc. of the lines instead of processing all
>> lines (in the window) by setting y_skip_mask_ from configure().
>> Skipping columns is left up the line-processing functions provided
>> by the implementation.
>>
>> Doxygen documentation by Dennis Bonke.
>>
>> Co-authored-by: Dennis Bonke <admin at dennisbonke.com>
>> Signed-off-by: Dennis Bonke <admin at dennisbonke.com>
>> Co-authored-by: Andrey Konovalov <andrey.konovalov at linaro.org>
>> Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> Tested-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org> # sc8280xp Lenovo x13s
>> Tested-by: Pavel Machek <pavel at ucw.cz>
>> ---
>>  include/libcamera/internal/meson.build        |   1 +
>>  .../internal/software_isp/meson.build         |   6 +
>>  .../internal/software_isp/swisp_stats.h       |  34 +++
>>  .../libcamera/internal/software_isp/swstats.h | 215 ++++++++++++++++++
>>  src/libcamera/meson.build                     |   1 +
>>  src/libcamera/software_isp/meson.build        |   5 +
>>  src/libcamera/software_isp/swstats.cpp        |  22 ++
>>  7 files changed, 284 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 include/libcamera/internal/software_isp/swstats.h
>>  create mode 100644 src/libcamera/software_isp/meson.build
>>  create mode 100644 src/libcamera/software_isp/swstats.cpp
>>
>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
>> index 1325941d..caa533c4 100644
>> --- a/include/libcamera/internal/meson.build
>> +++ b/include/libcamera/internal/meson.build
>> @@ -51,3 +51,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..1c43acc4
>> --- /dev/null
>> +++ b/include/libcamera/internal/software_isp/meson.build
>> @@ -0,0 +1,6 @@
>> +# SPDX-License-Identifier: CC0-1.0
>> +
>> +libcamera_internal_headers += files([
>> +    'swisp_stats.h',
>> +    'swstats.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..07ba7d6a
>> --- /dev/null
>> +++ b/include/libcamera/internal/software_isp/swisp_stats.h
>> @@ -0,0 +1,34 @@
>> +/* 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
>> +
>> +namespace libcamera {
>> +
>> +/**
>> + * \brief Struct that holds the statistics for the Software ISP.
>> + */
>> +struct SwIspStats {
>> +	/**
>> +	 * \brief Holds the sum of all sampled red pixels.
>> +	 */
>> +	unsigned long sumR_;
>> +	/**
>> +	 * \brief Holds the sum of all sampled green pixels.
>> +	 */
>> +	unsigned long sumG_;
>> +	/**
>> +	 * \brief Holds the sum of all sampled blue pixels.
>> +	 */
>> +	unsigned long sumB_;
>> +	/**
>> +	 * \brief A histogram of luminance values.
>> +	 */
>> +	unsigned int y_histogram[16];
>> +};
>> +
>> +} /* namespace libcamera */
>> diff --git a/include/libcamera/internal/software_isp/swstats.h b/include/libcamera/internal/software_isp/swstats.h
>> new file mode 100644
>> index 00000000..dcac7064
>> --- /dev/null
>> +++ b/include/libcamera/internal/software_isp/swstats.h
>> @@ -0,0 +1,215 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2023, Linaro Ltd
>> + * Copyright (C) 2023, Red Hat Inc.
>> + *
>> + * Authors:
>> + * Hans de Goede <hdegoede at redhat.com> 
>> + *
>> + * swstats.h - software statistics base class
>> + */
>> +
>> +#pragma once
>> +
>> +#include <stdint.h>
>> +
>> +#include <libcamera/base/log.h>
>> +#include <libcamera/base/signal.h>
>> +
>> +#include <libcamera/geometry.h>
>> +
>> +namespace libcamera {
>> +
>> +class PixelFormat;
>> +struct SharedFD;
>> +struct StreamConfiguration;
>> +
>> +LOG_DECLARE_CATEGORY(SwStats)
>> +
>> +/**
>> + * \class SwStats
>> + * \brief Base class for the software ISP statistics.
>> + *
>> + * Base class for the software ISP statistics.
>> + */
>> +class SwStats
>> +{
>> +public:
>> +	virtual ~SwStats() = 0;
>> +
>> +	/**
>> +	 * \brief Gets wether the statistics object is valid.
>> +	 * 
>> +	 * \return true if it's valid, false otherwise.
>> +	 */
>> +	virtual bool isValid() const = 0;
>> +
>> +	/**
>> +	 * \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.
>> +	 */
>> +	virtual int configure(const StreamConfiguration &inputCfg) = 0;
>> +
>> +	/**
>> +	 * \brief Get the file descriptor for the statistics.
>> +	 *
>> +	 * \return the file descriptor
>> +	 */
>> +	virtual const SharedFD &getStatsFD() = 0;
>> +
>> +protected:
>> +	/**
>> +	 * \brief Called when there is data to get statistics from.
>> +	 * \param[in] src The input data
>> +	 */
>> +	typedef void (SwStats::*statsProcessFn)(const uint8_t *src[]);
>> +	/**
>> +	 * \brief Called when the statistics gathering is done or when a new frame starts.
>> +	 */
>> +	typedef void (SwStats::*statsVoidFn)();
>> +
>> +	/* Variables set by configure(), used every line */
>> +	/**
>> +	 * \brief The function called when a line is ready for statistics processing.
>> +	 *
>> +	 * Used for line 0 and 1, repeating if there isn't a 3rd and a 4th line in the bayer order.
>> +	 */
>> +	statsProcessFn stats0_;
>> +	/**
>> +	 * \brief The function called when a line is ready for statistics processing.
>> +	 *
>> +	 * Used for line 3 and 4, only needed if the bayer order has 4 different lines.
>> +	 */
>> +	statsProcessFn stats2_;
>> +
>> +	/**
>> +	 * \brief The memory used per pixel in bits.
>> +	 */
>> +	unsigned int bpp_;
>> +	/**
>> +	 * \brief Skip lines where this bitmask is set in y.
>> +	 */
>> +	unsigned int y_skip_mask_;
>> +
>> +	/**
>> +	 * \brief Statistics window, set by setWindow(), used ever line.
>> +	 */
>> +	Rectangle window_;
>> +
>> +	/**
>> +	 * \brief The function called at the start of a frame.
>> +	 */
>> +	statsVoidFn startFrame_;
>> +	/**
>> +	 * \brief The function called at the end of a frame.
>> +	 */
>> +	statsVoidFn finishFrame_;
>> +	/**
>> +	 * \brief The size of the bayer pattern.
>> +	 */
>> +	Size patternSize_;
>> +	/**
>> +	 * \brief The offset of x, applied to window_.x for bayer variants.
>> +	 *
>> +	 * This can either be 0 or 1.
>> +	 */
>> +	unsigned int x_shift_;
>> +
>> +public:
>> +	/**
>> +	 * \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.
>> +	 */
>> +	const Size &patternSize() { return patternSize_; }
>> +
>> +	/**
>> +	 * \brief Specify window coordinates over which to gather statistics.
>> +	 * \param[in] window The window object.
>> +	 */
>> +	void setWindow(Rectangle window)
>> +	{
>> +		window_ = window;
>> +
>> +		window_.x &= ~(patternSize_.width - 1);
>> +		window_.x += x_shift_;
>> +		window_.y &= ~(patternSize_.height - 1);
>> +
>> +		/* width_ - x_shift_ to make sure the window fits */
>> +		window_.width -= x_shift_;
>> +		window_.width &= ~(patternSize_.width - 1);
>> +		window_.height &= ~(patternSize_.height - 1);
> 
> These operations work correctly only if width & height are powers of 2, right?
> Maybe this is satisfied for all Bayer patterns but I think such implicit
> assumptions should be either not introduced or sufficiently visibly documented.

Ack, I have added a comment that patternSize_.width and height always
must be either 2 or 4.

Regards,

Hans






More information about the libcamera-devel mailing list