Resizing BMP Files

Hey guys,

I'm currently studying a CS course, and I have a problem set I can't seem to solve, we're required to make a small program that takes 3 arguments: (factor, file.bmp, file2.bmp) and resizes the first bmp by factor then stores it in the second file.

I've supplied my code along with the output of a sample run in an attachment.

This is a header file supplied by our instructor

bmp.h

Code:

---

#include <stdint.h>
/**
 * Common Data Types
 *
 * The data types in this section are essentially aliases for C/C++
 * primitive data types.
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/cc230309(PROT.10).aspx.
 * See http://en.wikipedia.org/wiki/Stdint.h for more on stdint.h.
 */
typedef uint8_t  BYTE;
typedef uint32_t DWORD;
typedef int32_t  LONG;
typedef uint16_t WORD;

#define HEADER 54  // number of bytes offset for header

/**
 * BITMAPFILEHEADER
 *
 * The BITMAPFILEHEADER structure contains information about the type, size,
 * and layout of a file that contains a DIB [device-independent bitmap].
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/dd183374(VS.85).aspx.
 */
typedef struct
{
    WORD  bfType;
    DWORD  bfSize;
    WORD  bfReserved1;
    WORD  bfReserved2;
    DWORD  bfOffBits;
} __attribute__((__packed__))
BITMAPFILEHEADER;

/**
 * BITMAPINFOHEADER
 *
 * The BITMAPINFOHEADER structure contains information about the
 * dimensions and color format of a DIB [device-independent bitmap].
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/dd183376(VS.85).aspx.
 */
typedef struct
{
    DWORD  biSize;
    LONG  biWidth;
    LONG  biHeight;
    WORD  biPlanes;
    WORD  biBitCount;
    DWORD  biCompression;
    DWORD  biSizeImage;
    LONG  biXPelsPerMeter;
    LONG  biYPelsPerMeter;
    DWORD  biClrUsed;
    DWORD  biClrImportant;
} __attribute__((__packed__))
BITMAPINFOHEADER;

/**
 * RGBTRIPLE
 *
 * This structure describes a color consisting of relative intensities of
 * red, green, and blue.
 *
 * Adapted from http://msdn.microsoft.com/en-us/library/aa922590.aspx.
 */
typedef struct
{
    BYTE  rgbtBlue;
    BYTE  rgbtGreen;
    BYTE  rgbtRed;
} __attribute__((__packed__))
RGBTRIPLE;

---

Now here's my code resize.c

Code:

---

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

#include "bmp.h"

