fix(nests): bump preroll to 200 ms + surface persistent decode failures
Two audit follow-ups for receive-path interop with kixelated/moq web publishers (NestsUI v2's reference encoder): 1. ROOM_PLAYER_PREROLL_FRAMES 5 → 10 (≈100 ms → ≈200 ms). The web reference encoder uses `groupDuration: 100ms` (5 Opus frames per group) and stacks another 100–200 ms of jitter on top via the relay's outbound forward queue under residential network conditions. 100 ms of preroll reliably underran on web speakers, producing audible clicks between adjacent groups; 200 ms covers the observed envelope while keeping perceived join latency well below the wire latency a listener already sees. 2. Per-subscription consecutive Opus decode-error counter in NestViewModel.openSubscription. Single-frame decode errors are normal noise — Opus PLC papers over a single bad frame and the prior code silently swallowed them — but a structural mismatch (wrong wire format, missing legacy-container varint strip, codec/channel-count mismatch, corrupted Opus) fails *every* frame in a row. The previous behaviour made exactly that class of bug invisible: no audio, no log, no UI signal. New logic increments a per-subscription AtomicLong on each DecoderError / EncoderError, resets on every successful onLevel tick. When the streak hits DECODE_ERROR_STREAK_LOG_THRESHOLD (50 frames ≈ 1 s of solid failures) we log ONE conspicuous warning tagged `NestRx` with the underlying throwable's stacktrace. Logged once per crossing (exact-equality, not modulus) so a stuck stream produces a single attention-grabbing line instead of a 50 Hz flood. Picks the non-lambda Log.w overload deliberately so the throwable stacktrace survives — fires exactly once per stuck-stream streak, so the unconditional message-string build is fine. Both changes are additive — no callers changed, no public API touched. The stale `ROOM_PLAYER_PREROLL_FRAMES = 5` reference in NestMoqLiteBroadcaster's framesPerGroup kdoc is updated to match. https://claude.ai/code/session_014JfZJHSTvyYYWJbC9VbB47
This commit is contained in:
+77
-7
@@ -45,6 +45,7 @@ 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
|
||||
@@ -1142,6 +1143,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 ->
|
||||
@@ -1158,7 +1172,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,
|
||||
@@ -1170,7 +1205,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()
|
||||
@@ -1677,16 +1718,45 @@ sealed class BroadcastUiState {
|
||||
*/
|
||||
const val SPEAKING_TIMEOUT_MS: Long = 250L
|
||||
|
||||
/**
|
||||
* 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
|
||||
|
||||
+1
-1
@@ -460,7 +460,7 @@ class NestMoqLiteBroadcaster(
|
||||
* 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
|
||||
* `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
|
||||
|
||||
Reference in New Issue
Block a user