[v4,06/23] libcamera: shaders: Extend debayer shaders to apply RGB gain values on output
diff mbox series

Message ID 20251120233347.5046-7-bryan.odonoghue@linaro.org
State Superseded
Headers show
Series
  • Add GLES 2.0 GPUISP to libcamera
Related show

Commit Message

Bryan O'Donoghue Nov. 20, 2025, 11:33 p.m. UTC
Extend out the bayer fragment shaders to take 3 x 256 byte inputs as
textures from the CPU.

We then use an index to the table to recover the colour-corrected values
provided by the SoftIPA thread.

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../internal/shaders/bayer_1x_packed.frag     | 56 +++++++++++++++++
 .../internal/shaders/bayer_unpacked.frag      | 62 ++++++++++++++++++-
 2 files changed, 117 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Nov. 21, 2025, 3:14 a.m. UTC | #1
You're mixing tabs and spaces for indentation in the file.

On Thu, Nov 20, 2025 at 11:33:30PM +0000, Bryan O'Donoghue wrote:
> Extend out the bayer fragment shaders to take 3 x 256 byte inputs as
> textures from the CPU.
> 
> We then use an index to the table to recover the colour-corrected values
> provided by the SoftIPA thread.
> 
> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../internal/shaders/bayer_1x_packed.frag     | 56 +++++++++++++++++
>  .../internal/shaders/bayer_unpacked.frag      | 62 ++++++++++++++++++-
>  2 files changed, 117 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag
> index 19b13ad08..90bd64570 100644
> --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag
> +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag
> @@ -65,6 +65,10 @@ uniform vec2 tex_step;
>  uniform vec2 tex_bayer_first_red;
>  
>  uniform sampler2D tex_y;
> +uniform sampler2D red_param;
> +uniform sampler2D green_param;
> +uniform sampler2D blue_param;
> +uniform mat3 ccm;
>  
>  void main(void)
>  {
> @@ -212,5 +216,57 @@ void main(void)
>  			vec3(patterns.y, C, patterns.x) :
>  			vec3(patterns.wz, C));
>  
> +#if defined(APPLY_CCM_PARAMETERS)
> +	/*
> +	 *   CCM is a 3x3 in the format
> +	 *
> +	 *   +--------------+----------------+---------------+
> +	 *   | RedRedGain   | RedGreenGain   | RedBlueGain   |
> +	 *   +--------------+----------------+---------------+
> +	 *   | GreenRedGain | GreenGreenGain | GreenBlueGain |
> +	 *   +--------------+----------------+---------------+
> +	 *   | BlueRedGain  |  BlueGreenGain | BlueBlueGain  |
> +	 *   +--------------+----------------+---------------+
> +	 *
> +	 *   Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin
> +	 *   Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin
> +	 *   Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin
> +	 *
> +	 *   We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm);
> +	 *
> +	 *   CPU
> +	 *   float ccm [] = {
> +	 *             RedRedGain,   RedGreenGain,   RedBlueGain,
> +	 *             GreenRedGain, GreenGreenGain, GreenBlueGain,
> +	 *             BlueRedGain,  BlueGreenGain,  BlueBlueGain,
> +	 *   };
> +	 *
> +	 *   GPU
> +	 *   ccm = {
> +	 *             RedRedGain,   GreenRedGain,   BlueRedGain,
> +	 *             RedGreenGain, GreenGreenGain, BlueGreenGain,
> +	 *             RedBlueGain,  GreenBlueGain,  BlueBlueGain,
> +	 *   }
> +	 *
> +	 *   However the indexing for the mat data-type is column major hence
> +	 *   ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain
> +	 *
> +	 */
> +	float rin, gin, bin;
> +	rin = rgb.r;
> +	gin = rgb.g;
> +	bin = rgb.b;
> +
> +	rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]);
> +	rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]);
> +	rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]);
> +
> +#elif defined(APPLY_RGB_PARAMETERS)
> +	/* Apply bayer params */
> +	rgb.r = texture2D(red_param, vec2(rgb.r, 0.5)).r;
> +	rgb.g = texture2D(green_param, vec2(rgb.g, 0.5)).g;
> +	rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
> +#endif
> +
>  	gl_FragColor = vec4(rgb, 1.0);
>  }
> diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag
> index aa7a1b004..7c3d12b03 100644
> --- a/include/libcamera/internal/shaders/bayer_unpacked.frag
> +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag
> @@ -21,11 +21,17 @@ precision highp float;
>  
>  /** Monochrome RGBA or GL_LUMINANCE Bayer encoded texture.*/
>  uniform sampler2D       tex_y;
> +uniform sampler2D	red_param;
> +uniform sampler2D	green_param;
> +uniform sampler2D	blue_param;
>  varying vec4            center;
>  varying vec4            yCoord;
>  varying vec4            xCoord;
> +uniform mat3		ccm;
>  
>  void main(void) {
> +    vec3 rgb;
> +
>      #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r
>  
>      float C = texture2D(tex_y, center.xy).r; // ( 0, 0)
> @@ -97,11 +103,65 @@ void main(void) {
>      PATTERN.xw  += kB.xw * B;
>      PATTERN.xz  += kF.xz * F;
>  
> -    gl_FragColor.rgb = (alternate.y == 0.0) ?
> +    rgb =  (alternate.y == 0.0) ?
>          ((alternate.x == 0.0) ?
>              vec3(C, PATTERN.xy) :
>              vec3(PATTERN.z, C, PATTERN.w)) :
>          ((alternate.x == 0.0) ?
>              vec3(PATTERN.w, C, PATTERN.z) :
>              vec3(PATTERN.yx, C));
> +
> +#if defined(APPLY_CCM_PARAMETERS)
> +	/*
> +	 *   CCM is a 3x3 in the format
> +	 *
> +	 *   +--------------+----------------+---------------+
> +	 *   | RedRedGain   | RedGreenGain   | RedBlueGain   |
> +	 *   +--------------+----------------+---------------+
> +	 *   | GreenRedGain | GreenGreenGain | GreenBlueGain |
> +	 *   +--------------+----------------+---------------+
> +	 *   | BlueRedGain  |  BlueGreenGain | BlueBlueGain  |
> +	 *   +--------------+----------------+---------------+
> +	 *
> +	 *   Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin
> +	 *   Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin
> +	 *   Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin
> +	 *
> +	 *   We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm);
> +	 *
> +	 *   CPU
> +	 *   float ccm [] = {
> +	 *             RedRedGain,   RedGreenGain,   RedBlueGain,
> +	 *             GreenRedGain, GreenGreenGain, GreenBlueGain,
> +	 *             BlueRedGain,  BlueGreenGain,  BlueBlueGain,
> +	 *   };
> +	 *
> +	 *   GPU
> +	 *   ccm = {
> +	 *             RedRedGain,   GreenRedGain,   BlueRedGain,
> +	 *             RedGreenGain, GreenGreenGain, BlueGreenGain,
> +	 *             RedBlueGain,  GreenBlueGain,  BlueBlueGain,
> +	 *   }
> +	 *
> +	 *   However the indexing for the mat data-type is column major hence
> +	 *   ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain
> +	 *
> +	 */
> +	float rin, gin, bin;
> +	rin = rgb.r;
> +	gin = rgb.g;
> +	bin = rgb.b;
> +
> +	rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]);
> +	rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]);
> +	rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]);
> +
> +#elif defined(APPLY_RGB_PARAMETERS)
> +	/* Apply bayer params */
> +	rgb.r = texture2D(red_param, vec2(rgb.r, 0.5)).r;
> +	rgb.g = texture2D(green_param, vec2(rgb.g, 0.5)).g;
> +	rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
> +#endif
> +
> +    gl_FragColor.rgb = rgb;
>  }
> -- 
> 2.51.2
>
Laurent Pinchart Nov. 21, 2025, 3:18 a.m. UTC | #2
On Thu, Nov 20, 2025 at 11:33:30PM +0000, Bryan O'Donoghue wrote:
> Extend out the bayer fragment shaders to take 3 x 256 byte inputs as
> textures from the CPU.
> 
> We then use an index to the table to recover the colour-corrected values
> provided by the SoftIPA thread.
> 
> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../internal/shaders/bayer_1x_packed.frag     | 56 +++++++++++++++++
>  .../internal/shaders/bayer_unpacked.frag      | 62 ++++++++++++++++++-
>  2 files changed, 117 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag
> index 19b13ad08..90bd64570 100644
> --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag
> +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag
> @@ -65,6 +65,10 @@ uniform vec2 tex_step;
>  uniform vec2 tex_bayer_first_red;
>  
>  uniform sampler2D tex_y;
> +uniform sampler2D red_param;
> +uniform sampler2D green_param;
> +uniform sampler2D blue_param;
> +uniform mat3 ccm;
>  
>  void main(void)
>  {
> @@ -212,5 +216,57 @@ void main(void)
>  			vec3(patterns.y, C, patterns.x) :
>  			vec3(patterns.wz, C));
>  
> +#if defined(APPLY_CCM_PARAMETERS)

