I see some ways in which this code might be improved.
printf format specifiers match the data
In the sample
main, the format specifier used is
bitmap_color_replace returns a
dword which is an alias for
unsigned long, so the correct format specifier would actually be
"%lu" for that data.
Omit return statement in
is not needed with modern C compilers. Getting to the end of
main implicitly means that the program has succeeded, so the compiler will include code to do that return if you simply omit it.
Rearrange the code to reduce the need for declarations
A function declaration is only needed by the compiler if it hasn't already encountered the function itself. With that said, if the
bitmap_color_replace function is moved to the top of the file, above
main, the function declaration within
main can be omitted, making
main a single
Omit unused variables
argv are unused, you could use the alternative form of
int main ()
Avoid hardcoding values
IMAGE_OFFSET_PIXELS are not valid for most images. Rather than hardcode those, you could simply read them from the file header in the same way that
fp_size is read.
There are a lot of
fseek calls within the current routine which are not necessary because the program moves linearly through the data anyway. Eliminating them would likely speed up the program.
Calculate pad size just once
The padding size is the same for every row so you can calulate it based on the image width and the size of each color.
Process only the pixel data
In some version of BMP files, there is data after the pixel data, so the current code, which continues to the end of the physical file, would overwrite that data. Better would be to calculate just the size of the pixel data which is obtained from the height and width of the image.
Don't overwrite the input file
Create a new file instead of overwriting the input file. This helps the user as well as preventing file corruption if something goes wrong during the processing of the file. It may also speed the program.
Process larger chunks at a time in memory
Instead of reading the colors byte at a time, it would likely speed things up considerably by processing entire rows at a time, or even the entire file as a memory image. I/O is often the bottleneck for programs like this, so minimizing it can result in considerable time savings.
Note the correct data order
BMP files contain pixel data as R,G,B in that order. It's important to get the order right to correctly match colors.
Here's a version of the code with some, but not all of these ideas incorporated:
// these are absolute offsets to the two most important
// items in the BMP file
#define IMAGE_PIXEL_START 10
#define IMAGE_WIDTH 18
typedef unsigned char byte;
typedef unsigned long dword;
(char *file_name, byte old_r, byte old_g, byte old_b, byte new_r, byte new_g, byte new_b)
FILE* fp = fopen(file_name, "r+b");
dword pixels_replaced = 0;
new_color = new_r;
new_color = new_g;
new_color = new_b;
fseek(fp, IMAGE_PIXEL_START, SEEK_SET);
dword fp_pixelstart = fgetc(fp) + fgetc(fp)*256 + fgetc(fp)*(256*256) + fgetc(fp)*(256*256*256);
fseek(fp, IMAGE_WIDTH, SEEK_SET);
dword fp_width = fgetc(fp) + fgetc(fp)*256 + fgetc(fp)*(256*256) + fgetc(fp)*(256*256*256);
dword fp_height = fgetc(fp) + fgetc(fp)*256 + fgetc(fp)*(256*256) + fgetc(fp)*(256*256*256);
// rows are padded to a multiple of 4 bytes
dword padsize = (4 - ( (sizeof(new_color) * fp_width) & 0x3 )) & 0x3;
fseek(fp, fp_pixelstart, SEEK_SET);
for (int row = fp_height; row; --row)
for (int col = fp_width; col; --col)
fread(old_color, sizeof(old_color), 1, fp);
if( old_color == old_r
&& old_color == old_g
&& old_color == old_b)
fseek(fp, -sizeof(old_color), SEEK_CUR);
fwrite(new_color, sizeof(new_color), 1, fp);
fseek(fp, padsize, SEEK_CUR);