I'm trying to adjust the volume of some audio samples in real time while allowing for boosting the volume and clipping it if it exceeds the maximum 12 bit range of the DAC, and while my math seems correct I'm not getting any audio.
// Process next 16-bit sample:
// 16-bit sample is signed.
uint8_t dh, dl;
dl = playpos[0];
dh = playpos[1];
playpos += 2;
int16_t sample = (dh << 8) | dl; // Create signed 16 bit sample.
// Adjust by global volume level.
int32_t tmp32 = volume * sample; // Multiply 16 bit unsigned sample by 12+ bit volume... 0...4096+
int16_t tmp16 = tmp32 / 65536; // Then divide by 4096 which is 12 bit 100% volume * 16 which is the factor by which we need to reduce the 16 bit sample to get it down to 12 bits.
// This leaves us with tmp in the range of -2048 to 2047...
// That is, unless volume was greater than 4096, in which case the result may be outside that range.
// So, we need to clip it.
if (tmp16 > 2047) { tmp16 = 2047; } else if (tmp16 < -2048) { tmp16 = -2048; } // Clip signed 16 bit sample to 12bit range.
// Finally, we add 2048 to tmp16 to make it an unsigned 12 bit sample for our dac.
sample = tmp16 + 2048;
// Send configuration bits to the DAC and then bits 11..0:
mcpDacSend(sample);
I know the audio system and electronics are working fine, because if I get rid of everything after "adjust by global volume level" and replace it with:
sample = sample / 16;
sample = sample + 2048;
// Send configuration bits to the DAC and then bits 11..0:
mcpDacSend(sample);
It works! Of course, I can't adjust the volume level then.
I'm guessing maybe I need to cast something? But I tried this math in a separate sketch and it seems to work as it should. And I've tried casting things and I can't seem to get that to work either.
Here's some of my other variables and code that appear elsewhere in the program so you can see the types and such:
uint16_t volume = 4095; // Global volume level.
uint16_t SRL_volume = 0; // Slew-rate limited volume.
float humVolume = 1.0;
setVolume(port::analogValue[PORT_VOLUME] * volumeBoost);
void setVolume(float newVolume) {
volume = humVolume * newVolume * 4095.0;
}
scswift:
int32_t tmp32 = volume * sample; // Multiply 16 bit unsigned sample by 12+ bit volume... 0...4096+
Since volume is uint16_t and sample is int16_t the result of "volume * sample" will be 16 bit, then saved as 32-bit
This is what you want to happen:
int32_t tmp32 = (int32_t)volume * (int32_t)sample; // Multiply 16 bit unsigned sample by 12+ bit volume... 0...4096+
When you are calculating with different types, I find it best to always explicitly cast to the type you want (even if it is not actually needed, it shows the intent of the calculation)
Yours,
TonyWilk
P.S. I only scanned your code to this line... you'll have to check the rest
I'll try that tomorrow, but in this test program I wrote, I don't seem to need to cast those values to get the correct result:
void setup() {
SerialUSB.begin(9600);
while (!SerialUSB);
uint8_t dh, dl;
uint16_t SRL_volume = 4095;
// -32768 = 1000 0000 0000 0000 = 128 + 0
dl = 0;
dh = 128;
int16_t sample = (dh << 8) | dl; // Create signed 16 bit sample.
SerialUSB.print("sample = "); SerialUSB.println(sample);
// Adjust by global volume level.
// Do so after the peak detector so that the animated display doesn't change as the volume is turned down.
int32_t tmp32 = SRL_volume * sample; // Multiply 16 bit unsigned sample by 12+ bit volume... 0...4096+
SerialUSB.print("tmp32 = "); SerialUSB.println(tmp32);
int16_t tmp16 = tmp32 / 65536; // Then divide by 4096 which is 12 bit 100% volume.
//int16_t tmp16 = int32_t(tmp32 / 65536); // Then divide by 4096 which is 12 bit 100% volume.
//int16_t tmp16 = int32_t(tmp32) / 65536; // Then divide by 4096 which is 12 bit 100% volume.
//int16_t tmp16 = int32_t(tmp32) / int32_t(65536); // Then divide by 4096 which is 12 bit 100% volume.
SerialUSB.print("tmp16 = "); SerialUSB.println(tmp16);
// This leaves us with tmp in the rang of -2048 to 2047...
// That is, unless volume was greater than 4096, in which case the result may be outside that range.
// So, we need to clip it.
if (tmp16 > 2047) { tmp16 = 2047; } else if (tmp16 < -2048) { tmp16 = -2048; } // Clip signed 32 bit sample to 12bit range.
SerialUSB.print("tmp16 = "); SerialUSB.println(tmp16);
// Finally, we add 2048 to tmp16 to make it an unsigned 12 bit sample for our dac.
sample = tmp16 + 2048;
SerialUSB.print("sample = "); SerialUSB.println(sample);
/*
dl = 255;
dh = 127;
sample = 32767
tmp32 = 134180865
tmp16 = 2047 *correct* This is 32767 scaled down to 12 bit range.
tmp16 = 2047
sample = 4095
sample = -32768
tmp32 = -134184960
tmp16 = -2047 *also correct*
tmp16 = -2047
sample = 1
*/
}
void loop() {
delay(100);
}
You can see in my comments there at the end the debug output shows tmp32 = 134180865 after that multiply in the first test, and -134184960 in the second. That's a 32 bit integer.
scswift:
I'll try that tomorrow, but in this test program I wrote, I don't seem to need to cast those values to get the correct result: snip
Hmm, that's mighty strange. With this code:
uint8_t dh= 128;
uint8_t dl= 0;
uint16_t volume= 4095;
int32_t tmp32= 0;
int16_t sample= (dh << 8) | dl;
Serial.print("volume= "); Serial.println( volume );
Serial.print("sample= "); Serial.println( sample );
tmp32= volume * sample;
Serial.print("tmp32= volume * sample; result= ");Serial.println( tmp32 );
tmp32= (int32_t)volume * (int32_t)sample;
Serial.print("tmp32= (int32_t)volume * (int32_t)sample; result= ");Serial.println( tmp32 );
I get the result:
volume= 4095
sample= -32768
tmp32= volume * sample; result= 32768
tmp32= (int32_t)volume * (int32_t)sample; result= -134184960
...just explicitly cast everything to make sure 
Yours,
TonyWilk
Uh... I just ran you code, and I get:
volume= 4095
sample= -32768
tmp32= volume * sample; result= -134184960
tmp32= (int32_t)volume * (int32_t)sample; result= -134184960
This is weird.
I wonder if there's some issue with whatever version of the compiler I'm running and/or the hardware it is compiling for?
I'm running my test code on a SAMD21 board, but the actual program is running on an ATMEGA1284. The reason I'm doing that is because I don't have a serial output readily available on the ATMEGA board, and programming it is done by putting the binaries on an SD card, so it's a huge pain in the ass to debug directly. And I don't have any other 8 bit ATMEGAS to test with.
Also I'm using v1.8.3 of the IDE.
If I had to guess, I'd guess that the compiler for whatever reason is optimizing the ATMGEA code in such a way that it's using the smaller ints, while the SAMD compiler is choosing the larger ones for speed as well. Kind of annoying. Typecasting all the code makes it messy and harder to read.
I did try typecasting some of it... I think... but maybe I didn't do that particular line and maybe I did most of those tests in my sample program where I could actually see my debug output.
Anyway, I'll give it a shot in the morning. I don't want to wake the neighbors with a 40W amplifier spitting out static at max volume again if I screw up.
scswift:
I'm running my test code on a SAMD21 board, but the actual program is running on an ATMEGA1284.
AH !!
The SAMD21 is a 32-bit processor, so the default int is 32-bit. I guess the intermediate calculation uses 32bit by default.
...so you will get different results unless you explicitly cast 'em.
Yours,
TonyWilk
Well, it turned out that that's what was wrong with the code. I guess I must have failed to cast them during my attempts to get it to work, but in fairness, this was like the 15th version I'd compiled after altering my math a dozen times to try to get it to work so maybe I did cast it but it was an earlier version where something else was broken.
There was still one lingering issue in that the divide operation was far too slow to use in the middle of my sample processing interrupt, and I was concerned that bit shifting wouldn't work properly on a signed integer, but >> 16 worked a treat and the code runs normally again!