processing an audio wav file with C

Question

I'm working on processing the amplitude of a wav file and scaling it by some decimal factor. I'm trying to wrap my head around how to read and re-write the file in a memory-efficient way while also trying to tackle the nuances of the language (I'm new to C). The file can be in either an 8- or 16-bit format. The way I thought of doing this is by first reading the header data into some pre-defined struct, and then processing the actual data in a loop where I'll read a chunk of data into a buffer, do whatever is needed to it, and then write it to the output.

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


typedef struct header 
{
    char chunk_id[4];
    int chunk_size;
    char format[4];
    char subchunk1_id[4];
    int subchunk1_size;
    short int audio_format;
    short int num_channels;
    int sample_rate;
    int byte_rate;
    short int block_align;
    short int bits_per_sample;
    short int extra_param_size;
    char subchunk2_id[4];
    int subchunk2_size;
} header;

typedef struct header* header_p;

void scale_wav_file(char * input, float factor, int is_8bit)
{
    FILE * infile = fopen(input, "rb");
    FILE * outfile = fopen("outfile.wav", "wb");

    int BUFSIZE = 4000, i, MAX_8BIT_AMP = 255, MAX_16BIT_AMP = 32678;

    // used for processing 8-bit file
    unsigned char inbuff8[BUFSIZE], outbuff8[BUFSIZE];

    // used for processing 16-bit file
    short int inbuff16[BUFSIZE], outbuff16[BUFSIZE];

    // header_p points to a header struct that contains the file's metadata fields
    header_p meta = (header_p)malloc(sizeof(header));

    if (infile)
    {

        // read and write header data
        fread(meta, 1, sizeof(header), infile);
        fwrite(meta, 1, sizeof(meta), outfile);

        while (!feof(infile))
        {
            if (is_8bit)
            {
                fread(inbuff8, 1, BUFSIZE, infile);   
            } else {
                fread(inbuff16, 1, BUFSIZE, infile);      
            }

            // scale amplitude for 8/16 bits
            for (i=0; i < BUFSIZE; ++i)
            {
                if (is_8bit)
                {
                    outbuff8[i] = factor * inbuff8[i];
                    if ((int)outbuff8[i] > MAX_8BIT_AMP)
                    {
                        outbuff8[i] = MAX_8BIT_AMP;
                    }
                } else {
                    outbuff16[i] = factor * inbuff16[i];
                    if ((int)outbuff16[i] > MAX_16BIT_AMP)
                    {
                        outbuff16[i] = MAX_16BIT_AMP;
                    } else if ((int)outbuff16[i] < -MAX_16BIT_AMP) {
                        outbuff16[i] = -MAX_16BIT_AMP;
                    }
                }
            }

            // write to output file for 8/16 bit
            if (is_8bit)
            {
                fwrite(outbuff8, 1, BUFSIZE, outfile);
            } else {
                fwrite(outbuff16, 1, BUFSIZE, outfile);
            }
        }
    }

    // cleanup
    if (infile) { fclose(infile); }
    if (outfile) { fclose(outfile); }
    if (meta) { free(meta); }
}

int main (int argc, char const *argv[])
{
    char infile[] = "file.wav";
    float factor = 0.5;
    scale_wav_file(infile, factor, 0);
    return 0;
}

I'm getting differing file sizes at the end (by 1k or so, for a 40Mb file), and I suspect this is due to the fact that I'm writing an entire buffer to the output, even though the file may have terminated before filling the entire buffer size. Also, the output file is messed up - won't play or open - so I'm probably doing the whole thing wrong. Any tips on where I'm messing up will be great. Thanks!

Answer 1

1 You're reading bytes instead of 16-bit samples in this else branch:

while (!feof(infile))
    {
        if (is_8bit)
        {
            fread(inbuff8, 1, BUFSIZE, infile);   
        } else {
            fread(inbuff16, 1, BUFSIZE, infile); // <-- should be BUFSIZE*2     
        }

2 You don't saturate the values when scaling, e.g. original 16-bit sample = 32000 and factor = 1.5 will wrap around the integer value instead of clamping it to the maximum of 32767.

3 You don't look at the RIFF and other headers at all. In WAV files, it is possible that the audio data is followed by some informational footers or preceded by additional headers. Or in other words: Your header struct is too static. You should also read the WAV format from the file instead of having a parameter saying it's 8 bit samples.

4 This just won't happen:

                outbuff16[i] = factor * inbuff16[i];
                if ((int)outbuff16[i] > MAX_16BIT_AMP)

8-bit/16-bit values will never be greater than 255/32768 except if your computer inserts some magic bits into the memory when integers overflows :P

And audio samples are signed, so the ranges are -128;127 and -32768;32767. Overflow checking must occur in the multiplication expression. You're also making assumptions on the floating-point-to-integer rounding mode, which is configurable and should be considered. Something like if(roundf(factor * inbuff16[i]) > 32767 || roundf(factor * inbuff16[i]) < -32768), maybe.

5 You don't store the result of fread, so you will write too many samples to the output file.

6 And as a last point, you're reinventing the wheel. As long as this is for learning, it's okay. Else you should use existing libraries.