[4/9] libcamera: software_isp: Use common code to store debayered pixels
diff mbox series

Message ID 20241120180104.1221846-5-mzamazal@redhat.com
State Superseded
Headers show
Series
  • Software ISP support for CCM
Related show

Commit Message

Milan Zamazal Nov. 20, 2024, 6 p.m. UTC 
The debayering macros use the same pattern, let's extract it to a common
macro.  This reduces code duplication a bit now and it'll make changes
of debayering easier when color correction matrix is introduced.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/software_isp/debayer_cpu.cpp | 56 +++++++++++-----------
 1 file changed, 28 insertions(+), 28 deletions(-)

Comments

Robert Mader Nov. 24, 2024, 4:10 p.m. UTC | #1 
This temporarily introduces a red <-> blue color swap, see below.

On 20.11.24 19:00, Milan Zamazal wrote:
> The debayering macros use the same pattern, let's extract it to a common
> macro.  This reduces code duplication a bit now and it'll make changes
> of debayering easier when color correction matrix is introduced.
>
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>   src/libcamera/software_isp/debayer_cpu.cpp | 56 +++++++++++-----------
>   1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index cf5ecdf7a..b9497d574 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -61,57 +61,57 @@ DebayerCpu::~DebayerCpu() = default;
>   	const pixel_t *curr = (const pixel_t *)src[1] + xShift_; \
>   	const pixel_t *next = (const pixel_t *)src[2] + xShift_;
>   
> +#define STORE_PIXEL(b, g, r)        \
> +	*dst++ = blue_[r];          \
> +	*dst++ = green_[g];         \
> +	*dst++ = red_[b];           \

blue_[r] -> blue_[b] and red_[b] -> red_[r]

> +	if constexpr (addAlphaByte) \
> +		*dst++ = 255;       \
> +	x++;
> +
>   /*
>    * RGR
>    * GBG
>    * RGR
>    */
> -#define BGGR_BGR888(p, n, div)                                                                \
> -	*dst++ = blue_[curr[x] / (div)];                                                      \
> -	*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))];       \
> -	*dst++ = red_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \
> -	if constexpr (addAlphaByte)                                                           \
> -		*dst++ = 255;                                                                 \
> -	x++;
> +#define BGGR_BGR888(p, n, div)                                                 \
> +	STORE_PIXEL(                                                           \
> +		curr[x] / (div),                                               \
> +		(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div)), \
> +		(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div)))
>   
>   /*
>    * GBG
>    * RGR
>    * GBG
>    */
> -#define GRBG_BGR888(p, n, div)                                    \
> -	*dst++ = blue_[(prev[x] + next[x]) / (2 * (div))];        \
> -	*dst++ = green_[curr[x] / (div)];                         \
> -	*dst++ = red_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \
> -	if constexpr (addAlphaByte)                               \
> -		*dst++ = 255;                                     \
> -	x++;
> +#define GRBG_BGR888(p, n, div)                     \
> +	STORE_PIXEL(                               \
> +		(prev[x] + next[x]) / (2 * (div)), \
> +		curr[x] / (div),                   \
> +		(curr[x - p] + curr[x + n]) / (2 * (div)))
>   
>   /*
>    * GRG
>    * BGB
>    * GRG
>    */
> -#define GBRG_BGR888(p, n, div)                                     \
> -	*dst++ = blue_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \
> -	*dst++ = green_[curr[x] / (div)];                          \
> -	*dst++ = red_[(prev[x] + next[x]) / (2 * (div))];          \
> -	if constexpr (addAlphaByte)                                \
> -		*dst++ = 255;                                      \
> -	x++;
> +#define GBRG_BGR888(p, n, div)                             \
> +	STORE_PIXEL(                                       \
> +		(curr[x - p] + curr[x + n]) / (2 * (div)), \
> +		curr[x] / (div),                           \
> +		(prev[x] + next[x]) / (2 * (div)))
>   
>   /*
>    * BGB
>    * GRG
>    * BGB
>    */
> -#define RGGB_BGR888(p, n, div)                                                                 \
> -	*dst++ = blue_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \
> -	*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))];        \
> -	*dst++ = red_[curr[x] / (div)];                                                        \
> -	if constexpr (addAlphaByte)                                                            \
> -		*dst++ = 255;                                                                  \
> -	x++;
> +#define RGGB_BGR888(p, n, div)                                                         \
> +	STORE_PIXEL(                                                                   \
> +		(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div)), \
> +		(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div)),         \
> +		curr[x] / (div))
>   
>   template<bool addAlphaByte>
>   void DebayerCpu::debayer8_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])
Milan Zamazal Nov. 25, 2024, 10:54 a.m. UTC | #2 
Hi Robert,

