HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Turning music into an image

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
musicintoturningimage

Problem

I was bored, so I wrote something that could turn .wav files in to .bmps:

#include 
char bmp[12000029]; // Need 12000028 bits, 1 pixel is 3
                    // bits and table is 28 (1500^2 * 3 + 28)

int main() {

  bmp[0]  = 0x42;   // Bitmap signature
  bmp[1]  = 0x4D;   // 'BM'

  bmp[2]  = 0x1C;   //
  bmp[3]  = 0x1B;   // Size of bitmap
  bmp[4]  = 0xB7;   // in bytes
  bmp[5]  = 0x00;   //

  bmp[10] = 0x1A;   // Image data location

  bmp[14] = 0x0C;   // Size of header

  bmp[18] = 0xDC;   //
  bmp[19] = 0x05;   // Dimensions of image
  bmp[20] = 0xDC;   // 1500 * 1500
  bmp[21] = 0x05;   //

  bmp[22] = 0x01;   // Something
  bmp[24] = 0x18;   // standard (?)

  FILE *fp;
  fp = fopen("file.wav", "rb");
  if (fp == NULL) {
    printf("Error opening 'file.wav'\n");
    return 1;
  }
  for (int i = 41; i < 12000043; i++) {     // Actual music data starts at 41
    bmp[i - 15] = fgetc(fp);                // Store first data to bmp[26] (41 - 15)
  }
  fclose(fp);

  fp = fopen("file.bmp", "w+");
  for (int i = i; i < 12000028; i++) {
    fputc(bmp[i], fp);
  }
  fclose(fp);

return 0;
}


I know this is probably the most random thing ever, but it is quite intriguing in my opinion.

(You just got Rickroll'd).

Solution

Strange loop initialization

Hm, what's up with this loop initialization?

for (int i = i; i < 12000028; i++) {


i is not even defined at this point. Write it the non-confusing way:

for (int i = 0; i < 12000028; i++) {


Magic number 12000028

The number 12000028 and 12000028 + 1 and 12000028 + 15 appears in multiple places in the code. It would be better to #define and reuse.

#define BMP_SIZE 12000028

char bmp[BMP_SIZE]; // Need 12000028 bits, 1 pixel is 3
// ...

int main() {

    // ...

    for (int i = 26; i < BMP_SIZE; i++) {
        bmp[i] = fgetc(fp);
    }

    // ...

    for (int i = 0; i < BMP_SIZE; i++) {
        fputc(bmp[i], fp);
    }

    // ...
}


Error checking

You checked nicely the result of opening the input file file.wav.
But then, why didn't you do the same for the output file file.bmp?

Use puts instead of printf when possible

When you print plain text ending with newline and you don't need to print variables, then puts is better than printf.
So instead of:

printf("Error opening 'file.wav'\n");


This is slightly better:

puts("Error opening 'file.wav'");


Usability

The input and output filenames are hardcoded.
That makes this difficult to use.
It would be great to take them as command line arguments

Suggested implementation

Applying the above suggestions:

#include 
#define BMP_SIZE 12000028

char bmp[BMP_SIZE]; // Need 12000028 bytes, 1 pixel is 3
// bits and table is 28 (1500^2 * 3 + 28)

int main(int argc, char ** argv) {

    bmp[0]  = 0x42;   // Bitmap signature
    bmp[1]  = 0x4D;   // 'BM'

    bmp[2]  = 0x1C;   //
    bmp[3]  = 0x1B;   // Size of bitmap
    bmp[4]  = 0xB7;   // in bytes
    bmp[5]  = 0x00;   //

    bmp[10] = 0x1A;   // Image data location

    bmp[14] = 0x0C;   // Size of header

    bmp[18] = 0xDC;   //
    bmp[19] = 0x05;   // Dimensions of image
    bmp[20] = 0xDC;   // 1500 * 1500
    bmp[21] = 0x05;   //

    bmp[22] = 0x01;   // Something
    bmp[24] = 0x18;   // standard (?)

    if (argc != 3) {
        printf("usage: %s input.wav output.bmp\n", argv[0]);
        return 2;
    }
    char * input = argv[1];
    char * output = argv[2];

    FILE *fp;
    fp = fopen(input, "rb");
    if (fp == NULL) {
        printf("Error opening %s for reading", input);
        return 1;
    }
    for (int i = 26; i < BMP_SIZE; i++) {
        bmp[i] = fgetc(fp);
    }
    fclose(fp);

    fp = fopen(output, "w+");
    if (fp == NULL) {
        printf("Error opening %s for writing", output);
        return 1;
    }
    for (int i = 0; i < BMP_SIZE; i++) {
        fputc(bmp[i], fp);
    }
    fclose(fp);

    return 0;
}

Code Snippets

for (int i = i; i < 12000028; i++) {
for (int i = 0; i < 12000028; i++) {
#define BMP_SIZE 12000028

char bmp[BMP_SIZE]; // Need 12000028 bits, 1 pixel is 3
// ...

int main() {

    // ...

    for (int i = 26; i < BMP_SIZE; i++) {
        bmp[i] = fgetc(fp);
    }

    // ...

    for (int i = 0; i < BMP_SIZE; i++) {
        fputc(bmp[i], fp);
    }

    // ...
}
printf("Error opening 'file.wav'\n");
puts("Error opening 'file.wav'");

Context

StackExchange Code Review Q#109456, answer score: 11

Revisions (0)

No revisions yet.