Integrate deck-rate + recording-core final commits onto AAC-timing base (for JayD-Firmware)

[djdefi]

What

Combines the three independently-reviewed library foundations JayD-Firmware needs, on one branch/SHA:

• Base djdefi-aac-frame-timing (6750262) — already contains Fix Mixer clip() overflow bug; add per-channel hot-swap loading #19 mixer hot-swap fix (4ad5108) + Add frame-accurate AAC timing and seek #21's frame-accurate AAC timing/seek final fix.
• Cherry-picked deck-rate final commit 2afe71c (Q16.16 rate control, clean pick, no conflicts).
• Cherry-picked Harden post-mix WAV recording failures #20's final reviewed commits (6caffb5, 5230446, a4e0fdb, ac6ba80; two co-authorship-only commits from that branch were empty after picking and were skipped, not applied as no-ops).

All foundation commits are preserved as-is (-x cherry-picks, unamended).

Final SHA (superseded, see updates below): 35b4f80a47f5e5eef029e0c57db76699a076df4a.

Conflict #1 — SpeedModifier::reset() vs. sourcePosition rewrite

Textually all cherry-picks applied clean via git cherry-pick -x, but that hid a semantic break: deck-rate's rewrite of SpeedModifier replaced the old float remainder accumulator with a Q16.16 sourcePosition, while AAC-timing's reset() (added on a different branch, same base) still zeroed remainder. Since the two edits touched non-overlapping hunks, git merged them silently — the tree didn't build. Fixed in 5aa22cc (reset() now zeroes sourcePosition), plus one integration self-check in bb4c945 covering the exact contract this broke: a post-seek reset() must drop stale resampling state but must not touch the requested/ramped rate.

Conflict #2 — SDScheduler::addJob() bool contract vs. call sites (found by independent review after bb4c945)

Recording's rewrite of SDScheduler::addJob() to return bool (queue full → delete job, return false, non-blocking xQueueSend(..., 0)) wasn't honored at every call site merged from the other branches. SourceAAC::addReadJob and SourceAAC::seekSourceFrame ignored the return value: on a full queue (capacity 8, shared by two decks + recording), a rejected enqueue still left readJobPending = true (with readResult never arriving, so processReadJob(true)/open() busy-wait forever — watchdog risk) or silently committed a seek that was never actually queued.

Fixed in 78a9f3d: SourceAAC/SourceMP3/SourceWAV's read-job and SourceAAC's seek call sites only committed state on a successful enqueue; OutputAAC::addWriteJob no longer stranded its buffer on a failed enqueue. Added tests/JobQueueContractSelfCheck.cpp covering the fixed state machine (queue-full rejection + recovery, bounded close/teardown-no-hang), verified against the pre-fix regression.

This revision (Conflict #2) has since been superseded by Conflict #3 below — #20 changed its own contract again upstream, and 78a9f3d's fix no longer matches what ships. It's kept here for lineage; do not treat it as the current call-site contract.

Conflict #3 — #20 restored the legacy blocking addJob() contract; reconcile the #2 fix

#20 advanced past a4e0fdb to ac6ba80 ("Scope nonblocking SD enqueue to recording"), which reverts SDScheduler::addJob() back to legacy blocking void (xQueueSend(..., portMAX_DELAY)) and adds a new, separate bool tryAddJob() that is non-blocking (xQueueSend(..., 0), delete-on-full) and used only by OutputWAV's real-time write/finalize path. This makes 78a9f3d's universal bool-checking fix obsolete and non-compiling against the restored void signature.

Reconciled in 35b4f80 (new commit, does not amend 78a9f3d/bb4c945/5aa22cc):

• Cherry-picked ac6ba80 cleanly onto the branch (no textual conflicts).
• Reverted SourceAAC::addReadJob/seekSourceFrame, SourceMP3::addReadJob, SourceWAV::addReadJob, and OutputAAC::addWriteJob back to plain blocking Sched.addJob(...) calls with no return-value check, matching the restored contract. OutputWAV's two call sites already correctly use the renamed Sched.tryAddJob(...) with the bool checked — untouched.
• Independently reviewed queue-ownership/hang risk under the restored blocking contract: 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-waiting via Sched.loop(0) while awaiting isReadReady()). The audio task's blocking addJob() calls are drained by a different, independent thread/task, so a full queue only delays that call until the drain thread services it — no self-deadlock found. OutputWAV's write/finalize path runs on that same real-time audio task and must not block it, which is exactly why tryAddJob()'s non-blocking, checked, retry-on-false contract remains scoped there.
• Rewrote tests/JobQueueContractSelfCheck.cpp for the new dual-API contract: structural checks assert the decode/encode sources call only blocking Sched.addJob() and never Sched.tryAddJob(), OutputWAV calls only Sched.tryAddJob() (checked) and never the blocking Sched.addJob(), and SDScheduler.h declares both signatures; a functional harness mirroring OutputWAV::addWriteJob proves queue-full rejection still leaves the buffer retryable with no leak/false-pending state, with a "buggy" counterpart proving the check discriminates a regression of the original defect class.