Robert Mader <robert.mader@collabora.com> writes:

> This temporarily introduces a red <-> blue color swap, see below.
>
> On 20.11.24 19:00, Milan Zamazal wrote:
>> The debayering macros use the same pattern, let's extract it to a common
>> macro.  This reduces code duplication a bit now and it'll make changes
>> of debayering easier when color correction matrix is introduced.
>>
>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
>> ---
>>   src/libcamera/software_isp/debayer_cpu.cpp | 56 +++++++++++-----------
>>   1 file changed, 28 insertions(+), 28 deletions(-)
>>
>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
>> index cf5ecdf7a..b9497d574 100644
>> --- a/src/libcamera/software_isp/debayer_cpu.cpp
>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
>> @@ -61,57 +61,57 @@ DebayerCpu::~DebayerCpu() = default;
>>   	const pixel_t *curr = (const pixel_t *)src[1] + xShift_; \
>>   	const pixel_t *next = (const pixel_t *)src[2] + xShift_;
>>   +#define STORE_PIXEL(b, g, r)        \
>> +	*dst++ = blue_[r];          \
>> +	*dst++ = green_[g];         \
>> +	*dst++ = red_[b];           \
>
> blue_[r] -> blue_[b] and red_[b] -> red_[r]

Oh, thank you, I'll fix it in v2.

>> +	if constexpr (addAlphaByte) \
>> +		*dst++ = 255;       \
>> +	x++;
>> +
>>   /*
>>    * RGR
>>    * GBG
>>    * RGR
>>    */
>> -#define BGGR_BGR888(p, n, div)                                                                \
>> -	*dst++ = blue_[curr[x] / (div)];                                                      \
>> -	*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))];       \
>> -	*dst++ = red_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \
>> -	if constexpr (addAlphaByte)                                                           \
>> -		*dst++ = 255;                                                                 \
>> -	x++;
>> +#define BGGR_BGR888(p, n, div)                                                 \
>> +	STORE_PIXEL(                                                           \
>> +		curr[x] / (div),                                               \
>> +		(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div)), \
>> +		(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div)))
>>     /*
>>    * GBG
>>    * RGR
>>    * GBG
>>    */
>> -#define GRBG_BGR888(p, n, div)                                    \
>> -	*dst++ = blue_[(prev[x] + next[x]) / (2 * (div))];        \
>> -	*dst++ = green_[curr[x] / (div)];                         \
>> -	*dst++ = red_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \
>> -	if constexpr (addAlphaByte)                               \
>> -		*dst++ = 255;                                     \
>> -	x++;
>> +#define GRBG_BGR888(p, n, div)                     \
>> +	STORE_PIXEL(                               \
>> +		(prev[x] + next[x]) / (2 * (div)), \
>> +		curr[x] / (div),                   \
>> +		(curr[x - p] + curr[x + n]) / (2 * (div)))
>>     /*
>>    * GRG
>>    * BGB
>>    * GRG
>>    */
>> -#define GBRG_BGR888(p, n, div)                                     \
>> -	*dst++ = blue_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \
>> -	*dst++ = green_[curr[x] / (div)];                          \
>> -	*dst++ = red_[(prev[x] + next[x]) / (2 * (div))];          \
>> -	if constexpr (addAlphaByte)                                \
>> -		*dst++ = 255;                                      \
>> -	x++;
>> +#define GBRG_BGR888(p, n, div)                             \
>> +	STORE_PIXEL(                                       \
>> +		(curr[x - p] + curr[x + n]) / (2 * (div)), \
>> +		curr[x] / (div),                           \
>> +		(prev[x] + next[x]) / (2 * (div)))
>>     /*
>>    * BGB
>>    * GRG
>>    * BGB
>>    */
>> -#define RGGB_BGR888(p, n, div)                                                                 \
>> -	*dst++ = blue_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \
>> -	*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))];        \
>> -	*dst++ = red_[curr[x] / (div)];                                                        \
>> -	if constexpr (addAlphaByte)                                                            \
>> -		*dst++ = 255;                                                                  \
>> -	x++;
>> +#define RGGB_BGR888(p, n, div)                                                         \
>> +	STORE_PIXEL(                                                                   \
>> +		(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div)), \
>> +		(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div)),         \
>> +		curr[x] / (div))
>>     template<bool addAlphaByte>
>>   void DebayerCpu::debayer8_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index cf5ecdf7a..b9497d574 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -61,57 +61,57 @@  DebayerCpu::~DebayerCpu() = default;
 	const pixel_t *curr = (const pixel_t *)src[1] + xShift_; \
 	const pixel_t *next = (const pixel_t *)src[2] + xShift_;
 
