fix(nests): G4+G5 plumb catalog sampleRate through decoder + AudioTrack

G4: Audit-9 (catalog-driven decoder reconfig) plumbed numberOfChannels end-to-end but left sampleRate hardcoded at AudioFormat.SAMPLE_RATE_HZ in MediaCodecOpusDecoder.buildOpusIdHeader / buildFormat and in AudioTrackPlayer's MinBufferSize / 250 ms target / setSampleRate calls. For Opus this is benign in practice — Codec2 always emits 48 kHz PCM regardless of OpusHead inputSampleRate — but hardcoding the constant means a future codec or container variant whose decoder DOES respect input sample rate would mis-clock playback. And the OpusHead identification header should match what the catalog declares either way. Thread sampleRate alongside channelCount through every layer: - MediaCodecOpusDecoder constructor takes `sampleRate: Int = AudioFormat.SAMPLE_RATE_HZ`. buildFormat and buildOpusIdHeader both take it as a parameter. - AudioTrackPlayer constructor takes the same. Used in AudioTrack.getMinBufferSize, the 250 ms-equivalent target bytes calculation (`(sampleRate / 4) * BYTES_PER_SAMPLE * channelCount`), AndroidAudioFormat.Builder.setSampleRate, and the diagnostic log. - decoderFactory and playerFactory in NestViewModel become `(channelCount: Int, sampleRate: Int) -> ...`. - awaitDecoderChannelCount → awaitAudioPipelineConfig, returning a private `AudioPipelineConfig(channelCount, sampleRate)` struct so openSubscription handles both fields uniformly. `sampleRate` falls back to AudioFormat.SAMPLE_RATE_HZ on timeout / non-positive declaration with a warning log. - NestViewModelFactory + NestViewModelTest updated to the two-arg factory shape. G5: documented the SUBSCRIBE_BUFFER safety budget on CATALOG_AWAIT_TIMEOUT_MS's kdoc. With the current 500 ms timeout + SUBSCRIBE_BUFFER = 64-frame DROP_OLDEST flow + 50 fps Opus, at most 25 frames buffer during the wait — leaving ≥ 39 frames of margin (≈ 780 ms) before the oldest frame would be evicted. Even at the production framesPerGroup = 50 (1 group/sec) cadence this never trips during normal startup. No code change; just pinning the rationale so a future timeout bump is checked against the buffer size. https://claude.ai/code/session_014JfZJHSTvyYYWJbC9VbB47
2026-05-06 19:36:05 +00:00
parent 75f572ba3d
commit 8a49486b37
5 changed files with 157 additions and 59 deletions
@@ -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 = { channelCount -> MediaCodecOpusDecoder(channelCount = channelCount) },
-            playerFactory = { channelCount -> AudioTrackPlayer(channelCount = channelCount) },
+            decoderFactory = { channelCount, sampleRate ->
+                MediaCodecOpusDecoder(channelCount = channelCount, sampleRate = sampleRate)
+            },
+            playerFactory = { channelCount, sampleRate ->
+                AudioTrackPlayer(channelCount = channelCount, sampleRate = sampleRate)
+            },
            signer = signer,
            room = room,
            captureFactory = { AudioRecordCapture() },
