(DICE-4) 44k Sample Rate Implementation#41
Open
alicankaralar wants to merge 25 commits into
Open
Conversation
alicankaralar
force-pushed
the
feat/dice-sample-rates
branch
from
c13bb1c to
80805a8
Compare
alicankaralar
changed the title
alicankaralar
force-pushed
the
feat/dice-sample-rates
branch
from
80805a8 to
1b43427
Compare
alicankaralar
changed the title
alicankaralar
changed the title
…eardown IsochReceiveContext was an OSObject created via plain `new` (MakeOSObject) instead of OSTypeAlloc. In DriverKit, OSMetaClassBase has no constructor setting refcount — only OSObjectAllocate() (the OSTypeAlloc path) does — so the object was born with refcount 0. The single release() in ~IsochService then underflowed and hit the DriverKit over-release assert, SIGABRT-ing the whole dext during ASFWDriver::free() (crash report 2026-07-03 13:21). A crashed dext is not relaunched, so the device never re-published until reboot; this also explains recurring shutdown stalls. Nothing needs OSObject semantics here (no OSAction, no cast, no IIG), so make it a plain final class owned by std::unique_ptr, matching IsochTransmitContext, and delete the MakeOSObject helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ammed the AR path
OHCI §8.4.2: AR bufferFill appends a status/timestamp trailer quadlet to
every received packet. ARPacketParser treated it as optional: a packet whose
header+payload ended flush with the received-byte frontier parsed as
"complete" with totalLength 4 bytes short. The trailer then arrived as an
orphan quadlet at the head of the unread stream, every subsequent parse was
misaligned (responses completed against unknown tLabels, then an unparseable
byte pattern), and the AR response path jammed permanently — all async on
the bus timed out (ackCode=0x1: devices kept ACKing) until reboot.
Field trace 2026-07-03 13:53:28: tLabel=6's 104-byte read-block response
consumed as 120 bytes, its trailer left as the "offset 0/4" tail in
buffer[5]; three real responses destroyed ("No transaction for key"),
parser stuck at offset 56 forever. Chunked DICE section reads made the
flush-boundary case likely enough to hit on the third bring-up.
Fix: no trailer in the window means the packet is incomplete — return
nullopt so the tail is preserved and the stitched cross-buffer retry picks
the packet up whole once the trailer lands.
Adds a parser-level regression test and a ring-level reproduction of the
exact trailer-only straddle; fixes the tLabel-extraction test fixture to be
a valid AR DMA image (trailer included).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DICE stores global-section strings little-endian: the first character of
each quadlet lives in the least-significant byte. The wire transmits
quadlets big-endian, so reading characters MSB-first byte-reversed every
quadlet ("Veni" -> "ineV"). Read LSB-first instead.
Extracted the decode into DecodeDiceNickname() (header-only) and added
DiceNicknameTests covering the Venice regression, short names, payload
bounds, and the empty case.
cross-validated with FFADO dice_avdevice.cpp:696
("Strings from the device are always little-endian").
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CopyLabelBlob had the same byte-order bug as the nickname: it read each quadlet big-endian and emitted MSB-first, byte-reversing every 4-char group. DICE stores text fields little-endian (first char in the LSB), so emit LSB-first. Add SplitDiceLabels() to split a decoded TX/RX name blob into per-channel names (separator '\', list terminated by '\\'), mirroring FFADO splitNameString/splitString. Preserves a leading empty token so the result index stays aligned with the channel index. Covered by DiceLabelTests; the nickname/label byte order is locked by DiceNicknameTests. cross-validated with FFADO dice_avdevice.cpp:1527,1547 (TX/RX name strings little-endian) + splitNameString + ffadotypes.h splitString. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds the plumbing to surface real per-channel names (not just "In N"/"Out N") without changing behavior when no labels are present: - Widen kMaxNamedChannels 8 -> 32 (Venice F32 is 32x32); the ivars name arrays and the SetElementName loops follow the constant instead of a hardcoded 8, so all elements can be named. - Model::ASFWAudioDevice gains input/outputChannelNames vectors, published as OSArray<OSString> under kInput/kOutputChannelNames. - The audio side parses them into deviceInput/OutputChannelNames; a single BuildChannelNamesFromPlugs() now prefers a device label per slot and falls back to the synthesized "<plug> N" for empty slots. Both the initial parse and BuildAudioGraph's post-profile regeneration route through it, so they agree. Inert until a backend populates the names (next commit wires DICE). No behavior change for devices that publish no labels. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wires the DICE device-truth source into the channel-name carry path: - DICETcatProtocol retains the per-channel labels parsed from the TX/RX stream-format name sections, flattened across each direction's streams in channel order (input == device TX, output == device RX per AudioTypes.hpp), published through the existing runtimeCapsValid_ release/acquire fence. Exposed via a new IDeviceProtocol::GetChannelLabels(). - DiceAudioBackend::EnsureNubForGuid eagerly loads the runtime caps (which reads global + TX + RX, including the name sections) once before the first nub publish, then fills Model::ASFWAudioDevice input/outputChannelNames from GetChannelLabels. The load early-returns if caps are already cached, is guarded against teardown, and publishes regardless of outcome so names fall back to synthesized "<plug> N" when unavailable. CoreAudio (Logic / Audio MIDI Setup) now shows the device's real channel names. Covered by DICETcatProtocolTests.ChannelLabelsFlattenAcrossStreams...; the wire decode is locked by DiceLabelTests/DiceNicknameTests. cross-validated with FFADO dice_avdevice.cpp (capture==TX, playback==RX). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…els parse ReadTx/RxStreamConfig issued a single block read capped at 512 bytes (the safe single-read payload for DICE max_rec). A multi-stream section is larger: the Venice F32 needs 8 + 2*280 = 568 bytes, and stream 1's 256-byte label blob starts at byte 304 -- so ParseStreamConfig's bounds guard silently skipped it and only stream 0's channel names (first 16 channels per direction) reached CoreAudio; channels 17-32 fell back to synthesized names. Add ReadSectionChunked: chain <=512-byte reads into one buffer covering min(section size, 4KB), then parse once. A failure after the first chunk delivers the partial buffer (parser guards each region), preserving the old best-effort behavior; only a failed first chunk is a hard error. (The PrepareSequenceMatchesReferenceWindow expectation update that rode along in the original commit stays with the sample-rate series it belongs to.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…switch)
Completes the half-built DICE clock-config machinery so CoreAudio can
select among the device's supported sample rates instead of a hardcoded
48 kHz. Validated/gated to 1x rates (32/44.1/48 kHz); 2x/4x change
frames-per-packet and per-stream geometry and stay disabled until verified
(see kDiceMaxSupportedRateHz; FFADO likewise gates 4x).
Probe/encode primitives (DICETypes.hpp), cross-validated with FFADO
dice_defines.h:
- DiceClockCapsSupportRate(clockCaps, hz) decodes CLOCKCAPABILITIES[15:0].
- DiceClockSelectForRate(hz, source) builds CLOCK_SELECT [rate<<8 | source].
Wiring:
- IsSupportedClockConfig now accepts internal-clock 1x rates (was 48k-only),
which is the sole gate that previously pinned the (already
desiredClock-parameterized) bringup to 48 kHz.
- Profile SupportedSampleRates() (DICE default {44100,48000}); PublishNub
sets dev.sampleRates; PopulateNubProperties emits kSampleRates +
kCurrentSampleRate so the HAL advertises a stream format per rate.
- ASFWAudioDevice::HandleChangeSampleRate -> ASFWAudioNub::RequestSampleRateChange
-> AudioCoordinator::RequestDiceClockConfig(new clock, kSampleRateChange):
streaming -> stop+restart at the new rate; idle -> stored as pendingClock.
- RunStartStreaming honors a stored pendingClock (rate chosen while idle)
instead of always 48 kHz.
For Venice (44.1/48, both 1x) a switch is a pure CLOCK_SELECT change with
identical 2x16 geometry. Built + tests pass; HW test pending.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BuildAudioGraph already overrides channel counts from the resolved
profile but left sampleRates sourced only from the nub property dict,
which did not reliably carry kSampleRates to the HAL — so CoreAudio saw
a single 48k format and Audio MIDI Setup showed no rate dropdown.
Override parsedConfig.sampleRates/sampleRateCount from
profile->SupportedSampleRates() (same authoritative source as the
channel counts), and add a base IAudioDeviceProfile::SupportedSampleRates
({48000}) so the method is callable on the base pointer; DICE overrides
to {44100,48000}.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The real reason the rate dropdown never appeared: BuildAudioGraph called ApplyBringupSingleFormatPolicy() unconditionally, which hard-resets sampleRates to a single 48 kHz entry — and it ran *after* the profile-driven rate override, silently wiping it. CoreAudio then saw one format and Audio MIDI Setup showed no rate selector. Track whether the resolved profile supplied its own advertised rate set and skip the single-format policy in that case. Unprofiled devices, whose multi-rate path is still unvalidated, keep the conservative single-format fallback. Also corrects the now-misleading "Forcing single advertised format" log to report the actual advertised rate count. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HandleChangeSampleRate reconfigured the transport clock but then delegated to super::HandleChangeSampleRate, which does not move the ADK device's active format — so CoreAudio reverted to the previous rate and the picker snapped back. The validated ADK contract (ADKVirtualAudioLab) applies the change by calling SetSampleRate(in_sample_rate) directly. Call SetSampleRate after the transport reconfig succeeds and return its status, so the HAL actually adopts the new rate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two bugs kept the device pinned at 44.1 kHz: 1. BuildAudioGraph set each stream's *current* format to inputFormats[0] / outputFormats[0] — i.e. sampleRates[0] (44.1 kHz in profile order) — while setting the device nominal rate to currentSampleRate (48 kHz). The HAL reconciles that mismatch by snapping the device down to the stream's format, so it always came up at 44.1 kHz. Now select the format index whose rate matches currentSampleRate for both streams. 2. IOUserAudioDevice defaults to host save/restore of stream formats, so a stale rate from a prior session could override the rate this graph selects. Call SetWantsStreamFormatsRestored(false) before publish for deterministic bring-up; rate changes go through HandleChangeSampleRate. Also log HandleChangeSampleRate on entry and on the not-ready early return, so a failed pick is no longer silent in the trace. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HandleChangeSampleRate runs on the audio device and called the nub's RequestSampleRateChange, which was declared LOCALONLY. ASFWAudioNub and the audio driver are separate IOService objects across the cross-service seam, so the audio side holds a proxy of the nub whose local ivars are null — the call hit `ivars=null` and returned kIOReturnNotReady, so the HAL reverted the rate. Move RequestSampleRateChange to a non-LOCALONLY virtual (IPC) method, mirroring the existing AudioDriver->ASFWDriver RPC bridges (AllocateTxIsochResources, Get/SetProtocolBooleanControl). It now runs in the nub's own process where the parent -> ServiceContext -> AudioCoordinator chain is valid, and updates the nub's cached rate on success. Also logs the no-registry-record not-ready path in the coordinator. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two fixes toward working sample-rate selection: 1. Device clock now adopts the selected rate on (re)start. A rate pick while idle runs RunIdleClockApply, which persists the rate into session.desiredClock/appliedClock — but RunStartStreaming only honored session.pendingClock, so the selection was dropped and the start forced 48 kHz. The device then clocked at 48k while the host ran at the picked rate (no audio). RunStartStreaming now resolves pendingClock -> desiredClock -> appliedClock -> 48k default, mirroring the bus-reset rebind path. 2. Gate live (hot) rate changes. HandleChangeSampleRate now refuses with kIOReturnBusy while IO is active, so CoreAudio keeps the current rate instead of restarting the duplex transport underneath running IO across the cross-service seam (which desynced the clock and bricked audio until replug). Rate changes apply cleanly on the next idle pick + StartIO. Full hot-swap via RequestDeviceConfigurationChange is a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First step toward 44.1 kHz playback. The blocking cadence was a 48 kHz integer accumulator (6 frames/cycle, emit 8) — correct only because 48000/8000 is integral. At 44.1 kHz the rate is 5.5125 frames/cycle, so it needs fractional frame accounting. Generalize Blocking48kCadence into a rate-parametric BlockingCadence using a rational accumulator over an 8000 cycles/s denominator (Configure(sampleRateHz, sytInterval)). Cross-validated with FFADO iec61883_cip_fill_header (libffado-2.4.9 src/libstreaming/util/cip.c): data iff floor(ready_samples + rate/8000) >= syt_interval. The old 48 kHz integer form is exactly the rate==48000 special case, so default- constructed callers are byte-identical (Blocking48kCadence kept as an alias); the TX packetizer still gates 48 k for now. Tests: 48 k pattern unchanged; 44.1 k carries 8-frame data blocks with zero drift (exactly 88200 frames / 2 s, 441000 / 10 s). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire the rate-parametric cadence into the playback path so 44.1 kHz produces a correct stream instead of a 48 kHz cadence: - AmdtpTxPacketizer::Configure no longer hard-rejects non-48k. It resolves the rate via AmdtpRateGeometryForSampleRate, configures the blocking cadence with the rate + SYT interval, and sets the CIP FDF from the rate's AM824 SFC (44.1k=0x01, 48k=0x02) instead of the profile's hardcoded 0x02. Non-blocking stays integral-rate (48k) only. - AmdtpRateGeometry carries the per-rate FDF/SFC (cross-checked vs FFADO getFDF()). - StartIO overrides the profile's default 48k txConfig.sampleRate with the device's current nominal rate for both the primary and secondary (Venice 2x16) playback streams. Known remaining gap: the core endpoint runtime's config_.currentSampleRate (binding/RX clock nominal) is only refreshed via OnAVCAudioConfigurationReady, not on a DICE rate change, so it can stay 48k. That mainly affects RX/ppm diagnostics; TX SYT follows the device clock. To be verified on hardware. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
44.1 kHz playback never started: the IT prime failed with "committed prefill=7910 must cover 48 descriptors within 384 slots". Root cause: TxStreamControl had no producer-side cursor reset. On a sample-rate change CoreAudio re-probes the device with several rapid StartIO/StopIO cycles, and the shared TX slab is reused without being re-zeroed, so exposeCursor/completionCursor carried over and grew past the 384-slot ring — failing IsochTxDmaRing::Prime (which requires numPackets <= exposeCursor <= numSlots). 48 kHz never tripped this because it starts once and runs. Add TxStreamControl::ResetForStart() (clears the runtime progress cursors, preserves the core-written geometry) and call it in StartIO after mapping each TX control block (primary + Venice secondary), before the prefill and before the transport consumer arms. Makes repeated StartIO idempotent regardless of slab reuse. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…urn) After the prime fix, 44.1k TX started but CoreAudio churned StartIO/StopIO every ~1s and shipped only silence. The host streams were correctly at 44100 (ADK STATE rate=44100), but the RX direct-binding clock stayed at 48000 (IR: Arming direct Rx ... rate=48000): the core endpoint's config_.currentSampleRate is seeded at publish (48k) and a DICE rate change never updated it. The ZTS the driver reports to CoreAudio then advanced at 48k while the device ran 44.1k (~8.8% error), so CoreAudio rejected the device and restarted it repeatedly — never settling, so Logic never delivered real audio. Add AudioEndpointRuntime::SetCurrentSampleRate and call it from AudioCoordinator::RequestDiceClockConfig on success, so the next StartIO seeds the direct-binding/ZTS clock at the live rate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SetCurrentSampleRate updated config_.currentSampleRate but not directSampleRateHz_, the field CopyDirectAudioBinding actually reports to the IR. directSampleRateHz_ is latched only when EnsureDirectAudioMemoryLocked allocates the (rate-independent) ring buffers, which never re-runs on an idle rate change. So the RX/ZTS clock stayed at the publish-time 48 kHz while the device ran 44.1 kHz and CoreAudio churned StartIO/StopIO on the mismatch. Refresh directSampleRateHz_ and bump the binding generation here so the IR re-arms the clock at the live rate; buffers are unchanged, no reallocation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PrepareDuplex48k, the per-StartIO bring-up hook, hardcoded a 48 kHz CLOCK_SELECT and wrote it to the device on every StartIO. After an idle change to 44.1 kHz, each StartIO rewrote 48 kHz and fought the rate-change path: the device PLL flapped 44.1k<->48k (RX FDF 0x01<->0x02) and audio starved. Remember the last applied clock (ApplyClockConfig / PrepareDuplex) in selectedClock_ and target it from PrepareDuplex48k, falling back to 48 kHz only before any rate has been selected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The secondary (multi-stream) TX configure line omitted the sample rate, so the log could not confirm both Venice F32 sub-streams pick up a rate change. Add rate= to match the primary stream's line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bring-up clock "confirm" could fire on the device's CLOCK_ACCEPTED ack alone (notify bits) while the PLL was still relocking (status reports the old rate / locked=0), so isoch streams were programmed/enabled on an unstable clock. Add DoAwaitStreamingClockLock between clock-confirm and stream discovery in the kPrepareDuplex path: poll global status until IsSourceLocked and the nominal+sample rate equal the target, up to the existing 2 s timeout, then enable. A same-rate restart is already locked so it returns on the first read; kClockApply is untouched (it does not enable streams). HW: verified the gate works (round trip 48k<->44.1k shows "stable-locked ... enabling streams" only after locked=1, FDF tracks 0x02<->0x01). NOTE this corrects the clock layer but does NOT fix the rate-change audio loss -- that root cause is device-side dual-stream re-establishment, not the clock. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…m 0) DoStopDisableTx/Rx read the per-stream SIZE stride, discarded it, and wrote kDisabledIsoChannel only to stream[0]. On a multi-stream device (Venice F32 = 2x2 streams) the secondary TX/RX ISOC registers kept their stale channels across every stop. The next bring-up's FirstActiveIsoChannel then seeded stream[0] from the STALE SECONDARY channel, shifting the whole channel map (observed rxIso=0 txIso=1 -> rxIso=3 txIso=2), and DoProgramRx transiently put two device RX streams on the same channel - undefined firmware state that wedged the master stream (ch1-16 dead, ch17-32 alive) until a device power cycle reset the registers. Walk all streams with the SIZE-derived stride (same address math as DoProgramRx/DoProgramTx), clearing ISOC + speed/seq-start per stream; a failed stride read falls back to the legacy stream[0]-only clear. Cross-validated with FFADO dice_avdevice.cpp startstopStreamByIndex:1415-1446 (stop writes 0xFFFFFFFF to EVERY stream's ISO_CHANNEL; a non-cleared register at start is treated as crash recovery). Also: reset the master's shared replay/cadence timeline on every direct-audio rebind (not once per Start) so a rate-change rebind re-establishes cleanly, and log the reset + skip the redundant CLOCK_SELECT write when the device is already at the target clock (single PLL relock per rate change, while idle). HW-verified: channel map now identical across every bring-up, and a rate change no longer permanently wedges the device (full 32ch recovery at both rates after app restart; no power cycle needed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… deviations The reference trace always rewrites CLOCK_SELECT during prepare; ASFW deviates in two HW-validated ways (skip the redundant CLOCK_SELECT write when already at target; one extra global-state read for the stable-lock gate). Build the adjusted expectation from the generated fixture in the test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alicankaralar
force-pushed
the
feat/dice-sample-rates
branch
from
1b43427 to
40e9b54
Compare
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
1 participant
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.
Adds 44k Sample Rate change capability for DICE interfaces. The Config ROM advertises 44k capability. We're populating native MacOS Sample Rate selector with the options retrieved from the ROM. A Sample Rate Change (on Audio Midi Setup or Logic) should now instruct the Chipset to switch to 44k. Tested on Midas Venice.
Should be merged after the Venice PR, Fix Aggregate Devices PR and Channel Names PR since the changes are based on the changes to be introduced in these PRs