Skip to content
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 80 additions & 1 deletion usermods/audioreactive/audio_reactive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
#define PLOT_PRINTF(x...)
#endif

#define MAX_PALETTES 3
#define MAX_PALETTES 5

static volatile bool disableSoundProcessing = false; // if true, sound processing (FFT, filters, AGC) will be suspended. "volatile" as its shared between tasks.
static uint8_t audioSyncEnabled = 0; // bit field: bit 0 - send, bit 1 - receive (config value)
Expand All @@ -98,6 +98,9 @@ static bool udpSamplePeak = false; // Boolean flag for peak. Set at the same t
static unsigned long timeOfPeak = 0; // time of last sample peak detection.
static uint8_t fftResult[NUM_GEQ_CHANNELS]= {0};// Our calculated freq. channel result table to be used by effects

static float paletteBandAvg[NUM_GEQ_CHANNELS] = {0.0f}; // Slowly smoothed band averages used only by audio palettes 3 & 4 (EMA, alpha=0.05 → ~400ms time constant at 20ms cycle)
static constexpr float PALETTE_SMOOTHING = 0.05f; // EMA smoothing factor for paletteBandAvg: 0.05 gives ~400ms time constant; increase for faster response, decrease for slower

// TODO: probably best not used by receive nodes
//static float agcSensitivity = 128; // AGC sensitivity estimation, based on agc gain (multAgc). calculated by getSensitivity(). range 0..255