int main(int argc, char* argv[])
{
    // ensure proper usage
    if (argc != 4)
    {
        printf("Usage: resize factor infile outfile\n");
        return 1;
    }

    // remember arguments
    int factor = atoi(argv[1]);
    char* infile = argv[2];
    char* outfile = argv[3];

    // open input & output files
    FILE* inptr = fopen(infile, "r");
    if (inptr == NULL)
    {
        printf("Could not open %s.\n", infile);
        return 2;
    }

    FILE* outptr = fopen(outfile, "w");
    if (outptr == NULL)
    {
        fclose(inptr);
        fprintf(stderr, "Could not create %s.\n", outfile);
        return 3;
    }
   
    if ((factor <= 0) || (factor > 100))
    {
        fprintf(stderr, "Factor must be in 1 - 100 range\n");
        return 4;
    }
   
    // read infile's BITMAPFILEHEADER
    BITMAPFILEHEADER bf;
    fread(&bf, sizeof(BITMAPFILEHEADER), 1, inptr);

    // read infile's BITMAPINFOHEADER
    BITMAPINFOHEADER bi;
    fread(&bi, sizeof(BITMAPINFOHEADER), 1, inptr);
   
    BITMAPFILEHEADER bfo = bf;
    BITMAPINFOHEADER bio = bi;
   
    if (factor == 1)
    {
        bfo = bf;
        bio = bi;
    }
    else
    {
        bfo = bf;
        bio = bi;
       
        bio.biWidth = bi.biWidth * factor;
        bio.biHeight = bi.biHeight * factor;
        int padding =  (4 - (bio.biWidth * sizeof(RGBTRIPLE)) % 4) % 4;
        bio.biSizeImage = (bio.biWidth * abs(bio.biHeight) * 3) + (padding * bio.biWidth);
        bfo.bfSize = bio.biSizeImage + HEADER;
    }
   

    // ensure infile is (likely) a 24-bit uncompressed BMP 4.0
    if (bf.bfType != 0x4d42 || bf.bfOffBits != 54 || bi.biSize != 40 ||
        bi.biBitCount != 24 || bi.biCompression != 0)
    {
        fclose(outptr);
        fclose(inptr);
        fprintf(stderr, "Unsupported file format.\n");
        return 5;
    }

    // write outfile's BITMAPFILEHEADER
    fwrite(&bfo, sizeof(BITMAPFILEHEADER), 1, outptr);

    // write outfile's BITMAPINFOHEADER
    fwrite(&bio, sizeof(BITMAPINFOHEADER), 1, outptr);
   
    // if factor == 1 just copy file
    if (factor == 1)
    {
        // determine padding for scanlines
        int padding =  (4 - (bi.biWidth * sizeof(RGBTRIPLE)) % 4) % 4;

        // iterate over infile's scanlines
        for (int i = 0, biHeight = abs(bi.biHeight); i < biHeight; i++)
        {
            // iterate over pixels in scanline
            for (int j = 0; j < bi.biWidth; j++)
            {
                // temporary storage
                RGBTRIPLE triple;

                // read RGB triple from infile
                fread(&triple, sizeof(RGBTRIPLE), 1, inptr);

                // write RGB triple to outfile
                fwrite(&triple, sizeof(RGBTRIPLE), 1, outptr);
            }

            // skip over padding, if any
            fseek(inptr, padding, SEEK_CUR);

            // then add it back (to demonstrate how)
            for (int k = 0; k < padding; k++)
                fputc(0x00, outptr);
        }
     
        fclose(inptr);
        fclose(outptr);
        return 0;
    }
    else
    {
        int paddingIn =  (4 - (bi.biWidth * sizeof(RGBTRIPLE)) % 4) % 4;
        int paddingOut =  (4 - (bio.biWidth * sizeof(RGBTRIPLE)) % 4) % 4;
       
        for (int i = 0, biHeight = abs(bi.biHeight); i < biHeight; i++)
        {
            RGBTRIPLE* buffer = malloc(bi.biWidth * sizeof(RGBTRIPLE));
            RGBTRIPLE temp;
           
            for (int j = 0; j < bi.biWidth; j++)
            {
                fread(&temp, sizeof(RGBTRIPLE), 1, inptr);
                buffer[j] = temp;
            }
           
            for (int k = 0; k < bi.biWidth; k++)
                for (int l = 0; l < factor; l++)
                    fwrite(&buffer[k], sizeof(RGBTRIPLE), 1, outptr);
                   
            for (int k = 0; k < paddingOut; k++)
                fputc(0x00, outptr);
               
            fseek(inptr, paddingIn, SEEK_CUR);   
            free(buffer);
        }
    }

   
    fclose(inptr);
    fclose(outptr);
    return 0;
}

---

Here's the output of a sample run with resizing factor of 2: http://www.putlocker.com/file/6919273A6F21E25D


I'm almost sure the problem is with my looping constructs, and I've been using xxd and gdb for the last 3 hours trying to figure out what's wrong to no avail, any help will be appreciated... thanks.

Quote:

---

Originally Posted by VeNiX

I'm almost sure the problem is with my looping constructs, and I've been using xxd and gdb for the last 3 hours trying to figure out what's wrong to no avail, any help will be appreciated... thanks.

---

Did you write out the algorithm on paper before you started coding? Did you check if it works manually with a small example?
Why don't you write some pseudocode to explain what you think the algorithm should do.

Quote:

---

Originally Posted by D_Drmmr

Did you write out the algorithm on paper before you started coding? Did you check if it works manually with a small example?
Why don't you write some pseudocode to explain what you think the algorithm should do.

---

I did have some pseudocode, and it changed as I started coding, right now it goes like this:

Code:

---


loop over height of original bmp
  loop over width of original bmp
      store each pixel (RGBTRIPLE, as in blue, green red) in buffer

  loop over width of original bmp
      print each pixel in buffer factor times

  add padding

  free buffer and repeat

---

Well, now that I look at it, I think the problem is in the padding, it isn't written to the file after every scanline but only after all scanlines are done.... it should go inside the loop I think, I'll modify the code and see what happens now.

Well, now the new bmp file is even more messy than before... I'm really confused because I think I've translated my pseudocode correctly, maybe I need to approach the problem differently.

Quote:

---

Originally Posted by VeNiX

I did have some pseudocode, and it changed as I started coding, right now it goes like this:

Code:

---