@@ -87,10 +87,11 @@ import kotlin.coroutines.cancellation.CancellationException
 * [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. Both factories take a `channelCount` (1 for
- * mono, 2 for stereo) discovered from the publisher's `catalog.json`
- * audio rendition; subscriptions await a brief catalog-arrival window
- * before constructing the decoder + player so a stereo web publisher
- * doesn't get its frames decoded as mono with downmix artifacts.
+ * 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`,
@@ -107,8 +108,8 @@ import kotlin.coroutines.cancellation.CancellationException
 class NestViewModel(
    private val httpClient: NestsClient,
    private val transport: WebTransportFactory,
-    private val decoderFactory: (channelCount: Int) -> OpusDecoder,
-    private val playerFactory: (channelCount: Int) -> 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
@@ -1049,12 +1050,15 @@ class NestViewModel(

    /**
     * Wait briefly for [pubkey]'s catalog to land in [_speakerCatalogs]
-     * and pick the channel count for the decoder + AudioTrack. Returns
-     * [AudioFormat.CHANNELS] on timeout (the catalog never arrived
-     * within [timeoutMs]) or when the catalog declares an unsupported
-     * count (anything outside `1..2`). Caller is responsible for
-     * having started [fetchSpeakerCatalog] first; otherwise this
-     * always times out.
+     * 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
@@ -1065,10 +1069,10 @@ class NestViewModel(
     * within one round-trip of the SUBSCRIBE_OK, so this typically
     * returns within tens of ms.
     */
-    private suspend fun awaitDecoderChannelCount(
+    private suspend fun awaitAudioPipelineConfig(
        pubkey: String,
        timeoutMs: Long,
-    ): Int {
+    ): AudioPipelineConfig {
        val catalog =
            withTimeoutOrNull(timeoutMs) {
                _speakerCatalogs
@@ -1076,26 +1080,60 @@ class NestViewModel(
                    .filterNotNull()
                    .first()
            }
-        val declared = catalog?.primaryAudio()?.numberOfChannels
-        return when {
-            declared == null -> {
-                AudioFormat.CHANNELS
-            }
-
-            declared !in 1..2 -> {
-                Log.w("NestRx") {
-                    "publisher catalog for pubkey='${pubkey.take(8)}' declares numberOfChannels=$declared " +
-                        "(only 1 / 2 supported); falling back to mono"
+        val rendition = catalog?.primaryAudio()
+        val declaredChannels = rendition?.numberOfChannels
+        val channels =
+            when {
+                declaredChannels == null -> {
+                    AudioFormat.CHANNELS
                }
-                AudioFormat.CHANNELS
-            }

-            else -> {
-                declared
+                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 —
@@ -1171,25 +1209,28 @@ class NestViewModel(
            // [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 channelCount = awaitDecoderChannelCount(pubkey, CATALOG_AWAIT_TIMEOUT_MS)
+            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).
            // Per-subscription decoder factory closure: captures the
-            // catalog-derived [channelCount] so a publisher-boundary
+            // catalog-derived [audioCfg] so a publisher-boundary
            // decoder rebuild (see [NestPlayer]'s `decoderFactory`
-            // kdoc) reuses the SAME channel layout — without it, a
-            // rebuild after a cliff-recycle would default to mono and
-            // a stereo publisher would silently downmix on the new
-            // decoder. The factory is also called for the initial
-            // decoder so the construction path is uniform.
-            val perSubscriptionDecoderFactory: () -> OpusDecoder = { decoderFactory(channelCount) }
+            // 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(channelCount)
+                    playerFactory(audioCfg.channelCount, audioCfg.sampleRate)
                } catch (t: Throwable) {
                    runCatching { decoder.release() }
                    throw t
@@ -1808,16 +1849,27 @@ 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`, which
- * the decoder needs at construction time so a stereo web publisher
- * doesn't get its frames decoded as mono with downmix artifacts.
+ * 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 mono config.
+ * 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

@@ -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 =
@@ -86,11 +86,26 @@ class AudioTrackPlayer(
     * 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
@@ -123,14 +138,14 @@ class AudioTrackPlayer(

        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
@@ -138,10 +153,12 @@ class AudioTrackPlayer(
        // 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. Stereo doubles the byte count
-        // per sample (interleaved L,R 16-bit shorts) — scaling
-        // [channelCount] in keeps the wall-clock target constant.
+        // 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 * channelCount
+            (sampleRate / 4) * AudioFormat.BYTES_PER_SAMPLE * channelCount
        val bufferBytes = maxOf(minBuffer * 16, targetBytes250Ms)

        val newTrack =
@@ -158,7 +175,7 @@ class AudioTrackPlayer(
                        AndroidAudioFormat
                            .Builder()
                            .setEncoding(AndroidAudioFormat.ENCODING_PCM_16BIT)
-                            .setSampleRate(AudioFormat.SAMPLE_RATE_HZ)
+                            .setSampleRate(sampleRate)
                            .setChannelMask(channelMask)
                            .build(),
                    ).setBufferSizeInBytes(bufferBytes)
@@ -199,7 +216,7 @@ class AudioTrackPlayer(
        track = newTrack
        com.vitorpamplona.quartz.utils.Log.d("NestPlay") {
            "AudioTrack allocated: state=${newTrack.state} playState=${newTrack.playState} " +
-                "bufferSizeBytes=$bufferBytes minBuffer=$minBuffer sampleRate=${AudioFormat.SAMPLE_RATE_HZ}"
+                "bufferSizeBytes=$bufferBytes minBuffer=$minBuffer sampleRate=$sampleRate"
        }
    }

@@ -50,11 +50,30 @@ import java.nio.ByteOrder
 */
 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 =
@@ -62,7 +81,7 @@ class MediaCodecOpusDecoder(
            MediaCodec
                .createDecoderByType(MediaFormat.MIMETYPE_AUDIO_OPUS)
                .apply {
-                    configure(buildFormat(channelCount), null, null, 0)
+                    configure(buildFormat(channelCount, sampleRate), null, null, 0)
                    start()
                }.also {
                    com.vitorpamplona.quartz.utils.Log.d("NestPlay") {
@@ -221,14 +240,17 @@ class MediaCodecOpusDecoder(

        private const val FRAME_DURATION_US = 20_000L // 20 ms

-        private fun buildFormat(channelCount: Int): MediaFormat {
+        private fun buildFormat(
+            channelCount: Int,
+            sampleRate: Int,
+        ): MediaFormat {
            val format =
                MediaFormat.createAudioFormat(
                    MediaFormat.MIMETYPE_AUDIO_OPUS,
-                    AudioFormat.SAMPLE_RATE_HZ,
+                    sampleRate,
                    channelCount,
                )
-            format.setByteBuffer("csd-0", ByteBuffer.wrap(buildOpusIdHeader(channelCount)))
+            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()))
@@ -236,7 +258,10 @@ class MediaCodecOpusDecoder(
            return format
        }

-        private fun buildOpusIdHeader(channelCount: Int): ByteArray {
+        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).
@@ -245,7 +270,7 @@ class MediaCodecOpusDecoder(
            buf.put(1.toByte()) // version
            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()