audio: lift the Saffire UpdateIsochBufferParams latency model into TimingCursorPolicy#27
Open
boggspa wants to merge 269 commits into
Open
audio: lift the Saffire UpdateIsochBufferParams latency model into TimingCursorPolicy#27boggspa wants to merge 269 commits into
boggspa wants to merge 269 commits into
Conversation
SonarQube CRITICAL cpp:S5025 fired on IsochReceiveContext.cpp because
`new (std::nothrow)` was assigned to a raw pointer before being wrapped in
OSSharedPtr. Even with an immediate wrap, SonarQube flags the raw-pointer
assignment itself.
Introduce `ASFWDriver/Common/DriverKitUtils.hpp` with a
`MakeOSObject<T>(Args&&...)` factory template that centralises the
unavoidable DriverKit allocation pattern:
- Allocates with `new (std::nothrow)` (single NOSONAR suppression here)
- Null-checks the result
- Immediately transfers ownership into `OSSharedPtr<T>(raw, OSNoRetain)`
- Returns an empty OSSharedPtr on allocation failure
- `static_assert(std::is_base_of_v<OSObject, T>)` enforces correct usage
at compile time
- `[[nodiscard]]` prevents silent discard; `noexcept` since placement-new
never throws and DriverKit disables exceptions
Update `IsochReceiveContext::Create()` to use the new helper:
Before:
IsochReceiveContext* rawCtx = new (std::nothrow) IsochReceiveContext();
if (!rawCtx) return nullptr;
rawCtx->hardware_ = hw;
rawCtx->dmaMemory_ = std::move(dmaMemory);
if (!rawCtx->init()) {
rawCtx->release(); // manual, easy to miss on new paths
return nullptr;
}
return OSSharedPtr<IsochReceiveContext>(rawCtx, OSNoRetain);
After:
auto ctx = ASFW::Common::MakeOSObject<IsochReceiveContext>();
if (!ctx) return nullptr;
ctx->hardware_ = hw;
ctx->dmaMemory_ = std::move(dmaMemory);
if (!ctx->init()) return nullptr; // OSSharedPtr destructor calls release()
return ctx;
The manual `rawCtx->release()` on the init-failure path is eliminated —
the OSSharedPtr destructor handles it automatically. No change to the
function signature, return type, or caller behaviour.
MakeOSObject is variadic and reusable for any other OSObject factory in
the driver (e.g. ASFWProtocolBooleanControl::Create()). All 306 C++ unit
tests pass.
strncpy does not guarantee NUL-termination when the source is exactly as
long as the destination buffer, requiring an explicit
`buf[sizeof(buf)-1] = '\0'` guard after every call — a pattern that is
easy to omit on new code paths and was already inconsistent across the
codebase (ControllerMetrics::Reset() never set the sentinel).
Replace every occurrence with strlcpy, which always NUL-terminates and
takes the full buffer size rather than size-1, making the intent clear
and eliminating the manual sentinel writes.
Files updated:
- ASFWDriver/Debug/BusResetPacketCapture.cpp
- ASFWDriver/Diagnostics/ControllerMetrics.cpp
- ASFWDriver/Diagnostics/StatusPublisher.cpp
- ASFWDriver/UserClient/Handlers/BusResetHandler.cpp
- ASFWDriver/UserClient/Handlers/StatusHandler.cpp
- ASFWDriver/UserClient/Handlers/TopologyHandler.cpp
DriverVersionInfo.hpp additionally replaces the manual default constructor
(which called memset) with C++ default member initialisers on each field.
The memset-based constructor was redundant: aggregate initialisation with
`{}` already zero-initialises all members, and the explicit constructor
was only needed to paper over the missing in-class initialisers. The
change also migrates the six strlcpy calls in the static factory method.
…ead rule-of-5 The ROMScanner constructor was refactored in 314a182 to accept a ROMScannerParams struct, but the call-site in ASFWDriver::Start() was not updated, leaving the driver calling the old three-argument overload. Pass a default-constructed ROMScannerParams{} to match the new signature. The test helper struct PendingRead stores an InterfaceCompletionCallback (a std::function), which is non-trivially copyable. Storing PendingRead values in a std::vector<PendingRead> and calling operations like push_back or emplace_back can trigger implicit copy/move construction. Without explicit rule-of-5 declarations the compiler may suppress the move constructor when a non-trivial copy constructor is present, causing unnecessary copies or outright compilation errors depending on the C++ version and optimisation level. Explicitly default all five special members on PendingRead to restore the expected copy+move semantics.
… capture (S1048) Two bugs in ObserverGuard<T>: 1. Destructor could terminate on throwing Unregister (S1048) The destructor called unregister_() — a std::function storing a virtual Unregister* call — with no exception guard. The destructor is already marked noexcept, so any exception thrown by the callee would invoke std::terminate rather than propagate. Added a try/catch(...) to swallow any exception explicitly, making the intent clear and satisfying SonarQube cpp:S1048. 2. IUnitObserver branch captured registry by reference The lambda stored in unregister_ captured `registry` by reference (i.e. a reference-to-reference bound into a std::function that outlives the constructor). If the registry was destroyed before the ObserverGuard, the destructor would call through a dangling reference — UB with no diagnostic. The IDeviceObserver branch already handled this correctly by taking ®istry as a typed pointer and capturing that pointer by value. Apply the same pattern to IUnitObserver: cast to IUnitRegistry* and capture the pointer, matching the existing convention. static_cast is correct here (dynamic_cast is only needed in the IDeviceObserver branch where RegistryType may not implement IDeviceManager).
The branchless CRC32 inner loop used `-(crc & 1u)` to produce an all-zeros or all-ones uint32_t mask. While well-defined (unsigned wraparound), SonarQube cpp:S876 flags unary minus on unsigned types as error-prone — the result is a large positive value, not a negative one, which is surprising to readers. Replace with an explicit ternary that makes the all-zeros / all-ones intent clear without relying on unsigned wrap semantics.
…lication Group A (19 issues): Add NOSONAR(cpp:S3923) to if/else blocks where both branches contain only ASFW_LOG* calls with different format strings. os_log requires compile-time literal format strings, so branches cannot be merged with a ternary — the structural identity is a false positive. Files: AsyncSubsystem.cpp, Tracking.hpp, BusResetCoordinator.cpp, SelfIDCapture.cpp, DeviceRegistry.cpp, IsochAudioTxPipeline.cpp, LogConfig.cpp, AVCUnit.cpp, DICETransaction.cpp, SPro24DspProtocol.cpp, IsochHandler.cpp Group B (1 real fix): MusicSubunit.cpp — the first and third branches of the plug-direction if/else both assigned kInput. Extract it as the default before the conditional; keep the inner if/else only to override kOutput and log the out-of-range warning. No behaviour change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace raw IOBufferMemoryDescriptor* pointers with OSSharedPtr<> in StartIsochReceive, StartIsochTransmit (ASFWDriver.cpp) and StartStreaming (AVCAudioBackend.cpp, DiceAudioBackend.cpp). Use .attach() to receive the pointer from CopyRxQueueMemory / CopyTransmitQueueMemory and .detach() to transfer ownership into StartReceive / StartTransmit. This eliminates all manual ->release() calls on error paths and ensures the buffer is released automatically if an early-return path is added in future. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s, S3608 explicit lambda captures
… channel caps
TxBufferProfiles.hpp:
- Replace ASFW_TX_PROFILE_A/B/C #defines with enum class TxProfileId : uint8_t
- ASFW_TX_TUNING_PROFILE default changed to plain integer (0=A, 1=B, 2=C)
- static_assert bounds-checks the macro value at compile time
- Replace #if/#elif/#error profile selection with constexpr SelectTxProfile()
- Add GetActiveTxProfile() / SetActiveTxProfile() for future runtime toggling
AudioRingBuffer.hpp:
- Remove #ifndef ASFW_MAX_SUPPORTED_CHANNELS guard and NOSONAR
- Add static constexpr kMaxSupportedChannels = 16 inside the class template
StreamProcessor.hpp:
- Remove #ifndef ASFW_MAX_SUPPORTED_CHANNELS guard and NOSONAR
- kMaxSupportedPcmChannels inlined to 16 directly
tools/calc_buffer_sizes.py:
- Update profile selection to use plain integer map {"0":"A","1":"B","2":"C"}
- Remove endswith("_A/B/C") branches and ASFW_TX_PROFILE_A/B/C reverse lookup
All 306 unit tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Centralize audio caps/sizing in AudioConstants (PCM cap 32, DBS cap 32) - Rename Tx/Rx profile headers to AudioTx/AudioRx; remove RX macro selection - Wire Config constants through Encoding/TX/RX/Shared queue and update tooling/tests
- Add .clang-format and .clang-tidy, plus tools/quality/{format,tidy,analyze,all}.sh (tidy exits non-zero on warnings; host + DriverKit passes).
- Rewrite ConfigROM into Common/Parse/Store/Remote/Local with facade headers; remove legacy poll/event-bus/FSM plumbing.
- Introduce Local/MemoryMapView.hpp to centralize IOMemoryMap::GetAddress() integer->ptr conversions.
- Implement callback-driven ROMScanner::Start/Abort backed by ROMScanSession serial executor, bounded inflight, speed fallback, EnsurePrefix, and optional IRM read+CAS verification (marks bad IRM via TopologyManager).
- Update ControllerCore + UserClient ConfigROMHandler to the new scanner API; delete obsolete discovery polling/manual trigger paths.
- Update tests (parser/scanner/store) + CMake/build wiring and docs; isolate register enum casts in Register32FromOffsetUnchecked() for analyzer.
- Split ROMScanSession into core/details/IRM translation units - Convert ConfigROMParser APIs to std::expected and surface CRCStatus - Add ConfigROM offset/count unit types for bytes/quadlets - Run IRM verify even for minimal ROM; allow Complete from IRM states - Refactor details discovery to use ParseDirectory + async state helpers - Add host tests: CRC mismatch warning, IRM verify minimal ROM, abort ignores late callbacks, EnsurePrefix cap
Baseline changes landed in 897f938.
- Fix Doxygen/README inaccuracies (keyId vs key byte, CRC/endianness wording) - Clarify ConfigROMBuilder ImageBE/ImageNative and ConfigROMStager staging/restore sequence - Reduce clang-tidy noise: designated init, explicit math parentheses, move descriptor-dir fetch state out of function
- Replace ROMScanSession details discovery callback maze with a per-node DetailsDiscovery state machine. - Introduce TextDescriptorFetcher to unify leaf vs descriptor-directory textual descriptor fetch (best-effort, never fails node). - Add runtime-gated verbose traces (ASFW_LOG_V3(ConfigROM, ...)) at step boundaries and failure/skip points. - Add host tests for vendor/model leaf extraction, descriptor-dir fallback, and unit-dir modelName parsing.
- Document IFireWireBusOps contract + buffer lifetimes (no reentrancy; generation guard; cancel semantics).
- FireWireBusImpl: reject stale FW::Generation at boundary, post kStaleGeneration completion async, return AsyncHandle{0}.
- AsyncSubsystem: implement Cancel() for transaction handles and ReadWithRetry queued handles; ensure cancel completions are posted via PostToWorkloop (never inline).
- Tracking: map kIOReturnAborted to AsyncStatus::kAborted.
- Tests: add AsyncBusContractTests + host-only PostToWorkloop deferral hooks.
Quality: tools/quality/tidy.sh clean; ./build.sh --test-only --no-bump pass; tools/quality/analyze.sh run (existing analyzer issues remain outside Async/ConfigROM).
Milestone 2: invert dependencies so protocol layers consume IFireWireBus ports instead of AsyncSubsystem. - Added ASFWDriver/Protocols/Ports/FireWireBusPort.hpp aliases for IFireWireBusOps/IFireWireBusInfo. - Refactored protocol implementations (DICE, Oxford/Apogee, AV/C FCP/PCR) to take bus ops/info ports and FW::NodeId, dropping AsyncSubsystem includes. - Updated discovery wiring: DeviceRegistry/ControllerCore now pass bus ports into DeviceProtocolFactory; AV/C discovery + FCPResponseRouter are constructed from ControllerCore::Bus() and wired to PacketRouter in ASFWDriver::Start. - Added ControllerCore setters for AV/C dependencies and a const Bus() accessor. - Host/test build fixes: Logging.hpp now includes LogConfig.hpp; HostDriverKitStubs pulls DriverKit/IOLib.h stubs; AVCCommand.hpp includes <algorithm> for std::fill. Quality: host tests pass; clang-tidy clean; xcodebuild analyze succeeds.
- Change Discovery::Generation from uint16_t to ASFW::FW::Generation.\n- Update key packing/logging/call sites to use .value or Generation{...} at boundaries.\n- Fix UserClient discovery/ConfigROM handlers to marshal Generation to wire types.\n- Add static_asserts for Generation trivial copy + 32-bit size.\n- Update host tests for the strong type.
Mechanical refactor only; no behavior changes intended. AsyncSubsystem: split into Lifecycle/Interrupts/BusReset/CommandQueue/Diagnostics translation units. ControllerCore: split into Lifecycle/Interrupts/Discovery/Facades translation units (ControllerCore.cpp kept as a thin stub TU). BusResetCoordinator: split into FSM/Actions/DiscoveryDelay translation units. Xcode: project uses file-system synchronized groups, so new .cpp files are picked up automatically. Quality: tools/quality/tidy.sh, ./build.sh --test-only --no-bump, tools/quality/analyze.sh.
Introduce a protocol-facing BlockWriteRequestView + disposition so AV/C routing no longer includes Async/Rx PacketRouter, PacketHelpers, or ResponseCode headers. PacketRouter/ARPacketView adaptation now lives in ASFWDriver.cpp (composition root), keeping protocol code independent of Async Rx internals.
Checkpoint current async boundary cleanup, UserClient runtime-state work, FireWire IOReturn layer updates, and bus-reset recovery hardening. Includes typed Self-ID/topology validation, selfIDComplete2 handling, 2s software-reset holdoff, updated docs, and new host tests. Validation: build.sh --test-only --no-bump passes, tidy clean, analyze succeeds with existing unrelated analyzer findings outside this milestone.
Make bus-reset gap handling transactional with confirmed and in-flight state. Keep corrective reset dispatch fail-closed so invalid topology or failed PHY/reset operations never publish stale bus state. Tighten the coordinator reset-cycle state and merge logic, expand host tests for gap policy and failure paths, and stop tracking CLEAN_CODE.md while leaving it local-only.
Bus: finalize reset recovery and gap optimization
Fix analysis findings and prep local Sonar runs
Replace the old txScheduledSampleFrame_ TX timeline with a phase-driven model and fix the bugs that made it ship continuous silence after ~1s. Model: - TxOutputPhaseLoop is a free-running, unwrapped int64 phase accumulator plus a device-drift monitor. The AMDTP assembler owns the wire DATA/NO-DATA cadence; the loop advances its phase on the assembler's isData and never decides cadence itself. Glitch/lead are evaluated in the 1-second offset domain (extOffsetDiff1s) so the per-second transmit-cycle wrap is not mistaken for a discontinuity. - OutputPhaseToAudioMap converts the unwrapped phase to an absolute audio frame by plain subtraction (no modular wrap, no 4s ceiling), anchored once at reportedPlayhead - PacketLeadFrames via TimingCursorPolicy, re-anchored on a device discontinuity. - SYT generation is delegated to the existing Encoding::SYTGenerator (transmit-cycle anchored, advances by the fixed packet step, RX-cadence nudged via nudgeOffsetTicks) instead of being re-derived in the phase loop. Bugs fixed vs the in-progress phase-loop refactor: - deadband rebase fired every cycle, collapsing the 3:1 cadence to all-DATA and overrunning 48k; - the device phase was a 1-second sawtooth that tripped the glitch detector every second and made the derived audio frame non-monotonic; - the map was anchored at writtenEnd with no safety margin (packets landed in the future, uncovered) and wrapped after +/-4s; - the phase loop and the assembler were two independent cadence sources; - txScheduledSampleFrame was published unconditionally (0 on NO-DATA); - PhaseToFrame could underflow on a negative delta. Tests: - TxOutputPhaseLoop / OutputPhaseToAudioMap unit tests rewritten for the new semantics, incl. per-second-wrap-is-not-a-glitch, unwrapped large-span and negative-delta clamp; RealisticSteadyState48k now holds the 3:1 cadence. - New loop+map integration test drives a 1-second-wrapping device phase and an advancing writtenEnd for 3 simulated seconds and asserts coverage >=99% with no future-misses and no glitches (mirrors the tx_sim.py contract). - IsochAudioTxPipelineTimelineTests migrated to the new model: a recovered-clock bridge is established so the frame map can anchor, with two contract updates (a pre-write packet stays startup silence; an uncovered future range now ships silence instead of deferring). Add the TX timeline simulators (tools/debug/asfw_saffire_tx_sim.py and _sim2.py) that capture the contract this implementation follows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…fn, decouple populate
Items from validated review:
1. Add PreparedTxSlotState::SilenceFallback
- Distinct from InitialSilence (startup/pre-anchor) and PcmPrepared (real PCM).
- PreparePayload now branches on 'covered': PcmPrepared only when the packet ring
slot was actually populated; SilenceFallback for coverage misses.
- Increments control->counters.txSilenceFallback on the prepare path (previously
the counter was declared but never wired on this path).
- PreparePayloads skip condition includes SilenceFallback so covered-miss slots
are not re-prepared on subsequent passes.
2. Gate consumed-frame publication in OnTransmitSlotCompleted
- lastCompletedAudioFrame_ / txCompletedSampleFrame / PublishDirectTxConsumedEndFrame
now only advance for PcmPrepared slots with audioFrame != 0.
- Pre-anchor DATA (audioFrame==0) goes to txCompletedStartupSilenceSlots.
- SilenceFallback goes to txSilenceFallback + txSilenceSubstitutions (not to the
PCM completion path), preventing bogus consumed-frame values during warm-up.
- txNoPhaseSilencePackets / txValidPhaseSilencePackets accounting moved exclusively
to the SilenceFallback branch where it belongs.
3. Rename TryGetRecoveredDevicePhaseTicks -> TryBuildTransmitCycleDeviceSubphase
- Old name implied true device-phase recovery. The function actually grafts the
recovered device sub-cycle onto the local TX bus cycle base; long-term whole-cycle
drift is handled by SYTGenerator::nudgeOffsetTicks(), not here.
- Comment block on the definition documents what is and isn't computed.
- Updated stale test comment to match.
4. Decouple packet ring population from DoPrepareOnce
- PopulateClipStyleTxRingFromWrittenRange moved from DoPrepareOnce (per slot-prepare
iteration) to top of DrainPreparationRequests (once per drain pass).
- DoPrepareOnce is now a pure PreparePayloads caller.
- Honest transitional comment: still on isoch queue, not true audio-side-safe yet.
Add RebaseReason (InitialSeed/Glitch/LeadReject) to TxOutputPhaseLoop::CycleResult so a recurring re-seed can be told apart from the expected one-time start-of-stream seed. Diagnostics gains matching per-reason counters (rebases == sum of the three). Wire the previously-dead AudioRtCounters::txPhaseRebases counter and add a rate-limited "TX PHASE RESET reason=..." log at the rebase site, so a hardware run can show definitively whether LeadReject (phase loop not converging) is firing repeatedly vs. a one-off InitialSeed/Glitch. Also rate-limit the "txPhaseMap anchored" and "Armed transmit-cycle anchor" logs, which can otherwise repeat every cycle when unstable. Pure additive instrumentation -- no timing/behavior change. Extends SaffirePhaseLoopTests with reason/counter assertions and a new LeadRejectRecovery test driving that path without a device glitch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…adence-authoritative
Replaces the single combined TxPhaseEvent with independent ContinuityEvent and
EmissionEvent classifications, fixing two issues with the prior model:
- InitialSeed/OneCycleSlipCompensated no longer get coerced into NO-DATA outcomes
(Saffire ships DATA in the same callback it seeds/absorbs a slip in).
- Emission is now a pure lead-HEALTH read ("was the lead in range when we shipped
or withheld this cycle?"), decoupled from the wire cadence decision. The wire
DATA/NO-DATA cadence stays where it belongs -- PacketAssembler's spec-mandated
CIP/DBC sequencing -- and the loop tracks it via dataCandidate (emitData mirrors
it exactly, phase advances in lockstep with it) rather than re-deriving it.
Only kInitialSeed/kTimingDiscontinuity may reset the phase map / re-arm the SYT
anchor now; recurring lead-health reads never do, eliminating the "TX PHASE RESET
reason=Glitch" spam that fired on ordinary cadence.
Adds a Python reference model (tools/debug/tx_phase_loop_model.py) with JSON
fixtures pinning the per-cycle event sequence, and rewrites the C++ test suites
(SaffirePhaseLoopTests, OutputPhaseToAudioMapTests, IsochAudioTxPipeline call site)
to match.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…atch The continuity predictor's cycleDelta was a raw uint32_t subtraction of two wrapping 13-bit OHCI bus-cycle fields. At the 7999->0 boundary that produced ~2^32 - 7999 (~4.29B ticks), which the detector then classified as a giant device-phase jump and re-fired kTimingDiscontinuity exactly once per second -- a recurring "TX PHASE RESET reason=Glitch" anchor reset, in production, on ordinary cadence. Replaced the subtraction with the mod-kCyclesPerSecond form, added the kCyclesPerSecond constant + wrap-aware docstring to the class, and added a TxOutputPhaseLoop.TransmitCycleWrapIsNotDiscontinuity regression test that drives the loop through the 7999->0 boundary with exactly +1 cycle of device phase per call and asserts the wrap reads as kNominal (verified the test catches the bug by temporarily restoring the old form). Also adds a defensive log in IsochAudioTxPipeline::NextTransmitPacket for dataCandidate / pkt.isData drift: the phase loop tracks PacketAssembler's cadence decision via dataCandidate, so a silent disagreement means the loop's phase tracking has diverged from the wire. Treated as a hard inconsistency signal, not a recoverable warning. No change to the Python reference model -- it already takes cycleDelta directly, so the bug was purely in the C++ derivation. JSON fixtures remain valid contract pins. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… decompile Tighten the safety limit kSafetyLimitTicks from 12287 to 7620 in ExternalSyncDiscipline48k.hpp to align with Saffire's hardware rejection threshold. Saffire decompilation reveals that the hardware drops packets (emits SYT=0xFFFF) when the phase lead is >= 7620 ticks. The 12287 threshold is purely an advisory, log-escalation warning. Clarified comments in TxOutputPhaseLoop.hpp and SYTGenerator.hpp to document these boundaries accurately.
Introduce the closed, hardware-free AudioDriverKit lab simulation environment to test and validate the driver's AMDTP/DICE transmit protocol stack without real FireWire hardware. This includes: - Protocols/ (portable AMDTP/DICE/IEC61883 implementations) - Core/ (audio IO loop and device controller orchestration) - Ports/ (IAmdtpTxSlotProvider interface) - Lab/ (FakeIsochTxSlotProvider mock) - Driver/ (AudioDriverKit dext glue) - Tests/ (test harness and unit tests passing 8,341 checks) - project.yml (relocated and optimized project configuration) - Updated .gitignore to exclude generated .xcodeproj folders.
Ensure the ignore rule targets only the generated ADKVirtualAudioLab/ADKVirtualAudioLab.xcodeproj/ so that we don't accidentally ignore new file additions inside the main ASFW.xcodeproj, which is tracked.
Implements the Milestone 1 payoff step per the lab README: - Ports/IDiagSink: the RT-safe sticky-counter seam (increment-only, no logging, no allocation); Lab/StickyCounterSink as its fixed-array relaxed-atomic implementation for tests and the future dext dump. - Lab/VerifyingSlotProvider: decorator over any IAmdtpTxSlotProvider asserting P1-P4 on every PublishSlot — cadence window (6000/8000) and N,D,D,D run shape, DBC continuity (no-data carries unchanged), CIP bit-exactness re-parsed from the published wire image (Q0/Q1 fields, 8-byte no-data, byteCount vs frames*dbs), and gapless frame tiling. Observer not gate: packets always forward; violations resync so one fault counts once. Structural constants are asserted, stream constants (SID, frames-per-data) are learned from the wire — the verifier shares no code with the packetizer it judges. - Step 6 seams (additive only, no behavior change): controller BindLabSlotProvider() to interpose Verifying(Fake) between engine and ring, and PayloadCounters()/PayloadWriterCounters() accessors to read the writer's miss buckets from scenarios. - Tests/VerifyingSlotProviderTests: instrument self-tests — a hand-rolled golden driver (independent of AmdtpTxPacketizer) plus one corruption case per violation kind, each proving the targeted counter fires exactly once with no cross-talk. - Tests/VerifierScenarioTests: the scenario pump (controller -> engine -> Verifying(Fake)) — regular 512-frame soak past 1e6 cycles with zero violations, irregular frame counts, skipped callback (silence in structurally valid packets, never corruption), sample-time jumps landing in framesWithoutPacket/framesOutsidePacket, and stream restart. Suite: 17440 checks, 0 failures (was 8341). Dext target still compiles. Milestone 1 exit criteria are now executable checks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ovider lab: complete Step 6 — VerifyingSlotProvider, IDiagSink, scenario pump
…real pacing Implements the Milestone 3 dext clock model per the lab README: - IOTimerDispatchSource on the work queue fires once per ZTS period (512 frames @ 48k) in kIOTimerClockMachAbsoluteTime; each fire anchors UpdateCurrentZeroTimestamp(n*period, fire_time) with raw values — the deadline chain is nominal (derived from the period index via mach_timebase_info, drift-free), host times are actual fire times, the host smooths via the clock algorithm. No driver-side extrapolation. - The same fire exposes the next period's packets through the controller (the lab analog of an IT-ring refill interrupt): hardware requests data on its interrupt; WriteEnd only fills PCM into already-exposed packets. - Step 6 Verifying(Fake) + StickyCounterSink run for the whole IO session; StopIO dumps verifier counters, payload miss buckets, and the O/C instrumentation via IOLog (ADKLab[dump] prefix, never from the IO path). - O/C instrumentation: anchors before first WriteEnd (C1), WriteEnd shape stats — min/max io size, sample-time continuity, first-callback tuple (C3), io/timer fires after StopIO counted not crashed (O2, logged at free), other-op counter, prepare-failure counter. - SetTransportType(FireWire) — the contract ASFW will live under. - Output ring widened to 8 ZTS periods (4096 frames, matches the host scenario pump); a 1-period ring left the HAL zero headroom (C4 stays observable by varying kRingPeriods). - Info.plist: match on IOUserResources + IOResourceMatch IOKit (a hardware-free dext cannot match IOUserService as provider — it would never start); non-empty OSBundleUsageDescription. All APIs verified against the DriverKit 25.2 SDK headers (IOLib.h mach_timebase_info/mach_absolute_time, IOTimerDispatchSource.iig, IOUserAudioClockDevice.iig). Dext + host tool targets build; host suite 17440 checks, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- ADKLabHost: minimal SwiftUI host that embeds the dext (xcodegen embeds driver-extension dependencies into Contents/Library/SystemExtensions) and submits OSSystemExtensionRequest activate/deactivate. The dext identifier is discovered from the embedded bundle at launch so signing lanes that override bundle ids need no code edits. - Entitlements files for both targets (system-extension install; driverkit + family.audio) — referenced by the signing lanes, signing itself stays OFF by default exactly as before. - BENCH.md: the M3 procedure — both signing lanes (ad-hoc SIP-off bench / provisioned SIP-on), activation, driving audio, reading the ADKLab[dump] output, and the counter -> O1-O3/C1-C4 mapping with the M3 exit criteria. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lab: Milestone 3 dext bring-up — ZTS timer chain, activation host, bench runbook
… verification
Implements the M2 scope per the lab README, with every constant taken from
the DICE tree's Saffire-decompile ground truth (SYTGenerator.hpp,
TxOutputPhaseLoop.hpp, tools/debug/tx_phase_loop_model.py):
- Ports/ICycleTimeline: time as an explicit port (unwrapped 24.576 MHz
ticks). Lab/SimulatedCycleTimeline synthesizes it from audio frame
position (6 frames per cycle at 48 kHz; epoch + jitter knobs are test
inputs, per the README's adversarial-schedule design).
- Core/TxTimingModel: the frame->cycle->SYT unit under research. Seeds at
now + presentation delay (4096), grafts the device's constant sub-cycle
(phase - phase%3072 + 0x0B0, the reference-model formula), advances by
the fixed 4096-tick blocking step in the 16-cycle (49152) SYT domain,
and reports transmit-lead health: tight <=3071, accept <7620 (the real
rejection threshold), gate >=7620, escalate >=12287 (advisory only).
WHO DECIDES THE WIRE: not the model — cadence stays authoritative;
callers Peek the SYT, the packetizer decides data/no-data, and only
emitted data packets Commit (the model tracks, it never decides),
mirroring TxOutputPhaseLoop's contract.
- Verifier P5 (config-gated): data SYTs must step by exactly 4096 in the
16-cycle domain and sit on the graft lattice (offset = 0x0B0 mod 1024);
data packets carrying 0xFFFF in a timing-valid run are violations.
Self-tests prove each P5 counter fires exactly once per injected fault.
- Controller: EnableLabTiming/PrepareLabPacketTimed/TimingCounters — the
timed prepare path peeks, prepares, commits-on-data, and records lead
health telemetry (seeds counted at peek: the seeding peek can land on a
no-data cycle).
- Lab/WriteEndTraceReplayer: allocation-free trace-text parser
(sample_time host_time frames; comments; malformed lines counted, never
absorbed) + Replay — the regression harness Milestone 3's captured
trace will drop into. Scenario H replays a trace through the full
timed pipeline.
- Dext wiring: timing + P5 enabled in VirtualAudioDevice; the simulated
bus rides the exposure cursor (the packet's projected transmit
position, per Saffire FillFirewireBuffers); StopIO dump grows p5 and
timing lines.
Unit tests pin the contract exactly: seed SYT 0x10B0 at lead 3248, the
+1,+1,+2 cycle pattern with offsets {0x0B0,0x4B0,0x8B0}, the 12-packet
16-cycle wrap, all five lead-health boundaries, graft-disabled mode, the
monotonic guard, and re-arm/reseed. The timed pump soak holds P1-P5 at
zero violations with the lead pinned at the seed value across ~65k data
packets.
Suite: 18010 checks, 0 failures (was 17440). All three targets build.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lab: Milestone 2 — SYT realism: TxTimingModel, simulated timeline, P5 verification (stacked on mrmidi#24)
…lities to ADKVirtualAudioLab
…, and increase ZTS exposure lead to 3 periods
- Implement seqlock/generation-check snapshotting for PacketTimelineSlot on the RT IO thread (SnapshotSlotForAudioFrame) to prevent reading stale range limits or partially overwritten slot fields. - Use PacketSlotSnapshot copies in AmdtpPayloadWriter to guarantee that the destination pointer calculations do not underflow when a slot is concurrently reused by the ZTS timer thread on the work queue. - Move IOMemoryMap mapping creation to VirtualAudioDevice::init and cache the ringBase pointer, avoiding calls to GetAddress/GetOffset in the real-time IO path. - Keep the outputMemoryMap alive in StopIO until free() to safely handle late RT callbacks. - Add unit tests validating the seqlock snapshot behavior and mid-rewrite guard.
- Add LabDiagUserClient, a diagnostic user client supporting the LDBG connection type to retrieve PacketDumpBlobs via structureOutput. - Implement BuildPacketDumpBlob, capturing consistent packet metadata, timeline slot states, and raw packet bytes on the dext work queue. - Add Swift class PacketDumpClient for querying and parsing the little-endian PacketDumpBlob wire format. - Add PacketInspectorView, a SwiftUI visual inspector for packet structures, CIP headers, and raw hex and floating point PCM payloads. - Integrate PacketDumpBlobTests into the test suite and verify window clamping, evicted slots, and header fields.
- Set kPacketDumpMaxRecords to 6 and default to 4 in PacketDumpBlob.hpp to keep the output structure size under the 4096-byte IOKit inline limit. - Update PacketDumpClient maxRecords constant and dump() default to match. - Adjust PacketInspectorView picker to options [2, 4, 6] and default to 4. - Update PacketDumpBlobTests to use 6 records and adjust index/field assertions to match the new window boundaries. - Resolves the 0xe00002bc (kIOReturnBadArgument) error returned by IOConnectCallMethod.
- Root Cause Analysis:
The CoreAudio HAL wraps stream writes at the zero timestamp period
(512 frames), writing float PCM only to offsets 0..511 of the shared
memory. The driver was configured with a ring buffer size of 4096
frames (8 * 512), causing it to read silence (all zeroes) in offsets
512..4095. This resulted in sporadic audio payload data with a
mathematically precise 12.5% non-zero sample ratio.
- Resolution:
Aligned the driver's ring buffer frame count (ringFrames) directly
with the zero timestamp period (in_zero_timestamp_period = 512),
synchronizing wrapping boundaries between CoreAudio and the driver.
- Additional Telemetry & Diagnostics:
* Expanded AmdtpPayloadWriter with atomic counters for non-zero frames,
non-zero slots, and maximum absolute sample amplitude.
* Bumped PacketDumpBlob to version 2 and expanded the header struct
to expose payload committed cursors and signal level telemetry.
* Updated the Host SwiftUI App (PacketDumpClient, PacketInspectorView)
to parse and display the new telemetry metrics (Non-Zero ratio, Max Amp)
and clarify the committed payload boundary in the inspector view.
* Added a wrap mismatch simulation test in PayloadWriterTests.cpp to
assert the 12.5% non-zero ratio.
* Cleaned up the unused kRingPeriods constant in VirtualAudioDevice.cpp
to ensure warning-free compilation.
… geometry - PcmSlotCodec: RawSigned24In32BE no longer masks to 24 bits — negative samples sign-extend into the top byte (capture shows 0xFFxxxxxx, e.g. 0xFFFC9F0C; zero padding would flip negatives into large positive values for an int32 receiver). LE variant follows via byte swap. Production RawPcm24In32Encoder already did this; the lab now agrees. - FocusriteSaffireProfile: capture-confirmed stream shapes — host→device dbs 9 (8 audio + 1 MIDI, 296 B), device→host dbs 17 (16 AM824 audio + 1 MIDI, 552 B); empty MIDI slots carry 0x80000000 both directions. - VirtualAudioDeviceController: new GetOutputDeviceCaps() so the HAL shape (rate, PCM channels) comes from the selected profile; MIDI slots stay wire-only. - VirtualAudioDevice: publish format/channel layout from profile caps instead of hardcoded 8-channel constants; refuse non-48k profiles the lab clock math cannot honor. - Tests: bench-shape profile expectations, byte-exact fixtures pinned to an explicit stereo config, Saffire end-to-end asserts dbs 9 + MIDI placeholder per frame, codec sign-extension vectors, and the verifier gap-silence scenario now defines "silent" as PCM-slots-zero (the MIDI placeholder rides along even in silence, as on the wire). 18784 checks, 0 failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Aligns kAudioRingBufferFrames and kAudioOutputRingFrames with the 512-frame zero timestamp period (ZTS) to prevent wrapping mismatches and audio payload silence. Adjusts the output consumer lead to 384 frames and the cursor resync deadband to 64 frames to remain within the 512-frame buffer constraints. Updates unit tests to reflect the updated capacity, lead limits, and resync deadbands.
…mingCursorPolicy Redo of the misplaced PR mrmidi#26 — this belongs in the real driver, not the lab (the lab is the reference). - Audio/Runtime/SaffireIsochLatency.hpp: the decompile trace (Saffire::UpdateIsochBufferParams, 0xf506) as a constexpr unit — the latencyMode x rate delay-packet table ({14,2}/{16,6}/{18,10}/{20,14}, +2 at 88.2/96k, +4 at 176.4/192k), DMA-program depth (160/80/40 packets), frames-per-packet (8/16/32), derived delay-frame helpers. Full provenance in the header: host-internal chain (latencyMode is a host preference, not a device register; writer called only from initHardware/RestartStreaming), DICE-quirk note, unsupported rates rejected. Two prose/table discrepancies in the trace flagged for adjudication (the table wins in code): "safest output = 160 frames" matches the INPUT column (output mode 3 = 14x8 = 112), and the "constant ~20 ms DMA buffer" is 20/10/5 ms by packet time. - TimingCursorPolicy: the hand-planted Saffire literals now derive from the table at mode kLow — CursorOffsetFrames (48/128) and the Snapshot's outputDelayPackets/inputDelayPackets (6/16). Zero behavior change, proven by static_asserts pinning the old values. The bench-tuned ADK values (ReportedLatencyFrames 29, SafetyOffsetFrames 8) are deliberately untouched — they sit on top of the cursor offset and their mapping is still a bench question, now documented at the call sites. - tests/audio/SaffireIsochLatencyTests: every table cell pinned, rate bumps on both columns, depth packet-time math, the quoted frame figures, unsupported-rate rejection, and the policy-derives-from-table single-sourcing check. Host suite: 1217/1217 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
3 participants
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.
What
The
UpdateIsochBufferParamslift, redone in the right tree — replaces the misplaced #26 (lab untouched; the lab-side version stays parked onboggspa:lab/saffire-latency-modelif you ever want it as a rehearsal copy).Where it landed
Your
TimingCursorPolicyalready had the trace's numbers hand-planted (48/128cursor frames = mode-kLow × 8,6/16packets inSnapshot()) — so the table became the single source there:Audio/Runtime/SaffireIsochLatency.hpp— the trace as a constexpr unit: delay-packet table ({14,2}/{16,6}/{18,10}/{20,14}, +2 at 88.2/96k, +4 at 176.4/192k), DMA depth160/80/40, frames-per-packet8/16/32, derived delay-frame helpers, full provenance (host-internal chain, latencyMode = host preference, DICE-quirk note), unsupported rates rejected.TimingCursorPolicy—CursorOffsetFramesand the snapshot's delay packets now derive from the table atLatencyMode() = kLow. Zero behavior change, proven bystatic_asserts pinning 48/128/6/16. Your bench-tuned ADK values (ReportedLatencyFrames29,SafetyOffsetFrames8) are deliberately untouched — documented as sitting on top of the cursor offset, mapping still a bench question.tests/audio/SaffireIsochLatencyTests— every table cell pinned + the policy-derives-from-table single-sourcing check.Two prose/table discrepancies in the trace — please adjudicate (code follows the table)
Verification
🤖 Generated with Claude Code