If the goal of sharing shaders between qcam and the soft ISP is to allow
testing of the shaders in qcam for development, this should be done
unconditionally. It's fine to set the parameters to hardcoded defaults
in qcam as a first step.

If controlling the processing parameters of the shader from qcam isn't
desired, then I don't see why we should share the shaders.

> +	/*
> +	 *   CCM is a 3x3 in the format
> +	 *
> +	 *   +--------------+----------------+---------------+
> +	 *   | RedRedGain   | RedGreenGain   | RedBlueGain   |
> +	 *   +--------------+----------------+---------------+
> +	 *   | GreenRedGain | GreenGreenGain | GreenBlueGain |
> +	 *   +--------------+----------------+---------------+
> +	 *   | BlueRedGain  |  BlueGreenGain | BlueBlueGain  |
> +	 *   +--------------+----------------+---------------+
> +	 *
> +	 *   Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin
> +	 *   Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin
> +	 *   Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin
> +	 *
> +	 *   We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm);
> +	 *
> +	 *   CPU
> +	 *   float ccm [] = {
> +	 *             RedRedGain,   RedGreenGain,   RedBlueGain,
> +	 *             GreenRedGain, GreenGreenGain, GreenBlueGain,
> +	 *             BlueRedGain,  BlueGreenGain,  BlueBlueGain,
> +	 *   };
> +	 *
> +	 *   GPU
> +	 *   ccm = {
> +	 *             RedRedGain,   GreenRedGain,   BlueRedGain,
> +	 *             RedGreenGain, GreenGreenGain, BlueGreenGain,
> +	 *             RedBlueGain,  GreenBlueGain,  BlueBlueGain,
> +	 *   }
> +	 *
> +	 *   However the indexing for the mat data-type is column major hence
> +	 *   ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain
> +	 *
> +	 */
> +	float rin, gin, bin;
> +	rin = rgb.r;
> +	gin = rgb.g;
> +	bin = rgb.b;
> +
> +	rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]);
> +	rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]);
> +	rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]);
> +
> +#elif defined(APPLY_RGB_PARAMETERS)
> +	/* Apply bayer params */
> +	rgb.r = texture2D(red_param, vec2(rgb.r, 0.5)).r;
> +	rgb.g = texture2D(green_param, vec2(rgb.g, 0.5)).g;
> +	rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
> +#endif
> +
>  	gl_FragColor = vec4(rgb, 1.0);
>  }
> diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag
> index aa7a1b004..7c3d12b03 100644
> --- a/include/libcamera/internal/shaders/bayer_unpacked.frag
> +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag
> @@ -21,11 +21,17 @@ precision highp float;
>  
>  /** Monochrome RGBA or GL_LUMINANCE Bayer encoded texture.*/
>  uniform sampler2D       tex_y;
> +uniform sampler2D	red_param;
> +uniform sampler2D	green_param;
> +uniform sampler2D	blue_param;
>  varying vec4            center;
>  varying vec4            yCoord;
>  varying vec4            xCoord;
> +uniform mat3		ccm;
>  
>  void main(void) {
> +    vec3 rgb;
> +
>      #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r
>  
>      float C = texture2D(tex_y, center.xy).r; // ( 0, 0)
> @@ -97,11 +103,65 @@ void main(void) {
>      PATTERN.xw  += kB.xw * B;
>      PATTERN.xz  += kF.xz * F;
>  
> -    gl_FragColor.rgb = (alternate.y == 0.0) ?
> +    rgb =  (alternate.y == 0.0) ?
>          ((alternate.x == 0.0) ?
>              vec3(C, PATTERN.xy) :
>              vec3(PATTERN.z, C, PATTERN.w)) :
>          ((alternate.x == 0.0) ?
>              vec3(PATTERN.w, C, PATTERN.z) :
>              vec3(PATTERN.yx, C));
> +
> +#if defined(APPLY_CCM_PARAMETERS)
> +	/*
> +	 *   CCM is a 3x3 in the format
> +	 *
> +	 *   +--------------+----------------+---------------+
> +	 *   | RedRedGain   | RedGreenGain   | RedBlueGain   |
> +	 *   +--------------+----------------+---------------+
> +	 *   | GreenRedGain | GreenGreenGain | GreenBlueGain |
> +	 *   +--------------+----------------+---------------+
> +	 *   | BlueRedGain  |  BlueGreenGain | BlueBlueGain  |
> +	 *   +--------------+----------------+---------------+
> +	 *
> +	 *   Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin
> +	 *   Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin
> +	 *   Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin
> +	 *
> +	 *   We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm);
> +	 *
> +	 *   CPU
> +	 *   float ccm [] = {
> +	 *             RedRedGain,   RedGreenGain,   RedBlueGain,
> +	 *             GreenRedGain, GreenGreenGain, GreenBlueGain,
> +	 *             BlueRedGain,  BlueGreenGain,  BlueBlueGain,
> +	 *   };
> +	 *
> +	 *   GPU
> +	 *   ccm = {
> +	 *             RedRedGain,   GreenRedGain,   BlueRedGain,
> +	 *             RedGreenGain, GreenGreenGain, BlueGreenGain,
> +	 *             RedBlueGain,  GreenBlueGain,  BlueBlueGain,
> +	 *   }
> +	 *
> +	 *   However the indexing for the mat data-type is column major hence
> +	 *   ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain
> +	 *
> +	 */
> +	float rin, gin, bin;
> +	rin = rgb.r;
> +	gin = rgb.g;
> +	bin = rgb.b;
> +
> +	rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]);
> +	rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]);
> +	rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]);
> +
> +#elif defined(APPLY_RGB_PARAMETERS)
> +	/* Apply bayer params */
> +	rgb.r = texture2D(red_param, vec2(rgb.r, 0.5)).r;
> +	rgb.g = texture2D(green_param, vec2(rgb.g, 0.5)).g;
> +	rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
> +#endif
> +
> +    gl_FragColor.rgb = rgb;
>  }
Bryan O'Donoghue Nov. 21, 2025, 8:43 a.m. UTC | #3
On 21/11/2025 03:14, Laurent Pinchart wrote:
> You're mixing tabs and spaces for indentation in the file.
> 

