Skip to content
Closed
Show file tree
Hide file tree
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
86 changes: 86 additions & 0 deletions wled00/FX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7248,6 +7248,91 @@ uint16_t mode_2DGEQ(void) { // By Will Tatam. Code reduction by Ewoud Wijma.
} // mode_2DGEQ()
static const char _data_FX_MODE_2DGEQ[] PROGMEM = "GEQ@Fade speed,Ripple decay,# of bands,,,Color bars;!,,Peaks;!;2f;c1=255,c2=64,pal=11,si=0"; // Beatsin

/////////////////////////
// Single EQ Bar //
/////////////////////////
uint16_t mode_single_eqbar(void) {
if (!SEGENV.allocateData(sizeof(uint16_t))) return mode_static();
uint16_t *prevBarHeight = (uint16_t*)SEGENV.data;
if (SEGENV.call == 0) *prevBarHeight = 0;

// Grab audio data - we only need the fftResult here, as we want the frequency data
um_data_t *um_data = getAudioData();
uint8_t *fftResult = (uint8_t*)um_data->u_data[2];
if (!fftResult) return mode_static();
bool is2d = (strip.isMatrix || SEGMENT.is2D());

// User controls. Speed is referenced directly instead of creating a new varible.
uint8_t freqBin = map(SEGMENT.custom1, 0, 255, 0, 15); // 0-15 (or up to available bins)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add bounds checking for frequency bin access.

The frequency bin mapping doesn't ensure the selected bin is within the available FFT data range, which could lead to out-of-bounds access.

- uint8_t freqBin = map(SEGMENT.custom1, 0, 255, 0, 15); // 0-15 (or up to available bins)
+ uint8_t freqBin = map(SEGMENT.custom1, 0, 255, 0, min(15, um_data->u_size[2] - 1)); // 0-15 (or up to available bins)
🤖 Prompt for AI Agents
In wled00/FX.cpp at line 7266, the frequency bin calculated by mapping
SEGMENT.custom1 to a range 0-15 may exceed the actual available FFT bins,
risking out-of-bounds access. Add bounds checking after the map call to clamp
freqBin within the valid range of available FFT bins before using it.

uint8_t peakDecay = SEGMENT.intensity;
if (peakDecay > 0){
// Grab the intensity value for peak decay speed, bound it between 1 and 32, invert for ease of use.
peakDecay = map(255 - SEGMENT.intensity, 1, 255, 1, 32);
}
int barHeight = 0;
if (strip.isMatrix || SEGMENT.is2D()){
barHeight = map(fftResult[freqBin], 0, 255, 0, SEG_H); // Grab new bar height
} else {
barHeight = map(fftResult[freqBin], 0, 255, 0, SEGLEN); // Grab new bar height
}
if (barHeight > *prevBarHeight) *prevBarHeight = barHeight; // Update the previous bar height if the new height is greater

SEGMENT.fade_out(SEGMENT.speed); // Always fade out existing bars according to speed slider.

// Draw the main bar (but not peak pixel)
for (int i = 0; i < barHeight; i++) {
if (is2d){
// If we are in a matrix or 2D segment, draw the bar vertically
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, i, SEGMENT.color_from_palette(i, true, PALETTE_SOLID_WRAP, 0));
}
} else {
// Otherwise draw the bar horizontally
SEGMENT.setPixelColor(i, SEGMENT.color_from_palette(i, true, PALETTE_SOLID_WRAP, 0));
}
}

if (peakDecay == 0){
// No peak pixel if decay is set to zero, just draw the peak pixel of the bar.
if (barHeight > 0) {
if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, barHeight, SEGMENT.color_from_palette(barHeight, true, PALETTE_SOLID_WRAP, 0));
}
} else {
SEGMENT.setPixelColor(barHeight, SEGMENT.color_from_palette(barHeight, true, PALETTE_SOLID_WRAP, 0));
}
Comment on lines +7297 to +7304
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential out-of-bounds access for peak pixel.

