diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/Amethyst.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/Amethyst.kt index 8e488a83a..99703e822 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/Amethyst.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/Amethyst.kt @@ -43,6 +43,12 @@ class Amethyst : Application() { if (isDebug) { Logging.setup() + // Auto-enable the Nests session-trace recorder in debug + // builds so two-phone repros can be captured via + // adb logcat -s NestsTraceJsonl:D -v raw > nest-trace.jsonl + // without rebuilding to flip a flag. Off in release. + com.vitorpamplona.nestsclient.trace.NestsTrace + .setRecording(true) } instance.initiate(this) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/lifecycle/NestViewModelFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/lifecycle/NestViewModelFactory.kt index e5d25b475..b1389a5c7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/lifecycle/NestViewModelFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/lifecycle/NestViewModelFactory.kt @@ -53,8 +53,12 @@ internal class NestViewModelFactory( // `httpClient` slot rather than as the first positional arg. httpClient = OkHttpNestsClient(httpClient = Amethyst.instance.roleBasedHttpClientBuilder::okHttpClientForVideo), transport = QuicWebTransportFactory(), - decoderFactory = { MediaCodecOpusDecoder() }, - playerFactory = { AudioTrackPlayer() }, + decoderFactory = { channelCount, sampleRate -> + MediaCodecOpusDecoder(channelCount = channelCount, sampleRate = sampleRate) + }, + playerFactory = { channelCount, sampleRate -> + AudioTrackPlayer(channelCount = channelCount, sampleRate = sampleRate) + }, signer = signer, room = room, captureFactory = { AudioRecordCapture() }, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/screen/NestActionBar.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/screen/NestActionBar.kt index 4270f79c2..bd7ad6081 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/screen/NestActionBar.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/room/screen/NestActionBar.kt @@ -50,6 +50,7 @@ import androidx.compose.material3.OutlinedButton import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.saveable.rememberSaveable @@ -270,8 +271,11 @@ private fun OnStageControls( is BroadcastUiState.Failed -> { // Reason is shown in the status strip; this button retries. + // Same auto-muted semantics as the idle path: the retry + // should bring the publisher back up without opening the + // mic, then the user can unmute deliberately. TalkButton( - onClick = { viewModel.startBroadcast(speakerPubkeyHex) }, + onClick = { viewModel.startBroadcast(speakerPubkeyHex, initialMuted = true) }, contentDescription = stringRes(R.string.nest_talk), ) LeaveStageButton(onClick = { viewModel.setOnStage(false) }) @@ -289,17 +293,43 @@ private fun OnStageIdleControls( val permissionLauncher = rememberLauncherForActivityResult(ActivityResultContracts.RequestPermission()) { granted -> permissionDenied = !granted - if (granted) viewModel.startBroadcast(speakerPubkeyHex) + // Permission grant on a Talk-button tap should start a + // *muted* broadcast — auto-start semantics. The host + // taps Talk to claim the on-stage slot; unmute is a + // separate, deliberate action below. + if (granted) viewModel.startBroadcast(speakerPubkeyHex, initialMuted = true) } // The launcher callback never fires for Settings-deep-link grants, so // re-check on every recomposition to auto-clear the warning. val showDenialWarning = permissionDenied && !context.hasMicPermission() + // Auto-start the muted broadcast as soon as the host (or any + // on-stage speaker) lands on this composable WITH mic permission + // already granted. Without this, a host who creates a room sees + // a "Connecting..." spinner over their avatar until they tap Talk + // — the publisher session only opens on tap, so listeners can't + // subscribe yet, and the `connectingSpeakers` overlay sticks. By + // pre-opening the publisher in muted state here, listeners get + // an immediate Active announce and the host's avatar leaves the + // spinner state on its own. Unmute is still a separate tap on + // the mic toggle that appears once we transition to + // `BroadcastUiState.Broadcasting(isMuted = true)`. + // + // Permission-denied case is intentionally left to the manual + // Talk-button tap below: we don't want a host to be auto-prompted + // for the mic the instant they enter the room. Tapping Talk is + // the consent gesture that triggers `permissionLauncher`. + LaunchedEffect(speakerPubkeyHex) { + if (context.hasMicPermission()) { + viewModel.startBroadcast(speakerPubkeyHex, initialMuted = true) + } + } + TalkButton( onClick = { if (context.hasMicPermission()) { - viewModel.startBroadcast(speakerPubkeyHex) + viewModel.startBroadcast(speakerPubkeyHex, initialMuted = true) } else { permissionLauncher.launch(Manifest.permission.RECORD_AUDIO) } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsTest.kt index ff7ad1fd6..92384ab57 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/okhttp/SurgeDnsTest.kt @@ -141,16 +141,19 @@ class SurgeDnsTest { responses.get() } val syncRefresh = Executor { it.run() } + // 100ms TTL gives both branches enough headroom on slow CI: Thread.sleep(150) reliably + // expires the first entry, and the post-refresh entry can't expire between the two + // assertions below (microseconds apart on any sane runner). val dns = SurgeDns( delegate = upstream, - positiveTtlMs = 1, + positiveTtlMs = 100, positiveTtlJitterMs = 0, refreshExecutor = syncRefresh, ) assertEquals(listOf(ip("1.2.3.4")), dns.lookup("a.example")) - Thread.sleep(20) + Thread.sleep(150) // Upstream now returns a new IP. The next lookup should still serve the OLD IP // immediately, while the synchronous executor performs the refresh inline. diff --git a/commons/plans/2026-05-06-nest-subscription-manager-extraction.md b/commons/plans/2026-05-06-nest-subscription-manager-extraction.md new file mode 100644 index 000000000..61883b034 --- /dev/null +++ b/commons/plans/2026-05-06-nest-subscription-manager-extraction.md @@ -0,0 +1,109 @@ +# Extract `NestSubscriptionManager` from `NestViewModel` + +**Status**: deferred — flagged in the audit pass, not landed yet. + +## Why + +`NestViewModel.kt` is 2112 lines and growing. ~1000 of those lines are +the per-speaker subscription-lifecycle state machine: open / close / +reconcile / catalog-fetch / decoder-await / level-tap / mute-routing / +hush. The rest is room-level public API (connect, disconnect, +broadcast, presence, reactions, focus / network observers, UI state). + +These two concerns are coupled by shared mutable state but do not +share a *responsibility*. The audit-9 finding flagged it as the +single biggest SRP breach in the touched code. + +We extracted `ActiveSubscription` into its own file (`ActiveSubscription.kt`) +in commit `` as a stepping stone — the deferred extraction +proper is the orchestration class. + +## Target shape + +```kotlin +internal class NestSubscriptionManager( + private val viewModelScope: CoroutineScope, + private val signer: NostrSigner, + private val decoderFactory: (channelCount: Int, sampleRate: Int) -> OpusDecoder, + private val playerFactory: (channelCount: Int, sampleRate: Int) -> AudioPlayer, + private val onActiveSpeakersChanged: (Set) -> Unit, + private val onSpeakerActivity: (String) -> Unit, + private val onAudioLevel: (String, Float) -> Unit, + private val onConnectingSpeakerChanged: (pubkey: String, connecting: Boolean) -> Unit, + private val effectiveListenMuted: () -> Boolean, + private val locallyHushed: () -> Set, +) { + val speakerCatalogs: StateFlow> + val audioLevels: StateFlow> + + fun bind(listener: NestsListener) + fun unbind() // cancel everything; release native resources + fun updateSpeakers(requested: Set) + fun applyEffectiveMute() + fun setLocalHushed(pubkey: String, hushed: Boolean) +} +``` + +## State that moves + +From `NestViewModel`: +- `activeSubscriptions: MutableMap` +- `catalogJobs: MutableMap` +- `_speakerCatalogs: MutableStateFlow>` +- `requestedSpeakers: Set` +- `speakingExpiryJobs: MutableMap` (for `onSpeakerActivity` debounce) +- `_audioLevels` if separable + +## Methods that move + +- `reconcileSubscriptions` +- `openSubscription` (~150 lines) +- `closeSubscription` +- `fetchSpeakerCatalog` +- `awaitAudioPipelineConfig` +- `onSpeakerActivity` debounce timer +- `onAudioLevel` coalescing +- `applyEffectiveListenMute`'s subscription-touching half +- `setLocalHushed`'s player.setVolume half +- `publishActiveSpeakers` + +## What stays in `NestViewModel` + +- Public lifecycle (`connect`, `disconnect`, `onCleared`) +- Connection state machine (`ConnectionUiState`) +- Broadcast state (`broadcast` / `_uiState.broadcast`) +- Presence aggregation (kind 10312 events) +- Reactions +- Focus / network observers +- The `NestUiState` composition itself + +`NestViewModel` calls `manager.bind(listener)` on each fresh +listener and `manager.unbind()` on disconnect / cleanup. +`updateSpeakers` and mute / hush propagate through to the manager. + +## Why deferred + +- 1000-line code move across ~10 methods + 5 state fields +- Subtle coupling: catalog readiness affects spinner state, mute + state has effective + per-speaker flavours, expiry jobs need + parent scope, `closed` flag is currently a single VM-wide flag +- Test surface: `NestViewModelTest` exercises subscription paths + through the VM today; would need to either keep that surface or + add a `NestSubscriptionManagerTest`. + +The extraction has a clean contract (callbacks for the bits that +remain VM-side) but landing it without behavioural drift wants a +focused review pass plus its own dedicated test rebuild — bigger +than fits in the current audit-pass. + +## When to land + +After: +- The catalog-driven decoder reconfig (T3) has run in production + for long enough that the `awaitAudioPipelineConfig` pattern is + proven against real publishers. +- Any pending subscription-lifecycle bug fixes (e.g. the audit's + earlier "boundary-rebuild dangling decoder" path) are settled — + moving them mid-fix risks introducing regressions. + +Track as a follow-up issue rather than a near-term must-do. diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/ActiveSubscription.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/ActiveSubscription.kt new file mode 100644 index 000000000..db0a0bae3 --- /dev/null +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/ActiveSubscription.kt @@ -0,0 +1,91 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.viewmodels + +import com.vitorpamplona.nestsclient.audio.AudioPlayer +import com.vitorpamplona.nestsclient.audio.NestPlayer +import com.vitorpamplona.nestsclient.moq.SubscribeHandle + +/** + * Per-pubkey state slot held in [NestViewModel.activeSubscriptions]. + * + * Three lifecycle phases: + * - **Pending**: just-reserved by `reconcileSubscriptions` to dedupe + * concurrent reconciles. No handle / player attached yet. Constructor + * via [pending]. + * - **Active**: [attach] wires the moq subscribe handle, the + * [NestPlayer] decode loop, and the device player. [isPlaying] + * flips true. + * - **Detached**: [detach] returns the handle + roomPlayer pair so the + * caller can run the suspending teardown (`NestPlayer.stop()` + + * `SubscribeHandle.unsubscribe()`) in its own coroutine scope — + * keeps the native MediaCodec / AudioTrack release ordered after + * the decode loop has unwound (audit MoQ #11/#12). + * + * `internal` because only [NestViewModel]'s subscription-lifecycle + * paths (open / close / reconcile / mute / hush) construct or mutate + * these. Lifted to a top-level type rather than a nested class so the + * file split tracks concerns: this class is "subscription state + * machine"; the rest of NestViewModel is "ViewModel public surface + + * orchestration." + */ +internal class ActiveSubscription private constructor( + val pubkey: String, +) { + private var handle: SubscribeHandle? = null + private var roomPlayer: NestPlayer? = null + var player: AudioPlayer? = null + private set + var isPlaying: Boolean = false + private set + + fun attach( + handle: SubscribeHandle, + roomPlayer: NestPlayer, + player: AudioPlayer, + ) { + this.handle = handle + this.roomPlayer = roomPlayer + this.player = player + this.isPlaying = true + } + + /** + * Hand the player + handle back to the caller's coroutine scope — + * `NestPlayer.stop()` and `SubscribeHandle.unsubscribe()` are + * both suspend, and the caller has the right scope to await them + * (so native MediaCodec/AudioTrack release runs after the decode + * loop has unwound, per audit MoQ #11/#12). + */ + fun detach(): Pair { + isPlaying = false + val p = roomPlayer + val h = handle + roomPlayer = null + handle = null + player = null + return p to h + } + + companion object { + fun pending(pubkey: String) = ActiveSubscription(pubkey) + } +} diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModel.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModel.kt index b06bd9f36..5c85b2735 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModel.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModel.kt @@ -35,16 +35,17 @@ import com.vitorpamplona.nestsclient.NestsSpeaker import com.vitorpamplona.nestsclient.NestsSpeakerState import com.vitorpamplona.nestsclient.audio.AudioCapture import com.vitorpamplona.nestsclient.audio.AudioException +import com.vitorpamplona.nestsclient.audio.AudioFormat import com.vitorpamplona.nestsclient.audio.AudioPlayer import com.vitorpamplona.nestsclient.audio.NestPlayer import com.vitorpamplona.nestsclient.audio.OpusDecoder import com.vitorpamplona.nestsclient.audio.OpusEncoder import com.vitorpamplona.nestsclient.connectReconnectingNestsListener import com.vitorpamplona.nestsclient.connectReconnectingNestsSpeaker -import com.vitorpamplona.nestsclient.moq.SubscribeHandle import com.vitorpamplona.nestsclient.transport.WebTransportFactory import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner import com.vitorpamplona.quartz.nip53LiveActivities.chat.LiveActivitiesChatMessageEvent +import com.vitorpamplona.quartz.utils.Log import kotlinx.collections.immutable.ImmutableSet import kotlinx.collections.immutable.persistentSetOf import kotlinx.collections.immutable.toPersistentSet @@ -55,9 +56,13 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeoutOrNull import kotlin.coroutines.cancellation.CancellationException /** @@ -80,7 +85,12 @@ import kotlin.coroutines.cancellation.CancellationException * Audio-pipeline construction is injected via [decoderFactory] / * [playerFactory] so commonMain doesn't have to know which platform's * MediaCodec / AudioTrack is in play. M1 wires Android-only — desktop - * passes nothing here yet. + * passes nothing here yet. Both factories take a `channelCount` (1 for + * mono, 2 for stereo) AND a `sampleRate` (Hz) discovered from the + * publisher's `catalog.json` audio rendition; subscriptions await a + * brief catalog-arrival window before constructing the decoder + player + * so a stereo or non-48 kHz web publisher doesn't get its frames + * decoded with the wrong channel layout / clock. * * **Threading contract:** all public methods (`connect`, `disconnect`, * `updateSpeakers`, `setMuted`, `setMicMuted`, `startBroadcast`, @@ -97,8 +107,8 @@ import kotlin.coroutines.cancellation.CancellationException class NestViewModel( private val httpClient: NestsClient, private val transport: WebTransportFactory, - private val decoderFactory: () -> OpusDecoder, - private val playerFactory: () -> AudioPlayer, + private val decoderFactory: (channelCount: Int, sampleRate: Int) -> OpusDecoder, + private val playerFactory: (channelCount: Int, sampleRate: Int) -> AudioPlayer, private val signer: NostrSigner, private val room: NestsRoomConfig, // Speaker-side audio capture/encode actuals. Optional — desktop and @@ -265,6 +275,41 @@ class NestViewModel( private val activeSubscriptions = mutableMapOf() private val speakingExpiryJobs = mutableMapOf() + /** + * Monotonic [kotlin.time.TimeMark] of the last MoQ object delivered + * to the consumer flow per speaker pubkey. Updated in + * [onSpeakerActivity] alongside the speaking-now flag. Read by + * [cliffDetectorJob] to spot the relay-side forward-queue cliff + * documented in + * `nestsClient/plans/2026-05-01-quic-stream-cliff-investigation.md`: + * the relay stops opening new uni streams to the listener while + * the announce stream still says the broadcast is Active and the + * subscribe bidi is still open from QUIC's POV — meaning no other + * layer notices. + */ + private val lastFrameAt = mutableMapOf() + + /** + * Periodic scan of [activeSubscriptions] vs [_announcedSpeakers] + * vs [lastFrameAt]. When a speaker has been announced Active + * AND we have an active subscription AND no frames have arrived + * for [ROOM_AUDIO_CLIFF_TIMEOUT_MS], force a `recycleSession()` + * so the orchestrator opens a fresh QUIC transport — a clean + * recovery from the relay-side per-subscriber forward-queue + * starvation that has no QUIC- or moq-lite-level signal. + */ + private var cliffDetectorJob: Job? = null + + /** + * [kotlin.time.TimeMark] of the most recent recycle the cliff + * detector triggered, or `null` if it has never fired. Acts as a + * cooldown so a single cliff event doesn't kick off multiple + * recycles back-to-back while the wrapper is mid-handshake on + * the new session — the new session has no incoming frames yet, + * which would otherwise trip the detector again immediately. + */ + private var lastCliffRecycleAt: kotlin.time.TimeMark? = null + /** * Per-speaker catalog-fetch coroutines. Each entry is the * background `subscribeCatalog` collector launched in @@ -561,7 +606,10 @@ class NestViewModel( * session over a separate WebTransport — nests' protocol uses one * session per direction. */ - fun startBroadcast(speakerPubkeyHex: String) { + fun startBroadcast( + speakerPubkeyHex: String, + initialMuted: Boolean = false, + ) { if (closed || !canBroadcast) return if (_uiState.value.connection !is ConnectionUiState.Connected) return if (_uiState.value.broadcast is BroadcastUiState.Broadcasting || @@ -608,7 +656,26 @@ class NestViewModel( }, ) broadcastHandle = handle - _uiState.update { it.copy(broadcast = BroadcastUiState.Broadcasting(isMuted = false)) } + // Auto-start path (room creation): the host wants + // listeners to be able to subscribe BEFORE they hit + // unmute, so we open the publisher session pre-muted. + // The mic stays open + the broadcaster keeps + // capturing + encoding, but `setMuted(true)` makes + // every send drop on the floor inside + // `NestMoqLiteBroadcaster.start`'s `if (muted) continue` + // gate. Listener-side announces fire as soon as the + // publisher registers with the relay, so the audience + // can subscribe immediately and switch from + // "Connecting…" to "Live (silent)" while they wait + // for the host's first word. Composes with `focusMuted` + // exactly the same way [setMicMuted] does so an + // inbound phone call during the auto-start window + // doesn't get a silent unmute. + if (initialMuted) { + val effective = initialMuted || focusMuted + runCatching { handle.setMuted(effective) } + } + _uiState.update { it.copy(broadcast = BroadcastUiState.Broadcasting(isMuted = initialMuted)) } } catch (ce: CancellationException) { throw ce } catch (t: Throwable) { @@ -763,15 +830,31 @@ class NestViewModel( announcesJob?.cancel() announcesJob = viewModelScope.launch { - runCatching { - l.announces().collect { ann -> - if (closed) return@collect - if (ann.active) { - _announcedSpeakers.update { it + ann.pubkey } - } else { - _announcedSpeakers.update { it - ann.pubkey } + com.vitorpamplona.quartz.utils.Log + .d("NestRx") { "observeAnnounces launching collect on l.announces()" } + var emissionCount = 0 + val outcome = + runCatching { + l.announces().collect { ann -> + if (closed) return@collect + emissionCount += 1 + com.vitorpamplona.quartz.utils.Log.d("NestRx") { + "observeAnnounces #$emissionCount active=${ann.active} pubkey='${ann.pubkey.take(12)}' → ${if (ann.active) "ADD" else "REMOVE"}" + } + com.vitorpamplona.nestsclient.trace.NestsTrace.emit("vm_observe_announce") { + "\"emission\":$emissionCount,\"active\":${ann.active}," + + "\"pubkey\":${com.vitorpamplona.nestsclient.trace.jsonStr(ann.pubkey)}" + } + if (ann.active) { + _announcedSpeakers.update { it + ann.pubkey } + } else { + _announcedSpeakers.update { it - ann.pubkey } + } } } + com.vitorpamplona.quartz.utils.Log.w("NestRx") { + val why = outcome.exceptionOrNull()?.let { "${it::class.simpleName}: ${it.message}" } ?: "naturally" + "observeAnnounces collect ENDED $why (emissions=$emissionCount, _announcedSpeakers.size=${_announcedSpeakers.value.size})" } } } @@ -794,6 +877,18 @@ class NestViewModel( // this is typically a no-op in toAdd / toRemove // — runs anyway for the first-Connected path. reconcileSubscriptions() + // Defensive restart: if the wrapper went + // through a Closed transient (e.g. the cliff + // detector itself triggered `recycleSession()`, + // which closes the inner listener and waits + // for the orchestrator to reopen), our + // [Closed] branch below cancelled the + // detector via [teardown]. We need it back + // up on the fresh session — otherwise a + // second cliff event during the same room + // sits undetected. Idempotent: returns + // immediately if a job is already active. + startCliffDetector() } is NestsListenerState.Failed -> { @@ -802,11 +897,34 @@ class NestViewModel( // wait for a manual reconnect tap. } - // Server-initiated Closed: tear down stale - // local state so any later user-driven reconnect - // starts fresh. NestsListenerState.Closed -> { - teardown(targetState = ConnectionUiState.Closed) + // Closed from the wrapper is *almost always* + // transient — the orchestrator emits Closed + // → Reconnecting → Connecting → Connected + // around every cliff-detector recycle and + // every JWT refresh. We must NOT teardown + // here: teardown cancels `cliffDetectorJob`, + // `announcesJob`, etc., AND calls + // `wrapper.close()`, which cancels the + // wrapper's orchestrator before it can + // reopen the inner listener. End result + // pre-fix: the very first cliff-detector + // recycle permanently kills the room + // instead of recovering it (visible in the + // 15:56:25 receiver log: cliff fires → + // teardown fires 521 ms later → wrapper + // never reopens → `cliff-detector EXITED + // closed=false`). + // + // User-initiated close goes through + // `disconnect()` / `onCleared()` which call + // `teardown` directly; the wrapper's + // subsequent Closed emission here is a + // redundant signal — no-op'ing it doesn't + // change anything for those paths. The UI + // still picks up the Closed via + // `state.toUiState(ui.connection)` above + // for visual feedback. } else -> { /* no extra side effect */ } @@ -855,6 +973,7 @@ class NestViewModel( listener = l observeListenerState(l) observeAnnounces(l) + startCliffDetector() } catch (ce: CancellationException) { throw ce } catch (t: Throwable) { @@ -915,6 +1034,11 @@ class NestViewModel( if (rawAudioLevels.remove(slot.pubkey) != null) { _audioLevels.value = rawAudioLevels.toMap() } + // Drop the cliff-detector heartbeat for this pubkey so the next + // tick doesn't see a stale "last frame was 30s ago" entry for a + // speaker we already stopped subscribing to and trigger a + // gratuitous recycle. + lastFrameAt.remove(slot.pubkey) if (roomPlayer != null || handle != null) { viewModelScope.launch { roomPlayer?.runCatching { stop() } @@ -923,6 +1047,92 @@ class NestViewModel( } } + /** + * Wait briefly for [pubkey]'s catalog to land in [_speakerCatalogs] + * and pick the audio config (channel count + sample rate) for the + * decoder + AudioTrack. Falls back to [AudioFormat.CHANNELS] / + * [AudioFormat.SAMPLE_RATE_HZ] on timeout (the catalog never + * arrived within [timeoutMs]) or when the catalog declares + * unsupported values (channelCount outside `1..2`, or non-positive + * sampleRate). + * + * Caller is responsible for having started [fetchSpeakerCatalog] + * first; otherwise this always times out. + * + * Why a timeout: the catalog handshake is best-effort. If the + * publisher doesn't publish a catalog (legacy publishers) or the + * relay never forwards the catalog group, audio playback should + * still proceed in the default config rather than block forever. + * 500 ms is generous — with the speaker-side + * `setOnNewSubscriber` hook the catalog group is on the wire + * within one round-trip of the SUBSCRIBE_OK, so this typically + * returns within tens of ms. + */ + private suspend fun awaitAudioPipelineConfig( + pubkey: String, + timeoutMs: Long, + ): AudioPipelineConfig { + val catalog = + withTimeoutOrNull(timeoutMs) { + _speakerCatalogs + .map { it[pubkey] } + .filterNotNull() + .first() + } + val rendition = catalog?.primaryAudio() + val declaredChannels = rendition?.numberOfChannels + val channels = + when { + declaredChannels == null -> { + AudioFormat.CHANNELS + } + + declaredChannels !in 1..2 -> { + Log.w("NestRx") { + "publisher catalog for pubkey='${pubkey.take(8)}' declares numberOfChannels=$declaredChannels " + + "(only 1 / 2 supported); falling back to mono" + } + AudioFormat.CHANNELS + } + + else -> { + declaredChannels + } + } + val declaredRate = rendition?.sampleRate + val rate = + when { + declaredRate == null -> { + AudioFormat.SAMPLE_RATE_HZ + } + + declaredRate <= 0 -> { + Log.w("NestRx") { + "publisher catalog for pubkey='${pubkey.take(8)}' declares sampleRate=$declaredRate " + + "(must be positive); falling back to ${AudioFormat.SAMPLE_RATE_HZ} Hz" + } + AudioFormat.SAMPLE_RATE_HZ + } + + else -> { + declaredRate + } + } + return AudioPipelineConfig(channelCount = channels, sampleRate = rate) + } + + /** + * Output of [awaitAudioPipelineConfig] — the validated audio + * pipeline configuration for one subscription. Internal because the + * factories' two-arg shape (`(channelCount, sampleRate)`) is what + * the public surface deals in; this struct just bundles them inside + * `openSubscription`. + */ + private data class AudioPipelineConfig( + val channelCount: Int, + val sampleRate: Int, + ) + /** * Open the speaker's `catalog.json` track in the background, parse * the first frame, and stash it in [speakerCatalogs]. Best-effort — @@ -961,7 +1171,21 @@ class NestViewModel( ) { if (closed || activeSubscriptions[pubkey] !== slot) return try { - val handle = l.subscribeSpeaker(pubkey) + // [maxLatencyMs] tells the relay our staleness tolerance: + // groups older than this should be evicted, not queued. + // With `0L` (the wire default the JS reference uses) the + // relay's per-subscriber forward queue grows unbounded + // until it cliffs — see + // `nestsClient/plans/2026-05-01-quic-stream-cliff-investigation.md`. + // 500 ms is long enough to ride out a typical relay-side + // hiccup without pruning a healthy group, short enough + // that a wedged subscriber drains its buffer before the + // queue overflows. Combined with the cliff-detector below + // that triggers `recycleSession()` when frames stop + // arriving despite the broadcast still being announced, + // this is a defense-in-depth pair against the moq-rs + // forward-queue starvation behaviour. + val handle = l.subscribeSpeaker(pubkey, maxLatencyMs = ROOM_AUDIO_MAX_LATENCY_MS) // Re-check after the suspending subscribeSpeaker — the user // may have removed this speaker via updateSpeakers / disconnected // while the SUBSCRIBE was in flight. If so, abandon the handle @@ -971,15 +1195,41 @@ class NestViewModel( viewModelScope.launch { runCatching { handle.unsubscribe() } } return } + // Start the catalog fetch BEFORE we wait for it — runs in + // parallel with the main subscribe so the round-trip overlaps + // with the audio-side handshake. Cancelled by closeSubscription. + fetchSpeakerCatalog(l, pubkey) + // Wait briefly for the catalog so we can configure the + // decoder + AudioTrack to match the publisher's actual + // channel layout. With the speaker-side emit-on-subscribe + // hook the catalog frame is on the wire within one round-trip + // of the SUBSCRIBE_OK, so this typically returns within tens + // of ms. Falls back to mono if no catalog arrives within + // [CATALOG_AWAIT_TIMEOUT_MS] — covers legacy publishers and + // the failure-mode where the relay never forwards the catalog + // group; audio still plays, just in the default config. + val audioCfg = awaitAudioPipelineConfig(pubkey, CATALOG_AWAIT_TIMEOUT_MS) // Allocate native resources (MediaCodec decoder + AudioTrack // player on Android). Both are heavy and leaky if dropped on // the floor — wrap them in a nested try so any cancellation // or throw between here and slot.attach releases them // (audit round-2 VM #7). - val decoder = decoderFactory() + // Per-subscription decoder factory closure: captures the + // catalog-derived [audioCfg] so a publisher-boundary + // decoder rebuild (see [NestPlayer]'s `decoderFactory` + // kdoc) reuses the SAME channel layout AND sample rate — + // without it, a rebuild after a cliff-recycle would + // default to mono / 48 kHz and a stereo or non-48 kHz + // publisher would silently downmix / clock-mismatch on + // the new decoder. The factory is also called for the + // initial decoder so the construction path is uniform. + val perSubscriptionDecoderFactory: () -> OpusDecoder = { + decoderFactory(audioCfg.channelCount, audioCfg.sampleRate) + } + val decoder = perSubscriptionDecoderFactory() val player = try { - playerFactory() + playerFactory(audioCfg.channelCount, audioCfg.sampleRate) } catch (t: Throwable) { runCatching { decoder.release() } throw t @@ -994,7 +1244,7 @@ class NestViewModel( val isHushed = pubkey in _uiState.value.locallyHushed val roomPlayer = NestPlayer( - decoder = decoder, + initialDecoder = decoder, player = player, scope = viewModelScope, // ~100 ms of audio buffered before the AudioTrack @@ -1004,6 +1254,14 @@ class NestViewModel( // tuned in the audio-rooms audit; see NestPlayer // kdoc for details. prerollFrames = ROOM_PLAYER_PREROLL_FRAMES, + // Trigger a decoder rebuild on every publisher + // boundary (re-issuing wrapper spliced in a new + // SUBSCRIBE → trackAlias changes). Without this, + // Opus's predictor state from the prior + // publisher's last frame is fed into the new + // publisher's first frame and produces audible + // warble at every cliff-recycle / hot-swap. + decoderFactory = perSubscriptionDecoderFactory, ) // Apply current mute + per-speaker hush state before play() // opens the device so the first frame respects them. @@ -1014,6 +1272,19 @@ class NestViewModel( // Tap the object flow to drive the speaking-now indicator before // the decoder consumes it. val instrumented = handle.objects.onEach { onSpeakerActivity(pubkey) } + // Per-subscription consecutive-decoder-error counter. Single-frame + // Opus errors are normal noise (PLC handles them), but a structural + // mismatch — wrong wire format, missing legacy-container varint + // strip, codec mismatch — fails *every* frame in a row. The + // previous behaviour was to silently swallow per-packet + // [AudioException.Kind.DecoderError] indefinitely, which made + // exactly that class of bug invisible (no audio, no log, no UI + // signal). Track the streak and surface it once per crossing so + // the next protocol regression shows up in `adb logcat | grep + // NestRx` rather than in a user bug report. + val consecutiveDecodeErrors = + java.util.concurrent.atomic + .AtomicLong(0L) roomPlayer.play( instrumented, onError = { err -> @@ -1030,7 +1301,28 @@ class NestViewModel( AudioException.Kind.DecoderError, AudioException.Kind.EncoderError, -> { - Unit + val streak = consecutiveDecodeErrors.incrementAndGet() + // 50 frames = 1 s of solid decode failures. + // Anything past that is a structural bug, not + // jitter. Log once per crossing (use exact + // equality, not modulus) so we get a single + // attention-grabbing line per stuck stream + // rather than a 50 Hz flood. + if (streak == DECODE_ERROR_STREAK_LOG_THRESHOLD) { + // Non-lambda overload here (rather than the + // allocation-free lambda one) so we preserve + // the throwable stacktrace — this fires + // exactly once per stuck-stream streak, so + // the unconditional string build is fine. + Log.w( + "NestRx", + "decoder failed $DECODE_ERROR_STREAK_LOG_THRESHOLD consecutive frames " + + "for pubkey='${pubkey.take(8)}' — likely wire-format mismatch " + + "(missing legacy-container varint strip, codec/channel-count mismatch, " + + "or corrupted Opus). last error: ${err.message}", + err.cause, + ) + } } AudioException.Kind.PlaybackFailed, @@ -1042,7 +1334,13 @@ class NestViewModel( } } }, - onLevel = { onAudioLevel(pubkey, it) }, + onLevel = { lvl -> + // A successful decode resets the error streak. + // Sample-rate-fast (50 Hz) but cheap on the + // success path: Atomic.set with no allocation. + consecutiveDecodeErrors.set(0L) + onAudioLevel(pubkey, lvl) + }, ) slot.attach(handle, roomPlayer, player) publishActiveSpeakers() @@ -1052,12 +1350,10 @@ class NestViewModel( _uiState.update { it.copy(connectingSpeakers = (it.connectingSpeakers + pubkey).toPersistentSet()) } - // Parallel catalog fetch — best-effort, doesn't gate - // audio playback. Tracked in catalogJobs; cancelled - // by closeSubscription so a removed speaker doesn't - // leave the catalog collector running on the - // wrapper's still-live re-issuing handle. - fetchSpeakerCatalog(l, pubkey) + // Catalog fetch was started earlier (before the decoder + // wait) so it could overlap with the audio-side handshake; + // its job is already in [catalogJobs] and gets cancelled + // by [closeSubscription] alongside the audio path. } catch (t: Throwable) { // Either CancellationException (scope cancelled mid-construction) // or an unexpected throw — release the half-built pipeline and @@ -1089,6 +1385,10 @@ class NestViewModel( announcesJob = null levelEmitterJob?.cancel() levelEmitterJob = null + cliffDetectorJob?.cancel() + cliffDetectorJob = null + lastFrameAt.clear() + lastCliffRecycleAt = null // Audio-focus observation only ends on the final teardown — // a transient disconnect+reconnect (user retry, room swap) // keeps the bus subscription alive so a focus loss that @@ -1175,6 +1475,14 @@ class NestViewModel( */ private fun onSpeakerActivity(pubkey: String) { if (closed) return + // Cliff-detector heartbeat: each MoQ object proves the + // listener-side relay-forward queue is still flowing for this + // speaker. The detector only acts when an announced speaker's + // last-frame timestamp ages past `ROOM_AUDIO_CLIFF_TIMEOUT_MS`, + // so an active stream resets it on every frame. + lastFrameAt[pubkey] = + kotlin.time.TimeSource.Monotonic + .markNow() speakingExpiryJobs[pubkey]?.cancel() // First frame for this subscription — clear the buffering // overlay. Subsequent frames are no-ops here. @@ -1191,6 +1499,122 @@ class NestViewModel( } } + /** + * Periodic scan that detects the relay-side forward-queue cliff: + * an announced speaker whose subscription is open but hasn't + * delivered any object in [ROOM_AUDIO_CLIFF_TIMEOUT_MS]. Triggers + * `recycleSession()` on the listener so the wrapper opens a fresh + * QUIC transport — the new session has a fresh per-subscriber + * queue on the relay's side. + * + * Idempotent on start. Stops itself when the listener is gone + * (explicit teardown signal) — no-ops on transient empty + * `activeSubscriptions` so an audience-only listener doesn't + * needlessly recycle. + */ + private fun startCliffDetector() { + if (cliffDetectorJob?.isActive == true) return + com.vitorpamplona.quartz.utils.Log + .d("NestRx") { "cliff-detector launched" } + cliffDetectorJob = + viewModelScope.launch { + var ticks = 0L + try { + while (true) { + delay(ROOM_AUDIO_CLIFF_CHECK_INTERVAL_MS) + ticks += 1 + if (closed) return@launch + val l = listener + val connState = _uiState.value.connection + if (l == null || connState !is ConnectionUiState.Connected) { + // Periodic visibility into "why isn't the cliff + // detector acting" — the receiver-side cliff + // we're chasing has been silent in production + // logs even when the wire conditions look right, + // so dump our gating state every 5 s. + if (ticks % CLIFF_DIAG_LOG_EVERY == 0L) { + com.vitorpamplona.quartz.utils.Log.d("NestRx") { + "cliff-detector tick=$ticks gated: listener=${l != null} conn=${connState::class.simpleName}" + } + } + continue + } + val activeSpeakers = + activeSubscriptions + .asSequence() + .filter { (_, slot) -> slot.isPlaying } + .map { it.key } + .toSet() + val announced = _announcedSpeakers.value + if (ticks % CLIFF_DIAG_LOG_EVERY == 0L) { + val ages = + lastFrameAt.entries.joinToString { (k, mark) -> + "${k.take(8)}=${mark.elapsedNow().inWholeMilliseconds}ms" + } + com.vitorpamplona.quartz.utils.Log.d("NestRx") { + "cliff-detector tick=$ticks active=${activeSpeakers.size} announced=${announced.size} lastFrameAges=[$ages]" + } + com.vitorpamplona.nestsclient.trace.NestsTrace.emit("cliff_tick") { + "\"tick\":$ticks," + + "\"active\":${com.vitorpamplona.nestsclient.trace.jsonArrStr(activeSpeakers)}," + + "\"announced\":${com.vitorpamplona.nestsclient.trace.jsonArrStr(announced)}," + + "\"last_frame_age_ms\":{" + + lastFrameAt.entries.joinToString { + "${com.vitorpamplona.nestsclient.trace.jsonStr(it.key)}:${it.value.elapsedNow().inWholeMilliseconds}" + } + "}" + } + } + val stalled = + computeStalledSpeakers( + activeSpeakers = activeSpeakers, + announcedSpeakers = announced, + lastFrameAt = lastFrameAt, + lastRecycleAt = lastCliffRecycleAt, + cliffTimeoutMs = ROOM_AUDIO_CLIFF_TIMEOUT_MS, + cooldownMs = ROOM_AUDIO_CLIFF_COOLDOWN_MS, + ) + if (stalled.isEmpty()) continue + com.vitorpamplona.quartz.utils.Log.w("NestRx") { + "cliff-detector: announced+subscribed but silent for ≥${ROOM_AUDIO_CLIFF_TIMEOUT_MS}ms — recycling session. stalled=$stalled" + } + com.vitorpamplona.nestsclient.trace.NestsTrace.emit("cliff_recycle") { + "\"timeout_ms\":$ROOM_AUDIO_CLIFF_TIMEOUT_MS," + + "\"stalled\":${com.vitorpamplona.nestsclient.trace.jsonArrStr(stalled)}" + } + val recycleMark = + kotlin.time.TimeSource.Monotonic + .markNow() + lastCliffRecycleAt = recycleMark + // Reset `lastFrameAt` for every stalled pubkey so + // the cliff timer starts counting from the recycle + // moment, not from the old (pre-recycle) last + // frame timestamp. Without this reset the next + // tick after the cooldown immediately re-trips — + // because `lastFrameAt[pubkey].elapsedNow()` is + // still the giant pre-recycle value plus the + // cooldown window. Production logs at commit + // ea08c43 showed exactly this: 4 recycles in + // ~30 s eventually drove the relay into a + // "subscribe stream FIN before reply" loop where + // it refused fresh subscribes entirely. Using + // the recycle moment as a synthetic frame event + // gives the new session [ROOM_AUDIO_CLIFF_TIMEOUT_MS] + // wall-clock to deliver before the next cliff + // check, matching the expectation a freshly- + // attached subscription should clear inside + // that window if the relay is healthy. + for (pubkey in stalled) { + lastFrameAt[pubkey] = recycleMark + } + runCatching { l.recycleSession() } + } + } finally { + com.vitorpamplona.quartz.utils.Log + .w("NestRx") { "cliff-detector EXITED after $ticks ticks (closed=$closed)" } + } + } + } + private fun clearSpeaking(pubkey: String) { speakingExpiryJobs.remove(pubkey) if (_uiState.value.speakingNow.contains(pubkey)) { @@ -1247,48 +1671,8 @@ class NestViewModel( } } - private class ActiveSubscription private constructor( - val pubkey: String, - ) { - private var handle: SubscribeHandle? = null - private var roomPlayer: NestPlayer? = null - var player: AudioPlayer? = null - private set - var isPlaying: Boolean = false - private set - - fun attach( - handle: SubscribeHandle, - roomPlayer: NestPlayer, - player: AudioPlayer, - ) { - this.handle = handle - this.roomPlayer = roomPlayer - this.player = player - this.isPlaying = true - } - - /** - * Hand the player + handle back to the caller's coroutine scope — - * `NestPlayer.stop()` and `SubscribeHandle.unsubscribe()` are - * both suspend, and the caller has the right scope to await them - * (so native MediaCodec/AudioTrack release runs after the decode - * loop has unwound, per audit MoQ #11/#12). - */ - fun detach(): Pair { - isPlaying = false - val p = roomPlayer - val h = handle - roomPlayer = null - handle = null - player = null - return p to h - } - - companion object { - fun pending(pubkey: String) = ActiveSubscription(pubkey) - } - } + // ActiveSubscription was extracted to its own file in the same + // package — see [ActiveSubscription.kt]. // Platform-specific Factory lives in `amethyst/.../nests/room/`, // not commonMain — the lifecycle KMP `ViewModelProvider.Factory` @@ -1421,16 +1805,72 @@ sealed class BroadcastUiState { */ const val SPEAKING_TIMEOUT_MS: Long = 250L +/** + * How long [NestViewModel.openSubscription] waits for the publisher's + * `catalog.json` to land before constructing the decoder + AudioTrack. + * The catalog declares the audio rendition's `numberOfChannels` and + * `sampleRate`, which the decoder needs at construction time so a + * stereo or non-48 kHz web publisher doesn't get its frames decoded + * with the wrong layout / clock. + * + * Sized to be generous enough that a typical publisher's catalog + * (which arrives within a single round-trip of the SUBSCRIBE_OK with + * the speaker-side emit-on-subscribe hook in place) lands well before + * the timeout, and short enough that a publisher that never emits a + * catalog (legacy publishers, relay-side bug) doesn't visibly stall + * the listener — audio playback proceeds in the default config. + * + * **Frame-buffer budget**: while we wait, audio frames stream from + * the relay into the wrapper's + * [com.vitorpamplona.nestsclient.ReconnectingNestsListener] + * `SUBSCRIBE_BUFFER = 64` SharedFlow with `DROP_OLDEST` overflow. + * At 50 fps Opus that's 1.28 s of headroom; the 500 ms wait + * consumes at most 25 frames of buffer, leaving ≥ 39 frames of + * margin (≈780 ms) before the oldest frame would be dropped. Even + * at the production `framesPerGroup = 50` (= 1 group / sec) cadence + * this never trips the eviction path during normal startup. + */ +const val CATALOG_AWAIT_TIMEOUT_MS: Long = 500L + +/** + * Per-subscription consecutive Opus decode-error count that triggers a + * single conspicuous warning log. Single-frame decode errors are normal + * — Opus PLC papers over them — and are silently swallowed; a streak of + * this size is structural (wire-format mismatch, missing + * legacy-container varint strip, codec/channel-count mismatch, corrupt + * publisher) and demands attention because the user-visible symptom is + * "speaker tile shows up but no audio plays" with no other signal. + * + * 50 × 20 ms ≈ 1 s. Long enough that a transient flurry of bad frames + * doesn't cry wolf; short enough that an "every frame fails" structural + * bug surfaces in a logcat line within the first second of audio. + * + * Logged once per crossing (exact equality, not modulus) so a stuck + * stream produces ONE attention-grabbing line rather than 50 Hz of + * noise. + */ +const val DECODE_ERROR_STREAK_LOG_THRESHOLD: Long = 50L + /** * Per-speaker pre-roll: number of decoded PCM frames buffered before the * underlying [com.vitorpamplona.nestsclient.audio.AudioPlayer] starts - * consuming. 5 × 20 ms ≈ 100 ms of audio — long enough to mask a typical - * Main-thread stall (Compose recomposition / GC) without adding perceptible - * join latency. Combines with [com.vitorpamplona.nestsclient.audio.AudioTrackPlayer]'s - * ~250 ms AudioTrack buffer for ~350 ms of total slack between the + * consuming. 10 × 20 ms ≈ 200 ms of audio. + * + * Sized to absorb the slowest-cadence publisher we expect: kixelated/moq's + * web reference encoder uses `groupDuration: 100ms` (5 frames per group) + * by default but adds another 100–200 ms of jitter on the relay's + * outbound forward queue under residential network conditions. 100 ms of + * preroll (the previous value) reliably underran on web speakers — the + * AudioTrack ran out of samples between adjacent groups and the user + * heard short clicks. 200 ms covers the observed jitter envelope while + * keeping perceived join latency well under the typical "speaker becomes + * audible" delay on the wire. + * + * Combines with [com.vitorpamplona.nestsclient.audio.AudioTrackPlayer]'s + * ~250 ms AudioTrack buffer for ~450 ms of total slack between the * arrival-of-frame and the underrun horizon. */ -const val ROOM_PLAYER_PREROLL_FRAMES: Int = 5 +const val ROOM_PLAYER_PREROLL_FRAMES: Int = 10 /** * Coalescing interval for [NestViewModel.audioLevels]. The decode loop @@ -1442,6 +1882,125 @@ const val ROOM_PLAYER_PREROLL_FRAMES: Int = 5 */ const val LEVEL_TICK_MS: Long = 100L +/** + * `max_latency` we send on every speaker SUBSCRIBE. Tells the relay + * to evict groups older than this from the per-subscriber forward + * queue rather than holding them up to the relay's `MAX_GROUP_AGE` + * default (30 s). Defends against the moq-rs forward-queue cliff + * documented in + * `nestsClient/plans/2026-05-01-quic-stream-cliff-investigation.md`. + * 500 ms = 25 Opus frames at 20 ms cadence — long enough to ride + * out a typical hiccup without dropping a healthy frame, short + * enough to keep the relay's per-subscriber queue bounded. + */ +const val ROOM_AUDIO_MAX_LATENCY_MS: Long = 500L + +/** + * Inactivity threshold for the listener-side cliff detector. If + * a speaker is `_announcedSpeakers` (= relay says they're broadcasting) + * AND the wrapper's MoqObject flow doesn't deliver a frame for this + * many milliseconds, the listener forces a `recycleSession()` so the + * orchestrator opens a fresh QUIC transport. Resets the relay's + * per-subscriber forward queue. + * + * 2 500 ms is past every legitimate group boundary (`framesPerGroup = + * 50` packs 1 s of audio per uni stream, so the receiver sees one + * `drainOneGroup` FIN every ~1 s on a healthy stream — 2.5 s of + * silence means more than two group cycles missed, very likely a + * cliff). Earlier than 2 s would false-positive on the natural 1 s + * cadence; later than 3 s wastes audible audio at every cliff edge. + * + * Production logs (`6e4df4a` run 18:37:43..18:38:08) showed the + * cliff-after-streaming case losing ~6 s of audio at the 4 s + * detector + ~2 s reconnect handshake. Tightening detection to + * 2.5 s shaves ~1.5 s off the audible gap. + */ +const val ROOM_AUDIO_CLIFF_TIMEOUT_MS: Long = 2_500L + +/** + * How often the cliff detector wakes up to scan + * `activeSubscriptions` against `lastFrameAt`. 1 s is a coarse + * enough cadence not to cost anything (one map walk per second) + * while keeping detection latency well under the 4 s threshold. + */ +const val ROOM_AUDIO_CLIFF_CHECK_INTERVAL_MS: Long = 1_000L + +/** + * Cooldown after a cliff-detector-driven recycle. Suppresses + * back-to-back recycles while the wrapper is mid-handshake on the + * fresh session AND while the relay is recovering its + * per-subscriber forward queue. + * + * Production logs at commit ea08c43 showed an 8 s cooldown was + * not enough — moq-rs needed longer to recover between aggressive + * recycles, and 4 recycles in ~30 s drove the relay into a + * "subscribe stream FIN before reply" loop that refused all + * subsequent subscribes. 30 s gives the relay's per-subscriber + * forward task pool time to drain stale pending writes from the + * previous subscription before the new subscribe lands. The + * audible-gap cost rises from ~5 s to ~30 s in the worst case + * (a fully-stalled relay), but the trade is correct: 30 s of + * silence + recovered audio beats endless silence with the relay + * locked out. + */ +const val ROOM_AUDIO_CLIFF_COOLDOWN_MS: Long = 30_000L + +/** + * Diagnostic log frequency for the cliff detector. Emit a state-dump + * line every Nth 1-second tick — i.e. every 5 s while connected and + * idle, so logcat captures the "active / announced / lastFrameAges" + * snapshot we use to debug silent-cliff cases without flooding when + * everything is fine. + */ +private const val CLIFF_DIAG_LOG_EVERY: Long = 5L + +/** + * Pure logic of the cliff detector — extracted so headless tests can + * exercise it with a [kotlin.time.TestTimeSource] without standing up + * the full [NestViewModel] coroutine machinery. + * + * Returns the list of pubkeys that should trigger a `recycleSession()` + * RIGHT NOW. Empty list means "no cliff observed; do nothing on this + * tick." + * + * Recycle conditions, all-of: + * - the pubkey has an active subscription on our side + * ([activeSpeakers]) — i.e. we're actively pulling its audio, + * - the relay has told us this pubkey is broadcasting + * ([announcedSpeakers]), + * - we've received at least one MoQ object in the past + * ([lastFrameAt] has an entry — we don't preemptively recycle + * a brand-new subscription that simply hasn't ramped up yet), + * - the elapsed time since that last object is at or past + * [cliffTimeoutMs]. + * + * Suppression: + * - if [lastRecycleAt] is non-null and less than [cooldownMs] has + * elapsed since it, returns empty (no cascading recycles while + * the wrapper is mid-handshake on a fresh session). + */ +internal fun computeStalledSpeakers( + activeSpeakers: Set, + announcedSpeakers: Set, + lastFrameAt: Map, + lastRecycleAt: kotlin.time.TimeMark?, + cliffTimeoutMs: Long = ROOM_AUDIO_CLIFF_TIMEOUT_MS, + cooldownMs: Long = ROOM_AUDIO_CLIFF_COOLDOWN_MS, +): List { + if (lastRecycleAt != null && + lastRecycleAt.elapsedNow().inWholeMilliseconds < cooldownMs + ) { + return emptyList() + } + return activeSpeakers + .asSequence() + .filter { it in announcedSpeakers } + .filter { pubkey -> + val mark = lastFrameAt[pubkey] ?: return@filter false + mark.elapsedNow().inWholeMilliseconds >= cliffTimeoutMs + }.toList() +} + /** * How long a kind-7 reaction stays in * [NestViewModel.recentReactions] before the eviction sweep diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalog.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalog.kt index 3694843e8..8b3add61f 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalog.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalog.kt @@ -22,35 +22,67 @@ package com.vitorpamplona.amethyst.commons.viewmodels import androidx.compose.runtime.Immutable import com.vitorpamplona.quartz.nip01Core.core.JsonMapper -import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable /** * Decoded shape of a moq-lite `catalog.json` track for a single - * audio-room speaker. Each broadcast advertises one or more tracks - * (one per codec / quality tier) — for nests audio rooms there's - * exactly one Opus track today, but the model carries a list for - * forward compatibility. + * audio-room speaker. Mirrors the kixelated/moq `hang` reference + * catalog format — the canonical shape that `@kixelated/hang`'s + * browser watcher and the `moq-rs` Rust hang crate both produce and + * consume — so Amethyst publishers are visible to the moq-lite browser + * reference, and Amethyst listeners parse standards-aligned catalogs + * from non-Amethyst publishers. * - * Best-effort schema: nostrnests' upstream moq-lite catalog spec - * (kixelated/moq) has evolved across revisions, and not every - * publisher fills in every field. The parser tolerates missing - * keys via `ignoreUnknownKeys = true` on [JsonMapper] and the - * `?`-marked properties. + * Shape (verbatim from `kixelated/moq/rs/hang/src/catalog/`): + * + * { + * "audio": { + * "renditions": { + * "": { + * "codec": "opus", + * "container": { "kind": "legacy" }, + * "sampleRate": 48000, + * "numberOfChannels": 1, + * "bitrate": 32000 // optional + * } + * } + * } + * } + * + * Keys are camelCase per the upstream serde `rename_all = "camelCase"`. + * Every field except the rendition-key string is optional in the parser + * — older / partial publishers are tolerated. Field semantics: + * + * - rendition key: the moq-lite `track` name a subscriber should + * subscribe to for that audio rendition's frames (commonly the same + * string for single-rendition broadcasts). Subscribers pick a + * rendition (e.g. by codec / bitrate) and use this key as the + * Subscribe.track string. + * - `codec`: codec mimetype string (`"opus"`, `"mp4a.40.2"` for AAC). + * - `container.kind`: frame wrapper. `"legacy"` = each frame is + * `varint(timestamp_us) + raw_codec_payload` inside the moq-lite + * group. `"cmaf"` = MOOF/MDAT-fragmented MP4. We only emit/parse + * `legacy` today; unknown kinds are ignored at parse time. + * - `sampleRate` / `numberOfChannels`: PCM source params. */ @Immutable @Serializable data class RoomSpeakerCatalog( - val version: Int? = null, - val audio: List = emptyList(), + val audio: Audio? = null, ) { @Immutable @Serializable - data class AudioTrack( - val track: String? = null, + data class Audio( + val renditions: Map = emptyMap(), + ) + + @Immutable + @Serializable + data class AudioConfig( val codec: String? = null, - @SerialName("sample_rate") val sampleRate: Int? = null, - @SerialName("channel_count") val channelCount: Int? = null, + val container: Container? = null, + val sampleRate: Int? = null, + val numberOfChannels: Int? = null, val bitrate: Int? = null, ) { /** @@ -64,14 +96,48 @@ data class RoomSpeakerCatalog( buildList { codec?.takeIf { it.isNotBlank() }?.let { add(it.uppercase()) } sampleRate?.let { add("${it / 1000}kHz") } - channelCount?.let { add(if (it == 1) "mono" else "${it}ch") } + numberOfChannels?.let { add(if (it == 1) "mono" else "${it}ch") } } return parts.takeIf { it.isNotEmpty() }?.joinToString(" · ") } } - /** First audio track, if any. The current single-Opus reality. */ - fun primaryAudio(): AudioTrack? = audio.firstOrNull() + @Immutable + @Serializable + data class Container( + val kind: String? = null, + ) { + companion object { + /** + * The only `container.kind` value the Amethyst decoder + * pipeline currently understands: each moq-lite frame is + * `varint(timestamp_us) + raw_codec_payload`. Source: + * `kixelated/moq/rs/hang/src/container/legacy.rs`. + * + * Other documented kinds — `"cmaf"` (MOOF/MDAT-fragmented + * MP4) and a handful of in-flight experimental shapes — + * would require a different decoder path and are + * intentionally rejected by [primaryAudio]. + */ + const val LEGACY_KIND: String = "legacy" + } + } + + /** + * First audio rendition with a [Container.LEGACY_KIND] container, + * which is the only container layout the Amethyst decoder pipeline + * understands today (`varint(timestamp_us) + opus_packet` per + * frame). Returns null when no legacy rendition exists — the UI + * surfaces "no audio info" rather than a CMAF rendition we'd try + * to feed to our legacy decoder. + * + * Picks the first match in JSON iteration order + * (kotlinx.serialization uses LinkedHashMap), so the publisher's + * preferred legacy rendition wins. A future publisher that emits + * CMAF-first then legacy still surfaces the legacy entry; CMAF- + * only publishers correctly return null. + */ + fun primaryAudio(): AudioConfig? = audio?.renditions?.values?.firstOrNull { it.container?.kind == Container.LEGACY_KIND } companion object { /** diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/CliffDetectorTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/CliffDetectorTest.kt new file mode 100644 index 000000000..ac51d5ce8 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/CliffDetectorTest.kt @@ -0,0 +1,313 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.viewmodels + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.ExperimentalTime +import kotlin.time.TestTimeSource + +/** + * Unit tests for the [computeStalledSpeakers] predicate that drives the + * listener-side cliff detector in [NestViewModel]. The predicate is + * extracted as a pure function specifically so we can drive it with a + * [TestTimeSource] — neither `runTest`'s virtual scheduler nor a real + * monotonic clock would let us deterministically place events relative + * to the 4 s `ROOM_AUDIO_CLIFF_TIMEOUT_MS` threshold. + * + * The detector defends against the moq-rs production-relay forward- + * queue cliff documented in + * `nestsClient/plans/2026-05-01-quic-stream-cliff-investigation.md`: + * the relay opens fewer (or zero) new uni streams to a slow + * subscriber while the announce stream still says the broadcast is + * Active and the subscribe bidi is still alive at QUIC level — no + * other layer notices. These tests pin the recycle gate so a future + * refactor doesn't accidentally widen or narrow when we recycle. + */ +@OptIn(ExperimentalTime::class) +class CliffDetectorTest { + @Test + fun returnsEmptyWhenNoSpeakersActive() { + val ts = TestTimeSource() + val result = + computeStalledSpeakers( + activeSpeakers = emptySet(), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to ts.markNow()), + lastRecycleAt = null, + ) + assertTrue(result.isEmpty()) + } + + @Test + fun ignoresSpeakerNotInAnnounced() { + // A speaker we're subscribed to but who isn't in the relay's + // announce-stream output is most likely either (a) already + // off stage and the relay just hasn't propagated the Ended + // yet, or (b) in the brief gap between an Ended and the + // wrapper re-issuing. Recycling the whole transport here + // would be wasteful — we only act on confirmed cliffs. + val ts = TestTimeSource() + val frameMark = ts.markNow() + ts += 10_000.milliseconds // way past threshold + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = emptySet(), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = null, + ) + assertTrue(result.isEmpty()) + } + + @Test + fun ignoresSpeakerWithNoFrameYet() { + // Brand-new subscription that hasn't ramped up: lastFrameAt + // has no entry. Don't recycle — the wrapper's per-speaker + // re-issue + opener-throws backoff already retries on its + // own; the cliff detector only acts once we've proven the + // subscription was working. + val ts = TestTimeSource() + ts += 10_000.milliseconds + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = emptyMap(), + lastRecycleAt = null, + ) + assertTrue(result.isEmpty()) + } + + @Test + fun returnsEmptyJustBeforeThreshold() { + // Boundary case: 1 ms under the 2.5 s threshold. The detector + // should still consider this healthy. Relevant because audio + // groups arrive at ~1 s cadence (framesPerGroup=50), so a + // false positive at ≈ threshold would recycle on every + // group-rollover hiccup. + val ts = TestTimeSource() + val frameMark = ts.markNow() + ts += 2_499.milliseconds + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = null, + ) + assertTrue(result.isEmpty()) + } + + @Test + fun returnsStalledSpeakerAtThresholdInclusive() { + // Exactly at threshold: include. The check is `>=`, not `>`. + // Tested explicitly because boundary semantics here are + // user-visible (this is the "audio went silent" detection). + val ts = TestTimeSource() + val frameMark = ts.markNow() + ts += 2_500.milliseconds + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = null, + ) + assertEquals(listOf(ALICE), result) + } + + @Test + fun returnsStalledSpeakerWellPastThreshold() { + val ts = TestTimeSource() + val frameMark = ts.markNow() + ts += 30_000.milliseconds + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = null, + ) + assertEquals(listOf(ALICE), result) + } + + @Test + fun returnsAllStalledSpeakersAtOnceMixedWithFreshOnes() { + // Multi-speaker stage: alice and bob have stalled, charlie's + // subscription is fresh (just received a frame). Recycle + // should fire for the stalled set and leave charlie alone — + // the `recycleSession` itself takes the whole listener down, + // but the test here is the predicate's classification. + // Threshold = 2.5 s default. Spacings: alice/bob 4 s old (past), + // charlie 1.5 s old (under). + val ts = TestTimeSource() + val aliceFrame = ts.markNow() + val bobFrame = ts.markNow() + ts += 2_500.milliseconds + val charlieFrame = ts.markNow() + ts += 1_500.milliseconds + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE, BOB, CHARLIE), + announcedSpeakers = setOf(ALICE, BOB, CHARLIE), + lastFrameAt = + mapOf( + ALICE to aliceFrame, + BOB to bobFrame, + CHARLIE to charlieFrame, + ), + lastRecycleAt = null, + ) + assertEquals(setOf(ALICE, BOB), result.toSet()) + } + + @Test + fun cooldownSuppressesRecycleEvenWhenStalled() { + // After a recycle, the wrapper opens a fresh QUIC transport. + // The new session has no `lastFrameAt` entries yet for any + // pubkey; without a cooldown we would re-trigger on the + // very next 1 s tick because the prior tick's stalled + // pubkeys still age past the threshold (their lastFrameAt + // hasn't been updated by the new session yet). 30 s cooldown + // covers the typical reconnect handshake AND gives moq-rs + // time to drain its per-subscriber forward queue from the + // prior subscription before the new subscribe lands. + val ts = TestTimeSource() + val frameMark = ts.markNow() + ts += 4_500.milliseconds // past threshold + val recycleMark = ts.markNow() // recycle just fired + ts += 5_000.milliseconds // 5 s into cooldown — well within 30 s window + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = recycleMark, + ) + assertTrue(result.isEmpty()) + } + + @Test + fun cooldownReleasesAfterTimeoutPasses() { + // Once cooldown elapses, a still-stalled subscription + // becomes eligible to recycle again. Important for the + // case where the recycle didn't actually fix the cliff — + // we want a second attempt rather than getting wedged. + val ts = TestTimeSource() + val frameMark = ts.markNow() + ts += 4_500.milliseconds + val recycleMark = ts.markNow() + ts += 30_001.milliseconds // 1 ms past 30 s cooldown + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = recycleMark, + ) + assertEquals(listOf(ALICE), result) + } + + @Test + fun customTimeoutsAreHonored() { + // Defaults are wired into the production VM, but the function + // accepts overrides so a test for tighter / looser tolerances + // can drive it without mutating the constants. + val ts = TestTimeSource() + val frameMark = ts.markNow() + ts += 1_000.milliseconds + + val withDefault = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = null, + ) + assertTrue(withDefault.isEmpty(), "1 s elapsed shouldn't trip 2.5 s default") + + val withTightTimeout = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = null, + cliffTimeoutMs = 500L, + ) + assertEquals(listOf(ALICE), withTightTimeout, "1 s elapsed should trip a 500 ms threshold") + } + + @Test + fun activeSpeakerNotInLastFrameAtIsIgnoredEvenWithStalledPeers() { + // Mixed: alice is announced + active + stalled, bob is + // announced + active but has no frame yet. Detector should + // return alice only — bob hasn't proven the subscription + // works, so it's not "stalled" yet, just slow to start. + val ts = TestTimeSource() + val aliceFrame = ts.markNow() + ts += 5_000.milliseconds + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE, BOB), + announcedSpeakers = setOf(ALICE, BOB), + lastFrameAt = mapOf(ALICE to aliceFrame), + lastRecycleAt = null, + ) + assertEquals(listOf(ALICE), result) + } + + @Test + fun nullLastRecycleNeverSuppresses() { + // Null `lastRecycleAt` means "we've never recycled" — the + // very first cliff event of a session must fire. The + // cooldown check is gated on non-null. + val ts = TestTimeSource() + val frameMark = ts.markNow() + ts += 5_000.milliseconds + + val result = + computeStalledSpeakers( + activeSpeakers = setOf(ALICE), + announcedSpeakers = setOf(ALICE), + lastFrameAt = mapOf(ALICE to frameMark), + lastRecycleAt = null, + ) + assertEquals(listOf(ALICE), result) + } + + companion object { + private val ALICE = "a".repeat(64) + private val BOB = "b".repeat(64) + private val CHARLIE = "c".repeat(64) + } +} diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModelTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModelTest.kt index 2cddb7a0d..5c7415ab5 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModelTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/NestViewModelTest.kt @@ -455,8 +455,8 @@ class NestViewModelTest { NestViewModel( httpClient = NoopNestsClient, transport = NoopWebTransportFactory, - decoderFactory = { NoopOpusDecoder }, - playerFactory = { NoopAudioPlayer() }, + decoderFactory = { _, _ -> NoopOpusDecoder }, + playerFactory = { _, _ -> NoopAudioPlayer() }, signer = NoopSigner, room = ROOM_CONFIG, connector = diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalogTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalogTest.kt index 4f4ffab80..a4941b64a 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalogTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalogTest.kt @@ -27,69 +27,80 @@ import kotlin.test.assertNull class RoomSpeakerCatalogTest { @Test - fun parsesNostrnestsShape() { + fun parsesKixelatedHangShape() { + // Verbatim shape produced by the canonical kixelated/moq `hang` + // crate (`rs/hang/src/catalog/`). Verifies that an Amethyst + // listener picks up a standards-aligned publisher's catalog. val json = """ { - "version": 1, - "audio": [ - { - "track": "audio/data", - "codec": "opus", - "sample_rate": 48000, - "channel_count": 2, - "bitrate": 64000 + "audio": { + "renditions": { + "audio/data": { + "codec": "opus", + "container": { "kind": "legacy" }, + "sampleRate": 48000, + "numberOfChannels": 2, + "bitrate": 64000 + } } - ] + } } """.trimIndent() val catalog = RoomSpeakerCatalog.parseOrNull(json.encodeToByteArray()) assertNotNull(catalog) - assertEquals(1, catalog.version) - assertEquals(1, catalog.audio.size) - val track = catalog.primaryAudio() - assertNotNull(track) - assertEquals("audio/data", track.track) - assertEquals("opus", track.codec) - assertEquals(48_000, track.sampleRate) - assertEquals(2, track.channelCount) - assertEquals(64_000, track.bitrate) + assertEquals(1, catalog.audio?.renditions?.size) + val rendition = catalog.primaryAudio() + assertNotNull(rendition) + assertEquals("opus", rendition.codec) + assertEquals("legacy", rendition.container?.kind) + assertEquals(48_000, rendition.sampleRate) + assertEquals(2, rendition.numberOfChannels) + assertEquals(64_000, rendition.bitrate) } @Test fun describeFormatsHumanReadable() { - val track = - RoomSpeakerCatalog.AudioTrack( + val rendition = + RoomSpeakerCatalog.AudioConfig( codec = "opus", sampleRate = 48_000, - channelCount = 2, + numberOfChannels = 2, ) // Codec uppercased, kHz / channel count short-formed — // intended for a single-line tooltip in the participant // sheet, not a verbose codec dump. - assertEquals("OPUS · 48kHz · 2ch", track.describe()) + assertEquals("OPUS · 48kHz · 2ch", rendition.describe()) } @Test fun describeMonoIsLabelled() { - val track = RoomSpeakerCatalog.AudioTrack(codec = "opus", channelCount = 1) - assertEquals("OPUS · mono", track.describe()) + val rendition = RoomSpeakerCatalog.AudioConfig(codec = "opus", numberOfChannels = 1) + assertEquals("OPUS · mono", rendition.describe()) } @Test fun describeAllNullReturnsNull() { - val track = RoomSpeakerCatalog.AudioTrack() + val rendition = RoomSpeakerCatalog.AudioConfig() // Empty catalog → caller doesn't render a "unknown · unknown" // line; describe() returns null so the UI can omit cleanly. - assertNull(track.describe()) + assertNull(rendition.describe()) } @Test fun toleratesUnknownKeys() { - // Forward-compat: future moq-lite catalog revisions can add - // fields without breaking older clients. + // Forward-compat: future hang catalog revisions can add + // fields without breaking older clients. Container.kind is + // declared as `legacy` because `primaryAudio()` filters to + // that kind — the unknown-key tolerance we're testing here + // is about unrecognized siblings, not about the container + // requirement itself. val json = - """{"version":2,"audio":[{"track":"audio/data","codec":"opus","extra":"future-only"}],"new_top_level":true}""" + """{"audio":{"renditions":{"audio/data":{ + | "codec":"opus","container":{"kind":"legacy"}, + | "extra":"future-only" + |}}},"video":{},"newTopLevel":true} + """.trimMargin() val catalog = RoomSpeakerCatalog.parseOrNull(json.encodeToByteArray()) assertNotNull(catalog) assertEquals("opus", catalog.primaryAudio()?.codec) @@ -104,12 +115,103 @@ class RoomSpeakerCatalogTest { } @Test - fun emptyAudioListIsAllowed() { + fun emptyAudioRenditionsIsAllowed() { // A publisher might emit an "advertising" catalog with no - // tracks yet (e.g. between codec switches). Accept the shape - // and let primaryAudio() return null. - val catalog = RoomSpeakerCatalog.parseOrNull("""{"version":1,"audio":[]}""".encodeToByteArray()) + // renditions yet (e.g. between codec switches). Accept the + // shape and let primaryAudio() return null. + val catalog = RoomSpeakerCatalog.parseOrNull("""{"audio":{"renditions":{}}}""".encodeToByteArray()) assertNotNull(catalog) assertNull(catalog.primaryAudio()) } + + @Test + fun primaryAudioPicksLegacyEvenWhenCmafComesFirst() { + // A future publisher may emit CMAF-first then legacy in the + // renditions map. We only know how to decode the legacy + // container today (varint(ts)+opus); CMAF (MOOF/MDAT) would + // be fed bytes-as-Opus and decode to garbage. The filter + // skips non-legacy renditions so the chosen entry is one we + // can actually play. + val mixed = + """{"audio":{"renditions":{ + | "video/cmaf":{"codec":"opus","container":{"kind":"cmaf"},"sampleRate":48000,"numberOfChannels":1}, + | "audio/data":{"codec":"opus","container":{"kind":"legacy"},"sampleRate":48000,"numberOfChannels":1} + |}}} + """.trimMargin() + val catalog = RoomSpeakerCatalog.parseOrNull(mixed.encodeToByteArray()) + assertNotNull(catalog) + val rendition = catalog.primaryAudio() + assertNotNull(rendition, "expected the legacy rendition, not CMAF") + assertEquals("legacy", rendition.container?.kind) + } + + @Test + fun primaryAudioReturnsNullForCmafOnlyPublisher() { + // No legacy rendition → no decoder path. Returning null is + // the right contract: the caller falls back to "unknown + // codec / no audio" rather than feeding CMAF bytes to the + // legacy decoder. + val cmafOnly = + """{"audio":{"renditions":{"audio/cmaf":{ + | "codec":"opus","container":{"kind":"cmaf"}, + | "sampleRate":48000,"numberOfChannels":1 + |}}}} + """.trimMargin() + val catalog = RoomSpeakerCatalog.parseOrNull(cmafOnly.encodeToByteArray()) + assertNotNull(catalog) + assertNull(catalog.primaryAudio()) + } + + @Test + fun primaryAudioReturnsNullWhenContainerKindIsMissing() { + // Defensive: a malformed publisher that omits `container` + // entirely must NOT be treated as legacy by default — we'd + // be guessing the wire shape. Same null fallback as the + // CMAF-only case. + val noContainer = + """{"audio":{"renditions":{"audio/data":{ + | "codec":"opus","sampleRate":48000,"numberOfChannels":1 + |}}}} + """.trimMargin() + val catalog = RoomSpeakerCatalog.parseOrNull(noContainer.encodeToByteArray()) + assertNotNull(catalog) + assertNull(catalog.primaryAudio()) + } + + @Test + fun missingAudioReturnsNullPrimary() { + // hang catalogs declaring only video MUST still parse without + // throwing — primaryAudio() returns null. + val catalog = RoomSpeakerCatalog.parseOrNull("""{}""".encodeToByteArray()) + assertNotNull(catalog) + assertNull(catalog.primaryAudio()) + } + + @Test + fun stripPrefixRoundTripsCanonicalCatalog() { + // The catalog payload `MoqLiteHangCatalog.opusMono48k(...)` emits + // (in `:nestsClient`) MUST round-trip through this parser — the + // two classes target the same wire shape independently because + // `:nestsClient` does not depend on `:commons` and vice versa. + // `MoqLiteHangCatalog` isn't reachable from this module; keep an + // inline literal that mirrors what the publisher emits verbatim + // so a desync on either side trips this assertion. + val emitted = + "{\"audio\":{\"renditions\":{\"audio/data\":{" + + "\"codec\":\"opus\",\"container\":{\"kind\":\"legacy\"}," + + "\"sampleRate\":48000,\"numberOfChannels\":1,\"jitter\":20}}}}" + val catalog = RoomSpeakerCatalog.parseOrNull(emitted.encodeToByteArray()) + assertNotNull(catalog) + val rendition = catalog.primaryAudio() + assertNotNull(rendition) + assertEquals("opus", rendition.codec) + assertEquals("legacy", rendition.container?.kind) + assertEquals(48_000, rendition.sampleRate) + assertEquals(1, rendition.numberOfChannels) + // `jitter` is intentionally not asserted on `rendition` — the + // commons-side parser drops unknown fields via the JsonMapper's + // `ignoreUnknownKeys = true`. Adding it to `RoomSpeakerCatalog` + // is forward work; the byte-exact match in the literal above + // already pins the wire shape. + } } diff --git a/nestsClient/plans/2026-05-06-stream-priority-followup.md b/nestsClient/plans/2026-05-06-stream-priority-followup.md new file mode 100644 index 000000000..6e9578214 --- /dev/null +++ b/nestsClient/plans/2026-05-06-stream-priority-followup.md @@ -0,0 +1,151 @@ +# Stream priority for moq-lite group uni streams (T11.3 follow-up) + +**Status**: deferred — flagged in T11 (drop `bestEffort=true`), not landed. + +## Why + +`T11` removed `bestEffort=true` from `MoqLiteSession.openGroupStream` +because the QUIC contract it relied on (drop lost ranges silently +without `RESET_STREAM`) created undeliverable streams that wedge +peer reassembly buffers — the actual user-visible bug was a 30 s +silent dropout per lost packet on lossy networks. + +`bestEffort=true` was incidentally providing a "newer-groups-skip- +queued-retransmits" effect: the writer never queued retransmits in +the first place, so under congestion the loss budget naturally +biased toward dropping older lost ranges. With `bestEffort=true` +gone, all retransmits are now queued, and under sustained +congestion the writer drains streams in `streamRoundRobinStart` +order — which can mean the listener catches up on a stale group +when a fresh one would be more useful. + +The kixelated reference (`moq-rs`'s `Publisher::serve_group` in +`rs/moq-lite/src/lite/publisher.rs:347-406`) addresses this by +calling `stream.set_priority(priority.current())` on every group +stream and biasing the writer toward higher-priority (newer) +streams. We should do the same. + +## Target shape + +### `:quic` API + +Add a stable priority hook to `QuicStream`: + +```kotlin +class QuicStream(...) { + @Volatile + var priority: Int = 0 + // Higher drains first under contention. Default 0 = unchanged + // round-robin behaviour. +} +``` + +Expose at the WebTransport layer: + +```kotlin +interface WebTransportWriteStream { + fun setPriority(priority: Int) +} +``` + +### `QuicConnectionWriter` — the load-bearing change + +Today's send-frame loop (`QuicConnectionWriter.kt:411-416`): + +```kotlin +val streamsView = conn.streamsListLocked() +val start = conn.streamRoundRobinStart % streamsView.size +for (i in streamsView.indices) { + val stream = streamsView[(start + i) % streamsView.size] + // ... +} +``` + +Replace with priority-then-round-robin: + +```kotlin +val streamsView = conn.streamsListLocked() +val sorted = streamsView.sortedByDescending { it.priority } +val start = conn.streamRoundRobinStart % sorted.size +for (i in sorted.indices) { + val stream = sorted[(start + i) % sorted.size] + // ... +} +``` + +Sort is stable; same-priority streams retain insertion order, so the +existing round-robin behaviour holds within a priority tier. Higher- +priority streams always come first in the iteration. + +Cost: O(N log N) per drain pass, where N is active local-initiated +streams. N is small (1–10 in the moq-lite audio path); the +allocation of `sorted` per pass is the only real cost. If that +shows up in profiling, switch to `kotlin.collections.IntArray`- +backed indirect sort or maintain a priority-sorted list incrementally +on `setPriority` calls. + +### moq-lite wiring + +`MoqLiteSession.openGroupStream:1022-1037`: + +```kotlin +internal suspend fun openGroupStream( + subscribeId: Long, + sequence: Long, +): WebTransportWriteStream { + val uni = transport.openUniStream() + uni.setPriority(sequence.coerceAtMost(Int.MAX_VALUE.toLong()).toInt()) + uni.write(Varint.encode(MoqLiteDataType.Group.code)) + uni.write(MoqLiteCodec.encodeGroupHeader(MoqLiteGroupHeader(subscribeId, sequence))) + return uni +} +``` + +Newer groups have higher sequence → higher priority → drain first +under congestion. The saturation conversion handles broadcasts that +run long enough for `sequence` to exceed `Int.MAX_VALUE` (≈ 71 +years at our 1 group/sec production cadence; defensive only). + +## Test + +Add a unit test in `:quic` that builds a `QuicConnection`, opens +two streams, sets `.priority = 0` on the first and `.priority = 1` +on the second, queues bytes on both, and verifies the higher- +priority stream's bytes hit the wire first. Doesn't need to fake +real flow-control backpressure — pinning the iteration order via +the writer's emitted-frames tape is sufficient to catch the +regression case ("a later refactor accidentally re-introduces +round-robin order"). + +A second test in `:nestsClient` that opens 3 group streams and +asserts later-sequence streams drain before earlier ones is +nice-to-have but secondary; the `:quic`-level test pins the load- +bearing invariant. + +## Why deferred + +The change is small in lines but touches the QUIC writer's hot +path. Rebasing a future `:quic` retransmit / pacing change onto a +priority-sorted iteration order is doable but the diff conflicts +get noisy. Better landed as a focused PR after the current +interop-work series stabilises. + +Risk profile: +- **Bugs**: subtle starvation risk if a high-priority stream + always has `streamRemaining > 0` — round-robin tiebreaker within + a priority tier mitigates this for same-priority case, but the + cross-tier case needs deliberate thought (do we want strict + priority or weighted?). +- **Performance**: per-pass sort allocation. Negligible for N≤10 + but worth measuring if N grows. +- **Compat**: streams without an explicit priority default to 0, + matching today's behaviour. Existing tests should pass unchanged. + +## When to land + +After the catalog interop series is fully verified (production +audio with no degradation under realistic conditions), and ideally +alongside a `:quic` perf review pass that touches the same code +path. Not blocking on any audio bug today — `T11`'s reliable- +delivery fix is the actual correctness change; this is the +spec-aligned hardening. diff --git a/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/AudioTrackPlayer.kt b/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/AudioTrackPlayer.kt index 8434263ea..0cba8be7f 100644 --- a/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/AudioTrackPlayer.kt +++ b/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/AudioTrackPlayer.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.nestsclient.audio import android.media.AudioAttributes import android.media.AudioTrack import android.os.Process +import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.ExecutorCoroutineDispatcher import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.withContext @@ -77,7 +78,37 @@ import android.media.AudioFormat as AndroidAudioFormat class AudioTrackPlayer( private val usage: Int = AudioAttributes.USAGE_MEDIA, private val contentType: Int = AudioAttributes.CONTENT_TYPE_SPEECH, + /** + * Number of channels in the PCM stream this player will receive. Must + * match the [com.vitorpamplona.nestsclient.audio.OpusDecoder]'s output + * configuration (mono Opus → 1, stereo Opus → 2 with L/R interleaving). + * Drives both the AudioTrack channel mask and the underlying buffer- + * size target. Default is [AudioFormat.CHANNELS] (mono) so existing + * call sites that don't pass a channel count keep the prior behaviour. + */ + private val channelCount: Int = AudioFormat.CHANNELS, + /** + * PCM sample rate in Hz. Drives the AudioTrack output rate and the + * 250 ms target-buffer calculation. For Opus, Android's Codec2 + * decoder always emits 48 kHz PCM regardless of the OpusHead + * `inputSampleRate`, so for the production Opus path this is + * always [AudioFormat.SAMPLE_RATE_HZ] — but threading the + * parameter through keeps the AudioTrack's declared rate matched + * to whatever the catalog says, and lets a future codec or + * container variant whose decoder DOES respect input sample rate + * (a non-Opus rendition) get the correct PCM clock. + */ + private val sampleRate: Int = AudioFormat.SAMPLE_RATE_HZ, ) : AudioPlayer { + init { + require(channelCount in 1..2) { + "AudioTrackPlayer supports mono (1) or stereo (2) only, got $channelCount" + } + require(sampleRate > 0) { + "AudioTrackPlayer sampleRate must be positive, got $sampleRate" + } + } + private var track: AudioTrack? = null private var muted: Boolean = false private var volume: Float = 1f @@ -96,33 +127,39 @@ class AudioTrackPlayer( override fun start() { if (track != null) return + Log + .d("NestPlay") { "AudioTrackPlayer.start() — allocating AudioTrack" } val channelMask = - when (AudioFormat.CHANNELS) { + when (channelCount) { 1 -> AndroidAudioFormat.CHANNEL_OUT_MONO 2 -> AndroidAudioFormat.CHANNEL_OUT_STEREO - else -> error("unsupported channel count ${AudioFormat.CHANNELS}") + else -> error("unsupported channel count $channelCount") } val minBuffer = AudioTrack.getMinBufferSize( - AudioFormat.SAMPLE_RATE_HZ, + sampleRate, channelMask, AndroidAudioFormat.ENCODING_PCM_16BIT, ) if (minBuffer <= 0) { throw AudioException( AudioException.Kind.DeviceUnavailable, - "AudioTrack.getMinBufferSize returned $minBuffer for ${AudioFormat.SAMPLE_RATE_HZ} Hz", + "AudioTrack.getMinBufferSize returned $minBuffer for $sampleRate Hz", ) } // Target ~250 ms of audio: enough headroom so the decode loop can // miss its 20 ms cadence by an order of magnitude before the device // underruns. Take the larger of `minBuffer * 16` and an explicit // 250 ms-equivalent so devices that report a small minBuffer still - // get the same wall-clock slack. + // get the same wall-clock slack. Stereo doubles the byte count + // per sample (interleaved L,R 16-bit shorts); a higher sample + // rate scales the per-second byte count linearly — both factor + // into the target so the wall-clock 250 ms target is preserved + // regardless of channel layout / sample rate. val targetBytes250Ms = - (AudioFormat.SAMPLE_RATE_HZ / 4) * AudioFormat.BYTES_PER_SAMPLE * AudioFormat.CHANNELS + (sampleRate / 4) * AudioFormat.BYTES_PER_SAMPLE * channelCount val bufferBytes = maxOf(minBuffer * 16, targetBytes250Ms) val newTrack = @@ -139,7 +176,7 @@ class AudioTrackPlayer( AndroidAudioFormat .Builder() .setEncoding(AndroidAudioFormat.ENCODING_PCM_16BIT) - .setSampleRate(AudioFormat.SAMPLE_RATE_HZ) + .setSampleRate(sampleRate) .setChannelMask(channelMask) .build(), ).setBufferSizeInBytes(bufferBytes) @@ -178,6 +215,10 @@ class AudioTrackPlayer( audioExecutor = executor audioDispatcher = executor.asCoroutineDispatcher() track = newTrack + Log.d("NestPlay") { + "AudioTrack allocated: state=${newTrack.state} playState=${newTrack.playState} " + + "bufferSizeBytes=$bufferBytes minBuffer=$minBuffer sampleRate=$sampleRate" + } } override fun beginPlayback() { @@ -189,12 +230,18 @@ class AudioTrackPlayer( val t = track ?: return try { t.play() + Log.d("NestPlay") { + "AudioTrack.play() returned, state=${t.playState} bufferSize=${t.bufferSizeInFrames} muted=$muted volume=$volume" + } } catch (e: Throwable) { // PLAY-on-uninitialized AudioTrack is the only realistic // failure here, and we already guard against an unallocated // track above. Throw so [NestPlayer]'s outer catch surfaces // it via `onError(AudioException.PlaybackFailed)` — same path // that handles every other mid-stream device failure. + Log.w("NestPlay") { + "AudioTrack.play() threw: ${e::class.simpleName}: ${e.message}" + } throw AudioException( AudioException.Kind.DeviceUnavailable, "AudioTrack.play() rejected start", @@ -215,21 +262,33 @@ class AudioTrackPlayer( withContext(dispatcher) { val written = t.write(pcm, 0, pcm.size, AudioTrack.WRITE_BLOCKING) if (written < 0) { + Log.w("NestPlay") { + "AudioTrack.write returned error $written (state=${t.playState})" + } throw AudioException( AudioException.Kind.PlaybackFailed, "AudioTrack.write returned error code $written", ) } + if (written != pcm.size) { + Log.w("NestPlay") { + "AudioTrack.write partial: requested=${pcm.size} written=$written" + } + } } } override fun setMuted(muted: Boolean) { this.muted = muted + Log + .d("NestPlay") { "AudioTrackPlayer.setMuted($muted) volume=$volume" } track?.let { applyMuteVolume(it) } } override fun setVolume(volume: Float) { this.volume = volume.coerceIn(0f, 1f) + Log + .d("NestPlay") { "AudioTrackPlayer.setVolume($volume) muted=$muted" } track?.let { applyMuteVolume(it) } } diff --git a/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusDecoder.kt b/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusDecoder.kt index 5f044d508..d9abb819f 100644 --- a/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusDecoder.kt +++ b/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusDecoder.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.nestsclient.audio import android.media.MediaCodec import android.media.MediaFormat +import com.vitorpamplona.quartz.utils.Log import java.nio.ByteBuffer import java.nio.ByteOrder @@ -31,18 +32,67 @@ import java.nio.ByteOrder * across packets, so sharing a decoder across speakers would cause clicks. * * Configuration: - * - 48 kHz mono signed 16-bit PCM output (matches [AudioFormat]). - * - CSD-0: Opus identification header per RFC 7845 §5.1, 19 bytes. + * - 48 kHz signed 16-bit PCM output (Opus's internal sample rate; + * MediaCodec's Opus decoder always emits 48 kHz regardless of the + * OpusHead `inputSampleRate` field). + * - [channelCount] — 1 (mono) or 2 (stereo) interleaved L/R. Drives both + * the MediaFormat channel-count and the OpusHead CSD-0 channel byte. + * A web publisher (kixelated/hang) emits stereo when the user's + * AudioContext picks a stereo input; without matching channel + * configuration the decoder either downmixes with artifacts or + * refuses the packet. + * - CSD-0: Opus identification header per RFC 7845 §5.1, 19 bytes + * (mapping family 0; covers both mono and stereo with implicit + * L,R interleaving). * - CSD-1 / CSD-2: pre-skip + seek pre-roll, both zero (we don't seek). + * + * Default is [AudioFormat.CHANNELS] (mono) so existing call sites that + * don't pass a channel count continue to behave exactly as before. */ -class MediaCodecOpusDecoder : OpusDecoder { +class MediaCodecOpusDecoder( + private val channelCount: Int = AudioFormat.CHANNELS, + /** + * Source sample rate in Hz. Drives the OpusHead `inputSampleRate` + * field and the MediaFormat `audio/opus` sample-rate hint. + * + * **Note on Opus's actual decode rate**: Opus always decodes at + * 48 kHz internally regardless of [sampleRate] (the codec's + * design — RFC 6716). Android's Codec2 `audio/opus` decoder + * always emits 48 kHz PCM. So this parameter is informational at + * the OpusHead level and doesn't affect the PCM rate the + * downstream [AudioPlayer] sees. We thread it through anyway so + * the decoder declaration matches what the catalog says, and so a + * future codec or container variant that DOES respect input + * sample rate (e.g. a non-Opus rendition) gets the correct + * configuration. + */ + private val sampleRate: Int = AudioFormat.SAMPLE_RATE_HZ, +) : OpusDecoder { + init { + require(channelCount in 1..2) { + "MediaCodecOpusDecoder supports mono (1) or stereo (2) only, got $channelCount" + } + require(sampleRate > 0) { + "MediaCodecOpusDecoder sampleRate must be positive, got $sampleRate" + } + } + private val codec: MediaCodec = try { - MediaCodec.createDecoderByType(MediaFormat.MIMETYPE_AUDIO_OPUS).apply { - configure(buildFormat(), null, null, 0) - start() - } + MediaCodec + .createDecoderByType(MediaFormat.MIMETYPE_AUDIO_OPUS) + .apply { + configure(buildFormat(channelCount, sampleRate), null, null, 0) + start() + }.also { + Log.d("NestPlay") { + "MediaCodecOpusDecoder allocated codec='${it.name}' channelCount=$channelCount" + } + } } catch (t: Throwable) { + Log.w("NestPlay") { + "MediaCodec audio/opus decoder allocation FAILED: ${t::class.simpleName}: ${t.message}" + } throw AudioException( AudioException.Kind.DeviceUnavailable, "Failed to allocate MediaCodec audio/opus decoder", @@ -61,7 +111,11 @@ class MediaCodecOpusDecoder : OpusDecoder { // via ShortBuffer.get(dst, off, len) — the previous shape went // through ArrayList, which boxed every PCM sample // (~48 000 alloc/sec/speaker on the audio hot path). - val out = ShortArray(AudioFormat.FRAME_SIZE_SAMPLES) + // + // Stereo Opus emits L/R-interleaved samples, so a 20 ms / 960- + // sample frame produces `FRAME_SIZE_SAMPLES * channelCount` + // shorts — twice as many for stereo as for mono. + val out = ShortArray(AudioFormat.FRAME_SIZE_SAMPLES * channelCount) var outPos = 0 // 1. Acquire an input slot. On rare back-pressure (output buffers @@ -187,14 +241,17 @@ class MediaCodecOpusDecoder : OpusDecoder { private const val FRAME_DURATION_US = 20_000L // 20 ms - private fun buildFormat(): MediaFormat { + private fun buildFormat( + channelCount: Int, + sampleRate: Int, + ): MediaFormat { val format = MediaFormat.createAudioFormat( MediaFormat.MIMETYPE_AUDIO_OPUS, - AudioFormat.SAMPLE_RATE_HZ, - AudioFormat.CHANNELS, + sampleRate, + channelCount, ) - format.setByteBuffer("csd-0", ByteBuffer.wrap(buildOpusIdHeader())) + format.setByteBuffer("csd-0", ByteBuffer.wrap(buildOpusIdHeader(channelCount, sampleRate))) // Pre-skip + seek pre-roll: both zero, encoded as little-endian // 64-bit nanoseconds per Android's MediaCodec contract. format.setByteBuffer("csd-1", ByteBuffer.wrap(zeroLongLe())) @@ -202,14 +259,19 @@ class MediaCodecOpusDecoder : OpusDecoder { return format } - private fun buildOpusIdHeader(): ByteArray { - // RFC 7845 §5.1 — 19 bytes for mono, mapping family 0. + private fun buildOpusIdHeader( + channelCount: Int, + sampleRate: Int, + ): ByteArray { + // RFC 7845 §5.1 — 19 bytes, mapping family 0 (covers mono and + // stereo with implicit L,R interleaving; no per-channel + // mapping table required). val buf = ByteBuffer.allocate(19).order(ByteOrder.LITTLE_ENDIAN) buf.put("OpusHead".encodeToByteArray()) // 8 bytes magic buf.put(1.toByte()) // version - buf.put(AudioFormat.CHANNELS.toByte()) // channel count + buf.put(channelCount.toByte()) // channel count (1 or 2) buf.putShort(0) // pre-skip - buf.putInt(AudioFormat.SAMPLE_RATE_HZ) // input sample rate + buf.putInt(sampleRate) // input sample rate buf.putShort(0) // output gain (Q7.8 dB) buf.put(0.toByte()) // mapping family 0 return buf.array() diff --git a/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusEncoder.kt b/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusEncoder.kt index 5de9f3911..f18f7da99 100644 --- a/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusEncoder.kt +++ b/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusEncoder.kt @@ -22,6 +22,7 @@ package com.vitorpamplona.nestsclient.audio import android.media.MediaCodec import android.media.MediaFormat +import com.vitorpamplona.quartz.utils.Log import java.nio.ByteOrder /** @@ -56,6 +57,16 @@ class MediaCodecOpusEncoder( private var presentationTimeUs: Long = 0L private var released = false + /** + * Latch so a single conspicuous log fires the FIRST time we drop a + * [MediaCodec.BUFFER_FLAG_CODEC_CONFIG] output buffer. Without + * this latch the same encoder can produce 2-3 CSD buffers in a row + * (OpusHead + OpusTags on some Android Codec2 stacks) and we'd + * spam the log; with it, we record the fact once per encoder + * lifetime and stay quiet thereafter. + */ + private var loggedCsdSkip = false + override fun encode(pcm: ShortArray): ByteArray { check(!released) { "encoder released" } require(pcm.isNotEmpty()) { "PCM frame must not be empty" } @@ -81,11 +92,60 @@ class MediaCodecOpusEncoder( // without producing output, which would otherwise busy-spin at // 100 Hz against the 10 ms dequeue timeout). var formatChangeAbsorbed = false + // CSD-skip iteration cap. Codec2 stacks have been observed to + // emit OpusHead AND OpusTags as separate CSD buffers; some + // future stack might emit more. Cap the per-call CSD-skip + // count so a buggy encoder that emits CSD on every dequeue + // can't busy-loop the per-frame budget. 4 is generous: the + // expected case is 1 (OpusHead) or 2 (OpusHead + OpusTags). + var csdSkipsThisCall = 0 while (true) { val outputIndex = codec.dequeueOutputBuffer(bufferInfo, DEQUEUE_TIMEOUT_US) when { outputIndex >= 0 -> { val outputBuffer = codec.getOutputBuffer(outputIndex) ?: continue + // CODEC_CONFIG buffers carry codec-specific data + // (CSD): on the Android `audio/opus` encoder these + // are the 19-byte OpusHead identification header + // and (on some Codec2 stacks) the OpusTags + // comment header. They are NOT Opus packets — they + // are decoder-config blobs that should be supplied + // out-of-band on the receive side (we already do + // this in `MediaCodecOpusDecoder.buildOpusIdHeader`). + // Sending them to the wire as audio frames means + // the watcher's WebCodecs `AudioDecoder.decode` + // sees garbage in the first frame, burning a + // warmup slot and producing a click on group + // rollover after every JWT-refresh hot-swap. + val isCodecConfig = + bufferInfo.flags and MediaCodec.BUFFER_FLAG_CODEC_CONFIG != 0 + if (isCodecConfig) { + codec.releaseOutputBuffer(outputIndex, false) + if (!loggedCsdSkip) { + loggedCsdSkip = true + Log.d("NestTx") { + "MediaCodecOpusEncoder skipped ${bufferInfo.size}-byte CODEC_CONFIG (OpusHead/OpusTags) — not an audio frame" + } + } + csdSkipsThisCall += 1 + if (csdSkipsThisCall >= MAX_CSD_SKIPS_PER_CALL) { + // A buggy encoder is emitting CSD on every + // dequeue; bail this call rather than + // busy-looping the per-frame budget. + // Returning empty is the existing "warmup" + // contract — broadcaster's + // `if (opus.isEmpty()) continue` handles it + // and the next encode call retries. + Log.w("NestTx") { + "MediaCodecOpusEncoder hit MAX_CSD_SKIPS_PER_CALL=$MAX_CSD_SKIPS_PER_CALL; bailing this encode call (encoder may be misbehaving)" + } + return ByteArray(0) + } + // Don't return; loop to find the next output + // buffer (the next dequeue may be the actual + // first audio frame). + continue + } val opus = ByteArray(bufferInfo.size) outputBuffer.position(bufferInfo.offset) outputBuffer.limit(bufferInfo.offset + bufferInfo.size) @@ -125,6 +185,16 @@ class MediaCodecOpusEncoder( private const val DEQUEUE_TIMEOUT_US = 10_000L private const val FRAME_DURATION_US = 20_000L + /** + * Per-encode-call cap on consecutive + * [MediaCodec.BUFFER_FLAG_CODEC_CONFIG] outputs we'll skip + * before bailing the call. Expected steady state is 0; the + * one-time startup cost is 1 (OpusHead) or 2 (OpusHead + + * OpusTags on Codec2). 4 leaves headroom for a future stack + * that emits more without uncapping the loop entirely. + */ + private const val MAX_CSD_SKIPS_PER_CALL: Int = 4 + private fun buildFormat(bitrate: Int): MediaFormat = MediaFormat .createAudioFormat( diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/HotSwappablePublisherSource.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/HotSwappablePublisherSource.kt new file mode 100644 index 000000000..1a895d238 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/HotSwappablePublisherSource.kt @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient + +import com.vitorpamplona.nestsclient.moq.lite.MoqLitePublisherHandle + +/** + * Internal hot-swap seam: speakers that expose this interface let the + * reconnect wrapper retarget a long-lived + * [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster] onto a + * freshly-opened moq-lite session's publisher without restarting the + * AudioRecord / Opus encoder pipeline. Implemented by + * [MoqLiteNestsSpeaker]; not implemented by the IETF reference + * [DefaultNestsSpeaker], which falls back to the close-then-restart path + * inside [com.vitorpamplona.nestsclient.connectReconnectingNestsSpeaker]. + * + * The wrapper uses an `as?` cast to detect support so this interface + * can stay package-internal — protocol consumers never see it. + */ +internal interface HotSwappablePublisherSource { + /** + * Open a fresh [MoqLitePublisherHandle] on the underlying moq-lite + * session. Caller owns the returned handle's lifetime (typically + * via [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster.swapPublisher]'s + * close-the-old contract). + * + * @param startSequence first group sequence the new publisher will + * assign. Used by the hot-swap path to seed the new session's + * audio track with the previous session's + * [MoqLitePublisherHandle.nextSequence] so kixelated/hang's + * `Container.Consumer.#run` doesn't drop every post-recycle + * group as `sequence < #active`. Pass `0L` for fresh (non- + * continuation) publishers — the catalog track is one such + * case, since its `#active` semantics are different from audio. + */ + suspend fun openPublisherForHotSwap( + track: String, + startSequence: Long = 0L, + ): MoqLitePublisherHandle + + /** + * Surface a broadcast-pipeline terminal failure (e.g. sustained + * `publisher.send` errors past + * [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster.MAX_CONSECUTIVE_SEND_ERRORS]) + * by flipping the speaker's state to [NestsSpeakerState.Failed]. + * Called by the hot-swap pump when the long-lived broadcaster's + * `onTerminalFailure` fires; lets the reconnect orchestrator + * observe the terminal state and recycle the session, matching + * the legacy + * [MoqLiteNestsSpeaker.startBroadcasting] path's failure + * propagation. + */ + fun reportBroadcastTerminalFailure() +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteBroadcastHandle.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteBroadcastHandle.kt new file mode 100644 index 000000000..bd8635ac6 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteBroadcastHandle.kt @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient + +import com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster +import com.vitorpamplona.nestsclient.moq.lite.MoqLitePublisherHandle + +/** + * [BroadcastHandle] returned by [MoqLiteNestsSpeaker.startBroadcasting] + * for the non-reconnecting code path. Wraps: + * + * - the long-lived audio [broadcaster] (mic + encoder + send loop), + * - the audio-track [publisher] on the moq-lite session, + * - the [catalogPublisher] companion track that emits the + * `catalog.json` manifest on every new SUBSCRIBE. + * + * Mute is forwarded to the broadcaster (which also reports the user- + * facing state back via [parent.reportMuteState]). [close] tears down + * all three components in a fixed order — broadcaster → audio + * publisher → catalog publisher — so a `CancellationException` + * mid-shutdown still releases every resource before re-throwing. + * + * Reconnecting / hot-swap callers go through `ReissuingBroadcastHandle` + * in `ReconnectingNestsSpeaker` instead, which manages the audio + + * catalog publishers across session swaps. + */ +internal class MoqLiteBroadcastHandle( + private val broadcaster: NestMoqLiteBroadcaster, + private val publisher: MoqLitePublisherHandle, + private val catalogPublisher: MoqLitePublisherHandle, + private val parent: MoqLiteNestsSpeaker, +) : BroadcastHandle { + @Volatile private var muted: Boolean = false + + @Volatile private var closed: Boolean = false + + override val isMuted: Boolean get() = muted + + override suspend fun setMuted(muted: Boolean) { + if (closed) return + this.muted = muted + broadcaster.setMuted(muted) + parent.reportMuteState(muted) + } + + override suspend fun close() { + if (closed) return + closed = true + // Stop the broadcaster first so the audio capture + encoder + // don't keep producing into a closing publisher. + try { + broadcaster.stop() + } catch (ce: kotlinx.coroutines.CancellationException) { + // Even on cancel, run the rest of cleanup before rethrowing + // — broadcaster.stop already cancels its own job, so the + // mic + encoder + publisher are owed their close paths. + runCatching { catalogPublisher.close() } + runCatching { publisher.close() } + parent.broadcastClosed(this) + throw ce + } catch (_: Throwable) { + // Best-effort; fall through to the defensive publisher.close. + } + // broadcaster.stop() already calls publisher.close(); call again + // defensively to make this method idempotent against partial + // failures on the broadcaster.stop path. + try { + publisher.close() + } catch (ce: kotlinx.coroutines.CancellationException) { + runCatching { catalogPublisher.close() } + parent.broadcastClosed(this) + throw ce + } catch (_: Throwable) { + // Best-effort. + } + try { + catalogPublisher.close() + } catch (ce: kotlinx.coroutines.CancellationException) { + parent.broadcastClosed(this) + throw ce + } catch (_: Throwable) { + // Best-effort. + } + parent.broadcastClosed(this) + } +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsListener.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsListener.kt index 5986ab11c..424ad38d9 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsListener.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsListener.kt @@ -26,12 +26,16 @@ import com.vitorpamplona.nestsclient.moq.SubscribeOk import com.vitorpamplona.nestsclient.moq.lite.MoqLiteAnnounceStatus import com.vitorpamplona.nestsclient.moq.lite.MoqLiteSession import com.vitorpamplona.nestsclient.moq.lite.MoqLiteSubscribeException +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.channelFlow import kotlinx.coroutines.flow.map +import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import java.util.concurrent.atomic.AtomicLong /** @@ -78,14 +82,36 @@ class MoqLiteNestsListener internal constructor( override suspend fun subscribeSpeaker( speakerPubkeyHex: String, maxLatencyMs: Long, - ): SubscribeHandle = wrapSubscription(broadcast = speakerPubkeyHex, track = AUDIO_TRACK, maxLatencyMs = maxLatencyMs) + ): SubscribeHandle = + wrapSubscription( + broadcast = speakerPubkeyHex, + track = AUDIO_TRACK, + maxLatencyMs = maxLatencyMs, + // Audio frames arrive in kixelated/moq `hang` "legacy" + // container layout: each moq-lite frame is + // `varint(timestamp_us) + raw_opus_packet`. Strip the + // leading varint so downstream decoders see a pristine + // Opus packet exactly as if it came from + // [com.vitorpamplona.nestsclient.audio.OpusEncoder]. + stripLegacyTimestamp = true, + ) - override suspend fun subscribeCatalog(speakerPubkeyHex: String): SubscribeHandle = wrapSubscription(broadcast = speakerPubkeyHex, track = CATALOG_TRACK, maxLatencyMs = 0L) + override suspend fun subscribeCatalog(speakerPubkeyHex: String): SubscribeHandle = + wrapSubscription( + broadcast = speakerPubkeyHex, + track = CATALOG_TRACK, + maxLatencyMs = 0L, + // Catalog frames carry raw JSON bytes — no container + // wrapping (the catalog itself is what tells you which + // container the audio track uses). + stripLegacyTimestamp = false, + ) private suspend fun wrapSubscription( broadcast: String, track: String, maxLatencyMs: Long, + stripLegacyTimestamp: Boolean, ): SubscribeHandle { check(state.value is NestsListenerState.Connected) { "NestsListener.subscribe requires Connected state, was ${state.value}" @@ -108,12 +134,13 @@ class MoqLiteNestsListener internal constructor( val objectIdSeq = AtomicLong(0L) val mapped = handle.frames.map { frame -> + val payload = if (stripLegacyTimestamp) stripLegacyTimestampPrefix(frame.payload) else frame.payload MoqObject( trackAlias = handle.id, groupId = frame.groupSequence, objectId = objectIdSeq.getAndIncrement(), publisherPriority = MoqLiteSession.DEFAULT_PRIORITY, - payload = frame.payload, + payload = payload, ) } @@ -127,7 +154,26 @@ class MoqLiteNestsListener internal constructor( } override fun announces(): Flow = - flow { + // `channelFlow` (NOT `flow`) is mandatory here. The downstream + // emission source is `handle.updates`, a MutableSharedFlow + // populated by a launched bidi pump on the session's scope. + // SharedFlow's `collect { lambda }` resumes the lambda inline + // when emit happens — so when the pump emits a chunk, the + // collect lambda runs on the pump's coroutine, not the + // collector's. `flow {}` enforces the "emit only from the + // builder coroutine" invariant and throws + // `IllegalStateException: Flow invariant is violated` the + // moment we call `emit()` from a different coroutine. Two- + // phone production logs (commit 1fc8dbc, run 15:34:40) + // showed exactly this — the first Active arrived, the + // collect lambda fired, emit() threw, the wrapper's + // `runCatching { listener.announces().collect { emit(it) } }` + // swallowed it, `_announcedSpeakers` stayed empty forever, + // and the cliff detector reported `active=1 announced=0` + // indefinitely. `channelFlow` is the documented fix + // (the error message itself recommends it): emissions go + // through a buffered channel that's safe across coroutines. + channelFlow { check(state.value is NestsListenerState.Connected) { "NestsListener.announces requires Connected state, was ${state.value}" } @@ -136,26 +182,48 @@ class MoqLiteNestsListener internal constructor( // suffix is the broadcast's path component within the // room — for nests this is the speaker pubkey hex. val handle = session.announce(prefix = "") - try { - handle.updates.collect { announce -> - emit( - RoomAnnouncement( - pubkey = announce.suffix, - active = announce.status == MoqLiteAnnounceStatus.Active, - ), - ) - } - } finally { - // The flow's `finally` runs on both normal completion - // and cancellation — let cancel propagate after closing - // the announce handle. - try { - handle.close() - } catch (ce: kotlinx.coroutines.CancellationException) { - throw ce - } catch (_: Throwable) { - // Best-effort. + // Run the inner collect on a child of channelFlow's scope + // so we can `awaitClose` on the producer side and let + // close handle the unsub side-effect. Without the launch, + // we'd block here in `collect` forever and never reach + // `awaitClose` — but channelFlow's contract requires + // awaitClose for clean shutdown. + val pump = + launch { + try { + handle.updates.collect { announce -> + send( + RoomAnnouncement( + pubkey = announce.suffix, + active = announce.status == MoqLiteAnnounceStatus.Active, + ), + ) + } + } catch (ce: kotlinx.coroutines.CancellationException) { + throw ce + } catch (_: Throwable) { + // updates flow died — close the channel so the + // consumer sees end-of-flow and the wrapper + // can decide what to do (typically wait for + // the next activeListener emission). + } finally { + // Close the announce bidi on the same scope + // that owned the pump. `handle.close` is + // suspend (it FINs the bidi and joins the + // session-side pump); run it under + // NonCancellable so cancellation of this + // coroutine doesn't skip the FIN. + withContext(NonCancellable) { + runCatching { handle.close() } + } + } } + awaitClose { + // Caller cancelled, OR the wrapping `collectLatest` + // restarted (listener swap). Cancel the pump; its + // finally above runs the suspend close on a + // NonCancellable context. + pump.cancel() } } @@ -197,5 +265,23 @@ class MoqLiteNestsListener internal constructor( * [subscribeCatalog]. */ const val CATALOG_TRACK: String = "catalog.json" + + /** + * Strip the leading `varint(timestamp_us)` prefix from a + * kixelated/moq `hang` "legacy" container frame, returning + * the bare codec payload (e.g. an Opus packet) for downstream + * decoders. The varint length is encoded in the top 2 bits of + * the first byte per RFC 9000 §16: `00`→1, `01`→2, `10`→4, + * `11`→8 bytes. Returns the payload unchanged when the + * varint header would overrun the payload (malformed frame — + * surface upstream rather than silently mask). + */ + internal fun stripLegacyTimestampPrefix(payload: ByteArray): ByteArray { + if (payload.isEmpty()) return payload + val tag = (payload[0].toInt() ushr 6) and 0x3 + val varintLen = 1 shl tag + if (payload.size < varintLen) return payload + return payload.copyOfRange(varintLen, payload.size) + } } } diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt index c719a8cac..0d02d1051 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt @@ -23,8 +23,10 @@ package com.vitorpamplona.nestsclient import com.vitorpamplona.nestsclient.audio.AudioCapture import com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster import com.vitorpamplona.nestsclient.audio.OpusEncoder +import com.vitorpamplona.nestsclient.moq.lite.MoqLiteHangCatalog import com.vitorpamplona.nestsclient.moq.lite.MoqLitePublisherHandle import com.vitorpamplona.nestsclient.moq.lite.MoqLiteSession +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -96,6 +98,26 @@ class MoqLiteNestsSpeaker internal constructor( // session's `activePublisher` slot — otherwise a subsequent // startBroadcasting fails with "already publishing" and the // publisher's announce state never tears down. + // + // Companion catalog publisher: the kixelated/moq browser + // watcher (and any standards-aligned moq-lite consumer) + // discovers a broadcast's tracks by subscribing to the + // `catalog.json` track and parsing the latest group's + // payload as a JSON manifest. Without this track our + // broadcasts are *invisible* to the canonical web watcher + // even though our audio frames are streaming fine — the + // watcher has nothing to subscribe to. Open it alongside + // audio so a single broadcast advertises both tracks. + val catalogPublisher = + try { + session.publish( + broadcastSuffix = speakerPubkeyHex, + track = MoqLiteNestsListener.CATALOG_TRACK, + ) + } catch (t: Throwable) { + runCatching { publisher.close() } + throw t + } val broadcaster = try { NestMoqLiteBroadcaster( @@ -120,9 +142,28 @@ class MoqLiteNestsSpeaker internal constructor( ) } } catch (t: Throwable) { + runCatching { catalogPublisher.close() } runCatching { publisher.close() } throw t } + // Catalog emit-on-subscribe: every time the relay opens a + // SUBSCRIBE bidi for catalog.json, fire the hook to write + // one group + FIN. moq-lite serves new listeners from the + // relay's per-track latest-group cache, so emitting once + // per relay-side subscribe is enough — late-joining + // watchers behind the same relay get the cached blob + // without us having to maintain a periodic re-emit loop. + // Set BEFORE the relay can race a SUBSCRIBE in; in + // practice the relay's SUBSCRIBE bidi takes a network + // round-trip after our ANNOUNCE Active, so this is safe + // even though the setter is non-suspending. + val catalogJson = MoqLiteHangCatalog.OPUS_MONO_48K_AUDIO_DATA_JSON_BYTES + catalogPublisher.setOnNewSubscriber { + runCatching { + catalogPublisher.send(catalogJson) + catalogPublisher.endGroup() + } + } mutableState.value = NestsSpeakerState.Broadcasting( room = current.room, @@ -133,6 +174,7 @@ class MoqLiteNestsSpeaker internal constructor( MoqLiteBroadcastHandle( broadcaster = broadcaster, publisher = publisher, + catalogPublisher = catalogPublisher, parent = this, ) activeHandle = handle @@ -147,7 +189,15 @@ class MoqLiteNestsSpeaker internal constructor( * reconnect wrapper's hot-swap path; not called from the * non-reconnecting path which goes through [startBroadcasting]. */ - override suspend fun openPublisherForHotSwap(track: String): MoqLitePublisherHandle = session.publish(broadcastSuffix = speakerPubkeyHex, track = track) + override suspend fun openPublisherForHotSwap( + track: String, + startSequence: Long, + ): MoqLitePublisherHandle = + session.publish( + broadcastSuffix = speakerPubkeyHex, + track = track, + startSequence = startSequence, + ) /** * Compare-and-clear that runs from inside [close] (already holds @@ -224,88 +274,3 @@ class MoqLiteNestsSpeaker internal constructor( } } } - -internal class MoqLiteBroadcastHandle( - private val broadcaster: NestMoqLiteBroadcaster, - private val publisher: MoqLitePublisherHandle, - private val parent: MoqLiteNestsSpeaker, -) : BroadcastHandle { - @Volatile private var muted: Boolean = false - - @Volatile private var closed: Boolean = false - - override val isMuted: Boolean get() = muted - - override suspend fun setMuted(muted: Boolean) { - if (closed) return - this.muted = muted - broadcaster.setMuted(muted) - parent.reportMuteState(muted) - } - - override suspend fun close() { - if (closed) return - closed = true - try { - broadcaster.stop() - } catch (ce: kotlinx.coroutines.CancellationException) { - // Even on cancel, run the rest of cleanup before rethrowing - // — broadcaster.stop already cancels its own job, so the - // mic + encoder + publisher are owed their close paths. - runCatching { publisher.close() } - parent.broadcastClosed(this) - throw ce - } catch (_: Throwable) { - // Best-effort; fall through to the defensive publisher.close. - } - // broadcaster.stop() already calls publisher.close(); call again - // defensively to make this method idempotent against partial - // failures on the broadcaster.stop path. - try { - publisher.close() - } catch (ce: kotlinx.coroutines.CancellationException) { - parent.broadcastClosed(this) - throw ce - } catch (_: Throwable) { - // Best-effort. - } - parent.broadcastClosed(this) - } -} - -/** - * Internal hot-swap seam: speakers that expose this interface let the - * reconnect wrapper retarget a long-lived - * [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster] onto a - * freshly-opened moq-lite session's publisher without restarting the - * AudioRecord / Opus encoder pipeline. Implemented by - * [MoqLiteNestsSpeaker]; not implemented by the IETF reference - * [DefaultNestsSpeaker], which falls back to the close-then-restart path - * inside [com.vitorpamplona.nestsclient.connectReconnectingNestsSpeaker]. - * - * The wrapper uses an `as?` cast to detect support so this interface - * can stay package-internal — protocol consumers never see it. - */ -internal interface HotSwappablePublisherSource { - /** - * Open a fresh [MoqLitePublisherHandle] on the underlying moq-lite - * session. Caller owns the returned handle's lifetime (typically - * via [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster.swapPublisher]'s - * close-the-old contract). - */ - suspend fun openPublisherForHotSwap(track: String): MoqLitePublisherHandle - - /** - * Surface a broadcast-pipeline terminal failure (e.g. sustained - * `publisher.send` errors past - * [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster.MAX_CONSECUTIVE_SEND_ERRORS]) - * by flipping the speaker's state to [NestsSpeakerState.Failed]. - * Called by the hot-swap pump when the long-lived broadcaster's - * `onTerminalFailure` fires; lets the reconnect orchestrator - * observe the terminal state and recycle the session, matching - * the legacy - * [MoqLiteNestsSpeaker.startBroadcasting] path's failure - * propagation. - */ - fun reportBroadcastTerminalFailure() -} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsListener.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsListener.kt index 6c4e726d9..6ec082b75 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsListener.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsListener.kt @@ -29,6 +29,7 @@ import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.channels.BufferOverflow +import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow @@ -37,9 +38,9 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.channelFlow import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.isActive import kotlinx.coroutines.launch @@ -251,8 +252,29 @@ private class ReconnectingHandle( * swallowed so a future moq-lite session keeps emitting. */ override fun announces(): Flow = - flow { + // `channelFlow` (NOT `flow`) is required here for the same + // reason `MoqLiteNestsListener.announces` is channelFlow: + // we drive emissions from a `collectLatest` that internally + // launches a child coroutine per `activeListener` emission. + // `flow {}`'s SafeFlow guard rejects emissions from any + // coroutine other than the flow-builder's own — so on the + // very first listener emission the inner `emit(it)` lands + // in a child coroutine of `collectLatest`, throws + // IllegalStateException, the consumer's `runCatching` + // swallows it, and `_announcedSpeakers` never populates. + // Two-phone production logs at commit f17e7ad showed this + // exact failure even AFTER the inner listener was switched + // to channelFlow — the wrapper layer was the second + // un-fixed `flow {}`. `channelFlow` + `send(...)` instead + // of `emit(...)` allows cross-coroutine production, which + // is exactly what `collectLatest`'s per-emission child + // coroutines need. + channelFlow { + Log.d("NestRx") { "wrapper.announces() channelFlow starting collect on activeListener" } + var iter = 0 activeListener.collectLatest { listener -> + iter += 1 + Log.d("NestRx") { "wrapper.announces() iter=$iter activeListener=${if (listener == null) "null" else "set"}" } if (listener == null) return@collectLatest val terminalOrConnected = listener.state.first { state -> @@ -260,11 +282,27 @@ private class ReconnectingHandle( state is NestsListenerState.Closed || state is NestsListenerState.Failed } + Log.d("NestRx") { "wrapper.announces() iter=$iter inner state=${terminalOrConnected::class.simpleName}" } if (terminalOrConnected !is NestsListenerState.Connected) return@collectLatest - runCatching { - listener.announces().collect { emit(it) } + var fwd = 0 + val outcome = + runCatching { + listener.announces().collect { + fwd += 1 + send(it) + } + } + Log.w("NestRx") { + val why = outcome.exceptionOrNull()?.let { "${it::class.simpleName}: ${it.message}" } ?: "naturally" + "wrapper.announces() iter=$iter inner collect ended $why fwd=$fwd" } } + // collectLatest above never returns naturally — it + // collects activeListener forever. awaitClose fires when + // the consumer cancels the channelFlow, at which point + // the collectLatest is cancelled too via structured + // concurrency. + awaitClose { } } /** @@ -392,11 +430,17 @@ private class ReconnectingHandle( // inheriting the last failure window's saturation). subscribeRetryDelayMs = SUBSCRIBE_RETRY_BACKOFF_INITIAL_MS liveHandleRef.set(handle) + Log.d("NestRx") { "wrapper: handle attached id=${handle.subscribeId}, starting collect" } + var emitted = 0L try { - handle.objects.collect { frames.emit(it) } + handle.objects.collect { + emitted += 1 + frames.emit(it) + } } finally { if (liveHandleRef.get() === handle) liveHandleRef.set(null) } + Log.w("NestRx") { "wrapper: handle objects flow ENDED id=${handle.subscribeId} emitted=$emitted — re-issuing after ${RESUBSCRIBE_BACKOFF_MS}ms" } // Brief backoff so a permanently-gone // publisher doesn't tight-loop the relay // with re-subscribes. 100 ms stays well @@ -470,18 +514,23 @@ private class ReconnectingHandle( // before the publisher exists, and the relay FINs the bidi // without a SubscribeOk/Drop reply. // - // - INITIAL = 250 ms: under moq-rs's typical announce-propagation - // latency (< 200 ms in production traces), so the very first - // retry usually succeeds and the listener-side buffer hides - // the gap. - // - Doubles each miss → 500 ms → 1 000 ms. - // - MAX = 1 000 ms: matches the previous flat constant; long - // enough that a never-arriving publisher doesn't hammer the - // relay, short enough to stay under the wrapper SharedFlow's - // ~1.3 s buffer once playback is rolling on the next attempt. + // - INITIAL = 100 ms: lower than the prior 250 ms because + // join-time logs (`claude/fix-nests-audio-receiver-HCgOY`) + // showed the listener was paying ~4 s of dead air on every + // first-join while the broadcaster's session warmed up + // on the relay (5 retries: 250+500+1000+1000+1000 = 3.75 s). + // 100 ms initial halves that to ~1.9 s + // (100+200+400+800+1000) without thrashing the relay on a + // permanently-gone publisher (the MAX cap below still + // bounds the total per-attempt rate). + // - Doubles each miss → 200 ms → 400 ms → 800 ms → 1 000 ms. + // - MAX = 1 000 ms: long enough that a never-arriving + // publisher doesn't hammer the relay; short enough to stay + // under the wrapper SharedFlow's ~1.3 s buffer once playback + // is rolling on the next attempt. // - Reset on first successful subscribe so a subsequent // publisher-cycle gap doesn't inherit the previous saturation. - private const val SUBSCRIBE_RETRY_BACKOFF_INITIAL_MS = 250L + private const val SUBSCRIBE_RETRY_BACKOFF_INITIAL_MS = 100L private const val SUBSCRIBE_RETRY_BACKOFF_MAX_MS = 1_000L private val SYNTH_OK = diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsSpeaker.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsSpeaker.kt index a6ed6e68d..897a93656 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsSpeaker.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsSpeaker.kt @@ -23,6 +23,8 @@ package com.vitorpamplona.nestsclient import com.vitorpamplona.nestsclient.audio.AudioCapture import com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster import com.vitorpamplona.nestsclient.audio.OpusEncoder +import com.vitorpamplona.nestsclient.moq.lite.MoqLiteHangCatalog +import com.vitorpamplona.nestsclient.moq.lite.MoqLitePublisherHandle import com.vitorpamplona.nestsclient.transport.WebTransportFactory import com.vitorpamplona.quartz.nip01Core.signers.NostrSigner import kotlinx.coroutines.CoroutineScope @@ -427,6 +429,18 @@ private class ReissuingBroadcastHandle( /** Hot-swap path's long-lived broadcaster. Null until the first session arrives. */ @Volatile private var hotSwapBroadcaster: NestMoqLiteBroadcaster? = null + /** + * Hot-swap path's per-session catalog publisher and its periodic + * republish job. Catalog has no long-lived encoder pipeline (unlike + * audio), so each session gets a fresh publisher + republish loop; + * cleanup happens at the start of the next iteration after the new + * pair is installed (mirroring how the old audio publisher is + * closed only after [NestMoqLiteBroadcaster.swapPublisher] returns + * it). Both nullable so [close] can no-op when no iteration ever + * ran (i.e. wrapper closed before the first session connected). + */ + @Volatile private var hotSwapCatalogPublisher: MoqLitePublisherHandle? = null + /** Legacy path's per-session handle. Cleared when the session swaps. */ private val liveHandle = AtomicReference(null) private var pumpJob: Job? = null @@ -482,9 +496,25 @@ private class ReissuingBroadcastHandle( * when the wrapper itself closes. */ private suspend fun runHotSwapIteration(hotSwap: HotSwappablePublisherSource) { + // Carry the previous session's audio-track group sequence + // forward so kixelated/hang's `Container.Consumer.#run` + // doesn't drop every post-recycle group as `sequence < + // #active`. Read BEFORE opening the new publisher — there is + // a tiny race window where the broadcaster could send one + // more group on the old publisher between our read and the + // swap-snapshot below; in practice that window is microseconds + // (the broadcaster's swap is a single volatile write) and the + // broadcaster's group cadence is 1 group/sec at the production + // [NestMoqLiteBroadcaster.framesPerGroup], so the chance of a + // duplicate sequence is negligible. Worst case the watcher + // briefly stalls on one duplicate then catches up. + val startSequence: Long = hotSwapBroadcaster?.currentPublisher?.nextSequence ?: 0L val newPublisher = try { - hotSwap.openPublisherForHotSwap(MoqLiteNestsListener.AUDIO_TRACK) + hotSwap.openPublisherForHotSwap( + track = MoqLiteNestsListener.AUDIO_TRACK, + startSequence = startSequence, + ) } catch (ce: kotlinx.coroutines.CancellationException) { throw ce } catch (_: Throwable) { @@ -549,16 +579,60 @@ private class ReissuingBroadcastHandle( if (old != null) runCatching { old.close() } } + // Catalog track on the new session. Keeps the broadcast + // discoverable to standards-aligned moq-lite watchers (the + // kixelated/moq browser reference) across JWT refresh — without + // this the catalog goes silent the moment the session recycles + // and any watcher that attaches AFTER the recycle sees nothing + // to subscribe to. Mirror of [MoqLiteNestsSpeaker.startBroadcasting]'s + // catalog setup; same JSON, same emit-on-subscribe pattern. + val catalogPayload = MoqLiteHangCatalog.OPUS_MONO_48K_AUDIO_DATA_JSON_BYTES + val priorCatalogPublisher = hotSwapCatalogPublisher + val newCatalogPublisher = + try { + hotSwap.openPublisherForHotSwap(MoqLiteNestsListener.CATALOG_TRACK) + } catch (ce: kotlinx.coroutines.CancellationException) { + throw ce + } catch (_: Throwable) { + // Couldn't mint a catalog publisher on this session. + // Audio path is already live; treat as non-fatal — the + // next session swap retries. Leave prior catalog state + // alone (it's about to die with the prior session). + null + } + if (newCatalogPublisher != null) { + // Set the emit-on-subscribe hook BEFORE installing the + // publisher reference, so the relay can't race a SUBSCRIBE + // in between. + newCatalogPublisher.setOnNewSubscriber { + runCatching { + newCatalogPublisher.send(catalogPayload) + newCatalogPublisher.endGroup() + } + } + hotSwapCatalogPublisher = newCatalogPublisher + // Tear down the prior session's catalog publisher AFTER the + // new one is installed so the watcher only sees a brief + // overlap rather than a gap. The prior publisher's session + // is about to be torn down by the orchestrator anyway, but + // graceful Announce(Ended) keeps the relay book-keeping + // clean. + if (priorCatalogPublisher != null) runCatching { priorCatalogPublisher.close() } + } + try { // Park until [collectLatest] cancels us on the next session // swap, OR [close] cancels [pumpJob]. The broadcaster keeps // running through the cancellation; only close() stops it. awaitCancellation() } finally { - // Intentionally do NOT close the broadcaster here. - // collectLatest cancels this iteration on every session - // swap; closing the broadcaster would create the exact - // 50–150 ms gap this whole path exists to avoid. + // Intentionally do NOT close the broadcaster or the catalog + // publisher here. collectLatest cancels this iteration on + // every session swap; closing now would force the catalog + // republisher to drop a beat between sessions. The next + // iteration handles teardown of the prior catalog after + // installing its replacement (above), and [close] handles + // teardown when the wrapper itself closes. } } @@ -615,6 +689,15 @@ private class ReissuingBroadcastHandle( // current publisher (broadcaster.stop calls publisher.close // internally). For legacy, the per-session handle's close // releases its own broadcaster. + // + // Catalog gets explicit teardown because — unlike the audio + // publisher which broadcaster.stop() closes for us — the + // catalog publisher has no broadcaster wrapper. The + // emit-on-subscribe hook itself doesn't need shutdown: it + // only fires inside [PublisherStateImpl.registerInboundSubscription], + // and closing the publisher prevents any further hook fires. + hotSwapCatalogPublisher?.let { runCatching { it.close() } } + hotSwapCatalogPublisher = null hotSwapBroadcaster?.let { runCatching { it.stop() } } hotSwapBroadcaster = null liveHandle.getAndSet(null)?.let { runCatching { it.close() } } diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/Audio.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/Audio.kt index e015a49c1..c2be91b94 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/Audio.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/Audio.kt @@ -36,6 +36,15 @@ object AudioFormat { /** 20 ms at 48 kHz. */ const val FRAME_SIZE_SAMPLES: Int = 960 + /** + * Duration of one Opus frame in microseconds — derived from + * [FRAME_SIZE_SAMPLES] / [SAMPLE_RATE_HZ]. Used by the publisher + * to stamp each frame at frame-index × this value, giving hang.js's + * watcher a perfect 20 ms cadence even when the broadcaster's send + * loop suspends on transport backpressure. + */ + const val FRAME_DURATION_US: Long = (FRAME_SIZE_SAMPLES.toLong() * 1_000_000L) / SAMPLE_RATE_HZ + /** Bytes per PCM 16-bit sample. */ const val BYTES_PER_SAMPLE: Int = 2 } diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestMoqLiteBroadcaster.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestMoqLiteBroadcaster.kt index 385a68188..208ef3a6a 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestMoqLiteBroadcaster.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestMoqLiteBroadcaster.kt @@ -21,6 +21,8 @@ package com.vitorpamplona.nestsclient.audio import com.vitorpamplona.nestsclient.moq.lite.MoqLitePublisherHandle +import com.vitorpamplona.quartz.utils.Log +import com.vitorpamplona.quic.Varint import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job @@ -118,6 +120,18 @@ class NestMoqLiteBroadcaster( */ @Volatile private var publisher: MoqLitePublisherHandle = initialPublisher + /** + * Volatile read of the broadcaster's current publisher reference, + * for callers (typically the hot-swap pump in + * [com.vitorpamplona.nestsclient.connectReconnectingNestsSpeaker]) + * that want to read its [MoqLitePublisherHandle.nextSequence] + * before swapping in a fresh publisher. Don't use this to send + * — that contract belongs to the broadcaster's send loop and + * the caller would race the loop's swap-snapshot. + */ + val currentPublisher: MoqLitePublisherHandle + get() = publisher + /** * Start capturing + encoding + publishing in the background. * Returns immediately. Calling twice is an error. If @@ -181,6 +195,10 @@ class NestMoqLiteBroadcaster( // we bail. publisher.send returning `false` (no inbound // subscriber) is NOT counted — empty rooms are normal. var consecutiveSendErrors = 0 + // Diagnostic counters: throttled logging at 50Hz capture rate + // would flood logcat without these. + var sentFrames: Long = 0L + var droppedNoSubFrames: Long = 0L // Track which publisher we last sent to. On swapPublisher // (JWT-refresh hot swap), the snapshot below picks up the // new reference; we reset framesInCurrentGroup so the @@ -191,9 +209,68 @@ class NestMoqLiteBroadcaster( // and the relay would see two unrelated uni streams under // the same logical group. var lastPublisher: MoqLitePublisherHandle = publisher + // kixelated/moq `hang` "legacy" container wire format: + // every frame inside a moq-lite group is + // varint(timestamp_us) + raw_codec_payload + // (`rs/hang/src/container/frame.rs`, + // Timescale<1_000_000>). Watchers that read our + // catalog's `container.kind = "legacy"` declaration + // will skip the leading varint as a microsecond + // timestamp; they MUST receive a real timestamp or + // their decoder picks up garbage bytes ahead of the + // Opus packet. + // + // Frame-index-derived timestamp (NOT wall clock). Each + // captured PCM frame occupies exactly one + // [AudioFormat.FRAME_DURATION_US] slot in the timeline, + // and the timestamp is `nextFrameIndex * + // FRAME_DURATION_US` snapped to that grid. This matters + // because: + // - `current.send(...)` suspends on transport + // backpressure; a wall-clock timestamp captured at + // send-time would drift forward of the actual + // capture time of the audio sample, and the watcher's + // WebCodecs AudioDecoder would schedule playback + // at the wrong instant (audible offset between + // speaker and listener). + // - The capture loop is wall-clock-pinned anyway — + // `capture.readFrame()` blocks until the next 20 ms + // of PCM is ready — so frame-index advances at the + // same rate as wall time across the whole + // broadcast lifetime, with no codec drift relative + // to the watcher's playback clock. + // + // The counter advances on EVERY captured PCM frame — + // including encoder failures, empty-encoded frames, and + // muted frames — so a mute gap shows up on the wire as + // a real wall-clock gap (timestamps jump forward by the + // muted duration) rather than collapsing the silence. + // Survives publisher hot-swap (single-broadcast, + // single-encoder; the new publisher inherits the + // running counter) for the same reason the old + // wall-clock TimeMark did: timestamps are codec-payload + // metadata, not stream-position. + var nextFrameIndex = 0L + // Mute-edge tracking. On the unmuted→muted transition we + // FIN the open uni stream so the watcher sees a clean + // group boundary instead of a half-delivered group that + // never completes — kixelated/hang's + // `Container.Consumer.#runGroup` parks on + // `await group.consumer.readFrame()` until either a + // frame arrives or the group hits FIN, and renders a + // "stalled" indicator while it waits. Without the FIN, + // muting Amethyst lights up that indicator on every web + // watcher even though the broadcast is intentionally + // silent. The FIN clears it; unmuting opens a fresh + // group cleanly via the standard + // `currentGroup ?: openNextGroupLocked()` path inside + // `PublisherStateImpl.send`. + var wasMuted = false try { while (true) { val pcm = capture.readFrame() ?: break + val timestampUs = nextFrameIndex * AudioFormat.FRAME_DURATION_US + nextFrameIndex += 1 val opus = try { encoder.encode(pcm) @@ -210,7 +287,27 @@ class NestMoqLiteBroadcaster( continue } if (opus.isEmpty()) continue - if (muted) continue + if (muted) { + // Unmuted → muted edge: FIN the current + // group's uni stream once. Doesn't reset + // [framesInCurrentGroup] since + // `endGroup` is a no-op when no group is + // open, and the next post-unmute send will + // open a fresh group anyway. `runCatching` + // because a transport-side close during + // mute is non-fatal — the broadcaster's + // terminal-failure path picks up real + // breakage on the next live frame. + if (!wasMuted) { + wasMuted = true + runCatching { publisher.endGroup() } + framesInCurrentGroup = 0 + } + continue + } + // Muted → unmuted edge: clear the latch so the + // next mute transition fires endGroup again. + wasMuted = false // Snapshot the publisher reference once per frame. // If [swapPublisher] mid-loop installed a new // reference, pick it up here and reset the group @@ -229,25 +326,63 @@ class NestMoqLiteBroadcaster( // for the production cliff this works around. val sendOutcome = runCatching { - current.send(opus) + // Single-allocation framing: write the + // timestamp varint directly into a buffer + // sized for `varint + opus`, then copy + // the opus bytes after it. The earlier + // shape (`Varint.encode(...) + opus`) + // allocated twice per frame — once for + // the varint ByteArray, once for the + // concatenated payload. At 50 fps × N + // speakers this is measurable young-gen + // pressure on the audio hot path; the + // mirror optimisation already lives in + // `PublisherStateImpl.send` for the + // outer size-prefix wrap. + val tsLen = Varint.size(timestampUs) + val payload = ByteArray(tsLen + opus.size) + Varint.writeTo(timestampUs, payload, 0) + opus.copyInto(payload, tsLen) + val accepted = current.send(payload) framesInCurrentGroup += 1 if (framesInCurrentGroup >= framesPerGroup) { current.endGroup() framesInCurrentGroup = 0 } + accepted } sendOutcome - .onSuccess { + .onSuccess { accepted -> consecutiveSendErrors = 0 - // Fire the speaking-ring tap only on - // a successful send. If the publisher - // throws (transport gone, peer dead), - // the frame didn't actually go out and - // the UI shouldn't claim we're talking. - onLevel(peakAmplitude(pcm)) + if (accepted) { + sentFrames += 1 + if (sentFrames % SEND_LOG_THROTTLE == 0L) { + Log.d("NestTx") { + "broadcaster sent frame #$sentFrames (group $framesInCurrentGroup/$framesPerGroup)" + } + } + // Only tap the speaking-ring on a frame + // that actually reached an inbound + // subscriber. Without this gate the local + // ring lights up during the pre-subscribe + // window AND after a relay-side cliff — + // the speaker would believe they're being + // heard when no audio is on the wire. + onLevel(peakAmplitude(pcm)) + } else { + droppedNoSubFrames += 1 + if (droppedNoSubFrames % SEND_LOG_THROTTLE == 0L) { + Log.w("NestTx") { + "broadcaster send returned false — frame dropped (count=$droppedNoSubFrames, sent=$sentFrames)" + } + } + } }.onFailure { t -> if (t is CancellationException) throw t consecutiveSendErrors += 1 + Log.w("NestTx") { + "broadcaster send threw (consecutive=$consecutiveSendErrors): ${t::class.simpleName}: ${t.message}" + } onError( AudioException( AudioException.Kind.PlaybackFailed, @@ -359,15 +494,53 @@ class NestMoqLiteBroadcaster( companion object { /** - * Default moq-lite group size = 5 Opus frames ≈ 100 ms of audio. - * Picked to keep the QUIC uni-stream creation rate - * (10 streams/sec at 20 ms cadence) under the production - * nostrnests relay's sustained per-subscriber forward - * ceiling (~40 streams/sec) while still giving late-joining - * subscribers a sub-100 ms initial audio gap. See - * [framesPerGroup] kdoc for the full rationale + history. + * Default moq-lite group size = 50 Opus frames ≈ 1 s of audio. + * + * The relay-side cliff this defends against is per-subscriber + * forward-stream-rate, not per-frame or per-byte: moq-rs + * starts FIN'ing the per-subscriber publisher pipe once its + * per-subscriber uni-stream creation queue can't drain fast + * enough. Two-phone production tests on + * `claude/fix-nests-audio-receiver-HCgOY` showed: + * + * - `framesPerGroup = 10` (5 streams/sec) → cliff after + * ~16 s of streaming, ~6 s of audio lost at the tail + * (commit 6e4df4a logcat, run 18:37:43..18:38:08) + * - `framesPerGroup = 5` (10 streams/sec) → cliff after + * ~13 s of streaming, same dropout shape (commit + * d6517cf logcat run 14:25 — original bug report). + * + * 50 frames/group → 1 stream/sec at the production 50 fps + * Opus cadence. The relay's per-subscriber forward queue + * has measurable headroom at 1 stream/sec — sweep tests + * (`fpg-all` = 100 frames in a single group) consistently + * deliver 100/100 in production, and `fpg20` similarly + * passes. We pick 50 (not 100) because: + * + * - Late-join gap is bounded by group size: a listener + * joining mid-broadcast has to wait until the next + * group boundary for the first frame. 50 frames = 1 s + * gap, which matches what the existing + * `ROOM_PLAYER_PREROLL_FRAMES = 10` audio-buffer + the + * ~250 ms AudioTrack jitter buffer was already designed + * to mask. 100 frames (2 s) is past that. + * - QUIC stream-level reliability: each group is one uni + * stream, and a stream RST loses the whole group. 1 s + * of audio loss on RST is bearable; 2 s is noticeably + * a "skip". + * - Sender-side encode latency stays at 1 s — no worse + * than the natural buffering audio rooms already do. + * + * Pair with the listener-side cliff-detector in + * `NestViewModel` as belt-and-suspenders: if 50 frames/group + * STILL hits a cliff (relay version drift, network + * congestion, etc.) the detector recycles the transport so + * the next session reopens the relay's queue. + * + * Late-join gap: ≤ 1 s (one group boundary) — within the + * existing audio-room buffering envelope. */ - const val DEFAULT_FRAMES_PER_GROUP: Int = 5 + const val DEFAULT_FRAMES_PER_GROUP: Int = 50 /** * Maximum consecutive [MoqLitePublisherHandle.send] / [endGroup] @@ -377,5 +550,9 @@ class NestMoqLiteBroadcaster( * transport is irrecoverably dead. */ const val MAX_CONSECUTIVE_SEND_ERRORS: Int = 250 + + // Diagnostic: log every Nth frame (sent or dropped) so a sustained + // window doesn't flood logcat at 50 fps. + private const val SEND_LOG_THROTTLE: Long = 50L } } diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestPlayer.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestPlayer.kt index fe641b994..84b33f67e 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestPlayer.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestPlayer.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.nestsclient.audio import com.vitorpamplona.nestsclient.moq.MoqObject +import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job @@ -42,7 +43,7 @@ import kotlinx.coroutines.launch * decoder. Idempotent. */ class NestPlayer( - private val decoder: OpusDecoder, + initialDecoder: OpusDecoder, private val player: AudioPlayer, private val scope: CoroutineScope, /** @@ -61,11 +62,44 @@ class NestPlayer( * Default is `0` so existing tests stand without modification. */ private val prerollFrames: Int = 0, + /** + * Optional factory called on every detected publisher boundary + * (track-alias change in the inbound [MoqObject] stream) to mint a + * fresh [OpusDecoder]. Used by the listener wrapper's re-issuing + * subscription pump + * ([com.vitorpamplona.nestsclient.ReconnectingNestsListener.reissuingSubscribe]): + * each new SUBSCRIBE through the relay produces objects with a + * different `trackAlias`, but they're spliced into the same + * `SharedFlow` — without a decoder reset on the boundary, Opus's + * predictor state from the prior publisher's last frame is fed + * into the new publisher's first frame, producing an audible + * warble at every JWT-refresh hot-swap on the speaker side OR + * cliff-detector recycle on the listener side. + * + * Default `null` keeps the legacy behaviour (no boundary + * detection, decoder lives for the player's whole lifetime) so + * existing tests / callers that don't care about boundaries + * stand unchanged. Production callers in + * [com.vitorpamplona.amethyst.commons.viewmodels.NestViewModel.openSubscription] + * pass a closure that captures the per-subscription channel-count + * and rebuilds via `decoderFactory(channelCount)`. + */ + private val decoderFactory: (() -> OpusDecoder)? = null, ) { init { require(prerollFrames >= 0) { "prerollFrames must be >= 0, got $prerollFrames" } } + /** + * Active decoder. Replaced on detected publisher boundary when + * [decoderFactory] is non-null. `var` so the boundary path can + * release + rebuild without changing the rest of the loop's + * decoder reference; the `private` confines mutation to this + * class, and the decode loop runs single-coroutine so no cross- + * thread visibility hazards. + */ + private var decoder: OpusDecoder = initialDecoder + private var job: Job? = null private var stopped = false @@ -122,6 +156,15 @@ class NestPlayer( scope.launch { val preroll = ArrayDeque(prerollFrames.coerceAtLeast(1)) var playbackBegun = false + // Diagnostic counters: at 50 fps a per-frame log floods logcat; + // throttle to every Nth event. + var receivedObjects: Long = 0L + var decodedFrames: Long = 0L + var emptyDecodes: Long = 0L + var enqueued: Long = 0L + Log.d("NestPlay") { + "NestPlayer.play started (prerollFrames=$prerollFrames)" + } suspend fun beginAndFlushIfNeeded() { if (playbackBegun) return @@ -137,20 +180,84 @@ class NestPlayer( // hardware starts pulling samples; getting // [enqueue] in first means the very first sample // pulled is from our pre-rolled audio, not silence. + Log.d("NestPlay") { + "NestPlayer flushing preroll (${preroll.size} frames) → beginPlayback" + } while (preroll.isNotEmpty()) { player.enqueue(preroll.removeFirst()) } player.beginPlayback() playbackBegun = true + Log + .d("NestPlay") { "NestPlayer beginPlayback returned" } } + // Track-alias of the most recently observed object. + // A change signals a publisher boundary (re-issuing + // subscription wrapper spliced in a new SUBSCRIBE). + // Only consulted when [decoderFactory] is non-null; + // legacy callers without a factory keep the prior + // single-decoder behaviour. + var lastTrackAlias: Long? = null try { objects.collect { obj -> + receivedObjects += 1 + if (receivedObjects % PLAY_LOG_THROTTLE == 0L) { + Log.d("NestPlay") { + "NestPlayer received obj #$receivedObjects (decoded=$decodedFrames empty=$emptyDecodes enqueued=$enqueued playbackBegun=$playbackBegun)" + } + } + // Publisher-boundary detection: if the trackAlias + // changed since the last object AND we have a + // factory to mint a fresh decoder, release the + // current decoder + build a new one. Without + // this, Opus's predictor state from the prior + // publisher's last frame is fed into the new + // publisher's first frame, producing audible + // warble at every JWT-refresh hot-swap (speaker + // side) or cliff-detector recycle (listener side). + // The prior-trackAlias guard avoids a spurious + // rebuild on the very first frame. + // + // Build the replacement BEFORE releasing the old + // decoder so a factory failure (e.g. MediaCodec + // contention, audio policy denial mid-session) + // doesn't leave the field referencing a + // released codec — every subsequent decode + // would then throw `IllegalStateException` with + // no recovery path. On factory failure, log + // and keep using the existing decoder; cross- + // publisher predictor state is wrong but at + // least audio keeps playing. + val factory = decoderFactory + if (factory != null && lastTrackAlias != null && obj.trackAlias != lastTrackAlias) { + val replacement = + runCatching { factory() } + .onFailure { t -> + if (t is CancellationException) throw t + Log.w("NestPlay") { + "NestPlayer decoder factory threw on trackAlias " + + "$lastTrackAlias → ${obj.trackAlias}; keeping old decoder " + + "(${t::class.simpleName}: ${t.message})" + } + }.getOrNull() + if (replacement != null) { + Log.d("NestPlay") { + "NestPlayer publisher boundary: trackAlias $lastTrackAlias → ${obj.trackAlias}; rebuilding decoder" + } + runCatching { decoder.release() } + decoder = replacement + } + } + lastTrackAlias = obj.trackAlias val pcm = try { decoder.decode(obj.payload) } catch (ce: CancellationException) { throw ce } catch (t: Throwable) { + Log.w("NestPlay") { + "decoder.decode threw on obj #$receivedObjects: ${t::class.simpleName}: ${t.message}" + } onError( AudioException( AudioException.Kind.DecoderError, @@ -160,10 +267,26 @@ class NestPlayer( ) return@collect } - if (pcm.isNotEmpty()) { + if (pcm.isEmpty()) { + emptyDecodes += 1 + if (emptyDecodes % PLAY_LOG_THROTTLE == 0L) { + Log.w("NestPlay") { + "decoder returned empty pcm (count=$emptyDecodes / received=$receivedObjects)" + } + } + } else { + decodedFrames += 1 onLevel(peakAmplitude(pcm)) if (playbackBegun) { + val enqueueStart = System.currentTimeMillis() player.enqueue(pcm) + val enqueueMs = System.currentTimeMillis() - enqueueStart + enqueued += 1 + if (enqueued % PLAY_LOG_THROTTLE == 0L || enqueueMs > 50) { + Log.d("NestPlay") { + "NestPlayer enqueued #$enqueued (took ${enqueueMs}ms)" + } + } } else { preroll.addLast(pcm) if (preroll.size >= prerollFrames) { @@ -172,14 +295,23 @@ class NestPlayer( } } } + Log.w("NestPlay") { + "NestPlayer objects flow COMPLETED (received=$receivedObjects decoded=$decodedFrames empty=$emptyDecodes enqueued=$enqueued)" + } // Flow ended without enough frames to fill the pre-roll // (e.g. the publisher cycled before pre-roll was full, // or the room ended). Flush whatever's queued so any // already-decoded audio still reaches the device. beginAndFlushIfNeeded() } catch (ce: CancellationException) { + Log.d("NestPlay") { + "NestPlayer cancelled (received=$receivedObjects decoded=$decodedFrames enqueued=$enqueued)" + } throw ce } catch (t: Throwable) { + Log.w("NestPlay") { + "NestPlayer pipeline threw: ${t::class.simpleName}: ${t.message}" + } onError( AudioException( AudioException.Kind.PlaybackFailed, @@ -203,8 +335,16 @@ class NestPlayer( suspend fun stop() { if (stopped) return stopped = true + Log + .d("NestPlay") { "NestPlayer.stop()" } job?.cancelAndJoin() runCatching { player.stop() } runCatching { decoder.release() } } + + companion object { + // Diagnostic throttle: log every Nth event during normal flow so a + // sustained 50 fps stream doesn't flood logcat. + private const val PLAY_LOG_THROTTLE: Long = 50L + } } diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteAlpn.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteAlpn.kt new file mode 100644 index 000000000..2c3186ef6 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteAlpn.kt @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.moq.lite + +/** + * moq-lite ALPN strings. ONLY [LITE_03] is wire-compatible with the + * codec in [MoqLiteCodec]; the other constants are kept for + * documentation and future codec upgrades. + * + * Subscribe / Group framing is identical across Lite-03 and Lite-04, + * but Lite-04 reshapes Announce.hops into an `OriginList` + * (`kixelated/moq` commit 45db108, "moq-lite/moq-relay: hop-based + * clustering"), adds an `exclude_hop` field to AnnounceInterest, and + * adds an `rtt` field to Probe. A relay that picks Lite-04 with our + * Lite-03 codec on the wire desyncs on the first Announce exchange. + * Don't add [LITE_04] to `wt-available-protocols` until + * [MoqLiteCodec] is version-aware and [MoqLiteAnnounce.hops] is a + * list rather than a single varint. + * + * `"moql"` is the legacy combined ALPN that requires an in-band SETUP + * exchange — kept here for completeness; not advertised by the + * factory. + * + * Source: `kixelated/moq/rs/moq-lite/src/lite/version.rs`, + * `kixelated/moq/rs/moq-lite/src/lite/announce.rs:11-105`, + * `kixelated/moq/rs/moq-lite/src/lite/probe.rs:9-55`. + */ +object MoqLiteAlpn { + const val LITE_03: String = "moq-lite-03" + + /** + * `moq-lite-04` ALPN string. Wire-incompatible with [MoqLiteCodec] + * today — see the object kdoc for the codec diff. Defined here so + * a future patch that lands version-aware Announce / Probe codecs + * can drop it into the [QuicWebTransportFactory] sub-protocol list + * without re-deriving the constant. + */ + const val LITE_04: String = "moq-lite-04" + const val LEGACY: String = "moql" +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteCodec.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteCodec.kt index 3d5b6445a..239a41d55 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteCodec.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteCodec.kt @@ -251,6 +251,20 @@ object MoqLiteCodec { return MoqLiteProbe(bitrate = bitrate) } + /** + * Encode a single Lite-03 Probe message body + * (`lite/probe.rs` — `bitrate: u62` only; `rtt` is Lite-04+). + * The publisher writes these size-prefixed onto a Probe bidi the + * subscriber opened, advertising the publisher's expected + * bandwidth. Wrapping (size prefix) is the caller's responsibility, + * matching [encodeAnnouncePlease] / [encodeAnnounce]. + */ + fun encodeProbe(probe: MoqLiteProbe): ByteArray { + val body = MoqWriter() + body.writeVarint(probe.bitrate) + return wrapSizePrefixed(body) + } + // ---------------- internals ---------------- /** diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteControlCodes.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteControlCodes.kt new file mode 100644 index 000000000..2d55a2c81 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteControlCodes.kt @@ -0,0 +1,135 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.moq.lite + +// Wire-format discriminators for moq-lite control flow. +// +// The four enums below share a common job: they're varint / byte tags +// the wire-format parser (`MoqLiteCodec`) reads at message boundaries to +// decide which body to decode next. None of them carry payload data +// themselves; the data classes that DO live in `MoqLiteMessages.kt`. +// +// Source: `kixelated/moq/rs/moq-lite/src/lite/{stream,announce,subscribe}.rs`. + +/** + * ControlType varint discriminator written as the first datum on every + * client-initiated bidi stream. Selects which message body the peer + * should expect to read next. + * + * Source: `rs/moq-lite/src/lite/stream.rs:7-15`. + */ +enum class MoqLiteControlType( + val code: Long, +) { + /** Lite-01/02 only — unused on Lite-03. Reserved here for completeness. */ + Session(0L), + Announce(1L), + Subscribe(2L), + Fetch(3L), + Probe(4L), + + /** + * Graceful relay-shutdown signal. moq-rs's `Publisher::run` accepts + * `ControlType::Goaway = 5` (`rs/moq-lite/src/lite/publisher.rs`) + * to migrate a publisher to a different relay node. We don't act + * on it today — recognising the type code prevents + * [MoqLiteSession.handleInboundBidi] from silently FINing the + * bidi as an unknown control type, which would lose the relay's + * shutdown notification. Wire body decoding is left as a follow-up. + */ + Goaway(5L), + ; + + companion object { + private val byCode = entries.associateBy { it.code } + + fun fromCode(code: Long): MoqLiteControlType? = byCode[code] + } +} + +/** + * DataType varint written as the first byte of every uni stream that + * carries payload. Lite-03 currently uses `Group=0` only. + * + * Source: `rs/moq-lite/src/lite/stream.rs:32-36`. + */ +enum class MoqLiteDataType( + val code: Long, +) { + Group(0L), + ; + + companion object { + private val byCode = entries.associateBy { it.code } + + fun fromCode(code: Long): MoqLiteDataType? = byCode[code] + } +} + +/** + * Status byte at the head of an [MoqLiteAnnounce] payload. `Active=1` + * is sent on first publish; `Ended=0` on explicit unannounce. Disconnect + * is NOT signalled with `Ended` — the bidi just closes. + * + * Source: `rs/moq-lite/src/lite/announce.rs:84-90`. + */ +enum class MoqLiteAnnounceStatus( + val code: Int, +) { + Ended(0), + Active(1), + ; + + companion object { + fun fromCode(code: Int): MoqLiteAnnounceStatus? = + when (code) { + 0 -> Ended + 1 -> Active + else -> null + } + } +} + +/** + * Type discriminator at the head of a SubscribeResponse on the response + * side of a Subscribe bidi. Wire layout (Lite-03+): + * + * type varint (0 = Ok, 1 = Drop) + * body size-prefixed bytes + * + * The type sits OUTSIDE the body size prefix — see + * `rs/moq-lite/src/lite/subscribe.rs::SubscribeResponse::encode` (the + * `_` arm covers Lite03+). Earlier drafts wrapped type+body in one + * outer size prefix; Lite03 split them. + */ +enum class MoqLiteSubscribeResponseType( + val code: Long, +) { + Ok(0L), + Drop(1L), + ; + + companion object { + private val byCode = entries.associateBy { it.code } + + fun fromCode(code: Long): MoqLiteSubscribeResponseType? = byCode[code] + } +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteFrame.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteFrame.kt new file mode 100644 index 000000000..73c827014 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteFrame.kt @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.moq.lite + +/** + * One frame received from a subscription. moq-lite's wire format + * carries no per-frame envelope beyond the size; [groupSequence] is + * pulled from the group header so consumers can detect group rollover + * (e.g. for keyframe boundaries). + */ +data class MoqLiteFrame( + val groupSequence: Long, + val payload: ByteArray, +) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is MoqLiteFrame) return false + return groupSequence == other.groupSequence && payload.contentEquals(other.payload) + } + + override fun hashCode(): Int = 31 * groupSequence.hashCode() + payload.contentHashCode() +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHandles.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHandles.kt new file mode 100644 index 000000000..9f69f1f00 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHandles.kt @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.moq.lite + +import kotlinx.coroutines.flow.Flow + +// Caller-facing handles returned by MoqLiteSession.subscribe / .announce, +// plus the typed protocol-level rejection exception. Kept together +// because they share a single concern: "what the consumer gets back +// from a subscribe / announce request and how it surfaces failure." + +/** + * Active subscription handle returned by [MoqLiteSession.subscribe]. + * [frames] emits every frame the publisher pushes; [unsubscribe] + * FINs the bidi to signal "no longer interested" (moq-lite has no + * UNSUBSCRIBE message — FIN is the protocol). + */ +class MoqLiteSubscribeHandle internal constructor( + val id: Long, + val ok: MoqLiteSubscribeOk, + val frames: Flow, + private val unsubscribeAction: suspend () -> Unit, +) { + suspend fun unsubscribe() = unsubscribeAction() +} + +/** + * Active announce-discovery handle returned by [MoqLiteSession.announce]. + * [updates] emits every [MoqLiteAnnounce] update the relay streams + * back; [close] FINs the bidi to stop receiving updates. + */ +class MoqLiteAnnouncesHandle internal constructor( + val updates: Flow, + private val close: suspend () -> Unit, +) { + suspend fun close() = close.invoke() +} + +/** Thrown when subscribe is rejected (Drop) or the response stream dies. */ +class MoqLiteSubscribeException( + message: String, + cause: Throwable? = null, +) : RuntimeException(message, cause) diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHangCatalog.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHangCatalog.kt new file mode 100644 index 000000000..e52b75c08 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHangCatalog.kt @@ -0,0 +1,171 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.moq.lite + +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.Json + +/** + * Publisher-side model for the kixelated/moq `hang` `catalog.json` + * track. Kept distinct from + * `com.vitorpamplona.amethyst.commons.viewmodels.RoomSpeakerCatalog` + * (the parser in `:commons`) because `:nestsClient` does not depend on + * `:commons` — both classes target the same wire shape independently. + * + * Wire shape (verbatim from `kixelated/moq/rs/hang/src/catalog/`): + * + * { + * "audio": { + * "renditions": { + * "": { + * "codec": "opus", + * "container": { "kind": "legacy" }, + * "sampleRate": 48000, + * "numberOfChannels": 1, + * "jitter": 20, // ms; matches one Opus frame + * "bitrate": 32000 // optional + * } + * } + * } + * } + * + * Field names are camelCase per the upstream serde + * `rename_all = "camelCase"`. Every Amethyst-emitted field is required + * here (no nullable defaults) so the encoded JSON is stable byte-for- + * byte across runs and we don't accidentally publish `"bitrate": null` + * if hang.js's parser turns out to reject it. + */ +@Serializable +internal data class MoqLiteHangCatalog( + val audio: Audio, +) { + @Serializable + data class Audio( + val renditions: Map, + ) + + @Serializable + data class AudioRendition( + val codec: String, + val container: Container, + val sampleRate: Int, + val numberOfChannels: Int, + /** + * Publisher-side cadence hint (ms). hang.js's watcher uses + * this to size its jitter buffer — emit a real value rather + * than relying on the watcher's fallback default. For Opus + * this is the codec frame duration (20 ms at 48 kHz / 960 + * samples) and matches what hang.js's encoder emits at + * `js/publish/src/audio/encoder.ts:#runConfig`. + */ + val jitter: Int, + ) + + @Serializable + data class Container( + val kind: String, + ) + + /** UTF-8 JSON bytes ready to push on the `catalog.json` moq-lite track. */ + fun encodeJsonBytes(): ByteArray = JSON.encodeToString(serializer(), this).encodeToByteArray() + + companion object { + /** + * Json instance dedicated to catalog encoding. We can't reuse + * `:quartz`'s `JsonMapper` from common code without pulling + * `:commons` (which already imports it) into `:nestsClient`'s + * dependency closure — `:nestsClient` deliberately stays free + * of the UI/state layer. A local instance keeps the wire-format + * configuration (camelCase serde defaults, no pretty-printing, + * deterministic field order) co-located with the model. + */ + private val JSON = + Json { + // Default for kotlinx.serialization is `false`, but pin + // explicitly so a future global default-flip doesn't + // surface `"bitrate": null` on hang.js's parser. + encodeDefaults = false + explicitNulls = false + } + + /** + * Cached canonical-shape catalog JSON bytes for the default + * Opus mono 48 kHz audio track ([MoqLiteNestsListener.AUDIO_TRACK] + * keyed under `audio.renditions["audio/data"]`). The catalog is + * a fixed string for the whole publisher lifetime — caching + * avoids re-running kotlinx.serialization on every + * [com.vitorpamplona.nestsclient.MoqLiteNestsSpeaker.startBroadcasting] + * call and every JWT-refresh hot-swap iteration in + * [com.vitorpamplona.nestsclient.connectReconnectingNestsSpeaker]. + * + * Hard-coded to track name `"audio/data"` because that's the + * only track Amethyst publishes today; if a future caller + * needs a different name, fall back to + * [opusMono48k] + [encodeJsonBytes]. + */ + val OPUS_MONO_48K_AUDIO_DATA_JSON_BYTES: ByteArray = + opusMono48k("audio/data").encodeJsonBytes() + + /** + * Canonical Amethyst speaker catalog: a single `legacy`-container + * Opus rendition under [audioTrackName], matching the encoder + * config in [com.vitorpamplona.nestsclient.audio.OpusEncoder] + * (48 kHz mono). + * + * The rendition map is keyed by the moq-lite track name a + * subscriber should subscribe to for this rendition's frames — + * for nests audio rooms that's the same string the publisher + * publishes audio frames on + * (`MoqLiteNestsListener.AUDIO_TRACK`). + * + * If [com.vitorpamplona.nestsclient.audio.OpusEncoder] becomes + * parameterised in the future, this factory should take the + * encoder's config rather than hard-coding 48 kHz mono. + */ + fun opusMono48k(audioTrackName: String): MoqLiteHangCatalog = + MoqLiteHangCatalog( + audio = + Audio( + renditions = + mapOf( + audioTrackName to + AudioRendition( + codec = "opus", + container = Container(kind = "legacy"), + sampleRate = 48_000, + numberOfChannels = 1, + jitter = OPUS_FRAME_DURATION_MS, + ), + ), + ), + ) + + /** + * Opus frame duration in milliseconds — 960 samples / 48 kHz = + * 20 ms. Matches + * [com.vitorpamplona.nestsclient.audio.Audio.FRAME_SIZE_SAMPLES]. + * Published as the catalog `jitter` hint so hang.js's watcher + * sizes its jitter buffer to one Opus frame, the natural + * cadence of our encoder. + */ + private const val OPUS_FRAME_DURATION_MS: Int = 20 + } +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteMessages.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteMessages.kt index 3d8504c1a..48ff2f55d 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteMessages.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteMessages.kt @@ -20,111 +20,13 @@ */ package com.vitorpamplona.nestsclient.moq.lite -/** - * moq-lite ALPN strings. Lite-03 is preferred; `"moql"` is the legacy - * combined ALPN that requires an in-band SETUP exchange. - * - * Source: `kixelated/moq-rs/rs/moq-lite/src/version.rs:21-26`, - * `@moq/lite/connection/connect.js:277`. - */ -object MoqLiteAlpn { - const val LITE_03: String = "moq-lite-03" - const val LEGACY: String = "moql" -} - -/** - * ControlType varint discriminator written as the first datum on every - * client-initiated bidi stream. Selects which message body the peer - * should expect to read next. - * - * Source: `rs/moq-lite/src/lite/stream.rs:7-15`. - */ -enum class MoqLiteControlType( - val code: Long, -) { - /** Lite-01/02 only — unused on Lite-03. Reserved here for completeness. */ - Session(0L), - Announce(1L), - Subscribe(2L), - Fetch(3L), - Probe(4L), - ; - - companion object { - private val byCode = entries.associateBy { it.code } - - fun fromCode(code: Long): MoqLiteControlType? = byCode[code] - } -} - -/** - * DataType varint written as the first byte of every uni stream that - * carries payload. Lite-03 currently uses `Group=0` only. - * - * Source: `rs/moq-lite/src/lite/stream.rs:32-36`. - */ -enum class MoqLiteDataType( - val code: Long, -) { - Group(0L), - ; - - companion object { - private val byCode = entries.associateBy { it.code } - - fun fromCode(code: Long): MoqLiteDataType? = byCode[code] - } -} - -/** - * Status byte at the head of an [MoqLiteAnnounce] payload. `Active=1` - * is sent on first publish; `Ended=0` on explicit unannounce. Disconnect - * is NOT signalled with `Ended` — the bidi just closes. - * - * Source: `rs/moq-lite/src/lite/announce.rs:84-90`. - */ -enum class MoqLiteAnnounceStatus( - val code: Int, -) { - Ended(0), - Active(1), - ; - - companion object { - fun fromCode(code: Int): MoqLiteAnnounceStatus? = - when (code) { - 0 -> Ended - 1 -> Active - else -> null - } - } -} - -/** - * Type discriminator at the head of a SubscribeResponse on the response - * side of a Subscribe bidi. Wire layout (Lite-03+): - * - * type varint (0 = Ok, 1 = Drop) - * body size-prefixed bytes - * - * The type sits OUTSIDE the body size prefix — see - * `rs/moq-lite/src/lite/subscribe.rs::SubscribeResponse::encode` (the - * `_` arm covers Lite03+). Earlier drafts wrapped type+body in one - * outer size prefix; Lite03 split them. - */ -enum class MoqLiteSubscribeResponseType( - val code: Long, -) { - Ok(0L), - Drop(1L), - ; - - companion object { - private val byCode = entries.associateBy { it.code } - - fun fromCode(code: Long): MoqLiteSubscribeResponseType? = byCode[code] - } -} +// Wire-format payload data classes for moq-lite messages. +// +// The discriminator enums (which message-body to expect at the top of +// a bidi / uni stream) live in `MoqLiteControlCodes.kt`; the ALPN +// negotiation strings live in `MoqLiteAlpn.kt`. This file holds only +// the body shapes the codec encodes / decodes once a discriminator +// has selected one. /** * "I'm interested in broadcasts under this prefix" — the first message @@ -222,14 +124,40 @@ data class MoqLiteSubscribeOk( * RESET_STREAM, but the publisher can pre-emptively decline a * subscription with [MoqLiteSubscribeDrop] before any group flows. * - * Decode-only as far as the client cares — we never send Drop back - * upstream. + * Sent by [MoqLiteSession] when a relay opens a SUBSCRIBE bidi for a + * `track` we don't publish (e.g. a watcher subscribes to a `video/data` + * rendition but our broadcast only declares `audio/data` + `catalog.json` + * in its catalog). Without a Drop reply the watcher's response wait + * resolves only when the bidi is FIN'd, with no indication WHY — Drop + * carries the error code and a reason phrase the watcher can log / + * surface. + * + * Decoded by [MoqLiteSession] when the relay or upstream publisher + * rejects one of OUR subscriptions — we surface it as + * [MoqLiteSubscribeException] so callers see a typed protocol-level + * rejection rather than a silent end-of-flow. */ data class MoqLiteSubscribeDrop( val errorCode: Long, val reasonPhrase: String, ) +/** + * Error codes carried in the [MoqLiteSubscribeDrop.errorCode] varint. + * moq-lite leaves these application-defined; we mirror the IETF-MoQ + * conventions in `com.vitorpamplona.nestsclient.moq.ErrorCode` so a + * cross-protocol reader gets the same semantic from either path. + */ +object MoqLiteSubscribeDropCode { + /** + * The publisher does not serve this `(broadcast, track)` tuple. + * Sent for a subscribe whose `track` doesn't match any of the + * publishers we registered on this session. Mirrors + * `com.vitorpamplona.nestsclient.moq.ErrorCode.TRACK_DOES_NOT_EXIST`. + */ + const val TRACK_DOES_NOT_EXIST: Long = 0x04L +} + /** * Header at the start of a Group uni stream. After the * [MoqLiteDataType.Group] type byte, the publisher writes one diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLitePublisherHandle.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLitePublisherHandle.kt new file mode 100644 index 000000000..143917b78 --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLitePublisherHandle.kt @@ -0,0 +1,122 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.moq.lite + +/** + * Active publisher handle returned by [MoqLiteSession.publish]. + * + * Lifecycle: + * 1. Call [startGroup] (or [send] which auto-starts a fresh group on + * first call) to begin pushing frames for one Opus group. + * 2. Call [send] for each frame (one Opus packet = one frame). + * 3. Call [endGroup] to FIN the current group's uni stream and start + * a fresh group on the next [send]. Group rollover is the + * publisher's call — typically every N seconds or every keyframe. + * 4. Call [close] when the broadcast ends — sends `Announce(Ended)` + * on every active announce bidi and FINs every group stream. + */ +interface MoqLitePublisherHandle { + /** + * The broadcast suffix this publisher claimed at [MoqLiteSession.publish]. + * Always normalised per [MoqLitePath]. + */ + val suffix: String + + /** + * The next group sequence number that will be assigned by [send] / + * [startGroup]. Snapshot-only — read AFTER the broadcaster has + * stopped sending into this publisher (typically just before the + * caller closes the publisher in a hot-swap), so the value is the + * highest-already-used sequence + 1. + * + * Used by [com.vitorpamplona.nestsclient.MoqLiteNestsSpeaker]'s + * hot-swap path to seed the new session's publisher with a + * monotonically-continuing sequence — without this, every JWT + * refresh restarts at sequence 0 and kixelated/hang's + * `Container.Consumer.#run` drops every group whose sequence is + * less than its current `#active` high-water mark, killing audio + * for the watcher until either `#active` rolls over or the + * watcher re-subscribes. + * + * `@Volatile` on the implementation; safe to read from any + * coroutine. The accept-tiny-race window between read and a + * concurrent `send` is closed in practice because the broadcaster + * is responsible for swapping its publisher reference BEFORE the + * caller reads this value (so no further sends land on this + * publisher). + */ + val nextSequence: Long + + /** + * Start a new group. Allocates a fresh sequence id and opens a new + * uni stream pre-loaded with `DataType=Group + GroupHeader`. Idempotent + * — calling [startGroup] when the previous group hasn't been ended + * is treated as an implicit [endGroup] then a new start. + */ + suspend fun startGroup() + + /** + * Push one [payload] (one Opus packet) as a `varint(size) + payload` + * frame on the current group's uni stream. Auto-starts a group if + * none is active. + * + * Returns false if no inbound subscriber is currently attached. + * Subscriber-less sends silently drop on the wire — the relay keeps + * the publisher's announce active either way, so unmute is + * sample-accurate. + */ + suspend fun send(payload: ByteArray): Boolean + + /** FIN the current group's uni stream. The next [send] starts a fresh group. */ + suspend fun endGroup() + + /** + * Register a callback that fires once each time a new inbound + * subscriber is registered against this publisher's track (i.e. + * each track-matching SUBSCRIBE bidi the relay opens to us). Used + * to push a "track-latest" payload — the canonical example is the + * broadcast catalog manifest, which a watcher needs to receive on + * subscribe but doesn't change between subscribers — without + * forcing the publisher to maintain a periodic re-emit loop. + * + * Called once per accepted SUBSCRIBE (track filter passed). Fires + * OUTSIDE the publisher's serialisation lock, so the hook can + * safely call [send] / [endGroup] without deadlocking. + * + * Caller MUST set the hook before any subscriber attaches (typically + * immediately after [com.vitorpamplona.nestsclient.moq.lite.MoqLiteSession.publish] + * returns) — there's no "fire-on-set for existing subscribers" + * replay. For the typical catalog use case the publisher is fresh + * when the hook is set, and the relay's SUBSCRIBE bidi takes a + * round-trip to arrive, so this is safe in practice. + * + * Pass `null` to clear the hook. Calling twice with non-null + * replaces the previous hook (no de-duplication). + */ + fun setOnNewSubscriber(hook: (suspend () -> Unit)?) + + /** + * Stop publishing. Sends `Announce(Ended)` on every active announce + * bidi, FINs the current group, and releases all per-publisher + * resources. Idempotent. + */ + suspend fun close() +} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteSession.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteSession.kt index b6169fd03..3dc3825ed 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteSession.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteSession.kt @@ -22,6 +22,8 @@ package com.vitorpamplona.nestsclient.moq.lite import com.vitorpamplona.nestsclient.moq.MoqCodecException import com.vitorpamplona.nestsclient.moq.MoqWriter +import com.vitorpamplona.nestsclient.trace.NestsTrace +import com.vitorpamplona.nestsclient.trace.jsonStr import com.vitorpamplona.nestsclient.transport.WebTransportSession import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quic.Varint @@ -32,7 +34,6 @@ import kotlinx.coroutines.Job import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.consumeAsFlow import kotlinx.coroutines.launch @@ -96,8 +97,22 @@ class MoqLiteSession internal constructor( */ private var announceWatchJob: Job? = null - /** Single active publisher per session (moq-lite doesn't model multi-broadcast publishers). */ - private var activePublisher: PublisherStateImpl? = null + /** + * Active publishers on this session. moq-lite models a broadcast as + * `(suffix, set-of-tracks)`; the relay multiplexes Subscribe + * messages to whichever publisher claims the requested track. We + * allow N concurrent publishers per session as long as they share + * the same suffix (= same broadcast) and have distinct tracks. + * + * The nests audio room use case needs at least two: `audio/data` + * for Opus frames and `catalog.json` for the broadcast metadata + * the canonical kixelated/moq watcher subscribes to first to + * discover what's available. Without the catalog the broadcast is + * invisible to the JS reference watcher (the browser-side nests + * web client) — Amethyst-to-Amethyst kept working only because + * both sides hardcoded the audio track name and skipped discovery. + */ + private val activePublishers: MutableList = mutableListOf() @Volatile private var closed: Boolean = false @@ -119,22 +134,66 @@ class MoqLiteSession internal constructor( bidi.write(Varint.encode(MoqLiteControlType.Announce.code)) bidi.write(MoqLiteCodec.encodeAnnouncePlease(MoqLiteAnnouncePlease(prefix))) - val updates = MutableSharedFlow(replay = 0, extraBufferCapacity = 64) + // replay=64, DROP_OLDEST: announces emitted before the + // caller's `collect` attaches MUST NOT be dropped — that's + // the late-attach race that left `_announcedSpeakers` empty + // in production receiver logs even after a clear Active + // arrived (the session-internal pumpAnnounceWatch beat the + // VM-level `observeAnnounces` to subscribing, then the + // VM-level bidi's pump emitted to a SharedFlow with no + // collector yet, and the prior `replay=0` silently dropped + // it). Replay buffer covers up to 64 distinct announces — + // far more than nests rooms have speakers — and DROP_OLDEST + // means the bidi pump never has to suspend on emit, so + // backpressure can't propagate up into the QUIC read loop. + val updates = + MutableSharedFlow( + replay = 64, + onBufferOverflow = BufferOverflow.DROP_OLDEST, + ) + Log.d("NestRx") { "session.announce(prefix='$prefix') bidi opened, pump launching (replayCap=64)" } + NestsTrace.emit("announce_bidi_opened") { "\"prefix\":${jsonStr(prefix)}" } val pump = scope.launch { val buffer = MoqLiteFrameBuffer() + var chunkCount = 0 + var emitCount = 0 try { bidi.incoming().collect { chunk -> + chunkCount += 1 buffer.push(chunk) while (true) { val payload = buffer.readSizePrefixed() ?: break - updates.emit(MoqLiteCodec.decodeAnnounce(payload)) + val decoded = MoqLiteCodec.decodeAnnounce(payload) + emitCount += 1 + Log.d("NestRx") { + "session.announce(prefix='$prefix') bidi pump emit #$emitCount " + + "status=${decoded.status} suffix='${decoded.suffix.take(12)}' " + + "(chunks=$chunkCount)" + } + NestsTrace.emit("announce_pump_emit") { + "\"prefix\":${jsonStr(prefix)}," + + "\"emit_count\":$emitCount," + + "\"status\":${jsonStr(decoded.status.toString())}," + + "\"suffix\":${jsonStr(decoded.suffix)}," + + "\"chunks\":$chunkCount" + } + updates.emit(decoded) } } + Log.w("NestRx") { "session.announce(prefix='$prefix') bidi.incoming() ended naturally (chunks=$chunkCount, emits=$emitCount)" } + NestsTrace.emit("announce_bidi_ended_naturally") { + "\"prefix\":${jsonStr(prefix)},\"chunks\":$chunkCount,\"emits\":$emitCount" + } } catch (ce: CancellationException) { throw ce } catch (t: Throwable) { - Log.w("NestRx") { "announce(prefix='$prefix'): bidi.incoming() threw ${t::class.simpleName}: ${t.message}" } + Log.w("NestRx") { "announce(prefix='$prefix'): bidi.incoming() threw ${t::class.simpleName}: ${t.message} (chunks=$chunkCount, emits=$emitCount)" } + NestsTrace.emit("announce_bidi_threw") { + "\"prefix\":${jsonStr(prefix)},\"chunks\":$chunkCount,\"emits\":$emitCount," + + "\"error\":${jsonStr(t::class.simpleName ?: "?")}," + + "\"message\":${jsonStr(t.message ?: "")}" + } // Flow terminated (peer FIN or transport close). // The Announce stream's emit-side just stops; consumers // see an end-of-flow. @@ -262,6 +321,11 @@ class MoqLiteSession internal constructor( subscriptionsBySubscribeId[id] = sub if (groupPump == null) groupPump = scope.launch { pumpUniStreams() } } + Log.d("NestRx") { "SUBSCRIBE send id=$id broadcast='$broadcast' track='$track' maxLatencyMs=$maxLatencyMillis" } + NestsTrace.emit("subscribe_send") { + "\"id\":$id,\"broadcast\":${jsonStr(broadcast)},\"track\":${jsonStr(track)}," + + "\"max_latency_ms\":$maxLatencyMillis" + } // Now that the subscription is registered, push the SUBSCRIBE // bytes. If `bidi.write` throws (transport torn down, peer // reset) we'd otherwise leave an orphaned map entry whose @@ -272,6 +336,7 @@ class MoqLiteSession internal constructor( bidi.write(Varint.encode(MoqLiteControlType.Subscribe.code)) bidi.write(MoqLiteCodec.encodeSubscribe(request)) } catch (t: Throwable) { + Log.w("NestRx") { "SUBSCRIBE write failed id=$id: ${t::class.simpleName}: ${t.message}" } state.withLock { subscriptionsBySubscribeId.remove(id) } frames.close() runCatching { bidi.finish() } @@ -314,6 +379,7 @@ class MoqLiteSession internal constructor( if (!responseDeferred.isCompleted) responseDeferred.completeExceptionally(ce) throw ce } catch (t: Throwable) { + Log.w("NestRx") { "SUBSCRIBE bidi collector threw id=$id: ${t::class.simpleName}: ${t.message}" } if (!responseDeferred.isCompleted) responseDeferred.completeExceptionally(t) } if (!responseDeferred.isCompleted) { @@ -325,6 +391,13 @@ class MoqLiteSession internal constructor( // (or any throw from await), it already removed the // subscription before throwing. Either way: remove + close. val removed = state.withLock { subscriptionsBySubscribeId.remove(id) } + if (removed != null) { + Log.w("NestRx") { "SUBSCRIBE bidi exited, closing frames id=$id broadcast='${removed.request.broadcast}' track='${removed.request.track}'" } + NestsTrace.emit("subscribe_bidi_exited") { + "\"id\":$id,\"broadcast\":${jsonStr(removed.request.broadcast)}," + + "\"track\":${jsonStr(removed.request.track)}" + } + } removed?.frames?.close() } @@ -343,6 +416,11 @@ class MoqLiteSession internal constructor( "SUBSCRIBE_DROP id=$id broadcast='$broadcast' track='$track' " + "errCode=${resp.drop.errorCode} reason='${resp.drop.reasonPhrase}'" } + NestsTrace.emit("subscribe_drop") { + "\"id\":$id,\"broadcast\":${jsonStr(broadcast)},\"track\":${jsonStr(track)}," + + "\"err_code\":${resp.drop.errorCode}," + + "\"reason\":${jsonStr(resp.drop.reasonPhrase)}" + } state.withLock { subscriptionsBySubscribeId.remove(id) } frames.close() runCatching { bidi.finish() } @@ -353,6 +431,10 @@ class MoqLiteSession internal constructor( } is MoqLiteCodec.SubscribeResponse.Ok -> { + Log.d("NestRx") { "SUBSCRIBE_OK id=$id broadcast='$broadcast' track='$track'" } + NestsTrace.emit("subscribe_ok") { + "\"id\":$id,\"broadcast\":${jsonStr(broadcast)},\"track\":${jsonStr(track)}" + } return MoqLiteSubscribeHandle( id = id, ok = resp.ok, @@ -428,6 +510,12 @@ class MoqLiteSession internal constructor( private suspend fun pumpAnnounceWatch(handle: MoqLiteAnnouncesHandle) { try { handle.updates.collect { update -> + Log.d("NestRx") { "ANNOUNCE update status=${update.status} suffix='${update.suffix}' hops=${update.hops}" } + NestsTrace.emit("announce_watch_update") { + "\"status\":${jsonStr(update.status.toString())}," + + "\"suffix\":${jsonStr(update.suffix)}," + + "\"hops\":${update.hops}" + } if (update.status != MoqLiteAnnounceStatus.Ended) return@collect val targets = state.withLock { @@ -435,6 +523,13 @@ class MoqLiteSession internal constructor( .filter { it.request.broadcast == update.suffix } .toList() } + if (targets.isNotEmpty()) { + Log.w("NestRx") { "ANNOUNCE Ended for suffix='${update.suffix}' → closing ${targets.size} subscription(s): ${targets.map { "id=${it.id} track='${it.request.track}'" }}" } + NestsTrace.emit("announce_watch_ended_closing_subs") { + "\"suffix\":${jsonStr(update.suffix)}," + + "\"closed_count\":${targets.size}" + } + } for (sub in targets) { // Just close the frames channel — the // wrapper-level collect of `frames.consumeAsFlow()` @@ -450,9 +545,11 @@ class MoqLiteSession internal constructor( runCatching { sub.bidi.finish() } } } + Log.w("NestRx") { "ANNOUNCE pump: updates flow ended naturally" } } catch (ce: kotlinx.coroutines.CancellationException) { throw ce - } catch (_: Throwable) { + } catch (t: Throwable) { + Log.w("NestRx") { "ANNOUNCE pump threw ${t::class.simpleName}: ${t.message}" } // Announce bidi died — same best-effort fallback. } finally { runCatching { handle.close() } @@ -470,6 +567,9 @@ class MoqLiteSession internal constructor( * One pump per session — started lazily on the first subscribe. */ private suspend fun pumpUniStreams() { + Log.d("NestRx") { "pumpUniStreams started" } + NestsTrace.emit("uni_pump_started") + var streamCount = 0L try { // coroutineScope binds each per-stream drain to this pump's // job — when the pump is cancelled (session close), every @@ -478,24 +578,32 @@ class MoqLiteSession internal constructor( // independently errors out. kotlinx.coroutines.coroutineScope { transport.incomingUniStreams().collect { stream -> - launch { drainOneGroup(stream) } + val n = ++streamCount + launch { drainOneGroup(stream, n) } } } + Log.w("NestRx") { "pumpUniStreams: incomingUniStreams flow ended naturally after $streamCount streams" } } catch (ce: CancellationException) { throw ce } catch (t: Throwable) { - Log.w("NestRx") { "pumpUniStreams ended with ${t::class.simpleName}: ${t.message}" } + Log.w("NestRx") { "pumpUniStreams ended after $streamCount streams with ${t::class.simpleName}: ${t.message}" } // Transport closed — subscriptions will surface end-of-flow // via their own bidi pumps as well. } } - private suspend fun drainOneGroup(stream: com.vitorpamplona.nestsclient.transport.WebTransportReadStream) { + private suspend fun drainOneGroup( + stream: com.vitorpamplona.nestsclient.transport.WebTransportReadStream, + streamSeq: Long, + ) { val buffer = MoqLiteFrameBuffer() var typeRead = false var subscribeId: Long = -1L var groupSequence: Long = -1L var headerRead = false + var frameCount = 0 + var droppedNoSub = 0 + var trySendFailures = 0 try { stream.incoming().collect { chunk -> buffer.push(chunk) @@ -512,25 +620,48 @@ class MoqLiteSession internal constructor( subscribeId = hdr.subscribeId groupSequence = hdr.sequence headerRead = true + Log.d("NestRx") { "drainOneGroup#$streamSeq header subId=$subscribeId groupSeq=$groupSequence" } + NestsTrace.emit("group_header") { + "\"stream_seq\":$streamSeq,\"sub_id\":$subscribeId,\"group_seq\":$groupSequence" + } } while (true) { val frame = buffer.readSizePrefixed() ?: break val sub = state.withLock { subscriptionsBySubscribeId[subscribeId] } - sub?.frames?.trySend( - MoqLiteFrame( - groupSequence = groupSequence, - payload = frame, - ), - ) + if (sub == null) { + droppedNoSub += 1 + } else { + val sent = + sub.frames.trySend( + MoqLiteFrame( + groupSequence = groupSequence, + payload = frame, + ), + ) + if (!sent.isSuccess) trySendFailures += 1 + } + frameCount += 1 // If the subscription has been closed already we // silently drop the frame — the publisher hasn't // observed the unsubscribe yet (its uni streams // are independent of our bidi FIN). } } + Log.d("NestRx") { "drainOneGroup#$streamSeq FIN subId=$subscribeId groupSeq=$groupSequence frames=$frameCount droppedNoSub=$droppedNoSub trySendFail=$trySendFailures" } + NestsTrace.emit("group_fin") { + "\"stream_seq\":$streamSeq,\"sub_id\":$subscribeId,\"group_seq\":$groupSequence," + + "\"frames\":$frameCount,\"dropped_no_sub\":$droppedNoSub,\"try_send_fail\":$trySendFailures" + } } catch (ce: CancellationException) { throw ce - } catch (_: Throwable) { + } catch (t: Throwable) { + Log.w("NestRx") { "drainOneGroup#$streamSeq threw subId=$subscribeId groupSeq=$groupSequence frames=$frameCount: ${t::class.simpleName}: ${t.message}" } + NestsTrace.emit("group_threw") { + "\"stream_seq\":$streamSeq,\"sub_id\":$subscribeId,\"group_seq\":$groupSequence," + + "\"frames\":$frameCount," + + "\"error\":${jsonStr(t::class.simpleName ?: "?")}," + + "\"message\":${jsonStr(t.message ?: "")}" + } // Stream errored / FIN'd. Nothing to do — the next group // arrives on a fresh uni stream. } @@ -565,23 +696,49 @@ class MoqLiteSession internal constructor( * - we open uni streams ourselves to push group data * (`session.open_uni()`) * - * Only one [publish] is supported per session for now. Calling - * [publish] twice on the same session is rejected with [IllegalStateException]. + * Multiple [publish] calls are supported on the same session as + * long as every call shares the same `broadcastSuffix` (you publish + * one broadcast per session, with multiple tracks) and uses a + * distinct `track`. Re-publishing the same `(suffix, track)` pair + * or mixing different suffixes is rejected with + * [IllegalStateException]. + * + * @param startSequence first group sequence the publisher will + * assign. Defaults to 0 for fresh broadcasts. The hot-swap path + * in [com.vitorpamplona.nestsclient.MoqLiteNestsSpeaker.openPublisherForHotSwap] + * passes the previous publisher's + * [MoqLitePublisherHandle.nextSequence] so the new session's + * group lineage continues monotonically across JWT refreshes — + * kixelated/hang's `Container.Consumer.#run` drops any group + * with `sequence < #active`, so a reset to 0 after a recycle + * silences the watcher until `#active` rolls over. */ suspend fun publish( broadcastSuffix: String, track: String, + startSequence: Long = 0L, ): MoqLitePublisherHandle { ensureOpen() + require(startSequence >= 0L) { "startSequence must be >= 0, got $startSequence" } val normalised = MoqLitePath.normalize(broadcastSuffix) val publisher: PublisherStateImpl state.withLock { check(!closed) { "session is closed" } - check(activePublisher == null) { - "MoqLiteSession.publish called twice — only one broadcast per session is supported" + val existingSuffix = activePublishers.firstOrNull()?.suffix + check(existingSuffix == null || existingSuffix == normalised) { + "MoqLiteSession.publish suffix mismatch: existing='$existingSuffix', new='$normalised'. " + + "moq-lite models one broadcast per session — open a new session for a different broadcast." } - publisher = PublisherStateImpl(suffix = normalised, track = track) - activePublisher = publisher + check(activePublishers.none { it.track == track }) { + "MoqLiteSession.publish called twice for the same track '$track' on suffix '$normalised'." + } + publisher = + PublisherStateImpl( + suffix = normalised, + track = track, + startSequence = startSequence, + ) + activePublishers += publisher // Lazy launch — the inbound-bidi pump needs to keep running // for the lifetime of any active publisher. if (bidiPump == null) bidiPump = scope.launch { pumpInboundBidis() } @@ -616,7 +773,21 @@ class MoqLiteSession internal constructor( } private suspend fun handleInboundBidi(bidi: com.vitorpamplona.nestsclient.transport.WebTransportBidiStream) { - val publisher = state.withLock { activePublisher } ?: return + // Snapshot of publishers at bidi-arrival time. All publishers + // on a single session share a suffix (enforced in [publish]), + // so the Announce response is the same regardless of which + // publisher we route the bidi to. Subscribe responses pick the + // publisher whose `track` matches `sub.track`. + val publishersSnapshot = state.withLock { activePublishers.toList() } + if (publishersSnapshot.isEmpty()) return + // Designated publisher for Announce-bidi ownership: the first + // one. The Active/Ended pair is keyed off the broadcast + // SUFFIX (not per track), so emitting it once via one + // publisher matches the single-broadcast model the relay + // expects. All publishers close together in [close], so + // there's no risk of one closing while the announce bidi + // stays alive on another. + val announcePublisher = publishersSnapshot.first() // Single long-running collector for the bidi's full lifetime. // Pre-fix this dispatch was split into a `firstOrNull()` to @@ -641,6 +812,16 @@ class MoqLiteSession internal constructor( var typeCode: Long? = null var dispatched = false var inboundSub: MoqLiteSubscribe? = null + // Track which publisher this bidi was routed to so the peer-FIN + // cleanup at the bottom calls removeInboundSubscription on the + // right one. `inboundAnnouncePublisher` is currently unused for + // cleanup (announce bidis live until publisher.close fires + // Ended), but we keep the assignment for symmetry / future + // explicit teardown. + var inboundSubPublisher: PublisherStateImpl? = null + + @Suppress("UNUSED_VARIABLE") + var inboundAnnouncePublisher: PublisherStateImpl? = null try { bidi.incoming().collect { chunk -> buffer.push(chunk) @@ -658,7 +839,7 @@ class MoqLiteSession internal constructor( val pleasePayload = buffer.readSizePrefixed() ?: return@collect val please = MoqLiteCodec.decodeAnnouncePlease(pleasePayload) val emittedSuffix = - MoqLitePath.stripPrefix(please.prefix, publisher.suffix) ?: publisher.suffix + MoqLitePath.stripPrefix(please.prefix, announcePublisher.suffix) ?: announcePublisher.suffix bidi.write( MoqLiteCodec.encodeAnnounce( MoqLiteAnnounce( @@ -668,13 +849,54 @@ class MoqLiteSession internal constructor( ), ), ) - publisher.registerAnnounceBidi(bidi, emittedSuffix) + announcePublisher.registerAnnounceBidi(bidi, emittedSuffix) + // Routed to publisher that owns the announce bidi for the lifecycle + // — for the multi-track use case (audio + catalog), all share one + // suffix and one announce bidi pointing at the first publisher. + inboundAnnouncePublisher = announcePublisher + Log.d("NestTx") { "ANNOUNCE inbound prefix='${please.prefix}' → emitted Active suffix='$emittedSuffix' (publisher.suffix='${announcePublisher.suffix}')" } dispatched = true } MoqLiteControlType.Subscribe -> { val subPayload = buffer.readSizePrefixed() ?: return@collect val sub = MoqLiteCodec.decodeSubscribe(subPayload) + // Find the publisher that claims this track. With the + // single-track-per-publisher model, only one match is possible. + val targetPublisher = publishersSnapshot.firstOrNull { it.track == sub.track } + if (targetPublisher == null) { + // Reply SubscribeDrop with a TRACK_DOES_NOT_EXIST + // error code BEFORE we FIN — without this the + // peer's response wait resolves only on + // bidi-FIN with no indication WHY (looks + // identical to "publisher disappeared mid- + // subscribe"). Drop carries the error code + + // reason phrase the watcher can log / + // surface, and matches what kixelated's + // `rs/moq-lite/src/lite/subscribe.rs` + // expects for an unrecognised track on a + // live broadcast. + Log.w("NestTx") { + "SUBSCRIBE inbound id=${sub.id} track='${sub.track}' has no matching publisher " + + "on this session (have ${publishersSnapshot.map { it.track }}) — replying SubscribeDrop" + } + runCatching { + bidi.write( + MoqLiteCodec.encodeSubscribeDrop( + MoqLiteSubscribeDrop( + errorCode = MoqLiteSubscribeDropCode.TRACK_DOES_NOT_EXIST, + reasonPhrase = + "track '${sub.track}' is not published on this broadcast " + + "(available: ${publishersSnapshot.joinToString(",") { it.track }})", + ), + ), + ) + bidi.finish() + } + dispatched = true + return@collect + } + Log.d("NestTx") { "SUBSCRIBE inbound id=${sub.id} broadcast='${sub.broadcast}' track='${sub.track}' (publisher track='${targetPublisher.track}')" } // Register the subscription BEFORE sending Ok so the // peer's observation of Ok is a happens-after of // `inboundSubs += sub`. Otherwise on dispatchers that @@ -683,8 +905,9 @@ class MoqLiteSession internal constructor( // (notably Windows under Dispatchers.Default), the // peer's first `publisher.send` after Ok races the // registration and observes an empty subscriber set. - publisher.registerInboundSubscription(sub) + targetPublisher.registerInboundSubscription(sub) inboundSub = sub + inboundSubPublisher = targetPublisher bidi.write( MoqLiteCodec.encodeSubscribeOk( MoqLiteSubscribeOk( @@ -699,9 +922,69 @@ class MoqLiteSession internal constructor( dispatched = true } - else -> { - // Lite-03 treats Session/Fetch/Probe as - // separate flows; we don't implement them. + MoqLiteControlType.Probe -> { + // Subscriber-opened bidi asking us (the + // publisher) for a bitrate hint. Per + // `lite/probe.rs:Lite03+`, the publisher + // writes one or more size-prefixed + // `Probe { bitrate: u62 }` messages on + // this bidi. We're a fixed-rate Opus + // publisher, so emit a single hint and + // FIN our write side — the peer treats + // FIN as "no further updates" rather than + // an error. Better than the old behaviour + // of FINing without writing anything, + // which left the subscriber's ABR + // estimator with no signal. + runCatching { + bidi.write(MoqLiteCodec.encodeProbe(MoqLiteProbe(bitrate = NESTS_AUDIO_BITRATE_HINT_BPS))) + bidi.finish() + } + dispatched = true + } + + MoqLiteControlType.Fetch -> { + // Subscriber-opened bidi requesting a + // historical group (`lite/fetch.rs`). + // Audio rooms are live-only — we have no + // group history to serve. FINing the + // write side without any reply is the + // spec-clean way to signal "no groups + // available"; the subscriber's wait on + // its receive side resolves to + // end-of-stream and it falls back to a + // live Subscribe. Ignoring inbound bytes + // (the request body) is fine: we don't + // need to know which group was requested + // because we couldn't serve any of them. + runCatching { bidi.finish() } + dispatched = true + } + + MoqLiteControlType.Session -> { + // ControlType=0 was the Lite-01/02 setup + // exchange. Lite-03 doesn't use it; if a + // legacy peer sends one, FIN cleanly so + // their bidi resolves rather than hanging. + runCatching { bidi.finish() } + dispatched = true + } + + MoqLiteControlType.Goaway -> { + // Relay's graceful-shutdown signal — see + // [MoqLiteControlType.Goaway]. We don't + // act on the migration request today + // (no body decode, no preferred-relay + // failover); the `connectReconnecting*` + // wrappers' transport-loss reconnect path + // already handles the eventual hard + // disconnect, so all this arm needs to do + // is recognise the type code and FIN + // cleanly instead of treating it as an + // unknown control. Logged so a relay- + // initiated migration shows up in logcat + // rather than as a mystery silent reconnect. + Log.w("NestRx") { "Goaway received from relay — FIN bidi (no migration handler today)" } runCatching { bidi.finish() } dispatched = true } @@ -723,7 +1006,11 @@ class MoqLiteSession internal constructor( // groups off this dead subscriber. Announce bidis are // owned by the publisher state for sending Ended on // publisher-close — we don't remove them here. - inboundSub?.let { publisher.removeInboundSubscription(it) } + val sub = inboundSub + val pub = inboundSubPublisher + if (sub != null && pub != null) { + pub.removeInboundSubscription(sub) + } } /** @@ -735,14 +1022,21 @@ class MoqLiteSession internal constructor( subscribeId: Long, sequence: Long, ): com.vitorpamplona.nestsclient.transport.WebTransportWriteStream { - // Group streams carry a single Opus packet. They're real-time - // and best-effort — a STREAM frame arriving 200 ms late is - // worse than useless because the listener has already moved - // past that group's sequence number. Setting bestEffort=true - // tells the underlying QUIC SendBuffer to drop lost ranges - // instead of retransmitting them, bounding the bandwidth waste - // we'd otherwise incur on a lossy uplink. - val uni = transport.openUniStream(bestEffort = true) + // Group streams use reliable QUIC delivery to match the + // moq-lite reference (kixelated/moq-rs `serve_group` writes + // reliable streams; bestEffort is not a moq-lite concept). + // The previous shape opened with `bestEffort=true`, which + // caused `:quic`'s `SendBuffer.markLost` to drop lost ranges + // without retransmit AND without RESET_STREAM — leaving the + // peer's stream-reassembly buffer permanently wedged at the + // hole boundary. The watcher's `Group.readFrame` parks until + // the relay's 30 s `MAX_GROUP_AGE` ages the broadcast queue + // out, manifesting as a 30 s silent dropout per lost packet + // on lossy networks. Reliable delivery costs marginal extra + // bandwidth on retransmits (a lost STREAM range arriving 50– + // 150 ms late still falls inside hang's default ~200 ms + // jitter buffer) and avoids the dropout entirely. + val uni = transport.openUniStream() uni.write(Varint.encode(MoqLiteDataType.Group.code)) uni.write(MoqLiteCodec.encodeGroupHeader(MoqLiteGroupHeader(subscribeId, sequence))) return uni @@ -752,18 +1046,20 @@ class MoqLiteSession internal constructor( if (closed) return closed = true val toClose: List - val publisherToClose: PublisherStateImpl? + val publishersToClose: List state.withLock { toClose = subscriptionsBySubscribeId.values.toList() subscriptionsBySubscribeId.clear() - publisherToClose = activePublisher - activePublisher = null + publishersToClose = activePublishers.toList() + activePublishers.clear() } for (sub in toClose) { runCatching { sub.bidi.finish() } sub.frames.close() } - runCatching { publisherToClose?.close() } + for (p in publishersToClose) { + runCatching { p.close() } + } groupPump?.cancelAndJoin() bidiPump?.cancelAndJoin() runCatching { transport.close() } @@ -809,16 +1105,45 @@ class MoqLiteSession internal constructor( */ private inner class PublisherStateImpl( override val suffix: String, - private val track: String, + internal val track: String, + startSequence: Long, ) : MoqLitePublisherHandle { private val gate = Mutex() private val announceBidis = mutableListOf() private val inboundSubs = mutableListOf() private var currentGroup: GroupOutbound? = null - private var nextSequence: Long = 0L + + // `@Volatile` so the hot-swap caller can read this from outside + // the publisher's gate (see [MoqLitePublisherHandle.nextSequence] + // kdoc). Mutation happens only inside [openNextGroupLocked] + // (which holds [gate]); the volatile guarantees a cross-thread + // read sees the latest write without contending the gate. + @Volatile + private var nextSequenceField: Long = startSequence + + override val nextSequence: Long + get() = nextSequenceField + + // Diagnostic: throttled counter for "send returned false" logs so a + // long no-subscriber window doesn't flood logcat at 50 Hz. + private val sendNoSubLogCount = + java.util.concurrent.atomic + .AtomicLong(0L) @Volatile private var publisherClosed = false + /** + * Caller-installed hook fired once per accepted inbound + * SUBSCRIBE — see [MoqLitePublisherHandle.setOnNewSubscriber]. + * Read-and-fire happens OUTSIDE [gate] to avoid deadlocking on + * the hook's own calls to [send] / [endGroup]. + */ + @Volatile private var onNewSubscriberHook: (suspend () -> Unit)? = null + + override fun setOnNewSubscriber(hook: (suspend () -> Unit)?) { + onNewSubscriberHook = hook + } + suspend fun registerAnnounceBidi( bidi: com.vitorpamplona.nestsclient.transport.WebTransportBidiStream, emittedSuffix: String, @@ -833,10 +1158,29 @@ class MoqLiteSession internal constructor( } suspend fun registerInboundSubscription(sub: MoqLiteSubscribe) { + // Capture the hook INSIDE the lock — guarantees the hook + // observes a fully-registered subscriber when it fires — + // but invoke it OUTSIDE so a hook that calls [send] / + // [endGroup] doesn't deadlock on the same gate. + var hookToFire: (suspend () -> Unit)? = null gate.withLock { - if (publisherClosed) return - if (sub.track != track) return + if (publisherClosed) { + Log.w("NestTx") { "SUBSCRIBE inbound rejected (publisher closed) id=${sub.id} track='${sub.track}'" } + return + } + if (sub.track != track) { + Log.w("NestTx") { "SUBSCRIBE inbound track mismatch id=${sub.id} sub.track='${sub.track}' publisher.track='$track' — ignored" } + return + } inboundSubs += sub + hookToFire = onNewSubscriberHook + Log.d("NestTx") { "SUBSCRIBE registered id=${sub.id} broadcast='${sub.broadcast}' track='${sub.track}' inboundSubs.size=${inboundSubs.size}" } + } + // Launch on the session's scope so the hook outlives the + // bidi pump's per-bidi coroutine if it's slow (e.g. a + // catalog send blocked on transport backpressure). + hookToFire?.let { hook -> + scope.launch { runCatching { hook.invoke() } } } } @@ -853,6 +1197,7 @@ class MoqLiteSession internal constructor( gate.withLock { if (publisherClosed) return if (!inboundSubs.remove(sub)) return + Log.w("NestTx") { "SUBSCRIBE removed (peer FIN/error) id=${sub.id} track='${sub.track}' inboundSubs.size=${inboundSubs.size}" } runCatching { currentGroup?.uni?.finish() } currentGroup = null } @@ -868,8 +1213,18 @@ class MoqLiteSession internal constructor( override suspend fun send(payload: ByteArray): Boolean { gate.withLock { - if (publisherClosed) return false - if (inboundSubs.isEmpty()) return false + if (publisherClosed) { + if (sendNoSubLogCount.getAndIncrement() % SEND_LOG_THROTTLE == 0L) { + Log.w("NestTx") { "send returning false — publisher closed (count=${sendNoSubLogCount.get()})" } + } + return false + } + if (inboundSubs.isEmpty()) { + if (sendNoSubLogCount.getAndIncrement() % SEND_LOG_THROTTLE == 0L) { + Log.w("NestTx") { "send returning false — no inboundSubs (count=${sendNoSubLogCount.get()}, payload=${payload.size}B)" } + } + return false + } val group = currentGroup ?: openNextGroupLocked().also { currentGroup = it } // Single-allocation framing: write the varint length // directly into a buffer sized for `varint + payload`, @@ -937,7 +1292,7 @@ class MoqLiteSession internal constructor( runCatching { groupToFinish?.uni?.finish() } // Detach from the session so a subsequent `publish` can run. state.withLock { - if (activePublisher === this) activePublisher = null + activePublishers.remove(this@PublisherStateImpl) } } @@ -951,8 +1306,18 @@ class MoqLiteSession internal constructor( // expected to be small (1 in nests's listener-per-room // model), so this is fine. val sub = inboundSubs.first() - val sequence = nextSequence++ - val uni = openGroupStream(subscribeId = sub.id, sequence = sequence) + val sequence = nextSequenceField + nextSequenceField = sequence + 1L + val uni = + try { + openGroupStream(subscribeId = sub.id, sequence = sequence) + } catch (ce: CancellationException) { + throw ce + } catch (t: Throwable) { + Log.w("NestTx") { "openGroupStream threw subId=${sub.id} seq=$sequence: ${t::class.simpleName}: ${t.message}" } + throw t + } + Log.d("NestTx") { "openGroupStream subId=${sub.id} seq=$sequence" } return GroupOutbound(sequence = sequence, uni = uni) } } @@ -971,6 +1336,19 @@ class MoqLiteSession internal constructor( /** moq-lite priority byte midpoint — neutral default. */ const val DEFAULT_PRIORITY: Int = 0x80 + /** + * Bitrate hint (bits/sec) we report on inbound moq-lite Probe + * bidis as a publisher. Mirrors the upper-bound of an Opus + * voice profile at 48 kHz mono, ≈32 kbps. Subscriber-side ABR + * estimators use this to size their forward queue; we emit the + * single hint and FIN since our encoder runs at a fixed bitrate. + */ + const val NESTS_AUDIO_BITRATE_HINT_BPS: Long = 32_000L + + // Diagnostic: log "send returned false" once every N invocations. + // At 50 fps and N=50 → ≤ 1 log/sec for a sustained no-sub window. + private const val SEND_LOG_THROTTLE: Long = 50L + /** * Per-subscription channel buffer for inbound frames. 128 audio * frames at Opus 20 ms ≈ 2.5 s of backlog before DROP_OLDEST, @@ -992,106 +1370,3 @@ class MoqLiteSession internal constructor( ): MoqLiteSession = MoqLiteSession(transport, pumpScope) } } - -/** - * One frame received from a subscription. moq-lite's wire format - * carries no per-frame envelope beyond the size; [groupSequence] is - * pulled from the group header so consumers can detect group rollover - * (e.g. for keyframe boundaries). - */ -data class MoqLiteFrame( - val groupSequence: Long, - val payload: ByteArray, -) { - override fun equals(other: Any?): Boolean { - if (this === other) return true - if (other !is MoqLiteFrame) return false - return groupSequence == other.groupSequence && payload.contentEquals(other.payload) - } - - override fun hashCode(): Int = 31 * groupSequence.hashCode() + payload.contentHashCode() -} - -/** - * Active subscription handle returned by [MoqLiteSession.subscribe]. - * [frames] emits every frame the publisher pushes; [unsubscribe] - * FINs the bidi to signal "no longer interested" (moq-lite has no - * UNSUBSCRIBE message — FIN is the protocol). - */ -class MoqLiteSubscribeHandle internal constructor( - val id: Long, - val ok: MoqLiteSubscribeOk, - val frames: Flow, - private val unsubscribeAction: suspend () -> Unit, -) { - suspend fun unsubscribe() = unsubscribeAction() -} - -/** - * Active announce-discovery handle returned by [MoqLiteSession.announce]. - * [updates] emits every [MoqLiteAnnounce] update the relay streams - * back; [close] FINs the bidi to stop receiving updates. - */ -class MoqLiteAnnouncesHandle internal constructor( - val updates: Flow, - private val close: suspend () -> Unit, -) { - suspend fun close() = close.invoke() -} - -/** Thrown when subscribe is rejected (Drop) or the response stream dies. */ -class MoqLiteSubscribeException( - message: String, - cause: Throwable? = null, -) : RuntimeException(message, cause) - -/** - * Active publisher handle returned by [MoqLiteSession.publish]. - * - * Lifecycle: - * 1. Call [startGroup] (or [send] which auto-starts a fresh group on - * first call) to begin pushing frames for one Opus group. - * 2. Call [send] for each frame (one Opus packet = one frame). - * 3. Call [endGroup] to FIN the current group's uni stream and start - * a fresh group on the next [send]. Group rollover is the - * publisher's call — typically every N seconds or every keyframe. - * 4. Call [close] when the broadcast ends — sends `Announce(Ended)` - * on every active announce bidi and FINs every group stream. - */ -interface MoqLitePublisherHandle { - /** - * The broadcast suffix this publisher claimed at [MoqLiteSession.publish]. - * Always normalised per [MoqLitePath]. - */ - val suffix: String - - /** - * Start a new group. Allocates a fresh sequence id and opens a new - * uni stream pre-loaded with `DataType=Group + GroupHeader`. Idempotent - * — calling [startGroup] when the previous group hasn't been ended - * is treated as an implicit [endGroup] then a new start. - */ - suspend fun startGroup() - - /** - * Push one [payload] (one Opus packet) as a `varint(size) + payload` - * frame on the current group's uni stream. Auto-starts a group if - * none is active. - * - * Returns false if no inbound subscriber is currently attached. - * Subscriber-less sends silently drop on the wire — the relay keeps - * the publisher's announce active either way, so unmute is - * sample-accurate. - */ - suspend fun send(payload: ByteArray): Boolean - - /** FIN the current group's uni stream. The next [send] starts a fresh group. */ - suspend fun endGroup() - - /** - * Stop publishing. Sends `Announce(Ended)` on every active announce - * bidi, FINs the current group, and releases all per-publisher - * resources. Idempotent. - */ - suspend fun close() -} diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/trace/NestsTrace.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/trace/NestsTrace.kt new file mode 100644 index 000000000..b0b88912b --- /dev/null +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/trace/NestsTrace.kt @@ -0,0 +1,191 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.trace + +/** + * Append-only event recorder for the moq-lite + Nests audio path. + * + * Designed to capture enough wire-level + decision-level data from a real + * production session that the same communication can be replayed in a + * unit test without a network — i.e., feed the recorded events back + * through a `TraceReplayingTransport` (planned, not yet implemented) into + * the unmodified [com.vitorpamplona.nestsclient.moq.lite.MoqLiteSession] + * + [com.vitorpamplona.nestsclient.MoqLiteNestsListener] + + * [com.vitorpamplona.amethyst.commons.viewmodels.NestViewModel] stack + * and assert on observable behaviour (frames received, cliff-detector + * decisions, recycle counts). + * + * **Cost when disabled**: a single volatile-load + branch per call site + * (the [emit] inline guards `if (!enabled) return` before the lambda + * runs). Production builds that never call [setRecording] pay nothing. + * + * **Output format**: one JSON object per line, written via + * [com.vitorpamplona.quartz.utils.Log] at the [TAG] tag. Capture from a + * connected device with: + * + * adb logcat -c + * adb logcat -s NestsTraceJsonl:D -v raw > nest-trace.jsonl + * + * The `-v raw` formatter strips the `D NestsTraceJsonl:` prefix so the + * captured file is valid JSONL ready for replay tooling. + * + * **Schema**: each event is a JSON object with at least + * - `t_ms` — milliseconds since [setRecording] was called with `true` + * - `kind` — string discriminator naming the event + * and additional kind-specific fields. Keep all field names lower-snake- + * case and stable; the replay tool will pattern-match on `kind`. + * + * Anonymisation: pubkeys + track names are recorded verbatim because + * they're already in the production logs the user is sharing (the + * `NestRx` / `NestTx` tags). Frame payloads are NEVER recorded — only + * sizes — so audio content can't leak through a trace dump. + */ +object NestsTrace { + /** Tag the JSONL lines are emitted under. Filter logcat with `-s NestsTraceJsonl:D`. */ + const val TAG: String = "NestsTraceJsonl" + + // `@PublishedApi internal` rather than `private` because [emit] is + // inline — its body becomes part of every call site's bytecode and + // must be able to reach the backing fields. `private` would refuse + // to compile ("Public-API inline function cannot access non-public- + // API property"). Marking these `@PublishedApi internal` keeps them + // unreachable from outside the module while satisfying the inliner. + @PublishedApi + @Volatile + internal var enabled: Boolean = false + + @PublishedApi + @Volatile + internal var startMark: kotlin.time.TimeMark? = null + + /** + * Idempotent. When `on` flips from false → true, [startMark] is + * captured so subsequent [emit] calls record monotonic-time deltas + * relative to enable. Flipping true → false stops further emits but + * does NOT alter prior log output. + * + * Call from a debug-build app start-up hook, a debug menu toggle, or + * a test `@Before`. Production release builds should leave the + * default disabled. + * + * Named `setRecording` (not `setEnabled`) so it doesn't clash with + * the JVM-generated setter for the `enabled` backing property — + * that property is `@PublishedApi internal` for the inline [emit] + * to access, which forces the setter to share the `setEnabled` + * JVM name. + */ + fun setRecording(on: Boolean) { + if (on == enabled) return + if (on) { + startMark = + kotlin.time.TimeSource.Monotonic + .markNow() + } + enabled = on + } + + fun isRecording(): Boolean = enabled + + /** + * Record an event. The lambda is invoked ONLY when tracing is + * enabled, so the call site pays nothing in the disabled path beyond + * the field-load + comparison. + * + * The lambda must return the kind-specific JSON fields portion + * (no surrounding braces, no leading or trailing comma — empty + * string when there are no fields). This recorder prepends + * `{"t_ms":N,"kind":"K"`, joins fields with a comma when present, + * and appends `}`. + * + * Use [jsonStr] / [jsonArrStr] to safely quote string values and + * arrays. Numeric / boolean values can be interpolated directly. + * + * Example: + * + * NestsTrace.emit("subscribe_send") { + * "\"id\":$id,\"broadcast\":${jsonStr(broadcast)},\"track\":${jsonStr(track)}" + * } + */ + inline fun emit( + kind: String, + fieldsJson: () -> String = { "" }, + ) { + if (!enabled) return + val mark = startMark ?: return + val tMs = mark.elapsedNow().inWholeMilliseconds + val fields = fieldsJson() + val sep = if (fields.isEmpty()) "" else "," + com.vitorpamplona.quartz.utils.Log.d(TAG) { + "{\"t_ms\":$tMs,\"kind\":\"$kind\"$sep$fields}" + } + } +} + +/** + * Quote a string as a JSON literal, escaping the small set of characters + * that would otherwise produce invalid JSONL (`"`, `\`, control chars). + * Keeps the implementation small + commonMain-portable rather than + * pulling in a full JSON library for what is effectively a single + * field-value path. + */ +fun jsonStr(value: String): String { + val sb = StringBuilder(value.length + 2) + sb.append('"') + for (c in value) { + when (c) { + '"' -> { + sb.append('\\').append('"') + } + + '\\' -> { + sb.append('\\').append('\\') + } + + '\n' -> { + sb.append('\\').append('n') + } + + '\r' -> { + sb.append('\\').append('r') + } + + '\t' -> { + sb.append('\\').append('t') + } + + else -> { + if (c.code < 0x20) { + sb.append("\\u00") + val h = c.code.toString(16) + if (h.length == 1) sb.append('0') + sb.append(h) + } else { + sb.append(c) + } + } + } + } + sb.append('"') + return sb.toString() +} + +/** JSON array of strings: `["a","b","c"]`. */ +fun jsonArrStr(values: Iterable): String = values.joinToString(separator = ",", prefix = "[", postfix = "]") { jsonStr(it) } diff --git a/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/StripLegacyTimestampPrefixTest.kt b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/StripLegacyTimestampPrefixTest.kt new file mode 100644 index 000000000..62caecab0 --- /dev/null +++ b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/StripLegacyTimestampPrefixTest.kt @@ -0,0 +1,145 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient + +import com.vitorpamplona.quic.Varint +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertSame +import kotlin.test.assertTrue + +/** + * Unit tests for [MoqLiteNestsListener.Companion.stripLegacyTimestampPrefix]. + * + * This function is the entire receive-side wire-format converter for + * audio frames — every web speaker's Opus packet flows through it. A + * regression here is invisible to users (silent decode failure of every + * web speaker) and to interop tests (both sides agree on the same bug). + * Pin the four QUIC varint-length tiers, the malformed paths, and a + * round-trip against `NestMoqLiteBroadcaster`'s wire format. + * + * Varint length tags (RFC 9000 §16, top 2 bits of the first byte): + * 00 → 1 byte total (6-bit value, 0..63) + * 01 → 2 bytes total (14-bit, 0..16383) + * 10 → 4 bytes total (30-bit, 0..2^30-1) + * 11 → 8 bytes total (62-bit, 0..2^62-1) + */ +class StripLegacyTimestampPrefixTest { + @Test + fun strips_one_byte_varint_for_small_timestamp() { + // `Varint.encode(0L)` returns a single byte with tag 00. + val frame = Varint.encode(0L) + OPUS + val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame) + assertContentEquals(OPUS, stripped) + } + + @Test + fun strips_one_byte_varint_at_max_6bit_value() { + // 63 (0x3F) is the largest value still encoded in 1 byte. + val frame = Varint.encode(63L) + OPUS + assertContentEquals(byteArrayOf(0x3F) + OPUS, frame, "varint(63) is a single 0x3F byte") + val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame) + assertContentEquals(OPUS, stripped) + } + + @Test + fun strips_two_byte_varint() { + // 64 (just past the 1-byte boundary) → tag 01, 2 bytes. + val frame = Varint.encode(64L) + OPUS + val tag = (frame[0].toInt() ushr 6) and 0x3 + assertTrue(tag == 0x1, "varint(64) should use 2-byte form, got tag=$tag") + val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame) + assertContentEquals(OPUS, stripped) + } + + @Test + fun strips_four_byte_varint() { + // ~4.2 million µs ≈ 4.2 s — comfortably past the 2-byte 16383 µs + // limit and well within any real broadcast lifetime. + val frame = Varint.encode(4_200_000L) + OPUS + val tag = (frame[0].toInt() ushr 6) and 0x3 + assertTrue(tag == 0x2, "varint(4_200_000) should use 4-byte form, got tag=$tag") + val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame) + assertContentEquals(OPUS, stripped) + } + + @Test + fun strips_eight_byte_varint() { + // > 2^30 µs (~17.9 minutes) forces the 8-byte form. Models a + // broadcast that's been running long enough for the timestamp + // counter to overflow into the widest tier — perfectly valid + // Opus stream timestamps but not exercised by the smaller + // tiers. + val frame = Varint.encode(2_000_000_000L) + OPUS + val tag = (frame[0].toInt() ushr 6) and 0x3 + assertTrue(tag == 0x3, "varint(2_000_000_000) should use 8-byte form, got tag=$tag") + val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame) + assertContentEquals(OPUS, stripped) + } + + @Test + fun empty_payload_returns_empty() { + val empty = ByteArray(0) + // Same instance — no allocation on the empty path. + assertSame(empty, MoqLiteNestsListener.stripLegacyTimestampPrefix(empty)) + } + + @Test + fun malformed_payload_smaller_than_varint_returns_payload_unchanged() { + // A frame body that's just the high tag byte of an 8-byte + // varint with no follow-up bytes is malformed. The current + // contract is "return the payload unchanged so upstream + // surfaces the corruption rather than silently masking it." + // This pins that contract — if it ever changes (e.g. to + // throw or return empty), this test fails and forces an + // explicit decision. + val malformed = byteArrayOf(0xC0.toByte()) // tag=11, expects 8 bytes total + val result = MoqLiteNestsListener.stripLegacyTimestampPrefix(malformed) + assertContentEquals(malformed, result, "malformed frame returned unchanged") + assertSame(malformed, result, "no allocation on the unchanged-malformed path") + } + + @Test + fun round_trip_against_broadcaster_wire_shape() { + // Mirror the byte sequence + // [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster] writes: + // varint(timestamp_us) + raw_opus_packet + // Stripping MUST recover the exact opus packet byte-for-byte + // for every frame the watcher receives. Any drift here means + // every decoded frame loses or gains bytes at the front. + val timestamps = longArrayOf(0L, 20_000L, 40_000L, 1_000_000L, 2_000_000_000L) + for (ts in timestamps) { + val tsLen = Varint.size(ts) + val frame = ByteArray(tsLen + OPUS.size) + Varint.writeTo(ts, frame, 0) + OPUS.copyInto(frame, tsLen) + val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame) + assertContentEquals(OPUS, stripped, "round-trip failed for timestamp $ts") + } + } + + private companion object { + // 8 bytes of plausible-looking Opus packet bytes — the function + // doesn't care about the codec content, only the length / + // content equality after the strip. + private val OPUS = byteArrayOf(0x78, 0x12, 0x34, 0x56, 0x78.toByte(), 0x9A.toByte(), 0xBC.toByte(), 0xDE.toByte()) + } +} diff --git a/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/audio/NestPlayerTest.kt b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/audio/NestPlayerTest.kt index 9d63e22ca..43e729d4d 100644 --- a/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/audio/NestPlayerTest.kt +++ b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/audio/NestPlayerTest.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.test.runTest +import java.util.concurrent.atomic.AtomicInteger import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertEquals @@ -232,7 +233,7 @@ class NestPlayerTest { val sut = NestPlayer( - decoder = decoder, + initialDecoder = decoder, player = player, scope = this, prerollFrames = 3, @@ -289,7 +290,7 @@ class NestPlayerTest { val sut = NestPlayer( - decoder = decoder, + initialDecoder = decoder, player = player, scope = this, prerollFrames = 5, @@ -326,7 +327,7 @@ class NestPlayerTest { val sut = NestPlayer( - decoder = decoder, + initialDecoder = decoder, player = player, scope = this, prerollFrames = 3, @@ -340,11 +341,120 @@ class NestPlayerTest { sut.stop() } + @Test + fun publisher_boundary_rebuilds_decoder_when_factory_provided() = + runTest { + // Two distinct decoders so we can prove the factory was + // invoked. After the trackAlias change, frames should + // route through `decoderB`, NOT `decoderA`. + val decoderA = FakeOpusDecoder { ShortArray(it.size) { _ -> 0xAA.toShort() } } + val decoderB = FakeOpusDecoder { ShortArray(it.size) { _ -> 0xBB.toShort() } } + val factoryCallCount = AtomicInteger(0) + val factory: () -> OpusDecoder = { + if (factoryCallCount.getAndIncrement() == 0) decoderA else decoderB + } + val player = FakeAudioPlayer() + + val objects = + flowOf( + // First subscription cycle: trackAlias = 7 + moqObject(byteArrayOf(0x01), trackAlias = 7L), + moqObject(byteArrayOf(0x02), trackAlias = 7L), + // Wrapper re-issued — new SUBSCRIBE produces a + // different trackAlias. Decoder MUST be rebuilt + // before the next decode runs. + moqObject(byteArrayOf(0x03), trackAlias = 8L), + moqObject(byteArrayOf(0x04), trackAlias = 8L), + ) + + val sut = + NestPlayer( + initialDecoder = factory(), + player = player, + scope = this, + decoderFactory = factory, + ) + sut.play(objects) + testScheduler.advanceUntilIdle() + + assertEquals(2, factoryCallCount.get(), "factory invoked twice: initial + boundary") + assertEquals(1, decoderA.releaseCount, "decoderA released on the boundary") + assertEquals(0, decoderB.releaseCount, "decoderB still alive (released on stop)") + sut.stop() + assertEquals(1, decoderB.releaseCount, "decoderB released on stop") + } + + @Test + fun publisher_boundary_keeps_old_decoder_when_factory_throws() = + runTest { + // The decoder field MUST NOT be left referencing a released + // decoder if the factory throws — every subsequent decode + // would otherwise fail with `IllegalStateException` and the + // subscription would be permanently dead. + val decoderA = FakeOpusDecoder { ShortArray(it.size) { _ -> 0xAA.toShort() } } + val factoryFailures = AtomicInteger(0) + val factory: () -> OpusDecoder = { + factoryFailures.incrementAndGet() + throw IllegalStateException("synthetic factory failure") + } + val player = FakeAudioPlayer() + + val objects = + flowOf( + moqObject(byteArrayOf(0x01), trackAlias = 7L), + // Boundary: factory throws → decoderA must stay alive + // and decode the next frame normally. + moqObject(byteArrayOf(0x02), trackAlias = 8L), + ) + + val sut = + NestPlayer( + initialDecoder = decoderA, + player = player, + scope = this, + decoderFactory = factory, + ) + sut.play(objects) + testScheduler.advanceUntilIdle() + + assertEquals(1, factoryFailures.get(), "factory invoked once on the boundary") + // decoderA still alive → both frames decoded through it. + assertEquals(2, player.queued.size) + assertEquals(0, decoderA.releaseCount, "decoderA NOT released on factory failure") + sut.stop() + assertEquals(1, decoderA.releaseCount, "decoderA released exactly once on stop") + } + + @Test + fun publisher_boundary_no_op_when_factory_is_null() = + runTest { + // Without a factory, NestPlayer keeps the same decoder + // across trackAlias changes — backwards-compat path. + val decoder = FakeOpusDecoder { byteToShorts(it) } + val player = FakeAudioPlayer() + val objects = + flowOf( + moqObject(byteArrayOf(0x01), trackAlias = 7L), + moqObject(byteArrayOf(0x02), trackAlias = 8L), + ) + + val sut = NestPlayer(decoder, player, this) + sut.play(objects) + testScheduler.advanceUntilIdle() + + assertEquals(0, decoder.releaseCount, "no boundary-driven release without a factory") + sut.stop() + assertEquals(1, decoder.releaseCount, "released exactly once on stop") + } + // -- helpers ----------------------------------------------------------- - private fun moqObject(payload: ByteArray): MoqObject = + private fun moqObject( + payload: ByteArray, + trackAlias: Long = 1L, + ): MoqObject = MoqObject( - trackAlias = 1, + trackAlias = trackAlias, groupId = 0, objectId = 0, publisherPriority = 0x80, diff --git a/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHangCatalogTest.kt b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHangCatalogTest.kt new file mode 100644 index 000000000..138bb17b1 --- /dev/null +++ b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHangCatalogTest.kt @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.moq.lite + +import kotlin.test.Test +import kotlin.test.assertEquals + +class MoqLiteHangCatalogTest { + @Test + fun opusMono48kEmitsCanonicalHangShape() { + // Byte-exact assertion: `:commons`'s `RoomSpeakerCatalogTest` + // round-trips this same string against the parser. If either + // side drifts from the kixelated/hang wire shape, both tests + // fail at once. + val expected = + "{\"audio\":{\"renditions\":{\"audio/data\":{" + + "\"codec\":\"opus\",\"container\":{\"kind\":\"legacy\"}," + + "\"sampleRate\":48000,\"numberOfChannels\":1,\"jitter\":20}}}}" + val actual = + MoqLiteHangCatalog.opusMono48k("audio/data").encodeJsonBytes().decodeToString() + assertEquals(expected, actual) + } + + @Test + fun renditionKeyMatchesCallerSuppliedTrackName() { + val actual = + MoqLiteHangCatalog.opusMono48k("custom/track").encodeJsonBytes().decodeToString() + // The rendition map MUST be keyed on the caller-supplied track + // name — the watcher uses this string verbatim as the + // SUBSCRIBE.track on the audio subscription. + assertEquals(true, actual.contains("\"custom/track\":{")) + } +} diff --git a/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteSessionTest.kt b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteSessionTest.kt index a7cad504d..37eaa430c 100644 --- a/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteSessionTest.kt +++ b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteSessionTest.kt @@ -340,6 +340,65 @@ class MoqLiteSessionTest { session.close() } + @Test + fun publisher_startSequence_seeds_first_group_for_hot_swap_continuation() = + runBlocking { + val (clientSide, serverSide) = FakeWebTransport.pair() + val session = MoqLiteSession.client(clientSide, pumpScope) + + // Mint a publisher with a non-zero startSequence — simulates + // the hot-swap path's "carry forward old publisher's + // nextSequence" contract. + val publisher = + session.publish( + broadcastSuffix = "speakerPubkey", + track = "audio/data", + startSequence = 42L, + ) + assertEquals(42L, publisher.nextSequence, "fresh publisher reports startSequence as next") + + // Wire up a subscriber so send() doesn't short-circuit. + val subBidi = serverSide.openBidiStream() + subBidi.write(Varint.encode(MoqLiteControlType.Subscribe.code)) + subBidi.write( + MoqLiteCodec.encodeSubscribe( + MoqLiteSubscribe( + id = 11L, + broadcast = "speakerPubkey", + track = "audio/data", + priority = 0x80, + ordered = true, + maxLatencyMillis = 0L, + startGroup = null, + endGroup = null, + ), + ), + ) + withTimeout(2_000) { subBidi.incoming().first() } + + assertEquals(true, publisher.send("opus-1".encodeToByteArray())) + // FIN before draining so toList() terminates rather than + // blocking indefinitely on the still-open uni stream. + publisher.endGroup() + // After the first send, nextSequence should advance to 43. + assertEquals(43L, publisher.nextSequence) + + // First uni stream's GroupHeader.sequence MUST be 42, not 0. + val relayUni = withTimeout(2_000) { serverSide.incomingUniStreams().first() } + val uniChunks = relayUni.incoming().toList() + val buf = MoqLiteFrameBuffer() + uniChunks.forEach { buf.push(it) } + assertEquals(MoqLiteDataType.Group.code, buf.readVarint()) + val header = + MoqLiteCodec.decodeGroupHeader( + buf.readSizePrefixed() ?: error("group header missing"), + ) + assertEquals(42L, header.sequence, "first group's sequence is the seeded startSequence") + + publisher.close() + session.close() + } + @Test fun publisher_send_returns_false_when_no_inbound_subscriber() = runBlocking { @@ -357,6 +416,167 @@ class MoqLiteSessionTest { session.close() } + @Test + fun publisher_setOnNewSubscriber_hook_fires_per_inbound_subscribe() = + runBlocking { + val (clientSide, serverSide) = FakeWebTransport.pair() + val session = MoqLiteSession.client(clientSide, pumpScope) + + val publisher = session.publish(broadcastSuffix = "speakerPubkey", track = "audio/data") + val hookFireCount = + java.util.concurrent.atomic + .AtomicInteger(0) + publisher.setOnNewSubscriber { + hookFireCount.incrementAndGet() + } + + // First inbound SUBSCRIBE → hook fires once. + val subBidi1 = serverSide.openBidiStream() + subBidi1.write(Varint.encode(MoqLiteControlType.Subscribe.code)) + subBidi1.write( + MoqLiteCodec.encodeSubscribe( + MoqLiteSubscribe( + id = 0L, + broadcast = "speakerPubkey", + track = "audio/data", + priority = 0x80, + ordered = true, + maxLatencyMillis = 0L, + startGroup = null, + endGroup = null, + ), + ), + ) + // Wait for SubscribeOk to drain so we know registerInboundSubscription + // ran (and therefore the hook had its chance to launch). + withTimeout(2_000) { subBidi1.incoming().first() } + // Hook fires asynchronously on the session scope; give it a + // moment to land. Use a bounded retry rather than a flat + // delay so the test is fast on the happy path. + withTimeout(2_000) { + while (hookFireCount.get() < 1) kotlinx.coroutines.yield() + } + assertEquals(1, hookFireCount.get()) + + // Second inbound SUBSCRIBE → hook fires again. + val subBidi2 = serverSide.openBidiStream() + subBidi2.write(Varint.encode(MoqLiteControlType.Subscribe.code)) + subBidi2.write( + MoqLiteCodec.encodeSubscribe( + MoqLiteSubscribe( + id = 1L, + broadcast = "speakerPubkey", + track = "audio/data", + priority = 0x80, + ordered = true, + maxLatencyMillis = 0L, + startGroup = null, + endGroup = null, + ), + ), + ) + withTimeout(2_000) { subBidi2.incoming().first() } + withTimeout(2_000) { + while (hookFireCount.get() < 2) kotlinx.coroutines.yield() + } + assertEquals(2, hookFireCount.get()) + + publisher.close() + session.close() + } + + @Test + fun publisher_setOnNewSubscriber_hook_does_not_fire_on_track_mismatch() = + runBlocking { + val (clientSide, serverSide) = FakeWebTransport.pair() + val session = MoqLiteSession.client(clientSide, pumpScope) + + // Publisher serves audio/data only. + val publisher = session.publish(broadcastSuffix = "speakerPubkey", track = "audio/data") + val hookFireCount = + java.util.concurrent.atomic + .AtomicInteger(0) + publisher.setOnNewSubscriber { + hookFireCount.incrementAndGet() + } + + // Inbound SUBSCRIBE for a different track. Replies + // SubscribeDrop (covered in another test); hook MUST NOT + // fire because no subscriber was actually registered on + // this publisher. + val subBidi = serverSide.openBidiStream() + subBidi.write(Varint.encode(MoqLiteControlType.Subscribe.code)) + subBidi.write( + MoqLiteCodec.encodeSubscribe( + MoqLiteSubscribe( + id = 7L, + broadcast = "speakerPubkey", + track = "video/data", + priority = 0x80, + ordered = true, + maxLatencyMillis = 0L, + startGroup = null, + endGroup = null, + ), + ), + ) + // Drain the Drop reply. + withTimeout(2_000) { subBidi.incoming().first() } + // No way to wait deterministically for "the hook didn't + // fire"; sleep briefly to let any racing launch surface, + // then assert. Short delay because the hook would launch + // on the same scope as registerInboundSubscription's caller. + kotlinx.coroutines.delay(100) + assertEquals(0, hookFireCount.get()) + + publisher.close() + session.close() + } + + @Test + fun publisher_replies_subscribeDrop_when_track_is_not_published() = + runBlocking { + val (clientSide, serverSide) = FakeWebTransport.pair() + val session = MoqLiteSession.client(clientSide, pumpScope) + + // Publisher serves audio/data only. + val publisher = session.publish(broadcastSuffix = "speakerPubkey", track = "audio/data") + + // Relay opens a Subscribe bidi for a DIFFERENT track. The + // session must reply with SubscribeDrop carrying the + // TRACK_DOES_NOT_EXIST code rather than a silent FIN — + // otherwise the watcher's response wait resolves only when + // the bidi is FIN'd, with no indication WHY (looks + // identical to "publisher disappeared mid-subscribe"). + val subBidi = serverSide.openBidiStream() + subBidi.write(Varint.encode(MoqLiteControlType.Subscribe.code)) + subBidi.write( + MoqLiteCodec.encodeSubscribe( + MoqLiteSubscribe( + id = 99L, + broadcast = "speakerPubkey", + track = "video/data", + priority = 0x80, + ordered = true, + maxLatencyMillis = 0L, + startGroup = null, + endGroup = null, + ), + ), + ) + + val ackChunk = withTimeout(2_000) { subBidi.incoming().first() } + val resp = MoqLiteCodec.decodeSubscribeResponse(ackChunk) + val dropped = resp as MoqLiteCodec.SubscribeResponse.Dropped + assertEquals(MoqLiteSubscribeDropCode.TRACK_DOES_NOT_EXIST, dropped.drop.errorCode) + // Reason phrase is informational; pin substring rather than + // the exact text so we can keep tweaking the wording. + kotlin.test.assertContains(dropped.drop.reasonPhrase, "video/data") + + publisher.close() + session.close() + } + @Test fun publisher_close_emits_ended_announce() = runBlocking { diff --git a/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/trace/NestsTraceTest.kt b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/trace/NestsTraceTest.kt new file mode 100644 index 000000000..774e05f5e --- /dev/null +++ b/nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/trace/NestsTraceTest.kt @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.nestsclient.trace + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Pure-string tests for [jsonStr] / [jsonArrStr] (the JSON-quoting + * helpers used at every trace call site) and a small toggle test for + * [NestsTrace.setRecording]'s state-machine. + * + * The actual `emit` log-output side is untestable in commonTest because + * `com.vitorpamplona.quartz.utils.Log` writes to logcat / stdout via + * platform actuals, not via an injectable sink. The schema correctness + * we DO want to pin — a JSON-syntax bug at one of the call sites would + * silently corrupt the trace file and break replay tooling — is covered + * by exercising the quoting helpers exhaustively and asserting on + * concatenated round-trip equality with hand-built JSON literals. + */ +class NestsTraceTest { + @Test + fun jsonStrEscapesQuotesAndBackslashes() { + assertEquals("\"hello\"", jsonStr("hello")) + assertEquals("\"with \\\"quotes\\\"\"", jsonStr("with \"quotes\"")) + assertEquals("\"backslash \\\\ here\"", jsonStr("backslash \\ here")) + } + + @Test + fun jsonStrEscapesControlCharacters() { + assertEquals("\"line1\\nline2\"", jsonStr("line1\nline2")) + assertEquals("\"col1\\tcol2\"", jsonStr("col1\tcol2")) + assertEquals("\"crlf\\r\\n\"", jsonStr("crlf\r\n")) + } + + @Test + fun jsonStrEscapesLowControlCharsAsUnicode() { + //  (start of heading) — must be  in JSON, not raw. + val raw = "xy" + val quoted = jsonStr(raw) + assertEquals("\"x\\u0001y\"", quoted) + } + + @Test + fun jsonStrLeavesPrintableAsciiAlone() { + // Every printable ASCII char that isn't `"` or `\` must round-trip + // unmodified — most production trace fields are pubkey hex, + // track names, event-kind enums. + val allPrintable = + (0x20..0x7e) + .map { it.toChar() } + .filter { it != '"' && it != '\\' } + .joinToString("") + val quoted = jsonStr(allPrintable) + assertEquals("\"$allPrintable\"", quoted) + } + + @Test + fun jsonArrStrEmitsValidJsonArray() { + assertEquals("[]", jsonArrStr(emptyList())) + assertEquals("[\"a\"]", jsonArrStr(listOf("a"))) + assertEquals( + "[\"alpha\",\"beta\",\"gamma\"]", + jsonArrStr(listOf("alpha", "beta", "gamma")), + ) + } + + @Test + fun jsonArrStrEscapesElementsConsistentlyWithJsonStr() { + // Each element runs through jsonStr — quotes and backslashes + // inside an element must be escaped just like a stand-alone field. + assertEquals( + "[\"a\\\"b\",\"c\\\\d\"]", + jsonArrStr(listOf("a\"b", "c\\d")), + ) + } + + @Test + fun setRecordingIsIdempotent() { + // Set up clean state for the test — flip off in case a prior + // test left the recorder enabled. (No reset() API by design; + // tests share the singleton.) + NestsTrace.setRecording(false) + assertFalse(NestsTrace.isRecording()) + + NestsTrace.setRecording(true) + assertTrue(NestsTrace.isRecording()) + + // Double-enable: no change in state, no error. + NestsTrace.setRecording(true) + assertTrue(NestsTrace.isRecording()) + + NestsTrace.setRecording(false) + assertFalse(NestsTrace.isRecording()) + + // Double-disable: no change in state, no error. + NestsTrace.setRecording(false) + assertFalse(NestsTrace.isRecording()) + } + + @Test + fun emitIsNoOpWhenDisabled() { + // Lambda must not run when tracing is off — call sites pass + // a non-trivial allocator (string concat) and we promise zero + // work on the disabled path. + NestsTrace.setRecording(false) + var lambdaRanCount = 0 + NestsTrace.emit("would_have_recorded") { + lambdaRanCount += 1 + "" + } + assertEquals(0, lambdaRanCount, "emit's fields lambda must not run when tracing is disabled") + } + + @Test + fun emitRunsLambdaWhenEnabled() { + NestsTrace.setRecording(true) + try { + var lambdaRanCount = 0 + NestsTrace.emit("did_record") { + lambdaRanCount += 1 + "\"k\":\"v\"" + } + assertEquals(1, lambdaRanCount, "emit's fields lambda must run exactly once when enabled") + } finally { + NestsTrace.setRecording(false) + } + } +} diff --git a/nestsClient/src/jvmAndroid/kotlin/com/vitorpamplona/nestsclient/transport/QuicWebTransportFactory.kt b/nestsClient/src/jvmAndroid/kotlin/com/vitorpamplona/nestsclient/transport/QuicWebTransportFactory.kt index 5f24c438d..4f5fe7e59 100644 --- a/nestsClient/src/jvmAndroid/kotlin/com/vitorpamplona/nestsclient/transport/QuicWebTransportFactory.kt +++ b/nestsClient/src/jvmAndroid/kotlin/com/vitorpamplona/nestsclient/transport/QuicWebTransportFactory.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.nestsclient.transport +import com.vitorpamplona.nestsclient.moq.lite.MoqLiteAlpn import com.vitorpamplona.quic.connection.QuicConnection import com.vitorpamplona.quic.connection.QuicConnectionConfig import com.vitorpamplona.quic.connection.QuicConnectionDriver @@ -89,8 +90,27 @@ class QuicWebTransportFactory( * post-CONNECT message is decoded as SETUP_CLIENT, producing * `connection closed err=invalid value` on the relay side and a stalled * subscribe / `subscribe stream FIN before reply` on the client side. + * + * **Lite-04 is intentionally NOT advertised** even though the kixelated + * browser client now ships it. Lite-04 reshapes the on-the-wire + * Announce.hops field from a single varint count into a full + * `OriginList` (`kixelated/moq` commit 45db108, "moq-lite/moq-relay: + * hop-based clustering"), adds an `exclude_hop` field to + * AnnounceInterest, and adds an `rtt` field to Probe — Subscribe and + * Group framing are unchanged but Announce framing diverges. Our codec + * speaks pure Lite-03; if a Lite-04-preferring relay picks + * `moq-lite-04` from our advertised list, the very first Announce + * exchange would desync (we'd encode/read a bare hops varint where + * the peer expects `len + len × u62`) and the connection would abort. + * Until [com.vitorpamplona.nestsclient.moq.lite.MoqLiteCodec] gains + * version-aware Announce / AnnounceInterest / Probe paths and + * [com.vitorpamplona.nestsclient.moq.lite.MoqLiteAnnounce.hops] + * becomes a list, advertising Lite-04 is a footgun. See + * [com.vitorpamplona.nestsclient.moq.lite.MoqLiteAlpn] for the + * known-version constants. */ - private val webTransportSubProtocols: List = listOf("moq-lite-03"), + private val webTransportSubProtocols: List = + listOf(MoqLiteAlpn.LITE_03), ) : WebTransportFactory { override suspend fun connect( authority: String,