1

Im getting distortion/weird noises when I implement Coefficients Created with a FIR Digital Filter designer. I have each coefficient Multiplied then add the next one to each sample. What am I doing wrong?

Here's is my callback code.

    #define FILTER_LEN 80
double coeffs[FILTER_LEN] = {
    0.4125 ,
    0.30599221057325454 ,
    0.08276044954686006 ,
    -0.0712490359027435 ,
    -0.06955311277904458 ,
    0.012051224652113425 ,
    0.05063742923584152 ,
    0.014831744868668527 ,
    -0.029779626539569115 ,
    -0.025159694030674883 ,
    0.010775174598886429 ,
    0.024755079985752222 ,
    0.0034713457185069014 ,
    -0.01801104775186626 ,
    -0.011545826635692912 ,
    0.008870916828741674 ,
    0.013655646830993191 ,
    -5.071102664251368E-4 ,
    -0.011300641103149761 ,
    -0.00510565075476355 ,
    0.006635644627205735 ,
    0.007342041186401136 ,
    -0.0017580021987183615 ,
    -0.006718478769227455 ,
    -0.001849971567154631 ,
    0.004418212284366871 ,
    0.003574469697178196 ,
    -0.0017346630584036216 ,
    -0.0035938670191588205 ,
    -3.7421959758368194E-4 ,
    0.0025646904655304632 ,
    0.0014944285606302294 ,
    -0.0012317293951690197 ,
    -0.0017072619265551775 ,
    1.2436035789109519E-4 ,
    0.0013643560681614716 ,
    5.61402657685598E-4 ,
    -8.232131487266428E-4 ,
    -8.902383271571432E-4 ,
    2.7287466296042835E-4
};


OSStatus MusicPlayerCallback (
                          void *                            inRefCon,
                          AudioUnitRenderActionFlags *  ioActionFlags,
                          const AudioTimeStamp *            inTimeStamp,
                          UInt32                            inBusNumber,
                          UInt32                            inNumberFrames,
                          AudioBufferList *             ioData) {


MusicPlaybackState *musicPlaybackState = (MusicPlaybackState*) inRefCon;


double sampleinp;

double acc = 0;



    for (int i = 0 ; i < ioData->mNumberBuffers; i++){


        AudioBuffer buffer = ioData->mBuffers[i];
        SInt16 *outSampleBuffer = buffer.mData;

        for (int j = 0; j < inNumberFrames*2; j++){

            //Left Channel
            sampleinp = musicPlaybackState->samplePtr[packetIndex++];


            //For each sample add filter
            for (int k=0; k < FILTER_LEN ; k++) {
                acc += coeffs[k] * sampleinp;

            }

            outSampleBuffer[j]  = MAX(MIN(acc,32767.0),-32768.0);


            j++;

            //Right Channel
            sampleinp = musicPlaybackState->samplePtr[packetIndex++];

            //For each sample add filter
            for (int k=0; k < FILTER_LEN ; k++) {
                acc += coeffs[k] * sampleinp;

            }

             outSampleBuffer[j]=MAX(MIN(acc,32767.0),-32768.0);


        }} 
}
Sebastian
  • 7,670
  • 5
  • 38
  • 50
Cocell
  • 159
  • 2
  • 10
  • Not that I was any good in signal processing, but from what I see on [wikipedia](http://en.wikipedia.org/wiki/Finite_impulse_response) and in a simple [stanford](https://ccrma.stanford.edu/~jos/filters/FIR_Software_Implementations.html) implementation, you are supposed to include the original signal in your sum. – Till Mar 12 '13 at 03:04
  • sampleinp is the samples, so are you saying that I need a History of it then include it in the sum? So then I will end up with acc += coeffs[k] * sampleinp + (sampleinp - 1); ?? – Cocell Mar 12 '13 at 03:31
  • Actually, my comment makes no sense and is plain wrong - I am sorry. – Till Mar 12 '13 at 03:42
  • How about this solution: http://stackoverflow.com/a/14998159/91282 – Till Mar 12 '13 at 03:52
  • I tried the solution in the link but im getting the same results. I think solution is some what similar to what I have.. – Cocell Mar 12 '13 at 17:00

1 Answers1

1

I may be misreading your code, so I'm going to simplify it. Correct me if I'm wrong:

//  incorrect
double out = 0;
double in = ....
for( int i=0; i<FILTER_LEN; ++i ) {
   out += in * coeff[i];
}
//bound out

This isn't right at all -- if you do a little factoring, you'll see that this is equivilant to just multiplying by a constant (the sum of your coeffs). An FIR filter is actually a convolution operation. To perform this, you need to store your old inputs.

//  correct (or close to it)
double out = 0;
double in = ....
//shift stored inputs
for( int i=FILTER_LEN-2; i>=0; --i ) {
   inarr[i+1] = inarr[i];
}
// add new input to the mix
inarr[0] = in;
// perform convolution
for( int i=0; i<FILTER_LEN; ++i ) {
   out += inarr[i] * coeff[i];
}
//bound out

That code is untested, I may have reversed some indexes here or there, but you get the idea.

Bjorn Roche
  • 11,279
  • 6
  • 36
  • 58
  • Thanx.. I may be doing something wrong but I still end up with the same distortions after trying out your method. It just stores the current sample on inarr[0] and then adds the previous sample to inarr[1] thru [79] so its not really shifting. Why is this so easy but hard to implement? Errr!! – Cocell Mar 12 '13 at 16:58
  • I fixed an index in the shift for loop. Maybe that's the issue. – Bjorn Roche Mar 12 '13 at 20:06
  • Anyway, do some printfs to make sure the input is propagating correctly through the inarr, and if not my apologies. – Bjorn Roche Mar 12 '13 at 20:09