Skip to content

Commit c4204d3

Browse files
committed
fix(cam_hal): replace recursive cam_take() with bounded loop
– prevents stack-overflow in cam_task 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.
1 parent 920996f commit c4204d3

File tree

1 file changed

+49
-31
lines changed

1 file changed

+49
-31
lines changed

driver/cam_hal.c

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -477,47 +477,65 @@ void cam_start(void)
477477
camera_fb_t *cam_take(TickType_t timeout)
478478
{
479479
camera_fb_t *dma_buffer = NULL;
480-
TickType_t start = xTaskGetTickCount();
481-
xQueueReceive(cam_obj->frame_buffer_queue, (void *)&dma_buffer, timeout);
480+
const TickType_t start = xTaskGetTickCount();
482481
#if CONFIG_IDF_TARGET_ESP32S3
483-
// Currently (22.01.2024) there is a bug in ESP-IDF v5.2, that causes
484-
// GDMA to fall into a strange state if it is running while WiFi STA is connecting.
485-
// This code tries to reset GDMA if frame is not received, to try and help with
486-
// this case. It is possible to have some side effects too, though none come to mind
487-
if (!dma_buffer) {
488-
ll_cam_dma_reset(cam_obj);
489-
xQueueReceive(cam_obj->frame_buffer_queue, (void *)&dma_buffer, timeout);
490-
}
482+
uint16_t dma_reset_counter = 0;
483+
static const uint8_t MAX_GDMA_RESETS = 3;
484+
#endif
485+
486+
for (;;)
487+
{
488+
TickType_t elapsed = xTaskGetTickCount() - start; /* TickType_t is unsigned so rollover is safe */
489+
if (elapsed >= timeout) {
490+
ESP_LOGW(TAG, "Failed to get frame: timeout");
491+
return NULL;
492+
}
493+
TickType_t remaining = timeout - elapsed;
494+
495+
if (xQueueReceive(cam_obj->frame_buffer_queue, (void *)&dma_buffer, remaining) == pdFALSE) {
496+
continue;
497+
}
498+
499+
if (!dma_buffer) {
500+
/* Work-around for ESP32-S3 GDMA freeze when Wi-Fi STA starts.
501+
* See esp32-camera commit 984999f (issue #620). */
502+
#if CONFIG_IDF_TARGET_ESP32S3
503+
if (dma_reset_counter < MAX_GDMA_RESETS) {
504+
ll_cam_dma_reset(cam_obj);
505+
dma_reset_counter++;
506+
continue; /* retry with queue timeout */
507+
}
508+
if (dma_reset_counter == MAX_GDMA_RESETS) {
509+
ESP_LOGW(TAG, "Giving up GDMA reset after %u tries", dma_reset_counter);
510+
dma_reset_counter++; /* suppress further logs */
511+
}
512+
#else
513+
/* Early warning for misbehaving sensors on other chips */
514+
ESP_LOGW(TAG, "Unexpected NULL frame on %s", CONFIG_IDF_TARGET);
491515
#endif
492-
if (dma_buffer) {
493-
if(cam_obj->jpeg_mode){
494-
// find the end marker for JPEG. Data after that can be discarded
516+
vTaskDelay(1); /* immediate yield once resets are done */
517+
continue; /* go to top of loop */
518+
}
519+
520+
if (cam_obj->jpeg_mode) {
521+
/* find the end marker for JPEG. Data after that can be discarded */
495522
int offset_e = cam_verify_jpeg_eoi(dma_buffer->buf, dma_buffer->len);
496523
if (offset_e >= 0) {
497-
// adjust buffer length
498524
dma_buffer->len = offset_e + sizeof(JPEG_EOI_MARKER);
499525
return dma_buffer;
500-
} else {
501-
ESP_LOGW(TAG, "NO-EOI");
502-
cam_give(dma_buffer);
503-
TickType_t ticks_spent = xTaskGetTickCount() - start;
504-
if (ticks_spent >= timeout) {
505-
return NULL; /* We are out of time */
506-
}
507-
return cam_take(timeout - ticks_spent);//recurse!!!!
508526
}
509-
} else if(cam_obj->psram_mode && cam_obj->in_bytes_per_pixel != cam_obj->fb_bytes_per_pixel){
510-
//currently this is used only for YUV to GRAYSCALE
527+
528+
ESP_LOGW(TAG, "NO-EOI");
529+
cam_give(dma_buffer);
530+
continue; /* wait for another frame */
531+
} else if (cam_obj->psram_mode &&
532+
cam_obj->in_bytes_per_pixel != cam_obj->fb_bytes_per_pixel) {
533+
/* currently used only for YUV to GRAYSCALE */
511534
dma_buffer->len = ll_cam_memcpy(cam_obj, dma_buffer->buf, dma_buffer->buf, dma_buffer->len);
512535
}
536+
513537
return dma_buffer;
514-
} else {
515-
ESP_LOGW(TAG, "Failed to get the frame on time!");
516-
// #if CONFIG_IDF_TARGET_ESP32S3
517-
// ll_cam_dma_print_state(cam_obj);
518-
// #endif
519538
}
520-
return NULL;
521539
}
522540

523541
void cam_give(camera_fb_t *dma_buffer)
@@ -539,4 +557,4 @@ void cam_give_all(void) {
539557
bool cam_get_available_frames(void)
540558
{
541559
return 0 < uxQueueMessagesWaiting(cam_obj->frame_buffer_queue);
542-
}
560+
}

0 commit comments

Comments
 (0)