[libcamera-devel,RFC,v2,0/7] libcamera: introduce Software ISP and Software IPA
mbox series

Message ID 20231212115046.102726-1-andrey.konovalov@linaro.org
Headers show
Series
  • libcamera: introduce Software ISP and Software IPA
Related show

Message

Andrey Konovalov Dec. 12, 2023, 11:50 a.m. UTC
Here is a draft implementation of Software ISP and Software IPA
which provide debayering and implementation of image processing
algorithms for systems without a hardware ISP, or in cases when
there are no public drivers for the hardware ISP present in the
system.

The implementation of the Software ISP is a reference one.
A naive AWB alorithm is implemented as part of a function which
does debayering and statistics calculations - the algorithm part
is to be moved to the IPA in the next version of the patch set.
And for debayering itself there is already a more efficient
implementation by Hans de Goede. This patch set is currently using
the earlier debayering implementation as it is less lines of code
and is OK for the initial discussion.
Only RAW10P format from the sensor is currently supported, but
other debayering functions can be easily added (which of them to
call is decided based on the input format).

The Software IPA has only auto exposure and AGC. For the AGC
the analogue gain control of the camera sensor is used (if
available). The algorithm is very much simplified, and is
mostly included as a reference code.

The 6th patch renames some variables in the simple pipeline
handler for the Software ISP to use the same buffer handling
code as the Converter currently does. This lets one to
avoid adding extra code to the pipeline handler, but also
makes the Software ISP and the Converter mutually exclusive.

The Software ISP / IPA are intended to be used with the simple
pipeline handler. The pipeline handler doesn't interact with
the Software IPA directly - the Software IPA is hidden behind
the Software ISP interface. To use the Software ISP the build
must be configured with
  -Dpipelines=simple/linaro -Dipas=simple/linaro
and the Converter must not be used (the latter is hardcoded
in the simple pipeline handler).
If the build is configured with just
  -Dpipelines=simple
the Software ISP / IPA are not used, and the simple pipeline
handler works in the same way as without this patchset.

"linaro" in the
  -Dpipelines=simple/linaro -Dipas=simple/linaro
is the name of the Software ISP / IPA implementation.
  -Dpipelines=simple/<something else> -Dipas=simple/<something else>
should work too if another implementation were added. What
implementation to use is the build time choice, only one
implementation is included into the build.

This patch set uses SharedMemObject class used by the RPi pipeline
handler - the second patch in the series moves the header file
to a common directory.

This patch set has been tested on Qualcomm RB5 board with
a mezzanine board equipped with RPi camera v2 (not the
standard RB5 camera mezzanine).

git branch for the patch set v2:
https://gitlab.freedesktop.org/camera/libcamera-softisp/-/commits/SoftwareISP-v03

v1 of the patch set:
https://patchwork.libcamera.org/cover/19262/

Changes in v2 vs v1:
- patches are restructured and reordered
- the Software IPA is hidden behind the Software ISP interface. The
  pipeline handler doesn't interact with the Software IPA directly
- instantiation of the Software ISP / IPA is configurable (at build
  time)
- comment explaining the implementation limitations is added to
  SwIspLinaro::IspWorker::debayerRaw10P()

