Join the second-core render with a dedicated semaphore - #1137
Merged
Conversation
esp_render_on_cores() joined the worker with a plain task notification on the calling task's default index. Any app that also paces that task with task notifications - a timer-driven block clock notifying the task that runs amy_update() is the natural shape - shares that counter: a tick landing inside the join window releases the caller before the worker finishes, and the caller combines a half-written second-core buffer. The result is intermittent, load-dependent corruption in multicore configs. Replace the caller-side join with a dedicated binary semaphore that nothing else can signal, and retire amy_render_task_done_handle. The worker kick keeps its notification; the worker task is AMY-internal and nothing else notifies it. Seen on hardware (ESP32-S3): a GPTimer block clock notifying the render task produced intermittent dual-core-only garbling that stopped once the join no longer shared the notification counter.
Collaborator
⛓️ tulipcc integration PR openedThis merge was pinned into tulipcc for full-system CI: shorepine/tulipcc#1337 Test it there and merge that PR to move tulipcc onto this AMY. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
esp_render_on_cores()joins the second-core worker with a plain tasknotification on the calling task's default notification index. That counter is
not private to the join: the calling task belongs to the app (with
multithread = 0it is whatever task runsamy_update()), and pacing such atask with task notifications is idiomatic FreeRTOS - a timer ISR giving one
notification per audio block, for example. When a tick lands inside the join
window, the join returns on the tick instead of the worker: the caller runs
amy_fill_buffer()while the worker is still writingfbl[1]and combines ahalf-written buffer. The worker's real "done" give then miscounts the app's
next wait, desyncing its clock. It presents as intermittent, load-dependent
garbling that only appears in multicore configs.
This replaces the caller-side join with a dedicated binary semaphore, which
nothing else can signal, and retires
amy_render_task_done_handle. The workerkick keeps its notification: the worker task is AMY-internal and nothing else
notifies it.
Indexed notifications (
xTaskNotifyGiveIndexedon a non-zero index) wouldalso work but require
configTASK_NOTIFICATION_ARRAY_ENTRIES > 1, which thesupported cores don't all guarantee; the semaphore is portable.
Verification
A/B on ESP32-P4 (ESP-IDF 6.1,
multicore=1,multithread=0), with theamy_update()task paced by a GPTimer ISR giving one default-index tasknotification per block - the pacing shape described above. To make the
overlap deterministic, the second-core render window was stretched past the
5.33 ms block period so every join spans a tick; a worker-busy flag checked
at join return counts early releases.
essentially every block), the first within 3 s of boot, with the expected
knock-on desync as the join consumes the app's pacing ticks.
(194,000 joins) ran with no deadlock and no missed completion.
Originally observed on ESP32-S3 with the same GPTimer pacing: intermittent,
load-dependent dual-core corruption that stopped once the join no longer
shared the task's notification counter. Compile-verified on ESP32-S3
(ESP-IDF 6.0.2) as well.