pointer file randomly changes value in middle of reading raw bitmap data

Question

I am currently working on loading a bitmap file in C. I am kind of new to C, and I ran into a problem: I have a file pointer that reads unsigned chars to a struct of rgb pixels, (it's called rgb but it reads in the order of b,g,r and padding - this is the default of bitmap format file). My file is 12x12 pixels and when it reaches to row 9 it putting only the value '204' in each component, whike the image is white (i.e. all components = 255). All the components before this equal 255. EDIT: I changed the enum to a three defined values returned by the image state (didn't load, blue, not blue). EDIT2: I edited the code, but now im equals 0xffffffff and cannot be accessed to. here's the code:

int CalaculateBlueness()
 {

bih bih;
bfh bfh;
int counterblue = 0;
hsv hsv;
FILE *filePtr = fopen("C:\\Users\\mishe\\Desktop\\white.bmp", "rb");


//if the file doesn't exist in memory
if (filePtr == NULL)
{
    return IMAGE_NOT_LOADED;
}

//read the bitmap file header
fread(&bfh, sizeof(bfh), 1, filePtr);

//verify that this is a bmp file by check bitmap id
if (bfh.bitmap_type[0] != 'B' || bfh.bitmap_type[1] != 'M')
{
    fclose(filePtr);
    return IMAGE_NOT_LOADED;
}

fclose(filePtr);

//ensure that the filePtr will point at the start of the image info header
filePtr = fopen("C:\\Users\\mishe\\Desktop\\white.bmp", "rb");
fseek(filePtr, 14, SEEK_CUR);

//read the bitmap info header
fread(&bih, sizeof(bih), 1, filePtr);

if (bih.bit_count!=24)
{
    return ERROR_BPP;
}

//point the pointer file to the start of the raw data
//fseek(filePtr, bfh.file_size, SEEK_SET);

int size = bih.height * WIDTHBYTES(bih.width * 32);

unsigned char *im = calloc(1, size);
//put the raw bitmap pixel data in strcut rgb array 


fread(&im, sizeof(size), 1, filePtr);

//convert each pixel to it's hue value and check if in the range of blue
for (size_t i = 0; i < bih.height; i++)
{
    for (size_t j = 0; j < bih.width; j++)
    {
        hsv =rgbpixel_hue(im);
        if (hsv.h>190 && hsv.h<250)
        {
            counterblue++;
        }
        fseek(im, 3, SEEK_CUR);
    }

}

//check if more than 80% of the image is blue and return the defined state according to the result
if (counterblue > BLUE_THRESHOLD*(bih.height*bih.width))
{
    return  BLUE;
}
return  NOT_BLUE;

}

