[WIP] Add new audio-reactive palettes for smoothed FFT data#5467
Draft
[WIP] Add new audio-reactive palettes for smoothed FFT data#5467
Conversation
…ral Balance) Agent-Logs-Url: https://github.com/wled/WLED/sessions/a11e5210-96ae-4713-abdd-ba42b0e33bc5 Co-authored-by: netmindz <442066+netmindz@users.noreply.github.com>
Copilot stopped work on behalf of
netmindz due to an error
April 1, 2026 08:31
This comment was marked as outdated.
This comment was marked as outdated.
Agent-Logs-Url: https://github.com/wled/WLED/sessions/2dff17f8-2992-4060-8a37-5c72dd1fbf51 Co-authored-by: softhack007 <91616163+softhack007@users.noreply.github.com>
This comment was marked as low quality.
This comment was marked as low quality.
softhack007
reviewed
Apr 9, 2026
| 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; |
Contributor
There was a problem hiding this comment.
@softhack007 — 500.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.
Member
There was a problem hiding this comment.
could the code also use logf() instead of log2f() ? This would avoid pulling in another libm function that needs flash space on 8266.
Contributor
There was a problem hiding this comment.
@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.
* move paletteBandAvg out of esp32-only section (fixes 8266 build error) * style: move default case _below_ other cases * add missing "generated by AI" labels * mark some leftover bugs
softhack007
requested changes
Apr 9, 2026
Member
There was a problem hiding this comment.
This PR has some serious bugs and conceptual errors that need to be fixed.
- most important bug: the palette code assumes that getCRGBForBand() is activated with a regular 20ms rate - filter constants are selected for 400ms decay time.
--> this assumption is so wrong -> in reality, the audiorective::loop() is called each 0.3ms to 100ms, with peak delay up to 1000ms (for example due to file access).
I don't have a fix for this yet - a variable rate IIR filter is a really hard task.
One idea could be to "attach" getCRGBForBand() to FFTcode() AND receiveAudioData(). But then we need to verify that the palette function does not consume more that 1ms of time per call.
______________________________
< AI vs. bugs. Round 1. Fight! >
------------------------------
\
\ (\__/)
(•ㅅ•)
/ づ
softhack007
reviewed
Apr 9, 2026
| // 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) |
Member
There was a problem hiding this comment.
@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.
Contributor
There was a problem hiding this comment.
@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() unchangedWhy 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_msprevents a single huge jump after abnormally long pauses (OTA, file I/O), whereα → 1would 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.
Member
There was a problem hiding this comment.
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".
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request extends audio reactive palette support by increasing the maximum palette count from 3 to 5, adding band average tracking with exponential smoothing, and implementing two new palette generation behaviors that compute hues and saturation/brightness values from weighted frequency band energies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Member
|
@netmindz I've un-assigned Copilot from this PR, its seems like copilot has no clue about math 😉 I think i can fix it together with the rabbit. Good news: 99% sure the code can be back-ported to WLED-MM once I get the math to work. |
WTF, dude
softhack007
added
enhancement
usermod
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MAX_PALETTESfrom 3 to 5paletteBandAvg[NUM_GEQ_CHANNELS]static array alongside existing FFT arraysPALETTE_SMOOTHINGconstant at file scope (alongsidepaletteBandAvg) with explanatory commentpaletteBandAvginfillAudioPalettes()with comment explaining the smoothing rationalecase 3("Track Character") — spectral centroid from smoothed bands drives hue (warm=bass, cool=treble), fully documented with frequency ranges, hue mapping, and log-scale rationalecase 4("Spectral Balance") — bass/mid/high energy ratios drive hue with smooth transitions, fully documented with band boundaries, hue targets, and saturation logicpaletteBandAvginonUpdateBegin()alongside other FFT buffer resetsnpm test— all 16 tests passpio run -e esp32dev— build succeeds (SUCCESS, 00:01:59)Original prompt
Problem
The AudioReactive usermod currently contains three colour palettes (defined in
getCRGBForBand()at line 2211 ofusermods/audioreactive/audio_reactive.cpp) that are updated every frame using rawfftResult[]values. While this makes them very responsive, it also makes them "twitchy" — they react to instantaneous FFT values rather than reflecting the overall character or balance of the music.Goal
Add new audio-reactive palette(s) that use smoothed/averaged FFT data to reflect the overall tonal balance of the music. The key user experience should be:
Implementation Details
All changes should be in
usermods/audioreactive/audio_reactive.cpp.1. Add a smoothed band average buffer for palette use
Add a static
float paletteBandAvg[NUM_GEQ_CHANNELS]array (alongside the existingfftCalc,fftAvg,fftResultarrays near line 227). This buffer should be updated with an exponential moving average (EMA) infillAudioPalettes()before the palette colours are computed:2. Add a "Spectral Centroid / Track Character" palette
Add a new
case 3:to theswitch (pal)ingetCRGBForBand(). This palette computes the spectral centroid (centre-of-mass of the frequency spectrum) from the smoothed band averages and maps it to hue:3. Add a "Spectral Balance" palette
Add a new
case 4:to theswitch (pal)ingetCRGBForBand(). This palette computes energy ratios across bass/mid/high regions and uses the weighted balance to derive hue:4. Update
MAX_PALETTESChange line 74 from:
to:
5. Reset smoothing buffer on audio reset
In
onUpdateBegin()(around line 1683), add a reset ofpaletteBandAvgalongside the existingmemset(fftCalc, ...)calls:Key constraints
usermods/audioreactive/audio_reactive.cppfloat[16]array = 64 bytespaletteBandAvgarray must be declaredstaticat file scope alongside the existing FFT...This pull request was created from Copilot chat.
Summary by CodeRabbit
New Features
Bug Fixes