[PATCH v3 06/16] libcamera: software_isp: Add SwStatsCpu class

[Hans de Goede]

Hi Milan,

On 2/16/24 10:43, Milan Zamazal wrote:
> Hans de Goede <hdegoede at redhat.com> writes:

<snip>

>> +static const unsigned int kRedYMul = 77; /* 0.299 * 256 */
>> +static const unsigned int kGreenYMul = 150; /* 0.587 * 256 */
>> +static const unsigned int kBlueYMul = 29; /* 0.114 * 256 */
>> +
>> +#define SWSTATS_START_LINE_STATS(pixel_t) \
>> +	pixel_t r, g, g2, b;              \
>> +	unsigned int yVal;                \
>> +                                          \
>> +	unsigned int sumR = 0;            \
>> +	unsigned int sumG = 0;            \
>> +	unsigned int sumB = 0;
>> +
>> +#define SWSTATS_ACCUMULATE_LINE_STATS(div) \
>> +	sumR += r;                         \
>> +	sumG += g;                         \
>> +	sumB += b;                         \
>> +                                           \
>> +	yVal = r * kRedYMul;               \
>> +	yVal += g * kGreenYMul;            \
>> +	yVal += b * kBlueYMul;             \
>> +	stats_.yHistogram[yVal / (256 * SwIspStats::kYHistogramSize * (div))]++;
> 
> This formula is wrong.  I think it should be
> 
>   stats_.yHistogram[yVal * SwIspStats::kYHistogramSize / (256 * 256 * (div))]++;
> 
> (The maximum yVal is 255 * 256 and the size of the array is
> SwIspStats::kYHistogramSize.)
> 
> It works for kYHistogramSize == 16 by coincidence as in this case the two
> versions are the same.

You are right, thank you for catching this.

I have pushed fixup patch (to be squashed later) to:

https://github.com/jwrdegoede/libcamera/commits/SoftwareISP-v06

So that I don't forget to fix this for the next version.

Regards,

Hans