loop over height of original bmp
  loop over width of original bmp
      store each pixel (RGBTRIPLE, as in blue, green red) in buffer

  loop over width of original bmp
      print each pixel in buffer factor times

  add padding

  free buffer and repeat

---

Well, now that I look at it, I think the problem is in the padding, it isn't written to the file after every scanline but only after all scanlines are done.... it should go inside the loop I think, I'll modify the code and see what happens now.

---

Yes, that's one problem. The other is that you only create one row for each row in the original image. So your height will always stay the same, regardless of the value of 'factor'.
Like I said, first try it out on paper. Remember that every pixel in the original image will become 'factor' x 'factor' pixels in the resized image.

Quote:

---

Originally Posted by D_Drmmr

Yes, that's one problem. The other is that you only create one row for each row in the original image. So your height will always stay the same, regardless of the value of 'factor'.
Like I said, first try it out on paper. Remember that every pixel in the original image will become 'factor' x 'factor' pixels in the resized image.

---

Isn't that a job for the header file to handle? essentially since I say that width is say 6 pixels, the renderer will be expecting a width that is a multiple of 4, so it expects 6 pixels plus 2 pixels of padding, after which it starts rendering the next row anyway... and since I'm already printing the pixels factor * factor of times it should work.

But now that you've mentioned it, I'm treating the bmp file as just a sequence of bytes, is there something else that is required of me to mark rows for instance?

Thanks greatly for your help.

Quote:

---

Originally Posted by VeNiX

Isn't that a job for the header file to handle? essentially since I say that width is say 6 pixels, the renderer will be expecting a width that is a multiple of 4, so it expects 6 pixels plus 2 pixels of padding, after which it starts rendering the next row anyway... and since I'm already printing the pixels factor * factor of times it should work.

---

You are not printing pixels factor * factor times. That's why your algorithm doesn't work.
You essentially have:

Code:

