Onset detection using FFT

I am using this onset source code and this fft file to try to perform onset detection. Currently the code is working, but as my DSP and C skills are subpar I could use some advice on how to improve it.

My main points of insecurity are the FFT (maybe there is a more suitable method out there?) and the C code. All advice appreciated!

NB: This code will eventually being going into iPhone project.

OnsetsDS  *ods = malloc(sizeof *ods); 
float* odsdata = (float*) malloc(onsetsds_memneeded(ODS_ODF_RCOMPLEX, 512, 11));
onsetsds_init(ods, odsdata, ODS_FFT_FFTW3_HC, ODS_ODF_RCOMPLEX, 512, 11, 44100);

int i;
int x;
double (*vocalData)[2] =  malloc(2 * 512 * sizeof(double));
double (*doubleFFTData)[2] =  malloc(2 * 512 * sizeof(double));

for (i = 0; i  %i\n", i);
    }
}

free(ods->data); // Or free(odsdata), they point to the same thing in this case
return nil;

The link in the post to your fft file is broken, so I can't see the function
prototype. This is a pity, because it is otherwise difficult to know what to
make of your arrays:

double (*vocalData)[2] =  malloc(2 * 512 * sizeof(double));

This says vocalData is a pointer to an array of two doubles. You have
allocated 1024 doubles at this address. So on a 64bit system you have:

8 bytes
vocalData[0] ->  |--------|
                 |--------|
vocalData[1] ->  |--------|
                 |--------|
vocalData[2] ->  |--------|
                 |--------|
    etc

You fill the array with

for (x = 0; x < 512; x++){
    *vocalData[x] = (double)vocalBuffer.buffer[i+x];
}

This fills alternate doubles in your allocated space and leaves the others
uninitialized.

8 bytes
vocalData[0] ->  |xxxxxxxx|
                 |--------|
vocalData[1] ->  |xxxxxxxx|
                 |--------|
vocalData[2] ->  |xxxxxxxx|
                 |--------|
    etc

I don't know for sure that this is not what you want, but...

Also all that converting from float to double and then back to float is very
inefficient. As onsetsds_process takes floats, I can see the need, but
maybe you should use an FFT function that also deals in floats.

Finally it would be better to #define FFT_SIZE 512 at the top and use
FFT_SIZE throughout instead of the explicit 512. This is better practice,
for example making it much easier to change the FFT length if it is necessary.

Oh and don't forget to free any memory you have allocated once you have
finished with it. In this case, that means all of your allocations should be
freed on exit (although if your return 0 is the end of the program that is
academic).

StackExchange Code Review Q#12331, answer score: 3