Ah damnit you're right.

I develop in packed and then copy/paste to unpacked.

---
bod
Bryan O'Donoghue Nov. 21, 2025, 11:37 a.m. UTC | #4
On 21/11/2025 03:18, Laurent Pinchart wrote:
>> +#if defined(APPLY_CCM_PARAMETERS)
> If the goal of sharing shaders between qcam and the soft ISP is to allow
> testing of the shaders in qcam for development, this should be done
> unconditionally. It's fine to set the parameters to hardcoded defaults
> in qcam as a first step.

Its application logic - so for example qcam can do this unconditionally.

There's also the ability to use the ISP RGB lookup tables as textures.

The idea I had here was.

1. No colour correction - which is the pre gpuisp baseline
2. RGB lookup
3. CCM

supporting all three modes - proves nothing breaks from #1, allows you 
to test #2 and leaves up to the application to make #3 the default.

?

---
bod
Laurent Pinchart Nov. 24, 2025, 12:03 a.m. UTC | #5
On Fri, Nov 21, 2025 at 11:37:44AM +0000, Bryan O'Donoghue wrote:
> On 21/11/2025 03:18, Laurent Pinchart wrote:
> >> +#if defined(APPLY_CCM_PARAMETERS)
> > If the goal of sharing shaders between qcam and the soft ISP is to allow
> > testing of the shaders in qcam for development, this should be done
> > unconditionally. It's fine to set the parameters to hardcoded defaults
> > in qcam as a first step.
> 
> Its application logic - so for example qcam can do this unconditionally.
> 
> There's also the ability to use the ISP RGB lookup tables as textures.
> 
> The idea I had here was.
> 
> 1. No colour correction - which is the pre gpuisp baseline
> 2. RGB lookup
> 3. CCM
> 
> supporting all three modes - proves nothing breaks from #1, allows you 
> to test #2 and leaves up to the application to make #3 the default.
> 
> ?

