Some things to check -

Your 16-bit PCM may be [-2^15, 2^15-1] rather than [0, 2^16-1], if so,
use signed, not unsigned, with appropriate changes. (You want to scale
relative to 0, not relative to the most negative value.)

How is 0xFFFF being interpreted by your compiler?  If it is considered
16-bit signed, then it's decimal value is -1, not the max int value you
are expecting. You may be half-wave rectifying your signal. Check.

You are only checking the clipping in the positive direction, what
about negative?


Most of all: Figure out how your input data is represented, then put a
synthesized sine wave into this code and look at the output.  Debug
your code.

Dirk


spagthorpe@gmail.com wrote:
> Hi all.
>
> I've got some 16-bit PCM data, and I'm trying to add a level
> adjustment to my code.  I thought it would be really straight forward,
> but it's causing a lot of distortion when I try to lower the level. It
> sounds bad most of the time if I increase the level, but I'm pretty
> sure that's just clipping.  If the db_gain variables are set to 0,
> everything sounds fine.
>
> Here is the code:
>
> // in this case, db_gain_left and db_gain_right would have
> // values in the -12 to +12 range.
> int db_gain_left = -3, db_gain_right = -3;
>
> // calculate a multiplication factor from the gain
> double db_left_factor = pow(10.0, (db_gain_left/20.0));
> double db_right_factor = pow(10.0, (db_gain_right/20.0));
>
> // multiply by 2^16 to speed up conversion later
> // saves a step in the loop
> db_left_factor *= 65536.0;
> db_right_factor *= 65536.0;
>
> // make working with samples  easier
> unsigned short *in, *out;
> in = (unsigned short*)pOutBuf;
> out = (unsigned short*)tempBuf;
>
> // samples are two byte shorts, so there are half
> // as many samples as bytes
> int num_samples = inputBytes / 2;
>
> float left = 0.0, right = 0.0;
>
> for(int z = 0; z < num_samples; z+=2) {
>
> 	// convert each sample to from PCM to float between 0 and 1
> 	left = in[z] / 65536.0;
> 	right = in[z+1] / 65536.0;
>
> 	// multiply each sample by the corrected factor
> 	left *= db_left_factor;
> 	right *= db_right_factor;
>
> 	// check for clipping
> 	if(left > 0xFFFF) left = 0xFFFF;
> 	if(right > 0xFFFF) right = 0xFFFF;
>
> 	// put values in output
> 	out[z] = (int)left;
> 	out[z+1] = (int)right;
> }
>
> Anyone see anything obviouos I'm doing wrong?
>
> The way I see it, for example, a sample might have the values:
>
> left = 48500, right = 23450
>
> the gain is -3 for left and right
> db_factor_left = db_factor_right = 0.7079
>
> multiply this by 65536 now, or do it later, shouldn't matter
> db_factor_left = db_factor_right = 46395.9394
>
> convert to floating point by deviding each by 65536
>
> left = 0.7401, right = 0.3578
>
> multiply by the db_factors
>
> left = 34337.6314, right = 16600.4655
>
> which are values scaled about how you would think based on the change.
>
> I really don't see where the math is wrong, but for some reason, it
> sounds bad.
>
> How else do you adjust volume levels?
> 
> Thanks for reading.
> 
> -james

Hi all.

I've got some 16-bit PCM data, and I'm trying to add a level
adjustment to my code.  I thought it would be really straight forward,
but it's causing a lot of distortion when I try to lower the level. It
sounds bad most of the time if I increase the level, but I'm pretty
sure that's just clipping.  If the db_gain variables are set to 0,
everything sounds fine.  

Here is the code:

// in this case, db_gain_left and db_gain_right would have
// values in the -12 to +12 range.
int db_gain_left = -3, db_gain_right = -3;

// calculate a multiplication factor from the gain
double db_left_factor = pow(10.0, (db_gain_left/20.0));
double db_right_factor = pow(10.0, (db_gain_right/20.0));

// multiply by 2^16 to speed up conversion later
// saves a step in the loop
db_left_factor *= 65536.0;
db_right_factor *= 65536.0;

// make working with samples  easier
unsigned short *in, *out;
in = (unsigned short*)pOutBuf;
out = (unsigned short*)tempBuf;

// samples are two byte shorts, so there are half 
// as many samples as bytes
int num_samples = inputBytes / 2;

float left = 0.0, right = 0.0;

for(int z = 0; z < num_samples; z+=2) {

	// convert each sample to from PCM to float between 0 and 1
	left = in[z] / 65536.0;
	right = in[z+1] / 65536.0;

	// multiply each sample by the corrected factor
	left *= db_left_factor;
	right *= db_right_factor;

	// check for clipping
	if(left > 0xFFFF) left = 0xFFFF;
	if(right > 0xFFFF) right = 0xFFFF;

	// put values in output
	out[z] = (int)left;
	out[z+1] = (int)right;
}

Anyone see anything obviouos I'm doing wrong?

The way I see it, for example, a sample might have the values:

left = 48500, right = 23450

the gain is -3 for left and right
db_factor_left = db_factor_right = 0.7079

multiply this by 65536 now, or do it later, shouldn't matter
db_factor_left = db_factor_right = 46395.9394

convert to floating point by deviding each by 65536

left = 0.7401, right = 0.3578

multiply by the db_factors

left = 34337.6314, right = 16600.4655

which are values scaled about how you would think based on the change.

I really don't see where the math is wrong, but for some reason, it
sounds bad.

How else do you adjust volume levels?

Thanks for reading.

-james