diff options
| -rw-r--r-- | include/libcamera/internal/meson.build | 1 | ||||
| -rw-r--r-- | include/libcamera/internal/software_isp/meson.build | 5 | ||||
| -rw-r--r-- | include/libcamera/internal/software_isp/swisp_stats.h | 45 | ||||
| -rw-r--r-- | src/libcamera/meson.build | 1 | ||||
| -rw-r--r-- | src/libcamera/software_isp/TODO | 71 | ||||
| -rw-r--r-- | src/libcamera/software_isp/meson.build | 12 | ||||
| -rw-r--r-- | src/libcamera/software_isp/swstats_cpu.cpp | 304 | ||||
| -rw-r--r-- | src/libcamera/software_isp/swstats_cpu.h | 88 |
8 files changed, 527 insertions, 0 deletions
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 */ |