From 4f71b784fe212ab9edd3eee9556e01e5eb127adb Mon Sep 17 00:00:00 2001 From: Talon Date: Thu, 23 Jul 2026 13:37:05 +0200 Subject: [PATCH] docs: condense implementation comments --- PROGRESS.md | 10 ++ .../Sources/VoiceCatCore/Callbacks.swift | 18 +-- .../Sources/VoiceCatCore/VoiceCatClient.swift | 60 +------- clients/apple/iOS/VoiceCatiOS/AppState.swift | 142 +----------------- .../iOS/VoiceCatiOS/AudioSessionManager.swift | 33 +--- .../iOS/VoiceCatiOS/IOSAudioRouter.swift | 92 ++---------- .../IOSVoiceProcessingEngine.swift | 36 +---- .../Audio/ScreenAudioCapture.swift | 27 +--- .../VoiceCat.App/Audio/InputDeviceCapture.cs | 15 +- .../Audio/ProcessLoopbackCapture.cs | 15 +- clients/windows/VoiceCat.Interop/Structs.cs | 4 +- core/include/voicecat.h | 2 +- core/src/audio/audio_engine.cpp | 5 +- core/src/audio/audio_engine.h | 43 +----- core/src/core/client.cpp | 4 +- core/src/net/transport.cpp | 5 +- docs/protocol.md | 7 + docs/voice.md | 15 ++ server/src/conn_session.cpp | 11 +- server/src/main.cpp | 2 +- server/src/server.cpp | 8 +- server/src/session_registry.h | 55 +------ 22 files changed, 102 insertions(+), 507 deletions(-) diff --git a/PROGRESS.md b/PROGRESS.md index 109433b..86eaeac 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -10,6 +10,16 @@ up instantly. Newest status at the top. ## ▶ Where we left off / next action +- **Done (2026-07-23):** **First comment-density cleanup across core, server, and native + clients.** Condensed comments in the highest-noise audio, reconnect, registry, and binding + files; removed implementation history and narration; retained ABI ownership, threading, + real-time, ordering, and OS-API invariants. Moved the durable iOS audio-routing/pacing rules + to `docs/voice.md` and client reconnection policy to `docs/protocol.md`. No behavior, wire + format, or C ABI changes. **Verification:** `cmake --build --preset dev` green; + `ctest --preset dev` 29/29 green. `dotnet build VoiceCat.slnx` restores dependencies and + builds `VoiceCat.Interop` + `VoiceCat.App`, then fails in the unchanged test project because + `ExternalPcmTests.cs:49` references internal `NativeMethods` (`CS0122`). + - **Done (2026-06-25):** **Fixed iOS AirPods-disconnect reinitialize loop on A2DP presets (Stereo Mic / Mono Mic).** Regression from the 2026-06-25 audio-device-change recovery commit below, which broadened the route-change recovery set from diff --git a/clients/apple/Sources/VoiceCatCore/Callbacks.swift b/clients/apple/Sources/VoiceCatCore/Callbacks.swift index 9ffef0f..b4cb6e6 100644 --- a/clients/apple/Sources/VoiceCatCore/Callbacks.swift +++ b/clients/apple/Sources/VoiceCatCore/Callbacks.swift @@ -1,19 +1,5 @@ -// Callbacks — the C function pointers passed to `vc_callbacks`. These are the Swift -// equivalent of the C# client's `[UnmanagedCallersOnly]` static methods (NativeCallbacks.cs). -// -// The critical patterns (carried over from the proven C# implementation): -// 1. `@convention(c)` closures — plain C function pointers, NOT GC/ARC-managed closures. -// A @convention(c) closure cannot capture context, which is why the `user` pointer is -// used to resolve back to the VoiceCatClient instance (the C# version uses GCHandle for -// the same thing; Swift uses Unmanaged). -// 2. `Unmanaged.passUnretained(self).toOpaque()` as the `user` context — a stable raw -// pointer to the Swift object WITHOUT incrementing the retain count. This is safe -// because `deinit` calls `vc_client_destroy` (which synchronously joins every internal -// thread) BEFORE the object's memory is freed — so no callback can fire after the object -// is gone. (The C# equivalent: GCHandle.Alloc + GCHandle.Free in Dispose.) -// 3. Copy `ev.text` to a Swift `String` INSIDE `onEvent` (via `VoiceCatEvent.from(_:)`) -// before returning — the raw pointer is dangling after the callback returns. This is -// the #1 lifetime rule from voicecat.h's vc_event doc comment. +// C callbacks use an unretained `user` context. Client destruction joins callback threads, +// and transient event pointers are copied before the callback returns. import VoiceCatC import Foundation diff --git a/clients/apple/Sources/VoiceCatCore/VoiceCatClient.swift b/clients/apple/Sources/VoiceCatCore/VoiceCatClient.swift index eb32a64..d286e58 100644 --- a/clients/apple/Sources/VoiceCatCore/VoiceCatClient.swift +++ b/clients/apple/Sources/VoiceCatCore/VoiceCatClient.swift @@ -1,36 +1,6 @@ -// VoiceCatClient — the public, Swift-idiomatic surface over libvoicecat. This is the Swift -// analog of the C# client's `VoiceCatClient.cs` (clients/windows/VoiceCat.Interop). -// -// Key patterns carried over from the proven C# implementation (see docs/architecture.md §4 -// per-platform binding notes): -// -// 1. HANDLE OWNERSHIP: the class owns `vc_client*`; `deinit` calls `vc_client_destroy` -// (which synchronously joins every internal thread, so nothing can still be reading the -// config-string pointers or firing callbacks by the time it returns). -// -// 2. CONFIG STRING LIFETIMES: the core stores raw pointers from `vc_config` by value — it -// does NOT copy the string data. `client_name`/`client_version`/`tofu_store_path` are -// read later, whenever `connect()` actually runs on the io_thread_. So the native CString -// storage (`_clientNamePtr` etc.) must outlive the WHOLE client, not just `init`. It's -// freed in `deinit`, AFTER `vc_client_destroy` has returned. (C#: Marshal.StringToCoTask -// MemUTF8 in ctor, FreeCoTaskMem in Dispose after destroy.) -// -// 3. EVENT DELIVERY THREAD HANDOFF: `on_event` fires on the core's event thread. Events are -// buffered in a lock-protected array and drained on `DispatchQueue.main` — this is the -// boundary where the core's thread hands off to the UI thread. The C# analog is -// `Channel` drained by a 30ms WinForms Timer; the Swift analog is a -// coalesced main-queue drain (only one async block scheduled at a time). `on_event`'s -// `text` is copied to a Swift `String` inside the callback (Callbacks.swift) before -// enqueueing — the raw pointer is dangling by the time the main thread drains. -// -// 4. LEVEL METER COALESCING: `on_level` fires far more often than `on_event` and -// intermediate values are visually irrelevant — coalesced to "latest sample per -// stream_id" in a lock-protected dictionary, drained on main alongside events. -// (C#: ConcurrentDictionary cleared in PumpEvents.) -// -// 5. IMMEDIATE vc_free_* ON LIST READS: `listChannels()`/`listUsers()`/etc. walk the native -// array, convert to Swift value types, and call `vc_free_*_list` INSIDE the function — -// callers never manage native list lifetime. (C#: Marshaling.ToManaged does the same.) +// Swift binding invariants: native config strings outlive the handle, destroy joins callback +// threads before deallocation, and callback payloads are copied before main-queue delivery. +// See docs/architecture.md §4 for the complete binding contract. import VoiceCatC import Foundation @@ -56,26 +26,14 @@ public final class VoiceCatClient { // MARK: - Stored properties - /// The opaque C handle (`vc_client*` — Swift imports the incomplete C struct as - /// `OpaquePointer`). Set in `init`, passed to every C function, destroyed in `deinit`. private var handle: OpaquePointer? - /// Unmanaged pointer to `self` — passed as `vc_callbacks.user` so the C function-pointer - /// callbacks can resolve back to this instance. `passUnretained` (not `passRetained`) - /// because we want normal ARC to control the object's lifetime — `deinit` calls - /// `vc_client_destroy` (joins all threads) before the object's memory is freed, so no - /// callback can fire with a dangling `user` pointer. See Callbacks.swift. - /// - /// Computed (not stored) to break a circular init dependency: it needs `self`, but - /// stored properties must be initialized before `self` is available. `Unmanaged.passUn - /// retained(self).toOpaque()` always returns the same address for a given instance, so - /// computing it on demand is safe and consistent. + /// Unretained callback context; destroying the handle joins callback threads first. private var selfPointer: UnsafeMutableRawPointer { Unmanaged.passUnretained(self).toOpaque() } - /// Native CString storage backing `vc_config` — must outlive the whole client (the core - /// stores raw pointers, doesn't copy). Freed in `deinit` after `vc_client_destroy`. + /// The core retains these pointers for the handle's lifetime. private var clientNamePtr: UnsafeMutablePointer? private var clientVersionPtr: UnsafeMutablePointer? private var tofuStorePathPtr: UnsafeMutablePointer? @@ -90,7 +48,6 @@ public final class VoiceCatClient { /// Intermediate values are coalesced (only the latest per stream_id is delivered). public var onLevel: ((UInt32, Float) -> Void)? - /// Lock-protected buffers, written from the core's event thread, drained on main. private let bufferLock = NSLock() private var eventBuffer: [VoiceCatEvent] = [] private var levelSamples: [UInt32: Float] = [:] @@ -98,19 +55,12 @@ public final class VoiceCatClient { // MARK: - Init / deinit - /// Create a client. `config.clientName`/`clientVersion`/`tofuStorePath` are copied to - /// native CString storage held for the client's entire lifetime (the core reads them - /// later, e.g. when `connect()` runs on the io thread). public init(config: VoiceCatConfig) { - // Allocate native C strings — must persist until after vc_client_destroy in deinit. - // These don't need `self`, so they're safe to set first. self.clientNamePtr = strdup(config.clientName) self.clientVersionPtr = strdup(config.clientVersion) self.tofuStorePathPtr = config.tofuStorePath.flatMap { strdup($0) } self.handle = nil // placeholder — set below after callbacks are wired - // All stored properties are now initialized → `self` is fully available, so we can - // call `selfPointer` (the computed property) to build the callbacks struct. var nativeConfig = vc_config() nativeConfig.client_name = UnsafePointer(clientNamePtr) nativeConfig.client_version = UnsafePointer(clientVersionPtr) diff --git a/clients/apple/iOS/VoiceCatiOS/AppState.swift b/clients/apple/iOS/VoiceCatiOS/AppState.swift index 7620828..3378410 100644 --- a/clients/apple/iOS/VoiceCatiOS/AppState.swift +++ b/clients/apple/iOS/VoiceCatiOS/AppState.swift @@ -26,41 +26,14 @@ final class AppState { private(set) var connectingServer: SavedServer? private var identityHandled = false - /// The server we are currently fully connected to. Set on auth success (when `session` is - /// created) and cleared on teardown. Used to build the `lastSession` restore snapshot when a - /// live-session disconnect fires through `SessionState.handleEvent` — `connectingServer` is - /// already nil by then, and `handleConnectEvent`'s `server` parameter is out of scope because - /// `SessionState` owns `client.onEvent` after auth success (see `SessionState.init`). + /// Retained after authentication so an interrupted session can be restored. private var connectedServer: SavedServer? // MARK: - Reconnect state - // - // The C core surfaces every unexpected connection drop as a `.disconnected` event; nothing - // in the C core auto-reconnects (intentional — reconnect UX is the client's job). Two layers - // drive iOS reconnect: - // - // 1. **Event-driven** (the C core's TCP read eventually fails after the keepalive/reaper - // timeout, ~30-60 s on a hard Wi-Fi drop): `SessionState.handleEvent` `.disconnected` - // plays the audible cue, then calls back into AppState via `onLiveSessionDisconnected`, - // which snapshots the live session, tears it down, and arms `scheduleReconnect`. - // (Live-session events never reach `AppState.handleConnectEvent` — `SessionState.init` - // overwrites `client.onEvent`, so `AppState` cannot see them without the callback.) - // - // 2. **Path-driven** (proactive, much faster): `NWPathMonitor` runs the whole time we are - // connected (started on auth success) and reacts to network changes — a Wi-Fi↔cellular - // flip or the path becoming `.unsatisfied` calls `proactiveReconnect`, which tears the - // live session down BEFORE the C core notices the dead TCP path. This is what makes the - // 30-60 s wait collapse into ~1 s + the backoff tick. While mid-reconnect (no session) - // the same monitor arms a fast-fresh retry whenever a path becomes `.satisfied`. - // - // Manual Disconnect cancels everything (task + path monitor) and clears `lastSession`. - /// True at the top of `disconnect()`/`cancelConnect()` — suppresses auto-reconnect for the - /// `.disconnected` event the core then emits in response to our `vc_disconnect()` call. + /// Distinguishes an explicit disconnect from a transport failure. private var userInitiatedDisconnect = false - /// Snapshot of the live session state needed to restore after a reconnect. Cleared on - /// successful restore and on user-initiated disconnect. private struct LastSession { let server: SavedServer let channelId: UInt32 @@ -70,24 +43,14 @@ final class AppState { } private var lastSession: LastSession? - /// Reconnect attempt counter — drives exponential backoff. Reset to 0 on successful auth and - /// on a path-driven fast-fresh retry. private var reconnectAttempt = 0 - /// The in-flight reconnect `Task` (sleeps for the backoff, then calls `connectTo`). One - /// at a time; cancelled on user disconnect / successful restore. private var reconnectTask: Task? - /// Started on auth success and kept running while connected / mid-reconnect; stopped only on - /// user-initiated disconnect. Its `pathUpdateHandler` (dispatched to @MainActor) handles two - /// cases: a path change while connected → proactive reconnect; a satisfied path while - /// mid-reconnect → fast-fresh retry. See the reconnect-state header comment. + /// Detects interface changes before TCP keepalive notices a dead path. private var pathMonitor: NWPathMonitor? private let pathQueue = DispatchQueue(label: "cat.voice.network.path") - /// Signature of the last path seen by the monitor (a stable string encoding status + active - /// interface types). The very first path callback (when the monitor starts) sets this and is - /// otherwise ignored — it's the baseline; only subsequent CHANGES are reconnect triggers. private var lastPathSignature: String? // MARK: - Server list management @@ -118,14 +81,10 @@ final class AppState { // MARK: - Connect flow - /// Public connect entry. Always starts a fresh session (no restore). func connectTo(_ server: SavedServer) { connectTo(server, restoring: nil) } - /// Internal connect that drives the full TLS/auth saga. `restoring` is non-nil for a - /// reconnect attempt following an unexpected disconnect; the captured channel + voice/mic - /// state is handed to the new `SessionState` after auth succeeds. private func connectTo(_ server: SavedServer, restoring: LastSession?) { guard !isConnecting else { return } isConnecting = true @@ -134,10 +93,7 @@ final class AppState { identityHandled = false userInitiatedDisconnect = false - // Discard any leftover connecting client. nil'ing the strong ref calls VoiceCatClient's - // deinit, which synchronously joins the C core's io thread (vc_client_destroy) before - // freeing the config-string storage — safe from @MainActor because the io thread never - // blocks on main (it enqueues events via DispatchQueue.main.async and returns). + // Releasing the wrapper joins the core's I/O thread before freeing native strings. connectingClient = nil let config = VoiceCatConfig( @@ -153,20 +109,10 @@ final class AppState { self?.handleConnectEvent(ev, server: server, restoring: restoring) } } - // Put the core into external-playback mode BEFORE connect, so the flag is set on the - // io thread before any message is processed. The server sends AuthResult immediately - // followed by ServerStateSnapshot; handle_server_state runs ensure_audio_running() on - // the io thread, and if external_playback_ were still false at that point the core - // would open a hardware miniaudio playback+capture device (see the matching fix in - // vc_client::ensure_audio_running). Setting it here — before connect — guarantees the - // unified external path is in effect from the first frame. setExternalPlayback only - // flips an atomic + forwards to the engine's setter; both are safe pre-connect. + // Authentication can start audio, so select the external path before connecting. client.setExternalPlayback(true) client.connect(host: server.host, port: server.port) - // Auth is queued immediately — the core serialises it behind TLS + TOFU. On a reconnect - // the TOFU pin already matches (VC_TOFU_MATCHED), so the identity gate auto-confirms - // inside the .serverIdentity case below and auth proceeds unattended. switch server.authMode { case .guest: let nick = (server.nickname?.isEmpty == false) ? server.nickname! : "iOS User" @@ -182,8 +128,7 @@ final class AppState { } func disconnect() { - // Mark BEFORE we ask the core to disconnect, so the .disconnected event the core emits - // in response is treated as user-initiated (no reconnect) rather than an unexpected drop. + // Set before disconnect so its event cannot arm reconnect. userInitiatedDisconnect = true cancelReconnect() lastSession = nil @@ -232,31 +177,13 @@ final class AppState { // MARK: - Reconnect orchestration - /// Cancel any in-flight reconnect task and stop the path monitor. Safe to call when nothing - /// is armed (no-op). Does NOT touch `userInitiatedDisconnect` or `lastSession` — callers set - /// those as needed (disconnect/cancelConnect clear them; scheduleReconnect keeps them). - /// - /// `AppState` is the @Observable app root owned by the SwiftUI `App`; it lives for the - /// whole app process and is torn down only on process exit, at which point OS cleanup - /// suffices. This method is driven by `disconnect()`/`cancelConnect()` and on successful - /// restore — those run on user-initiated teardown, which is the only path that matters. - /// (The reconnect `Task` captures `[weak self]` and guards on `nil`/`userInitiatedDisconnect`, - /// so a stray task left running when AppState is gone is a no-op; the monitor similarly guards.) private func cancelReconnect() { reconnectTask?.cancel() reconnectTask = nil stopPathMonitor() - // Don't reset `reconnectAttempt` here: scheduleReconnect resets it on successful auth, - // and the path monitor resets it to 0 for a fast fresh attempt on a path-satisfied event. - // If a fresh user connect follows, connectTo() doesn't reset it either, but it doesn't - // need to — `reconnectAttempt` only matters while we're mid-reconnect. } - /// Arm the next reconnect attempt with exponential backoff (1s → 2s → 4s → 8s → 16s → 30s - /// cap). Cancelled cleanly by `cancelReconnect()` on user disconnect or successful auth. - /// Idempotent: a new call supersedes any in-flight one. The path monitor is armed here and - /// disarmed on cancel; on a path-satisfied event it resets the attempt counter to 0 and - /// re-arms via this same method, yielding a fast refresh after Wi-Fi ↔ cellular transitions. + /// Schedules the next reconnect with exponential backoff capped at 30 seconds. private func scheduleReconnect() { guard !userInitiatedDisconnect, let last = lastSession else { return } reconnectTask?.cancel() @@ -270,7 +197,6 @@ final class AppState { guard let self else { return } try? await Task.sleep(nanoseconds: UInt64(delaySec * 1_000_000_000)) if Task.isCancelled { return } - // Re-check under Task: a user disconnect between the sleep and this line must abort. guard !self.userInitiatedDisconnect else { return } guard self.lastSession != nil else { return } guard self.session == nil else { return } @@ -279,20 +205,6 @@ final class AppState { reconnectTask = task } - /// Start (if not already running) the network path monitor. Runs the whole time we are - /// connected (started on auth success) and stays armed across reconnects; stopped only on - /// user-initiated disconnect. The handler dispatches to @MainActor before touching state and - /// does two distinct things: - /// - **While connected** (`session != nil`): a Wi-Fi↔cellular interface change OR the path - /// becoming `.unsatisfied` triggers `proactiveReconnect()` — tearing the live session - /// down before the C core notices the dead TCP read. Without this the disconnect would - /// take 30-60 s (the TCP keepalive/reaper timeout); proactive teardown collapses that to - /// ~1 s + the first backoff tick. Same-interface path refreshes (e.g. a Wi-Fi roam - /// without an IP change) are ignored — likely the connection is still good. - /// - **While mid-reconnect** (`session == nil`, `lastSession != nil`): a path becoming - /// `.satisfied` arms a fast-fresh retry (backoff counter reset, `scheduleReconnect`). - /// This is what makes a Wi-Fi→cellular flip reconnect on roughly the next tick instead - /// of waiting out a long backoff. private func startPathMonitor() { guard pathMonitor == nil else { return } let monitor = NWPathMonitor() @@ -303,12 +215,9 @@ final class AppState { let sig = Self.pathSignature(path) let prevSig = self.lastPathSignature self.lastPathSignature = sig - // The first callback (when the monitor starts) is the baseline, not a change. if prevSig == nil { return } if self.session != nil { - // See this method's doc comment for why these conditions trigger a - // proactive reconnect. if path.status != .satisfied || sig != prevSig { self.proactiveReconnect() } @@ -320,8 +229,6 @@ final class AppState { } } } - // Listen on a dedicated queue — the path monitor can't share the main queue (it would - // re-enter main if any handler dispatched to main synchronously). monitor.start(queue: pathQueue) pathMonitor = monitor } @@ -332,11 +239,6 @@ final class AppState { lastPathSignature = nil } - /// A stable string signature of a network path: the path status plus the set of interface - /// types it uses. Two paths with the same signature are treated as equivalent — no - /// reconnect. A signature change is the trigger for `proactiveReconnect`. Used to ignore - /// same-interface refreshes (signal-strength changes, BSSID roams) which usually don't break - /// the TCP connection. private static func pathSignature(_ path: NWPath) -> String { guard path.status == .satisfied else { return "unsatisfied" } var parts: [String] = [] @@ -349,42 +251,19 @@ final class AppState { // MARK: - Live-session disconnect (called by SessionState) - /// Called by `SessionState.handleEvent` `.disconnected` after the audible cue has already - /// played. Once `SessionState` is created (auth success) it owns `client.onEvent`, so - /// `AppState.handleConnectEvent` never sees live-session events — this callback is the only - /// way AppState learns that a live session dropped. Snapshots the live session state, - /// tears the session down, and arms `scheduleReconnect` so the backoff loop drives a fresh - /// TLS/auth/restoration. Guarded against user-initiated disconnect (which nil's `session` - /// synchronously, so SessionState is gone before the event could fire this callback) — but - /// the guard is cheap insurance. + /// Receives disconnects after `SessionState` takes ownership of authenticated events. func onLiveSessionDisconnected() { guard !userInitiatedDisconnect else { return } teardownLiveSessionAndReconnect(sound: false) } - /// Called by `NWPathMonitor` when a path change is detected while a live session exists. - /// Tears the session down immediately — nil'ing `session` releases `VoiceCatClient`, whose - /// `deinit` calls `vc_client_destroy`; that closes the socket and joins the C core's io - /// thread, so the io thread exits in milliseconds rather than blocking on a dead read for - /// ~30-60 s. The proactive tear-down is what collapses the long TCP-reaper wait into a - /// ~1 s reconnect. Plays the audible cue (no `.disconnected` event fires through to - /// `SessionState` for this path, since `SessionState` is being torn down here — so the cue - /// would otherwise be missing). private func proactiveReconnect() { guard !userInitiatedDisconnect else { return } guard session != nil else { return } teardownLiveSessionAndReconnect(sound: true) } - /// Shared teardown for a live-session disconnect (event- or path-driven). Snapshots the live - /// session into `lastSession`, stops audio, deactivates the AVAudioSession, releases the - /// session (which releases `VoiceCatClient` → io-thread join), resets the backoff counter, - /// and arms `scheduleReconnect`. `sound` is true for the proactive (path-driven) case — the - /// `.disconnected` event that would have played it never fires because we're tearing down - /// ahead of the C core noticing. The event-driven caller (`onLiveSessionDisconnected`) has - /// ALREADY played the cue via `SessionState.handleEvent`, so it passes `sound: false`. private func teardownLiveSessionAndReconnect(sound: Bool) { - // Snapshot BEFORE nil'ing `session` — we need the channel + voice/mic state to restore. if let s = session, let srv = connectedServer { lastSession = LastSession( server: srv, @@ -395,8 +274,6 @@ final class AppState { } IOSAudioEngine.shared.stop() AudioSessionManager.shared.deactivateSession() - // Releasing `session` releases `VoiceCatClient`; its deinit joins the C core's io thread. - // For a path-driven proactive teardown this is what avoids the 30-60 s reaper timeout. session = nil isConnecting = false connectingClient = nil @@ -405,9 +282,6 @@ final class AppState { EventFeedback.shared.play(.connectionLost) EventFeedback.shared.speak("Network changed — reconnecting") } - // Reset the backoff counter so the first reconnect attempt after a drop uses the short - // 1 s delay (the immediate path-driven attempt matters most; sustained-outage backoff is - // driven by `scheduleReconnect`'s increment). reconnectAttempt = 0 scheduleReconnect() } diff --git a/clients/apple/iOS/VoiceCatiOS/AudioSessionManager.swift b/clients/apple/iOS/VoiceCatiOS/AudioSessionManager.swift index c4cf62c..aa35f4d 100644 --- a/clients/apple/iOS/VoiceCatiOS/AudioSessionManager.swift +++ b/clients/apple/iOS/VoiceCatiOS/AudioSessionManager.swift @@ -35,14 +35,7 @@ final class AudioSessionManager { name: AVAudioSession.routeChangeNotification, object: nil) } - /// The single end-to-end audio recovery path, driven by *intent* (`IOSAudioEngine.isConnected`) - /// — not by session bookkeeping flags that can drift out of sync (e.g. an interruption ended - /// without `.shouldResume`, which used to leave `isSessionActive` false forever). Safe to call - /// speculatively: the underlying calls are idempotent (AVAudioSession.setActive(true), - /// `IOSAudioRouter.applyConfiguration` has a re-entrancy guard, `IOSAudioEngine.reconfigure` - /// no-ops when not connected). Call this whenever the audio environment changes in a way that - /// could have stopped the engine — interruption end, route change, AVAudioEngine - /// configuration-change — and we still want audio back. + /// Idempotently restores audio after an interruption or external route change. func recoverAudio() { guard IOSAudioEngine.shared.isConnected else { return } do { @@ -158,27 +151,9 @@ final class AudioSessionManager { IOSAudioRouter.shared.refreshRoutes() NotificationCenter.default.post(name: .voiceCatDeviceListChanged, object: nil) - // Recover audio on every externally-initiated route change. `.categoryChange`, - // `.routeConfigurationChange`, and `.override` are fired by our OWN calls: - // - `.categoryChange` / `.routeConfigurationChange` ← applyConfiguration()'s - // setCategory / setPreferredInput / ... - // - `.override` ← applyA2dpSpeakerFallback()'s overrideOutputAudioPort(.speaker), - // which fires on every AirPods disconnect (and reconnect) on an A2DP preset. - // Acting on any of these would create a tight ping-pong loop with the re-entrancy - // guard (handleRouteChange → recoverAudio → applyA2dpSpeakerFallback → - // overrideOutputAudioPort → .override routeChange → recoverAudio → ...). The - // `.override` skip is what fixes the AirPods-disconnect reinitialize loop: each - // iteration also calls IOSAudioEngine.reconfigure() → rebuild() (a full - // stop/restart of AVAudioEngine), which is the audible cycling. IOSAudioRouter's - // guard is the backstop that bounds it to ONE extra iteration, but skipping these - // three reasons avoids even that, so we reconfigure only in response to genuine - // environmental changes. - // - // The recovery set below (oldDeviceUnavailable, newDeviceAvailable, wakeFromSleep, - // noSuitableRouteForCategory, unknown) covers headphone/AirPods/wired unplug-replug - // — the previously-reported "audio dies when headphones disconnect" bug. If an - // override ever actually stops the AVAudioEngine, the - // AVAudioEngineConfigurationChange handler in IOSVoiceProcessingEngine catches it. + // Ignore notifications caused by our own configuration calls; rebuilding for them + // recursively emits more route changes. Engine-configuration notifications remain + // the recovery path if a self-initiated change actually stops AVAudioEngine. if reason != .categoryChange && reason != .routeConfigurationChange && reason != .override { recoverAudio() } diff --git a/clients/apple/iOS/VoiceCatiOS/IOSAudioRouter.swift b/clients/apple/iOS/VoiceCatiOS/IOSAudioRouter.swift index 440e6a0..1064dc8 100644 --- a/clients/apple/iOS/VoiceCatiOS/IOSAudioRouter.swift +++ b/clients/apple/iOS/VoiceCatiOS/IOSAudioRouter.swift @@ -4,48 +4,8 @@ import VoiceCatCore private let logger = Logger(subsystem: "cat.voice.VoiceCatiOS", category: "IOSAudioRouter") -/// iOS audio routing layer — the sole owner of `AVAudioSession` on iOS. On iOS the core never -/// opens a hardware (miniaudio) device: a single `AVAudioEngine` (`IOSAudioEngine`) drives both -/// capture and playback and the core runs fully external (see docs/voice.md §8). This class just -/// configures the *route* — category / mode / options, preferred input, data source, polar -/// pattern, stereo capsule — and `IOSAudioEngine` binds to whatever route is established. After -/// any change here the engine is rebuilt via `IOSAudioEngine.reconfigure()` (a deterministic -/// Swift-only stop → reconfigure → start); there is no second (miniaudio) audio path to hand off -/// to, so a change cannot leave one direction dropped. -/// -/// (The core's iOS `ma_context` is still configured with `sessionCategory = none` + -/// `noAudioSessionActivate/Deactivate` in `AudioEngine::make_context_config` so that, should the -/// core ever open a device, miniaudio would not reset the category — but on iOS it does not.) -/// -/// The three user-facing choices: -/// 1. **Input port** — which physical input (built-in mic, Bluetooth HFP, headset, -/// USB, AirPlay). For the built-in mic, a sub-selection of **data source** -/// (orientation: front/back/top/bottom) and **polar pattern** -/// (omni/cardioid/subcardioid/bidirectional). -/// 2. **Bluetooth mode** — how Bluetooth headsets are handled: -/// - "BT HFP voice" (`.allowBluetoothHFP` + `.allowBluetoothA2DP`): both profiles -/// allowed, iOS picks HFP for two-way mic or A2DP for output-only. Mono, AEC on. -/// - "Built-in Mic + BT A2DP stereo" (`.allowBluetoothA2DP` only): stereo output, -/// built-in mic, no HFP processing. -/// - "Built-in Mic + Speaker" (neither): no Bluetooth at all. -/// 3. **Mic processing mode** — Standard (`.voiceChat`: AEC/AGC/HPF on) or -/// Raw/Studio (`.measurement`: all processing off). Raw mode is allowed always -/// but shows a warning when the output route is the speaker (echo risk, no AEC). -/// -/// Additionally, **stereo capture** (2-channel built-in mic) is enabled by switching the -/// built-in mic's data source to the `.stereo` polar pattern. The recipe is: -/// `setPreferredDataSource(.stereo source)` + `setPreferredPolarPattern(.stereo)` + -/// `setPreferredInput(built-in mic)` + `setInputDataSource(stereo source)`. The channel -/// count itself must NOT be requested via `setPreferredInputNumberOfChannels(2)` — that -/// session-level call collapses the A2DP output route. Instead the core is told to open the -/// device with 2 channels via `vc_set_capture_channels(streamId, 2)`, and the AVAudioSession -/// input anchor (`setPreferredInput` + `setInputDataSource`) keeps the route stable during -/// the HFP→A2DP and mono→stereo reconfigurations. -/// -/// Voice Isolation / Wide Spectrum (iOS 17+/18+) are user-toggleable in Control Center -/// for `.voiceChat` apps — surfaced as a hint, not a programmatic toggle. -/// -/// All choices are persisted in `UserDefaults` and re-applied on route changes. +/// Owns `AVAudioSession` routing for the iOS external-audio path. +/// Route configuration and ordering constraints are documented in `docs/voice.md`. @MainActor final class IOSAudioRouter: ObservableObject { @@ -156,17 +116,10 @@ final class IOSAudioRouter: ObservableObject { private let kVoiceProcessing = "cat.voice.audio.voiceProcessing" private let kAgc = "cat.voice.audio.agc" - /// Re-entrancy guard: setCategory/setPreferredInput/etc. trigger route-change - /// notifications synchronously on the same thread. Without this guard, - /// handleRouteChange → applyConfiguration → setCategory → route-change notification - /// → handleRouteChange → applyConfiguration → ... creates an infinite loop that - /// burns CPU and cycles the audio session on/off (the "glitching" bug). + /// AVAudioSession setters can synchronously emit route-change notifications. private var isApplyingConfiguration = false - /// Last `overrideOutputAudioPort` value we successfully applied (`.none` or `.speaker`). - /// See `applyA2dpSpeakerFallback`'s doc comment for why this cache exists. - /// `nil` = "unknown / assume not applied" — reset at the top of `applyConfiguration()` - /// because `setCategory` can reset the override out from under us, and on first run. + /// Prevents redundant overrides; `setCategory` invalidates the cached value. private var lastAppliedOutputOverride: AVAudioSession.PortOverride? private init() {} @@ -399,16 +352,8 @@ final class IOSAudioRouter: ObservableObject { updateWarnings() } - /// Enable 2-channel capture on the built-in mic. The recipe that achieves stereo mic + - /// A2DP Bluetooth output simultaneously: - /// 1. `setPreferredDataSource(stereoSource)` on the built-in mic port - /// 2. `setPreferredPolarPattern(.stereo)` on that data source - /// 3. `setPreferredInput(builtIn)` — anchor the input route explicitly. Without this - /// anchor the route can collapse during the mode switch (.voiceChat → .default). - /// 4. `setInputDataSource(stereoSource)` — commit the data source at the session level - /// The channel count itself is carried by the engine's mic tap (which captures 2 channels) - /// plus `vc_set_capture_channels(2)` so the core encodes stereo. We must NOT call - /// `setPreferredInputNumberOfChannels(2)` — that session-level call collapses the A2DP route. + /// Anchors the built-in stereo data source without using + /// `setPreferredInputNumberOfChannels`, which disrupts A2DP routing. private func configureStereoCapture(session: AVAudioSession) { guard let builtIn = session.availableInputs?.first(where: { $0.portType == .builtInMic }) else { @@ -530,11 +475,7 @@ final class IOSAudioRouter: ObservableObject { // MARK: - Selection setters (called from SettingsView pickers) - /// Shared tail for every setting change: persist, re-apply the AVAudioSession config, refresh - /// the route lists, re-evaluate the A2DP speaker fallback, and rebind the live engine to the - /// new route. `IOSAudioEngine.reconfigure()` is a no-op when not connected, so this is safe to - /// call from Settings whether or not a session is in progress. There is no longer a second - /// (miniaudio) audio path to hand off to, so one engine rebuild is the whole story. + /// Persists the selection and rebuilds the engine against the resulting route. private func applyAndReconfigure() { savePreferences() applyConfiguration() @@ -658,23 +599,8 @@ final class IOSAudioRouter: ObservableObject { showsA2dpNoAecWarning = (bluetoothMode == .builtInMicBtA2dp) } - /// Route fallback for the A2DP-output presets (Stereo Mic / Studio / BT Headphones + Mono - /// Mic, all `.builtInMicBtA2dp`). These presets deliberately omit `.defaultToSpeaker` (it - /// breaks A2DP routing) and skip the `forceSpeaker` override, so when NO external output - /// (Bluetooth A2DP / wired / AirPlay) is connected `.playAndRecord` pins output to the quiet - /// built-in receiver (earpiece). This routes to the loud built-in speaker instead via a - /// post-activation `overrideOutputAudioPort(.speaker)` — the documented "A2DP if connected, - /// else speaker" behavior. When an external output IS present we clear the override so A2DP / - /// headphones / AirPlay are honored. No-op outside `.builtInMicBtA2dp` mode (other modes pick - /// their route via category options). Must be called AFTER the session is active. - /// - /// Idempotent: skips the `overrideOutputAudioPort` call when the desired override already - /// matches the last one we successfully applied. Each call fires a `.override` route-change - /// notification, and `AudioSessionManager.recoverAudio()` invokes this on every recovery — - /// so on an AirPods disconnect, without this guard, override + recoverAudio ping-pong and - /// each iteration also rebuilds the AVAudioEngine (the audible reinitialize loop). The cache - /// is reset to `nil` at the top of `applyConfiguration()` (setCategory can reset the - /// override) and on a failed call (so the next attempt re-derives from the live session). + /// Uses the speaker only when an A2DP-capable preset has no external output. + /// The cached override avoids recursively generated route-change notifications. func applyA2dpSpeakerFallback() { guard bluetoothMode == .builtInMicBtA2dp else { return } let session = AVAudioSession.sharedInstance() diff --git a/clients/apple/iOS/VoiceCatiOS/IOSVoiceProcessingEngine.swift b/clients/apple/iOS/VoiceCatiOS/IOSVoiceProcessingEngine.swift index 51856a8..c6259c0 100644 --- a/clients/apple/iOS/VoiceCatiOS/IOSVoiceProcessingEngine.swift +++ b/clients/apple/iOS/VoiceCatiOS/IOSVoiceProcessingEngine.swift @@ -131,26 +131,9 @@ final class IOSAudioEngine { private var micStreamId: UInt32 = 0 private var captureChannels: UInt32 = 1 - // Mic feed pacing. The core sends each captured frame SYNCHRONOUSLY as it arrives - // (on_capture_frame → encode → sendto, client.cpp) — there is no send pacer in the core. On - // desktop miniaudio capture fires one 960-sample frame every 20 ms, so packets leave at a - // steady 20 ms. On iOS the AVAudioEngine input tap fires at the hardware IO-buffer period - // (often ~40 ms under VPIO), delivering ~2 frames at once: feeding those straight to the core - // bursts 2 packets out then goes quiet for ~40 ms, and the receiver's ~40 ms jitter buffer - // underruns on every gap → PLC fade ("talking through a slow fan" + ~40–60 ms flutter). - // - // Fix: pace the feed to a steady 20 ms. The tap converts to int16 and writes to a lock-free - // SPSC ring (producer, audio clock); a 20 ms timer releases ONE 960-sample frame per tick to - // feedPcm (consumer). The producer's average rate is locked to 48 kHz = exactly one frame per - // 20 ms, so it matches the consumer; the ring just absorbs the tap's 2-at-a-time bursts. - // - // Two correctness rules learned the hard way (these caused the earlier crackle + octave): - // 1. NEVER read a partial frame — `read` consumes whatever it returns, so reading <960 would - // silently discard those samples (crackle). The timer checks `availableSamples` first and - // only reads when a full frame is present; an underrun just skips the tick (nothing lost). - // 2. NEVER freeze the channel count in the timer — mono↔stereo preset switches change it. The - // timer is torn down and recreated inside `rebuild()`, so it always captures the current - // `captureChannels`; the ring is reset while the timer is stopped (no cross-thread race). + // AVAudioEngine may deliver several codec frames per callback. Pace complete 20 ms frames + // through an SPSC ring; never consume partial frames, and recreate the timer when the channel + // count changes. private let micRing = PCMRing(capacitySamples: 48000 * 2) // ~1 s stereo — ample elastic slack private var micTimer: DispatchSourceTimer? private let micQueue = DispatchQueue(label: "cat.voice.mic.feedPump") @@ -314,8 +297,7 @@ final class IOSAudioEngine { engine.inputNode.isVoiceProcessingAGCEnabled = IOSAudioRouter.shared.agcEnabled } - // (Re)build the playback source node AFTER the VPIO state is set, so it connects against the - // correct (voice-processed or plain) output unit — mirrors the proven original ordering. + // The source node must bind to the selected voice-processing output unit. rebuildSourceNode() if micActive { installMicTap() } @@ -331,11 +313,7 @@ final class IOSAudioEngine { inFormat=\(inFmt) outputNode=\(outFmt) outputRoute=[\(route)] """) } catch { - // iOS occasionally refuses to start the engine immediately after a route change — - // the AVAudioSession needs a re-activation nudge before the engine will start. Do - // ONE recovery attempt: re-activate the session, re-apply the route config, then - // try `engine.start()` again. Recovering here is what fixes the silent-death bug - // where unplugging headphones left the engine stopped forever. + // Route changes can leave AVAudioSession inactive; retry once after reactivation. logger.error("engine start failed: \(error.localizedDescription) — attempting one-shot recovery") do { try AudioSessionManager.shared.ensureSessionActive() @@ -476,9 +454,7 @@ final class IOSAudioEngine { if frames < state.targetFrames { return } // still filling the cushion (into silence) state.primed = true } else if frames == 0 { - // Underrun: the cushion drained. Grow it (capped) so it won't recur, then re-prime. - // Never read a partial frame — `read` consumes what it returns, so that would - // discard samples (the old crackle bug); skipping loses nothing, the samples wait. + // Re-prime with a larger cushion; consuming a partial frame would lose samples. if state.targetFrames < PumpState.maxTargetFrames { state.targetFrames += 1 } state.primed = false return diff --git a/clients/apple/macOS/VoiceCatMac/Audio/ScreenAudioCapture.swift b/clients/apple/macOS/VoiceCatMac/Audio/ScreenAudioCapture.swift index 3559603..cc5a3c2 100644 --- a/clients/apple/macOS/VoiceCatMac/Audio/ScreenAudioCapture.swift +++ b/clients/apple/macOS/VoiceCatMac/Audio/ScreenAudioCapture.swift @@ -1,14 +1,11 @@ import AVFoundation import ScreenCaptureKit -// Which apps' audio the SCREEN_AUDIO stream captures. ScreenCaptureKit filters audio at the -// *application* level (not per-window), so the selection is expressed as bundle IDs. The -// picker UI (ScreenSharePickerSheet) produces a `ScreenAudioSelection`; `start()` turns it -// into the matching `SCContentFilter`. +// ScreenCaptureKit filters audio by application bundle identifier. enum ScreenAudioScope: Equatable { - case entireDesktop // whole display — the original behaviour - case onlyApps([String]) // capture only these bundle IDs - case allExcept([String]) // capture everything except these bundle IDs + case entireDesktop + case onlyApps([String]) + case allExcept([String]) } struct ScreenAudioSelection: Equatable { @@ -20,20 +17,8 @@ struct ScreenAudioSelection: Equatable { static let `default` = ScreenAudioSelection() } -// ScreenAudioCapture — macOS system/desktop audio capture for the SCREEN_AUDIO stream. -// -// The macOS analog of the Windows WASAPI loopback path (docs/voice.md §9). ScreenCaptureKit -// (macOS 13+) captures whatever the system is playing; we convert each audio CMSampleBuffer -// (Float32) → int16 interleaved and push 20 ms frames (960 samples/channel @ 48 kHz) into the -// core via `vc_stream_feed_pcm` (exposed as `VoiceCatClient.feedPcm`). The core then runs the -// same Opus-encode → media-AEAD → UDP path as any other stream — only the *source* is -// platform-specific (architecture.md §4). -// -// Audio-only: we request a 2×2 video plane at 1 fps purely because SCStream needs a video -// configuration, and we never add a `.screen` output — only `.audio`. `excludesCurrentProcess -// Audio` prevents the self-echo loop of re-capturing our own incoming voice mix. -// -// `feedPcm` is thread-safe (any thread), so we forward straight from the sample-handler queue. +// SCStream requires a minimal video configuration even for audio-only capture. Only its audio +// output is registered, and current-process audio is excluded to prevent feedback. final class ScreenAudioCapture: NSObject, SCStreamOutput, SCStreamDelegate { /// Receives a full 20 ms frame: (interleaved int16 PCM, samplesPerChannel = 960, channels). diff --git a/clients/windows/VoiceCat.App/Audio/InputDeviceCapture.cs b/clients/windows/VoiceCat.App/Audio/InputDeviceCapture.cs index f115b2e..8fe43eb 100644 --- a/clients/windows/VoiceCat.App/Audio/InputDeviceCapture.cs +++ b/clients/windows/VoiceCat.App/Audio/InputDeviceCapture.cs @@ -1,18 +1,7 @@ using System.Runtime.InteropServices; -// WASAPI shared-mode capture from a real hardware INPUT device (a microphone / line-in / aux -// device), plus enumeration of capture endpoints for the aux-stream picker. -// -// This is the input-device analogue of ProcessLoopbackCapture (which captures *render* loopback -// via the process-loopback activation hack). Here the source is an ordinary capture endpoint, so -// we use the standard IMMDevice.Activate(IAudioClient) path with RCW interfaces — no vtable -// gymnastics needed (a normal device's COM objects honour QueryInterface). -// -// Why client-side capture at all? The core already owns ONE capture device (the mic). It can't -// open a second arbitrary input device, so for the aux stream the client captures the device and -// feeds 48 kHz / 20 ms int16 frames into the core via vc_stream_feed_pcm — the same external-feed -// pipeline screen-audio sharing uses. The device ids here are WASAPI endpoint ids and are NOT the -// core's miniaudio ids, so the aux picker is populated independently of vc_list_devices. +// Captures a second hardware input for AUX_DEVICE and feeds it through vc_stream_feed_pcm. +// Endpoint identifiers are WASAPI-specific and cannot be exchanged with the core's miniaudio ids. namespace VoiceCat.App.Audio; /// An audio input (capture) endpoint for the aux-stream device picker. diff --git a/clients/windows/VoiceCat.App/Audio/ProcessLoopbackCapture.cs b/clients/windows/VoiceCat.App/Audio/ProcessLoopbackCapture.cs index a6efd0e..ea3e148 100644 --- a/clients/windows/VoiceCat.App/Audio/ProcessLoopbackCapture.cs +++ b/clients/windows/VoiceCat.App/Audio/ProcessLoopbackCapture.cs @@ -1,17 +1,8 @@ using System.Runtime.InteropServices; -// Single-process WASAPI loopback capture via AUDIOCLIENT_ACTIVATION_PARAMS -// (Windows 10 2004+ / Build 19041+). -// -// Threading: ALL WASAPI init runs on the capture thread (MTA). If called from the -// WinForms UI thread (STA), ActivateAudioInterfaceAsync fires ActivateCompleted on -// an MTA pool thread; COM marshals that back to the STA pump — but the STA thread is -// blocked on CompletionEvent.Wait → deadlock. MTA capture thread avoids this. -// -// COM QI policy: the COM objects returned by the process-loopback activation path -// reject QueryInterface for their own IIDs under .NET's RCW mechanism. Every call -// to IAudioClient and IAudioCaptureClient is therefore dispatched via raw vtable -// pointer arithmetic, bypassing .NET COM interop entirely. +// Process loopback requires MTA activation; blocking activation from the WinForms STA +// deadlocks COM completion. The returned interfaces also reject RCW QueryInterface, so audio +// calls use explicitly owned raw pointers and vtable dispatch. namespace VoiceCat.App.Audio; public sealed class ProcessLoopbackCapture : IDisposable diff --git a/clients/windows/VoiceCat.Interop/Structs.cs b/clients/windows/VoiceCat.Interop/Structs.cs index 273c955..9269a95 100644 --- a/clients/windows/VoiceCat.Interop/Structs.cs +++ b/clients/windows/VoiceCat.Interop/Structs.cs @@ -43,8 +43,8 @@ internal struct VcCallbacksNative internal struct VcStreamDescNative { public VcStreamKind Kind; - public IntPtr DeviceId; // unused by vc_stream_start today — device selection is a - // separate vc_set_input_device call; always IntPtr.Zero here. + // Device selection uses vc_set_input_device; stream start passes null. + public IntPtr DeviceId; public IntPtr Label; // Mirrors vc_stream_desc::external_feed. When 1, the core skips its own WASAPI loopback // and the caller feeds PCM via StreamFeedPcm (per-app capture path on Windows). diff --git a/core/include/voicecat.h b/core/include/voicecat.h index 7be74f2..16801a0 100644 --- a/core/include/voicecat.h +++ b/core/include/voicecat.h @@ -132,7 +132,7 @@ typedef enum vc_event_type { * vc_confirm_server_identity. Pins the TLS leaf certificate's own SHA-256 fingerprint * (verifiable directly from the handshake), NOT the declared Ed25519 * server_identity_fingerprint from ServerHello — the TLS cert and the server's Ed25519 - * identity key are generated independently with no cryptographic binding between them today + * identity key are generated independently with no cryptographic binding between them * (docs/security.md §1.1), so pinning the self-declared value would be circular. The Ed25519 * fingerprint is still available for human-readable display via * vc_get_server_identity_display(), it just isn't the value this gate accepts/rejects on. */ diff --git a/core/src/audio/audio_engine.cpp b/core/src/audio/audio_engine.cpp index 0511423..7a954a0 100644 --- a/core/src/audio/audio_engine.cpp +++ b/core/src/audio/audio_engine.cpp @@ -632,9 +632,8 @@ void AudioEngine::on_playback(int16_t* out, ma_uint32 frames) { // the engine-wide playback channel count // dec_channels/frame_samples are bitstream properties (fixed at decoder init); `frames` // below is the *hardware* playback callback's period, an independent value miniaudio - // picks on its own — opus_decode's max_samples must be frame_samples, never `frames` - // (see RemoteStream::ring in audio_engine.h for what went wrong when it was). The ring - // decouples the two: top it up by decoding whole Opus frames, then drain exactly + // picks on its own — opus_decode's max_samples must be frame_samples, never `frames`. + // The ring decouples the two: top it up by decoding whole Opus frames, then drain exactly // `frames` samples-per-channel from it below (silence-padding on underrun = PLC). const int dec_channels = std::max(1, stream.decoder.channels()); const int frame_samples = stream.decoder.frame_samples(); diff --git a/core/src/audio/audio_engine.h b/core/src/audio/audio_engine.h index 9fe2051..f71b4d3 100644 --- a/core/src/audio/audio_engine.h +++ b/core/src/audio/audio_engine.h @@ -1,7 +1,4 @@ -/* - * audio/audio_engine.h: capture/playback + DSP + jitter buffer + mixer. - * - */ +/* Capture, playback, jitter buffering, and mixing. */ #ifndef VOICECAT_AUDIO_AUDIO_ENGINE_H #define VOICECAT_AUDIO_AUDIO_ENGINE_H @@ -394,13 +391,8 @@ class AudioEngine { std::atomic running_{false}; std::atomic output_volume_{1.0f}; - // Capture-side frame accumulators: miniaudio fires the capture (and loopback) callback at - // whatever period the hardware/driver chooses — commonly 480 samples (10 ms) on WASAPI - // shared mode, while the Opus encoder requires exactly frame_samples_ per call (960 for - // 20 ms @ 48 kHz). Accumulate incoming PCM until a full frame is ready, then call - // capture_cb_. This mirrors the RemoteStream::ring fix on the playback side. Both - // accumulators are pre-allocated once in start(); never resized from the RT callback - // thread (satisfies architecture.md §3 — no allocation on RT threads). + // Device callback periods are independent of codec frame size. These preallocated + // accumulators emit complete frames without allocating on an RT thread. struct CaptureAccum { std::vector buf; // pre-sized to frame_samples_ in start() int count = 0; @@ -428,15 +420,10 @@ class AudioEngine { float gain = 1.0f; bool mute = false; uint32_t playout_ts = 0; - // playout_ts free-runs (advances every callback via PLC), so it must be seeded from, and - // periodically re-synced to, the actual stream timeline — otherwise it drifts past the - // jitter buffer's drop window across VAD/PTT gaps and late joins and every frame is - // dropped/never-due (silent playback). false until the first frame seeds it (on_playback). + // Re-seeded from the stream timeline after late joins and transmission gaps. bool playout_started = false; - // Set by push_recv_frame when a kFlagMarker (talkspurt-start) frame arrives; consumed by - // on_playback to force an immediate playout-clock reseed at the new talkspurt, so the - // bounded-depth target is re-established cleanly across silence gaps. See on_playback. + // A talkspurt marker forces playout-clock reseeding. bool pending_marker = false; // Diagnostic: times the decode/playback ring underran (produced silence because the @@ -444,12 +431,7 @@ class AudioEngine { // "frames arriving but silent / latency starved" signal. Polled via stream_underruns(). std::atomic underruns{0}; - // PLC cap (defense-in-depth): consecutive samples produced by packet-loss - // concealment since the last real decoded frame. Reset to 0 on every real frame. - // When it exceeds kPlcCapSamples (audio_engine.cpp), on_playback stops calling - // opus_decode(nullptr,0,...) and emits silence instead — bounding the comfort-noise - // hiss to ~2 s so a stale stream can never hiss forever even if remove_stream is - // never called. See on_playback's decode loop. + // Bounds consecutive PLC output so a stale stream eventually becomes silent. int64_t plc_samples_since_real = 0; // Listener-chosen, local-only noise reduction (docs/voice.md §10). Lazily @@ -483,17 +465,8 @@ class AudioEngine { std::atomic last_voice_ms{0}; bool talking = false; - // Decode/playback decoupling ring - // opus_decode() must be called with max_samples == the encoder's fixed frame size - // (decoder.frame_samples(), e.g. 960 @ 20ms/48kHz) — that's a property of the bitstream, - // not a choice. miniaudio's playback callback period is a *separate*, independently - // chosen value (often smaller, e.g. ~480 @ low-latency WASAPI defaults) and must never - // be passed to opus_decode as max_samples (doing so made decode fail basically every - // callback — silent playback bug, fixed by this ring). on_playback() tops this ring up - // by decoding whole Opus frames (decoder's channel count) and drains exactly the - // hardware-requested sample count from it each callback, padding with silence (PLC) on - // underrun. Sized once in init_ring() (called off the audio thread); never resized from - // on_playback (real-time rule). + // Decoding uses the bitstream frame size, while playback drains the device callback + // size. This preallocated ring decouples those clocks and is never resized on the RT path. std::vector ring; // capacity = (frame_samples * 8) frames * ring_channels size_t ring_channels = 1; size_t ring_head = 0; // next frame (sample-per-channel) to read diff --git a/core/src/core/client.cpp b/core/src/core/client.cpp index eac3548..db6a703 100644 --- a/core/src/core/client.cpp +++ b/core/src/core/client.cpp @@ -950,9 +950,7 @@ int64_t client_now_ms() { .count(); } -// Builds an OpusParams from a wire AudioConfig, applying the same field-by-field mapping on -// both the send (local-stream encoder) and receive (remote-stream decoder) paths — fixes a -// gap where mode/dtx/complexity/application were silently dropped. +// Maps the complete wire AudioConfig for both local encoders and remote decoders. voicecat::codec::OpusParams opus_params_from_audio_config(const voicecat::v1::AudioConfig& a) { voicecat::codec::OpusParams p; // Opus always runs at 48 kHz internally: the whole AudioEngine clock is diff --git a/core/src/net/transport.cpp b/core/src/net/transport.cpp index a8dad8f..99c594f 100644 --- a/core/src/net/transport.cpp +++ b/core/src/net/transport.cpp @@ -333,9 +333,8 @@ void TcpServerConn::wait_closed() { namespace { -// Try IPv6 dual-stack first (one socket handles both ::1 and 127.0.0.1 — fixes the common -// Windows case where `localhost` resolves to ::1 before 127.0.0.1). Falls back to IPv4-only -// if the OS has IPv6 disabled or the dual-stack bind fails for any reason. +// Prefer IPv6 dual-stack so one listener accepts both IPv6 and IPv4 localhost addresses. +// Fall back to IPv4 when dual-stack binding is unavailable. asio::ip::tcp::acceptor make_acceptor(asio::io_context& io, uint16_t port) { asio::ip::tcp::acceptor acc(io); std::error_code ec; diff --git a/docs/protocol.md b/docs/protocol.md index e542383..a9716ee 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -314,6 +314,13 @@ message TextMessage { it before closing the socket. `code = 0` is reserved for client-initiated graceful disconnect; server-sent fatal `Disconnect` uses `code ≥ 1` (1 = protocol error, 2 = kicked). +- **Client reconnection is local policy.** The core reports transport loss but does not reconnect. + The iOS client snapshots the server, channel, voice subscription, mute, and deafen state, then + reconnects with exponential backoff capped at 30 seconds. `NWPathMonitor` proactively replaces + a live session when the active interface changes or becomes unavailable, avoiding the TCP + keepalive delay; same-interface refreshes are ignored. A user-initiated disconnect cancels the + retry task and path monitor. After authentication, the client rejoins the prior channel before + restoring voice and local mute/deafen state. ## 8. Client-local features (no protocol changes) diff --git a/docs/voice.md b/docs/voice.md index a059e2d..214617e 100644 --- a/docs/voice.md +++ b/docs/voice.md @@ -281,6 +281,21 @@ Each receiver keeps an **adaptive jitter buffer per ssrc** with **bounded-depth and **Advanced** (every knob manual). A2DP output requires an internal-mic preset (the Bluetooth device is output-only); the Stereo/Mono Mic presets fall back to the built-in speaker when no external output is connected (`applyA2dpSpeakerFallback`). +- **iOS implementation invariants:** + - External playback is enabled before connecting because authentication can start the audio + engine before the UI receives another turn. + - AVAudioEngine callbacks may contain multiple codec frames. The mic path writes them to an + SPSC ring and releases complete 20 ms frames at a steady cadence; it never consumes a partial + frame, and the pacer is recreated when mono/stereo capture changes. + - `AVAudioSession` setters can synchronously emit route-change notifications. Configuration is + re-entrancy guarded, and recovery ignores `.categoryChange`, `.routeConfigurationChange`, and + `.override` because those reasons are generated by the app's own routing calls. External route + changes and engine-configuration notifications still rebuild the graph. + - Stereo capture anchors the built-in mic's stereo data source. It does not call + `setPreferredInputNumberOfChannels(2)`, which can disrupt A2DP output; the core receives the + channel count through `vc_set_capture_channels`. + - The A2DP speaker fallback caches its last output override. Reapplying the same override would + emit another `.override` notification and recursively trigger recovery. - **DSP engine: see §11.** The original plan was `webrtc-audio-processing` (AEC + NS + AGC + VAD in one tuned module, BSD-licensed) — but it has no working Windows/MSVC build upstream (confirmed via its own issue tracker: GCC-only Meson build, MinGW support unfinished, hard diff --git a/server/src/conn_session.cpp b/server/src/conn_session.cpp index 9c85b54..de9e44e 100644 --- a/server/src/conn_session.cpp +++ b/server/src/conn_session.cpp @@ -186,15 +186,8 @@ void ConnSession::close() { state_.store(State::Disconnecting, std::memory_order_release); uint32_t uid = user_id_.load(); if (uid) { - // Broadcast LEFT BEFORE erasing the user, so remaining clients (and the audio - // engine's remove_stream path on each peer) learn about the departure. This - // covers ungraceful disconnects (TCP drop, crash, network loss) that previously - // silently erased the user from the registry without notifying anyone — which - // left stale users in peer client lists and kept Opus PLC hissing forever on - // peers whose remove_stream was never triggered. Mirrors kick_user's first half - // (session_registry.cpp kick_user). broadcast_left releases its shared_lock - // before remove_user acquires the unique_lock, so no deadlock; and send_envelope - // on this session is a no-op now that closed_ is true. + // Broadcast before erasing so peers can remove the user's streams. broadcast_left + // releases its shared lock before remove_user acquires the unique lock. registry_->broadcast_left(uid, ""); registry_->remove_user(uid); } diff --git a/server/src/main.cpp b/server/src/main.cpp index a5f932f..d1d74c8 100644 --- a/server/src/main.cpp +++ b/server/src/main.cpp @@ -36,7 +36,7 @@ int main(int argc, char** argv) { // launcher redirects stdout to a pipe, as the C# interop smoke test's Process does to // read the bound port) — go unbuffered so the startup banner (incl. "TCP :") is // visible immediately instead of sitting in the CRT's buffer until it fills or the - // process exits. Same fix as tools/vccli/src/main.cpp. + // process exits. std::setvbuf(stdout, nullptr, _IONBF, 0); voicecat::server::Config cfg; diff --git a/server/src/server.cpp b/server/src/server.cpp index e77b829..f4e5f2f 100644 --- a/server/src/server.cpp +++ b/server/src/server.cpp @@ -169,12 +169,8 @@ int Server::run() { }); // ── Keepalive reaper (docs/protocol.md §7) ───────────────────────────────── - // Sweeps every reaper_sweep_ms and drops any session whose last_seen is older than - // reaper_timeout_ms. Each close() broadcasts UserEvent::LEFT via the Tier 1 fix, so - // peers learn about the timeout exactly like a normal disconnect — their audio engines - // call remove_stream and stop PLC. This catches half-open connections (NAT timeout, - // wifi loss without RST, laptop sleep) that never produce a TCP EOF and would otherwise - // leave ghost users forever. Disabled when reaper_timeout_ms <= 0. + // Drops half-open sessions and broadcasts LEFT so peers remove their streams. + // Disabled when reaper_timeout_ms <= 0. asio::steady_timer reaper_timer(io); std::function arm_reaper; if (cfg_.reaper_timeout_ms > 0) { diff --git a/server/src/session_registry.h b/server/src/session_registry.h index dfb0d73..dc65942 100644 --- a/server/src/session_registry.h +++ b/server/src/session_registry.h @@ -1,10 +1,4 @@ -/* - * server/session_registry.h — In-memory session, channel, and user registry. - * - * Tracks all authenticated sessions, the channel tree, user<→>channel assignments, - * UDP endpoint bindings, and SSRC<→>session mappings. - * Protected by a shared_mutex (many readers, few writers). All methods are thread-safe. - */ +/* Thread-safe in-memory session, channel, user, and media registry. */ #ifndef VOICECAT_SERVER_SESSION_REGISTRY_H #define VOICECAT_SERVER_SESSION_REGISTRY_H @@ -49,19 +43,14 @@ class SessionRegistry { public: explicit SessionRegistry(std::shared_ptr db); - // Load channels from the database, seeding defaults on first run. void load_channels(); - // Register a session (before auth). Returns the assigned session_id. uint64_t register_session(std::weak_ptr session); - // Remove a session (called on disconnect). void unregister_session(uint64_t session_id); - // Add a user once authenticated. Returns the assigned user_id. uint32_t add_user(uint64_t session_id, const voicecat::v1::User& user); - // Remove a user (called on disconnect after auth). void remove_user(uint32_t user_id); // Broadcast a UserEvent::LEFT for a user to all other sessions. Called by @@ -70,29 +59,21 @@ class SessionRegistry { // kick_user(). Takes the shared lock internally; safe to call from ConnSession::close. void broadcast_left(uint32_t user_id, const std::string& reason); - // Move a user to a channel. Returns false if channel doesn't exist. bool set_user_channel(uint32_t user_id, uint32_t channel_id); - // Set the user's voice-plane subscription flag on their proto (broadcast-ready). void set_user_voice_subscribed(uint32_t user_id, bool subscribed); - // Snapshot for ServerStateSnapshot message. std::vector channel_snapshot() const; std::vector user_snapshot() const; std::optional user_snapshot_user(uint32_t user_id) const; std::optional user_nickname(uint32_t user_id) const; - // Resolve target sessions for a text message relay. std::vector> resolve_text_targets( uint64_t sender_session_id, voicecat::v1::TextScope scope, uint32_t target_id) const; - // Broadcast an envelope to all sessions except the excluded one. void broadcast(const voicecat::v1::Envelope& env, uint64_t exclude_session_id = 0) const; - // Return all sessions whose last_seen is older than max_age_ms (steady_clock ms), i.e. - // have not had any inbound TCP or UDP activity in that span. The reaper (server.cpp) - // calls close() on each — which broadcasts UserEvent::LEFT via the Tier 1 fix. Locks - // only to collect the list; close() runs outside the lock (mirrors kick_user's pattern). + // The caller closes returned sessions outside the registry lock because close re-enters it. std::vector> find_stale_sessions(int64_t max_age_ms) const; private: @@ -100,65 +81,42 @@ class SessionRegistry { uint64_t exclude_session_id = 0) const; public: - // ── Permissions ──────────────────────────────────────────────────────────── - void set_session_permissions(uint64_t session_id, const voicecat::v1::Permissions& perms); std::optional get_session_permissions( uint64_t session_id) const; - // ── Moderation ───────────────────────────────────────────────────────────── - - // Find a live session by its user_id. Returns nullptr if offline. std::shared_ptr find_session_by_user_id(uint32_t user_id) const; - // Forcibly disconnect a user with a reason. Broadcasts UserEvent::LEFT. - // Returns true if the user was online. bool kick_user(uint32_t user_id, const std::string& reason); - // Kick a user and insert a persistent ban. Returns true if the user was online. bool ban_user(uint32_t user_id, const std::string& reason, int64_t expires_at); - // Set server-mute/deafen flags on a user and broadcast the update. bool set_server_mute(uint32_t user_id, bool muted, bool deafened); - // Move a user to a channel (permission-checked by caller). bool move_user(uint32_t user_id, uint32_t channel_id); - // ── Channel CRUD ─────────────────────────────────────────────────────────── - - // Create a channel. Returns the new channel id, or 0 on error. uint32_t create_channel(const voicecat::v1::Channel& ch, const std::string& password, std::string& error); - // Update a channel. Returns false on error. bool update_channel(const voicecat::v1::Channel& ch, const std::string& password, std::string& error); - // Delete a channel. Remaining users are moved to Lobby (id=1). Returns false on error. + // Deleting a channel moves its users to Lobby. bool delete_channel(uint32_t channel_id, std::string& error); - // Return a channel proto by id, or nullopt. std::optional get_channel(uint32_t channel_id) const; - // Check a channel password. bool check_channel_password(uint32_t channel_id, const std::string& password) const; - // ── UDP / media ──────────────────────────────────────────────────────────── - - // Register a session's UDP token (called at auth success). void register_udp_token(const std::array& token, uint64_t session_id); - // Locate a session by its UDP binding token (called by MediaRelay on UDP_BINDING). std::shared_ptr find_by_udp_token(const std::array& token) const; - // Associate a UDP endpoint with a session (called by MediaRelay after token verification). void register_udp_endpoint(asio::ip::udp::endpoint ep, uint64_t session_id); - // Locate the session that owns a UDP sender endpoint (called per incoming voice packet). std::shared_ptr find_by_udp_endpoint(const asio::ip::udp::endpoint& ep) const; - // Assign an SSRC for a new stream. Returns the assigned SSRC. uint32_t assign_ssrc(uint64_t session_id); // Add/replace a stream entry on a user (called when StreamAnnounce succeeds). @@ -170,17 +128,12 @@ class SessionRegistry { // User proto for broadcasting, or nullopt if user not found. std::optional clear_user_stream(uint32_t user_id, uint32_t stream_id); - // Get all sessions in a channel except the one excluded (for SFU relay). std::vector> find_channel_sessions( uint32_t channel_id, uint64_t exclude_session_id = 0) const; - // Return the channel_id of a user (0 if not found). uint32_t user_channel(uint32_t user_id) const; - // Return a channel's authoritative AudioConfig (per-channel Opus tuning), or nullopt - // if the channel doesn't exist. There is no per-id Channel getter today otherwise — - // channel_snapshot() copies every channel, which callers needing just one config should - // avoid. + // Returns the authoritative per-channel Opus configuration. std::optional channel_audio_config(uint32_t channel_id) const; private: