[libcamera-devel] [RFC PATCH v2 4/7] libcamera: ipa: Soft IPA: add a Soft IPA implementation

Hans de Goede hdegoede at redhat.com
Wed Dec 13 21:35:56 CET 2023


Hi Pavel,

On 12/13/23 08:39, Pavel Machek wrote:
> Hi!
> 
>> ATM the statistics and debayer classes are mostly ready,
>> but I need to merge this with Andrey's v2 RFC and
>> then produce a clean new series from this
>> (assuming people like the approach).
> 
> Ok, what about this one? I only converted half of the code, but the
> other half of conversion should be very similar.
> 
> Compile-tested only as my sensor is bayer 8.
> 
> It makes it easier to see how we are skipping the low bits in 10P
> bayer, and will make bayer 8 support easier, too, since the pixel_XXX
> helpers are designed to be shared.

I have merged a variant of this with:

1. The missing bits completed
2. Various renames to make it clear that the helpers are for 8bit bayer
   sources (we are treating 10bpp packed as 8 bit here) and that
   the output is bgr888 this should make life easier when adding
   support for more input / output fmts.

Here is what I've merged / squashed:

>From 9e7bd0f37c38bf9500924187720621538f4581da Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede at redhat.com>
Date: Wed, 13 Dec 2023 19:19:11 +0100
Subject: [PATCH] Pavel debayer changes

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 .../internal/software_isp/debayer_cpu.h       |   8 +-
 src/libcamera/software_isp/debayer_cpu.cpp    | 247 +++++++-----------
 2 files changed, 97 insertions(+), 158 deletions(-)

diff --git a/include/libcamera/internal/software_isp/debayer_cpu.h b/include/libcamera/internal/software_isp/debayer_cpu.h
index 89824851..7fb5be77 100644
--- a/include/libcamera/internal/software_isp/debayer_cpu.h
+++ b/include/libcamera/internal/software_isp/debayer_cpu.h
@@ -62,10 +62,10 @@ private:
 	void process2(const uint8_t *src, uint8_t *dst);
 	void process4(const uint8_t *src, uint8_t *dst);
 	/* CSI-2 packed 10-bit raw bayer format (all the 4 orders) */
-	void debayerBGGR10PLine0(uint8_t *dst, const uint8_t *src);
-	void debayerBGGR10PLine1(uint8_t *dst, const uint8_t *src);
-	void debayerGBRG10PLine0(uint8_t *dst, const uint8_t *src);
-	void debayerGBRG10PLine1(uint8_t *dst, const uint8_t *src);
+	void debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src);
+	void debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src);
+	void debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src);
+	void debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src);
 
 	typedef void (DebayerCpu::*debayerFn)(uint8_t *dst, const uint8_t *src);
 
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index b6ed12de..3eacdd5d 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -29,180 +29,121 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats)
 		green_[i] = i;
 }
 