When drawing the peak pixel at barHeight, there's no bounds checking to ensure it's within the segment dimensions.

  if (peakDecay == 0){
    // No peak pixel if decay is set to zero, just draw the peak pixel of the bar.
-   if (barHeight > 0) {
+   if (barHeight > 0 && barHeight < (is2d ? SEG_H : SEGLEN)) {
      if (is2d) {
        for (int j = 0; j < SEG_W; j++) {
          SEGMENT.setPixelColorXY(j, barHeight, SEGMENT.color_from_palette(barHeight, true, PALETTE_SOLID_WRAP, 0));
        } 
      } else {
        SEGMENT.setPixelColor(barHeight, SEGMENT.color_from_palette(barHeight, true, PALETTE_SOLID_WRAP, 0));
      }
    }
  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (barHeight > 0) {
if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, barHeight, SEGMENT.color_from_palette(barHeight, true, PALETTE_SOLID_WRAP, 0));
}
} else {
SEGMENT.setPixelColor(barHeight, SEGMENT.color_from_palette(barHeight, true, PALETTE_SOLID_WRAP, 0));
}
if (peakDecay == 0){
// No peak pixel if decay is set to zero, just draw the peak pixel of the bar.
if (barHeight > 0 && barHeight < (is2d ? SEG_H : SEGLEN)) {
if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, barHeight,
SEGMENT.color_from_palette(barHeight, true, PALETTE_SOLID_WRAP, 0));
}
} else {
SEGMENT.setPixelColor(barHeight,
SEGMENT.color_from_palette(barHeight, true, PALETTE_SOLID_WRAP, 0));
}
}
}
🤖 Prompt for AI Agents
In wled00/FX.cpp around lines 7297 to 7304, the code sets the pixel color at
barHeight without verifying if barHeight is within valid segment bounds. Add
bounds checking to ensure barHeight is less than SEG_W for 2D mode and less than
the segment length for 1D mode before setting the pixel color to prevent
out-of-bounds access.

}
} else {
// Decrement prevBarHeight according to peakDecay
if (*prevBarHeight > 0 && (SEGENV.call % (peakDecay > 1 ? peakDecay : 1)) == 0) (*prevBarHeight)--;

// Set peak pixel and clear pixels over peak (otherwise they would fadewith value from speed slider)
if (*prevBarHeight > 0) {

if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, *prevBarHeight, SEGCOLOR(2));
}
} else {
SEGMENT.setPixelColor(*prevBarHeight, SEGCOLOR(2));
}
}
Comment on lines +7311 to +7320
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add bounds checking for peak pixel drawing.

The peak pixel is drawn at *prevBarHeight without bounds checking, which could cause out-of-bounds access.

    // Set peak pixel and clear pixels over peak (otherwise they would fadewith value from speed slider)
-   if (*prevBarHeight > 0) {
+   if (*prevBarHeight > 0 && *prevBarHeight < (is2d ? SEG_H : SEGLEN)) {
      if (is2d) {
        for (int j = 0; j < SEG_W; j++) {
          SEGMENT.setPixelColorXY(j, *prevBarHeight, SEGCOLOR(2));
        }
      } else {
        SEGMENT.setPixelColor(*prevBarHeight, SEGCOLOR(2));
      }
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (*prevBarHeight > 0) {
if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, *prevBarHeight, SEGCOLOR(2));
}
} else {
SEGMENT.setPixelColor(*prevBarHeight, SEGCOLOR(2));
}
}
// Set peak pixel and clear pixels over peak (otherwise they would fadewith value from speed slider)
if (*prevBarHeight > 0 && *prevBarHeight < (is2d ? SEG_H : SEGLEN)) {
if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, *prevBarHeight, SEGCOLOR(2));
}
} else {
SEGMENT.setPixelColor(*prevBarHeight, SEGCOLOR(2));
}
}
🤖 Prompt for AI Agents
In wled00/FX.cpp around lines 7311 to 7320, the code draws a peak pixel at
*prevBarHeight without verifying if this value is within valid bounds. Add
bounds checking to ensure *prevBarHeight is within the allowed pixel range
before calling setPixelColorXY or setPixelColor to prevent out-of-bounds access
errors.

if (*prevBarHeight < SEGLEN) {

if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, *prevBarHeight + 1, BLACK); // clear next pixel immediately.
}
} else {
SEGMENT.setPixelColor(*prevBarHeight + 1, BLACK); // clear next pixel immediately.
}
}
Comment on lines +7321 to +7330
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix inconsistent bounds checking for 2D vs 1D modes.

The bounds check uses SEGLEN for both 1D and 2D modes, but 2D mode should use SEG_H for vertical bars.