The shaders are internal to libcamera, I don't want to make them part of
the public API. qcam is cheating there. That's acceptable for now I
think, it's an internal application for development and testing purpose.
This means that qcam can be developed in sync with the GPU ISP.

We can decide to make qcam a test application for the GPU ISP, or keep
it as-is and use OpenGL only for format conversion. If we decide to go
for the latter, then I think we should duplicate the shaders. Otherwise
(which I think is my preferred option), I think we should avoid
conditional compilation that would result in different shaders between
the GPU ISP and qcam.

And now that I wrote this, I realize that the series makes use of the
conditional compilation in the GPU ISP as well, to support both the
lookup table (2) and CCM (3) configurations. What do you envision as use
cases for supporting lookup table in the GPU ISP ?
Bryan O'Donoghue Nov. 26, 2025, 1:46 p.m. UTC | #6
On 24/11/2025 00:03, Laurent Pinchart wrote:
> And now that I wrote this, I realize that the series makes use of the
> conditional compilation in the GPU ISP as well, to support both the
> lookup table (2) and CCM (3) configurations. What do you envision as use
> cases for supporting lookup table in the GPU ISP ?

We discussed @ softisp meeting keeping the lookup tables as example code.

At least that's what I thought you wanted. Probably useful for people 
adding the LSC code.