-void DebayerCpu::debayerBGGR10PLine0(uint8_t *dst, const uint8_t *src)
+struct ctxt_8bit_src {
+	/* Pointers to previous, current and next lines */
+	const uint8_t *prev;
+	const uint8_t *curr;
+	const uint8_t *next;
+
+	/* Pointers to gamma correction tables */
+	const uint8_t *red;
+	const uint8_t *green;
+	const uint8_t *blue;
+};
+
+static inline void bggr8_bgr888(const struct ctxt_8bit_src &c, uint8_t *&dst, int x, int p, int n)
+{
+	*dst++ = c.blue[c.curr[x]];
+	*dst++ = c.green[(c.prev[x] + c.curr[x - p] + c.curr[x + n] + c.next[x]) / 4];
+	*dst++ = c.red[(c.prev[x - p] + c.prev[x + n] + c.next[x - p]  + c.next[x + n]) / 4];
+}
+
+static inline void grbg8_bgr888(const struct ctxt_8bit_src &c, uint8_t *&dst, int x, int p, int n)
+{
+	*dst++ = c.blue[(c.prev[x] + c.next[x]) / 2];
+	*dst++ = c.green[c.curr[x]];
+	*dst++ = c.red[(c.curr[x - p] + c.curr[x + n]) / 2];
+}
+
+static inline void gbrg8_bgr888(const struct ctxt_8bit_src &c, uint8_t *&dst, int x, int p, int n)
+{
+	*dst++ = c.blue[(c.curr[x - p] + c.curr[x + n]) / 2];
+	*dst++ = c.green[c.curr[x]];
+	*dst++ = c.red[(c.prev[x] + c.next[x]) / 2];
+}
+
+static inline void rggb8_bgr888(const struct ctxt_8bit_src &c, uint8_t *&dst, int x, int p, int n)
+{
+	*dst++ = c.blue[(c.prev[x - p] + c.prev[x + n] + c.next[x - p]  + c.next[x + n]) / 4];
+	*dst++ = c.green[(c.prev[x] + c.curr[x - p] + c.curr[x + n] + c.next[x]) / 4];
+	*dst++ = c.red[c.curr[x]];
+}
+
+void DebayerCpu::debayer10P_BGBG_BGR888(uint8_t *dst, const uint8_t *src)
 {
 	const int width_in_bytes = window_.width * 5 / 4;
-	/* Pointers to previous, current and next lines */
-	const uint8_t *prev = src - inputConfig_.stride;
-	const uint8_t *curr = src;
-	const uint8_t *next = src + inputConfig_.stride;
+	struct ctxt_8bit_src c = {
+		src - inputConfig_.stride, src, src + inputConfig_.stride,
+		red_, green_, blue_ };
 
 	/*
 	 * For the first pixel getting a pixel from the previous column uses
 	 * x - 2 to skip the 5th byte with least-significant bits for 4 pixels.
 	 * Same for last pixel (uses x + 2) and looking at the next column.
+	 * x++ in the for-loop skips the 5th byte with 4 x 2 lsb-s for 10bit packed.
 	 */
-	for (int x = 0; x < width_in_bytes; x += 2) {
-		/*
-		 * BGBG line even pixel: RGR
-		 *                       GBG
-		 *                       RGR
-		 * Write BGR
-		 */
-		*dst++ = blue_[curr[x]];
-		*dst++ = green_[(prev[x] + curr[x - 2] + curr[x + 1] + next[x]) / 4];
-		*dst++ = red_[(prev[x - 2] + prev[x + 1] + next[x - 2]  + next[x + 1]) / 4];
-		x++;
-
-		/*
-		 * BGBG line odd pixel: GRG
-		 *                      BGB
-		 *                      GRG
-		 * Write BGR
-		 */
-		*dst++ = blue_[(curr[x - 1] + curr[x + 1]) / 2];
-		*dst++ = green_[curr[x]];
-		*dst++ = red_[(prev[x] + next[x]) / 2];
-		x++;
-
+	for (int x = 0; x < width_in_bytes; x++) {
+		/* Even pixel */
+		bggr8_bgr888(c, dst, x++, 2, 1);
+		/* Odd pixel BGGR -> GBRG */
+		gbrg8_bgr888(c, dst, x++, 1, 1);
 		/* Same thing for next 2 pixels */
-		*dst++ = blue_[curr[x]];
-		*dst++ = green_[(prev[x] + curr[x - 1] + curr[x + 1] + next[x]) / 4];
-		*dst++ = red_[(prev[x - 1] + prev[x + 1] + next[x - 1]  + next[x + 1]) / 4];
-		x++;
-
-		*dst++ = blue_[(curr[x - 1] + curr[x + 2]) / 2];
-		*dst++ = green_[curr[x]];
-		*dst++ = red_[(prev[x] + next[x]) / 2];
+		bggr8_bgr888(c, dst, x++, 1, 1);
+		gbrg8_bgr888(c, dst, x++, 1, 2);
 	}
 }
 
-void DebayerCpu::debayerBGGR10PLine1(uint8_t *dst, const uint8_t *src)
+void DebayerCpu::debayer10P_GRGR_BGR888(uint8_t *dst, const uint8_t *src)
 {
 	const int width_in_bytes = window_.width * 5 / 4;
-	/* Pointers to previous, current and next lines */
-	const uint8_t *prev = src - inputConfig_.stride;
-	const uint8_t *curr = src;
-	const uint8_t *next = src + inputConfig_.stride;
-
-	for (int x = 0; x < width_in_bytes; x += 2) {
-		/*
-		 * GRGR line even pixel: GBG
-		 *                       RGR
-		 *                       GBG
-		 * Write BGR
-		 */
-		*dst++ = blue_[(prev[x] + next[x]) / 2];
-		*dst++ = green_[curr[x]];
-		*dst++ = red_[(curr[x - 2] + curr[x + 1]) / 2];
-		x++;
-
-		/*
-		 * GRGR line odd pixel: BGB
-		 *                      GRG
-		 *                      BGB
-		 * Write BGR
-		 */
-		*dst++ = blue_[(prev[x - 1] + prev[x + 1] + next[x - 1] + next[x + 1]) / 4];
-		*dst++ = green_[(prev[x] + curr[x - 1] + curr[x + 1] + next[x]) / 4];
-		*dst++ = red_[curr[x]];
-		x++;
+	struct ctxt_8bit_src c = {
+		src - inputConfig_.stride, src, src + inputConfig_.stride,
+		red_, green_, blue_ };
 
+	for (int x = 0; x < width_in_bytes; x++) {
+		/* Even pixel */
+		grbg8_bgr888(c, dst, x++, 2, 1);
+		/* Odd pixel GRBG -> RGGB */
+		rggb8_bgr888(c, dst, x++, 1, 1);
 		/* Same thing for next 2 pixels */
-		*dst++ = blue_[(prev[x] + next[x]) / 2];
-		*dst++ = green_[curr[x]];
-		*dst++ = red_[(curr[x - 1] + curr[x + 1]) / 2];
-		x++;
-
-		*dst++ = blue_[(prev[x - 1] + prev[x + 2] + next[x - 1] + next[x + 2]) / 4];
-		*dst++ = green_[(prev[x] + curr[x - 1] + curr[x + 2] + next[x]) / 4];
-		*dst++ = red_[curr[x]];
+		grbg8_bgr888(c, dst, x++, 1, 1);
+		rggb8_bgr888(c, dst, x++, 1, 2);
 	}
 }
 
-void DebayerCpu::debayerGBRG10PLine0(uint8_t *dst, const uint8_t *src)
+void DebayerCpu::debayer10P_GBGB_BGR888(uint8_t *dst, const uint8_t *src)
 {
 	const int width_in_bytes = window_.width * 5 / 4;
-	/* Pointers to previous, current and next lines */
-	const uint8_t *prev = src - inputConfig_.stride;
-	const uint8_t *curr = src;
-	const uint8_t *next = src + inputConfig_.stride;
-
-	for (int x = 0; x < width_in_bytes; x += 2) {
-		/*
-		 * GBGB line even pixel: GRG
-		 *                       BGB
-		 *                       GRG
-		 * Write BGR
-		 */
-		*dst++ = blue_[(curr[x - 2] + curr[x + 1]) / 2];
-		*dst++ = green_[curr[x]];
-		*dst++ = red_[(prev[x] + next[x]) / 2];
-		x++;
-
-		/*
-		 * GBGB line odd pixel: RGR
-		 *                      GBG
-		 *                      RGR
-		 * Write BGR
-		 */
-		*dst++ = blue_[curr[x]];
-		*dst++ = green_[(prev[x] + curr[x - 1] + curr[x + 1] + next[x]) / 4];
-		*dst++ = red_[(prev[x - 1] + prev[x + 1] + next[x - 1]  + next[x + 1]) / 4];
-		x++;
+	struct ctxt_8bit_src c = {
+		src - inputConfig_.stride, src, src + inputConfig_.stride,
+		red_, green_, blue_ };
 
+	for (int x = 0; x < width_in_bytes; x++) {
+		/* Even pixel */
+		gbrg8_bgr888(c, dst, x++, 2, 1);
+		/* Odd pixel GBGR -> BGGR */
+		bggr8_bgr888(c, dst, x++, 1, 1);
 		/* Same thing for next 2 pixels */
-		*dst++ = blue_[(curr[x - 1] + curr[x + 1]) / 2];
-		*dst++ = green_[curr[x]];
-		*dst++ = red_[(prev[x] + next[x]) / 2];
-		x++;
-
-		*dst++ = blue_[curr[x]];
-		*dst++ = green_[(prev[x] + curr[x - 1] + curr[x + 2] + next[x]) / 4];
-		*dst++ = red_[(prev[x - 1] + prev[x + 2] + next[x - 1]  + next[x + 2]) / 4];
+		gbrg8_bgr888(c, dst, x++, 1, 1);
+		bggr8_bgr888(c, dst, x++, 1, 2);
 	}
 }
 
-void DebayerCpu::debayerGBRG10PLine1(uint8_t *dst, const uint8_t *src)
+void DebayerCpu::debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src)
 {
 	const int width_in_bytes = window_.width * 5 / 4;
-	/* Pointers to previous, current and next lines */
-	const uint8_t *prev = src - inputConfig_.stride;
-	const uint8_t *curr = src;
-	const uint8_t *next = src + inputConfig_.stride;
-
-	for (int x = 0; x < width_in_bytes; x += 2) {
-		/*
-		 * RGRG line even pixel: BGB
-		 *                       GRG
-		 *                       BGB
-		 * Write BGR
-		 */
-		*dst++ = blue_[(prev[x - 2] + prev[x + 1] + next[x - 2] + next[x + 1]) / 4];
-		*dst++ = green_[(prev[x] + curr[x - 2] + curr[x + 1] + next[x]) / 4];
-		*dst++ = red_[curr[x]];
-		x++;
-
-		/*
-		 * RGRG line odd pixel: GBG
-		 *                      RGR
-		 *                      GBG
-		 * Write BGR
-		 */
-		*dst++ = blue_[(prev[x] + next[x]) / 2];
-		*dst++ = green_[curr[x]];
-		*dst++ = red_[(curr[x - 1] + curr[x + 1]) / 2];
-		x++;
+	struct ctxt_8bit_src c = {
+		src - inputConfig_.stride, src, src + inputConfig_.stride,
+		red_, green_, blue_ };
 
+	for (int x = 0; x < width_in_bytes; x++) {
+		/* Even pixel */
+		rggb8_bgr888(c, dst, x++, 2, 1);
+		/* Odd pixel RGGB -> GRBG*/
+		grbg8_bgr888(c, dst, x++, 1, 1);
 		/* Same thing for next 2 pixels */
-		*dst++ = blue_[(prev[x - 1] + prev[x + 1] + next[x - 1] + next[x + 1]) / 4];
-		*dst++ = green_[(prev[x] + curr[x - 1] + curr[x + 1] + next[x]) / 4];
-		*dst++ = red_[curr[x]];
-		x++;
-
-		*dst++ = blue_[(prev[x] + next[x]) / 2];
-		*dst++ = green_[curr[x]];
-		*dst++ = red_[(curr[x - 1] + curr[x + 2]) / 2];
+		rggb8_bgr888(c, dst, x++, 1, 1);
+		grbg8_bgr888(c, dst, x++, 1, 2);
 	}
 }
 
@@ -258,22 +199,20 @@ int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, [[maybe_unused]] Pi
 	    bayerFormat.packing == BayerFormat::Packing::CSI2) {
 		switch (bayerFormat.order) {
 		case BayerFormat::BGGR:
-			debayer0_ = &DebayerCpu::debayerBGGR10PLine0;
-			debayer1_ = &DebayerCpu::debayerBGGR10PLine1;
+			debayer0_ = &DebayerCpu::debayer10P_BGBG_BGR888;
+			debayer1_ = &DebayerCpu::debayer10P_GRGR_BGR888;
 			return 0;
 		case BayerFormat::GBRG:
-			debayer0_ = &DebayerCpu::debayerGBRG10PLine0;
-			debayer1_ = &DebayerCpu::debayerGBRG10PLine1;
+			debayer0_ = &DebayerCpu::debayer10P_GBGB_BGR888;
+			debayer1_ = &DebayerCpu::debayer10P_RGRG_BGR888;
 			return 0;
 		case BayerFormat::GRBG:
-			/* GRBG is BGGR with the lines swapped */
-			debayer0_ = &DebayerCpu::debayerBGGR10PLine1;
-			debayer1_ = &DebayerCpu::debayerBGGR10PLine0;
+			debayer0_ = &DebayerCpu::debayer10P_GRGR_BGR888;
+			debayer1_ = &DebayerCpu::debayer10P_BGBG_BGR888;
 			return 0;
 		case BayerFormat::RGGB:
-			/* RGGB is GBRG with the lines swapped */
-			debayer0_ = &DebayerCpu::debayerGBRG10PLine1;
-			debayer1_ = &DebayerCpu::debayerGBRG10PLine0;
+			debayer0_ = &DebayerCpu::debayer10P_RGRG_BGR888;
+			debayer1_ = &DebayerCpu::debayer10P_GBGB_BGR888;
 			return 0;
 		default:
 			break;
-- 
2.41.0


Regards,

Hans




More information about the libcamera-devel mailing list