Skip to content

Harden post-mix WAV recording failures - #20

Open
djdefi wants to merge 7 commits into
CircuitMess:masterfrom
djdefi:djdefi-recording-reliability-core
Open

djdefi wants to merge 7 commits into
CircuitMess:masterfrom
djdefi:djdefi-recording-reliability-core

Conversation

@djdefi

@djdefi djdefi commented Aug 22, 2026 •

Copy link
Copy Markdown

Depends on #19 (4ad5108); merge #19 first. The recording changes are directly based on that head, so the diff will collapse to the focused recording change when #19 lands.

The recording path added in d74ec00/d2018b6 treated an attached splitter output as success, spun when recorder buffers filled, ignored short SD writes, and closed before queued finalization was proven. This keeps the post-mix OutputSplitter tap but adds explicit IDLE/STARTING/RECORDING/STOPPING/COMPLETE/FAILED status, concrete SD/open/write/finalize/overrun/queue errors, and written-byte, duration, dropped-byte, retry, and file-validity reporting.

Live start only dispatches a request. The audio task opens the temp file, then initial WAV seek/header setup advances asynchronously through scheduler queue/wait states while mixer/I2S output continues; the recorder is attached only after the zero-length header is confirmed. Existing decoders/encoders retain the scheduler's legacy blocking addJob() contract; OutputWAV alone uses zero-timeout tryAddJob(), so recording queue pressure is nonblocking without changing playback job semantics. Initial and final header queue rejection stays pending and retries up to 32 times; exhaustion reports an error with fileValid=false. Buffer exhaustion or short/full/removed-SD writes detach recording without stalling playback. Immediate/repeated start-stop, rejected stop dispatch, queued teardown, failed restart validity, and duplicate attachment are guarded explicitly.

Validation:

  • Sanitized host self-check covers header sizes/rewrite, queue rejection→recovery/exhaustion, delayed result wait states, audio-thread dispatch ownership, no explicit waits in initial setup/service, scoped nonblocking scheduler enqueue, attach-after-ready, and transition serialization
  • arduino-cli compile --fqbn cm:esp32:jayd
  • Focused scheduler compatibility review: no findings
  • Size vs Fix Mixer clip() overflow bug; add per-channel hot-swap loading #19: +248 bytes flash, -8 bytes global RAM

Deliberately unchanged: firmware naming/UI, AAC conversion, WiFi, DjSession, automix, the existing per-write job allocation model, and synchronous SD.exists/remove/open primitives. Those calls contain no explicit wait loop here; moving filesystem open itself off the audio task requires a broader serialized FS scheduler API.

djdefi and others added 6 commits August 21, 2026 14:28
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1b2cfc57-c6c7-44ce-95c9-72368d409ca8
Add explicit recording status and errors, detect short/failed SD writes without blocking playback, and finalize WAV headers through the existing scheduler before closing files.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep live recorder setup on the audio task and retry finalization queue pressure before reporting an invalid file.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Drive initial WAV seek and header writes from the service state machine without blocking mixer output, and make SD scheduler enqueue genuinely nonblocking.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve the scheduler contract used by existing decoders and encoders while keeping OutputWAV queue retries nonblocking.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
djdefi added a commit to djdefi/JayD-Library that referenced this pull request Aug 25, 2026
PR CircuitMess#20 advanced to ac6ba80 ("Scope nonblocking SD enqueue to
recording"), reverting SDScheduler::addJob() back to the legacy
blocking void contract and adding a new nonblocking bool tryAddJob()
scoped exclusively to OutputWAV's real-time recording write/finalize
path. That makes the earlier 78a9f3d fix (which retrofitted bool-return
checking onto every addJob() caller, matching an intermediate
nonblocking-everywhere revision of CircuitMess#20) obsolete and non-compiling
against the restored void signature.

Revert SourceAAC::addReadJob/seekSourceFrame, SourceMP3::addReadJob,
SourceWAV::addReadJob, and OutputAAC::addWriteJob to plain blocking
Sched.addJob(...) calls with no return-value check, matching the
restored contract. OutputWAV's two call sites already correctly use
the new Sched.tryAddJob(...) (renamed by ac6ba80 from its prior
addJob() bool usage) with the return value checked; no changes
needed there.

Queue-ownership review: Sched is drained both by the main sketch loop
(LoopManager::addListener(&Sched)) and, at specific synchronous wait
points, directly by the calling thread (e.g. MixSystem open/openChannel
busy-wait via Sched.loop(0) while awaiting isReadReady()). The audio
task's blocking addJob() calls (SourceAAC/MP3/WAV read jobs, OutputAAC
write jobs) are drained by a different, independent thread/task, so a
full queue blocks that call only until the drain thread services it;
no self-deadlock. OutputWAV's write/finalize path runs on the same
real-time audio task and must not block it, which is exactly why
tryAddJob()'s nonblocking, checked, retry-on-false contract remains
scoped there.

Rewrite tests/JobQueueContractSelfCheck.cpp for the new dual-API
contract: structural checks assert SourceAAC/SourceMP3/SourceWAV/
OutputAAC call only the blocking Sched.addJob and never
Sched.tryAddJob, OutputWAV calls only Sched.tryAddJob with every call
site's return checked and never the blocking Sched.addJob, and
SDScheduler.h declares both signatures. A functional harness
(mirroring OutputWAV::addWriteJob) proves queue-full rejection under
tryAddJob() still leaves the buffer retryable with no leak/false
pending state, plus a "buggy" counterpart proving the check
discriminates a regression of the original defect class.

All four host self-checks (adts_timing_self_check,
run-speed-modifier-self-check.sh, wav_header_selfcheck,
JobQueueContractSelfCheck) pass with -Wall -Wextra -Werror
-fsanitize=address,undefined.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant