3
\$\begingroup\$

I am trying to build functionality to check if an image taken by the camera is too dark. So I added the following 2 functions to a utility class:

public static boolean checkIfImageIsTooDark(BitmapRegionDecoder bitmapRegionDecoder)
{
    if(bitmapRegionDecoder == null) {
        Timber.e("Expected bitmapRegionDecoder was null");
        return false;
    }

    int loadingBitmapHeight = bitmapRegionDecoder.getHeight();
    int loadingBitmapWidth = bitmapRegionDecoder.getWidth();

    boolean isImageTooDark = true;

    int checkBitmapWidth = loadingBitmapWidth / 10;
    int checkBitmapHeight = loadingBitmapHeight / 10;
    int topBitmapPosition = 0;
    int leftBitmapPosition = 0;

    while((checkBitmapWidth <= loadingBitmapWidth) && (leftBitmapPosition < checkBitmapWidth)) {
        while((checkBitmapHeight <= loadingBitmapHeight) && (topBitmapPosition < checkBitmapHeight)) {
            Rect bitmapRect = new Rect(leftBitmapPosition,topBitmapPosition,checkBitmapWidth,checkBitmapHeight);
            Timber.d("left: " + leftBitmapPosition + " right: " + checkBitmapWidth + " top: " + topBitmapPosition + " bottom: " + checkBitmapHeight);

            Bitmap processBitmap = bitmapRegionDecoder.decodeRegion(bitmapRect,null);

            if(!checkIfImageIsTooDark(processBitmap)) {
                isImageTooDark = false;
                break;
            }

            topBitmapPosition = checkBitmapHeight;
            checkBitmapHeight += (checkBitmapHeight < (loadingBitmapHeight - checkBitmapHeight)) ? checkBitmapHeight : (loadingBitmapHeight - checkBitmapHeight);
        }

        if(!isImageTooDark) {
            break;
        }

        topBitmapPosition = 0; 
        checkBitmapHeight = loadingBitmapHeight / 10; 
        leftBitmapPosition = checkBitmapWidth;
        checkBitmapWidth += (checkBitmapWidth < (loadingBitmapWidth - checkBitmapWidth)) ? checkBitmapWidth : (loadingBitmapWidth - checkBitmapWidth);
    }

    return isImageTooDark;
}

public static boolean checkIfImageIsTooDark(Bitmap bitmap) {
    if(bitmap == null)
    {
        Timber.e("Expected bitmap was null");
        return false;
    }

    int[] brightnessHistogram = new int[256];

    for(int i = 0; i < bitmap.getHeight(); i++) {
        for(int j = 0; j < bitmap.getWidth(); j++) {
            int pixel = bitmap.getPixel(j,i);

            int r = Color.red(pixel);
            int g = Color.green(pixel);
            int b = Color.blue(pixel);

            int brightness = (int)(0.2126 * r + 0.7152 * g + 0.0722 * b);
            brightnessHistogram[brightness]++;
        }
    }

    int allPixelsCount = bitmap.getWidth() * bitmap.getHeight();

    //counting pixels with brightness less than 10
    int darkPixelCount = 0;
    for(int i = 0; i < 10; i++)
    {
        darkPixelCount += brightnessHistogram[i];
    }

    //if more than 70% pixels are too dark then image is too dark
    return darkPixelCount > allPixelsCount * 0.70;
}

The brightness histogram approach I found over here -> https://stackoverflow.com/a/35914745/3287853. And it works quite well.

The only problem however is that processing an image takes way too much time. I tried it on my phone which produces 4160x3120 and it takes close to a minute to get a result off it. Also, the 4 loops don't look great and I think there should be a better way to make this work, personally.

I am using an AsyncTask to do this in the background so one improvement that I could think of is to break up the task across multiple threads.

I could also compress the image initially bringing it down to a certain limit of width and height while attempting to maintain its aspect ratio and then process the image. However I wonder if that would actually help in reducing the amount of time taken at all.

\$\endgroup\$
  • \$\begingroup\$ On first glance, a quick optimization is to count brightness < 10 directly without allocating full histogram and break once the threshold is exceeded (or clearly will not be). \$\endgroup\$ – user650881 Oct 11 '17 at 8:40
  • \$\begingroup\$ Also this strikes me as a matrix operation you might do in the GPU. \$\endgroup\$ – user650881 Oct 11 '17 at 8:44
2
\$\begingroup\$

Intelligibility

I can't quite work out what's going on with the main loop.

    while((checkBitmapWidth <= loadingBitmapWidth) && (leftBitmapPosition < checkBitmapWidth)) {
        while((checkBitmapHeight <= loadingBitmapHeight) && (topBitmapPosition < checkBitmapHeight)) {

My best guess here is that the image is being loaded in 100 chunks to reduce the memory load, but the variable names and lack of comments don't help me. In addition, the described behaviour is "if more than 70% pixels are too dark then image is too dark", but given the way that method is used it seems that actually you just want at least 1 of the 100 chunks to have bright pixels, so a bright cluster of 31% of 1% of the pixels is enough to mark the whole image as "bright enough". I'm not sure whether that's intentional, or whether it's a speed optimisation gone wrong. More comments needed.


Speed

Starting with the big one:

    for(int i = 0; i < bitmap.getHeight(); i++) {
        for(int j = 0; j < bitmap.getWidth(); j++) {
            int pixel = bitmap.getPixel(j,i);

That is a classic blunder on the level of invading Russia in the winter or going up against a Sicilian when death is on the line.

bitmap.getPixels will probably give you a speed-up of about two orders of magnitude.

The other thing which would probably give a moderate speedup is changing the way the image is divided into chunks. Image formats are compressed: you will get better performance if the way you access the image respects the compression format. For JPEGs that means using chunk sizes which are multiples of 8 pixels wide and high where possible, and I think (although a quick search has not sufficed to confirm) that you'd be better off with full-width small-height chunks than medium-width medium-height chunks. (I.e. I expect the 8x8 JPEG blocks to be laid out in English reading order).

\$\endgroup\$
2
\$\begingroup\$

Sampling: Don't process ALL pixels

The speed mainly depend on the number of operations, in our case, pixels. And a lot of pixels. This means that the larger the image is, the slower your code will run.

You probably don't need to process all pixels, just a representative subset of them.

Taking this subset is called sampling. You can for example only process every 100th pixel.

If you want to go more fancy, try Monte Carlo sampling.

If you really need all the pixels, check OpenCV, that is optimized for image processing.

\$\endgroup\$
  • 1
    \$\begingroup\$ So essentially pick a pixel see if its dark or not. And because the image is so huge, all the nearby 100(for example) pixels will also ideally be that dark. Is that the right understanding? \$\endgroup\$ – Bluesir9 Oct 11 '17 at 12:10
  • 1
    \$\begingroup\$ Also in that case the number 100 should be a factor of the actual width and height of the image as well right? \$\endgroup\$ – Bluesir9 Oct 11 '17 at 12:11
  • 1
    \$\begingroup\$ Yes, pick some pixels instead of all. 100 was just an example, it should be taken in such a way that the pixels that are sampled cover the image as well as possible. \$\endgroup\$ – RobAu Oct 11 '17 at 12:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.