Skip to content

Commit e2f5bec

Browse files
authored
Bugfixes in FX data allocation (#4783)
- Bugfixes in FX data allocation: realloc was not handled properly. - Added *intermediate* fix for waitForIt(), see #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"
1 parent 71301dd commit e2f5bec

File tree

6 files changed

+68
-19
lines changed

6 files changed

+68
-19
lines changed

wled00/FX_fcn.cpp

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,16 @@ bool Segment::allocateData(size_t len) {
159159
return false;
160160
}
161161
// prefer DRAM over SPI RAM on ESP32 since it is slow
162-
if (data) data = (byte*)d_realloc(data, len);
163-
else data = (byte*)d_malloc(len);
162+
if (data) {
163+
data = (byte*)d_realloc_malloc(data, len); // realloc with malloc fallback
164+
if (!data) {
165+
data = nullptr;
166+
Segment::addUsedSegmentData(-_dataLen); // subtract original buffer size
167+
_dataLen = 0; // reset data length
168+
}
169+
}
170+
else data = (byte*)d_malloc(len);
171+
164172
if (data) {
165173
memset(data, 0, len); // erase buffer
166174
Segment::addUsedSegmentData(len - _dataLen);
@@ -170,7 +178,6 @@ bool Segment::allocateData(size_t len) {
170178
}
171179
// allocation failed
172180
DEBUG_PRINTLN(F("!!! Allocation failed. !!!"));
173-
Segment::addUsedSegmentData(-_dataLen); // subtract original buffer size
174181
errorFlag = ERR_NORAM;
175182
return false;
176183
}
@@ -449,8 +456,8 @@ void Segment::setGeometry(uint16_t i1, uint16_t i2, uint8_t grp, uint8_t spc, ui
449456
}
450457
// re-allocate FX render buffer
451458
if (length() != oldLength) {
452-
if (pixels) pixels = static_cast<uint32_t*>(d_realloc(pixels, sizeof(uint32_t) * length()));
453-
else pixels = static_cast<uint32_t*>(d_malloc(sizeof(uint32_t) * length()));
459+
if (pixels) d_free(pixels); // using realloc on large buffers can cause additional fragmentation instead of reducing it
460+
pixels = static_cast<uint32_t*>(d_malloc(sizeof(uint32_t) * length()));
454461
if (!pixels) {
455462
DEBUG_PRINTLN(F("!!! Not enough RAM for pixel buffer !!!"));
456463
errorFlag = ERR_NORAM_PX;
@@ -563,8 +570,8 @@ Segment &Segment::setName(const char *newName) {
563570
if (newName) {
564571
const int newLen = min(strlen(newName), (size_t)WLED_MAX_SEGNAME_LEN);
565572
if (newLen) {
566-
if (name) name = static_cast<char*>(d_realloc(name, newLen+1));
567-
else name = static_cast<char*>(d_malloc(newLen+1));
573+
if (name) d_free(name); // free old name
574+
name = static_cast<char*>(d_malloc(newLen+1));
568575
if (name) strlcpy(name, newName, newLen+1);
569576
name[newLen] = 0;
570577
return *this;
@@ -737,8 +744,8 @@ void IRAM_ATTR_YN Segment::setPixelColor(int i, uint32_t col) const
737744
}
738745
break;
739746
case M12_pCorner:
740-
for (int x = 0; x <= i; x++) setPixelColorRaw(XY(x, i), col);
741-
for (int y = 0; y < i; y++) setPixelColorRaw(XY(i, y), col);
747+
for (int x = 0; x <= i; x++) setPixelColorXY(x, i, col); // note: <= to include i=0. Relies on overflow check in sPC()
748+
for (int y = 0; y < i; y++) setPixelColorXY(i, y, col);
742749
break;
743750
case M12_sPinwheel: {
744751
// Uses Bresenham's algorithm to place coordinates of two lines in arrays then draws between them
@@ -1203,8 +1210,8 @@ void WS2812FX::finalizeInit() {
12031210
deserializeMap(); // (re)load default ledmap (will also setUpMatrix() if ledmap does not exist)
12041211

12051212
// allocate frame buffer after matrix has been set up (gaps!)
1206-
if (_pixels) _pixels = static_cast<uint32_t*>(d_realloc(_pixels, getLengthTotal() * sizeof(uint32_t)));
1207-
else _pixels = static_cast<uint32_t*>(d_malloc(getLengthTotal() * sizeof(uint32_t)));
1213+
if (_pixels) d_free(_pixels); // using realloc on large buffers can cause additional fragmentation instead of reducing it
1214+
_pixels = static_cast<uint32_t*>(d_malloc(getLengthTotal() * sizeof(uint32_t)));
12081215
DEBUG_PRINTF_P(PSTR("strip buffer size: %uB\n"), getLengthTotal() * sizeof(uint32_t));
12091216

12101217
DEBUG_PRINTF_P(PSTR("Heap after strip init: %uB\n"), ESP.getFreeHeap());
@@ -1691,7 +1698,7 @@ void WS2812FX::setTransitionMode(bool t) {
16911698

16921699
// wait until frame is over (service() has finished or time for 1 frame has passed; yield() crashes on 8266)
16931700
void WS2812FX::waitForIt() {
1694-
unsigned long maxWait = millis() + getFrameTime();
1701+
unsigned long maxWait = millis() + getFrameTime() + 100; // TODO: this needs a proper fix for timeout!
16951702
while (isServicing() && maxWait > millis()) delay(1);
16961703
#ifdef WLED_DEBUG
16971704
if (millis() >= maxWait) DEBUG_PRINTLN(F("Waited for strip to finish servicing."));

wled00/bus_manager.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,25 +36,32 @@ uint8_t realtimeBroadcast(uint8_t type, IPAddress client, uint16_t length, const
3636

3737
//util.cpp
3838
// PSRAM allocation wrappers
39-
#ifndef ESP8266
39+
#if !defined(ESP8266) && !defined(CONFIG_IDF_TARGET_ESP32C3)
4040
extern "C" {
4141
void *p_malloc(size_t); // prefer PSRAM over DRAM
4242
void *p_calloc(size_t, size_t); // prefer PSRAM over DRAM
4343
void *p_realloc(void *, size_t); // prefer PSRAM over DRAM
44+
void *p_realloc_malloc(void *ptr, size_t size); // realloc with malloc fallback, prefer PSRAM over DRAM
4445
inline void p_free(void *ptr) { heap_caps_free(ptr); }
4546
void *d_malloc(size_t); // prefer DRAM over PSRAM
4647
void *d_calloc(size_t, size_t); // prefer DRAM over PSRAM
4748
void *d_realloc(void *, size_t); // prefer DRAM over PSRAM
49+
void *d_realloc_malloc(void *ptr, size_t size); // realloc with malloc fallback, prefer DRAM over PSRAM
4850
inline void d_free(void *ptr) { heap_caps_free(ptr); }
4951
}
5052
#else
53+
extern "C" {
54+
void *realloc_malloc(void *ptr, size_t size);
55+
}
5156
#define p_malloc malloc
5257
#define p_calloc calloc
5358
#define p_realloc realloc
59+
#define p_realloc_malloc realloc_malloc
5460
#define p_free free
5561
#define d_malloc malloc
5662
#define d_calloc calloc
5763
#define d_realloc realloc
64+
#define d_realloc_malloc realloc_malloc
5865
#define d_free free
5966
#endif
6067

wled00/const.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,8 +546,8 @@ static_assert(WLED_MAX_BUSSES <= 32, "WLED_MAX_BUSSES exceeds hard limit");
546546
#endif
547547
#endif
548548

549-
//#define MIN_HEAP_SIZE
550-
#define MIN_HEAP_SIZE 2048
549+
// minimum heap size required to process web requests
550+
#define MIN_HEAP_SIZE 8192
551551

552552
// Web server limits
553553
#ifdef ESP8266

wled00/data/index.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,6 +1527,9 @@ function readState(s,command=false)
15271527
case 3:
15281528
errstr = "Buffer locked!";
15291529
break;
1530+
case 7:
1531+
errstr = "No RAM for buffer!";
1532+
break;
15301533
case 8:
15311534
errstr = "Effect RAM depleted!";
15321535
break;

wled00/fcn_declare.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,25 +551,32 @@ inline uint8_t hw_random8(uint32_t upperlimit) { return (hw_random8() * upperlim
551551
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
552552

553553
// PSRAM allocation wrappers
554-
#ifndef ESP8266
554+
#if !defined(ESP8266) && !defined(CONFIG_IDF_TARGET_ESP32C3)
555555
extern "C" {
556556
void *p_malloc(size_t); // prefer PSRAM over DRAM
557557
void *p_calloc(size_t, size_t); // prefer PSRAM over DRAM
558558
void *p_realloc(void *, size_t); // prefer PSRAM over DRAM
559+
void *p_realloc_malloc(void *ptr, size_t size); // realloc with malloc fallback, prefer PSRAM over DRAM
559560
inline void p_free(void *ptr) { heap_caps_free(ptr); }
560561
void *d_malloc(size_t); // prefer DRAM over PSRAM
561562
void *d_calloc(size_t, size_t); // prefer DRAM over PSRAM
562563
void *d_realloc(void *, size_t); // prefer DRAM over PSRAM
564+
void *d_realloc_malloc(void *ptr, size_t size); // realloc with malloc fallback, prefer DRAM over PSRAM
563565
inline void d_free(void *ptr) { heap_caps_free(ptr); }
564566
}
565567
#else
568+
extern "C" {
569+
void *realloc_malloc(void *ptr, size_t size);
570+
}
566571
#define p_malloc malloc
567572
#define p_calloc calloc
568573
#define p_realloc realloc
574+
#define p_realloc_malloc realloc_malloc
569575
#define p_free free
570576
#define d_malloc malloc
571577
#define d_calloc calloc
572578
#define d_realloc realloc
579+
#define d_realloc_malloc realloc_malloc
573580
#define d_free free
574581
#endif
575582

wled00/util.cpp

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,8 @@ int32_t hw_random(int32_t lowerlimit, int32_t upperlimit) {
619619
return hw_random(diff) + lowerlimit;
620620
}
621621

622-
#ifndef ESP8266
622+
#if !defined(ESP8266) && !defined(CONFIG_IDF_TARGET_ESP32C3) // ESP8266 does not support PSRAM, ESP32-C3 does not have PSRAM
623+
// p_x prefer PSRAM, d_x prefer DRAM
623624
void *p_malloc(size_t size) {
624625
int caps1 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT;
625626
int caps2 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT;
@@ -640,6 +641,14 @@ void *p_realloc(void *ptr, size_t size) {
640641
return heap_caps_realloc(ptr, size, caps2);
641642
}
642643

644+
// realloc with malloc fallback, original buffer is freed if realloc fails but not copied!
645+
void *p_realloc_malloc(void *ptr, size_t size) {
646+
void *newbuf = p_realloc(ptr, size); // try realloc first
647+
if (newbuf) return newbuf; // realloc successful
648+
p_free(ptr); // free old buffer if realloc failed
649+
return p_malloc(size); // fallback to malloc
650+
}
651+
643652
void *p_calloc(size_t count, size_t size) {
644653
int caps1 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT;
645654
int caps2 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT;
@@ -654,7 +663,7 @@ void *d_malloc(size_t size) {
654663
int caps1 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT;
655664
int caps2 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT;
656665
if (psramSafe) {
657-
if (size > MIN_HEAP_SIZE) std::swap(caps1, caps2); // prefer PSRAM for large alloactions
666+
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
658667
return heap_caps_malloc_prefer(size, 2, caps1, caps2); // otherwise prefer DRAM
659668
}
660669
return heap_caps_malloc(size, caps1);
@@ -664,12 +673,20 @@ void *d_realloc(void *ptr, size_t size) {
664673
int caps1 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT;
665674
int caps2 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT;
666675
if (psramSafe) {
667-
if (size > MIN_HEAP_SIZE) std::swap(caps1, caps2); // prefer PSRAM for large alloactions
676+
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
668677
return heap_caps_realloc_prefer(ptr, size, 2, caps1, caps2); // otherwise prefer DRAM
669678
}
670679
return heap_caps_realloc(ptr, size, caps1);
671680
}
672681

682+
// realloc with malloc fallback, original buffer is freed if realloc fails but not copied!
683+
void *d_realloc_malloc(void *ptr, size_t size) {
684+
void *newbuf = d_realloc(ptr, size); // try realloc first
685+
if (newbuf) return newbuf; // realloc successful
686+
d_free(ptr); // free old buffer if realloc failed
687+
return d_malloc(size); // fallback to malloc
688+
}
689+
673690
void *d_calloc(size_t count, size_t size) {
674691
int caps1 = MALLOC_CAP_DEFAULT | MALLOC_CAP_8BIT;
675692
int caps2 = MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT;
@@ -679,6 +696,14 @@ void *d_calloc(size_t count, size_t size) {
679696
}
680697
return heap_caps_calloc(count, size, caps1);
681698
}
699+
#else // ESP8266 & ESP32-C3
700+
// realloc with malloc fallback, original buffer is freed if realloc fails but not copied!
701+
void *realloc_malloc(void *ptr, size_t size) {
702+
void *newbuf = realloc(ptr, size); // try realloc first
703+
if (newbuf) return newbuf; // realloc successful
704+
free(ptr); // free old buffer if realloc failed
705+
return malloc(size); // fallback to malloc
706+
}
682707
#endif
683708

684709
/*

0 commit comments

Comments
 (0)