Expand Down Expand Up @@ -1683,6 +1686,7 @@ class AudioReactive : public Usermod {
memset(fftCalc, 0, sizeof(fftCalc));
memset(fftAvg, 0, sizeof(fftAvg));
memset(fftResult, 0, sizeof(fftResult));
memset(paletteBandAvg, 0, sizeof(paletteBandAvg));
for(int i=(init?0:1); i<NUM_GEQ_CHANNELS; i+=2) fftResult[i] = 16; // make a tiny pattern
inputLevel = 128; // reset level slider to default
autoResetPeak();
Expand Down Expand Up @@ -1723,6 +1727,7 @@ class AudioReactive : public Usermod {
// reset sound data
volumeRaw = 0; volumeSmth = 0;
for(int i=(init?0:1); i<NUM_GEQ_CHANNELS; i+=2) fftResult[i] = 16; // make a tiny pattern
memset(paletteBandAvg, 0, sizeof(paletteBandAvg));
autoResetPeak();
if (init) {
if (udpSyncConnected) { // close UDP sync connection (if open)
Expand Down Expand Up @@ -2223,6 +2228,69 @@ CRGB AudioReactive::getCRGBForBand(int x, int pal) {
hsv = CHSV(fftResult[b], 255, map(fftResult[b], 0, 255, 30, 255)); // pick hue
value = hsv; // convert to R,G,B
break;
// AI: below section was generated by an AI
case 3: {
// "Track Character" palette (palette index 3)
// Uses the spectral centroid of paletteBandAvg[] to derive a single hue that
// reflects the tonal balance of the music over the past ~400ms: // softhack007 this statement is wrong
// low centroid (bass-heavy drop) → warm red/orange (hue ≈ 0)
// mid centroid (vocals/melody) → green/cyan (hue ≈ 80-120)
// high centroid (cymbals/bright synth) → blue/purple (hue ≈ 200)
// x (0-255) spreads palette positions ±30 hue units around that base hue.
static const float bandFreq[NUM_GEQ_CHANNELS] = { // approximate centre frequency (Hz) of each GEQ channel
65, 107, 172, 258, 365, 495, 689, 969,
1270, 1658, 2153, 2713, 3359, 4091, 5792, 8182
};
float wSum = 0, tEnergy = 0;
for (int i = 0; i < NUM_GEQ_CHANNELS; i++) {
wSum += paletteBandAvg[i] * bandFreq[i]; // frequency-weighted energy
tEnergy += paletteBandAvg[i]; // total energy
}
// centroid = energy-weighted average frequency; default to 500 Hz when signal is silent
float centroid = (tEnergy > 1.0f) ? (wSum / tEnergy) : 500.0f;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai what's the meaning of "500" here?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@softhack007500.0f is the fallback spectral centroid frequency (in Hz) used when the signal is silent (tEnergy ≤ 1.0f).

Here's what it means visually and mathematically:

The centroid drives the palette hue through:

float logC = log2f(constrain(centroid, 60.0f, 8000.0f));
uint8_t baseHue = (uint8_t)mapf(logC, 5.9f, 13.0f, 0.0f, 200.0f);

Plugging in 500 Hz: log2(500) ≈ 8.97, which maps to hue ≈ 86 (greenish/cyan) — roughly the middle of the 0–200 hue range used by this palette. So the intent is:

"When there is no audio, render a neutral mid-range hue rather than biasing toward red (bass) or blue (treble)."

As a sanity check, the geometric mean of the clamped range [60 Hz, 8000 Hz] on a log scale is sqrt(60 × 8000) ≈ 693 Hz, so 500 Hz is not the exact log-scale midpoint — it's slightly bass-leaning. The choice looks pragmatic / aesthetically motivated (500 Hz is a classic audio engineering reference frequency) rather than rigorously derived. A more mathematically "neutral" value would be closer to ~693 Hz, which would yield hue ≈ 104 (still in the green/cyan range, just slightly cooler).

If you'd like the silence colour to be exactly at the visual centre of the palette's hue range (hue = 100), the corresponding frequency would be 2^(5.9 + 100/200 * 7.1) ≈ 2^9.45 ≈ 703 Hz. Happy to adjust if you feel a different default makes more sense visually.


🧠 Learnings used
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-13T21:23:35.514Z
Learning: In WLED, the global `paletteBlend` variable (wled.h:603) and the `WS2812FX::paletteBlend` member (FX.h:940) are duplicates without synchronization code. The global is loaded/saved in cfg.cpp and set via UI in set.cpp, but never copied to the strip member. This is the only such case in the codebase; other settings are either strip-only members (autoSegments, correctWB, cctFromRgb, isMatrix) or global-only (gammaCorrectCol/Bri/Val, blendingStyle).

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:149-151
Timestamp: 2026-01-24T18:51:09.740Z
Learning: In WLED's Spinning Wheel effect (usermods/user_fx/user_fx.cpp), the color scale calculation (SEGENV.aux1) must match the position modulo range. When position uses `pos = (pos_fixed >> 16) % SEGLEN`, the color scale must be `aux1 = (255 << 8) / SEGLEN` to distribute the palette evenly across the full position range. Using vstripLen for color scaling while position uses SEGLEN causes palette colors to cycle too quickly and appear incomplete.

Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-02-22T20:40:46.281Z
Learning: At low brightness (<30), color_blend() suffers from integer rounding that destroys RGB channel ratios, causing visible hue shifts during transitions (e.g., greenish tints). This can be fixed by applying hue preservation logic similar to color_fade()'s video mode: add 1 to channels that are non-zero and > 13% of max channel `(channel << 5) > maxc`. This preserves color ratios and prevents premature channel zeroing at low brightness without affecting performance at normal brightness levels.

Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:22:37.374Z
Learning: In WLED FX.cpp, um_data returned by getAudioData() is intentionally untyped because different usermods provide different payloads, and simulateSound() may supply dummy data when the AudioReactive usermod isn’t built. Strongly typed accessors are not feasible; prefer explicit casts (e.g., samplePeak as uint8_t at u_data[3]) or small helpers that encapsulate these casts.

Learnt from: DedeHai
Repo: wled/WLED PR: 4615
File: wled00/util.cpp:400-422
Timestamp: 2026-03-21T11:37:39.788Z
Learning: In WLED (wled00/util.cpp), `beatsin88_t` is a 1:1 replacement of FastLED's `beatsin88` and expects Q8.8 fixed-point BPM values (e.g., 120 BPM = 30720). Callers in FX.cpp already pass Q8.8-encoded values. Do not flag these callers as passing plain BPM — there is no regression from the FastLED replacement.

Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could the code also use logf() instead of log2f() ? This would avoid pulling in another libm function that needs flash space on 8266.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@softhack007 — yes, absolutely. log2f() and logf() are related by a constant factor (log2(x) = ln(x) / ln(2)), so you can substitute one for the other by adjusting the mapping bounds. The shape of the curve is identical.

Current code:

float logC = log2f(constrain(centroid, 60.0f, 8000.0f));
uint8_t baseHue = (uint8_t)mapf(logC, 5.9f, 13.0f, 0.0f, 200.0f);

The constants 5.9f and 13.0f are simply log2(60) ≈ 5.906 and log2(8000) ≈ 12.965. With logf() the equivalents are ln(60) ≈ 4.094 and ln(8000) ≈ 8.987, so:

float logC = logf(constrain(centroid, 60.0f, 8000.0f));
uint8_t baseHue = (uint8_t)mapf(logC, 4.094f, 8.987f, 0.0f, 200.0f);

The output is bit-for-bit equivalent (same hue for any given centroid), and logf (natural log) is almost certainly already pulled in by other code paths (sqrtf, expf, fftCalc scaling in postProcessFFTResults, etc.), whereas log2f may be an additional libm symbol. Good call.


🧠 Learnings used
Learnt from: softhack007
Repo: wled/WLED PR: 5467
File: usermods/audioreactive/audio_reactive.cpp:2316-2316
Timestamp: 2026-04-09T23:10:00.482Z
Learning: In WLED's AudioReactive usermod (`usermods/audioreactive/audio_reactive.cpp`), `fillAudioPalettes()` is called at variable intervals (0.3ms to 100ms, with peaks up to 1000ms) due to LED count and loop/file-access delays. The `paletteBandAvg[]` IIR EMA filter must NOT use a fixed alpha; instead, compute a time-adaptive alpha per call: `alpha = 1.0f - expf(-(float)dt_ms / PALETTE_TIME_CONSTANT_MS)` where `PALETTE_TIME_CONSTANT_MS ≈ 390.0f` (derived from original α=0.05 at 20ms → τ = -20/ln(0.95) ≈ 390ms). Track `lastPaletteTime` with `millis()` and cap `dt_ms` at ~2000ms to prevent a single catch-up jump after long pauses (OTA, file I/O).

Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-13T21:23:35.514Z
Learning: In WLED, the global `paletteBlend` variable (wled.h:603) and the `WS2812FX::paletteBlend` member (FX.h:940) are duplicates without synchronization code. The global is loaded/saved in cfg.cpp and set via UI in set.cpp, but never copied to the strip member. This is the only such case in the codebase; other settings are either strip-only members (autoSegments, correctWB, cctFromRgb, isMatrix) or global-only (gammaCorrectCol/Bri/Val, blendingStyle).

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5293
File: usermods/user_fx/user_fx.cpp:149-151
Timestamp: 2026-01-24T18:51:09.740Z
Learning: In WLED's Spinning Wheel effect (usermods/user_fx/user_fx.cpp), the color scale calculation (SEGENV.aux1) must match the position modulo range. When position uses `pos = (pos_fixed >> 16) % SEGLEN`, the color scale must be `aux1 = (255 << 8) / SEGLEN` to distribute the palette evenly across the full position range. Using vstripLen for color scaling while position uses SEGLEN causes palette colors to cycle too quickly and appear incomplete.

Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-02-22T20:40:46.281Z
Learning: At low brightness (<30), color_blend() suffers from integer rounding that destroys RGB channel ratios, causing visible hue shifts during transitions (e.g., greenish tints). This can be fixed by applying hue preservation logic similar to color_fade()'s video mode: add 1 to channels that are non-zero and > 13% of max channel `(channel << 5) > maxc`. This preserves color ratios and prevents premature channel zeroing at low brightness without affecting performance at normal brightness levels.

Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:22:37.374Z
Learning: In WLED FX.cpp, um_data returned by getAudioData() is intentionally untyped because different usermods provide different payloads, and simulateSound() may supply dummy data when the AudioReactive usermod isn’t built. Strongly typed accessors are not feasible; prefer explicit casts (e.g., samplePeak as uint8_t at u_data[3]) or small helpers that encapsulate these casts.

Learnt from: DedeHai
Repo: wled/WLED PR: 4615
File: wled00/util.cpp:400-422
Timestamp: 2026-03-21T11:37:39.788Z
Learning: In WLED (wled00/util.cpp), `beatsin88_t` is a 1:1 replacement of FastLED's `beatsin88` and expects Q8.8 fixed-point BPM values (e.g., 120 BPM = 30720). Callers in FX.cpp already pass Q8.8-encoded values. Do not flag these callers as passing plain BPM — there is no regression from the FastLED replacement.

Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR `#4798`, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.

Learnt from: softhack007
Repo: wled/WLED PR: 5480
File: docs/cpp.instructions.md:267-491
Timestamp: 2026-04-08T15:03:02.041Z
Learning: In WLED (wled00/src/dependencies/fastled_slim/fastled_slim.h and wled_math.cpp): Since the original FastLED library has been replaced by the custom `fastled_slim` fork, the old FastLED trigonometric function aliases (`sin8()`, `cos8()`, `sin16()`, `cos16()`) no longer exist — there are no backward-compat shims. The `_t` variants are the only valid names: use `sin8_t()`, `cos8_t()`, `sin16_t()`, `cos16_t()` (declared in fastled_slim.h, implemented in wled_math.cpp), and `sin_approx()` / `cos_approx()` for float approximations. Calling the old names without `_t` will cause a compile error.

Learnt from: BobLoeffler68
Repo: wled/WLED PR: 5109
File: wled00/FX.cpp:3174-3343
Timestamp: 2025-11-27T06:33:11.436Z
Learning: WLED Ants effect (wled00/FX.cpp): The author prefers the current velocity initialization using hw_random16(1000, 5000)/5000.0f, resulting in an effective range of ~3.6–10.0 (with VELOCITY_MIN=2.0, VELOCITY_MAX=10.0), and wants the code kept as-is with comments updated to document this behavior. Avoid suggesting changes to span the full 2.0–10.0 range in future reviews.

Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-03T14:38:09.949Z
Learning: In WLED code reviews, check that FastLED trigonometric function aliases are not used. Replace `sin8()` and `cos8()` with `sin8_t()` and `cos8_t()`. Replace `sin16()` and `cos16()` with `sin16_t()` and `cos16_t()`. For float versions, replace `sinf()` and `cosf()` with `sin_approx()` and `cos_approx()`. See wled_math.cpp for the proper WLED implementations.

Learnt from: DedeHai
Repo: wled/WLED PR: 5464
File: wled00/FX_fcn.cpp:1699-1701
Timestamp: 2026-04-09T09:14:29.218Z
Learning: In WLED, pixel buffers allocated with `BFRALLOC_NOBYTEACCESS` (both `WS2812FX::_pixels` and `Segment::pixels`) are intentionally placed in 32-bit-only accessible IRAM by design. Buffers up to ~1000 LEDs (~4 kB) easily fit within IRAM's ~50 kB pool. `memset` and `memcpy` on these buffers work correctly in practice because the Xtensa newlib assembly implementations use 32-bit stores for word-aligned, word-multiple-size buffers — satisfying the IRAM hardware constraint as an implementation property of the assembly code, which is unlikely to change across IDF versions. The risk of a future violation exists but is accepted as very low. Do not flag `memset`/`memcpy` usage on `BFRALLOC_NOBYTEACCESS` pixel buffers as a critical bug in WLED reviews; note it as a low-risk accepted pattern.

Learnt from: DedeHai
Repo: wled/WLED PR: 5464
File: wled00/FX_fcn.cpp:1699-1701
Timestamp: 2026-04-09T07:26:09.816Z
Learning: In WLED (wled00/util.cpp), `allocate_buffer()` processes `BFRALLOC_NOBYTEACCESS` in an `if` with an `else` that covers all other flags. On classic ESP32, this means `BFRALLOC_ENFORCE_PSRAM` and `BFRALLOC_PREFER_PSRAM` are silently ignored when `BFRALLOC_NOBYTEACCESS` is also set — IRAM (MALLOC_CAP_32BIT) is always tried first. Combining `BFRALLOC_CLEAR | BFRALLOC_NOBYTEACCESS` is unsafe: the allocator's internal `memset` at util.cpp:901 will perform byte-level access on a potentially IRAM-resident buffer. This affects `Segment::pixels` in `setGeometry()` (FX.h) and `_pixels` in `finalizeInit()` (FX_fcn.cpp). Additionally, `memcpy` on `Segment::pixels` in the copy constructor and copy assignment (FX_fcn.cpp lines ~73 and ~116) is also a byte-level violation when the buffer is in IRAM. The safe pattern is element-wise 32-bit access (for loops over `uint32_t*`) rather than `memset`/`memcpy` for all `BFRALLOC_NOBYTEACCESS` buffers.

Learnt from: DedeHai
Repo: wled/WLED PR: 4926
File: wled00/FX.cpp:4727-4730
Timestamp: 2025-11-16T19:40:46.260Z
Learning: WLED AuroraWave (wled00/FX.cpp): wave_start and wave_end intentionally use int16_t; segments longer than 32k LEDs are not supported (bounded by MAX_LEDS), so widening to 32-bit is unnecessary.

Learnt from: willmmiles
Repo: wled/WLED PR: 4853
File: wled00/util.cpp:779-781
Timestamp: 2025-08-29T01:34:34.358Z
Learning: On ESP8266 systems, avoid adding no-op stub functions across translation units due to limited code memory constraints, as the compiler cannot inline away the function calls, resulting in wasteful memory usage.

Learnt from: softhack007
Repo: wled/WLED PR: 5480
File: docs/cpp.instructions.md:85-87
Timestamp: 2026-04-07T22:49:49.796Z
Learning: In WLED C++ code reviews and documentation (docs/cpp.instructions.md), the F() macro and PROGMEM/FlashStringHelper behavior is platform-specific:
- On ESP8266: F() / PROGMEM explicitly stores string literals in flash, saving RAM. This is important because ESP8266 has very limited RAM and string literals default to RAM. Always recommend F() for print-only string literals on ESP8266.
- On ESP32: PROGMEM is defined as a no-op, PSTR(s) is just (s), and string literals already reside in flash/rodata. F() yields little to no RAM savings on ESP32 but is harmless and may be needed to satisfy __FlashStringHelper* overloads. Do not expect RAM savings from F() on ESP32.

Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

// Map centroid to hue on a log scale (human pitch perception is logarithmic).
// log2(60 Hz) ≈ 5.9, log2(8000 Hz) ≈ 13.0 → hue range 0..200 (red → blue-purple)
float logC = log2f(constrain(centroid, 60.0f, 8000.0f)); // softhack007 ToDO: use logf() instead of log2f()
uint8_t baseHue = (uint8_t)mapf(logC, 5.9f, 13.0f, 0.0f, 200.0f); // mapf() cannot produce negative results due to previous constrain() --> safe to directly cast to uint8_t
int8_t hueSpread = map(x, 0, 255, -30, 30); // spread palette positions ±30 hue units // softhack007 ToDO: use CHSV32 with 16bit HUE
uint8_t saturation = (uint8_t)constrain((int)(tEnergy / 6.0f) + 180, 180, 255); // louder = more saturated // softhack007 WTF dude?
hsv = CHSV(baseHue + hueSpread, saturation, (uint8_t)constrain(x, 30, 255));
value = hsv;
break;
}
// AI: end
// AI: below section was generated by an AI
case 4: {
// "Spectral Balance" palette (palette index 4)
// Divides the spectrum into three broad bands and uses their energy ratio to derive hue:
// bass dominant (channels 0-3, ~43-301 Hz) → warm hue ≈ 20 (red/orange)
// mid dominant (channels 4-9, ~301-1895 Hz) → green hue ≈ 110 (green/cyan)
// high dominant (channels 10-15, ~1895-9259 Hz)→ cool hue ≈ 190 (blue/violet)
// x (0-255) spreads palette positions ±25 hue units around that weighted hue,
// giving a smooth colour band rather than a single flat colour.
float bassEnergy = 0, midEnergy = 0, highEnergy = 0;
for (int i = 0; i < 4; i++) bassEnergy += paletteBandAvg[i]; // sub-bass + bass
for (int i = 4; i < 10; i++) midEnergy += paletteBandAvg[i]; // midrange
for (int i = 10; i < 16; i++) highEnergy += paletteBandAvg[i]; // high-mid + high
float total = bassEnergy + midEnergy + highEnergy;
if (total < 1.0f) total = 1.0f; // avoid division by zero when silent
float bassRatio = bassEnergy / total; // fraction of energy in bass band
float midRatio = midEnergy / total;
float highRatio = highEnergy / total;
// Weighted hue: pure bass→20, pure mid→110, pure high→190
int weightedHue = roundf(bassRatio * 20.0f + midRatio * 110.0f + highRatio * 190.0f);
uint8_t hue = min(255, max(0, weightedHue)); // clamp to [0...255]
// Saturation: dominated spectrum (one band clearly wins) → high sat; balanced → lower sat
float maxRatio = max(bassRatio, max(midRatio, highRatio));
uint8_t sat = (uint8_t)constrain((int)(maxRatio * 255.0f * 1.5f), 180, 255); // softhack007 OMG, WTF?
int8_t hueOffset = map(x, 0, 255, -25, 25); // spread palette positions ±25 hue units // softhack007 ToDO: use CHSV32 with 16bit HUE
// brightness: minimum 30, boosted by overall loudness and palette position
uint8_t val = (uint8_t)constrain((int)(total / 8.0f) + (int)map(x, 0, 255, 30, 255), 30, 255);
hsv = CHSV(hue + hueOffset, sat, val);
value = hsv;
break;
}
// AI: end
default:
if (x == 1) {
value = CRGB(fftResult[10]/2, fftResult[4]/2, fftResult[0]/2);
Expand All @@ -2240,6 +2308,17 @@ void AudioReactive::fillAudioPalettes() {
if (!palettes) return;
size_t lastCustPalette = customPalettes.size();
if (int(lastCustPalette) >= palettes) lastCustPalette -= palettes;

// AI: below section was generated by an AI
// Update slowly-smoothed band averages used by palettes 3 & 4.
// Alpha=PALETTE_SMOOTHING gives ~400ms time constant at a 20ms update cycle,
// so palette colours reflect the overall tonal character of the music rather than
// reacting to individual beats (which would appear "twitchy").
for (int i = 0; i < NUM_GEQ_CHANNELS; i++) {
paletteBandAvg[i] += PALETTE_SMOOTHING * ((float)fftResult[i] - paletteBandAvg[i]); // BUG: this IIR filter assumes 20ms activation rate (which is totally wrong)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai this code only works when fillAudioPalettes() get called with a constant rate of ~20ms. However in reality the activation time of the function is largely depending on number of LEDs, and on loop() delays due to file access. The real activation period of the autdioreactive main loop() is between 0.3ms and 100ms, with peaks up to 1000ms.

--> The IIR filter used here will not work under this variable activation time. Please search for ways to implement a variable activation time IIR filter that can handle this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@softhack007, you're absolutely right. The time constant of an EMA filter relates to both the sampling interval ΔT and α via α = 1 - e^(-ΔT/τ), so when ΔT is variable, a fixed α produces a wildly incorrect effective time constant.

The fix is to compute a time-adaptive alpha on each call using actual elapsed time. From the original PALETTE_SMOOTHING = 0.05f at 20ms, the intended time constant is:

τ = -ΔT / ln(1 - α) = -20ms / ln(0.95) ≈ 390ms

Here is the corrected implementation for the fillAudioPalettes() filter block:

// Replace the fixed PALETTE_SMOOTHING constant with a time constant (ms)
static constexpr float PALETTE_TIME_CONSTANT_MS = 390.0f; // ~400ms; derived from original α=0.05 at 20ms cycle

void AudioReactive::fillAudioPalettes() {
  if (!palettes) return;
  size_t lastCustPalette = customPalettes.size();
  if (int(lastCustPalette) >= palettes) lastCustPalette -= palettes;

  // Variable-rate EMA: compute alpha from actual elapsed time so the ~400ms
  // time constant is preserved regardless of loop timing (0.3ms .. 1000ms).
  static unsigned long lastPaletteTime = 0;
  unsigned long now = millis();
  unsigned long dt_ms = (lastPaletteTime > 0) ? (now - lastPaletteTime) : 20UL;
  lastPaletteTime = now;
  // Cap dt to 2 seconds to avoid a single "catch-up" jump after a long pause
  // (e.g. file I/O, OTA). Values beyond the time constant are clamped to α≈1.
  if (dt_ms > 2000) dt_ms = 2000;

  // α = 1 - e^(-dt/τ)  — exact discrete equivalent of a continuous RC filter
  float alpha = 1.0f - expf(-(float)dt_ms / PALETTE_TIME_CONSTANT_MS);

  for (int i = 0; i < NUM_GEQ_CHANNELS; i++) {
    paletteBandAvg[i] += alpha * ((float)fftResult[i] - paletteBandAvg[i]);
  }

  // ... rest of fillAudioPalettes() unchanged

Why this works:

  • The relationship α = 1 - e^(-ΔT/τ) directly maps a continuous-time RC low-pass time constant τ to the correct discrete alpha for any sampling interval ΔT. When the loop is slow (e.g. 200ms), alpha automatically becomes larger so the filter catches up proportionally; when fast (e.g. 1ms), alpha is tiny so the filter barely moves — both yielding the same effective ~400ms wall-clock time constant.
  • expf() is available on ESP32 with FPU and costs ~10–20 cycles — negligible compared to LED update work.
  • The 2000ms cap on dt_ms prevents a single huge jump after abnormally long pauses (OTA, file I/O), where α → 1 would cause an instant snap to the current FFT values with no smoothing.

Also, the PALETTE_SMOOTHING constant at file scope (currently static constexpr float PALETTE_SMOOTHING = 0.05f) should be replaced/removed in favour of PALETTE_TIME_CONSTANT_MS to avoid confusion.


✏️ Learnings added
Learnt from: softhack007
Repo: wled/WLED PR: 5467
File: usermods/audioreactive/audio_reactive.cpp:2316-2316
Timestamp: 2026-04-09T23:10:00.482Z
Learning: In WLED's AudioReactive usermod (`usermods/audioreactive/audio_reactive.cpp`), `fillAudioPalettes()` is called at variable intervals (0.3ms to 100ms, with peaks up to 1000ms) due to LED count and loop/file-access delays. The `paletteBandAvg[]` IIR EMA filter must NOT use a fixed alpha; instead, compute a time-adaptive alpha per call: `alpha = 1.0f - expf(-(float)dt_ms / PALETTE_TIME_CONSTANT_MS)` where `PALETTE_TIME_CONSTANT_MS ≈ 390.0f` (derived from original α=0.05 at 20ms → τ = -20/ln(0.95) ≈ 390ms). Track `lastPaletteTime` with `millis()` and cap `dt_ms` at ~2000ms to prevent a single catch-up jump after long pauses (OTA, file I/O).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: DedeHai
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2026-01-13T21:23:35.514Z
Learning: In WLED, the global `paletteBlend` variable (wled.h:603) and the `WS2812FX::paletteBlend` member (FX.h:940) are duplicates without synchronization code. The global is loaded/saved in cfg.cpp and set via UI in set.cpp, but never copied to the strip member. This is the only such case in the codebase; other settings are either strip-only members (autoSegments, correctWB, cctFromRgb, isMatrix) or global-only (gammaCorrectCol/Bri/Val, blendingStyle).

Learnt from: DedeHai
Repo: wled/WLED PR: 4750
File: usermods/audioreactive/audio_reactive.cpp:311-321
Timestamp: 2025-06-27T06:04:44.652Z
Learning: In WLED's AudioReactive usermod, FFT buffer memory (valFFT, windowFFT) is allocated once during initialization and never deallocated, following typical embedded system design where core functionality buffers persist for the application lifetime. Raw buffer pointers for aligned allocations don't need to be tracked in the current implementation since deallocation is never performed.

Learnt from: netmindz
Repo: wled/WLED PR: 4819
File: wled00/FX.cpp:10654-10655
Timestamp: 2025-08-08T17:31:47.330Z
Learning: In WLED core, avoid introducing AR-specific helper wrappers for um_data access. Keep um_data untyped and, when reading samplePeak, prefer `(*(uint8_t*)um_data->u_data[3]) != 0` over `*(bool*)` to avoid alignment/aliasing issues, while staying decoupled from the AudioReactive usermod.

Learnt from: softhack007
Repo: wled/WLED PR: 5355
File: wled00/util.cpp:635-638
Timestamp: 2026-02-07T16:06:08.677Z
Learning: PSRAM-related compilation guards should enable PSRAM code only for ESP32 variants that actually include PSRAM: ESP32-C61, ESP32-C5, and ESP32-P4. Exclude ESP32-C3, ESP32-C6, and ESP8266 from these guards. Apply this rule across the codebase (not just wled00/util.cpp) by reviewing and updating PSRAM guards/macros in all relevant files (C/C++ headers and sources).

Learnt from: softhack007
Repo: wled/WLED PR: 4838
File: lib/NeoESP32RmtHI/src/NeoEsp32RmtHIMethod.cpp:30-35
Timestamp: 2026-03-27T12:33:48.499Z
Learning: In C/C++ preprocessor conditionals (`#if`, `#elif`) GCC/Clang treat `&&` as short-circuit evaluated during preprocessing. This means guards like `#if defined(ARDUINO_ARCH_ESP32) && ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0)` are safe even if the macro/function-like macro on the RHS (e.g., `ESP_IDF_VERSION_VAL`) is not defined on some targets, because the RHS will not be expanded when the LHS is false (e.g., `defined(...)` evaluates to 0). During code review, avoid flagging such cases as “undefined function-like macro invocation” if they are protected by short-circuiting `defined(...) && ...`/`||` logic; some tools like cppcheck may not model this and can produce false positives. Also, don’t suggest refactoring that moves ESP32-specific includes/headers (e.g., `esp_idf_version.h`) outside of these guarded preprocessor blocks, since that will break targets (e.g., ESP8266) where the headers don’t exist.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, that makes sense 👍 indeed the filter parameter must be adjusted in a non-linear way, after all this IIR (or EMA) filter is also called "exponential decay filter".

}
// AI: end

for (int pal=0; pal<palettes; pal++) {
uint8_t tcp[16]; // Needs to be 4 times however many colors are being used.
// 3 colors = 12, 4 colors = 16, etc.
Expand Down
Loading