views:

840

answers:

3

Hi - 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!

+1  A: 

I would recommend looking at the original file and the output file in a hex editor to see if you are re-writing the data properly. If the resulting file won't play or open, chances are the header of the output file isn't correct.

Another option is to remove your audio processing logic and simply read in the source file to your internal buffer and write it out to a file. If your code can generate a valid, working output file in this manner, then you can narrow down the problem to your processing code.

You may also want to start with a smaller file than 40Mb. If nothing else, make a copy of that input file and trim it down to a couple of seconds of audio. A smaller file will be easier to manually inspect.

Edit: The calls to fread() and fwrite() need to have their return values verified. These functions return the number of elements read or written, and if a call to either function returns a value less than expected then this could be the source of your file size difference.

Also, the second parameter to fread is in bytes. Therefore, if you want to read-fill an entire buffer, you would need to say something more like fread(inbuff16, sizeof(inbuff16[0]), BUFSIZE, infile);. The current code will only read in BUFSIZE bytes (which works for the 8-bit case by coincidence, but I would recommend changing it too for clarity).

bta
you're right about the file size, it's definitely worth shrinking just to see if it works
sa125
+5  A: 

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.

AndiDog
I think you caught most things - I would add that sizeof(meta) is wrong when writing out the header as meta is a pointer - needs to be sizeof(header) or sizeof(*meta).
Dipstick
that's great feedback, I'll try these things out
sa125
re: item 3, you also can't make assumptions based on inspecting the output of your favourite wave editor, as they all have quirks. Apparently much of the work in libsndfile involved working around oddities in various wave editors. If you are comfortable with the libsndfile license, all this reduces to a few sf_read_floats and sf_write_floats with some initializing stuff thrown in.
kibibu
+1  A: 

It is much better to use libraries for reading and writing sound files. E.g. libsndfile. That web page has a list of "other similar projects" you can also look at. The sndfile-tools could be good code examples to learn how to use the library.

Craig McQueen