24 bit bmp mirror image in c

I am trying to make a mirror image for any 24 bit bmp image in c, but when I run the program it always gets distorted. This is suppose to be the nested for loop that replaces all right and left pixels with each other. Anyone see any faults with this logic? The program makes a new image.

insert

Code:

---


for (j = 0; j < image.height; j++)
    {
        for (i = 0; i < image.width+(image.padding); i++)
        {
            int mirrorrow = image.width - i -1;
           
            if (i == image.width-1  && image.padding!=0)
            {
                for (t = 0;t < image.padding;t++)
                    new.pixels[(image.width * 3 + t) + (j*image.width * 3 + image.padding)] = image.pixels[(image.width * 3 + t) + (j*image.width * 3 + image.padding)];
               
            }
            else
            {
            int xPos = j * image.width * 3 + 3 * i;
            int xEndPos = j * image.width * 3 + 3*mirrorrow;


            unsigned char temp1 = image.pixels[xEndPos - 2];
            unsigned char temp2 = image.pixels[xEndPos - 1];
            unsigned char temp3 = image.pixels[xEndPos];


            new.pixels[xPos] = temp1;
            new.pixels[xPos + 1] = temp2;
            new.pixels[xPos + 2] = temp3;
            }
        }

---

This is what I get when the program finishes, original image vs the new one that is suppose to be the mirrored original one.
Attachment 15789

Here is my original unedited source code for the program, it should only work for 24 bit bmp images.

insert

Code:

---

#define _CRT_SECURE_NO_WARNINGS#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <conio.h>
#include<string.h>


typedef struct {
        int width;
        int height;
        int pixeloffset;
        int padding;
        unsigned char header[54];
        unsigned char* pixels;
        int size;
}BMP;


BMP mirrorimage(BMP image)
{


        int um = image.height * image.width;
        int i,j,t;
        BMP novi;
        novi.width = image.width;
        novi.height = image.height;
        novi.size = image.size;
        novi.pixeloffset = image.pixeloffset;
        novi.padding = image.padding;


        for (i = 0; i < 54; i++)
        {
                novi.header[i] = image.header[i];
        }


        novi.pixels = (unsigned char*)malloc((image.padding*image.height)+image.width * image.height * 3 * sizeof(unsigned char));


        for (j = 0; j < image.height; j++)
        {
                for (i = 0; i < image.width+(image.padding); i++)
                {
                        int zrcalnakolona = image.width - i -1;
                        //zrcalnakolona = i;
                        if (i == image.width-1  && image.padding!=0)
                        {
                                for (t = 0;t < image.padding;t++)
                                        novi.pixels[(image.width * 3 + t) + (j*image.width * 3 + image.padding)] = image.pixels[(image.width * 3 + t) + (j*image.width * 3 + image.padding)];
                               
                        }
                        else
                        {
                        int xPos = j * image.width * 3 + 3 * i;
                        int xEndPos = j * image.width * 3 + 3*zrcalnakolona+image.width*2;


                        unsigned char temp1 = image.pixels[xEndPos - 2];
                        unsigned char temp2 = image.pixels[xEndPos - 1];
                        unsigned char temp3 = image.pixels[xEndPos];


                        novi.pixels[xPos] = temp1;
                        novi.pixels[xPos + 1] = temp2;
                        novi.pixels[xPos + 2] = temp3;
                        }
                }


        }






        return novi;
}




BMP citanjeBMP()
{
        BMP image;
        int i;
        FILE* slika = fopen("C:\\Temp\\test.bmp", "rb");
        fread(image.header, sizeof(unsigned char), 54, slika);


        image.width = *(int*)& image.header[18];
        image.height = *(int*)& image.header[22];
        image.pixeloffset = *(int*)& image.header[10];
        image.padding = (image.width*3) % 4;
        image.size = 3 * image.width * image.height;
        image.pixels = (unsigned char*)malloc(image.padding*image.height + image.width * image.height * 3 * sizeof(unsigned char));
        fread(image.pixels, sizeof(unsigned char), image.size, slika);
        fclose(slika);


        for (i = 0; i < image.size + (image.padding*image.height); i += 3)
        {
                if (i%image.width*3-1 == 0 && image.padding != 0 && i != 0)
                {
                        i = i + image.padding-3;
                }
                else
                {
                unsigned char tmp = image.pixels[i];
                image.pixels[i] = image.pixels[i + 2];
                image.pixels[i + 2] = tmp;
                }
        }
        return image;
}


void upisivanjeBMP(BMP image)
{
        FILE* slika = fopen("C:\\Temp\\test.bmp", "rb");
        FILE* novi = fopen("C:\\Temp\\testnovi2.bmp", "wb");
        char c;
        c = fgetc(slika);
        while (c != EOF)
        {
                fputc(c, novi);
                c = fgetc(slika);
        }
        fclose(slika);
        fwrite(image.header, sizeof(unsigned char), 54, novi);
        int i;
        unsigned char tmp;
        for (i = 0; i < image.size+(image.padding*image.height); i += 3)
        {
                if (i%image.width*3-1 == 0 && image.padding!=0 && i!=0)
                {
                        i = i + image.padding-3;
                }
                else
                {
                        tmp = image.pixels[i];
                        image.pixels[i] = image.pixels[i + 2];
                        image.pixels[i + 2] = tmp;
                }
        }
        fwrite(image.pixels, sizeof(unsigned char), image.size+image.padding*image.height, novi);
        fclose(novi);
}


int main(void)
{
        FILE* slika;
        int i = 0;
        char check[2];
        char slovo;
        slika = fopen("C:\\Temp\\test.bmp", "rb");
        if (slika == NULL)
        {
                fprintf(stderr, "\nNemoguce pristupiti podatku\n");
                system("pause");
                return -1;
        }


        rewind(slika);


        do
        {
                slovo = ((char)fgetc(slika));
                printf(" %c ", slovo);
                check[i] = slovo;


                i = i + 1;


        } while (i < 2);




        if (check[0] != 'B' || check[1] != 'M')
        {
                printf("Podatak nije bmp!");
                system("pause");
                return -1;
        }
        else
        {
                printf("Podatak je bmp!");
        }
        fclose(slika);


        BMP image = citanjeBMP();


        printf("\nVelicina slike je %d x %d", image.width, image.height);
        system("pause");




        BMP imageNovi = mirrorimage(image);
        upisivanjeBMP(imageNovi);




        return(0);
}

---

Issue #1, do not use the "new" keyword as the name of an instantiation. It is a reserved keyword.

Second, what is a pixel? It is a structure? If so, each pixel may already be indexed by 3 values. In other words, if pixels is a structure of 3 unsigned char's (or 3 floats or whatever the image format actually requires), then I'd expect image.pixels[9] to be the 10th pixel of the first row.

If i is 9, and I use i * 3 (27), i'd expect to be referencing pixel 28 of the first row, not pixel 9.

See if that seems right to you.

Quote:

---

Originally Posted by Niccolo

Issue #1, do not use the "new" keyword as the name of an instantiation. It is a reserved keyword.

---

This is the C programming forum, so that statement is not true, though it is still a valid recommendation if the code is meant to be compilable as C++.

It's not my original name for the variable i just changed it here so people may have an easier time reading my code, but yes it is a struct. I can post the entire code if someone wants to run it. and see any faults? I reserved the memory for pixels and included all colours and possible image padding. I have been trying to figure this out for days now, and just no luck at all.

Sure, if it fits, post. I'll find or make a bmp to test.

It just occurred to me, looking at the image, that the angle of the skew looks like the bmp may have an alpha, but you're assuming an RGB image (it might be RGBA).

@laserlight - I do have to remind myself of that, but indeed, "new" isn't a good idea even so.

Pixels have 3 colours RGB, so for every pixel you need 3 bytes. You need to cover every colour individually when replacing left and right pixels.

It shouldn't have RGBA, it is a 24 bit bmp, I would post the original image here, but it's to big for the forum so here is a link to it
bmp tiger image - Google Search

Yes, I needed to better verify the image format.

Using a couple of test images, what I can say is that your algorithm can work, but what appears to be the issue is the complex issues of interpreting the header and what that implies about the orientation of the image.

I'd recommend that instead of reading the header yourself that you consider a free library for reading the BMP format, so as to provide pixels to you. Just scanning through the documentation (which has some considerable irrelevant format information, like OS/2 bitmaps), there are a number of "gotchas" I think aren't handled here.

For example, a simple 24 bit bmp I created in photoshop, drawing a black square on a white background, did NOT skew the image...but the image was offset improperly (meaning part of the image rows on the left side of the image were off by 1 line).

What this tells me is that you'll need more information from the header (like the pixeloffset value, perhaps).

Further, the compiler code analysis warns that there are buffer overruns likely (there isn't quite enough in the buffer - because while you have the number of pixels accounted, there might not be sufficient for all pixeloffset and padding accounted.

This is why I suggest a library. If your purpose is to study and write such libraries, then I'm off target, but if you're just trying to write image convolution code (mirroring/brightness/contrast/ect)...use a library. Life will be much better that way.

I am doing this for a college project so I have to do it this way :/. Still out of 3 sites where I asked about this, you are the only one that had given me useful insight. And by your comment i see I don't quite understand the offset term. I thought it was just information on where the pixel array data starts. I put that in this line image.pixeloffset = *(int*)& image.header[10]; since i googled it is on place 10 of the header. The value this gave was 54 which is exactly the header size, so using it anywhere didn't make sense to me
since I would basically just start filling the new pixel array exactly after the header. Thanks for the help btw (: !

In citanjeBMP you return a variable declared locally

Bitmap File Format

You don't take into account padding on xPos and xEndPos.

Some thoughts.
1. Your functions are too long.
2. Your functions are working at too low a level.

All that *3, +4, % this, / that, +/- padding is clouding the issues.

You NEED abstraction.
If you're dealing with pixels, then read pixels. Fiddling with 3-byte blocks is just madness.
If it were any other image format apart from BMP, you would be doing this.

Your first step is making sure you can read an image in, then write it back out again

Code:

---

#include <stdio.h>
#include <stdlib.h>

typedef struct {
    unsigned char r;
    unsigned char g;
    unsigned char b;
} pixel;

typedef struct {
    int width;
    int height;
    //int pixeloffset;
    int padding;
    unsigned char header[54];
    //unsigned char* pixels;
    //int size;
    pixel **pixels;
}BMP;

void readHeader(FILE *fp, BMP *bmp) {
    fread(bmp->header, sizeof(bmp->header), 1, fp);
}

void writeHeader(FILE *fp, BMP *bmp) {
    fwrite(bmp->header, sizeof(bmp->header), 1, fp);
}

void decodeHeader(BMP *bmp) {
    bmp->width = *(int*)&bmp->header[18];
    bmp->height = *(int*)&bmp->header[22];
    //bmp->pixeloffset = *(int*)&bmp->header[10];
    bmp->padding = (bmp->width*3) % 4;
    //bmp->size = 3 * bmp->width * bmp->height;
    bmp->pixels = malloc( sizeof(*bmp->pixels) * bmp->height );
    for ( int i = 0 ; i < bmp->height ; i++ ) {
        bmp->pixels[i] = malloc( sizeof(*bmp->pixels[i]) * bmp->width );
    }
}

void cleanupHeader(BMP *bmp) {
    for ( int i = 0 ; i < bmp->height ; i++ ) {
        free(bmp->pixels[i]);
    }
    free(bmp->pixels);
}

pixel readPixel(FILE *fp) {
    pixel result;
    fread(&result, sizeof(result), 1, fp );
    return result;
}

void writePixel(FILE *fp, pixel p) {
    fwrite(&p, sizeof(p), 1, fp);
}

void readPadding(FILE *fp, int padding) {
    for ( int i = 0 ; i < padding ; i++ )
        fgetc(fp);  // we don't care, just burn them
}

void writePadding(FILE *fp, int padding) {
    for ( int i = 0 ; i < padding ; i++ )
        fputc(0,fp);  // we don't care, just write zeros
}

void readImage(FILE *fp, BMP *bmp) {
    for ( int r = 0 ; r < bmp->height ; r++ ) {
        for ( int c = 0 ; c < bmp->width ; c++ ) {
            bmp->pixels[r][c] = readPixel(fp);
        }
        readPadding(fp,bmp->padding);
    }
}

void writeImage(FILE *fp, BMP *bmp) {
    for ( int r = 0 ; r < bmp->height ; r++ ) {
        for ( int c = 0 ; c < bmp->width ; c++ ) {
            writePixel(fp,bmp->pixels[r][c]);
        }
        writePadding(fp,bmp->padding);
    }
}

int main ( ) {
    FILE *in = fopen("tiger.bmp","rb");
    FILE *out= fopen("new.bmp","wb");

    BMP bmp;
    readHeader(in,&bmp);
    decodeHeader(&bmp);
    readImage(in,&bmp);
    writeHeader(out,&bmp);
    writeImage(out,&bmp);
    cleanupHeader(&bmp);

    fclose(in);
    fclose(out);
}

---

With this, you should find writing mirrorImage a lot less stressful.