Histogram not accurate for dark images

Question

I'm using the next algorithm to calculate the histogram from a YUV420sp image. Seems to work but the result is not 100% accurate for a fully dark image. When the image is dark I would expect to have on the left side of the histogram a high pick showing that the image is too dark, but the algorithm in such scenario shows instead a flat line, no pick. On the other light scenarios the histogram seems to be accurate.

void calculateHistogram(const unsigned char* yuv420sp, const int yuvWidth, const int yuvHeight, const int histogramControlHeight, int* outHistogramData)
{
    const int BINS = 256;

    // Clear the output

    memset(outHistogramData, 0, BINS * sizeof(int));

    // Get YUV brightness values

    const int totalPixels = yuvWidth * yuvHeight;

    for (int index = 0; index < totalPixels; index++)
    {
        char brightness = yuv420sp[index];
        outHistogramData[brightness]++;
    }

    // Get the maximum brightness

    int maxBrightness = 0;

    for (int index = 0; index < BINS; index++)
    {
        if (outHistogramData[index] > maxBrightness)
        {
            maxBrightness = outHistogramData[index];
        }
    }

    // Normalize to fit the UI control height

    const int maxNormalized = BINS * histogramControlHeight / maxBrightness;

    for(int index = 0; index < BINS; index++)
    {
        outHistogramData[index] = (outHistogramData[index] * maxNormalized) >> 8;
    }
}

[SOLVED by galop1n] Though Galop1n implementation is much nicer I'm updating this one with the corrections in case is of use to anyone.

Changes:

1) Reading brightness values into an unsigned char instead of a char.

2) Placed UI normalization division into the normalization loop.

void calculateHistogram(const unsigned char* yuv420sp, const int yuvWidth, const int yuvHeight, const int histogramCanvasHeight, int* outHistogramData)
{
    const int BINS = 256;

    // Clear the output

    memset(outHistogramData, 0, BINS * sizeof(int));

    // Get YUV brightness values

    const int totalPixels = yuvWidth * yuvHeight;

    for (int index = 0; index < totalPixels; index++)
    {
        unsigned char brightness = yuv420sp[index];
        outHistogramData[brightness]++;
    }

    // Get the maximum brightness

    int maxBrightness = 0;

    for (int index = 0; index < BINS; index++)
    {
        if (outHistogramData[index] > maxBrightness)
        {
            maxBrightness = outHistogramData[index];
        }
    }

    // Normalize to fit the UI control height

    for(int index = 0; index < BINS; index++)
    {
        outHistogramData[index] = outHistogramData[index] * histogramCanvasHeight / maxBrightness;
    }
}

Answer 1

There is at least two bugs in your implementation.

1. The indexing by the brightness because of using a temporary of type signed char.
2. The final normalization result can be influence by the value of control height and the maximum count of pixel in a bin. The division cannot really be put outside of the loop because of that.

I recommend also to use a std::array ( need c++11 ) to store the result instead of a raw pointer as there is a risk the caller do not allocate enough space for what will use the function.

#include <algorithm>
#include <array>

void calculateHistogram(const unsigned char* yuv420sp, const int yuvWidth, const int yuvHeight, const int histogramControlHeight, std::array<int, 256> &outHistogramData ) {
    outHistogramData.fill(0);
    std::for_each( yuv420sp, yuv420sp + yuvWidth * yuvHeight, [&](int e) {
       outHistogramData[e]++;
    } );
    int maxCountInBins = * std::max_element( begin(outHistogramData), end(outHistogramData) );
    for( int &bin : outHistogramData )
        bin = bin * histogramControlHeight / maxCountInBins;
}