Perhaps they can also remove the lookup tables in the same series.

---
bod
Bryan O'Donoghue Nov. 26, 2025, 10:19 p.m. UTC | #7
On 26/11/2025 13:46, Bryan O'Donoghue wrote:
> On 24/11/2025 00:03, Laurent Pinchart wrote:
>> And now that I wrote this, I realize that the series makes use of the
>> conditional compilation in the GPU ISP as well, to support both the
>> lookup table (2) and CCM (3) configurations. What do you envision as use
>> cases for supporting lookup table in the GPU ISP ?
> 
> We discussed @ softisp meeting keeping the lookup tables as example code.
> 
> At least that's what I thought you wanted. Probably useful for people 
> adding the LSC code.
> 
> Perhaps they can also remove the lookup tables in the same series.
> 
> ---
> bod

The main reason to have conditional compilation here is when running the 
shaders in qcam without softisp we can verify the core demosaic stuff works.

Then when switching to softisp - we run the shaders with all of the good 
stuff switched on CCM, etc.

Re: the tables, let me know if you want them dropped. As I say I thought 
we had discussed keeping them as reference code for some amount of time.

For me anyway, I like to be able to run the shaders in their original 
mode - no ccm, contrast etc, so that I can verify the basic demosaic 
works when making changes.

Maybe at this point we don't need to be so cautious but, it was very 
useful for me this far.

---
bod
Laurent Pinchart Nov. 27, 2025, 8 a.m. UTC | #8
Hi Bryan,

