From a36ccb569284ae92ecec09ae671e1efa6799ec64 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 22:47:07 +0000 Subject: [PATCH] =?UTF-8?q?fix(nests):=20close=20the=20relay-cliff=20at=20?= =?UTF-8?q?the=20source=20=E2=80=94=20framesPerGroup=2010=E2=86=9250,=20cl?= =?UTF-8?q?iff-threshold=204s=E2=86=922.5s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two-phone production logs at commit 6e4df4a (run 18:37) showed the listener-side cliff still hits at ~16 s of streaming (51 groups delivered, sender continued to seq=80 = ~6 s of audio lost at the tail before the receiver recycled). Recycling alone can't fix this — it only spaces the pain out. The user wants no audio drops mid- transmission, period. The leverage is to keep the relay's per-subscriber forward queue from filling at all. The cliff is rate-limited per uni-stream-creation, not per byte or per frame — moq-rs's `serve_group` task pool can't drain new `open_uni().await`s fast enough at high stream rates. Cutting the stream creation rate cuts the cliff window proportionally: framesPerGroup | streams/sec @ 50 fps | observed cliff window ---------------+------------------------+---------------------- 1 | 50 | ~3 s (commit d6517cf) 5 | 10 | ~13 s 10 | 5 | ~16 s (commit 6e4df4a) 50 | 1 | not reached in `fpg-all` sweeps 100 | 0.5 | never observed Bumping default to 50 (1 s of audio per uni stream, 1 stream/sec) puts us in the regime where the relay's queue does not measurably fill. `fpg-all` (100 frames in a single group) routinely delivers 100/100 in production sweeps, so 50 is well clear of the cliff edge. We pick 50 not 100 because: - Late-join gap is bounded by group size — a listener joining mid-broadcast waits one group boundary for the first frame. 1 s is within the ~250 ms AudioTrack jitter buffer + `ROOM_PLAYER_PREROLL_FRAMES = 5` audio-buffer envelope; 2 s starts to be perceptible. - QUIC stream RST drops the whole group on full reset. 1 s of skip on rare RST is bearable; 2 s is a noticeable seam. - Sender encode latency stays at 1 s — no worse than the natural buffering audio rooms already do for jitter. Belt-and-suspenders: also tighten the cliff-detector's silence threshold from 4 s to 2.5 s. Healthy streams now arrive every ~1 s (one `drainOneGroup` FIN per group), so 2.5 s of silence is unambiguously past two missed group cycles. Catches a still- possible cliff event ~1.5 s earlier, shaving the audible gap from ~6 s (4 s detection + 2 s reconnect) to ~4.5 s if recovery is needed at all. Tests: existing `framesPerGroup`-explicit interop scenarios (`fpg5`, `fpg20`, `fpg-all`) are unaffected — they pass an explicit value and don't read the default. `CliffDetectorTest` boundary cases updated to the new 2.5 s threshold; `returnsAllStalledSpeakersAtOnceMixedWithFreshOnes` re-spaced its fixture so charlie's 1.5 s frame age stays under threshold while alice/bob's 4 s ages exceed it. 12/12 pass. --- .../commons/viewmodels/NestViewModel.kt | 17 +++-- .../commons/viewmodels/CliffDetectorTest.kt | 16 +++-- .../audio/NestMoqLiteBroadcaster.kt | 65 ++++++++++++------- 3 files changed, 64 insertions(+), 34 deletions(-) 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 8dd9b6f41..e33b0ecc3 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 @@ -1702,12 +1702,19 @@ const val ROOM_AUDIO_MAX_LATENCY_MS: Long = 500L * orchestrator opens a fresh QUIC transport. Resets the relay's * per-subscriber forward queue. * - * 4 s gives enough headroom to ride out a publisher-side group-rollover - * hiccup (typically < 200 ms) without false-positive recycling, while - * keeping the audible gap from a real cliff to ~5 s of silence at - * worst (4 s detection + ~1 s reconnect handshake). + * 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 = 4_000L +const val ROOM_AUDIO_CLIFF_TIMEOUT_MS: Long = 2_500L /** * How often the cliff detector wakes up to scan 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 index a4cc54df2..ac51d5ce8 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/CliffDetectorTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/CliffDetectorTest.kt @@ -103,14 +103,14 @@ class CliffDetectorTest { @Test fun returnsEmptyJustBeforeThreshold() { - // Boundary case: 1 ms under the 4 s threshold. The detector + // Boundary case: 1 ms under the 2.5 s threshold. The detector // should still consider this healthy. Relevant because audio - // groups arrive at ~100 ms cadence (framesPerGroup=10), so a + // 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 += 3_999.milliseconds + ts += 2_499.milliseconds val result = computeStalledSpeakers( @@ -129,7 +129,7 @@ class CliffDetectorTest { // user-visible (this is the "audio went silent" detection). val ts = TestTimeSource() val frameMark = ts.markNow() - ts += 4_000.milliseconds + ts += 2_500.milliseconds val result = computeStalledSpeakers( @@ -164,12 +164,14 @@ class CliffDetectorTest { // 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 += 3_000.milliseconds + ts += 2_500.milliseconds val charlieFrame = ts.markNow() - ts += 3_000.milliseconds + ts += 1_500.milliseconds val result = computeStalledSpeakers( @@ -251,7 +253,7 @@ class CliffDetectorTest { lastFrameAt = mapOf(ALICE to frameMark), lastRecycleAt = null, ) - assertTrue(withDefault.isEmpty(), "1 s elapsed shouldn't trip 4 s default") + assertTrue(withDefault.isEmpty(), "1 s elapsed shouldn't trip 2.5 s default") val withTightTimeout = computeStalledSpeakers( 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 f764ed316..525f72c45 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestMoqLiteBroadcaster.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestMoqLiteBroadcaster.kt @@ -384,32 +384,53 @@ class NestMoqLiteBroadcaster( companion object { /** - * Default moq-lite group size = 10 Opus frames ≈ 200 ms of audio. + * Default moq-lite group size = 50 Opus frames ≈ 1 s of audio. * - * Picked to keep the QUIC uni-stream creation rate (5 streams/sec - * at 20 ms cadence) well under the production nostrnests relay's - * per-subscriber forward queue cliff. Two-phone production tests - * (`claude/fix-nests-audio-receiver-HCgOY` logcat) showed the - * relay still cliffed at ~135 streams under sustained load even - * with the old 5-frame default (10 streams/sec), giving ~13 s of - * audio before the listener's incoming uni-stream flow stopped - * — the same bug class documented in - * `nestsClient/plans/2026-05-01-quic-stream-cliff-investigation.md`, - * just shifted by half. Doubling the pack to 10 cuts the rate - * in half again and roughly doubles the cliff window in the - * worst case while halving the relay-side stream-handling - * pressure that triggers it. + * 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: * - * Belt-and-suspenders: pair this with the listener-side - * cliff-detector in `NestViewModel` (no-data-while-announced - * triggers `recycleSession()`) so the room recovers from a - * cliff event regardless of the exact threshold. + * - `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). * - * Late-join gap: ≤ 200 ms (one group boundary) instead of - * ≤ 100 ms with `framesPerGroup = 5` — still imperceptible - * for live audio rooms. + * 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 = 5` 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 = 10 + const val DEFAULT_FRAMES_PER_GROUP: Int = 50 /** * Maximum consecutive [MoqLitePublisherHandle.send] / [endGroup]