[PATCH v5 17/18] libcamera: software_isp: Apply black level compensation

[Milan Zamazal]

Hans de Goede <hdegoede at redhat.com> writes:

> From: Milan Zamazal <mzamazal at redhat.com>
>
> Black may not be represented as 0 pixel value for given hardware, it may be
> higher.  If this is not compensated then various problems may occur such as low
> contrast or suboptimal exposure.
>
> The black pixel value can be either retrieved from a tuning file for the given
> hardware, or automatically on fly.  The former is the right and correct method,
> while the latter can be used when a tuning file is not available for the given
> hardware.  Since there is currently no support for tuning files in software ISP,
> the automatic, hardware independent way, is always used.  Support for tuning
> files should be added in future but it will require more work than this patch.
>
> The patch looks at the image histogram and assumes that black starts when pixel
> values start occurring on the left.  A certain amount of the darkest pixels is
> ignored; it doesn't matter whether they represent various kinds of noise or are
> real, they are better to omit in any case to make the image looking better.  It
> also doesn't matter whether the darkest pixels occur around the supposed black
> level or are spread between 0 and the black level, the difference is not
> important.
>
> An arbitrary threshold of 2% darkest pixels is applied; there is no magic about
> that value.
>
> The patch assumes that the black values for different colors are the same and
> doesn't attempt any other non-primitive enhancements.  It cannot completely
> replace tuning files and simplicity, while providing visible benefit, is its
> goal.  Anything more sophisticated is left for future patches.
>
> A possible cheap enhancement, if needed, could be setting exposure + gain to
> minimum values temporarily, before setting the black level.  In theory, the
> black level should be fixed but it may not be reached in all images.  For this
> reason, the patch updates black level only if the observed value is lower than
> the current one; it should be never increased.
>
> The purpose of the patch is to compensate for hardware properties.  General
> image contrast enhancements are out of scope of this patch.
>
> Stats are still gathered as an uncorrected histogram, to avoid any confusion and
> to represent the raw image data.  Exposure must be determined after the black
> level correction -- it has no influence on the sub-black area and must be
> correct after applying the black level correction.  The granularity of the
> histogram is increased from 16 to 64 to provide a better precision (there is no
> theory behind either of those numbers).
>
> Reviewed-by: Hans de Goede <hdegoede at redhat.com>
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---

[...]

> +void BlackLevel::update(SwIspStats::histogram &yHistogram)
> +{
> +	// The constant is selected to be "good enough", not overly conservative or
> +	// aggressive. There is no magic about the given value.
> +	constexpr float ignoredPercentage_ = 0.02;
> +	const unsigned int total =
> +		std::accumulate(begin(yHistogram), end(yHistogram), 0);
> +	const unsigned int pixelThreshold = ignoredPercentage_ * total;
> +	const unsigned int currentBlackIdx =
> +		blackLevel_ / (256 / SwIspStats::kYHistogramSize);

Should the newly introduced kRGBLookupSize constant be used instead of 256 here
and in other places in this patch?

[...]