-   if (*prevBarHeight < SEGLEN) {
+   if (*prevBarHeight < (is2d ? SEG_H - 1 : SEGLEN - 1)) {
      if (is2d) {
        for (int j = 0; j < SEG_W; j++) {
          SEGMENT.setPixelColorXY(j, *prevBarHeight + 1, BLACK); // clear next pixel immediately.
        }
      } else {
        SEGMENT.setPixelColor(*prevBarHeight + 1, BLACK); // clear next pixel immediately.
      }
    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (*prevBarHeight < SEGLEN) {
if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, *prevBarHeight + 1, BLACK); // clear next pixel immediately.
}
} else {
SEGMENT.setPixelColor(*prevBarHeight + 1, BLACK); // clear next pixel immediately.
}
}
if (*prevBarHeight < (is2d ? SEG_H - 1 : SEGLEN - 1)) {
if (is2d) {
for (int j = 0; j < SEG_W; j++) {
SEGMENT.setPixelColorXY(j, *prevBarHeight + 1, BLACK); // clear next pixel immediately.
}
} else {
SEGMENT.setPixelColor(*prevBarHeight + 1, BLACK); // clear next pixel immediately.
}
}
🤖 Prompt for AI Agents
In wled00/FX.cpp around lines 7321 to 7330, the bounds check incorrectly uses
SEGLEN for both 1D and 2D modes. Update the condition to check against SEG_H
when in 2D mode to correctly limit vertical bar height, while keeping SEGLEN for
1D mode. This ensures proper bounds checking depending on the mode.

}

return FRAMETIME;
}
static const char _data_FX_MODE_SINGLE_EQBAR[] PROGMEM = "Single EQ Bar@Fade speed,Peak decay,Frequency bin;!,,Peaks;!;!;12vf;sx=128,ix=170,c1=128,si=0";

/////////////////////////
// ** 2D Funky plank //
Expand Down Expand Up @@ -10818,5 +10903,6 @@ addEffect(FX_MODE_PS1DSONICSTREAM, &mode_particle1DsonicStream, _data_FX_MODE_PS
addEffect(FX_MODE_PS1DSONICBOOM, &mode_particle1DsonicBoom, _data_FX_MODE_PS_SONICBOOM);
addEffect(FX_MODE_PS1DSPRINGY, &mode_particleSpringy, _data_FX_MODE_PS_SPRINGY);
#endif // WLED_DISABLE_PARTICLESYSTEM1D
addEffect(FX_MODE_SINGLE_EQBAR, &mode_single_eqbar, _data_FX_MODE_SINGLE_EQBAR);

}
1 change: 1 addition & 0 deletions wled00/FX.h
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ extern byte realtimeMode; // used in getMappedPixelIndex()
#define FX_MODE_PS1DSPRINGY 216
#define FX_MODE_PARTICLEGALAXY 217
#define MODE_COUNT 218
#define FX_MODE_SINGLE_EQBAR 219

Comment on lines 376 to 378
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

MODE_COUNT is now off-by-one – update to reflect the new effect ID

MODE_COUNT must always be highest FX_MODE_ value + 1 (see long-term learning).
With FX_MODE_SINGLE_EQBAR added as 219, MODE_COUNT should be 220; leaving it at 218 will:

  • truncate the _mode / _modeData vectors (they’re pre-reserved with _modeCount = MODE_COUNT)
  • make array-index checks wrong and silently skip registering the last two modes
  • open up latent out-of-bounds writes if code assumes id < MODE_COUNT.

Suggested fix:

-#define MODE_COUNT                     218
+#define MODE_COUNT                     220   // 0 … 219

Also confirm that any build-time assertions or size-dependent allocations relying on MODE_COUNT are re-compiled.

🤖 Prompt for AI Agents
In wled00/FX.h around lines 376 to 378, the MODE_COUNT macro is incorrectly set
to 218 while the highest FX_MODE_ value is now 219 with FX_MODE_SINGLE_EQBAR.
Update MODE_COUNT to 220 to be one greater than the highest FX_MODE_ value. This
ensures correct sizing of _mode and _modeData vectors and prevents out-of-bounds
errors. Also verify that any build-time assertions or allocations depending on
MODE_COUNT are recompiled to reflect this change.


#define BLEND_STYLE_FADE 0x00 // universal
Expand Down