| Message ID | 20260211131728.96413-3-mzamazal@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On 11/02/2026 13:17, Milan Zamazal wrote: > In GPU ISP fragment shaders, the black level is simply subtracted from > the pixel value. This means the highest pixel values can never be > reached, possibly resulting in wrong brightness or colour shifts. Fix > this by spreading the resulting value to the whole 0.0..1.0 range. > > The preceding simple pipeline IPA patch ensures `blacklevel' is less > than 1.0, preventing division by zero here. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/shaders/bayer_1x_packed.frag | 2 +- > src/libcamera/shaders/bayer_unpacked.frag | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag > index 23747f78a..06ddc040b 100644 > --- a/src/libcamera/shaders/bayer_1x_packed.frag > +++ b/src/libcamera/shaders/bayer_1x_packed.frag > @@ -225,7 +225,7 @@ void main(void) > vec3(patterns.y, C, patterns.x) : > vec3(patterns.wz, C)); > > - rgb = rgb - blacklevel; > + rgb = (rgb - blacklevel) / (1.0 - blacklevel); > > /* > * CCM is a 3x3 in the format > diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag > index 1b85196ae..98dea512c 100644 > --- a/src/libcamera/shaders/bayer_unpacked.frag > +++ b/src/libcamera/shaders/bayer_unpacked.frag > @@ -128,7 +128,7 @@ void main(void) { > vec3(PATTERN.w, C, PATTERN.z) : > vec3(PATTERN.yx, C)); > > - rgb = rgb - blacklevel; > + rgb = (rgb - blacklevel) / (1.0 - blacklevel); > > /* > * CCM is a 3x3 in the format Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
2026. 02. 11. 14:17 keltezéssel, Milan Zamazal írta: > In GPU ISP fragment shaders, the black level is simply subtracted from > the pixel value. This means the highest pixel values can never be > reached, possibly resulting in wrong brightness or colour shifts. Fix > this by spreading the resulting value to the whole 0.0..1.0 range. > > The preceding simple pipeline IPA patch ensures `blacklevel' is less > than 1.0, preventing division by zero here. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/shaders/bayer_1x_packed.frag | 2 +- > src/libcamera/shaders/bayer_unpacked.frag | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag > index 23747f78a..06ddc040b 100644 > --- a/src/libcamera/shaders/bayer_1x_packed.frag > +++ b/src/libcamera/shaders/bayer_1x_packed.frag > @@ -225,7 +225,7 @@ void main(void) > vec3(patterns.y, C, patterns.x) : > vec3(patterns.wz, C)); > > - rgb = rgb - blacklevel; > + rgb = (rgb - blacklevel) / (1.0 - blacklevel); This may be a dumb question, but how come this is not written as: rgb = clamp(rgb, blackLevel, 1.0) / (1.0 - blackLevel) Can't negative values cause issues in later calculations? > > /* > * CCM is a 3x3 in the format > diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag > index 1b85196ae..98dea512c 100644 > --- a/src/libcamera/shaders/bayer_unpacked.frag > +++ b/src/libcamera/shaders/bayer_unpacked.frag > @@ -128,7 +128,7 @@ void main(void) { > vec3(PATTERN.w, C, PATTERN.z) : > vec3(PATTERN.yx, C)); > > - rgb = rgb - blacklevel; > + rgb = (rgb - blacklevel) / (1.0 - blacklevel); > > /* > * CCM is a 3x3 in the format
Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > 2026. 02. 11. 14:17 keltezéssel, Milan Zamazal írta: >> In GPU ISP fragment shaders, the black level is simply subtracted from >> the pixel value. This means the highest pixel values can never be >> reached, possibly resulting in wrong brightness or colour shifts. Fix >> this by spreading the resulting value to the whole 0.0..1.0 range. >> The preceding simple pipeline IPA patch ensures `blacklevel' is less >> than 1.0, preventing division by zero here. >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- >> src/libcamera/shaders/bayer_1x_packed.frag | 2 +- >> src/libcamera/shaders/bayer_unpacked.frag | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag >> index 23747f78a..06ddc040b 100644 >> --- a/src/libcamera/shaders/bayer_1x_packed.frag >> +++ b/src/libcamera/shaders/bayer_1x_packed.frag >> @@ -225,7 +225,7 @@ void main(void) >> vec3(patterns.y, C, patterns.x) : >> vec3(patterns.wz, C)); >> - rgb = rgb - blacklevel; >> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); > > This may be a dumb question, but how come this is not written as: > > rgb = clamp(rgb, blackLevel, 1.0) / (1.0 - blackLevel) > > Can't negative values cause issues in later calculations? Currently not, because CCM may contain negative values and produce negative pixel values anyway. The resulting value is clamped later. So in case rgb < blackLevel (not expected, but still possible), the most likely result (depending on the CCM) is that it ends up being 0 eventually. That said, I think yes, clamping the value here is better, to prevent possible future trouble once more functionality is added to the shaders. >> /* >> * CCM is a 3x3 in the format >> diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag >> index 1b85196ae..98dea512c 100644 >> --- a/src/libcamera/shaders/bayer_unpacked.frag >> +++ b/src/libcamera/shaders/bayer_unpacked.frag >> @@ -128,7 +128,7 @@ void main(void) { >> vec3(PATTERN.w, C, PATTERN.z) : >> vec3(PATTERN.yx, C)); >> - rgb = rgb - blacklevel; >> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >> /* >> * CCM is a 3x3 in the format
Milan Zamazal <mzamazal@redhat.com> writes: > Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > >> 2026. 02. 11. 14:17 keltezéssel, Milan Zamazal írta: >>> In GPU ISP fragment shaders, the black level is simply subtracted from >>> the pixel value. This means the highest pixel values can never be >>> reached, possibly resulting in wrong brightness or colour shifts. Fix >>> this by spreading the resulting value to the whole 0.0..1.0 range. >>> The preceding simple pipeline IPA patch ensures `blacklevel' is less >>> than 1.0, preventing division by zero here. >>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >>> --- >>> src/libcamera/shaders/bayer_1x_packed.frag | 2 +- >>> src/libcamera/shaders/bayer_unpacked.frag | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag >>> index 23747f78a..06ddc040b 100644 >>> --- a/src/libcamera/shaders/bayer_1x_packed.frag >>> +++ b/src/libcamera/shaders/bayer_1x_packed.frag >>> @@ -225,7 +225,7 @@ void main(void) >>> vec3(patterns.y, C, patterns.x) : >>> vec3(patterns.wz, C)); >>> - rgb = rgb - blacklevel; >>> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >> >> This may be a dumb question, but how come this is not written as: >> >> rgb = clamp(rgb, blackLevel, 1.0) / (1.0 - blackLevel) BTW did you actually mean rgb = (clamp(rgb, blacklevel, 1.0) - blacklevel) / (1.0 - blacklevel) or rgb = clamp((rgb - blacklevel) / (1.0 - blacklevel), 0.0, 1.0) ? (I don't think there is a simpler way to express it using common GLSL functions.) >> Can't negative values cause issues in later calculations? > > Currently not, because CCM may contain negative values and produce > negative pixel values anyway. The resulting value is clamped later. So > in case rgb < blackLevel (not expected, but still possible), the most > likely result (depending on the CCM) is that it ends up being 0 > eventually. > > That said, I think yes, clamping the value here is better, to prevent > possible future trouble once more functionality is added to the shaders. > >>> /* >>> * CCM is a 3x3 in the format >>> diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag >>> index 1b85196ae..98dea512c 100644 >>> --- a/src/libcamera/shaders/bayer_unpacked.frag >>> +++ b/src/libcamera/shaders/bayer_unpacked.frag >>> @@ -128,7 +128,7 @@ void main(void) { >>> vec3(PATTERN.w, C, PATTERN.z) : >>> vec3(PATTERN.yx, C)); >>> - rgb = rgb - blacklevel; >>> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >>> /* >>> * CCM is a 3x3 in the format
On 11/02/2026 15:47, Milan Zamazal wrote:
> rgb = clamp((rgb - blacklevel) / (1.0 - blacklevel), 0.0, 1.0)
This one, as you can never be outside the 0.0-1.0 window then.
---
bod
2026. 02. 11. 16:47 keltezéssel, Milan Zamazal írta: > Milan Zamazal <mzamazal@redhat.com> writes: > >> Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: >> >>> 2026. 02. 11. 14:17 keltezéssel, Milan Zamazal írta: >>>> In GPU ISP fragment shaders, the black level is simply subtracted from >>>> the pixel value. This means the highest pixel values can never be >>>> reached, possibly resulting in wrong brightness or colour shifts. Fix >>>> this by spreading the resulting value to the whole 0.0..1.0 range. >>>> The preceding simple pipeline IPA patch ensures `blacklevel' is less >>>> than 1.0, preventing division by zero here. >>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >>>> --- >>>> src/libcamera/shaders/bayer_1x_packed.frag | 2 +- >>>> src/libcamera/shaders/bayer_unpacked.frag | 2 +- >>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>> diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag >>>> index 23747f78a..06ddc040b 100644 >>>> --- a/src/libcamera/shaders/bayer_1x_packed.frag >>>> +++ b/src/libcamera/shaders/bayer_1x_packed.frag >>>> @@ -225,7 +225,7 @@ void main(void) >>>> vec3(patterns.y, C, patterns.x) : >>>> vec3(patterns.wz, C)); >>>> - rgb = rgb - blacklevel; >>>> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >>> >>> This may be a dumb question, but how come this is not written as: >>> >>> rgb = clamp(rgb, blackLevel, 1.0) / (1.0 - blackLevel) > > BTW did you actually mean > > rgb = (clamp(rgb, blacklevel, 1.0) - blacklevel) / (1.0 - blacklevel) > > or > > rgb = clamp((rgb - blacklevel) / (1.0 - blacklevel), 0.0, 1.0) > > ? > > (I don't think there is a simpler way to express it using common GLSL > functions.) Oops, yes, I have forgotten a subtraction. Assuming 0 <= blacklevel < 1, the two seem to be mathematically equivalent. So I think either is fine, but clamping at the end makes the intent clearer in my opinion. > >>> Can't negative values cause issues in later calculations? >> >> Currently not, because CCM may contain negative values and produce >> negative pixel values anyway. The resulting value is clamped later. So >> in case rgb < blackLevel (not expected, but still possible), the most >> likely result (depending on the CCM) is that it ends up being 0 >> eventually. >> >> That said, I think yes, clamping the value here is better, to prevent >> possible future trouble once more functionality is added to the shaders. >> >>>> /* >>>> * CCM is a 3x3 in the format >>>> diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag >>>> index 1b85196ae..98dea512c 100644 >>>> --- a/src/libcamera/shaders/bayer_unpacked.frag >>>> +++ b/src/libcamera/shaders/bayer_unpacked.frag >>>> @@ -128,7 +128,7 @@ void main(void) { >>>> vec3(PATTERN.w, C, PATTERN.z) : >>>> vec3(PATTERN.yx, C)); >>>> - rgb = rgb - blacklevel; >>>> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >>>> /* >>>> * CCM is a 3x3 in the format >
Hi Milan, Thank you for the patch. On Wed, Feb 11, 2026 at 02:17:28PM +0100, Milan Zamazal wrote: > In GPU ISP fragment shaders, the black level is simply subtracted from > the pixel value. This means the highest pixel values can never be > reached, possibly resulting in wrong brightness or colour shifts. Fix > this by spreading the resulting value to the whole 0.0..1.0 range. It's expected though. Black level substraction is not the place where you should perform histogram spreading, this is the job of tone mapping, further down the pipeline. > The preceding simple pipeline IPA patch ensures `blacklevel' is less > than 1.0, preventing division by zero here. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/shaders/bayer_1x_packed.frag | 2 +- > src/libcamera/shaders/bayer_unpacked.frag | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag > index 23747f78a..06ddc040b 100644 > --- a/src/libcamera/shaders/bayer_1x_packed.frag > +++ b/src/libcamera/shaders/bayer_1x_packed.frag > @@ -225,7 +225,7 @@ void main(void) > vec3(patterns.y, C, patterns.x) : > vec3(patterns.wz, C)); > > - rgb = rgb - blacklevel; > + rgb = (rgb - blacklevel) / (1.0 - blacklevel); > > /* > * CCM is a 3x3 in the format > diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag > index 1b85196ae..98dea512c 100644 > --- a/src/libcamera/shaders/bayer_unpacked.frag > +++ b/src/libcamera/shaders/bayer_unpacked.frag > @@ -128,7 +128,7 @@ void main(void) { > vec3(PATTERN.w, C, PATTERN.z) : > vec3(PATTERN.yx, C)); > > - rgb = rgb - blacklevel; > + rgb = (rgb - blacklevel) / (1.0 - blacklevel); > > /* > * CCM is a 3x3 in the format
Hi, On 11-Feb-26 18:50, Laurent Pinchart wrote: > Hi Milan, > > Thank you for the patch. > > On Wed, Feb 11, 2026 at 02:17:28PM +0100, Milan Zamazal wrote: >> In GPU ISP fragment shaders, the black level is simply subtracted from >> the pixel value. This means the highest pixel values can never be >> reached, possibly resulting in wrong brightness or colour shifts. Fix >> this by spreading the resulting value to the whole 0.0..1.0 range. > > It's expected though. Black level substraction is not the place where > you should perform histogram spreading, this is the job of tone mapping, > further down the pipeline. At the moment we are not correcting for this at all though. So I do think this change makes sense. I do wonder if we cannot fold this into some future multiplication though to reduce the amount of multiplications the GPU needs to do? E.g. multiply all values in the CCM by 1.0 / (1.0 - blacklevel) ? Then we get the same correction factor for free. Regards, Hans > >> The preceding simple pipeline IPA patch ensures `blacklevel' is less >> than 1.0, preventing division by zero here. >> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- >> src/libcamera/shaders/bayer_1x_packed.frag | 2 +- >> src/libcamera/shaders/bayer_unpacked.frag | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag >> index 23747f78a..06ddc040b 100644 >> --- a/src/libcamera/shaders/bayer_1x_packed.frag >> +++ b/src/libcamera/shaders/bayer_1x_packed.frag >> @@ -225,7 +225,7 @@ void main(void) >> vec3(patterns.y, C, patterns.x) : >> vec3(patterns.wz, C)); >> >> - rgb = rgb - blacklevel; >> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >> >> /* >> * CCM is a 3x3 in the format >> diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag >> index 1b85196ae..98dea512c 100644 >> --- a/src/libcamera/shaders/bayer_unpacked.frag >> +++ b/src/libcamera/shaders/bayer_unpacked.frag >> @@ -128,7 +128,7 @@ void main(void) { >> vec3(PATTERN.w, C, PATTERN.z) : >> vec3(PATTERN.yx, C)); >> >> - rgb = rgb - blacklevel; >> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >> >> /* >> * CCM is a 3x3 in the format >
On Wed, Feb 11, 2026 at 07:08:35PM +0100, johannes.goede@oss.qualcomm.com wrote: > On 11-Feb-26 18:50, Laurent Pinchart wrote: > > On Wed, Feb 11, 2026 at 02:17:28PM +0100, Milan Zamazal wrote: > >> In GPU ISP fragment shaders, the black level is simply subtracted from > >> the pixel value. This means the highest pixel values can never be > >> reached, possibly resulting in wrong brightness or colour shifts. Fix > >> this by spreading the resulting value to the whole 0.0..1.0 range. > > > > It's expected though. Black level substraction is not the place where > > you should perform histogram spreading, this is the job of tone mapping, > > further down the pipeline. > > At the moment we are not correcting for this at all though. So I do > think this change makes sense. I'm worried that we are adding quite a few changes that go in the opposite direction of the one we should be taking, and it will then be more difficult to reverse course. > I do wonder if we cannot fold this into some future multiplication > though to reduce the amount of multiplications the GPU needs to do? > > E.g. multiply all values in the CCM by 1.0 / (1.0 - blacklevel) ? > > Then we get the same correction factor for free. > > >> The preceding simple pipeline IPA patch ensures `blacklevel' is less > >> than 1.0, preventing division by zero here. > >> > >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > >> --- > >> src/libcamera/shaders/bayer_1x_packed.frag | 2 +- > >> src/libcamera/shaders/bayer_unpacked.frag | 2 +- > >> 2 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag > >> index 23747f78a..06ddc040b 100644 > >> --- a/src/libcamera/shaders/bayer_1x_packed.frag > >> +++ b/src/libcamera/shaders/bayer_1x_packed.frag > >> @@ -225,7 +225,7 @@ void main(void) > >> vec3(patterns.y, C, patterns.x) : > >> vec3(patterns.wz, C)); > >> > >> - rgb = rgb - blacklevel; > >> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); > >> > >> /* > >> * CCM is a 3x3 in the format > >> diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag > >> index 1b85196ae..98dea512c 100644 > >> --- a/src/libcamera/shaders/bayer_unpacked.frag > >> +++ b/src/libcamera/shaders/bayer_unpacked.frag > >> @@ -128,7 +128,7 @@ void main(void) { > >> vec3(PATTERN.w, C, PATTERN.z) : > >> vec3(PATTERN.yx, C)); > >> > >> - rgb = rgb - blacklevel; > >> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); > >> > >> /* > >> * CCM is a 3x3 in the format
Hi Laurent, Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > On Wed, Feb 11, 2026 at 07:08:35PM +0100, johannes.goede@oss.qualcomm.com wrote: >> On 11-Feb-26 18:50, Laurent Pinchart wrote: >> > On Wed, Feb 11, 2026 at 02:17:28PM +0100, Milan Zamazal wrote: > >> >> In GPU ISP fragment shaders, the black level is simply subtracted from >> >> the pixel value. This means the highest pixel values can never be >> >> reached, possibly resulting in wrong brightness or colour shifts. Fix >> >> this by spreading the resulting value to the whole 0.0..1.0 range. >> > >> > It's expected though. Black level substraction is not the place where >> > you should perform histogram spreading, this is the job of tone mapping, >> > further down the pipeline. >> >> At the moment we are not correcting for this at all though. So I do >> think this change makes sense. > > I'm worried that we are adding quite a few changes that go in the > opposite direction of the one we should be taking, and it will then be > more difficult to reverse course. What would be the correct solution with the current state? Do you mean something like step 8 in https://karaimer.github.io/camera-pipeline/? Should a histogram spreading be performed between CCM and contrast? Clipping to 0.0 at bottom and determining an approximately maximum value (regardless of colour?) from the image and then dividing by that value, which can be any positive number? Stuff for compute shaders? Or doing something else? In any case, when applying black level, we should still clip values below it since they are invalid, right? >> I do wonder if we cannot fold this into some future multiplication >> though to reduce the amount of multiplications the GPU needs to do? >> >> E.g. multiply all values in the CCM by 1.0 / (1.0 - blacklevel) ? >> >> Then we get the same correction factor for free. >> >> >> The preceding simple pipeline IPA patch ensures `blacklevel' is less >> >> than 1.0, preventing division by zero here. >> >> >> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> >> --- >> >> src/libcamera/shaders/bayer_1x_packed.frag | 2 +- >> >> src/libcamera/shaders/bayer_unpacked.frag | 2 +- >> >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag >> >> index 23747f78a..06ddc040b 100644 >> >> --- a/src/libcamera/shaders/bayer_1x_packed.frag >> >> +++ b/src/libcamera/shaders/bayer_1x_packed.frag >> >> @@ -225,7 +225,7 @@ void main(void) >> >> vec3(patterns.y, C, patterns.x) : >> >> vec3(patterns.wz, C)); >> >> >> >> - rgb = rgb - blacklevel; >> >> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >> >> >> >> /* >> >> * CCM is a 3x3 in the format >> >> diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag >> >> index 1b85196ae..98dea512c 100644 >> >> --- a/src/libcamera/shaders/bayer_unpacked.frag >> >> +++ b/src/libcamera/shaders/bayer_unpacked.frag >> >> @@ -128,7 +128,7 @@ void main(void) { >> >> vec3(PATTERN.w, C, PATTERN.z) : >> >> vec3(PATTERN.yx, C)); >> >> >> >> - rgb = rgb - blacklevel; >> >> + rgb = (rgb - blacklevel) / (1.0 - blacklevel); >> >> >> >> /* >> >> * CCM is a 3x3 in the format
diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag index 23747f78a..06ddc040b 100644 --- a/src/libcamera/shaders/bayer_1x_packed.frag +++ b/src/libcamera/shaders/bayer_1x_packed.frag @@ -225,7 +225,7 @@ void main(void) vec3(patterns.y, C, patterns.x) : vec3(patterns.wz, C)); - rgb = rgb - blacklevel; + rgb = (rgb - blacklevel) / (1.0 - blacklevel); /* * CCM is a 3x3 in the format diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag index 1b85196ae..98dea512c 100644 --- a/src/libcamera/shaders/bayer_unpacked.frag +++ b/src/libcamera/shaders/bayer_unpacked.frag @@ -128,7 +128,7 @@ void main(void) { vec3(PATTERN.w, C, PATTERN.z) : vec3(PATTERN.yx, C)); - rgb = rgb - blacklevel; + rgb = (rgb - blacklevel) / (1.0 - blacklevel); /* * CCM is a 3x3 in the format
In GPU ISP fragment shaders, the black level is simply subtracted from the pixel value. This means the highest pixel values can never be reached, possibly resulting in wrong brightness or colour shifts. Fix this by spreading the resulting value to the whole 0.0..1.0 range. The preceding simple pipeline IPA patch ensures `blacklevel' is less than 1.0, preventing division by zero here. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/shaders/bayer_1x_packed.frag | 2 +- src/libcamera/shaders/bayer_unpacked.frag | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)