libcamera: software_isp: Fix gamma table when CCM is used

24984 diff mbox series

Software CPU ISP computes a gamma lookup table.  The table incorporates
black level and contrast.  All entries in the table below the black
level are set to 0.  This is not necessarily correct all the time.

Let's consider this case: The CCM is

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

and contrast is set to zero.  The gamma table has all the entries above
the black level set to 186 (due to zero contrast) and all the entries
below the black level set to 0.  CCM is applied before gamma, a
non-black level pixel has the blue component set to 0 with the CCM
above.  Now, when the gamma lookup is applied, the red and green
components are set to 186, while the blue component is set to 0.  The
resulting pixel is then yellow rather than grey (as it should be with
zero contrast).

There are two ways to fix this: Either clamping pixel colour channels to
the black level in debayering or setting the below black level entries
in the gamma lookup table to the lowest value of the gamma table rather
than 0.  Both should have the same effect.  Let's opt for the latter for
its simplicity.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/ipa/simple/algorithms/lut.cpp | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Hi,


On 7-Nov-25 4:20 PM, Milan Zamazal wrote:
> Software CPU ISP computes a gamma lookup table.  The table incorporates
> black level and contrast.  All entries in the table below the black
> level are set to 0.  This is not necessarily correct all the time.
> 
> Let's consider this case: The CCM is
> 
>   [1 0 0]
>   [0 1 0]
>   [0 0 0]
> 
> and contrast is set to zero.  The gamma table has all the entries above
> the black level set to 186 (due to zero contrast) and all the entries
> below the black level set to 0.  CCM is applied before gamma, a
> non-black level pixel has the blue component set to 0 with the CCM
> above.  Now, when the gamma lookup is applied, the red and green
> components are set to 186, while the blue component is set to 0.  The
> resulting pixel is then yellow rather than grey (as it should be with
> zero contrast).
> 
> There are two ways to fix this: Either clamping pixel colour channels to
> the black level in debayering or setting the below black level entries
> in the gamma lookup table to the lowest value of the gamma table rather
> than 0.  Both should have the same effect.  Let's opt for the latter for
> its simplicity.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans


> ---
>  src/ipa/simple/algorithms/lut.cpp | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp
> index d1d5f7271..d4a79e101 100644
> --- a/src/ipa/simple/algorithms/lut.cpp
> +++ b/src/ipa/simple/algorithms/lut.cpp
> @@ -61,7 +61,6 @@ void Lut::updateGammaTable(IPAContext &context)
>  	const unsigned int blackIndex = blackLevel * gammaTable.size() / 256;
>  	const auto contrast = context.activeState.knobs.contrast.value_or(1.0);
>  
> -	std::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);
>  	const float divisor = gammaTable.size() - blackIndex - 1.0;
>  	for (unsigned int i = blackIndex; i < gammaTable.size(); i++) {
>  		double normalized = (i - blackIndex) / divisor;
> @@ -75,6 +74,14 @@ void Lut::updateGammaTable(IPAContext &context)
>  		gammaTable[i] = UINT8_MAX *
>  				std::pow(normalized, context.configuration.gamma);
>  	}
> +	/*
> +	 * Due to CCM operations, the table lookup may reach indices below the black
> +	 * level. Let's set the table values below black level to the minimum
> +	 * non-black value to prevent problems when the minimum value is
> +	 * significantly non-zero (for example, when the image should be all grey).
> +	 */
> +	std::fill(gammaTable.begin(), gammaTable.begin() + blackIndex,
> +		  gammaTable[blackIndex]);
>  
>  	context.activeState.gamma.blackLevel = blackLevel;
>  	context.activeState.gamma.contrast = contrast;