On Wed, Nov 26, 2025 at 10:19:58PM +0000, Bryan O'Donoghue wrote:
> On 26/11/2025 13:46, Bryan O'Donoghue wrote:
> > On 24/11/2025 00:03, Laurent Pinchart wrote:
> >> And now that I wrote this, I realize that the series makes use of the
> >> conditional compilation in the GPU ISP as well, to support both the
> >> lookup table (2) and CCM (3) configurations. What do you envision as use
> >> cases for supporting lookup table in the GPU ISP ?
> > 
> > We discussed @ softisp meeting keeping the lookup tables as example code.
> > 
> > At least that's what I thought you wanted. Probably useful for people 
> > adding the LSC code.
> > 
> > Perhaps they can also remove the lookup tables in the same series.
> 
> The main reason to have conditional compilation here is when running the 
> shaders in qcam without softisp we can verify the core demosaic stuff works.
> 
> Then when switching to softisp - we run the shaders with all of the good 
> stuff switched on CCM, etc.
> 
> Re: the tables, let me know if you want them dropped. As I say I thought 
> we had discussed keeping them as reference code for some amount of time.

If you think it's useful we can keep them for the time being, but I'd
like to simplify the implementation at some point.

> For me anyway, I like to be able to run the shaders in their original 
> mode - no ccm, contrast etc, so that I can verify the basic demosaic 
> works when making changes.
> 
> Maybe at this point we don't need to be so cautious but, it was very 
> useful for me this far.

Can't we achieve the same by setting all parameters to identify values ?
Bryan O'Donoghue Nov. 27, 2025, 11:55 a.m. UTC | #9
On 27/11/2025 08:00, Laurent Pinchart wrote:
>> Re: the tables, let me know if you want them dropped. As I say I thought
>> we had discussed keeping them as reference code for some amount of time.
> If you think it's useful we can keep them for the time being, but I'd
> like to simplify the implementation at some point.
> 
>> For me anyway, I like to be able to run the shaders in their original
>> mode - no ccm, contrast etc, so that I can verify the basic demosaic
>> works when making changes.
>>
>> Maybe at this point we don't need to be so cautious but, it was very
>> useful for me this far.
> Can't we achieve the same by setting all parameters to identify values ?

In theory I suppose so...

It should "only" be a matter of

- An identity matrix for the CCM
- Setting all other parameters to 1.0f

Something x 1 == Something

I'm happy enough to drop the lookup tables, after all we have several 
branches with the reference code so if we leave those branches in a git 
repo, the information hasn't been lost.

---
bod

Patch
diff mbox series

diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag
index 19b13ad08..90bd64570 100644
--- a/include/libcamera/internal/shaders/bayer_1x_packed.frag
+++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag
@@ -65,6 +65,10 @@  uniform vec2 tex_step;
 uniform vec2 tex_bayer_first_red;
 
 uniform sampler2D tex_y;
+uniform sampler2D red_param;
+uniform sampler2D green_param;
+uniform sampler2D blue_param;
+uniform mat3 ccm;
 
 void main(void)
 {
@@ -212,5 +216,57 @@  void main(void)
 			vec3(patterns.y, C, patterns.x) :
 			vec3(patterns.wz, C));
 
+#if defined(APPLY_CCM_PARAMETERS)
+	/*
+	 *   CCM is a 3x3 in the format
+	 *
+	 *   +--------------+----------------+---------------+
+	 *   | RedRedGain   | RedGreenGain   | RedBlueGain   |
+	 *   +--------------+----------------+---------------+
+	 *   | GreenRedGain | GreenGreenGain | GreenBlueGain |
+	 *   +--------------+----------------+---------------+
+	 *   | BlueRedGain  |  BlueGreenGain | BlueBlueGain  |
+	 *   +--------------+----------------+---------------+
+	 *
+	 *   Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin
+	 *   Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin
+	 *   Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin
+	 *
+	 *   We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm);
+	 *
+	 *   CPU
+	 *   float ccm [] = {
+	 *             RedRedGain,   RedGreenGain,   RedBlueGain,
+	 *             GreenRedGain, GreenGreenGain, GreenBlueGain,
+	 *             BlueRedGain,  BlueGreenGain,  BlueBlueGain,
+	 *   };
+	 *
+	 *   GPU
+	 *   ccm = {
+	 *             RedRedGain,   GreenRedGain,   BlueRedGain,
+	 *             RedGreenGain, GreenGreenGain, BlueGreenGain,
+	 *             RedBlueGain,  GreenBlueGain,  BlueBlueGain,
+	 *   }
+	 *
+	 *   However the indexing for the mat data-type is column major hence
+	 *   ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain
+	 *
+	 */
+	float rin, gin, bin;
+	rin = rgb.r;
+	gin = rgb.g;
+	bin = rgb.b;
+
+	rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]);
+	rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]);
+	rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]);
+
+#elif defined(APPLY_RGB_PARAMETERS)
+	/* Apply bayer params */
+	rgb.r = texture2D(red_param, vec2(rgb.r, 0.5)).r;
+	rgb.g = texture2D(green_param, vec2(rgb.g, 0.5)).g;
+	rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
+#endif
+
 	gl_FragColor = vec4(rgb, 1.0);
 }
diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag
index aa7a1b004..7c3d12b03 100644
--- a/include/libcamera/internal/shaders/bayer_unpacked.frag
+++ b/include/libcamera/internal/shaders/bayer_unpacked.frag
@@ -21,11 +21,17 @@  precision highp float;
 
 /** Monochrome RGBA or GL_LUMINANCE Bayer encoded texture.*/
 uniform sampler2D       tex_y;
+uniform sampler2D	red_param;
+uniform sampler2D	green_param;
+uniform sampler2D	blue_param;
 varying vec4            center;
 varying vec4            yCoord;
 varying vec4            xCoord;
+uniform mat3		ccm;
 
 void main(void) {
+    vec3 rgb;
+
     #define fetch(x, y) texture2D(tex_y, vec2(x, y)).r
 
     float C = texture2D(tex_y, center.xy).r; // ( 0, 0)
@@ -97,11 +103,65 @@  void main(void) {
     PATTERN.xw  += kB.xw * B;
     PATTERN.xz  += kF.xz * F;
 
-    gl_FragColor.rgb = (alternate.y == 0.0) ?
+    rgb =  (alternate.y == 0.0) ?
         ((alternate.x == 0.0) ?
             vec3(C, PATTERN.xy) :
             vec3(PATTERN.z, C, PATTERN.w)) :
         ((alternate.x == 0.0) ?
             vec3(PATTERN.w, C, PATTERN.z) :
             vec3(PATTERN.yx, C));
+
+#if defined(APPLY_CCM_PARAMETERS)
+	/*
+	 *   CCM is a 3x3 in the format
+	 *
+	 *   +--------------+----------------+---------------+
+	 *   | RedRedGain   | RedGreenGain   | RedBlueGain   |
+	 *   +--------------+----------------+---------------+
+	 *   | GreenRedGain | GreenGreenGain | GreenBlueGain |
+	 *   +--------------+----------------+---------------+
+	 *   | BlueRedGain  |  BlueGreenGain | BlueBlueGain  |
+	 *   +--------------+----------------+---------------+
+	 *
+	 *   Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin
+	 *   Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin
+	 *   Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin
+	 *
+	 *   We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm);
+	 *
+	 *   CPU
+	 *   float ccm [] = {
+	 *             RedRedGain,   RedGreenGain,   RedBlueGain,
+	 *             GreenRedGain, GreenGreenGain, GreenBlueGain,
+	 *             BlueRedGain,  BlueGreenGain,  BlueBlueGain,
+	 *   };
+	 *
+	 *   GPU
+	 *   ccm = {
+	 *             RedRedGain,   GreenRedGain,   BlueRedGain,
+	 *             RedGreenGain, GreenGreenGain, BlueGreenGain,
+	 *             RedBlueGain,  GreenBlueGain,  BlueBlueGain,
+	 *   }
+	 *
+	 *   However the indexing for the mat data-type is column major hence
+	 *   ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain
+	 *
+	 */
+	float rin, gin, bin;
+	rin = rgb.r;
+	gin = rgb.g;
+	bin = rgb.b;
+
+	rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]);
+	rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]);
+	rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]);
+
+#elif defined(APPLY_RGB_PARAMETERS)
+	/* Apply bayer params */
+	rgb.r = texture2D(red_param, vec2(rgb.r, 0.5)).r;
+	rgb.g = texture2D(green_param, vec2(rgb.g, 0.5)).g;
+	rgb.b = texture2D(blue_param, vec2(rgb.b, 0.5)).b;
+#endif
+
+    gl_FragColor.rgb = rgb;
 }