libcamera: software_isp: Apply CCM swap also on green
diff mbox series

Message ID 20251017154412.41430-1-mzamazal@redhat.com
State New
Headers show
Series
  • libcamera: software_isp: Apply CCM swap also on green
Related show

Commit Message

Milan Zamazal Oct. 17, 2025, 3:44 p.m. UTC
When CPU ISP is asked to apply the CCM matrix

  [0 1 0]
  [0 0 0]
  [0 0 0]

for a format that requires swapping red and blue channels, the resulting
image has a wrong colour.  The CCM matrix above should take green from
pixels and make it red.  Instead, the image is blue.

The problem is that the lookup tables setup in CPU debayering swaps red
and blue in the lookup tables for red and blue, but not for green.  The
colours must be swapped also in the lookup table for green, which this
patch adds.

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

Comments

Bryan O'Donoghue Oct. 17, 2025, 4:24 p.m. UTC | #1
On 17/10/2025 16:44, Milan Zamazal wrote:
> When CPU ISP is asked to apply the CCM matrix
> 
>    [0 1 0]
>    [0 0 0]
>    [0 0 0]
> 
> for a format that requires swapping red and blue channels, the resulting
> image has a wrong colour.  The CCM matrix above should take green from
> pixels and make it red.  Instead, the image is blue.
> 
> The problem is that the lookup tables setup in CPU debayering swaps red
> and blue in the lookup tables for red and blue, but not for green.  The
> colours must be swapped also in the lookup table for green, which this
> patch adds.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>   src/libcamera/software_isp/debayer_cpu.cpp | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index c2fb11bae..67a303c65 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -803,6 +803,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output
>   		blueCcm_ = params.redCcm;
>   		for (unsigned int i = 0; i < 256; i++) {
>   			std::swap(redCcm_[i].r, redCcm_[i].b);
> +			std::swap(greenCcm_[i].r, greenCcm_[i].b);
>   			std::swap(blueCcm_[i].r, blueCcm_[i].b);
>   		}
>   	} else {
> --
> 2.51.0
> 
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Kieran Bingham Oct. 18, 2025, 11:11 a.m. UTC | #2
Quoting Milan Zamazal (2025-10-17 16:44:12)
> When CPU ISP is asked to apply the CCM matrix
> 
>   [0 1 0]
>   [0 0 0]
>   [0 0 0]
> 
> for a format that requires swapping red and blue channels, the resulting
> image has a wrong colour.  The CCM matrix above should take green from
> pixels and make it red.  Instead, the image is blue.
> 
> The problem is that the lookup tables setup in CPU debayering swaps red
> and blue in the lookup tables for red and blue, but not for green.  The
> colours must be swapped also in the lookup table for green, which this
> patch adds.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>  src/libcamera/software_isp/debayer_cpu.cpp | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index c2fb11bae..67a303c65 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -803,6 +803,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output
>                 blueCcm_ = params.redCcm;
>                 for (unsigned int i = 0; i < 256; i++) {
>                         std::swap(redCcm_[i].r, redCcm_[i].b);
> +                       std::swap(greenCcm_[i].r, greenCcm_[i].b);
>                         std::swap(blueCcm_[i].r, blueCcm_[i].b);

I haven't looked at the full context surrounding this - but it seems
reasonable - and it sounds like this fixes an issue for you in the
SoftISP too.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>                 }
>         } else {
> -- 
> 2.51.0
>

Patch
diff mbox series

diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index c2fb11bae..67a303c65 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -803,6 +803,7 @@  void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output
 		blueCcm_ = params.redCcm;
 		for (unsigned int i = 0; i < 256; i++) {
 			std::swap(redCcm_[i].r, redCcm_[i].b);
+			std::swap(greenCcm_[i].r, greenCcm_[i].b);
 			std::swap(blueCcm_[i].r, blueCcm_[i].b);
 		}
 	} else {