Reading the BGR colors from a bitmap in C

Question

I was trying to obtain the RGB values from a 24-bit BMP file. The image that I am using is a tiny image, all red, so all pixels BGR configuration should be B:0 G:0 R:255. I do this:

int main(int argc, char **argv)
{
    principal();
    return 0;
}

typedef struct {
    unsigned char blue;
    unsigned char green;
    unsigned char red;
} rgb;

typedef struct {
    int ancho, alto;
    rgb *pixeles[MAX_COORD][MAX_COORD];
} tBitmapData;

void principal()
{

    FILE *fichero;
    tBitmapData *bmpdata = (tBitmapData *) malloc(sizeof(tBitmapData));
    rgb *pixel;
    int i, j, num_bytes;
    unsigned char *buffer_imag;
    char nombre[] = "imagen.bmp";
    fichero = fopen(nombre, "r");
    if (fichero == NULL)
            puts("No encontrado\n");
    else {
            fseek(fichero, 18, SEEK_SET);
            fread(&(bmpdata->ancho), sizeof((bmpdata->ancho)), 4, fichero);
            printf("Ancho: %d\n", bmpdata->ancho);
            fseek(fichero, 22, SEEK_SET);
            fread(&(bmpdata->alto), sizeof((bmpdata->alto)), 4, fichero);
            printf("Alto: %d\n", bmpdata->alto);
    }

    num_bytes = (bmpdata->alto * bmpdata->ancho * 3);
    fseek(fichero, 54, SEEK_SET);
    for (j = 0; j < bmpdata->alto; j++) {
            printf("R   G   B Fila %d\n", j + 1);
            for (i = 0; i < bmpdata->ancho; i++) {
                    pixel =
                        (rgb *) malloc(sizeof(rgb) * bmpdata->alto *
                                       bmpdata->ancho * 3);
                    fread(pixel, 1, sizeof(rgb), fichero);
                    printf("Pixel %d: B: %3d G: %d R: %d \n", i + 1,
                           pixel->blue, pixel->green, pixel->red);
            }
    }
    fclose(fichero);
}

The problem is that when I print them, the first pixels are fine, B:0 G:0 R:255, but then they start to change to B:0 G:255 R:0, and then to B:255 G:0 R:0. If the width is 10 pixels, then the change happens every 10 pixels.

This probably isn't the cause of your problem, but do you realize that you're allocating *three times* the memory necessary to hold the *entire image*, for *every pixel*, and leaking all of it? If you're going to `fread` each pixel individually, you could just use an `rgb` struct allocated on the stack. — zwol, Mar 19 '11 at 00:36
Also, it would be helpful if you could upload an image file that shows the problem, somewhere we can get at it. — zwol, Mar 19 '11 at 00:37
The simple explanation is that the pixel format of the bitmap is 32bpp. You don't check it, that's a mistake. You are ignoring stride as well, fatal on a 24bpp format. Don't write this code yourself, libraries that know how to read image files in arbitrary formats are plentiful. — Hans Passant, Mar 19 '11 at 01:01

score 5 · Answer 1 · answered Mar 19 '11 at 00:39

5

In the BMP file format, each row of pixel data may be padded in order to round up to a multiple of 4 bytes.

If you have 10 24-bit pixels, that's 30 bytes, which are then followed by 2 bytes of padding. Your code doesn't skip over the padding.

answered Mar 19 '11 at 00:39

Matthew Slattery

45,290
8
103
119

score 2 · Answer 2 · answered Mar 19 '11 at 00:53

I think your fread(3) calls are wrong:

        fread(&(bmpdata->ancho), sizeof((bmpdata->ancho)), 4, fichero);

This asks to read 4*sizeof((bmpdata->ancho)) bytes into an int. I assume sizeof((bmpdata->ancho)) returns 4, so I think you're scribbling over unrelated memory with these two calls. Change the 4 to 1 -- you're only reading one item.

You never use num_bytes; delete it. Unused code makes thinking about the used code that much more difficult. :)

You're allocating three times as much memory as you need to:

                pixel =
                    (rgb *) malloc(sizeof(rgb) * bmpdata->alto *
                                   bmpdata->ancho * 3);

The 3 looks like an attempt to account for each of red, green, blue, in your rgb structure, but sizeof(rgb) already knows the correct size of the structure. (Which might be 4 bytes, for convenient alignment for 32-bit CPUs, or it might be 12 bytes, again for alignment (each char on its own 4 byte boundary), or maybe even 24 bytes on 64-bit systems that really enjoy working with data aligned on 8 byte boundaries.)

And the final thing I note:

                fread(pixel, 1, sizeof(rgb), fichero);

Because the C compiler is allowed to insert holes into structures, you cannot assume that the on-disk format matches your in-memory structure definition. You need to either use the GNU C extension __packed__ attribute or you need to read data from the from using libraries or structures designed for the bmp format. If this is a fun project for you, then definitely try the __packed__ route: if it works, good, if it doesn't work, hopefully you can learn why not, and re-write your code to load each element of the structure manually. If you're just trying to get something that can correctly parse bitmaps, then you might want to try to find some pre-written libraries that already parse images correctly.

(And yes, it is VERY IMPORTANT to get image parsing correct; CVE has a list of malformed image exploits that allow attackers control over programs, many of them are remotely exploitable.)

Reading the BGR colors from a bitmap in C

2 Answers2