What is the values of `bih.height` and `bih.width`? Are they perhaps larger than `12`? Are you *sure* about that? Have you printed them out, or used a debugger to find out? Or used a debugger to step through the code or add a watch-point on whatever variable that supposedly changes value? And you should *really* add some error checking! — Some programmer dude, Dec 12 '17 at 08:26
if (filePtr == NULL) { return img_state = 2; } This should have been immediately after fopen — asio_guy, Dec 12 '17 at 08:28
On a totally unrelated note, what is the use of the `img_state` variable? What do you need it for? Can't you just do e.g. `return 2` instead? — Some programmer dude, Dec 12 '17 at 08:29
@potato correct, should have checked that. copied it from old code and didnt't pay attention. — Meowstar, Dec 12 '17 at 08:30
I could, but I like being organized and returning an enum that has 3 constant states is organized for me — Meowstar, Dec 12 '17 at 08:31
Then return the *enumeration symbol* instead of an integer like `2`. The `2` is just a [*magic number*](https://en.wikipedia.org/wiki/Magic_number_(programming)) and doesn't tell us (or other readers of the code) anything. Defining an enumeration also defines the symbols in it as symbols that can be used. — Some programmer dude, Dec 12 '17 at 08:34
well how would you do it ? I need the organization in my head and if i wont have the enum I wont remeber which state says what and hat it's value. In other words, how t make it clear to readers as well as to myself ? — Meowstar, Dec 12 '17 at 08:36
When you define an enumeration you do it like e.g. `typedef enum { STATE1, STATE2, STATE3 } state;`. Then you can use `STATE1` instead of the magic number `0`. So you can do `return STATE1;` instead of `return 0;`. Returning `0` could mean *anything*, but returning `STATE1` tells the readers something specific about what is being returned. — Some programmer dude, Dec 12 '17 at 08:42
Talking about enumerations and how to use them is getting a little bit off-topic though. [Get a good beginners book or two](http://stackoverflow.com/questions/562303/the-definitive-c-book-guide-and-list) to learn more about them and how they are used. As for your actual problem, I suggest you [learn how to debug your programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). — Some programmer dude, Dec 12 '17 at 08:44
`bih bih; bfh bfh;` This doesn't look like some compileable code... — Gerhardh, Dec 12 '17 at 09:06
Maybe but you cannot use same name for a typedef and a variable. — Gerhardh, Dec 12 '17 at 09:15

Paul Ogilvie · Accepted Answer · 2020-01-12T15:47:59.330

Reading in bitmaps is always a difficult thing.

There are quite some points to consider.

fread(&im[i][j].res, sizeof(unsigned char), 1, filePtr);

With this line your read the reserved byte of an RGBQUAD from the file. However, this member is not in the file. The image data in the file contains scanlines. See below.

After reading the Bitmap File Header, you open the file again. However, you didn't close it. This might be succesful or it fails because the file was already open. But you don't chek the return value of fopen. Anyway, there is no need to do this because after having read the BFH the filepointer is positioned at the BITMAPINFOHEADER; you can just read it in. And you need to read it in, otherwise you won't know the dimensions of the bitmap.

From MSDN documentation:

The established bitmap file format consists of a BITMAPFILEHEADER structure followed by a BITMAPINFOHEADER [...] structure. An array of RGBQUAD structures (also called a color table) follows the bitmap information header structure. The color table is followed by a second array of indexes into the color table (the actual bitmap data).

For 24 bits-per-pixel bitmaps, there is no color table.

so the sequence is now:

    //read the bitmap file header
    fread(&bfh, sizeof(bfh), 1, filePtr);

    //read the bitmap info header
    fread(&bih, sizeof(bih), 1, filePtr);

    int bpp= bih.biBitCount;
    if (bpp != 24) return 0;  // error: must be 24 bpp/ 3 bytes per pixel

Now we must calculate the amount of memory needed to store the image. An image consists of heighth rows of width pixels. The rows are called scanlines.

For some reason, a scanline is alligned on a 4 byte boundary. This means that the last bytes of a scanline may not be in use. So the amount of memory to read the bitmap data from the file is the heigth of the image, times the number of scanlines of the image:

// WIDTHBYTES takes # of bits in a scanline and rounds up to nearest word.
#define WIDTHBYTES(bits)    (((bits) + 31) / 32 * 4)

    int size= bih.biHeight * WIDTHBYTES(bih.biWidth*32));
    unsigned char *im= malloc(size);

Finally you can read the data. If there was a color table, you should have skipped that with a seek but since there is none, you can now read the data:

    fread(im, size, 1, filePtr);

Now, to address the pixels you cannot use a simple two-dimensional notation governed by width and heigth of the image...due to the scanlines. You can use the following:

    int scanlineSize= WIDTHBYTES(bih.biWidth*bih.biBitCount);
    unsigned char *p, *scanline= im;

    for (int i=0; i<bih.biHeight; i++)
    {
        p= scanline;
        for (int j=0; j<bih.biWidth; j++)
        {
            g= *p++;
            b= *p++;
            r= *p++;
        }
        scanline += scanlineSize;
    }

So to address a 3-byte pixel (x,y) use: im[y*scanlineSize + x*3]

.. except that I believe that scanlines are reversed, so the pixel would be at im[(bih.biHeight-y)*scanlinesize + x*3].

UPDATE: complete function

#include <winGDI.h>
// WIDTHBYTES takes # of bits in a scanline and rounds up to nearest word.
#define WIDTHBYTES(bits)    (((bits) + 31) / 32 * 4)

unsigned char *readBitmap(char *szFilename)
{
    BITMAPFILEHEADER bfh;
    BITMAPINFOHEADER bih;
    int i, j, size, scanlineSize;
    unsigned char r, g, b, *p, *img, *scanline;
    FILE *filePtr;

    if ((filePtr=fopen(szFilename, "rb"))==0) return 0;

    //read the bitmap file header
    if (fread(&bfh, sizeof(bfh), 1, filePtr)!=1
    ||  bfh.bfType != 'MB') {fclose(filePtr); return 0;}

    //read the bitmap info header
    if (fread(&bih, sizeof(bih), 1, filePtr)!=1
    ||  bih.biSize!=sizeof(bih)) {fclose(filePtr); return 0;}

    if (bih.biBitCount != 24) {fclose(filePtr); return 0;}  // error: must be 24 bpp/ 3 bytes per pixel

    // allocate memory and read the image
    scanlineSize= WIDTHBYTES(bih.biWidth * bih.biBitCount);
    size= bih.biHeight * scanlineSize;
    if ((img= malloc(size))==0) {fclose(filePtr); return 0;}
    if (fread(img, size, 1, filePtr)!=1) {free (img); fclose(filePtr); return 0;}
    fclose(filePtr);

    scanline= img;
    for (i=0; i<bih.biHeight; i++)
    {
        p= scanline;
        for (j=0; j<bih.biWidth; j++)
        {
            g= *p++;
            b= *p++;
            r= *p++;
        }
        scanline += scanlineSize;
    }
    return img;
}

thank you. Exactly the answer I was looking for and +1 for the detailed example — Meowstar, Dec 12 '17 at 12:05
@Meowstar The answer has 0 upvotes... Did you forget the upvote or has someone actually downvoted this? — niCk cAMel, Dec 12 '17 at 12:10
can you add an explanation of the logic behind the calculation of the memory needed to store the image ? — Meowstar, Dec 12 '17 at 12:15

score 0 · Answer 2 · answered Dec 12 '17 at 09:13

close filePtr before opening file again

fclose(filePtr);
filePtr = fopen("C:\\Users\\mishe\\Desktop\\white.bmp", "rb");

and offset to raw data is bfh.offset

  //point the pointer file to the start of the raw data
    fseek(filePtr, bfh.offset, SEEK_SET);

pointer file randomly changes value in middle of reading raw bitmap data

2 Answers2

Linked