Happy little incident #11

Mcu32

Well-known member
Two numerical issues in control_sgtl5000.cpp

1. autoVolumeControl() uses uint8_t for values that are calculated much larger​

thresh, att and dec are declared as uint8_t.

The calculations immediately produce values larger than 255, so assigning the result to uint8_t truncates the value to its lowest 8 bits before it is passed to write().

A calculated value such as:

0x39BC
becomes:

0xBC
when stored in uint8_t.

The exact final width can then be limited when writing to the codec registers, rather than accidentally losing the upper bits during the calculation.
Register width: 16 bit.
There's even an old issue on GitHub about this already.


2. calcBiquad() does not round negative coefficients correctly​

At the end of calcBiquad() the coefficients are converted using:

*coef++ = (int)(b0 + 0.499);
*coef++ = (int)(b1 + 0.499);
*coef++ = (int)(b2 + 0.499);
*coef++ = (int)(a1 + 0.499);
*coef++ = (int)(a2 + 0.499);
This works approximately as intended for positive values, but it is not symmetric for negative values because conversion from floating point to int truncates toward zero.
 
Last edited:
Which AI are you using? I've been using Claude so far, and I've been impressed by its ability to analyze code and find bugs.
 
Various. I think Claude is the best, but ChatGPT does a good job, too. Gemini is good sometimes, but on other days it hallucinates too much.
I’m currently porting the audio library to the ESP32. The AIs are a huge help there and really take a lot of the tedious work off my hands.
We’ll see how it goes when I start optimizing for the S3’s DSP. I don’t expect much-the ESP can’t hold a candle to ARM when it comes to 16-bit calculations-but I think 90% of normal audio tasks will work just fine there too. There are some positives, too. Since the 16-bit limitation often makes it better for me to just use 32-bit right away, I take advantage of that “on the side” to improve the audio quality vs Teensy.
Without AI, I would never have started this.
 
Last edited:
Okay, it's working so far.
On an old ESP32 A1S1(not an S3):
Core 0 (rtos, Wi-Fi, MP3, AAC) is currently using 59% of the CPU, and Core 1 (user code + audio library, connections, statistics, I2S + DMA handling, etc.) is using 7%.
- for an MP3 radio stream at 128k+ VBR - not bad; I think I can work with that.
(and not a single “disable interrupt”)
 
Last edited:
void AudioEffectGranular::beginFreeze_int(int grain_samples)
{
__disable_irq(); //arghhhhh
grain_mode = 1;
if (grain_samples < max_sample_len) {
freeze_len = grain_samples;
} else {
freeze_len = grain_samples;
}

sample_loaded = false;
write_en = false;
sample_req = true;
__enable_irq();
}
Also: It doesn't check whether the IRQ was disabled beforehand … okay, I know you don't want to hear that … that pattern is everywhere.
 
Last edited:
Back
Top