Note on the addJob() signature itself: relative to the transient, unmerged 78a9f3d head, SDScheduler::addJob()'s return type changed (bool → void). That is expected and correct — 78a9f3d's bool addJob() was never released or merged anywhere; 35b4f80's void addJob() + bool tryAddJob() restores exact compatibility with upstream master/#20's real, merged-target design. No compatibility wrapper was added since no consumer of the transient 78a9f3d signature was ever found (verified against the guarded firmware and a fresh clone of upstream CircuitMess/JayD-Firmware: neither calls SDScheduler directly).

No other conflicts, refactors, or new abstractions were needed — openChannel hot-swap, MixSystem's recording state machine, and SDScheduler's job queue otherwise merged cleanly across the three branches' non-overlapping edits to MixSystem.cpp/.h.

Guarded-firmware compatibility — SourceAAC::Status / MixSystem::getChannelStatus / updateGain

A separate, wireless-bringup-guarded fork of JayD-Firmware (-DJAYD_WIRELESS_BRINGUP, an HTTP status/control server) has a MixScreen that calls MixSystem::getChannelStatus()/updateGain() and switches on SourceAAC::Status, none of which existed on this branch. getVolume()/getMix() (also referenced there) already existed from #19's integrated hot-swap work.

Added in ad85f45 (new commit):

• SourceAAC::Status (CLOSED/DATA/STARVED/END_OF_STREAM/FAILED) + getStatus() — purely observational, set at existing open/construct-fail/decoder-init-fail/generate() decision points. No ADTS parsing, frame-index, seek, EOF one-shot reset, or decode-discard logic was touched or replaced.
• MixSystem::getChannelStatus(channel), following the same sourceMutex/cleanupRetiredSources() locking hasChannel() already uses.
• MixSystem::updateGain() — extracted from the constructor's existing inline i2s->setGain(0.4f*volumeLevel/255.0f) expression (now called from both the constructor and externally).

Verified by compiling the actual guarded firmware (-DJAYD_WIRELESS_BRINGUP) against this branch: MixScreen.cpp builds clean.

Three-band isolator EQ

Cherry-picked 285b0160 ("Add per-deck three-band isolator EQ") from djdefi-three-band-isolator-eq — a clean pick (21d90a7, no conflicts; git auto-merged its MixSystem.cpp/.h hunks against the status/gain addition above). Adds ThreeBandEQ (low/mid/high band levels) wired per-channel via MixSystem::setEQ(), plus tests/ThreeBandEQSelfCheck.cpp.

Validation