Andrey Konovalov (7):
  libcamera: introduce SoftwareIsp class
  libcamera: internal: Move SharedMemObject class to a common directory
  libcamera: ipa: add Soft IPA common files
  libcamera: ipa: Soft IPA: add a Soft IPA implementation
  libcamera: software_isp: add SwIspLinaro implementation of SoftwareIsp
  libcamera: pipeline: simple: rename converterBuffers_ and related vars
  libcamera: pipeline: simple: enable use of Soft ISP and Soft IPA

 include/libcamera/internal/meson.build        |   3 +
 .../libcamera/internal}/shared_mem_object.h   |   4 -
 include/libcamera/internal/software_isp.h     | 106 ++++
 .../internal/software_isp/meson.build         |   6 +
 .../internal/software_isp/statistics-linaro.h |  17 +
 .../internal/software_isp/swisp_linaro.h      | 117 ++++
 include/libcamera/ipa/meson.build             |   1 +
 include/libcamera/ipa/soft.mojom              |  27 +
 meson_options.txt                             |   3 +-
 src/ipa/simple/common/meson.build             |  17 +
 src/ipa/simple/common/soft_base.cpp           |  66 ++
 src/ipa/simple/common/soft_base.h             |  47 ++
 src/ipa/simple/linaro/data/meson.build        |   9 +
 src/ipa/simple/linaro/data/soft.conf          |   3 +
 src/ipa/simple/linaro/meson.build             |  26 +
 src/ipa/simple/linaro/soft_linaro.cpp         | 210 +++++++
 src/ipa/simple/meson.build                    |  12 +
 src/libcamera/meson.build                     |   2 +
 src/libcamera/pipeline/simple/simple.cpp      | 165 +++--
 src/libcamera/software_isp.cpp                |  62 ++
 src/libcamera/software_isp/meson.build        |  20 +
 src/libcamera/software_isp/swisp_linaro.cpp   | 589 ++++++++++++++++++
 22 files changed, 1455 insertions(+), 57 deletions(-)
 rename {src/libcamera/pipeline/rpi/common => include/libcamera/internal}/shared_mem_object.h (98%)
 create mode 100644 include/libcamera/internal/software_isp.h
 create mode 100644 include/libcamera/internal/software_isp/meson.build
 create mode 100644 include/libcamera/internal/software_isp/statistics-linaro.h
 create mode 100644 include/libcamera/internal/software_isp/swisp_linaro.h
 create mode 100644 include/libcamera/ipa/soft.mojom
 create mode 100644 src/ipa/simple/common/meson.build
 create mode 100644 src/ipa/simple/common/soft_base.cpp
 create mode 100644 src/ipa/simple/common/soft_base.h
 create mode 100644 src/ipa/simple/linaro/data/meson.build
 create mode 100644 src/ipa/simple/linaro/data/soft.conf
 create mode 100644 src/ipa/simple/linaro/meson.build
 create mode 100644 src/ipa/simple/linaro/soft_linaro.cpp
 create mode 100644 src/ipa/simple/meson.build
 create mode 100644 src/libcamera/software_isp.cpp
 create mode 100644 src/libcamera/software_isp/meson.build
 create mode 100644 src/libcamera/software_isp/swisp_linaro.cpp

Comments

Pavel Machek Dec. 12, 2023, 5:43 p.m. UTC | #1
Hi!

> Here is a draft implementation of Software ISP and Software IPA
> which provide debayering and implementation of image processing
> algorithms for systems without a hardware ISP, or in cases when
> there are no public drivers for the hardware ISP present in the
> system.

After tweaks to support 8-bit it works for me and is quite useful on
PinePhone.

IMO something needs fixing in
SimplePipelineHandler::generateConfiguration, otherwise code will
sigsegv when suitable formats are not available.

Tested-by: Pavel Machek <pavel@ucw.cz>

Best regards,
								Pavel

commit a9966b193ebe4c9624e00a962ba17786a1aeaad5
Author: Pavel Machek <pavel@ucw.cz>
Date:   Tue Dec 5 13:01:54 2023 +0100

    b8: Got 8-bit bayer to work.

diff --git a/include/libcamera/internal/software_isp/swisp_linaro.h b/include/libcamera/internal/software_isp/swisp_linaro.h
index c0df7863..f7fc7682 100644
--- a/include/libcamera/internal/software_isp/swisp_linaro.h
+++ b/include/libcamera/internal/software_isp/swisp_linaro.h
@@ -89,6 +89,11 @@ private:
 		int setDebayerInfo(PixelFormat format);
 		debayerInfo *debayerInfo_;
 