+#define STORE_PIXEL(b, g, r)        \
+	*dst++ = blue_[r];          \
+	*dst++ = green_[g];         \
+	*dst++ = red_[b];           \
+	if constexpr (addAlphaByte) \
+		*dst++ = 255;       \
+	x++;
+
 /*
  * RGR
  * GBG
  * RGR
  */
-#define BGGR_BGR888(p, n, div)                                                                \
-	*dst++ = blue_[curr[x] / (div)];                                                      \
-	*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))];       \
-	*dst++ = red_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \
-	if constexpr (addAlphaByte)                                                           \
-		*dst++ = 255;                                                                 \
-	x++;
+#define BGGR_BGR888(p, n, div)                                                 \
+	STORE_PIXEL(                                                           \
+		curr[x] / (div),                                               \
+		(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div)), \
+		(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div)))
 
 /*
  * GBG
  * RGR
  * GBG
  */
-#define GRBG_BGR888(p, n, div)                                    \
-	*dst++ = blue_[(prev[x] + next[x]) / (2 * (div))];        \
-	*dst++ = green_[curr[x] / (div)];                         \
-	*dst++ = red_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \
-	if constexpr (addAlphaByte)                               \
-		*dst++ = 255;                                     \
-	x++;
+#define GRBG_BGR888(p, n, div)                     \
+	STORE_PIXEL(                               \
+		(prev[x] + next[x]) / (2 * (div)), \
+		curr[x] / (div),                   \
+		(curr[x - p] + curr[x + n]) / (2 * (div)))
 
 /*
  * GRG
  * BGB
  * GRG
  */
-#define GBRG_BGR888(p, n, div)                                     \
-	*dst++ = blue_[(curr[x - p] + curr[x + n]) / (2 * (div))]; \
-	*dst++ = green_[curr[x] / (div)];                          \
-	*dst++ = red_[(prev[x] + next[x]) / (2 * (div))];          \
-	if constexpr (addAlphaByte)                                \
-		*dst++ = 255;                                      \
-	x++;
+#define GBRG_BGR888(p, n, div)                             \
+	STORE_PIXEL(                                       \
+		(curr[x - p] + curr[x + n]) / (2 * (div)), \
+		curr[x] / (div),                           \
+		(prev[x] + next[x]) / (2 * (div)))
 
 /*
  * BGB
  * GRG
  * BGB
  */
-#define RGGB_BGR888(p, n, div)                                                                 \
-	*dst++ = blue_[(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div))]; \
-	*dst++ = green_[(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div))];        \
-	*dst++ = red_[curr[x] / (div)];                                                        \
-	if constexpr (addAlphaByte)                                                            \
-		*dst++ = 255;                                                                  \
-	x++;
+#define RGGB_BGR888(p, n, div)                                                         \
+	STORE_PIXEL(                                                                   \
+		(prev[x - p] + prev[x + n] + next[x - p] + next[x + n]) / (4 * (div)), \
+		(prev[x] + curr[x - p] + curr[x + n] + next[x]) / (4 * (div)),         \
+		curr[x] / (div))
 
 template<bool addAlphaByte>
 void DebayerCpu::debayer8_BGBG_BGR888(uint8_t *dst, const uint8_t *src[])