Mixing PCM samples - dealing with clicks & overflow

Hi all,

I'm trying to mix multiple sample buffers (16bit) together into a single output. I do this by using pretty much the following code..

Code:

---

#define BUFFER_LEN 256
#include "limits.h"

// stuff here......

short buffer[BUFFER_LEN];
short output[BUFFER_LEN];
long temp;


while (!done) {
    getsamples(buffer);
    for(int i=0; i<BUFFER_LEN; i++) {
        temp = long(output[i] += buffer[i]*0.5);
        if (temp > SHRT_MAX) temp=SHRT_MAX;
        else if (temp < SHRT_MIN) temp=SHRT_MIN;
        output[i] = temp;
    }

    // stuff ....

}

---

The audio does mix ok, however at points when there are a lot of channels being mixed it produces very noticeable 'blips and clicks' in the sound.

The "long temp" variable was an attempt to overcome this by preventing overflow of the 16 bit short's, but this didn't really do anything.

The only other method I know of is dividing by the number of channels, which I don't really want to do since the number of channels at any one time could quite easily go above 10 (depending on the input), and dividing would loose a lot of volume and therefor accuracy.

Has anyone got a better suggestion?

Thanks.

>> temp = long(output[i] += buffer[i]*0.5);

you're casting a result that is already (potentially) truncated. try instead:

Code:

---

temp = long(output[i]) + long(buffer[i] >> 1);

---

Code:

---

temp = long(output[i] += buffer[i]*0.5);
// can be re-written as:

output[i] += buffer[i] * 0.5;
// warning C4244: '+=' : conversion from 'double' to 'short', possible loss of data

temp = (long)output[i];
// warning C4244: '=' : conversion from 'long' to 'short', possible loss of data

---

Not good. You should be getting warnings as well. Treat them like errors. Don't try to make them "go away" by casting. You'll want to really understand why your compiler is warning you about so you can make the appropriate fix.

gg

Quote:

---

Originally Posted by Sebastiani

>> temp = long(output[i] += buffer[i]*0.5);

you're casting a result that is already (potentially) truncated. try instead:

Code:

---

temp = long(output[i]) + long(buffer[i] >> 1);

---

---

I see what your saying, but that code just produces a horrible noise.

Quote:

---

Originally Posted by Codeplug

Not good. You should be getting warnings as well. Treat them like errors. Don't try to make them "go away" by casting. You'll want to really understand why your compiler is warning you about so you can make the appropriate fix.

gg

---

I get those warnings but I ignore them. I am well aware of the loss of precision and under these circumstances it is acceptable. The most it can ever mis-calculate by is 0.5, which for a 16bit audio sample is completely in-audible.

I need to halve each sample value because otherwise the overflows and distortion become really bad. Reducing it by even more, say *0.3, get's rid of this problem but makes the output very quiet. Surely there is some technique that can be applied to keep volume while getting rid of the overflow blips.

>> but I ignore them
And here we are ;)

>> I am well aware of the loss of precision...
That was my first thought as well. Simple test case - two channels, A and B. A is silent and B is 75% volume - what should the result be?

I found this: http://www.vttoth.com/digimix.htm

gg

>> I see what your saying, but that code just produces a horrible noise.

I was addressing the correctness of the cast, not the mixing algorithm. I think the standard way is to halve both samples, right? eg:

Code:

---

temp = (int(output[i]) + int(buffer[i])) >> 1;

---

I changed the cast from long to int since the shift optimization results in an int.

Quote:

---

Originally Posted by Codeplug

I found this: http://www.vttoth.com/digimix.htm

gg

---

Thanks, that is EXACTLY what I was after :)

Only problem I have now is implementing it. The equation he gives is..

Z = 2(A+B) – A·B / 128 – 256

Where Z = resulting sample, A = channel 1, B = channel 2. Since he explain's the concept using 8bit samples and i'm using 16bit, I presume the 128 becomes 32767 and the 256 is 65535.

so my code is..
output[j] = (2*(output[j]+buffer[j])) - ((output[j]*buffer[j]) / 32767) - 65535;

..which compiles without warnings, but the resulting mix is hard to hear behind the cackle of distortion. It's not just random noise because you can hear the original sounds so at least it worked to some extent.

>> Since he explain's the concept using 8bit samples and i'm using 16bit, I presume the 128 becomes 32767 and the 256 is 65535.

but since 16 bit samples are signed you don't need to normalize the data. so using his original algorithm you would do:

Code:

---

int a = output[j], b = buffer[j];
output[j] = a + b - (a * b) - 65535;

---

Man I wish I could program like that and muck in on the convo, sadly I'm far behind :(

Quote:

---

Originally Posted by Sebastiani

>> Since he explain's the concept using 8bit samples and i'm using 16bit, I presume the 128 becomes 32767 and the 256 is 65535.

but since 16 bit samples are signed you don't need to normalize the data. so using his original algorithm you would do:

Code:

---

int a = output[j], b = buffer[j];
output[j] = a + b - (a * b) - 65535;

---

---

That works, thank you! :)
except instead of deducting by 65535 you divide, so it becomes..
output[j] = a + b - ((a * b) / 65535);

I still get a few clicks when new channels are introduced or cut off, but I think this can be solved by implementing an ADSR (Attack, Delay, Sustain, Release) envelope.

Thanks for your help

>> output[j] = a + b - ((a * b) / 65535);

in the original algorithm from that site he divides by 256, which means you probably want to divide by 65536 (not totally sure, though). but if so, you can optimize out the divide with a bit shift, eg:

Code:

---

output[j] = a + b - ((a * b) >> 0x10);

---

Quote:

---

Originally Posted by Sebastiani

>> output[j] = a + b - ((a * b) / 65535);

in the original algorithm from that site he divides by 256, which means you probably want to divide by 65536 (not totally sure, though). but if so, you can optimize out the divide with a bit shift, eg:

Code:

---

output[j] = a + b - ((a * b) >> 0x10);

---

---

Yes that works and will be much faster under heavy load I imagine, which is vital considering this will be a cgi app.