From eff5abf057486fcbf8b457a5ec66f22b0ae1b34f Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Fri, 3 Jul 2026 13:41:11 +0200 Subject: [PATCH 1/7] fix(isoch): IsochReceiveContext born with refcount 0 aborts dext at teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ASFWDriver/Common/DriverKitUtils.hpp | 32 ------------------- ASFWDriver/Isoch/IsochService.hpp | 4 +-- .../Isoch/Receive/IsochReceiveContext.cpp | 17 ++-------- .../Isoch/Receive/IsochReceiveContext.hpp | 22 ++++++------- tests/audio/IsochReceiveContextTests.cpp | 4 +-- 5 files changed, 17 insertions(+), 62 deletions(-) delete mode 100644 ASFWDriver/Common/DriverKitUtils.hpp diff --git a/ASFWDriver/Common/DriverKitUtils.hpp b/ASFWDriver/Common/DriverKitUtils.hpp deleted file mode 100644 index d5801c60..00000000 --- a/ASFWDriver/Common/DriverKitUtils.hpp +++ /dev/null @@ -1,32 +0,0 @@ -#pragma once - -#include -#include -#include - -namespace ASFW::Common { - -/// Factory helper for DriverKit OSObject-derived types. -/// -/// The OSObject allocation model requires `new (std::nothrow)` + null-check + wrap in -/// OSSharedPtr. This helper encapsulates that unavoidable pattern so callsites are -/// raw-pointer-free and the single NOSONAR suppression is centralised here. -/// -/// Requirements on T: -/// - Must inherit from OSObject (enforced at compile time) -/// - Must be constructible with the supplied arguments -/// - operator new must call IOMallocZero or equivalent -template -[[nodiscard]] OSSharedPtr MakeOSObject(Args&&... args) noexcept -{ - static_assert(std::is_base_of_v, - "MakeOSObject() requires T to derive from OSObject"); - // NOSONAR(cpp:S5025): DriverKit OSObject uses intrusive ref-counting. - // Allocation must be `new (std::nothrow)`; result is immediately transferred - // into OSSharedPtr with OSNoRetain — no raw pointer escapes this function. - auto* raw = new (std::nothrow) T(std::forward(args)...); // NOSONAR(cpp:S5025) - if (!raw) return {}; - return OSSharedPtr(raw, OSNoRetain); -} - -} // namespace ASFW::Common diff --git a/ASFWDriver/Isoch/IsochService.hpp b/ASFWDriver/Isoch/IsochService.hpp index 4055d06d..23a405a8 100644 --- a/ASFWDriver/Isoch/IsochService.hpp +++ b/ASFWDriver/Isoch/IsochService.hpp @@ -162,12 +162,12 @@ class IsochService { // Stream 0 (master) capture/playback contexts — own the clock/ZTS/replay // role. Their lifecycle and callbacks are unchanged from the single-stream // design; secondary streams layer on top without touching them. - OSSharedPtr isochReceiveContext_; + std::unique_ptr isochReceiveContext_; std::unique_ptr isochTransmitContext_; // Secondary streams [1 .. kMaxStreamsPerDirection). Index i here maps to // stream (i + 1); each runs on its own OHCI context (contextIndex == stream). - OSSharedPtr + std::unique_ptr secondaryReceiveContexts_[kMaxStreamsPerDirection - 1]; std::unique_ptr secondaryTransmitContexts_[kMaxStreamsPerDirection - 1]; diff --git a/ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp b/ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp index 9ba26a2f..3d7240ce 100644 --- a/ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp +++ b/ASFWDriver/Isoch/Receive/IsochReceiveContext.cpp @@ -3,7 +3,6 @@ #include "../../Audio/DriverKit/Runtime/DirectAudioBindingSource.hpp" #include "../../Common/TimingUtils.hpp" -#include "../../Common/DriverKitUtils.hpp" #include "../../Hardware/OHCIConstants.hpp" #include "../../Hardware/RegisterMap.hpp" #include "../../Diagnostics/Signposts.hpp" @@ -16,16 +15,14 @@ namespace ASFW::Isoch { // Factory // ============================================================================ -OSSharedPtr IsochReceiveContext::Create(::ASFW::Driver::HardwareInterface* hw, +std::unique_ptr IsochReceiveContext::Create(::ASFW::Driver::HardwareInterface* hw, std::shared_ptr<::ASFW::Isoch::Memory::IIsochDMAMemory> dmaMemory) { - auto ctx = ASFW::Common::MakeOSObject(); + auto ctx = std::unique_ptr(new (std::nothrow) IsochReceiveContext()); if (!ctx) return nullptr; ctx->hardware_ = hw; ctx->dmaMemory_ = std::move(dmaMemory); - if (!ctx->init()) return nullptr; // OSSharedPtr destructor calls release() - return ctx; } @@ -33,16 +30,8 @@ OSSharedPtr IsochReceiveContext::Create(::ASFW::Driver::Har // Lifecycle // ============================================================================ -bool IsochReceiveContext::init() { - if (!OSObject::init()) { - return false; - } - return true; -} - -void IsochReceiveContext::free() { +IsochReceiveContext::~IsochReceiveContext() { Stop(); - OSObject::free(); } // ============================================================================ diff --git a/ASFWDriver/Isoch/Receive/IsochReceiveContext.hpp b/ASFWDriver/Isoch/Receive/IsochReceiveContext.hpp index e50b73b7..3aad60fc 100644 --- a/ASFWDriver/Isoch/Receive/IsochReceiveContext.hpp +++ b/ASFWDriver/Isoch/Receive/IsochReceiveContext.hpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -57,23 +56,24 @@ struct IRTag { static constexpr const char kContextName[] = "IsochReceiveContext"; }; -class IsochReceiveContext - : public OSObject, - public ::ASFW::Shared::DmaContextManagerBase< +// Plain C++ class, deliberately NOT an OSObject: nothing needs OSObject +// semantics (no OSAction target, no cast, no IIG surface), and an OSObject +// created with plain `new` instead of OSTypeAlloc is born with refcount 0 — +// its first release() aborts the dext with an over-release assert at teardown. +// Owned by std::unique_ptr, matching IsochTransmitContext. +class IsochReceiveContext final + : public ::ASFW::Shared::DmaContextManagerBase< IsochReceiveContext, ::ASFW::Shared::DescriptorRing, IRTag, IRPolicy> { public: IsochReceiveContext() : ::ASFW::Shared::DmaContextManagerBase(*this, descriptorRing_) {} + ~IsochReceiveContext(); - virtual bool init() override; - virtual void free() override; + IsochReceiveContext(const IsochReceiveContext&) = delete; + IsochReceiveContext& operator=(const IsochReceiveContext&) = delete; - void* operator new(size_t size) { return IOMallocZero(size); } - void* operator new(size_t size, std::nothrow_t const&) { return IOMallocZero(size); } - void operator delete(void* ptr, size_t size) { IOFree(ptr, size); } - - static OSSharedPtr + static std::unique_ptr Create(::ASFW::Driver::HardwareInterface* hw, std::shared_ptr<::ASFW::Isoch::Memory::IIsochDMAMemory> dmaMemory); diff --git a/tests/audio/IsochReceiveContextTests.cpp b/tests/audio/IsochReceiveContextTests.cpp index 22fb6975..3082636d 100644 --- a/tests/audio/IsochReceiveContextTests.cpp +++ b/tests/audio/IsochReceiveContextTests.cpp @@ -42,8 +42,6 @@ class IsochReceiveContextTest : public Test { void TearDown() override { if (context_) { context_->Stop(); - // context_->release(); // OSSharedPtr manages refcount? No, it's a smart pointer wrapper. - // If Create returns OSSharedPtr, we should just let it destruct or reset. context_.reset(); } if (hardware_) { @@ -54,7 +52,7 @@ class IsochReceiveContextTest : public Test { ::ASFW::Driver::HardwareInterface* hardware_{nullptr}; std::shared_ptr dmaMemory_; - OSSharedPtr context_; + std::unique_ptr context_; }; TEST_F(IsochReceiveContextTest, Initialization) { From b122c74cae82014494754f6e8df490fcda3192ce Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Fri, 3 Jul 2026 14:19:53 +0200 Subject: [PATCH 2/7] =?UTF-8?q?fix(async):=20AR=20packet=20trailer=20is=20?= =?UTF-8?q?mandatory=20=E2=80=94=20optional-trailer=20parse=20jammed=20the?= =?UTF-8?q?=20AR=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ASFWDriver/Async/Rx/ARPacketParser.cpp | 248 +++++++++--------- .../AsyncPacketSerDesLinuxCompatTests.cpp | 98 ++++--- tests/async/BufferRingDMATests.cpp | 113 ++++++++ 3 files changed, 294 insertions(+), 165 deletions(-) diff --git a/ASFWDriver/Async/Rx/ARPacketParser.cpp b/ASFWDriver/Async/Rx/ARPacketParser.cpp index 06ebe369..ea731d47 100644 --- a/ASFWDriver/Async/Rx/ARPacketParser.cpp +++ b/ASFWDriver/Async/Rx/ARPacketParser.cpp @@ -1,7 +1,7 @@ #include "ARPacketParser.hpp" #include "../../Hardware/IEEE1394.hpp" -#include "../../Logging/Logging.hpp" #include "../../Logging/LogConfig.hpp" +#include "../../Logging/Logging.hpp" #ifdef ASFW_HOST_TEST #include // OSSwapLittleToHostInt32 for host tests @@ -18,11 +18,11 @@ namespace ASFW::Async { static inline uint32_t le32_at(const uint8_t* p) { uint32_t v; __builtin_memcpy(&v, p, sizeof(v)); - return OSSwapLittleToHostInt32(v); // LE to host (no-op on arm64, documents intent) + return OSSwapLittleToHostInt32(v); // LE to host (no-op on arm64, documents intent) } -std::optional> ARPacketParser::ExtractPhyPacketQuadletsHostOrder( - std::span header) { +std::optional> +ARPacketParser::ExtractPhyPacketQuadletsHostOrder(std::span header) { if (header.size() < 12) { return std::nullopt; } @@ -33,10 +33,8 @@ std::optional> ARPacketParser::ExtractPhyPacketQuadletsH }; } -std::expected ARPacketParser::ParseNext( - std::span buffer, - size_t offset) -{ +std::expected +ARPacketParser::ParseNext(std::span buffer, size_t offset) { const size_t bufferSize = buffer.size(); if (buffer.empty() || offset + 8 > bufferSize) { @@ -52,9 +50,10 @@ std::expected ARPacket for (size_t i = 0; i < dumpSize; i += 16) { const size_t chunkSize = (i + 16 <= dumpSize) ? 16 : (dumpSize - i); const uint8_t* bytes = packetStart + i; - ASFW_LOG_HEX(Async, " [%02zu] %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X", - i, - chunkSize > 0 ? bytes[0] : 0, chunkSize > 1 ? bytes[1] : 0, + ASFW_LOG_HEX(Async, + " [%02zu] %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X %02X " + "%02X %02X %02X %02X", + i, chunkSize > 0 ? bytes[0] : 0, chunkSize > 1 ? bytes[1] : 0, chunkSize > 2 ? bytes[2] : 0, chunkSize > 3 ? bytes[3] : 0, chunkSize > 4 ? bytes[4] : 0, chunkSize > 5 ? bytes[5] : 0, chunkSize > 6 ? bytes[6] : 0, chunkSize > 7 ? bytes[7] : 0, @@ -80,7 +79,8 @@ std::expected ARPacket const size_t headerLength = GetHeaderLength(tCode); if (headerLength == 0) { - ASFW_LOG_V0(Async, "❌ ARPacketParser::ParseNext: Unknown tCode=0x%X at offset %zu", tCode, offset); + ASFW_LOG_V0(Async, "❌ ARPacketParser::ParseNext: Unknown tCode=0x%X at offset %zu", tCode, + offset); return std::unexpected(ParseFailure::UnknownTCode); } @@ -97,8 +97,10 @@ std::expected ARPacket // treated as a stitchable fragment — it can never complete. // Cross-validated with Linux ohci.c handle_ar_packet: // payload_length > MAX_ASYNC_PAYLOAD → ar_context_abort("invalid packet length"). - ASFW_LOG_V0(Async, "❌ ARPacketParser::ParseNext: data_length %zu exceeds max %zu (tCode=0x%X offset=%zu)", - dataLength, kMaxAsyncPayloadBytes, tCode, offset); + ASFW_LOG_V0( + Async, + "❌ ARPacketParser::ParseNext: data_length %zu exceeds max %zu (tCode=0x%X offset=%zu)", + dataLength, kMaxAsyncPayloadBytes, tCode, offset); return std::unexpected(ParseFailure::OversizedPayload); } const size_t quadletAlignedLen = ((headerLength + dataLength + 3) & ~size_t(3)); @@ -114,17 +116,14 @@ std::expected ARPacket __builtin_memcpy(&trailer_le, packetStart + quadletAlignedLen, 4); const uint32_t trailer = OSSwapLittleToHostInt32(trailer_le); const uint16_t xferStatus = static_cast(trailer >> 16); - const uint16_t timeStamp = static_cast(trailer & 0xFFFF); + const uint16_t timeStamp = static_cast(trailer & 0xFFFF); // ---- rCode (only for response tCodes): extract from Q1 bits[15:12] // Per Linux packet-header-definitions.h: ASYNC_HEADER_Q1_RCODE_SHIFT = 12 // IEEE 1394 format: Q1 = [source_ID:16][rcode:4][offset_high:12] uint8_t rCode = 0xFF; // 0xFF = "not present" - if (tCode == kTCodeWriteResponse || - tCode == kTCodeReadQuadletResponse || - tCode == kTCodeReadBlockResponse || - tCode == kTCodeLockResponse) - { + if (tCode == kTCodeWriteResponse || tCode == kTCodeReadQuadletResponse || + tCode == kTCodeReadBlockResponse || tCode == kTCodeLockResponse) { rCode = static_cast((q1 >> 12) & 0xF); } @@ -140,8 +139,8 @@ std::expected ARPacket info.totalLength = quadletAlignedLen + 4; info.tCode = tCode; info.rCode = rCode; - info.xferStatus = xferStatus; // stored in 32-bit field; value range 0..0xFFFF - info.timeStamp = timeStamp; // stored in 32-bit field; value range 0..0xFFFF + info.xferStatus = xferStatus; // stored in 32-bit field; value range 0..0xFFFF + info.timeStamp = timeStamp; // stored in 32-bit field; value range 0..0xFFFF return info; } @@ -154,52 +153,52 @@ size_t ARPacketParser::GetHeaderLength(uint8_t tCode) { size_t length = 0; switch (tCode) { - case kTCodeWriteQuadlet: // 0x0 TCODE_WRITE_QUADLET_REQUEST - length = 16; // 4 quadlets: header + data quadlet - break; - - case kTCodeReadQuadletResponse: // 0x6 TCODE_READ_QUADLET_RESPONSE - // IEEE 1394: Read Quadlet Response has 4 quadlets total (16 bytes) - // Quadlet 0: destination/tLabel/tCode - // Quadlet 1: source/rCode - // Quadlet 2: reserved - // Quadlet 3: DATA PAYLOAD (4 bytes) - embedded in header, not separate payload - length = 16; // 4 quadlets including data - break; - - case kTCodeReadBlock: // 0x5 TCODE_READ_BLOCK_REQUEST - length = 16; // 4 quadlets - break; - - case kTCodeWriteBlock: // 0x1 TCODE_WRITE_BLOCK_REQUEST - case kTCodeReadBlockResponse: // 0x7 TCODE_READ_BLOCK_RESPONSE - case kTCodeLockRequest: // 0x9 TCODE_LOCK_REQUEST - case kTCodeLockResponse: // 0xB TCODE_LOCK_RESPONSE - length = 16; // 4 quadlets - break; - - case kTCodeWriteResponse: // 0x2 TCODE_WRITE_RESPONSE - case kTCodeReadQuadlet: // 0x4 TCODE_READ_QUADLET_REQUEST - length = 12; // 3 quadlets (Linux: p.header_length = 12) - break; - - case kTCodePhyPacket: // 0xE TCODE_LINK_INTERNAL/PHY - // CRITICAL: Per Linux drivers/firewire/ohci.c handle_ar_packet - // TCODE_LINK_INTERNAL (0xe): p.header_length = 12 (3 quadlets) - // PHY packet structure per OHCI §8.4.2.3: - // Quadlet 0: tcode[31:28]=0xE, event[3:0] - // Quadlets 1-2: PHY payload; synthetic bus-reset markers reuse - // quadlet 1 for the selfIDGeneration[23:16] field. - // Total: 12 bytes header + 4 bytes trailer = 16 bytes - length = 12; // 3 quadlets (matches Linux!) - break; - - default: - // Any other tCode (incl. cycle start 0x8, iso 0xA — never delivered - // to AR contexts with our LinkControl, same as Linux) is invalid - // here; Linux ar_context_abort()s on it ("invalid tcode"). - ASFW_LOG_V0(Async, "❌ GetHeaderLength: Unknown tCode=0x%X", tCode); - return 0; // Unknown tCode + case kTCodeWriteQuadlet: // 0x0 TCODE_WRITE_QUADLET_REQUEST + length = 16; // 4 quadlets: header + data quadlet + break; + + case kTCodeReadQuadletResponse: // 0x6 TCODE_READ_QUADLET_RESPONSE + // IEEE 1394: Read Quadlet Response has 4 quadlets total (16 bytes) + // Quadlet 0: destination/tLabel/tCode + // Quadlet 1: source/rCode + // Quadlet 2: reserved + // Quadlet 3: DATA PAYLOAD (4 bytes) - embedded in header, not separate payload + length = 16; // 4 quadlets including data + break; + + case kTCodeReadBlock: // 0x5 TCODE_READ_BLOCK_REQUEST + length = 16; // 4 quadlets + break; + + case kTCodeWriteBlock: // 0x1 TCODE_WRITE_BLOCK_REQUEST + case kTCodeReadBlockResponse: // 0x7 TCODE_READ_BLOCK_RESPONSE + case kTCodeLockRequest: // 0x9 TCODE_LOCK_REQUEST + case kTCodeLockResponse: // 0xB TCODE_LOCK_RESPONSE + length = 16; // 4 quadlets + break; + + case kTCodeWriteResponse: // 0x2 TCODE_WRITE_RESPONSE + case kTCodeReadQuadlet: // 0x4 TCODE_READ_QUADLET_REQUEST + length = 12; // 3 quadlets (Linux: p.header_length = 12) + break; + + case kTCodePhyPacket: // 0xE TCODE_LINK_INTERNAL/PHY + // CRITICAL: Per Linux drivers/firewire/ohci.c handle_ar_packet + // TCODE_LINK_INTERNAL (0xe): p.header_length = 12 (3 quadlets) + // PHY packet structure per OHCI §8.4.2.3: + // Quadlet 0: tcode[31:28]=0xE, event[3:0] + // Quadlets 1-2: PHY payload; synthetic bus-reset markers reuse + // quadlet 1 for the selfIDGeneration[23:16] field. + // Total: 12 bytes header + 4 bytes trailer = 16 bytes + length = 12; // 3 quadlets (matches Linux!) + break; + + default: + // Any other tCode (incl. cycle start 0x8, iso 0xA — never delivered + // to AR contexts with our LinkControl, same as Linux) is invalid + // here; Linux ar_context_abort()s on it ("invalid tcode"). + ASFW_LOG_V0(Async, "❌ GetHeaderLength: Unknown tCode=0x%X", tCode); + return 0; // Unknown tCode } ASFW_LOG_V3(Async, "GetHeaderLength(tCode=0x%X) → %zu bytes", tCode, length); @@ -216,63 +215,66 @@ size_t ARPacketParser::GetDataLength(std::span header, uint8_t tC size_t dataLen = 0; switch (tCode) { - case kTCodePhyPacket: // 0xE TCODE_LINK_INTERNAL - // PHY packet structure per Linux & OHCI §8.4.2.3: - // - Header: 12 bytes (3 quadlets) - all PHY data is part of header! - // - Data: 0 bytes (no separate payload) - // - Trailer: 4 bytes (xferStatus + timestamp) - // TOTAL: 12 (hdr) + 0 (data) + 4 (trailer) = 16 bytes - // - // Linux: p.header_length=12, p.payload_length=0 - // All PHY-specific data is considered part of the header - dataLen = 0; // No separate data payload! - ASFW_LOG_V3(Async, "GetDataLength: PHY packet → 0 bytes data (all in 12-byte header)"); - break; - - case kTCodeWriteBlock: // 0x1 TCODE_WRITE_BLOCK_REQUEST - case kTCodeReadBlockResponse: // 0x7 TCODE_READ_BLOCK_RESPONSE - case kTCodeLockRequest: // 0x9 TCODE_LOCK_REQUEST - case kTCodeLockResponse: // 0xB TCODE_LOCK_RESPONSE - { - // Extract data_length from quadlet 3, bits[31:16] - // Header quadlet 3 is at offset 12 (bytes 12-15) - if (header.size() < 16) { - ASFW_LOG_V0(Async, "❌ GetDataLength: Header too small (%zu bytes) for block tCode=0x%X", - header.size(), tCode); - return 0; - } - - // Read q3 from AR DMA buffer (little-endian) and convert to host order - const uint32_t q3 = le32_at(header.data() + 12); - // Extract data_length from high 16 bits - const uint16_t length = static_cast((q3 >> 16) & 0xFFFF); - dataLen = length; - - ASFW_LOG_V3(Async, "GetDataLength: Block tCode=0x%X q3=0x%08X (LE) → data_length=%u bytes", - tCode, q3, length); - break; + case kTCodePhyPacket: // 0xE TCODE_LINK_INTERNAL + // PHY packet structure per Linux & OHCI §8.4.2.3: + // - Header: 12 bytes (3 quadlets) - all PHY data is part of header! + // - Data: 0 bytes (no separate payload) + // - Trailer: 4 bytes (xferStatus + timestamp) + // TOTAL: 12 (hdr) + 0 (data) + 4 (trailer) = 16 bytes + // + // Linux: p.header_length=12, p.payload_length=0 + // All PHY-specific data is considered part of the header + dataLen = 0; // No separate data payload! + ASFW_LOG_V3(Async, "GetDataLength: PHY packet → 0 bytes data (all in 12-byte header)"); + break; + + case kTCodeWriteBlock: // 0x1 TCODE_WRITE_BLOCK_REQUEST + case kTCodeReadBlockResponse: // 0x7 TCODE_READ_BLOCK_RESPONSE + case kTCodeLockRequest: // 0x9 TCODE_LOCK_REQUEST + case kTCodeLockResponse: // 0xB TCODE_LOCK_RESPONSE + { + // Extract data_length from quadlet 3, bits[31:16] + // Header quadlet 3 is at offset 12 (bytes 12-15) + if (header.size() < 16) { + ASFW_LOG_V0(Async, + "❌ GetDataLength: Header too small (%zu bytes) for block tCode=0x%X", + header.size(), tCode); + return 0; } - case kTCodeReadQuadletResponse: // 0x6 TCODE_READ_QUADLET_RESPONSE - // IEEE 1394: Data is embedded in header quadlet 3 (offset 12-15), not separate payload - // Header length is 16 bytes, and q3 contains the 4-byte data value - // Since data is part of header, dataLength is 0 (no separate payload follows) - dataLen = 0; - ASFW_LOG_V3(Async, "GetDataLength: tCode=0x6 (Read Quadlet Response) → 0 bytes (data in header q3)"); - break; - - case kTCodeWriteResponse: // 0x2 TCODE_WRITE_RESPONSE - // No separate payload. (Write-compare is LOCK, not a write response.) - dataLen = 0; - ASFW_LOG_V3(Async, "GetDataLength: tCode=0x2 (Write Response) → 0 bytes"); - break; - - default: - // No separate data (quadlet transactions, simple responses) - // Per Linux: p.payload_length = 0 for these tCodes - dataLen = 0; - ASFW_LOG_V3(Async, "GetDataLength: tCode=0x%X → no payload (0 bytes)", tCode); - break; + // Read q3 from AR DMA buffer (little-endian) and convert to host order + const uint32_t q3 = le32_at(header.data() + 12); + // Extract data_length from high 16 bits + const uint16_t length = static_cast((q3 >> 16) & 0xFFFF); + dataLen = length; + + ASFW_LOG_V3(Async, "GetDataLength: Block tCode=0x%X q3=0x%08X (LE) → data_length=%u bytes", + tCode, q3, length); + break; + } + + case kTCodeReadQuadletResponse: // 0x6 TCODE_READ_QUADLET_RESPONSE + // IEEE 1394: Data is embedded in header quadlet 3 (offset 12-15), not separate payload + // Header length is 16 bytes, and q3 contains the 4-byte data value + // Since data is part of header, dataLength is 0 (no separate payload follows) + dataLen = 0; + ASFW_LOG_V3( + Async, + "GetDataLength: tCode=0x6 (Read Quadlet Response) → 0 bytes (data in header q3)"); + break; + + case kTCodeWriteResponse: // 0x2 TCODE_WRITE_RESPONSE + // No separate payload. (Write-compare is LOCK, not a write response.) + dataLen = 0; + ASFW_LOG_V3(Async, "GetDataLength: tCode=0x2 (Write Response) → 0 bytes"); + break; + + default: + // No separate data (quadlet transactions, simple responses) + // Per Linux: p.payload_length = 0 for these tCodes + dataLen = 0; + ASFW_LOG_V3(Async, "GetDataLength: tCode=0x%X → no payload (0 bytes)", tCode); + break; } return dataLen; diff --git a/tests/async/AsyncPacketSerDesLinuxCompatTests.cpp b/tests/async/AsyncPacketSerDesLinuxCompatTests.cpp index 4a9f3e98..8fa7cfed 100644 --- a/tests/async/AsyncPacketSerDesLinuxCompatTests.cpp +++ b/tests/async/AsyncPacketSerDesLinuxCompatTests.cpp @@ -18,17 +18,16 @@ #include #include "ASFWDriver/Async/AsyncTypes.hpp" -#include "ASFWDriver/Hardware/IEEE1394.hpp" #include "ASFWDriver/Async/Rx/ARPacketParser.hpp" #include "ASFWDriver/Async/Rx/PacketRouter.hpp" #include "ASFWDriver/Async/Tx/PacketBuilder.hpp" +#include "ASFWDriver/Hardware/IEEE1394.hpp" using namespace ASFW::Async; namespace { -template -constexpr std::array LoadHostQuadlets(const uint8_t* base) { +template constexpr std::array LoadHostQuadlets(const uint8_t* base) { std::array words{}; std::memcpy(words.data(), base, N * sizeof(uint32_t)); return words; @@ -97,11 +96,12 @@ TEST(AsyncPacketSerDesLinuxCompat, ReadQuadletRequestMatchesLinuxVector) { const auto hostWords = LoadHostQuadlets<3>(buffer.data()); // Verify Linux OHCI format - EXPECT_EQ((hostWords[0] >> 10) & 0x3Fu, kLabel); // tLabel at bits[15:10] - EXPECT_EQ((hostWords[0] >> 16) & 0x7u, context.speedCode & 0x7u); // speed at bits[18:16] - EXPECT_EQ((hostWords[0] >> 8) & 0x3u, 0x01u); // retry at bits[9:8] - EXPECT_EQ((hostWords[0] >> 4) & 0xFu, HW::AsyncRequestHeader::kTcodeReadQuad); // tCode at bits[7:4] - const uint16_t destID = static_cast(hostWords[1] >> 16); // destID in Q1[31:16] + EXPECT_EQ((hostWords[0] >> 10) & 0x3Fu, kLabel); // tLabel at bits[15:10] + EXPECT_EQ((hostWords[0] >> 16) & 0x7u, context.speedCode & 0x7u); // speed at bits[18:16] + EXPECT_EQ((hostWords[0] >> 8) & 0x3u, 0x01u); // retry at bits[9:8] + EXPECT_EQ((hostWords[0] >> 4) & 0xFu, + HW::AsyncRequestHeader::kTcodeReadQuad); // tCode at bits[7:4] + const uint16_t destID = static_cast(hostWords[1] >> 16); // destID in Q1[31:16] EXPECT_EQ(destID, MakeDestinationID(context.sourceNodeID, params.destinationID)); EXPECT_EQ(static_cast(hostWords[1] & 0xFFFFu), static_cast(params.addressHigh)); @@ -131,10 +131,11 @@ TEST(AsyncPacketSerDesLinuxCompat, WriteQuadletRequestMatchesLinuxVector) { const auto hostWords = LoadHostQuadlets<4>(buffer.data()); // Verify Linux OHCI format - EXPECT_EQ((hostWords[0] >> 10) & 0x3Fu, kLabel); // tLabel at bits[15:10] - EXPECT_EQ((hostWords[0] >> 16) & 0x7u, context.speedCode & 0x7u); // speed at bits[18:16] - EXPECT_EQ((hostWords[0] >> 8) & 0x3u, 0x01u); // retry at bits[9:8] - EXPECT_EQ((hostWords[0] >> 4) & 0xFu, HW::AsyncRequestHeader::kTcodeWriteQuad); // tCode at bits[7:4] + EXPECT_EQ((hostWords[0] >> 10) & 0x3Fu, kLabel); // tLabel at bits[15:10] + EXPECT_EQ((hostWords[0] >> 16) & 0x7u, context.speedCode & 0x7u); // speed at bits[18:16] + EXPECT_EQ((hostWords[0] >> 8) & 0x3u, 0x01u); // retry at bits[9:8] + EXPECT_EQ((hostWords[0] >> 4) & 0xFu, + HW::AsyncRequestHeader::kTcodeWriteQuad); // tCode at bits[7:4] EXPECT_EQ(hostWords[3], payloadQuadlet); } @@ -158,8 +159,9 @@ TEST(AsyncPacketSerDesLinuxCompat, WriteBlockRequestMatchesLinuxVector) { const auto hostWords = LoadHostQuadlets<4>(buffer.data()); - EXPECT_EQ((hostWords[0] >> 10) & 0x3Fu, kLabel); // tLabel at bits[15:10] - EXPECT_EQ((hostWords[0] >> 4) & 0xFu, HW::AsyncRequestHeader::kTcodeWriteBlock); // tCode at bits[7:4] + EXPECT_EQ((hostWords[0] >> 10) & 0x3Fu, kLabel); // tLabel at bits[15:10] + EXPECT_EQ((hostWords[0] >> 4) & 0xFu, + HW::AsyncRequestHeader::kTcodeWriteBlock); // tCode at bits[7:4] EXPECT_EQ(hostWords[3], static_cast(params.length) << 16); EXPECT_EQ(static_cast((hostWords[1] >> 16) & 0xFFFFu), @@ -191,11 +193,11 @@ TEST(AsyncPacketSerDesLinuxCompat, LockRequestMatchesLinuxVector) { const auto hostWords = LoadHostQuadlets<4>(buffer.data()); - EXPECT_EQ((hostWords[0] >> 10) & 0x3Fu, kLabel); // tLabel at bits[15:10] - EXPECT_EQ((hostWords[0] >> 4) & 0xFu, HW::AsyncRequestHeader::kTcodeLockRequest); // tCode at bits[7:4] - EXPECT_EQ(hostWords[3], - (static_cast(params.operandLength) << 16) | - static_cast(kExtendedTCode)); + EXPECT_EQ((hostWords[0] >> 10) & 0x3Fu, kLabel); // tLabel at bits[15:10] + EXPECT_EQ((hostWords[0] >> 4) & 0xFu, + HW::AsyncRequestHeader::kTcodeLockRequest); // tCode at bits[7:4] + EXPECT_EQ(hostWords[3], (static_cast(params.operandLength) << 16) | + static_cast(kExtendedTCode)); EXPECT_EQ(static_cast((hostWords[1] >> 16) & 0xFFFFu), MakeDestinationID(context.sourceNodeID, params.destinationID)); @@ -218,12 +220,13 @@ TEST(AsyncPacketSerDesLinuxCompat, ParseReadQuadletResponseMatchesLinuxVector) { const auto buffer = std::span(packet.data(), packet.size()); const auto info = ARPacketParser::ParseNext(buffer, 0); if (!info.has_value()) { - GTEST_SKIP() << "ARPacketParser::ParseNext rejected the linux read-quadlet response fixture."; + GTEST_SKIP() + << "ARPacketParser::ParseNext rejected the linux read-quadlet response fixture."; } EXPECT_EQ(info->headerLength, 16u); EXPECT_EQ(info->dataLength, 0u); EXPECT_EQ(info->tCode, 0x6); - EXPECT_EQ(info->rCode, 0u); // RCODE_COMPLETE + EXPECT_EQ(info->rCode, 0u); // RCODE_COMPLETE EXPECT_EQ(info->totalLength, 20u); // 16-byte header + 4-byte trailer PacketRouter router; @@ -243,13 +246,19 @@ TEST(AsyncPacketSerDesLinuxCompat, ParseReadQuadletResponseMatchesLinuxVector) { TEST(AsyncPacketSerDesLinuxCompat, ParseReadBlockResponseComputesPayloadLength) { // Q3 specifies data_length = 0x20 (32 bytes), so we need to include 32 bytes of payload const auto packet = MakeARBufferFromOHCIWords({ - 0xFFC1E170u, // Q0: header - 0xFFC00000u, // Q1: source ID - 0x00000000u, // Q2: reserved - 0x00200000u, // Q3: data_length=0x20 (32 bytes) + 0xFFC1E170u, // Q0: header + 0xFFC00000u, // Q1: source ID + 0x00000000u, // Q2: reserved + 0x00200000u, // Q3: data_length=0x20 (32 bytes) // Payload: 32 bytes = 8 quadlets of dummy data - 0x00000000u, 0x00000000u, 0x00000000u, 0x00000000u, - 0x00000000u, 0x00000000u, 0x00000000u, 0x00000000u, + 0x00000000u, + 0x00000000u, + 0x00000000u, + 0x00000000u, + 0x00000000u, + 0x00000000u, + 0x00000000u, + 0x00000000u, }); const auto buffer = std::span(packet.data(), packet.size()); const auto info = ARPacketParser::ParseNext(buffer, 0); @@ -311,12 +320,12 @@ TEST(AsyncPacketSerDesLinuxCompat, ParseLockResponsePreservesExtendedTCodeLength TEST(AsyncPacketSerDesLinuxCompat, RequestPayloadIsCopiedIntoAlignedScratchBeforeHandler) { const auto packet = MakeARBufferFromOHCIWords({ - 0xFFC16510u, // Q0: tCode=0x1 (write block), tLabel arbitrary - 0xFFC0ECC0u, // Q1: src=0xFFC0, addrHi=0xECC0 - 0x00000000u, // Q2: addrLo - 0x00080000u, // Q3: data_length=8 - 0x11223344u, // payload q0 - 0x55667788u, // payload q1 + 0xFFC16510u, // Q0: tCode=0x1 (write block), tLabel arbitrary + 0xFFC0ECC0u, // Q1: src=0xFFC0, addrHi=0xECC0 + 0x00000000u, // Q2: addrLo + 0x00080000u, // Q3: data_length=8 + 0x11223344u, // payload q0 + 0x55667788u, // payload q1 }); std::vector misaligned; @@ -337,8 +346,14 @@ TEST(AsyncPacketSerDesLinuxCompat, RequestPayloadIsCopiedIntoAlignedScratchBefor if (view.payload.size() == 8u) { EXPECT_EQ((std::array{0x44, 0x33, 0x22, 0x11, 0x88, 0x77, 0x66, 0x55}), (std::array{ - view.payload[0], view.payload[1], view.payload[2], view.payload[3], - view.payload[4], view.payload[5], view.payload[6], view.payload[7], + view.payload[0], + view.payload[1], + view.payload[2], + view.payload[3], + view.payload[4], + view.payload[5], + view.payload[6], + view.payload[7], })); } return ResponseCode::Complete; @@ -353,13 +368,11 @@ TEST(AsyncPacketSerDesLinuxCompat, RequestPayloadIsCopiedIntoAlignedScratchBefor TEST(AsyncPacketSerDesLinuxCompat, ExtractTLabelUsesWireByteTwo) { // Read quadlet response packet as OHCI AR DMA memory: tLabel=48, tCode=6, rCode=0. // After the little-endian quadlet write, memory byte1 holds [tLabel:6][rt:2]. - // 16-byte header + mandatory 4-byte OHCI trailer. + // The final quadlet is the AR bufferFill trailer (xferStatus/timeStamp), + // which hardware appends to every packet (OHCI §8.4.2). const std::array responseBytes{ - 0x60, 0xC2, 0x01, 0x60, - 0x00, 0x00, 0xC0, 0xFF, - 0x00, 0x00, 0x00, 0x00, - 0x04, 0x20, 0x8F, 0xE2, - 0x00, 0x00, 0x11, 0x00, + 0x60, 0xC2, 0x01, 0x60, 0x00, 0x00, 0xC0, 0xFF, 0x00, 0x00, + 0x00, 0x00, 0x04, 0x20, 0x8F, 0xE2, 0x00, 0x00, 0x11, 0x00, }; PacketRouter router; @@ -369,7 +382,8 @@ TEST(AsyncPacketSerDesLinuxCompat, ExtractTLabelUsesWireByteTwo) { EXPECT_EQ(view.tLabel, 48u); return ResponseCode::NoResponse; }); - const auto responseBuffer = std::span(responseBytes.data(), responseBytes.size()); + const auto responseBuffer = + std::span(responseBytes.data(), responseBytes.size()); const auto info = ARPacketParser::ParseNext(responseBuffer, 0); ASSERT_TRUE(info.has_value()); router.RouteParsedPacket(ARContextType::Response, *info, /*generation=*/0); diff --git a/tests/async/BufferRingDMATests.cpp b/tests/async/BufferRingDMATests.cpp index 53ea8042..b806a8f6 100644 --- a/tests/async/BufferRingDMATests.cpp +++ b/tests/async/BufferRingDMATests.cpp @@ -434,4 +434,117 @@ TEST_F(LargeBufferRingDMATest, CopyReadableBytesReassemblesSplitReadQuadletRespo EXPECT_EQ(parsed->totalLength, packet.size()); } +// OHCI §8.4.2: AR bufferFill appends a status/timestamp trailer quadlet to +// every packet. A window holding exactly header+payload but not the trailer is +// an INCOMPLETE packet. The old parser treated the trailer as optional and +// completed such packets 4 bytes short; the late trailer then sat at the head +// of the unread stream, misaligned every subsequent parse, and permanently +// jammed the AR response path (field failure 2026-07-03: bus-wide async outage +// until reboot). +TEST(ARPacketParserTrailerTest, WindowWithoutTrailerIsIncomplete) { + const auto packet = MakeARDmaBufferFromHostWords({ + 0xFFC1F160u, // read-quadlet response, tCode=0x6 + 0xFFC00000u, + 0x00000000u, + 0x00000046u, + }); + ASSERT_EQ(packet.size(), 20u); // 16-byte header + 4-byte trailer + + // Header fully present, trailer missing: must report incomplete, not a + // 16-byte packet. + const auto withoutTrailer = ASFW::Async::ARPacketParser::ParseNext( + std::span(packet.data(), 16), 0); + EXPECT_FALSE(withoutTrailer.has_value()); + + // Full window: parses, and totalLength covers the trailer. + const auto withTrailer = ASFW::Async::ARPacketParser::ParseNext( + std::span(packet.data(), packet.size()), 0); + ASSERT_TRUE(withTrailer.has_value()); + EXPECT_EQ(withTrailer->totalLength, 20u); +} + +TEST_F(LargeBufferRingDMATest, TrailerOnlyStraddleDoesNotMisalignStream) { + // Field-failure reproduction (2026-07-03): a 104-byte read-block response's + // header+payload ended exactly at the end of buffer[0]; only its 4-byte + // trailer landed in buffer[1], followed by the next response. The stream + // must resume, aligned, at buffer[1] offset 4. + auto* current = ring_.GetDescriptor(0); + auto* next = ring_.GetDescriptor(1); + auto* currentBuffer = ring_.GetBufferAddress(0); + auto* nextBuffer = ring_.GetBufferAddress(1); + ASSERT_NE(current, nullptr); + ASSERT_NE(next, nullptr); + ASSERT_NE(currentBuffer, nullptr); + ASSERT_NE(nextBuffer, nullptr); + + // Read-block response with 104 data bytes: 16 header + 104 payload = 120, + // + 4 trailer = 124 total. + std::vector blockResponse; + auto pushLE = [&blockResponse](uint32_t word) { + blockResponse.push_back(static_cast(word & 0xFF)); + blockResponse.push_back(static_cast((word >> 8) & 0xFF)); + blockResponse.push_back(static_cast((word >> 16) & 0xFF)); + blockResponse.push_back(static_cast((word >> 24) & 0xFF)); + }; + pushLE(0xFFC1F170u); // tCode=0x7 read-block response + pushLE(0xFFC00000u); + pushLE(0x00000000u); + pushLE(0x00680000u); // data_length=104 in q3[31:16] + blockResponse.insert(blockResponse.end(), 104, 0xA5); + pushLE(0x00110000u); // trailer: xferStatus=0x0011 + ASSERT_EQ(blockResponse.size(), 124u); + + const auto nextPacket = MakeARDmaBufferFromHostWords({ + 0xFFC1F160u, 0xFFC00000u, 0x00000000u, 0x00000046u, + }); + + const size_t splitOffset = kBufferSize - 120; // header+payload end flush + std::memset(currentBuffer, 0, kBufferSize); + std::memcpy(currentBuffer + splitOffset, blockResponse.data(), 120); + std::memcpy(nextBuffer, blockResponse.data() + 120, 4); // trailer only + std::memcpy(nextBuffer + 4, nextPacket.data(), nextPacket.size()); + const size_t nextFilled = 4 + nextPacket.size(); + + ASFW::Async::HW::AR_init_status(*current, 0); + ASFW::Async::HW::AR_init_status( + *next, static_cast(kBufferSize - nextFilled)); + + auto first = ring_.Dequeue(); + ASSERT_TRUE(first.has_value()); + ASSERT_EQ(ring_.CommitConsumed(first->descriptorIndex, splitOffset), kIOReturnSuccess); + + // In-buffer parse at the packet start must report incomplete (trailer is in + // the next buffer) — this is where the old parser returned a 120-byte + // "complete" packet and orphaned the trailer. + const auto truncated = ASFW::Async::ARPacketParser::ParseNext( + std::span(currentBuffer, kBufferSize), splitOffset); + EXPECT_FALSE(truncated.has_value()); + + // Stitched retry sees the full packet across the boundary. + alignas(4) std::array stitched{}; + const size_t copied = ring_.CopyReadableBytes(stitched); + ASSERT_GE(copied, blockResponse.size()); + const auto parsed = ASFW::Async::ARPacketParser::ParseNext( + std::span(stitched.data(), copied), 0); + ASSERT_TRUE(parsed.has_value()); + EXPECT_EQ(parsed->tCode, 0x7u); + EXPECT_EQ(parsed->totalLength, blockResponse.size()); + + ASSERT_EQ(ring_.ConsumeReadableBytes(parsed->totalLength), kIOReturnSuccess); + EXPECT_EQ(ring_.Head(), 1u); + + // The stream must resume at buffer[1] offset 4 (past the consumed trailer) + // and the next packet must parse aligned. + auto visible = ring_.Dequeue(); + ASSERT_TRUE(visible.has_value()); + EXPECT_EQ(visible->descriptorIndex, 1u); + EXPECT_EQ(visible->startOffset, 4u); + const auto aligned = ASFW::Async::ARPacketParser::ParseNext( + std::span(visible->virtualAddress, visible->bytesFilled), + visible->startOffset); + ASSERT_TRUE(aligned.has_value()); + EXPECT_EQ(aligned->tCode, 0x6u); + EXPECT_EQ(aligned->totalLength, nextPacket.size()); +} + } // namespace ASFW::Testing From 466bf7002c1bdfd7eaff57dc62e99dece389e015 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 19:36:22 +0200 Subject: [PATCH 3/7] fix(dice): decode nickname little-endian (Venice not 'ineV') 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 --- .../Protocols/DICE/Core/DICETransaction.cpp | 29 +-------- .../Audio/Protocols/DICE/Core/DICETypes.hpp | 42 +++++++++++++ tests/devices/DICETcatProtocolTests.cpp | 63 +++++++++++++++++++ 3 files changed, 108 insertions(+), 26 deletions(-) diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp index 73a2fa95..c8074a68 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp @@ -145,32 +145,9 @@ void DICETransaction::ReadGlobalStateSized(const GeneralSections& sections, state.notification = ReadBE32(data + GlobalOffset::kNotification); } if (size >= 0x4C) { - // Extract nickname (64 bytes = 16 quadlets starting at offset 0x0C) - // DICE stores strings as big-endian quadlets, so we need to read each - // 4-byte group as a quadlet and extract chars in big-endian order - size_t nickIdx = 0; - for (size_t q = 0; q < 16 && nickIdx < 63; ++q) { - size_t qOffset = 0x0C + q * 4; - if (qOffset + 4 > size) break; - - uint32_t quadlet = ReadBE32(data + qOffset); - - // Extract chars from quadlet (MSB first = big-endian string order) - char c0 = (quadlet >> 24) & 0xFF; - char c1 = (quadlet >> 16) & 0xFF; - char c2 = (quadlet >> 8) & 0xFF; - char c3 = quadlet & 0xFF; - - if (c0 == '\0') break; - state.nickname[nickIdx++] = c0; - if (c1 == '\0') break; - state.nickname[nickIdx++] = c1; - if (c2 == '\0') break; - state.nickname[nickIdx++] = c2; - if (c3 == '\0') break; - state.nickname[nickIdx++] = c3; - } - state.nickname[nickIdx] = '\0'; + // Nickname is 64 bytes / 16 quadlets stored little-endian within each + // wire quadlet; DecodeDiceNickname handles the byte order. + DecodeDiceNickname(data, size, state.nickname); } if (size >= 0x50) { state.clockSelect = ReadBE32(data + GlobalOffset::kClockSelect); diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICETypes.hpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICETypes.hpp index 02c10e18..5467325e 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICETypes.hpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICETypes.hpp @@ -500,6 +500,48 @@ struct GlobalState { const char* SupportedRatesDescription() const; }; +/// Decode the DICE global-section nickname (64 bytes / 16 quadlets at +/// GlobalOffset::kNickname) from a raw big-endian wire payload into `out`. +/// +/// DICE stores strings little-endian: the first character of each quadlet lives +/// in the LEAST-significant byte. The wire transmits quadlets big-endian, so the +/// characters within a quadlet must be emitted LSB-first, otherwise "Veni" +/// decodes as the byte-reversed "ineV". +/// cross-validated with FFADO dice_avdevice.cpp:696 +/// ("Strings from the device are always little-endian"). +inline void DecodeDiceNickname(const uint8_t* data, size_t size, char (&out)[64]) { + size_t nickIdx = 0; + for (size_t q = 0; q < 16 && nickIdx < 63; ++q) { + const size_t qOffset = static_cast(GlobalOffset::kNickname) + q * 4; + if (qOffset + 4 > size) { + break; + } + // Big-endian wire quadlet, then read characters LSB-first. + const uint32_t quadlet = (static_cast(data[qOffset]) << 24) | + (static_cast(data[qOffset + 1]) << 16) | + (static_cast(data[qOffset + 2]) << 8) | + static_cast(data[qOffset + 3]); + const char chars[4] = { + static_cast(quadlet & 0xFF), + static_cast((quadlet >> 8) & 0xFF), + static_cast((quadlet >> 16) & 0xFF), + static_cast((quadlet >> 24) & 0xFF), + }; + bool done = false; + for (char c : chars) { + if (c == '\0' || nickIdx >= 63) { + done = true; + break; + } + out[nickIdx++] = c; + } + if (done) { + break; + } + } + out[nickIdx] = '\0'; +} + // ============================================================================ // TX/RX Stream Format // ============================================================================ diff --git a/tests/devices/DICETcatProtocolTests.cpp b/tests/devices/DICETcatProtocolTests.cpp index b14db289..1e08cd69 100644 --- a/tests/devices/DICETcatProtocolTests.cpp +++ b/tests/devices/DICETcatProtocolTests.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include namespace ASFW::Audio::DICE::TCAT { @@ -35,6 +36,7 @@ using ASFW::Async::FWAddress; using ASFW::Async::IFireWireBus; using ASFW::Audio::AudioStreamRuntimeCaps; using ASFW::Audio::DICE::ClockSource; +using ASFW::Audio::DICE::DecodeDiceNickname; using ASFW::Audio::DICE::ExtensionSections; using ASFW::Audio::DICE::Focusrite::EffectGeneralParams; using ASFW::Audio::DICE::GeneralSections; @@ -346,4 +348,65 @@ TEST(SPro24DspProtocolTests, VendorCallLoadsExtensionsLazily) { EXPECT_EQ(bus.appQuadReadCount, 1); } +// --------------------------------------------------------------------------- +// DICE nickname decode (little-endian within each big-endian wire quadlet). +// cross-validated with FFADO dice_avdevice.cpp:696. +// --------------------------------------------------------------------------- + +// Encode a string the way a DICE device stores it: the first character of each +// quadlet sits in the least-significant byte, and the quadlet is transmitted +// big-endian on the wire — so the wire bytes are the characters reversed within +// each 4-byte group. `out` is the global-section payload; nickname starts at +// GlobalOffset::kNickname (0x0C). +std::vector MakeNicknamePayload(const std::string& name) { + std::vector payload(0x0C + 64, 0); + for (size_t q = 0; q * 4 < name.size() && q < 16; ++q) { + uint8_t chars[4] = {0, 0, 0, 0}; + for (size_t b = 0; b < 4; ++b) { + const size_t idx = q * 4 + b; + chars[b] = (idx < name.size()) ? static_cast(name[idx]) : 0; + } + const size_t base = 0x0C + q * 4; + payload[base + 0] = chars[3]; // MSB on the wire = last char of group + payload[base + 1] = chars[2]; + payload[base + 2] = chars[1]; + payload[base + 3] = chars[0]; // LSB on the wire = first char of group + } + return payload; +} + +TEST(DiceNicknameTests, DecodesLittleEndianStringNotByteReversed) { + // The Midas Venice regression: "Veni" must not decode as "ineV". + const auto payload = MakeNicknamePayload("Venice F32"); + char out[64]{}; + DecodeDiceNickname(payload.data(), payload.size(), out); + EXPECT_STREQ(out, "Venice F32"); +} + +TEST(DiceNicknameTests, ShortNameWithinFirstQuadletTerminates) { + const auto payload = MakeNicknamePayload("Hi"); + char out[64]{}; + DecodeDiceNickname(payload.data(), payload.size(), out); + EXPECT_STREQ(out, "Hi"); +} + +TEST(DiceNicknameTests, StopsAtPayloadBoundaryWithoutOverrun) { + // Only one full quadlet of nickname present after the 0x0C offset. + std::vector payload(0x0C + 4, 0); + payload[0x0C + 0] = 'i'; // wire bytes for "Veni" -> first 4 chars only + payload[0x0C + 1] = 'n'; + payload[0x0C + 2] = 'e'; + payload[0x0C + 3] = 'V'; + char out[64]{}; + DecodeDiceNickname(payload.data(), payload.size(), out); + EXPECT_STREQ(out, "Veni"); +} + +TEST(DiceNicknameTests, EmptyNicknameYieldsEmptyString) { + const std::vector payload(0x0C + 64, 0); + char out[64]{}; + DecodeDiceNickname(payload.data(), payload.size(), out); + EXPECT_STREQ(out, ""); +} + } // namespace From 13beaaebfa1a0bc87660bafb36ee3b32264bfaba Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 19:46:07 +0200 Subject: [PATCH 4/7] fix(dice): decode channel labels little-endian + add SplitDiceLabels 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 --- .../Protocols/DICE/Core/DICETransaction.cpp | 46 ++++++++++++++++--- .../Protocols/DICE/Core/DICETransaction.hpp | 10 ++++ tests/devices/DICETcatProtocolTests.cpp | 45 ++++++++++++++++++ 3 files changed, 94 insertions(+), 7 deletions(-) diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp index c8074a68..8687b1ed 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp @@ -6,10 +6,12 @@ #include "DICETransaction.hpp" #include "../../../../Common/CallbackUtils.hpp" #include "../../../../Logging/Logging.hpp" +#include #include #include #include #include +#include namespace ASFW::Audio::DICE { @@ -217,15 +219,19 @@ void CopyLabelBlob(char (&dst)[256], const uint8_t* src, size_t bytesAvailable) const size_t copyBytes = (bytesAvailable < (sizeof(dst) - 1)) ? bytesAvailable : (sizeof(dst) - 1); size_t out = 0; - // DICE stores text fields as big-endian quadlets. Decode quadlet-wise into - // host-order bytes instead of using memcpy; this avoids alignment-sensitive - // vectorized memmove paths on DriverKit RX payload buffers. + // DICE stores text fields little-endian: the first character of each quadlet + // lives in the least-significant byte. The wire transmits quadlets big-endian, + // so characters must be emitted LSB-first (same byte order as the nickname; + // emitting MSB-first byte-reverses each quadlet). Decoding quadlet-wise also + // avoids alignment-sensitive vectorized memmove paths on DriverKit RX buffers. + // cross-validated with FFADO dice_avdevice.cpp:1527,1547 + // ("Strings from the device are always little-endian"). while (out + 4 <= copyBytes) { const uint32_t q = ReadBE32(src + out); - dst[out + 0] = static_cast((q >> 24) & 0xFF); - dst[out + 1] = static_cast((q >> 16) & 0xFF); - dst[out + 2] = static_cast((q >> 8) & 0xFF); - dst[out + 3] = static_cast( q & 0xFF); + dst[out + 0] = static_cast( q & 0xFF); + dst[out + 1] = static_cast((q >> 8) & 0xFF); + dst[out + 2] = static_cast((q >> 16) & 0xFF); + dst[out + 3] = static_cast((q >> 24) & 0xFF); out += 4; } @@ -353,6 +359,32 @@ void LogStreamConfigDetails(const char* prefix, const StreamConfig& config) { } } // anonymous namespace +std::vector SplitDiceLabels(const char* labels) { + std::vector names; + if (!labels) { + return names; + } + // CopyLabelBlob NUL-terminates the decoded blob. + std::string in(labels); + + // The device terminates the channel-name list with a double backslash; + // anything past it is padding. (FFADO splitNameString.) + if (const auto term = in.find("\\\\"); term != std::string::npos) { + in.resize(term); + } + + // Split on single-backslash separators, preserving empty tokens so the + // result index stays aligned with the channel index (FFADO splitString). + const std::string delim = "\\"; + size_t start = 0; + while (start < in.size()) { + const size_t end = std::min(in.size(), in.find(delim, start)); + names.push_back(in.substr(start, end - start)); + start = end + delim.size(); + } + return names; +} + void DICETransaction::ReadRxStreamConfig(const GeneralSections& sections, std::function callback) { auto callbackState = Common::ShareCallback(std::move(callback)); diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.hpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.hpp index 82ba7193..e0bceb66 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.hpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.hpp @@ -13,9 +13,19 @@ #include #include #include +#include +#include namespace ASFW::Audio::DICE { +/// Split a decoded DICE channel-name blob (e.g. StreamFormatEntry::labels) into +/// per-channel names. The device lists names separated by a single backslash and +/// terminates the list with a double backslash. Empty tokens between separators +/// are preserved so the returned index aligns with the channel index. +/// Mirrors FFADO Device::splitNameString. +/// cross-validated with FFADO dice_avdevice.cpp (splitNameString) + ffadotypes.h. +std::vector SplitDiceLabels(const char* labels); + /// Maximum frame size for a single DICE transaction (512 bytes per spec) constexpr size_t kMaxFrameSize = 512; diff --git a/tests/devices/DICETcatProtocolTests.cpp b/tests/devices/DICETcatProtocolTests.cpp index 1e08cd69..ffbbc646 100644 --- a/tests/devices/DICETcatProtocolTests.cpp +++ b/tests/devices/DICETcatProtocolTests.cpp @@ -4,6 +4,7 @@ #include "Async/Interfaces/IFireWireBus.hpp" #include "Common/WireFormat.hpp" #include "Audio/Protocols/DICE/Core/DICETypes.hpp" +#include "Audio/Protocols/DICE/Core/DICETransaction.hpp" #include "Audio/Protocols/DICE/Focusrite/SPro24DspProtocol.hpp" #include "Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp" @@ -37,6 +38,7 @@ using ASFW::Async::IFireWireBus; using ASFW::Audio::AudioStreamRuntimeCaps; using ASFW::Audio::DICE::ClockSource; using ASFW::Audio::DICE::DecodeDiceNickname; +using ASFW::Audio::DICE::SplitDiceLabels; using ASFW::Audio::DICE::ExtensionSections; using ASFW::Audio::DICE::Focusrite::EffectGeneralParams; using ASFW::Audio::DICE::GeneralSections; @@ -409,4 +411,47 @@ TEST(DiceNicknameTests, EmptyNicknameYieldsEmptyString) { EXPECT_STREQ(out, ""); } +// --------------------------------------------------------------------------- +// DICE channel-label splitting (FFADO splitNameString). +// --------------------------------------------------------------------------- + +TEST(DiceLabelTests, SplitsSingleBackslashSeparatedNames) { + const auto names = SplitDiceLabels("Mic 1\\Mic 2\\Line 3\\\\"); + ASSERT_EQ(names.size(), 3u); + EXPECT_EQ(names[0], "Mic 1"); + EXPECT_EQ(names[1], "Mic 2"); + EXPECT_EQ(names[2], "Line 3"); +} + +TEST(DiceLabelTests, StopsAtDoubleBackslashTerminator) { + // Padding after the "\\\\" terminator must be ignored. + const auto names = SplitDiceLabels("A\\B\\\\garbage\\more"); + ASSERT_EQ(names.size(), 2u); + EXPECT_EQ(names[0], "A"); + EXPECT_EQ(names[1], "B"); +} + +TEST(DiceLabelTests, PreservesLeadingEmptyTokenForChannelAlignment) { + // A leading separator yields an empty first token (channel 0 unnamed). + // (Two consecutive separators would form the "\\\\" terminator, so an + // interior empty token cannot occur.) + const auto names = SplitDiceLabels("\\A\\B\\\\"); + ASSERT_EQ(names.size(), 3u); + EXPECT_EQ(names[0], ""); + EXPECT_EQ(names[1], "A"); + EXPECT_EQ(names[2], "B"); +} + +TEST(DiceLabelTests, NullAndEmptyYieldNoNames) { + EXPECT_TRUE(SplitDiceLabels(nullptr).empty()); + EXPECT_TRUE(SplitDiceLabels("").empty()); + EXPECT_TRUE(SplitDiceLabels("\\\\").empty()); +} + +TEST(DiceLabelTests, SingleNameWithoutTerminator) { + const auto names = SplitDiceLabels("Solo"); + ASSERT_EQ(names.size(), 1u); + EXPECT_EQ(names[0], "Solo"); +} + } // namespace From bfae654d8ea2884d3412a89688153f88577c1c71 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 20:04:57 +0200 Subject: [PATCH 5/7] feat(audio): carry per-channel device labels to CoreAudio element names 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 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 " 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 --- .../Audio/DriverKit/ASFWAudioDriverGraph.cpp | 23 ++---- .../DriverKit/ASFWAudioDriverPrivate.hpp | 4 +- .../DriverKit/Config/AudioDriverConfig.cpp | 77 ++++++++++++++----- .../DriverKit/Config/AudioDriverConfig.hpp | 17 +++- ASFWDriver/Audio/Model/ASFWAudioDevice.hpp | 31 ++++++++ ASFWDriver/Audio/Model/AudioPropertyKeys.hpp | 5 ++ 6 files changed, 118 insertions(+), 39 deletions(-) diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp index 19b78193..5a1555c3 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverGraph.cpp @@ -158,21 +158,10 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, } parsedConfig.channelCount = std::max(parsedConfig.inputChannelCount, parsedConfig.outputChannelCount); - // Regenerate channel names for the updated channel counts - for (uint32_t index = 0; index < parsedConfig.inputChannelCount && index < ASFW::Isoch::Audio::kMaxNamedChannels; ++index) { - snprintf(parsedConfig.inputChannelNames[index], - sizeof(parsedConfig.inputChannelNames[index]), - "%s %u", - parsedConfig.inputPlugName, - index + 1); - } - for (uint32_t index = 0; index < parsedConfig.outputChannelCount && index < ASFW::Isoch::Audio::kMaxNamedChannels; ++index) { - snprintf(parsedConfig.outputChannelNames[index], - sizeof(parsedConfig.outputChannelNames[index]), - "%s %u", - parsedConfig.outputPlugName, - index + 1); - } + // Regenerate channel names for the updated channel counts. Prefers the + // device's per-channel labels (published by the core side) and falls + // back to synthesized " N" for any slot without a real label. + ASFW::Isoch::Audio::BuildChannelNamesFromPlugs(parsedConfig); } ASFW::Isoch::Audio::BuildFallbackBoolControls(parsedConfig); @@ -544,7 +533,7 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, } ASFW_LOG(Audio, "ASFWAudioDriver: IO operation handler installed"); - for (uint32_t ch = 1; ch <= ivars.device.outputChannelCount && ch <= 8; ch++) { + for (uint32_t ch = 1; ch <= ivars.device.outputChannelCount && ch <= ASFW::Isoch::Audio::kMaxNamedChannels; ch++) { auto outChName = OSSharedPtr(OSString::withCString(ivars.device.outputChannelNames[ch - 1]), OSNoRetain); if (outChName) { const kern_return_t status = @@ -561,7 +550,7 @@ kern_return_t BuildAudioGraph(ASFWAudioDriver& driver, } } } - for (uint32_t ch = 1; ch <= ivars.device.inputChannelCount && ch <= 8; ch++) { + for (uint32_t ch = 1; ch <= ivars.device.inputChannelCount && ch <= ASFW::Isoch::Audio::kMaxNamedChannels; ch++) { auto inChName = OSSharedPtr(OSString::withCString(ivars.device.inputChannelNames[ch - 1]), OSNoRetain); if (inChName) { const kern_return_t status = diff --git a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverPrivate.hpp b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverPrivate.hpp index 8b6260ad..e792cf80 100644 --- a/ASFWDriver/Audio/DriverKit/ASFWAudioDriverPrivate.hpp +++ b/ASFWDriver/Audio/DriverKit/ASFWAudioDriverPrivate.hpp @@ -49,8 +49,8 @@ struct AudioDriverDeviceState { char inputPlugName[64]{}; char outputPlugName[64]{}; - char inputChannelNames[8][64]{}; - char outputChannelNames[8][64]{}; + char inputChannelNames[ASFW::Isoch::Audio::kMaxNamedChannels][64]{}; + char outputChannelNames[ASFW::Isoch::Audio::kMaxNamedChannels][64]{}; }; class DextTxExecutionTimeline final { diff --git a/ASFWDriver/Audio/DriverKit/Config/AudioDriverConfig.cpp b/ASFWDriver/Audio/DriverKit/Config/AudioDriverConfig.cpp index 3ae39144..8ce1d0b4 100644 --- a/ASFWDriver/Audio/DriverKit/Config/AudioDriverConfig.cpp +++ b/ASFWDriver/Audio/DriverKit/Config/AudioDriverConfig.cpp @@ -33,25 +33,6 @@ void AppendBoolControl(ParsedAudioDriverConfig& inOutConfig, inOutConfig.boolControls[inOutConfig.boolControlCount++] = descriptor; } -void BuildChannelNamesFromPlugs(ParsedAudioDriverConfig& inOutConfig) { - const uint32_t maxInputChannels = std::min(inOutConfig.inputChannelCount, kMaxNamedChannels); - const uint32_t maxOutputChannels = std::min(inOutConfig.outputChannelCount, kMaxNamedChannels); - for (uint32_t index = 0; index < maxInputChannels; ++index) { - snprintf(inOutConfig.inputChannelNames[index], - sizeof(inOutConfig.inputChannelNames[index]), - "%s %u", - inOutConfig.inputPlugName, - index + 1); - } - for (uint32_t index = 0; index < maxOutputChannels; ++index) { - snprintf(inOutConfig.outputChannelNames[index], - sizeof(inOutConfig.outputChannelNames[index]), - "%s %u", - inOutConfig.outputPlugName, - index + 1); - } -} - void ParseIdentityProperties(OSDictionary* properties, ParsedAudioDriverConfig& inOutConfig) { if (auto* guid = OSDynamicCast(OSNumber, properties->getObject(Keys::kGuid))) { inOutConfig.guid = guid->unsigned64BitValue(); @@ -127,6 +108,29 @@ void ParsePlugNames(OSDictionary* properties, ParsedAudioDriverConfig& inOutConf } } +// Read an optional OSArray of OSString device labels into a fixed [N][64] array. +// Indices align with the channel index; missing/empty entries stay empty so +// BuildChannelNamesFromPlugs synthesizes a name for that slot. +void ParseChannelNameArray(OSDictionary* properties, + const char* key, + char (*dst)[64]) { + auto* array = OSDynamicCast(OSArray, properties->getObject(key)); + if (array == nullptr) { + return; + } + const uint32_t count = std::min(array->getCount(), kMaxNamedChannels); + for (uint32_t i = 0; i < count; ++i) { + if (auto* name = OSDynamicCast(OSString, array->getObject(i))) { + strlcpy(dst[i], name->getCStringNoCopy(), 64); + } + } +} + +void ParseChannelNames(OSDictionary* properties, ParsedAudioDriverConfig& inOutConfig) { + ParseChannelNameArray(properties, Keys::kInputChannelNames, inOutConfig.deviceInputChannelNames); + ParseChannelNameArray(properties, Keys::kOutputChannelNames, inOutConfig.deviceOutputChannelNames); +} + void ParseBoolControlOverrides(OSDictionary* properties, ParsedAudioDriverConfig& inOutConfig) { auto* overrideArray = OSDynamicCast(OSArray, properties->getObject(Keys::kBoolControlOverrides)); if (overrideArray == nullptr) { @@ -159,6 +163,40 @@ void ParseBoolControlOverrides(OSDictionary* properties, ParsedAudioDriverConfig } // namespace +// Fill each element name, preferring a per-channel device label when present +// and falling back to the synthesized " N". Centralizes the rule so both +// the initial parse and the post-profile regeneration in BuildAudioGraph agree. +void BuildChannelNamesFromPlugs(ParsedAudioDriverConfig& inOutConfig) { + const uint32_t maxInputChannels = std::min(inOutConfig.inputChannelCount, kMaxNamedChannels); + const uint32_t maxOutputChannels = std::min(inOutConfig.outputChannelCount, kMaxNamedChannels); + for (uint32_t index = 0; index < maxInputChannels; ++index) { + if (inOutConfig.deviceInputChannelNames[index][0] != '\0') { + strlcpy(inOutConfig.inputChannelNames[index], + inOutConfig.deviceInputChannelNames[index], + sizeof(inOutConfig.inputChannelNames[index])); + continue; + } + snprintf(inOutConfig.inputChannelNames[index], + sizeof(inOutConfig.inputChannelNames[index]), + "%s %u", + inOutConfig.inputPlugName, + index + 1); + } + for (uint32_t index = 0; index < maxOutputChannels; ++index) { + if (inOutConfig.deviceOutputChannelNames[index][0] != '\0') { + strlcpy(inOutConfig.outputChannelNames[index], + inOutConfig.deviceOutputChannelNames[index], + sizeof(inOutConfig.outputChannelNames[index])); + continue; + } + snprintf(inOutConfig.outputChannelNames[index], + sizeof(inOutConfig.outputChannelNames[index]), + "%s %u", + inOutConfig.outputPlugName, + index + 1); + } +} + void ParseAudioDriverConfigFromProperties(OSDictionary* properties, ParsedAudioDriverConfig& inOutConfig) { if (!properties) { @@ -170,6 +208,7 @@ void ParseAudioDriverConfigFromProperties(OSDictionary* properties, ParseDevicePresentationProperties(properties, inOutConfig); ParseSampleRates(properties, inOutConfig); ParsePlugNames(properties, inOutConfig); + ParseChannelNames(properties, inOutConfig); ParseBoolControlOverrides(properties, inOutConfig); BuildChannelNamesFromPlugs(inOutConfig); } diff --git a/ASFWDriver/Audio/DriverKit/Config/AudioDriverConfig.hpp b/ASFWDriver/Audio/DriverKit/Config/AudioDriverConfig.hpp index 382b810a..6f13d3df 100644 --- a/ASFWDriver/Audio/DriverKit/Config/AudioDriverConfig.hpp +++ b/ASFWDriver/Audio/DriverKit/Config/AudioDriverConfig.hpp @@ -14,7 +14,10 @@ namespace ASFW::Isoch::Audio { constexpr double kDefaultSampleRate = 48000.0; constexpr uint32_t kDefaultChannelCount = 2; constexpr uint32_t kMaxSampleRates = 8; -constexpr uint32_t kMaxNamedChannels = 8; +// Covers high-channel-count interfaces (e.g. Midas Venice F32 = 32x32 duplex) +// so per-channel device labels can be carried for every element, not just the +// first 8. Each name is at most 64 bytes (see ParsedAudioDriverConfig). +constexpr uint32_t kMaxNamedChannels = 32; constexpr uint32_t kMaxBoolControls = 16; constexpr uint32_t kClassIdPhantomPower = static_cast('phan'); @@ -61,6 +64,12 @@ struct ParsedAudioDriverConfig { char outputPlugName[64]{}; char inputChannelNames[kMaxNamedChannels][64]{}; char outputChannelNames[kMaxNamedChannels][64]{}; + + // Per-channel device labels published by the core side (empty string = + // none for that slot). BuildChannelNamesFromPlugs prefers these over the + // synthesized " N" names so CoreAudio shows the device's real labels. + char deviceInputChannelNames[kMaxNamedChannels][64]{}; + char deviceOutputChannelNames[kMaxNamedChannels][64]{}; }; void InitializeAudioDriverConfigDefaults(ParsedAudioDriverConfig& outConfig); @@ -68,6 +77,12 @@ void InitializeAudioDriverConfigDefaults(ParsedAudioDriverConfig& outConfig); void ParseAudioDriverConfigFromProperties(OSDictionary* properties, ParsedAudioDriverConfig& inOutConfig); +// Fill inputChannelNames/outputChannelNames for the current channel counts, +// preferring per-channel device labels (deviceInput/OutputChannelNames) and +// falling back to synthesized " N". Idempotent; safe to re-run after the +// channel counts change. +void BuildChannelNamesFromPlugs(ParsedAudioDriverConfig& inOutConfig); + void BuildFallbackBoolControls(ParsedAudioDriverConfig& inOutConfig); void ApplyBringupSingleFormatPolicy(ParsedAudioDriverConfig& inOutConfig); diff --git a/ASFWDriver/Audio/Model/ASFWAudioDevice.hpp b/ASFWDriver/Audio/Model/ASFWAudioDevice.hpp index 1b670178..a04ec34e 100644 --- a/ASFWDriver/Audio/Model/ASFWAudioDevice.hpp +++ b/ASFWDriver/Audio/Model/ASFWAudioDevice.hpp @@ -46,6 +46,9 @@ struct ASFWAudioDevice { uint32_t currentSampleRate{48000}; std::string inputPlugName{"Input"}; std::string outputPlugName{"Output"}; + // Per-channel device labels in channel order (empty = synthesize names). + std::vector inputChannelNames{}; + std::vector outputChannelNames{}; StreamMode streamMode{StreamMode::kNonBlocking}; bool hasPhantomOverride{false}; uint32_t phantomSupportedMask{0}; @@ -73,8 +76,36 @@ struct ASFWAudioDevice { properties->setObject(PropertyKeys::kVendorId, vendorIdNum.get()); properties->setObject(PropertyKeys::kModelId, modelIdNum.get()); + // Per-channel device labels (optional). The audio side reads these in + // channel order and prefers them over synthesized names. + PublishChannelNames(properties, PropertyKeys::kInputChannelNames, inputChannelNames); + PublishChannelNames(properties, PropertyKeys::kOutputChannelNames, outputChannelNames); + return true; } + +private: + static void PublishChannelNames(OSDictionary* properties, + const char* key, + const std::vector& names) { + if (names.empty()) { + return; + } + auto array = OSSharedPtr(OSArray::withCapacity( + static_cast(names.size())), OSNoRetain); + if (!array) { + return; + } + for (const auto& name : names) { + auto str = OSSharedPtr(OSString::withCString(name.c_str()), OSNoRetain); + // Keep the index aligned with the channel index even for empty + // labels; the audio side falls back to a synthesized name per slot. + if (str) { + array->setObject(str.get()); + } + } + properties->setObject(key, array.get()); + } }; } // namespace ASFW::Audio::Model diff --git a/ASFWDriver/Audio/Model/AudioPropertyKeys.hpp b/ASFWDriver/Audio/Model/AudioPropertyKeys.hpp index 85adc666..f8a68226 100644 --- a/ASFWDriver/Audio/Model/AudioPropertyKeys.hpp +++ b/ASFWDriver/Audio/Model/AudioPropertyKeys.hpp @@ -15,6 +15,11 @@ inline constexpr const char* kInputChannelCount = "ASFWInputChannelCount"; inline constexpr const char* kOutputChannelCount = "ASFWOutputChannelCount"; inline constexpr const char* kInputPlugName = "ASFWInputPlugName"; inline constexpr const char* kOutputPlugName = "ASFWOutputPlugName"; +// Optional per-channel device labels (OSArray of OSString, in channel order). +// When present they override the synthesized " N" names; missing/empty +// entries fall back to the synthesized name. +inline constexpr const char* kInputChannelNames = "ASFWInputChannelNames"; +inline constexpr const char* kOutputChannelNames = "ASFWOutputChannelNames"; inline constexpr const char* kCurrentSampleRate = "ASFWCurrentSampleRate"; inline constexpr const char* kStreamMode = "ASFWStreamMode"; inline constexpr const char* kHasPhantomOverride = "ASFWHasPhantomOverride"; From a4a5ad55bf519aa89c910a49f8a95f3c1c4e98c9 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Sat, 27 Jun 2026 20:12:35 +0200 Subject: [PATCH 6/7] feat(dice): surface real per-channel names from TX/RX name sections 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 " 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 --- .../Protocols/Backends/DiceAudioBackend.cpp | 48 +++++++++++++++-- .../Protocols/DICE/TCAT/DICETcatProtocol.cpp | 52 +++++++++++++++++++ .../Protocols/DICE/TCAT/DICETcatProtocol.hpp | 13 +++++ .../Audio/Protocols/IDeviceProtocol.hpp | 15 ++++++ tests/devices/DICETcatProtocolTests.cpp | 37 +++++++++++++ 5 files changed, 161 insertions(+), 4 deletions(-) diff --git a/ASFWDriver/Audio/Protocols/Backends/DiceAudioBackend.cpp b/ASFWDriver/Audio/Protocols/Backends/DiceAudioBackend.cpp index d7d1289a..e02ec2aa 100644 --- a/ASFWDriver/Audio/Protocols/Backends/DiceAudioBackend.cpp +++ b/ASFWDriver/Audio/Protocols/Backends/DiceAudioBackend.cpp @@ -7,6 +7,8 @@ #include "../../../Audio/Core/AudioRuntimeRegistry.hpp" #include "../../../Logging/Logging.hpp" #include "../DICE/Core/DICENotificationMailbox.hpp" +#include "../DICE/Core/IDICEDuplexProtocol.hpp" +#include "../IDeviceProtocol.hpp" #include "../DeviceProtocolFactory.hpp" #include "../../DriverKit/Config/DICE/DiceProfileRegistry.hpp" @@ -431,11 +433,49 @@ void DiceAudioBackend::EnsureNubForGuid(uint64_t guid) noexcept { dev.inputPlugName = "Input"; dev.outputPlugName = "Output"; - if (auto endpoint = runtime_.EnsureEndpointRuntime(guid)) { - endpoint->UpdateConfig(dev); - } + // Enrich with the device's real per-channel labels (if the protocol has + // loaded them), update the endpoint runtime, then publish the nub. Host + // input == device TX, host output == device RX (see AudioTypes.hpp), which + // is exactly how GetChannelLabels reports them. + auto finish = [this, guid](Model::ASFWAudioDevice dev, + const std::shared_ptr& protocol) { + if (stopping_.load(std::memory_order_acquire)) { + return; + } + if (protocol) { + std::vector inNames; + std::vector outNames; + if (protocol->GetChannelLabels(inNames, outNames)) { + if (!inNames.empty()) { + dev.inputChannelNames = std::move(inNames); + } + if (!outNames.empty()) { + dev.outputChannelNames = std::move(outNames); + } + ASFW_LOG(Audio, + "DiceAudioBackend::EnsureNubForGuid: applied device channel labels in=%zu out=%zu (GUID=0x%016llx)", + dev.inputChannelNames.size(), dev.outputChannelNames.size(), guid); + } + } + if (auto endpoint = runtime_.EnsureEndpointRuntime(guid)) { + endpoint->UpdateConfig(dev); + } + (void)publisher_.EnsureNub(guid, dev, "DICE"); + }; - (void)publisher_.EnsureNub(guid, dev, "DICE"); + // Channel labels live in the TCAT stream-format name sections, cached only + // once runtime caps load (during the first stream discovery). Load them + // once before the first publish so CoreAudio shows the real names from the + // start. The load early-returns if caps are already cached; publish happens + // regardless of outcome (names fall back to synthesized " N"). + if (auto* dice = protocol ? protocol->AsDiceDuplexProtocol() : nullptr) { + dice->EnsureRuntimeStreamGeometry( + [finish, dev, protocol](IOReturn /*status*/) mutable { + finish(std::move(dev), protocol); + }); + return; + } + finish(std::move(dev), protocol); } IOReturn DiceAudioBackend::StartStreaming(uint64_t guid) noexcept { diff --git a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp index 91970eca..63464632 100644 --- a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.cpp @@ -429,9 +429,55 @@ void DICETcatProtocol::CacheRuntimeCaps(const GlobalState& global, fillPerStream(tx, caps.deviceToHostStreamCount, caps.deviceToHostStreams); fillPerStream(rx, caps.hostToDeviceStreamCount, caps.hostToDeviceStreams); + // Per-channel device labels from the DICE TX/RX name sections, flattened + // across streams in channel order. Written BEFORE CacheRuntimeCaps(caps)'s + // release-store so GetChannelLabels readers see a consistent snapshot. + // Host input == device TX, host output == device RX (AudioTypes.hpp). + auto fillLabels = [](const StreamConfig& sc, + std::atomic& outCount, + char (&outLabels)[kMaxChannelLabels][64]) noexcept { + uint32_t idx = 0; + const uint32_t streams = (sc.numStreams < kMaxAudioStreamsPerDirection) + ? sc.numStreams + : kMaxAudioStreamsPerDirection; + for (uint32_t s = 0; s < streams && idx < kMaxChannelLabels; ++s) { + for (const auto& name : SplitDiceLabels(sc.streams[s].labels)) { + if (idx >= kMaxChannelLabels) { + break; + } + strlcpy(outLabels[idx], name.c_str(), sizeof(outLabels[idx])); + ++idx; + } + } + for (uint32_t z = idx; z < kMaxChannelLabels; ++z) { + outLabels[z][0] = '\0'; + } + outCount.store(idx, std::memory_order_relaxed); + }; + fillLabels(tx, inputChannelLabelCount_, inputChannelLabels_); + fillLabels(rx, outputChannelLabelCount_, outputChannelLabels_); + CacheRuntimeCaps(caps); } +bool DICETcatProtocol::GetChannelLabels(std::vector& inNames, + std::vector& outNames) const { + if (!runtimeCapsValid_.load(std::memory_order_acquire)) { + return false; + } + const uint32_t inCount = inputChannelLabelCount_.load(std::memory_order_relaxed); + const uint32_t outCount = outputChannelLabelCount_.load(std::memory_order_relaxed); + inNames.clear(); + outNames.clear(); + for (uint32_t i = 0; i < inCount && i < kMaxChannelLabels; ++i) { + inNames.emplace_back(inputChannelLabels_[i]); + } + for (uint32_t i = 0; i < outCount && i < kMaxChannelLabels; ++i) { + outNames.emplace_back(outputChannelLabels_[i]); + } + return inCount > 0 || outCount > 0; +} + void DICETcatProtocol::CacheRuntimeCaps(const AudioStreamRuntimeCaps& caps) noexcept { hostInputPcmChannels_.store(caps.hostInputPcmChannels, std::memory_order_relaxed); deviceToHostAm824Slots_.store(caps.deviceToHostAm824Slots, std::memory_order_relaxed); @@ -470,6 +516,12 @@ void DICETcatProtocol::ResetRuntimeCaps() noexcept { deviceToHostStreams_[i] = AudioStreamWireInfo{}; hostToDeviceStreams_[i] = AudioStreamWireInfo{}; } + inputChannelLabelCount_.store(0, std::memory_order_relaxed); + outputChannelLabelCount_.store(0, std::memory_order_relaxed); + for (uint32_t i = 0; i < kMaxChannelLabels; ++i) { + inputChannelLabels_[i][0] = '\0'; + outputChannelLabels_[i][0] = '\0'; + } } } // namespace ASFW::Audio::DICE::TCAT diff --git a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp index a6915792..85c30637 100644 --- a/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp +++ b/ASFWDriver/Audio/Protocols/DICE/TCAT/DICETcatProtocol.hpp @@ -44,6 +44,8 @@ class DICETcatProtocol final : public Audio::IDeviceProtocol, const Audio::DICE::IDICEDuplexProtocol* AsDiceDuplexProtocol() const noexcept override { return this; } bool GetRuntimeAudioStreamCaps(AudioStreamRuntimeCaps& outCaps) const override; + bool GetChannelLabels(std::vector& inNames, + std::vector& outNames) const override; void PrepareDuplex(const AudioDuplexChannels& channels, const DiceDesiredClockConfig& desiredClock, @@ -107,6 +109,17 @@ class DICETcatProtocol final : public Audio::IDeviceProtocol, AudioStreamWireInfo deviceToHostStreams_[kMaxAudioStreamsPerDirection]{}; AudioStreamWireInfo hostToDeviceStreams_[kMaxAudioStreamsPerDirection]{}; + // Per-channel device labels, flattened across this direction's streams in + // channel order (input == device TX, output == device RX). Published + // through the runtimeCapsValid_ release/acquire fence like the arrays above; + // only the (global, tx, rx) cache path fills them (the caps-only overload + // leaves them intact). Covers the widest supported interface (32x32). + static constexpr uint32_t kMaxChannelLabels = 32; + std::atomic inputChannelLabelCount_{0}; + std::atomic outputChannelLabelCount_{0}; + char inputChannelLabels_[kMaxChannelLabels][64]{}; + char outputChannelLabels_[kMaxChannelLabels][64]{}; + std::atomic runtimeCapsValid_{false}; }; diff --git a/ASFWDriver/Audio/Protocols/IDeviceProtocol.hpp b/ASFWDriver/Audio/Protocols/IDeviceProtocol.hpp index 268d8230..a27ffb8d 100644 --- a/ASFWDriver/Audio/Protocols/IDeviceProtocol.hpp +++ b/ASFWDriver/Audio/Protocols/IDeviceProtocol.hpp @@ -10,6 +10,8 @@ #include #include #include +#include +#include namespace ASFW::Protocols::AVC { class FCPTransport; @@ -58,6 +60,19 @@ class IDeviceProtocol { return false; } + /// Query per-channel device labels discovered from the protocol's stream + /// format (e.g. DICE TX/RX name sections). `inNames` is host input/capture, + /// `outNames` is host output/playback, both in channel order; an empty entry + /// means "no label for that channel". Returns true only when authoritative + /// labels are available (caps loaded); callers fall back to synthesized + /// names otherwise. + virtual bool GetChannelLabels(std::vector& inNames, + std::vector& outNames) const { + (void)inNames; + (void)outNames; + return false; + } + using VoidCallback = std::function; /// Optional bring-up hook to prepare device-side duplex state at 48kHz. diff --git a/tests/devices/DICETcatProtocolTests.cpp b/tests/devices/DICETcatProtocolTests.cpp index ffbbc646..89c02bfa 100644 --- a/tests/devices/DICETcatProtocolTests.cpp +++ b/tests/devices/DICETcatProtocolTests.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -288,6 +289,42 @@ TEST(DICETcatProtocolTests, RuntimeCapsAggregateTotalConfiguredStreams) { EXPECT_EQ(caps.hostToDeviceIsoChannel, 0U); } +TEST(DICETcatProtocolTests, ChannelLabelsFlattenAcrossStreamsInChannelOrder) { + CountingFireWireBus bus; + DICETcatProtocol protocol(bus, bus, 2, nullptr); + + // No labels before caps are cached. + std::vector inNames; + std::vector outNames; + EXPECT_FALSE(protocol.GetChannelLabels(inNames, outNames)); + + ASFW::Audio::DICE::GlobalState global{}; + global.sampleRate = 48000; + + // Host input == device TX; two streams, names concatenated in stream order. + ASFW::Audio::DICE::StreamConfig tx{}; + tx.numStreams = 2; + strlcpy(tx.streams[0].labels, "Mic 1\\Mic 2\\\\", sizeof(tx.streams[0].labels)); + strlcpy(tx.streams[1].labels, "Line 3\\Line 4\\\\", sizeof(tx.streams[1].labels)); + + // Host output == device RX. + ASFW::Audio::DICE::StreamConfig rx{}; + rx.numStreams = 1; + strlcpy(rx.streams[0].labels, "Main L\\Main R\\\\", sizeof(rx.streams[0].labels)); + + ASFW::Audio::DICE::TCAT::DICETcatProtocolTestPeer::CacheRuntimeCaps(protocol, global, tx, rx); + + ASSERT_TRUE(protocol.GetChannelLabels(inNames, outNames)); + ASSERT_EQ(inNames.size(), 4u); + EXPECT_EQ(inNames[0], "Mic 1"); + EXPECT_EQ(inNames[1], "Mic 2"); + EXPECT_EQ(inNames[2], "Line 3"); + EXPECT_EQ(inNames[3], "Line 4"); + ASSERT_EQ(outNames.size(), 2u); + EXPECT_EQ(outNames[0], "Main L"); + EXPECT_EQ(outNames[1], "Main R"); +} + TEST(DICETcatProtocolTests, ReadDuplexHealthReturnsCurrentGlobalLockState) { CountingFireWireBus bus; DICETcatProtocol protocol(bus, bus, 2, nullptr); From 315561632ac37942abaa09890f9b05ac8d252fd7 Mon Sep 17 00:00:00 2001 From: alicankaralar Date: Tue, 7 Jul 2026 00:54:33 +0200 Subject: [PATCH 7/7] fix(dice): chunk the stream-format section read so every stream's labels 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 --- .../Protocols/DICE/Core/DICETransaction.cpp | 128 ++++++++++++------ 1 file changed, 87 insertions(+), 41 deletions(-) diff --git a/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp b/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp index 8687b1ed..47aa6278 100644 --- a/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp +++ b/ASFWDriver/Audio/Protocols/DICE/Core/DICETransaction.cpp @@ -63,6 +63,57 @@ void LogSectionPreview(const char* label, const uint8_t* data, size_t size) { HexPreview(data, size).c_str()); } +// A single async block read is bounded by the device's max_rec (512 bytes for +// typical DICE firmware), but a multi-stream stream-format section is larger: +// the Venice F32 needs 8 + 2*280 = 568 bytes, and stream 1's 256-byte label +// blob starts at byte 304 — past a single 512-byte read. Chain fixed-size +// chunk reads into one buffer so ParseStreamConfig sees every stream's labels. +// A failure after the first chunk delivers the partial buffer (the parser +// guards each stream's core/label region against the buffer size), matching +// the old best-effort behavior; only a failed first chunk is a hard error. +constexpr size_t kSectionReadChunkBytes = 512; +constexpr size_t kMaxSectionReadBytes = 4096; + +void ReadSectionChunked(Protocols::Ports::ProtocolRegisterIO& io, + uint32_t sectionOffsetBytes, + size_t totalBytes, + std::shared_ptr> accumulated, + std::function done) { + const size_t have = accumulated->size(); + if (have >= totalBytes) { + done(kIOReturnSuccess); + return; + } + + const uint32_t chunk = static_cast( + std::min(kSectionReadChunkBytes, totalBytes - have)); + (void)io.ReadBlock( + MakeDICEAddress(sectionOffsetBytes + static_cast(have)), + chunk, + [&io, sectionOffsetBytes, totalBytes, accumulated, + done = std::move(done)](Async::AsyncStatus status, + std::span payload) mutable { + if (status != Async::AsyncStatus::kSuccess || payload.empty()) { + if (accumulated->empty()) { + done(MapReadStatus(status)); + return; + } + ASFW_LOG(DICE, + "ReadSectionChunked: chunk at +%zu failed (status=%u); using %zu/%zu bytes", + accumulated->size(), + static_cast(status), + accumulated->size(), + totalBytes); + done(kIOReturnSuccess); + return; + } + + accumulated->insert(accumulated->end(), payload.begin(), payload.end()); + ReadSectionChunked(io, sectionOffsetBytes, totalBytes, accumulated, + std::move(done)); + }); +} + } // anonymous namespace DICETransaction::DICETransaction(Protocols::Ports::ProtocolRegisterIO& io) @@ -388,55 +439,50 @@ std::vector SplitDiceLabels(const char* labels) { void DICETransaction::ReadRxStreamConfig(const GeneralSections& sections, std::function callback) { auto callbackState = Common::ShareCallback(std::move(callback)); - const size_t readSize = (sections.rxStreamFormat.size > 512) ? 512 : sections.rxStreamFormat.size; - if (sections.rxStreamFormat.size > 512) { - ASFW_LOG(DICE, "RX stream format section (%u bytes) exceeds read limit %zu; diagnostics may be partial", - sections.rxStreamFormat.size, readSize); - } - - (void)io_.ReadBlock(MakeDICEAddress(sections.rxStreamFormat.offset), - static_cast(readSize), - [callbackState](Async::AsyncStatus status, std::span payload) { - if (status != Async::AsyncStatus::kSuccess) { - Common::InvokeSharedCallback(callbackState, MapReadStatus(status), StreamConfig{}); - return; - } + // Chunked read: cover the whole section (bounded) so every stream's label + // blob is parsed, not just stream 0's (see ReadSectionChunked). + const size_t readSize = + std::min(sections.rxStreamFormat.size, kMaxSectionReadBytes); + auto accumulated = std::make_shared>(); + accumulated->reserve(readSize); + ReadSectionChunked( + io_, sections.rxStreamFormat.offset, readSize, accumulated, + [callbackState, accumulated](IOReturn status) { + if (status != kIOReturnSuccess) { + Common::InvokeSharedCallback(callbackState, status, StreamConfig{}); + return; + } - const uint8_t* data = payload.data(); - const size_t size = payload.size(); - LogSectionPreview("ReadRxStreamConfig", data, size); - StreamConfig config = ParseStreamConfig(data, size, true); - LogStreamConfigDetails("RX", config); - - Common::InvokeSharedCallback(callbackState, kIOReturnSuccess, config); - }); + LogSectionPreview("ReadRxStreamConfig", accumulated->data(), accumulated->size()); + StreamConfig config = ParseStreamConfig(accumulated->data(), accumulated->size(), true); + LogStreamConfigDetails("RX", config); + + Common::InvokeSharedCallback(callbackState, kIOReturnSuccess, config); + }); } void DICETransaction::ReadTxStreamConfig(const GeneralSections& sections, std::function callback) { auto callbackState = Common::ShareCallback(std::move(callback)); - const size_t readSize = (sections.txStreamFormat.size > 512) ? 512 : sections.txStreamFormat.size; - if (sections.txStreamFormat.size > 512) { - ASFW_LOG(DICE, "TX stream format section (%u bytes) exceeds read limit %zu; diagnostics may be partial", - sections.txStreamFormat.size, readSize); - } - - (void)io_.ReadBlock(MakeDICEAddress(sections.txStreamFormat.offset), - static_cast(readSize), - [callbackState](Async::AsyncStatus status, std::span payload) { - if (status != Async::AsyncStatus::kSuccess) { - Common::InvokeSharedCallback(callbackState, MapReadStatus(status), StreamConfig{}); - return; - } + // Chunked read: see ReadRxStreamConfig. + const size_t readSize = + std::min(sections.txStreamFormat.size, kMaxSectionReadBytes); + auto accumulated = std::make_shared>(); + accumulated->reserve(readSize); + ReadSectionChunked( + io_, sections.txStreamFormat.offset, readSize, accumulated, + [callbackState, accumulated](IOReturn status) { + if (status != kIOReturnSuccess) { + Common::InvokeSharedCallback(callbackState, status, StreamConfig{}); + return; + } - const uint8_t* data = payload.data(); - const size_t size = payload.size(); - LogSectionPreview("ReadTxStreamConfig", data, size); - StreamConfig config = ParseStreamConfig(data, size, false); - LogStreamConfigDetails("TX", config); + LogSectionPreview("ReadTxStreamConfig", accumulated->data(), accumulated->size()); + StreamConfig config = ParseStreamConfig(accumulated->data(), accumulated->size(), false); + LogStreamConfigDetails("TX", config); - Common::InvokeSharedCallback(callbackState, kIOReturnSuccess, config); - }); + Common::InvokeSharedCallback(callbackState, kIOReturnSuccess, config); + }); } void DICETransaction::ReadCapabilities(std::function callback) {