diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/PcmTapRegistry.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/PcmTapRegistry.kt index 5348d81d99..70b32c1a03 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/PcmTapRegistry.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/PcmTapRegistry.kt @@ -66,6 +66,10 @@ class SpectrumAudioBufferSink( private var channels = 1 private var encoding = C.ENCODING_PCM_16BIT + // Audio time one fft frame covers: fftSize samples per channel at the stream's sample rate. + // Zero until the first flush reports a rate, which leaves the frame unpaced rather than wrong. + private var frameDurationNanos = 0L + @kotlin.OptIn(ExperimentalCoroutinesApi::class) override fun flush( sampleRateHz: Int, @@ -74,6 +78,7 @@ class SpectrumAudioBufferSink( ) { this.channels = channelCount.coerceAtLeast(1) this.encoding = encoding + this.frameDurationNanos = if (sampleRateHz > 0) fftSize * 1_000_000_000L / sampleRateHz else 0L filled = 0 output?.resetReplayCache() } @@ -99,7 +104,7 @@ class SpectrumAudioBufferSink( // Skip the DC bin (index 0): toLogBins ignores it, so letting a DC/offset component be the // peak would scale every audible bin toward zero and wash the spectrum out. mags.normalizeToPeakInPlace(fromIndex = 1) - output?.tryEmit(Spectrum(mags.toLogBins(binCount))) + output?.tryEmit(Spectrum(mags.toLogBins(binCount), frameDurationNanos)) } } @@ -113,8 +118,8 @@ class SpectrumAudioBufferSink( object PcmTapRegistry { private const val MAX_TRACKED_FLOWS = 64 - // Frames buffered per media flow beyond the 1-frame replay. One decoder buffer is typically a - // handful of 1024-sample hops; 63 leaves room for an unusually large one without letting a + // Frames buffered per media flow beyond the 1-frame replay. A cluster is ~15 fft frames (~0.33 s + // of audio, measured on a Pixel 9a); 63 leaves room for an unusually large one without letting a // stalled UI bank more than ~1.5 s of stale spectrum. private const val SPECTRUM_BUFFER_FRAMES = 63 @@ -173,12 +178,13 @@ object PcmTapRegistry { if (!fedByLiveSink && !stillCollected) iter.remove() } } - // The audio thread emits every fft frame of a decoder buffer synchronously, with no - // suspension point, while the UI collector sits on the main dispatcher and cannot - // interleave. A 2-slot buffer therefore capped the visualizer at two frames per - // decoder buffer however much audio it carried — the update rate tracked the decoder - // buffer rate (~5 Hz), not the ~43 Hz the fft produces. Hold a whole burst instead, - // and drop the STALEST frame rather than the newest when the UI does fall behind. + // The audio thread emits in clusters — the pipeline fills its output buffer ~3 times a + // second, so ~15 fft frames land within a few ms of each other (one per decoder call, + // or several from one large call) — while the UI collector sits on the main dispatcher + // and cannot run in between. A 2-slot buffer therefore kept only ~2 frames of each + // cluster and dropped the rest. Hold a whole cluster instead, and drop the STALEST + // frame rather than the newest if the UI does fall behind. (Spreading the cluster + // back over time is SpectrumTrail's job, not this flow's.) MutableSharedFlow( replay = 1, extraBufferCapacity = SPECTRUM_BUFFER_FRAMES, diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumAudioBufferSinkTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumAudioBufferSinkTest.kt index 6e9ce2ea35..012ddf6e72 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumAudioBufferSinkTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumAudioBufferSinkTest.kt @@ -136,4 +136,29 @@ class SpectrumAudioBufferSinkTest { assertTrue("non-16-bit PCM must not emit a spectrum", out.replayCache.isEmpty()) } + + // Pacing draws each frame for the audio time it covers, so the sink has to say how long that is. + // It is fftSize samples PER CHANNEL: a stereo stream must not report half (or double) the time. + @Test + fun eachFrameCarriesTheAudioTimeItCovers() { + val sink = SpectrumAudioBufferSink(fftSize = fftSize, binCount = binCount) + val out = MutableSharedFlow(replay = 1, extraBufferCapacity = 1) + sink.output = out + sink.flush(48000, 1, C.ENCODING_PCM_16BIT) + sink.handleBuffer(monoPcm(sineShorts(k = 2, n = fftSize))) + + assertEquals(fftSize * 1_000_000_000L / 48000, out.replayCache.last().durationNanos) + } + + @Test + fun stereoFramesCoverTheSameTimeAsMono() { + val sink = SpectrumAudioBufferSink(fftSize = fftSize, binCount = binCount) + val out = MutableSharedFlow(replay = 1, extraBufferCapacity = 1) + sink.output = out + sink.flush(44100, 2, C.ENCODING_PCM_16BIT) + val tone = sineShorts(k = 2, n = fftSize) + sink.handleBuffer(interleavedStereoPcm(tone, tone)) + + assertEquals(fftSize * 1_000_000_000L / 44100, out.replayCache.last().durationNanos) + } } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/AudioSpectrum.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/AudioSpectrum.kt index 86eb82822c..c074eecc49 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/AudioSpectrum.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/AudioSpectrum.kt @@ -26,9 +26,16 @@ import kotlin.math.exp import kotlin.math.ln import kotlin.math.log10 -/** One frame of frequency-domain magnitudes, ordered low→high Hz and normalized 0f..1f. */ +/** + * One frame of frequency-domain magnitudes, ordered low→high Hz and normalized 0f..1f. + * + * [durationNanos] is how much audio the frame describes (fft size / sample rate), which is how long + * it should stay on screen. Zero — the default — means the producer is not pacing to audio (the + * synthetic preview emits one per display frame), so the frame is shown as soon as it arrives. + */ class Spectrum( val bins: FloatArray, + val durationNanos: Long = 0L, ) /** diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacer.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacer.kt index bc8fc2c913..e6f8cc2e1d 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacer.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacer.kt @@ -21,17 +21,13 @@ package com.vitorpamplona.amethyst.commons.audio /** - * Spreads a bursty spectrum stream over the frames that draw it. + * The bounded queue between the audio tap and the visualizer. * - * The decoder hands the pcm tap a whole buffer at once, so spectrum frames arrive in bursts that run - * ahead of what is audible (the tap sits upstream of the audio output — see `delayedByFrames`). - * Delivering that burst straight into Compose state collapses it: every write lands before the next - * vsync, so the burst draws ONCE, showing only its newest frame. The visual then steps at the - * decoder-buffer rate instead of the ~43 Hz the fft produces. - * - * Buffering here and taking exactly one frame per drawn frame turns the burst back into motion. - * Production (~43 Hz) is slower than the display (60 Hz+), so the queue drains and sits near empty; - * [next] then returns null and the caller simply holds the frame it already has. + * Frames reach the UI in clusters, because the audio pipeline fills its output buffer in chunks + * (~15 frames about three times a second, measured on a Pixel 9a). This holds them until + * [SpectrumTrail] releases each one when its audio time comes due. It is bounded and evicts the + * stalest frame, so faster-than-real-time playback or a stalled UI cannot bank an ever-growing + * backlog and leave the picture permanently behind the sound. * * Not thread-safe by design: both ends run on the UI dispatcher. */ @@ -48,6 +44,8 @@ class SpectrumPacer( queue.addLast(frame) } + fun isEmpty(): Boolean = queue.isEmpty() + /** The next frame to draw, or null when the queue is empty and the last frame should persist. */ fun next(): Spectrum? = queue.removeFirstOrNull() diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrail.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrail.kt index e4bb934759..98e40f7770 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrail.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrail.kt @@ -21,36 +21,68 @@ package com.vitorpamplona.amethyst.commons.audio /** - * The whole per-displayed-frame step of the spectrum visualizer: take one queued frame, apply the - * decay trail against what is already on screen, and return a fresh array to draw. + * The whole per-displayed-frame step of the spectrum visualizer: release queued frames as their + * audio time comes due, apply the decay trail against what is already on screen, and return a + * fresh array to draw. * - * Bursty spectrum frames are paced through a [SpectrumPacer] (see its docs for why), and the decay - * gives each bin an instant attack and a gradual release, so bars snap up to a transient and fall - * back smoothly instead of flickering. + * Frames arrive in clusters (see [SpectrumPacer]). Releasing one per display refresh drained a + * ~15-frame cluster in ~110 ms at 120 Hz and then froze until the next one — measured as ~3 stalls + * of ~210 ms every second. Each frame instead stays up for the [Spectrum.durationNanos] of audio it + * describes, so a cluster spreads across the gap to the next. Time lost while starved is not owed + * back: when frames return after a gap they resume from the current frame time rather than being + * dumped at once to catch up. * - * This lives here, rather than inline in the Compose collector, so the queueing, starvation and - * decay rules are unit-testable without a Compose harness; the composable is left as plumbing. + * The decay gives each bin an instant attack and a gradual release, so bars snap up to a transient + * and fall back smoothly instead of flickering. + * + * This lives here, rather than inline in the Compose collector, so the pacing, starvation and decay + * rules are unit-testable without a Compose harness; the composable is left as plumbing. * * Not thread-safe by design: both ends run on the UI dispatcher. */ class SpectrumTrail( private val decay: Float, - capacity: Int = SpectrumPacer.DEFAULT_CAPACITY, + capacity: Int = MAX_BACKLOG_FRAMES, ) { private val pacer = SpectrumPacer(capacity) private var drawn = FloatArray(0) - /** Queues a freshly decoded frame, evicting the stalest if the UI has fallen behind. */ + // When the frame at the head of the queue may be drawn, in the caller's frame-clock nanos. + private var dueNanos = Long.MIN_VALUE + private var starved = true + + /** Queues a freshly decoded frame, evicting the stalest if the backlog is at capacity. */ fun offer(frame: Spectrum) = pacer.offer(frame) /** - * The next array to draw, or null when no frame is queued and the current one should persist. + * The array to draw at [frameTimeNanos], or null when nothing new is due and the current one + * should persist. * * A fresh array each time is intentional: `mutableStateOf` compares by reference, so a new * instance is what signals Compose to redraw. Do NOT switch to in-place mutation. */ - fun nextOrNull(): FloatArray? { - val frame = pacer.next() ?: return null + fun nextOrNull(frameTimeNanos: Long): FloatArray? { + if (pacer.isEmpty()) { + starved = true + return null + } + if (starved) { + // Resume from now, but never earlier than the last drawn frame's audio time runs out. + dueNanos = maxOf(dueNanos, frameTimeNanos) + starved = false + } + + var result: FloatArray? = null + while (dueNanos <= frameTimeNanos) { + val frame = pacer.next() ?: break + result = decayedFrom(frame) + dueNanos += frame.durationNanos + } + + return result + } + + private fun decayedFrom(frame: Spectrum): FloatArray { val prev = drawn val next = FloatArray(frame.bins.size) { i -> @@ -60,4 +92,11 @@ class SpectrumTrail( drawn = next return next } + + companion object { + // A cluster is ~15 frames, so 24 (~0.5 s of audio) never trims ordinary playback, while + // capping how far the picture can lag the sound when frames come faster than real time + // (2x playback speed). + const val MAX_BACKLOG_FRAMES = 24 + } } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrailTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrailTest.kt index 9708e33105..39a73cb261 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrailTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrailTest.kt @@ -24,49 +24,139 @@ import kotlin.test.Test import kotlin.test.assertContentEquals import kotlin.test.assertNotSame import kotlin.test.assertNull -import kotlin.test.assertTrue /** - * The whole per-displayed-frame step the visualizer runs: take one queued frame, apply the decay - * trail against what is already drawn, and hand back a fresh array. Extracted from SpectrumCanvas - * so it is testable without a Compose harness — the composable is left as plumbing around this. + * The per-displayed-frame step of the visualizer: release queued frames as their audio time comes + * due, apply the decay trail against what is drawn, and hand back a fresh array. + * + * Frames reach the UI in clusters (~15 at a time, ~3 times a second, measured on a Pixel 9a) because + * the audio pipeline fills its output buffer in chunks. Releasing one per display refresh drained a + * cluster in ~110 ms at 120 Hz and then froze for ~220 ms. Frames must instead be released at the + * rate of the audio they describe, so a cluster spreads across the gap to the next one. */ class SpectrumTrailTest { - private fun frame(vararg bins: Float) = Spectrum(bins) + private val ms = 1_000_000L + + private fun frame( + value: Float, + durationMs: Long = 20, + ) = Spectrum(floatArrayOf(value), durationNanos = durationMs * ms) @Test fun yieldsNothingWhenStarvedSoTheDrawnFrameIsHeld() { val trail = SpectrumTrail(decay = 0.5f) - assertNull(trail.nextOrNull()) + assertNull(trail.nextOrNull(0)) } @Test - fun theFirstFrameIsDrawnAsIsWithNothingToDecayFrom() { + fun theFirstFrameIsDrawnOnArrivalWithNothingToDecayFrom() { val trail = SpectrumTrail(decay = 0.5f) - trail.offer(frame(1f, 0.5f, 0f)) + trail.offer(frame(1f)) - assertContentEquals(floatArrayOf(1f, 0.5f, 0f), trail.nextOrNull()) + assertContentEquals(floatArrayOf(1f), trail.nextOrNull(0)) + } + + @Test + fun aClusterIsSpreadOverTheAudioTimeItCoversNotTheScreenRefresh() { + val trail = SpectrumTrail(decay = 0f) + trail.offer(frame(1f)) + trail.offer(frame(2f)) + trail.offer(frame(3f)) + + // 120 Hz refreshes (~8 ms) must not drain a frame that covers 20 ms of audio. + assertContentEquals(floatArrayOf(1f), trail.nextOrNull(0)) + assertNull(trail.nextOrNull(8 * ms)) + assertNull(trail.nextOrNull(16 * ms)) + assertContentEquals(floatArrayOf(2f), trail.nextOrNull(20 * ms)) + assertNull(trail.nextOrNull(33 * ms)) + assertContentEquals(floatArrayOf(3f), trail.nextOrNull(40 * ms)) + assertNull(trail.nextOrNull(48 * ms)) + } + + @Test + fun afterStarvingTheNextClusterStartsWhenItArrivesInsteadOfBeingDumpedToCatchUp() { + val trail = SpectrumTrail(decay = 0f) + trail.offer(frame(1f)) + assertContentEquals(floatArrayOf(1f), trail.nextOrNull(0)) + assertNull(trail.nextOrNull(100 * ms)) // starved for a while + + trail.offer(frame(2f)) + trail.offer(frame(3f)) + + // The idle time is not owed: frame 2 shows now and frame 3 a full frame later. + assertContentEquals(floatArrayOf(2f), trail.nextOrNull(300 * ms)) + assertNull(trail.nextOrNull(308 * ms)) + assertContentEquals(floatArrayOf(3f), trail.nextOrNull(320 * ms)) + } + + @Test + fun aFrameArrivingBeforeThePreviousOneElapsedStillWaitsItsTurn() { + val trail = SpectrumTrail(decay = 0f) + trail.offer(frame(1f)) + assertContentEquals(floatArrayOf(1f), trail.nextOrNull(0)) + assertNull(trail.nextOrNull(8 * ms)) // queue momentarily empty + + trail.offer(frame(2f)) + + assertNull(trail.nextOrNull(16 * ms)) // frame 1 still covers until 20 ms + assertContentEquals(floatArrayOf(2f), trail.nextOrNull(24 * ms)) + } + + @Test + fun aSlowFrameClockJumpsToTheLatestDueFrame() { + val trail = SpectrumTrail(decay = 0f) + trail.offer(frame(1f)) + trail.offer(frame(2f)) + trail.offer(frame(3f)) + + assertContentEquals(floatArrayOf(1f), trail.nextOrNull(0)) + // A janky 45 ms frame: frames 2 (due 20) and 3 (due 40) are both due; show the newest. + assertContentEquals(floatArrayOf(3f), trail.nextOrNull(45 * ms)) + } + + @Test + fun aFrameWithNoDurationIsShownOnArrival() { + // Producers that do not declare a duration (the synthetic preview emits one per display + // frame) keep the old behaviour: whatever is queued is current. + val trail = SpectrumTrail(decay = 0f) + trail.offer(Spectrum(floatArrayOf(1f))) + trail.offer(Spectrum(floatArrayOf(2f))) + + assertContentEquals(floatArrayOf(2f), trail.nextOrNull(0)) + } + + @Test + fun aBacklogIsCappedByDroppingTheStalestFrame() { + // Faster-than-real-time playback (2x speed) produces frames faster than they come due; + // the cap bounds how far the picture can lag the sound. + val trail = SpectrumTrail(decay = 0f, capacity = 2) + trail.offer(frame(1f)) + trail.offer(frame(2f)) + trail.offer(frame(3f)) // evicts frame 1 + + assertContentEquals(floatArrayOf(2f), trail.nextOrNull(0)) + assertContentEquals(floatArrayOf(3f), trail.nextOrNull(20 * ms)) + assertNull(trail.nextOrNull(40 * ms)) } @Test fun aRisingBinTakesItsNewValueImmediately() { val trail = SpectrumTrail(decay = 0.5f) trail.offer(frame(0.2f)) - trail.nextOrNull() + trail.nextOrNull(0) trail.offer(frame(0.9f)) - assertContentEquals(floatArrayOf(0.9f), trail.nextOrNull()) + assertContentEquals(floatArrayOf(0.9f), trail.nextOrNull(20 * ms)) } @Test fun aFallingBinDecaysFromWhatWasDrawnRatherThanSnapping() { val trail = SpectrumTrail(decay = 0.5f) trail.offer(frame(1f)) - trail.nextOrNull() + trail.nextOrNull(0) trail.offer(frame(0f)) - // 1f * 0.5 decay beats the new 0f, so the bar falls gradually. - assertContentEquals(floatArrayOf(0.5f), trail.nextOrNull()) + assertContentEquals(floatArrayOf(0.5f), trail.nextOrNull(20 * ms)) } @Test @@ -75,22 +165,10 @@ class SpectrumTrailTest { trail.offer(frame(1f)) trail.offer(frame(1f)) - val first = trail.nextOrNull() - val second = trail.nextOrNull() + val first = trail.nextOrNull(0) + val second = trail.nextOrNull(20 * ms) // mutableStateOf compares by reference; reusing one array would never trigger a redraw. assertNotSame(first, second) } - - @Test - fun aBacklogIsBoundedByDroppingTheStalestFrame() { - val trail = SpectrumTrail(decay = 0f, capacity = 2) - trail.offer(frame(1f)) - trail.offer(frame(2f)) - trail.offer(frame(3f)) // evicts the 1f frame - - assertContentEquals(floatArrayOf(2f), trail.nextOrNull()) - assertContentEquals(floatArrayOf(3f), trail.nextOrNull()) - assertTrue(trail.nextOrNull() == null) - } } diff --git a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumCanvas.kt b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumCanvas.kt index 1326ffac57..56a2ec66d9 100644 --- a/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumCanvas.kt +++ b/commonsUI/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumCanvas.kt @@ -26,6 +26,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.withFrameMillis +import androidx.compose.runtime.withFrameNanos import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.drawscope.DrawScope import kotlinx.coroutines.flow.Flow @@ -35,10 +36,10 @@ import kotlinx.coroutines.flow.Flow * then calls [draw] inside the Canvas draw lambda. The fast-changing state is read * ONLY in the draw lambda, so new frames trigger the draw phase, never recomposition. * - * Frames are paced one per displayed frame through [SpectrumTrail] — they arrive from the decoder in - * bursts, and writing a burst straight into state would collapse it into a single draw. The pacing - * loop runs every frame but only writes state when a spectrum frame is actually queued, so the redraw - * rate still tracks the ~43 Hz the fft produces rather than the display. + * Frames are released through [SpectrumTrail] as the audio they describe comes due — they arrive + * in clusters, and neither dumping a cluster into state nor draining it one per vsync looks live. + * The pacing loop runs every frame but only writes state when a frame is due, so the redraw rate + * tracks the ~43-47 Hz the fft produces rather than the display. * * Pass [animated] = false for non-time-varying styles (bars, radial): that drops the monotonic clock, * whose whole purpose is to redraw every frame even when the spectrum has not moved. @@ -54,10 +55,10 @@ fun SpectrumCanvas( ) { val smoothed = remember { mutableStateOf(FloatArray(0)) } - // Frames arrive in decoder-sized bursts that run ahead of the audio, so they are queued and drawn - // one per displayed frame. Writing a whole burst straight into `smoothed` would collapse it into a - // single draw at the next vsync and the visual would step at the decoder-buffer rate. Queueing and - // decay live in SpectrumTrail so they are testable without a Compose harness. + // Frames arrive in clusters, so they are queued and released as their audio time comes due. + // Writing a cluster straight into `smoothed` collapses it into one draw; releasing one per vsync + // races through it and then freezes. Pacing and decay live in SpectrumTrail so they are testable + // without a Compose harness. val trail = remember(spectrum, decay) { SpectrumTrail(decay) } LaunchedEffect(spectrum, trail) { spectrum.collect { trail.offer(it) } @@ -65,10 +66,10 @@ fun SpectrumCanvas( LaunchedEffect(trail) { while (true) { - withFrameMillis { } - // Null means starved. Production (~43 Hz) is slower than the display, so this is the common - // case between frames: hold what is drawn rather than redrawing identical bins. - smoothed.value = trail.nextOrNull() ?: continue + val frameTimeNanos = withFrameNanos { it } + // Null means nothing new is due. Frames cover ~21 ms of audio against an 8-16 ms refresh, + // so this is the common case: hold what is drawn rather than redrawing identical bins. + smoothed.value = trail.nextOrNull(frameTimeNanos) ?: continue } }