From a94d12638f9936d1730299271064d7d06fa2d06d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 6 May 2026 18:37:19 +0000 Subject: [PATCH] perf(nests): bound encoder CSD loop, single-alloc audio framing, cache catalog JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three audit follow-ups, all in the audio publish hot path: Audit-3: MediaCodecOpusEncoder.encode could busy-loop on a buggy encoder that emits BUFFER_FLAG_CODEC_CONFIG on every dequeue. The existing format-change path has a one-shot `formatChangeAbsorbed` guard for exactly this reason; the new CSD-skip path didn't. Cap consecutive CSD skips per encode call at MAX_CSD_SKIPS_PER_CALL=4 (generous: Codec2 emits 1 OpusHead or 2 OpusHead+OpusTags in practice). On overshoot, log a warning and return ByteArray(0) — the broadcaster's `if (opus.isEmpty()) continue` already handles that contract. Audit-6: NestMoqLiteBroadcaster's per-frame send path allocated twice per Opus packet — once for the timestamp Varint and once for the concatenated `varint+opus` payload. At 50 fps × N speakers that's measurable young-gen pressure. Switch to the same single-allocation pattern PublisherStateImpl.send already uses: compute Varint.size(timestampUs), allocate the final payload buffer once, write the varint directly into it via Varint.writeTo, then copy the opus bytes. Drops audio-thread allocations from 3/frame to 1/frame (the third was the wrap in PublisherStateImpl.send, already optimised). Audit-7: MoqLiteNestsSpeaker.startBroadcasting and ReconnectingNestsSpeaker.runHotSwapIteration both encode the same fixed catalog JSON on every call — once per broadcast start and once per JWT-refresh hot-swap iteration. Cache the encoded bytes on MoqLiteHangCatalog.Companion.OPUS_MONO_48K_AUDIO_DATA_JSON_BYTES so the kotlinx.serialization run happens at class-init time and both call sites read a static reference. Trivial perf, but co-locates the "default Amethyst publish shape" in one place. https://claude.ai/code/session_014JfZJHSTvyYYWJbC9VbB47 --- .../audio/MediaCodecOpusEncoder.kt | 31 +++++++++++++++++++ .../nestsclient/MoqLiteNestsSpeaker.kt | 3 +- .../nestsclient/ReconnectingNestsSpeaker.kt | 3 +- .../audio/NestMoqLiteBroadcaster.kt | 21 ++++++++++--- .../moq/lite/MoqLiteHangCatalog.kt | 18 +++++++++++ 5 files changed, 68 insertions(+), 8 deletions(-) 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 fbdec2790..038ee293a 100644 --- a/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusEncoder.kt +++ b/nestsClient/src/androidMain/kotlin/com/vitorpamplona/nestsclient/audio/MediaCodecOpusEncoder.kt @@ -91,6 +91,13 @@ 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 { @@ -119,6 +126,20 @@ class MediaCodecOpusEncoder( "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. + com.vitorpamplona.quartz.utils.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). @@ -163,6 +184,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/MoqLiteNestsSpeaker.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt index d324bc753..de09a3424 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/MoqLiteNestsSpeaker.kt @@ -157,8 +157,7 @@ class MoqLiteNestsSpeaker internal constructor( // 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.opusMono48k(MoqLiteNestsListener.AUDIO_TRACK).encodeJsonBytes() + val catalogJson = MoqLiteHangCatalog.OPUS_MONO_48K_AUDIO_DATA_JSON_BYTES catalogPublisher.setOnNewSubscriber { runCatching { catalogPublisher.send(catalogJson) diff --git a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsSpeaker.kt b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsSpeaker.kt index 79cf850d7..897a93656 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsSpeaker.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/ReconnectingNestsSpeaker.kt @@ -586,8 +586,7 @@ private class ReissuingBroadcastHandle( // 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.opusMono48k(MoqLiteNestsListener.AUDIO_TRACK).encodeJsonBytes() + val catalogPayload = MoqLiteHangCatalog.OPUS_MONO_48K_AUDIO_DATA_JSON_BYTES val priorCatalogPublisher = hotSwapCatalogPublisher val newCatalogPublisher = try { 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 f83344db1..4a8144d0b 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestMoqLiteBroadcaster.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/audio/NestMoqLiteBroadcaster.kt @@ -325,10 +325,23 @@ class NestMoqLiteBroadcaster( // for the production cliff this works around. val sendOutcome = runCatching { - val tsBytes = Varint.encode(timestampUs) - val payload = ByteArray(tsBytes.size + opus.size) - tsBytes.copyInto(payload, 0) - opus.copyInto(payload, tsBytes.size) + // 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) { 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 index 6e2483790..e52b75c08 100644 --- a/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHangCatalog.kt +++ b/nestsClient/src/commonMain/kotlin/com/vitorpamplona/nestsclient/moq/lite/MoqLiteHangCatalog.kt @@ -106,6 +106,24 @@ internal data class MoqLiteHangCatalog( 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