• All 5 host self-checks pass (-std=c++11 -Wall -Wextra -Werror -fsanitize=address,undefined): adts_timing_self_check, SpeedModifierSelfCheck (incl. reset-after-seek case), wav_header_selfcheck (asserting the dual addJob/tryAddJob contract), JobQueueContractSelfCheck (dual-API contract), ThreeBandEQSelfCheck.
• Standalone arduino-cli compile --fqbn cm:esp32:jayd (library's own example sketch): 1,026,978 B flash / 44,464 B RAM (+144 B flash / +8 B RAM vs 35b4f80's 1,026,834 B / 44,456 B — the status/gain + EQ additions; +996 B / -8 B vs Fix Mixer clip() overflow bug; add per-channel hot-swap loading #19 baseline 4ad5108's 1,025,982 B / 44,464 B).
• Guarded JayD-Firmware build (-DJAYD_WIRELESS_BRINGUP, full screens/wireless server) against this branch: 1,840,826 B flash / 54,976 B RAM, compiles clean.
• Dynamic PSRAM (computed from source, unchanged by these updates): each SourceAAC deck uses 104 KiB core buffers (64 KiB read + 8 KiB decode + 32 KiB PCM out, matching the 4×8192B raw-block/32 KiB-output bound) plus a bounded frame index capped at 128 KiB (PSRAM-first with internal-RAM fallback). Two decks + the 32 KiB OutputWAV recording ring buffer ≈ 496 KiB steady-state; transiently up to ~732 KiB while a channel hot-swap's retired source hasn't been reaped yet.

Hot-swap pause-state regression (concrete device defect)

Physical hardware validation of 21d90a7 found: Deck B paused at elapsed=2s, remoteLoad replaced its source, the request was accepted, but status then read elapsed=2, paused=false — the pause was silently dropped by a successful hot-swap.

Root cause in MixSystem::_openChannel():

if(replaceSource(channel, newSource) || !wasPaused) mixer->resumeChannel(channel);

replaceSource() returns true on any successful swap, so the || resumed the channel on every successful replacement regardless of wasPaused — the prior pause state was only ever consulted on failure.

Fixed in 1d5feb8 (new commit, no amend):

replaceSource(channel, newSource);
if(!wasPaused) mixer->resumeChannel(channel);

Replacement now always runs; resume is decided solely by the pre-swap pause state, independent of success/failure — a paused deck stays paused across a successful swap, a playing deck keeps playing, and a failed replacement leaves the deck exactly as it was (matches replaceSource()'s existing no-op-on-failure semantics).

Added tests/OpenChannelPauseSelfCheck.cpp: a structural scan of the shipped _openChannel() proving the fixed pattern is present and the buggy || pattern is gone, plus a functional harness mirroring the exact resume decision across paused/playing × success/failure, including a negative case proving the pre-fix pattern would have been caught.

Also fixed the coupled firmware-side UI bug this defect exposed, in the guarded firmware's MixScreen::loadChannel(): it unconditionally called bar->setPlaying(true) after queueing any replacement. Now captures wasPaused = system->isChannelPaused(channel) before queueing and sets bar->setPlaying(!wasPaused), matching the Library's restored semantics. (This lives in the guarded firmware checkout, not this repo — reported to the coordinator alongside this PR update.)

Validation (re-run after the pause/resume fix)

• All 6 host self-checks pass (-std=c++11 -Wall -Wextra -Werror -fsanitize=address,undefined): the 5 above plus OpenChannelPauseSelfCheck.
• Standalone arduino-cli compile --fqbn cm:esp32:jayd: 1,026,978 B flash / 44,464 B RAM (unchanged — the fix only changes a boolean condition, no size delta).
• Guarded JayD-Firmware build (-DJAYD_WIRELESS_BRINGUP): 1,840,854 B flash / 54,976 B RAM (+28 B flash from the MixScreen.cpp pause-capture fix, RAM unchanged).

Not done

No physical hardware was available in this environment, so I did not upload/boot or smoke-test two-deck playback/rate/seek/recording on-device. A hardware-validation coordinator session is running the physical matrix separately against this branch; this PR remains explicitly validation-only pending that outcome.

Merge order

This branch is a combination for firmware consumption, not a replacement for #19/#20/#21. Once those merge upstream in dependency order (#19 → #21 → #20/deck-rate), this branch's diff against master should shrink to nothing and it can be closed.

Current final SHA: 1d5feb8df9a5d2faa4bfeb263bc0ecaaa93cb367 — see branch djdefi-integration-aac-deckrate-recording (1d5feb8 pause/resume fix on top of 21d90a7 three-band EQ + ad85f45 status/gain APIs + 35b4f80).

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

[djdefi]

Fixed the critical `SDScheduler::addJob()` bool-contract gap flagged in review, in a new commit `78a9f3d` (does not amend `bb4c945`/`5aa22cc`).

Bug: recording-core changed `addJob()` to return `bool` (queue-full -> delete job, return `false`), but `SourceAAC` ignored the return value at its read-job (`addReadJob`) and seek (`seekSourceFrame`) call sites. On a full queue (capacity 8, contended by two decks + recording), `readJobPending` was still set true with `readResult` never arriving -> `processReadJob(true)`/`open()` spin forever. Seek silently "succeeded" while the file position never moved.

Fix: audited every `Sched.addJob()` caller and applied the same pattern everywhere:

• `SourceAAC::addReadJob`: only set `readJobPending` on a successful enqueue; free the buffer instead of leaking it on failure.
• `SourceAAC::seekSourceFrame`: enqueue the seek first; return `false` with elapsed/decoded state untouched if it can't be queued.
• `SourceMP3::addReadJob` / `SourceWAV::addReadJob`: same read-job fix.
• `OutputAAC::addWriteJob`: leave the buffer in `freeBuffers` on a failed enqueue instead of stranding it pending forever.
• `OutputWAV`'s two call sites already checked the return value correctly - no changes needed there.

New test: `tests/JobQueueContractSelfCheck.cpp` - a structural scan proving every real `Sched.addJob()` call site checks the return value, plus a deterministic harness (mirroring the fixed state machine against an injectable fake scheduler) covering queue-full rejection + recovery for read and seek jobs, and bounded close/teardown. Verified the check fails against the pre-fix source/pattern before passing on the fix.

Validation: all 4 host self-checks (adts_timing, SpeedModifier, wav_header, JobQueueContract) pass with `-std=c++11 -Wall -Wextra -Werror -fsanitize=address,undefined`. Rebuilt the Arduino consumer firmware (`cm:esp32:jayd`): 1,026,806 bytes flash / 44,456 bytes static RAM - unchanged from `bb4c945` (pure logic fix, no size impact).

No hardware available to smoke-test on-device.

[djdefi]

Reconciled with #20's advanced head ac6ba80 ("Scope nonblocking SD enqueue to recording"), which restores legacy blocking SDScheduler::addJob() and scopes the new nonblocking tryAddJob() to OutputWAV only. Superseded the prior 78a9f3d universal-bool-check fix (now obsolete/non-compiling) by reverting SourceAAC/SourceMP3/SourceWAV/OutputAAC to plain blocking addJob() calls and rewriting JobQueueContractSelfCheck for the new dual-API contract. New final SHA: 35b4f80a47f5e5eef029e0c57db76699a076df4a. All 4 host self-checks pass, full firmware build green (1,026,834 B flash / 44,456 B RAM). See updated PR body (Conflict #3) for the full reconciliation writeup and queue-ownership review.