| Message ID | 20251015012251.17508-33-bryan.odonoghue@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Bryan, Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > Add the ability to apply a vec3 AWB array against the demosaiced data prior > to application of colour correction data. I may be missing something but I think white balance is already applied in lut.cpp to both the CCM and the R/G/B lookup tables. > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > include/libcamera/internal/shaders/bayer_1x_packed.frag | 6 ++++++ > include/libcamera/internal/shaders/bayer_unpacked.frag | 6 ++++++ > 2 files changed, 12 insertions(+) > > diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag > index c0632eb1..8b35dd63 100644 > --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag > +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag > @@ -69,6 +69,7 @@ uniform sampler2D red_param; > uniform sampler2D green_param; > uniform sampler2D blue_param; > uniform mat3 ccm; > +uniform vec3 awb; > > void main(void) > { > @@ -216,6 +217,11 @@ void main(void) > vec3(patterns.y, C, patterns.x) : > vec3(patterns.wz, C)); > > + /* Apply white balance before colour correction matrix */ > + rgb.r = rgb.r * awb.r; > + rgb.g = rgb.g * awb.g; > + rgb.b = rgb.b * awb.b; > + > #if defined(APPLY_CCM_PARAMETERS) > /* > * CCM is a 3x3 in the format > diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag > index 74ce1509..37bd4812 100644 > --- a/include/libcamera/internal/shaders/bayer_unpacked.frag > +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag > @@ -28,6 +28,7 @@ varying vec4 center; > varying vec4 yCoord; > varying vec4 xCoord; > uniform mat3 ccm; > +uniform vec3 awb; > > void main(void) { > vec3 rgb; > @@ -111,6 +112,11 @@ void main(void) { > vec3(PATTERN.w, C, PATTERN.z) : > vec3(PATTERN.yx, C)); > > + /* Apply white balance before colour correction matrix */ > + rgb.r = rgb.r * awb.r; > + rgb.g = rgb.g * awb.g; > + rgb.b = rgb.b * awb.b; > + > #if defined(APPLY_CCM_PARAMETERS) > /* > * CCM is a 3x3 in the format
Milan Zamazal <mzamazal@redhat.com> writes: > Hi Bryan, > > Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > >> Add the ability to apply a vec3 AWB array against the demosaiced data prior >> to application of colour correction data. > > I may be missing something but I think white balance is already applied > in lut.cpp to both the CCM and the R/G/B lookup tables. I must be something missing, I get a colour cast without this, at least when CCM is not used. >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> --- >> include/libcamera/internal/shaders/bayer_1x_packed.frag | 6 ++++++ >> include/libcamera/internal/shaders/bayer_unpacked.frag | 6 ++++++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag >> index c0632eb1..8b35dd63 100644 >> --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag >> +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag >> @@ -69,6 +69,7 @@ uniform sampler2D red_param; >> uniform sampler2D green_param; >> uniform sampler2D blue_param; >> uniform mat3 ccm; >> +uniform vec3 awb; >> >> void main(void) >> { >> @@ -216,6 +217,11 @@ void main(void) >> vec3(patterns.y, C, patterns.x) : >> vec3(patterns.wz, C)); >> >> + /* Apply white balance before colour correction matrix */ >> + rgb.r = rgb.r * awb.r; >> + rgb.g = rgb.g * awb.g; >> + rgb.b = rgb.b * awb.b; >> + >> #if defined(APPLY_CCM_PARAMETERS) >> /* >> * CCM is a 3x3 in the format >> diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag >> index 74ce1509..37bd4812 100644 >> --- a/include/libcamera/internal/shaders/bayer_unpacked.frag >> +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag >> @@ -28,6 +28,7 @@ varying vec4 center; >> varying vec4 yCoord; >> varying vec4 xCoord; >> uniform mat3 ccm; >> +uniform vec3 awb; >> >> void main(void) { >> vec3 rgb; >> @@ -111,6 +112,11 @@ void main(void) { >> vec3(PATTERN.w, C, PATTERN.z) : >> vec3(PATTERN.yx, C)); >> >> + /* Apply white balance before colour correction matrix */ >> + rgb.r = rgb.r * awb.r; >> + rgb.g = rgb.g * awb.g; >> + rgb.b = rgb.b * awb.b; >> + >> #if defined(APPLY_CCM_PARAMETERS) >> /* >> * CCM is a 3x3 in the format
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > Add the ability to apply a vec3 AWB array against the demosaiced data prior > to application of colour correction data. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > include/libcamera/internal/shaders/bayer_1x_packed.frag | 6 ++++++ > include/libcamera/internal/shaders/bayer_unpacked.frag | 6 ++++++ > 2 files changed, 12 insertions(+) > > diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag > index c0632eb1..8b35dd63 100644 > --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag > +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag > @@ -69,6 +69,7 @@ uniform sampler2D red_param; > uniform sampler2D green_param; > uniform sampler2D blue_param; > uniform mat3 ccm; > +uniform vec3 awb; > > void main(void) > { > @@ -216,6 +217,11 @@ void main(void) > vec3(patterns.y, C, patterns.x) : > vec3(patterns.wz, C)); > > + /* Apply white balance before colour correction matrix */ > + rgb.r = rgb.r * awb.r; > + rgb.g = rgb.g * awb.g; > + rgb.b = rgb.b * awb.b; This block is misaligned, should be indented by 4 spaces rather than a tab. And I think it can be simplified as: rgb = rgb * awb; And it should be applied only if APPLY_RGB_PARAMETERS. For CCM, it's applied to the CCM in lut.cpp: Matrix<float, 3, 3> gainCcm = { { gains.r(), 0, 0, 0, gains.g(), 0, 0, 0, gains.b() } }; auto ccm = context.activeState.ccm.ccm * gainCcm; The same for the other shader. > + > #if defined(APPLY_CCM_PARAMETERS) > /* > * CCM is a 3x3 in the format > diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag > index 74ce1509..37bd4812 100644 > --- a/include/libcamera/internal/shaders/bayer_unpacked.frag > +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag > @@ -28,6 +28,7 @@ varying vec4 center; > varying vec4 yCoord; > varying vec4 xCoord; > uniform mat3 ccm; > +uniform vec3 awb; > > void main(void) { > vec3 rgb; > @@ -111,6 +112,11 @@ void main(void) { > vec3(PATTERN.w, C, PATTERN.z) : > vec3(PATTERN.yx, C)); > > + /* Apply white balance before colour correction matrix */ > + rgb.r = rgb.r * awb.r; > + rgb.g = rgb.g * awb.g; > + rgb.b = rgb.b * awb.b; > + > #if defined(APPLY_CCM_PARAMETERS) > /* > * CCM is a 3x3 in the format
Milan Zamazal <mzamazal@redhat.com> writes: > Milan Zamazal <mzamazal@redhat.com> writes: > >> Hi Bryan, >> >> Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: >> >>> Add the ability to apply a vec3 AWB array against the demosaiced data prior >>> to application of colour correction data. >> >> I may be missing something but I think white balance is already applied >> in lut.cpp to both the CCM and the R/G/B lookup tables. > > I must be something missing, I get a colour cast without this, at least > when CCM is not used. The problem was that the shader looks up all the RGB channels in red_param by mistake, as reported on the corresponding patch. AWB shouldn't be handled in the shaders at all, it's already incorporated in the lookup tables or CCM. With the fixes I reported yesterday and today, I get the same output with and without CCM. \o/ >>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >>> --- >>> include/libcamera/internal/shaders/bayer_1x_packed.frag | 6 ++++++ >>> include/libcamera/internal/shaders/bayer_unpacked.frag | 6 ++++++ >>> 2 files changed, 12 insertions(+) >>> >>> diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag >>> index c0632eb1..8b35dd63 100644 >>> --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag >>> +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag >>> @@ -69,6 +69,7 @@ uniform sampler2D red_param; >>> uniform sampler2D green_param; >>> uniform sampler2D blue_param; >>> uniform mat3 ccm; >>> +uniform vec3 awb; >>> >>> void main(void) >>> { >>> @@ -216,6 +217,11 @@ void main(void) >>> vec3(patterns.y, C, patterns.x) : >>> vec3(patterns.wz, C)); >>> >>> + /* Apply white balance before colour correction matrix */ >>> + rgb.r = rgb.r * awb.r; >>> + rgb.g = rgb.g * awb.g; >>> + rgb.b = rgb.b * awb.b; >>> + >>> #if defined(APPLY_CCM_PARAMETERS) >>> /* >>> * CCM is a 3x3 in the format >>> diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag >>> index 74ce1509..37bd4812 100644 >>> --- a/include/libcamera/internal/shaders/bayer_unpacked.frag >>> +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag >>> @@ -28,6 +28,7 @@ varying vec4 center; >>> varying vec4 yCoord; >>> varying vec4 xCoord; >>> uniform mat3 ccm; >>> +uniform vec3 awb; >>> >>> void main(void) { >>> vec3 rgb; >>> @@ -111,6 +112,11 @@ void main(void) { >>> vec3(PATTERN.w, C, PATTERN.z) : >>> vec3(PATTERN.yx, C)); >>> >>> + /* Apply white balance before colour correction matrix */ >>> + rgb.r = rgb.r * awb.r; >>> + rgb.g = rgb.g * awb.g; >>> + rgb.b = rgb.b * awb.b; >>> + >>> #if defined(APPLY_CCM_PARAMETERS) >>> /* >>> * CCM is a 3x3 in the format
Milan Zamazal <mzamazal@redhat.com> writes: > Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > >> Add the ability to apply a vec3 AWB array against the demosaiced data prior >> to application of colour correction data. >> >> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >> --- >> include/libcamera/internal/shaders/bayer_1x_packed.frag | 6 ++++++ >> include/libcamera/internal/shaders/bayer_unpacked.frag | 6 ++++++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag >> index c0632eb1..8b35dd63 100644 >> --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag >> +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag >> @@ -69,6 +69,7 @@ uniform sampler2D red_param; >> uniform sampler2D green_param; >> uniform sampler2D blue_param; >> uniform mat3 ccm; >> +uniform vec3 awb; >> >> void main(void) >> { >> @@ -216,6 +217,11 @@ void main(void) >> vec3(patterns.y, C, patterns.x) : >> vec3(patterns.wz, C)); >> >> + /* Apply white balance before colour correction matrix */ >> + rgb.r = rgb.r * awb.r; >> + rgb.g = rgb.g * awb.g; >> + rgb.b = rgb.b * awb.b; > > This block is misaligned, should be indented by 4 spaces rather than a > tab. And I think it can be simplified as: > > rgb = rgb * awb; > > And it should be applied only if APPLY_RGB_PARAMETERS. It shouldn't be applied anywhere. AWB is already part of the lookup tables created in lut.cpp: auto &gains = context.activeState.awb.gains; ... if (!context.ccmEnabled) { for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) { /* Apply gamma after gain! */ const RGB<float> lutGains = (gains * i / div).min(gammaTableSize - 1); params->red[i] = gammaTable[static_cast<unsigned int>(lutGains.r())]; params->green[i] = gammaTable[static_cast<unsigned int>(lutGains.g())]; params->blue[i] = gammaTable[static_cast<unsigned int>(lutGains.b())]; An unrelated bug in the shader confused me when testing this. Sorry for the incremental reviews. I think I'm done with v3 review and testing now. > For CCM, it's applied to the CCM in lut.cpp: > > Matrix<float, 3, 3> gainCcm = { { gains.r(), 0, 0, > 0, gains.g(), 0, > 0, 0, gains.b() } }; > auto ccm = context.activeState.ccm.ccm * gainCcm; > > The same for the other shader. > >> + >> #if defined(APPLY_CCM_PARAMETERS) >> /* >> * CCM is a 3x3 in the format >> diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag >> index 74ce1509..37bd4812 100644 >> --- a/include/libcamera/internal/shaders/bayer_unpacked.frag >> +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag >> @@ -28,6 +28,7 @@ varying vec4 center; >> varying vec4 yCoord; >> varying vec4 xCoord; >> uniform mat3 ccm; >> +uniform vec3 awb; >> >> void main(void) { >> vec3 rgb; >> @@ -111,6 +112,11 @@ void main(void) { >> vec3(PATTERN.w, C, PATTERN.z) : >> vec3(PATTERN.yx, C)); >> >> + /* Apply white balance before colour correction matrix */ >> + rgb.r = rgb.r * awb.r; >> + rgb.g = rgb.g * awb.g; >> + rgb.b = rgb.b * awb.b; >> + >> #if defined(APPLY_CCM_PARAMETERS) >> /* >> * CCM is a 3x3 in the format
diff --git a/include/libcamera/internal/shaders/bayer_1x_packed.frag b/include/libcamera/internal/shaders/bayer_1x_packed.frag index c0632eb1..8b35dd63 100644 --- a/include/libcamera/internal/shaders/bayer_1x_packed.frag +++ b/include/libcamera/internal/shaders/bayer_1x_packed.frag @@ -69,6 +69,7 @@ uniform sampler2D red_param; uniform sampler2D green_param; uniform sampler2D blue_param; uniform mat3 ccm; +uniform vec3 awb; void main(void) { @@ -216,6 +217,11 @@ void main(void) vec3(patterns.y, C, patterns.x) : vec3(patterns.wz, C)); + /* Apply white balance before colour correction matrix */ + rgb.r = rgb.r * awb.r; + rgb.g = rgb.g * awb.g; + rgb.b = rgb.b * awb.b; + #if defined(APPLY_CCM_PARAMETERS) /* * CCM is a 3x3 in the format diff --git a/include/libcamera/internal/shaders/bayer_unpacked.frag b/include/libcamera/internal/shaders/bayer_unpacked.frag index 74ce1509..37bd4812 100644 --- a/include/libcamera/internal/shaders/bayer_unpacked.frag +++ b/include/libcamera/internal/shaders/bayer_unpacked.frag @@ -28,6 +28,7 @@ varying vec4 center; varying vec4 yCoord; varying vec4 xCoord; uniform mat3 ccm; +uniform vec3 awb; void main(void) { vec3 rgb; @@ -111,6 +112,11 @@ void main(void) { vec3(PATTERN.w, C, PATTERN.z) : vec3(PATTERN.yx, C)); + /* Apply white balance before colour correction matrix */ + rgb.r = rgb.r * awb.r; + rgb.g = rgb.g * awb.g; + rgb.b = rgb.b * awb.b; + #if defined(APPLY_CCM_PARAMETERS) /* * CCM is a 3x3 in the format
Add the ability to apply a vec3 AWB array against the demosaiced data prior to application of colour correction data. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- include/libcamera/internal/shaders/bayer_1x_packed.frag | 6 ++++++ include/libcamera/internal/shaders/bayer_unpacked.frag | 6 ++++++ 2 files changed, 12 insertions(+)