From e2f5becdd02ad3c486fe6ddbda4aa7246cd5ba69 Mon Sep 17 00:00:00 2001 From: Damian Schneider Date: Wed, 23 Jul 2025 06:42:06 +0200 Subject: [PATCH] Bugfixes in FX data allocation (#4783) - Bugfixes in FX data allocation: realloc was not handled properly. - Added *intermediate* fix for waitForIt(), see https://github.com/wled/WLED/issues/4779 - Bugfix in 1D->2D expansions: corner-expansion MUST be boundary checked as it blindly writes the max dimension - removed some realloc(), improving fragmentation on large setups - increased min heap constant - ESP32 C3 has no PSRAM, it now uses default alloc functions - also added missing UI info for "Error 7" --- wled00/FX_fcn.cpp | 31 +++++++++++++++++++------------ wled00/bus_manager.cpp | 9 ++++++++- wled00/const.h | 4 ++-- wled00/data/index.js | 3 +++ wled00/fcn_declare.h | 9 ++++++++- wled00/util.cpp | 31 ++++++++++++++++++++++++++++--- 6 files changed, 68 insertions(+), 19 deletions(-) diff --git a/wled00/FX_fcn.cpp b/wled00/FX_fcn.cpp index 8f7263365..a479b655a 100755 --- a/wled00/FX_fcn.cpp +++ b/wled00/FX_fcn.cpp @@ -159,8 +159,16 @@ bool Segment::allocateData(size_t len) { return false; } // prefer DRAM over SPI RAM on ESP32 since it is slow - if (data) data = (byte*)d_realloc(data, len); - else data = (byte*)d_malloc(len); + if (data) { + data = (byte*)d_realloc_malloc(data, len); // realloc with malloc fallback + if (!data) { + data = nullptr; + Segment::addUsedSegmentData(-_dataLen); // subtract original buffer size + _dataLen = 0; // reset data length + } + } + else data = (byte*)d_malloc(len); + if (data) { memset(data, 0, len); // erase buffer Segment::addUsedSegmentData(len - _dataLen); @@ -170,7 +178,6 @@ bool Segment::allocateData(size_t len) { } // allocation failed DEBUG_PRINTLN(F("!!! Allocation failed. !!!")); - Segment::addUsedSegmentData(-_dataLen); // subtract original buffer size errorFlag = ERR_NORAM; return false; } @@ -449,8 +456,8 @@ void Segment::setGeometry(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, ui } // re-allocate FX render buffer if (length() != oldLength) { - if (pixels) pixels = static_cast(d_realloc(pixels, sizeof(uint32_t) * length())); - else pixels = static_cast(d_malloc(sizeof(uint32_t) * length())); + if (pixels) d_free(pixels); // using realloc on large buffers can cause additional fragmentation instead of reducing it + pixels = static_cast(d_malloc(sizeof(uint32_t) * length())); if (!pixels) { DEBUG_PRINTLN(F("!!! Not enough RAM for pixel buffer !!!")); errorFlag = ERR_NORAM_PX; @@ -563,8 +570,8 @@ Segment &Segment::setName(const char *newName) { if (newName) { const int newLen = min(strlen(newName), (size_t)WLED_MAX_SEGNAME_LEN); if (newLen) { - if (name) name = static_cast(d_realloc(name, newLen+1)); - else name = static_cast(d_malloc(newLen+1)); + if (name) d_free(name); // free old name + name = static_cast(d_malloc(newLen+1)); if (name) strlcpy(name, newName, newLen+1); name[newLen] = 0; return *this; @@ -737,8 +744,8 @@ void IRAM_ATTR_YN Segment::setPixelColor(int i, uint32_t col) const } break; case M12_pCorner: - for (int x = 0; x <= i; x++) setPixelColorRaw(XY(x, i), col); - for (int y = 0; y < i; y++) setPixelColorRaw(XY(i, y), col); + for (int x = 0; x <= i; x++) setPixelColorXY(x, i, col); // note: <= to include i=0. Relies on overflow check in sPC() + for (int y = 0; y < i; y++) setPixelColorXY(i, y, col); break; case M12_sPinwheel: { // Uses Bresenham's algorithm to place coordinates of two lines in arrays then draws between them @@ -1203,8 +1210,8 @@ void WS2812FX::finalizeInit() { deserializeMap(); // (re)load default ledmap (will also setUpMatrix() if ledmap does not exist) // allocate frame buffer after matrix has been set up (gaps!) - if (_pixels) _pixels = static_cast(d_realloc(_pixels, getLengthTotal() * sizeof(uint32_t))); - else _pixels = static_cast(d_malloc(getLengthTotal() * sizeof(uint32_t))); + if (_pixels) d_free(_pixels); // using realloc on large buffers can cause additional fragmentation instead of reducing it + _pixels = static_cast(d_malloc(getLengthTotal() * sizeof(uint32_t))); DEBUG_PRINTF_P(PSTR("strip buffer size: %uB\n"), getLengthTotal() * sizeof(uint32_t)); DEBUG_PRINTF_P(PSTR("Heap after strip init: %uB\n"), ESP.getFreeHeap()); @@ -1691,7 +1698,7 @@ void WS2812FX::setTransitionMode(bool t) { // wait until frame is over (service() has finished or time for 1 frame has passed; yield() crashes on 8266) void WS2812FX::waitForIt() { - unsigned long maxWait = millis() + getFrameTime(); + unsigned long maxWait = millis() + getFrameTime() + 100; // TODO: this needs a proper fix for timeout! while (isServicing() && maxWait > millis()) delay(1); #ifdef WLED_DEBUG if (millis() >= maxWait) DEBUG_PRINTLN(F("Waited for strip to finish servicing.")); diff --git a/wled00/bus_manager.cpp b/wled00/bus_manager.cpp index 3193af4cd..a83b29bde 100644 --- a/wled00/bus_manager.cpp +++ b/wled00/bus_manager.cpp @@ -36,25 +36,32 @@ uint8_t realtimeBroadcast(uint8_t type, IPAddress client, uint16_t length, const //util.cpp // PSRAM allocation wrappers -#ifndef ESP8266 +#if !defined(ESP8266) && !defined(CONFIG_IDF_TARGET_ESP32C3) extern "C" { void *p_malloc(size_t); // prefer PSRAM over DRAM void *p_calloc(size_t, size_t); // prefer PSRAM over DRAM void *p_realloc(void *, size_t); // prefer PSRAM over DRAM + void *p_realloc_malloc(void *ptr, size_t size); // realloc with malloc fallback, prefer PSRAM over DRAM inline void p_free(void *ptr) { heap_caps_free(ptr); } void *d_malloc(size_t); // prefer DRAM over PSRAM void *d_calloc(size_t, size_t); // prefer DRAM over PSRAM void *d_realloc(void *, size_t); // prefer DRAM over PSRAM + void *d_realloc_malloc(void *ptr, size_t size); // realloc with malloc fallback, prefer DRAM over PSRAM inline void d_free(void *ptr) { heap_caps_free(ptr); } } #else +extern "C" { + void *realloc_malloc(void *ptr, size_t size); +} #define p_malloc malloc #define p_calloc calloc #define p_realloc realloc +#define p_realloc_malloc realloc_malloc #define p_free free #define d_malloc malloc #define d_calloc calloc #define d_realloc realloc +#define d_realloc_malloc realloc_malloc #define d_free free #endif diff --git a/wled00/const.h b/wled00/const.h index c19843c42..1abf24539 100644 --- a/wled00/const.h +++ b/wled00/const.h @@ -546,8 +546,8 @@ static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit"); #endif #endif -//#define MIN_HEAP_SIZE -#define MIN_HEAP_SIZE 2048 +// minimum heap size required to process web requests +#define MIN_HEAP_SIZE 8192 // Web server limits #ifdef ESP8266 diff --git a/wled00/data/index.js b/wled00/data/index.js index 295a3403b..f23b5949f 100644 --- a/wled00/data/index.js +++ b/wled00/data/index.js @@ -1527,6 +1527,9 @@ function readState(s,command=false) case 3: errstr = "Buffer locked!"; break; + case 7: + errstr = "No RAM for buffer!"; + break; case 8: errstr = "Effect RAM depleted!"; break; diff --git a/wled00/fcn_declare.h b/wled00/fcn_declare.h index 0b73c90ac..d19f89b27 100644 --- a/wled00/fcn_declare.h +++ b/wled00/fcn_declare.h @@ -551,25 +551,32 @@ inline uint8_t hw_random8(uint32_t upperlimit) { return (hw_random8() * upperlim inline uint8_t hw_random8(uint32_t lowerlimit, uint32_t upperlimit) { uint32_t range = upperlimit - lowerlimit; return lowerlimit + hw_random8(range); }; // input range 0-255 // PSRAM allocation wrappers -#ifndef ESP8266 +#if !defined(ESP8266) && !defined(CONFIG_IDF_TARGET_ESP32C3) extern "C" { void *p_malloc(size_t); // prefer PSRAM over DRAM void *p_calloc(size_t, size_t); // prefer PSRAM over DRAM void *p_realloc(void *, size_t); // prefer PSRAM over DRAM + void *p_realloc_malloc(void *ptr, size_t size); // realloc with malloc fallback, prefer PSRAM over DRAM inline void p_free(void *ptr) { heap_caps_free(ptr); } void *d_malloc(size_t); // prefer DRAM over PSRAM void *d_calloc(size_t, size_t); // prefer DRAM over PSRAM void *d_realloc(void *, size_t); // prefer DRAM over PSRAM + void *d_realloc_malloc(void *ptr, size_t size); // realloc with malloc fallback, prefer DRAM over PSRAM inline void d_free(void *ptr) { heap_caps_free(ptr); } } #else +extern "C" { + void *realloc_malloc(void *ptr, size_t size); +} #define p_malloc malloc #define p_calloc calloc #define p_realloc realloc +#define p_realloc_malloc realloc_malloc #define p_free free #define d_malloc malloc #define d_calloc calloc #define d_realloc realloc +#define d_realloc_malloc realloc_malloc #define d_free free #endif diff --git a/wled00/util.cpp b/wled00/util.cpp index 97e1e3b03..6ff7b05df 100644 --- a/wled00/util.cpp +++ b/wled00/util.cpp @@ -619,7 +619,8 @@ int32_t hw_random(int32_t lowerlimit, int32_t upperlimit) { return hw_random(diff) + lowerlimit; } -#ifndef ESP8266 +#if !defined(ESP8266) && !defined(CONFIG_IDF_TARGET_ESP32C3) // ESP8266 does not support PSRAM, ESP32-C3 does not have PSRAM +// p_x prefer PSRAM, d_x prefer DRAM void *p_malloc(size_t size) { int caps1 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT; int caps2 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT; @@ -640,6 +641,14 @@ void *p_realloc(void *ptr, size_t size) { return heap_caps_realloc(ptr, size, caps2); } +// realloc with malloc fallback, original buffer is freed if realloc fails but not copied! +void *p_realloc_malloc(void *ptr, size_t size) { + void *newbuf = p_realloc(ptr, size); // try realloc first + if (newbuf) return newbuf; // realloc successful + p_free(ptr); // free old buffer if realloc failed + return p_malloc(size); // fallback to malloc +} + void *p_calloc(size_t count, size_t size) { int caps1 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT; int caps2 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT; @@ -654,7 +663,7 @@ void *d_malloc(size_t size) { int caps1 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT; int caps2 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT; if (psramSafe) { - if (size > MIN_HEAP_SIZE) std::swap(caps1, caps2); // prefer PSRAM for large alloactions + if (heap_caps_get_largest_free_block(caps1) < 3*MIN_HEAP_SIZE && size > MIN_HEAP_SIZE) std::swap(caps1, caps2); // prefer PSRAM for large alloactions & when DRAM is low return heap_caps_malloc_prefer(size, 2, caps1, caps2); // otherwise prefer DRAM } return heap_caps_malloc(size, caps1); @@ -664,12 +673,20 @@ void *d_realloc(void *ptr, size_t size) { int caps1 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT; int caps2 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT; if (psramSafe) { - if (size > MIN_HEAP_SIZE) std::swap(caps1, caps2); // prefer PSRAM for large alloactions + if (heap_caps_get_largest_free_block(caps1) < 3*MIN_HEAP_SIZE && size > MIN_HEAP_SIZE) std::swap(caps1, caps2); // prefer PSRAM for large alloactions & when DRAM is low return heap_caps_realloc_prefer(ptr, size, 2, caps1, caps2); // otherwise prefer DRAM } return heap_caps_realloc(ptr, size, caps1); } +// realloc with malloc fallback, original buffer is freed if realloc fails but not copied! +void *d_realloc_malloc(void *ptr, size_t size) { + void *newbuf = d_realloc(ptr, size); // try realloc first + if (newbuf) return newbuf; // realloc successful + d_free(ptr); // free old buffer if realloc failed + return d_malloc(size); // fallback to malloc +} + void *d_calloc(size_t count, size_t size) { int caps1 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT; int caps2 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT; @@ -679,6 +696,14 @@ void *d_calloc(size_t count, size_t size) { } return heap_caps_calloc(count, size, caps1); } +#else // ESP8266 & ESP32-C3 +// realloc with malloc fallback, original buffer is freed if realloc fails but not copied! +void *realloc_malloc(void *ptr, size_t size) { + void *newbuf = realloc(ptr, size); // try realloc first + if (newbuf) return newbuf; // realloc successful + free(ptr); // free old buffer if realloc failed + return malloc(size); // fallback to malloc +} #endif /*