---

        for (int i = 0, biHeight = abs(bi.biHeight); i < biHeight; i++)
        {
            //...
            for (int k = 0; k < bi.biWidth; k++)
                for (int l = 0; l < factor; l++)
                    fwrite(&buffer[k], sizeof(RGBTRIPLE), 1, outptr);

---

So horizontally you print each pixel of the original image 'factor' times, but not vertically.

Maybe you should take a simpler example.

Code:

---

int original[3][3] = { { 1, 2, 3 }, { 4, 5, 6 }, { 7, 8, 9 } };
const int factor = 2;
int resized[3 * factor][3 * factor] = { 0 };

---

Copy the values from original to resized in the same way you would do it for an image. You can easily check your results by printing the matrix to the screen.
There are no bytes or padding to worry about here; just the basic algorithm that you need.

Much thanks for the help... I've managed to tweak my algorithm to get it right.

Code:

---

   
    else
    {
        int paddingIn =  (4 - (bi.biWidth * sizeof(RGBTRIPLE)) % 4) % 4;
        int paddingOut =  (4 - (bio.biWidth * sizeof(RGBTRIPLE)) % 4) % 4;
       
        for (int i = 0, biHeight = abs(bi.biHeight); i < biHeight; i++)
        {
            RGBTRIPLE* buffer = malloc(bi.biWidth * sizeof(RGBTRIPLE));
            RGBTRIPLE temp;
           
            for (int j = 0; j < bi.biWidth; j++)
            {
                fread(&temp, sizeof(RGBTRIPLE), 1, inptr);
                buffer[j] = temp;
            }
           
            for (int k = 0; k < factor; k++)
            {
                int iterator = 0;
               
                for (int l = 0; l < bi.biWidth; l++)
                {
                    for(int m = 0; m < factor; m++)
                    {
                        fwrite(&buffer[iterator], sizeof(RGBTRIPLE), 1, outptr);
                    }
                   
                    iterator += 1 % bi.biWidth;
                }
               
                for (int k = 0; k < paddingOut; k++)
                    fputc(0x00, outptr);
            }
               
            fseek(inptr, paddingIn, SEEK_CUR);   
            free(buffer);
        }

---

Now that I've actually fixed the algorithm I can throw the whole if-else construct away since it handles a factor of 1 correctly too.

Code:

---

            RGBTRIPLE* buffer = malloc(bi.biWidth * sizeof(RGBTRIPLE));
            RGBTRIPLE temp;
           
            for (int j = 0; j < bi.biWidth; j++)
            {
                fread(&temp, sizeof(RGBTRIPLE), 1, inptr);
                buffer[j] = temp;
            }

---

You don't need the for-loop here. You can use a single call to fread that reads the entire row.

Code:

---

                int iterator = 0;
                for (int l = 0; l < bi.biWidth; l++)
                {
                    for(int m = 0; m < factor; m++)
                    {
                        fwrite(&buffer[iterator], sizeof(RGBTRIPLE), 1, outptr);
                    }
                   
                    iterator += 1 % bi.biWidth;
                }

---

Same here; instead of calling fwrite 'factor' times, you can just call it once.
Also, have a good look at what the value of 'iterator' really does here. You can make your code simpler.

Code:

---

            fseek(inptr, paddingIn, SEEK_CUR);   
            free(buffer);
        }

---

What is the fseek for here? After you've written a row (including the padding), shouldn't you just continue writing the next row?

Quote:

---

Originally Posted by D_Drmmr

Code:

---

            RGBTRIPLE* buffer = malloc(bi.biWidth * sizeof(RGBTRIPLE));
            RGBTRIPLE temp;
           
            for (int j = 0; j < bi.biWidth; j++)
            {
                fread(&temp, sizeof(RGBTRIPLE), 1, inptr);
                buffer[j] = temp;
            }

---

You don't need the for-loop here. You can use a single call to fread that reads the entire row.

Code:

---

                int iterator = 0;
                for (int l = 0; l < bi.biWidth; l++)
                {
                    for(int m = 0; m < factor; m++)
                    {
                        fwrite(&buffer[iterator], sizeof(RGBTRIPLE), 1, outptr);
                    }
                   
                    iterator += 1 % bi.biWidth;
                }

---

Same here; instead of calling fwrite 'factor' times, you can just call it once.
Also, have a good look at what the value of 'iterator' really does here. You can make your code simpler.

---

Many thanks for the insights, I've understood what you meant about fread() and iterator and adjusted my code accordingly:

Code:

---

    // loop over input file's scanlines   
    for (int i = 0, biHeight = abs(bi.biHeight); i < biHeight; i++)
    {
        // make buffer & store input file's row worth of pixels
        RGBTRIPLE* buffer = malloc(bi.biWidth * sizeof(RGBTRIPLE));
        fread(buffer, sizeof(RGBTRIPLE), bi.biWidth, inptr);
 
        // print every pixel in buffer factor*factor times
        for (int k = 0; k < factor; k++)
        {
            for (int l = 0; l < bi.biWidth; l++)
            {
                for(int m = 0; m < factor; m++)
                    fwrite(&buffer[l], sizeof(RGBTRIPLE), 1, outptr);
            }
           
            // print required padding   
            for (int k = 0; k < paddingOut; k++)
                fputc(0x00, outptr);
        }

---

So I replaced the fread() loop with a single line reading width worth of pixels into the buffer, and I removed the iterator since it's not needed and replaced it with the original loop's count variable.

But I don't see how can I remove the looping construct around fwrite() and replace it with a single line, according to the fwrite() manual page:

Code:

---

size_t fwrite(const void *ptr, size_t size, size_t num, FILE *stream);

---

The function fwrite() writes num elements of data, each size bytes long, to the stream pointed to by stream, obtaining them from the location given by ptr.

And while fread() works the same, it worked because I wanted to access each consecutive pixel once, can you give me a hint as to what's the trick here, what am I missing?

Quote:

---

Originally Posted by D_Drmmr

Code:

---

            fseek(inptr, paddingIn, SEEK_CUR);   
            free(buffer);
        }

---

What is the fseek for here? After you've written a row (including the padding), shouldn't you just continue writing the next row?

---

fseek() here just skips over padding in the input file, so that none of it is stored in the buffer.

Quote:

---

Originally Posted by VeNiX

But I don't see how can I remove the looping construct around fwrite() and replace it with a single line, according to the fwrite() manual page:

Code:

---

size_t fwrite(const void *ptr, size_t size, size_t num, FILE *stream);

---

The function fwrite() writes num elements of data, each size bytes long, to the stream pointed to by stream, obtaining them from the location given by ptr.

And while fread() works the same, it worked because I wanted to access each consecutive pixel once, can you give me a hint as to what's the trick here, what am I missing?

---

You're right, the loop is necessary, because you write the same value multiple times.

Note that you can move the allocation/deallocation for the buffer outside the outermost loop, since the size of the buffer will be the same in each iteration. This will result in less calls to malloc/free, which is faster (unless the compiler already optimized this).

Done, I can probably check if the compiler optimized this already by looking at buffer's memory address, no?