From c4204d31bf2c422b2b46e2f27026deb6bb1cd261 Mon Sep 17 00:00:00 2001 From: RubenKelevra Date: Mon, 30 Jun 2025 20:45:45 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(cam=5Fhal):=20replace=20recursive=20cam?= =?UTF-8?q?=5Ftake()=20with=20bounded=20loop=20=20=20=20=20=20=20=20=20=20?= =?UTF-8?q?=20=20=20=20=20=20=E2=80=93=20prevents=20stack-overflow=20in=20?= =?UTF-8?q?cam=5Ftask?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old cam_take() used recursion to retry if * no JPEG EOI was found or * a NULL frame pointer was returned (GDMA glitch on ESP32-S3). Under heavy loss conditions this could overflow the cam_task stack and reboot the whole system. * Re-wrote cam_take() as a single while-loop – stack depth is now constant and independent of retry count. * Added strict timeout tracking (`remaining = timeout - elapsed`); function can never block longer than the caller’s timeout. * ESP32-S3 only * capped GDMA reset storms to 3 attempts (`MAX_GDMA_RESETS`) * logs one “giving up” warning, then yields (`vTaskDelay(1)`) to avoid busy-spin after hardware is wedged. * Non-S3 targets * emit early `ESP_LOGW` when a NULL frame ever appears, then yield one tick per loop to prevent CPU thrash. * Maintained existing JPEG-EOI trimming and YUV→GRAY memcpy paths; behaviour unchanged on successful frames. * Inline comment links to esp32-camera commit 984999f / issue #620 for future context. --- driver/cam_hal.c | 80 +++++++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/driver/cam_hal.c b/driver/cam_hal.c index 4daaee7264..1890218ae4 100644 --- a/driver/cam_hal.c +++ b/driver/cam_hal.c @@ -477,47 +477,65 @@ void cam_start(void) camera_fb_t *cam_take(TickType_t timeout) { camera_fb_t *dma_buffer = NULL; - TickType_t start = xTaskGetTickCount(); - xQueueReceive(cam_obj->frame_buffer_queue, (void *)&dma_buffer, timeout); + const TickType_t start = xTaskGetTickCount(); #if CONFIG_IDF_TARGET_ESP32S3 - // Currently (22.01.2024) there is a bug in ESP-IDF v5.2, that causes - // GDMA to fall into a strange state if it is running while WiFi STA is connecting. - // This code tries to reset GDMA if frame is not received, to try and help with - // this case. It is possible to have some side effects too, though none come to mind - if (!dma_buffer) { - ll_cam_dma_reset(cam_obj); - xQueueReceive(cam_obj->frame_buffer_queue, (void *)&dma_buffer, timeout); - } + uint16_t dma_reset_counter = 0; + static const uint8_t MAX_GDMA_RESETS = 3; +#endif + + for (;;) + { + TickType_t elapsed = xTaskGetTickCount() - start; /* TickType_t is unsigned so rollover is safe */ + if (elapsed >= timeout) { + ESP_LOGW(TAG, "Failed to get frame: timeout"); + return NULL; + } + TickType_t remaining = timeout - elapsed; + + if (xQueueReceive(cam_obj->frame_buffer_queue, (void *)&dma_buffer, remaining) == pdFALSE) { + continue; + } + + if (!dma_buffer) { + /* Work-around for ESP32-S3 GDMA freeze when Wi-Fi STA starts. + * See esp32-camera commit 984999f (issue #620). */ +#if CONFIG_IDF_TARGET_ESP32S3 + if (dma_reset_counter < MAX_GDMA_RESETS) { + ll_cam_dma_reset(cam_obj); + dma_reset_counter++; + continue; /* retry with queue timeout */ + } + if (dma_reset_counter == MAX_GDMA_RESETS) { + ESP_LOGW(TAG, "Giving up GDMA reset after %u tries", dma_reset_counter); + dma_reset_counter++; /* suppress further logs */ + } +#else + /* Early warning for misbehaving sensors on other chips */ + ESP_LOGW(TAG, "Unexpected NULL frame on %s", CONFIG_IDF_TARGET); #endif - if (dma_buffer) { - if(cam_obj->jpeg_mode){ - // find the end marker for JPEG. Data after that can be discarded + vTaskDelay(1); /* immediate yield once resets are done */ + continue; /* go to top of loop */ + } + + if (cam_obj->jpeg_mode) { + /* find the end marker for JPEG. Data after that can be discarded */ int offset_e = cam_verify_jpeg_eoi(dma_buffer->buf, dma_buffer->len); if (offset_e >= 0) { - // adjust buffer length dma_buffer->len = offset_e + sizeof(JPEG_EOI_MARKER); return dma_buffer; - } else { - ESP_LOGW(TAG, "NO-EOI"); - cam_give(dma_buffer); - TickType_t ticks_spent = xTaskGetTickCount() - start; - if (ticks_spent >= timeout) { - return NULL; /* We are out of time */ - } - return cam_take(timeout - ticks_spent);//recurse!!!! } - } else if(cam_obj->psram_mode && cam_obj->in_bytes_per_pixel != cam_obj->fb_bytes_per_pixel){ - //currently this is used only for YUV to GRAYSCALE + + ESP_LOGW(TAG, "NO-EOI"); + cam_give(dma_buffer); + continue; /* wait for another frame */ + } else if (cam_obj->psram_mode && + cam_obj->in_bytes_per_pixel != cam_obj->fb_bytes_per_pixel) { + /* currently used only for YUV to GRAYSCALE */ dma_buffer->len = ll_cam_memcpy(cam_obj, dma_buffer->buf, dma_buffer->buf, dma_buffer->len); } + return dma_buffer; - } else { - ESP_LOGW(TAG, "Failed to get the frame on time!"); -// #if CONFIG_IDF_TARGET_ESP32S3 -// ll_cam_dma_print_state(cam_obj); -// #endif } - return NULL; } void cam_give(camera_fb_t *dma_buffer) @@ -539,4 +557,4 @@ void cam_give_all(void) { bool cam_get_available_frames(void) { return 0 < uxQueueMessagesWaiting(cam_obj->frame_buffer_queue); -} \ No newline at end of file +} From c439f081690abe2a6d5c04032a4dbba4b3b03329 Mon Sep 17 00:00:00 2001 From: RubenKelevra Date: Tue, 1 Jul 2025 21:31:23 +0200 Subject: [PATCH 2/2] cam_hal: throttle WARN spam so cam_task reduces it's stack footprint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Background ---------- • Every bad frame produced an ESP_LOGW(): - “Unexpected NULL frame …” (non-S3) - “NO-EOI – JPEG end marker missing” • Each ESP_LOGW() invokes vsnprintf() once ⇒ ~300 B of stack for that call. What’s changed -------------- * First occurrence of each warning: ESP_DRAM_LOGW_ONCE() → zero stack after the first print. * Subsequent occurrences: count with a uint16_t and emit a **literal** ESP_LOGW() only every 100th event (~ 60 B stack usage). * Counters auto-reset at 10 000 to avoid wraparound. * On ESP32-S3 the “Giving up GDMA reset…” message is also moved to ESP_DRAM_LOGW_ONCE() to avoid stack usage. Effects ------- * Occasional diagnostics are still available: first warning and then a summary every 100 events. --- driver/cam_hal.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/driver/cam_hal.c b/driver/cam_hal.c index 1890218ae4..c9b746b021 100644 --- a/driver/cam_hal.c +++ b/driver/cam_hal.c @@ -481,7 +481,12 @@ camera_fb_t *cam_take(TickType_t timeout) #if CONFIG_IDF_TARGET_ESP32S3 uint16_t dma_reset_counter = 0; static const uint8_t MAX_GDMA_RESETS = 3; +#else + /* throttle repeated NULL frame warnings */ + static uint16_t warn_null_cnt; #endif + /* throttle repeated NO-EOI warnings */ + static uint16_t warn_eoi_miss_cnt; for (;;) { @@ -506,12 +511,21 @@ camera_fb_t *cam_take(TickType_t timeout) continue; /* retry with queue timeout */ } if (dma_reset_counter == MAX_GDMA_RESETS) { - ESP_LOGW(TAG, "Giving up GDMA reset after %u tries", dma_reset_counter); + ESP_DRAM_LOGW(TAG, "Giving up GDMA reset after %u tries", dma_reset_counter); dma_reset_counter++; /* suppress further logs */ } #else /* Early warning for misbehaving sensors on other chips */ - ESP_LOGW(TAG, "Unexpected NULL frame on %s", CONFIG_IDF_TARGET); + if (warn_null_cnt == 0) { + ESP_DRAM_LOGW(TAG, "Unexpected NULL frame on " CONFIG_IDF_TARGET); + } + if (++warn_null_cnt % 100 == 0) { + ESP_LOGW(TAG, "Unexpected NULL frame – 100 additional drops"); + } + if (warn_null_cnt >= 10000) { + ESP_LOGW(TAG, "Unexpected NULL frame log counter reset at 10000"); + warn_null_cnt = 0; + } #endif vTaskDelay(1); /* immediate yield once resets are done */ continue; /* go to top of loop */ @@ -525,7 +539,16 @@ camera_fb_t *cam_take(TickType_t timeout) return dma_buffer; } - ESP_LOGW(TAG, "NO-EOI"); + if (warn_eoi_miss_cnt == 0) { + ESP_DRAM_LOGW(TAG, "NO-EOI – JPEG end marker missing"); + } + if (++warn_eoi_miss_cnt % 100 == 0) { + ESP_LOGW(TAG, "NO-EOI – 100 additional misses"); + } + if (warn_eoi_miss_cnt >= 10000) { + ESP_LOGW(TAG, "NO-EOI counter reset at 10000"); + warn_eoi_miss_cnt = 0; + } cam_give(dma_buffer); continue; /* wait for another frame */ } else if (cam_obj->psram_mode &&