+	  	void debayerRaw(uint8_t *dst, const uint8_t *src, bool is10p);
+	  
+		/* Bayer 8 */
+		void debayerRaw8(uint8_t *dst, const uint8_t *src);
+
 		/* CSI-2 packed 10-bit raw bayer format (all the 4 orders) */
 		void debayerRaw10P(uint8_t *dst, const uint8_t *src);
 		static SizeRange outSizesRaw10P(const Size &inSize);
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index b95d7689..4561961e 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -1137,6 +1137,14 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo
 	 */
 	for ([[maybe_unused]] StreamRole role : roles) {
 		StreamConfiguration cfg{ StreamFormats{ formats } };
+		if (! formats.begin()->first) {
+		  LOG(SimplePipeline, Error)
+			<< "No formats -- first?";
+		  continue;
+		}
+		  LOG(SimplePipeline, Error)
+			<< "Going through roles?";
+
 		cfg.pixelFormat = formats.begin()->first;
 		cfg.size = formats.begin()->second[0].max;
 
diff --git a/src/libcamera/software_isp/swisp_linaro.cpp b/src/libcamera/software_isp/swisp_linaro.cpp
index b7f36db1..0255ce85 100644
--- a/src/libcamera/software_isp/swisp_linaro.cpp
+++ b/src/libcamera/software_isp/swisp_linaro.cpp
@@ -88,7 +88,7 @@ bool SwIspLinaro::isValid() const
  * \todo Split the stats calculations out of this function.
  * \todo Move the AWB algorithm into the IPA module.
  */
-void SwIspLinaro::IspWorker::debayerRaw10P(uint8_t *dst, const uint8_t *src)
+void SwIspLinaro::IspWorker::debayerRaw(uint8_t *dst, const uint8_t *src, bool is10p)
 {
 	/* for brightness values in the 0 to 255 range: */
 	static const unsigned int BRIGHT_LVL = 200U << 8;
@@ -118,9 +118,15 @@ void SwIspLinaro::IspWorker::debayerRaw10P(uint8_t *dst, const uint8_t *src)
 			int phase = 2 * phase_y + phase_x;
 
 			/* x part of the offset in the input buffer: */
-			int x_m1 = x + x / 4;		/* offset for (x-1) */
-			int x_0 = x + 1 + (x + 1) / 4;	/* offset for x */
-			int x_p1 = x + 2 + (x + 2) / 4;	/* offset for (x+1) */
+			int x_m1 = x;		/* offset for (x-1) */
+			int x_0 = x + 1;	/* offset for x */
+			int x_p1 = x + 2;	/* offset for (x+1) */
+
+			if (is10p) {
+				x_m1 += x_m1 / 4;	/* offset for (x-1) */
+				x_0 += x_0 / 4;	/* offset for x */
+				x_p1 += x_p1 / 4;	/* offset for (x+1) */
+			}
 			/* the colour component value to write to the output */
 			unsigned val;
 			/* Y value times 256 */
@@ -267,6 +273,16 @@ void SwIspLinaro::IspWorker::debayerRaw10P(uint8_t *dst, const uint8_t *src)
 		<< " (minDenom = " << minDenom << ")";
 }
 
+void SwIspLinaro::IspWorker::debayerRaw10P(uint8_t *dst, const uint8_t *src)
+{
+	debayerRaw(dst, src, true);
+}
+
+void SwIspLinaro::IspWorker::debayerRaw8(uint8_t *dst, const uint8_t *src)
+{
+	debayerRaw(dst, src, false);
+}
+
 SizeRange SwIspLinaro::IspWorker::outSizesRaw10P(const Size &inSize)
 {
 	if (inSize.width < 2 || inSize.height < 2) {
@@ -286,6 +302,10 @@ unsigned int SwIspLinaro::IspWorker::outStrideRaw10P(const Size &outSize)
 SwIspLinaro::IspWorker::IspWorker(SwIspLinaro *swIsp)
 	: swIsp_(swIsp)
 {
+	debayerInfos_[formats::SBGGR8] = { formats::RGB888,
+						  &SwIspLinaro::IspWorker::debayerRaw8,
+						  &SwIspLinaro::IspWorker::outSizesRaw10P,
+						  &SwIspLinaro::IspWorker::outStrideRaw10P };
 	debayerInfos_[formats::SBGGR10_CSI2P] = { formats::RGB888,
 						  &SwIspLinaro::IspWorker::debayerRaw10P,
 						  &SwIspLinaro::IspWorker::outSizesRaw10P,