Skip to content

Bugfixes in FX data allocation #4783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DedeHai merged 5 commits into from
Jul 23, 2025
Merged

Bugfixes in FX data allocation #4783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1960066
Bugfixes in FX data allocation
DedeHai Jul 19, 2025
04c9c93
only allow realloc() for pixelbuffer if it grows + bugfix
DedeHai Jul 19, 2025
633eb04
reverting realloc() block as it was cleaner before
DedeHai Jul 19, 2025
3ce0cc7
added realloc_malloc functions, increased min heap constant
DedeHai Jul 20, 2025
716da32
adding missing function declare
DedeHai Jul 20, 2025
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
31 changes: 19 additions & 12 deletions wled00/FX_fcn.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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<uint32_t*>(d_realloc(pixels, sizeof(uint32_t) * length()));
else pixels = static_cast<uint32_t*>(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<uint32_t*>(d_malloc(sizeof(uint32_t) * length()));
if (!pixels) {
DEBUG_PRINTLN(F("!!! Not enough RAM for pixel buffer !!!"));
errorFlag = ERR_NORAM_PX;
Expand Down Expand Up @@ -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<char*>(d_realloc(name, newLen+1));
else name = static_cast<char*>(d_malloc(newLen+1));
if (name) d_free(name); // free old name
name = static_cast<char*>(d_malloc(newLen+1));
if (name) strlcpy(name, newName, newLen+1);
name[newLen] = 0;
return *this;
Expand Down Expand Up @@ -729,8 +736,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
Expand Down Expand Up @@ -1189,8 +1196,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<uint32_t*>(d_realloc(_pixels, getLengthTotal() * sizeof(uint32_t)));
else _pixels = static_cast<uint32_t*>(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<uint32_t*>(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());
Expand Down Expand Up @@ -1677,7 +1684,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."));
Expand Down
9 changes: 8 additions & 1 deletion wled00/bus_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions wled00/const.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions wled00/data/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 8 additions & 1 deletion wled00/fcn_declare.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
31 changes: 28 additions & 3 deletions wled00/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Copy link
Member

@willmmiles willmmiles Jul 22, 2025

Choose a reason for hiding this comment

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

This isn't new in this commit, but one thing to be aware of is that MALLOC_CAP_DEFAULT by itself doesn't mean "use internal memory" -- it means "use the memory pool that malloc() draws from". This can matter if PSRAM is available to malloc(), either because the IDF was built with CONFIG_SPIRAM_USE_MALLOC, or if heap_caps_malloc_extmem_enable() was called; both allow this to return a PSRAM buffer. This isn't necessarily wrong per se -- certainly p_*alloc() above doesn't care -- but it might be worth thinking about here in the d_*alloc() functions.

If we want to insist on using internal memory, use MALLOC_CAP_INTERNAL. If we want to limit it to the pools that malloc() can also reach, use MALLOC_CAP_INTERNAL | MALLOC_CAP_DEFAULT. As you've noticed, there are some internal SRAM pools that malloc() won't touch, such as the block of shared data/instruction RAM intended for dynamically loadable or self-modifying code; and when SPI RAM is enabled, the IDF configuration can reserve some internal memory in a separate non-malloc()-accessible pool for DMA buffers. Using only MALLOC_CAP_INTERNAL will permit usage of those pools, if we want to.

Copy link
Contributor

@blazoncek blazoncek Jul 22, 2025

Choose a reason for hiding this comment

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

By all means! I set up the framework, more skilled people should tune it. 😉

Copy link
Collaborator Author

@DedeHai DedeHai Jul 22, 2025

Choose a reason for hiding this comment

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

Using only MALLOC_CAP_INTERNAL will permit usage of those pools, if we want to.

you beat me by a few hours, I ran tests on exactly this and already prepared a few new function calls that work as intended. Conclusion of my endevours in a nutshell:
S3, S2 and C3 all have IRAM and DRAM as well as RTCRAM enabled to be used as heap. I initially thought they do not but turns out that was a false test.
ESP32 however does not and can not BUT: as we are using all 32bit colors now, we can tap into IRAM which is 32bit accessible only. I tested that and it works, just need to run some more tests and make sure the buffers NEVER get accessed in any other way. It will give us about 50k of extra internal RAM (until more functions are put in IRAM that is).

Copy link
Member

@willmmiles willmmiles Jul 22, 2025

Choose a reason for hiding this comment

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

By all means! I set up the framework, more skilled people should tune it. 😉

Now if you really want to get fancy, you can play fun and games with the MMU to coalesce fragmented blocks to a region of contiguous virtual address space so they look and feel like a flat buffer ... ;)

Copy link
Member

@willmmiles willmmiles Jul 22, 2025

Choose a reason for hiding this comment

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

Now if you really want to get fancy, you can play fun and games with the MMU to coalesce fragmented blocks to a region of contiguous virtual address space so they look and feel like a flat buffer ... ;)

Eh, scrub that. The MMU implementation isn't general enough to be useful for this purpose. It looks like it's meant to be used for task stacks and flash/SPIRAM caching only, pretty much.

Copy link
Contributor

@blazoncek blazoncek Jul 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If we want to insist on using internal memory, use MALLOC_CAP_INTERNAL

Causes bootloop when allocating (and initialising) ledmap on ESP32.
EDIT: ... when paired with MALLOC_CAP_32BIT.

All is well.

int caps2 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT;
Expand All @@ -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

/*
Expand Down