From 0af3395b582507bdc5ccffe2a3b6726b3b0eb382 Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 24 Sep 2026 12:04:13 +0200 Subject: [PATCH 1/6] fix(audio): stop dropping all but 2 spectrum frames per decoder buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pcm tap emits every fft frame of a decoder buffer synchronously from the audio thread, with no suspension point in between, while the UI collector sits on the main dispatcher and cannot interleave. The per-media flow was built with `replay = 1, extraBufferCapacity = 1` and published via `tryEmit`, which on a SUSPEND-overflow flow discards rather than blocks. The buffer therefore filled at 2 and the rest of every burst was thrown away — a hard cap of two frames per decoder buffer no matter how much audio it carried. The visualizer updated at the decoder-buffer rate (~5 Hz) instead of the ~43 Hz the fft actually produces, which is why every live style (bars, waves, radial, aurora) felt equally sluggish. Hold a whole burst instead, and drop the stalest frame rather than the newest if the UI does fall behind. The test pins delivery through the real PcmTapRegistry wiring; before this change it reported 2 frames delivered for bursts of 8, 20 and 50 alike. --- .../playback/playerPool/PcmTapRegistry.kt | 18 +++- .../playerPool/SpectrumBurstDeliveryTest.kt | 86 +++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumBurstDeliveryTest.kt 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 62129db719..5348d81d99 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 @@ -30,6 +30,7 @@ import com.vitorpamplona.amethyst.commons.audio.Spectrum import com.vitorpamplona.amethyst.commons.audio.normalizeToPeakInPlace import com.vitorpamplona.amethyst.commons.audio.toLogBins import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import java.nio.ByteBuffer @@ -112,6 +113,11 @@ 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 + // stalled UI bank more than ~1.5 s of stale spectrum. + private const val SPECTRUM_BUFFER_FRAMES = 63 + private val lock = Any() // access-order LinkedHashMap → eldest (least-recently-used) entries iterate first for eviction. @@ -167,7 +173,17 @@ object PcmTapRegistry { if (!fedByLiveSink && !stillCollected) iter.remove() } } - MutableSharedFlow(replay = 1, extraBufferCapacity = 1) + // 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. + MutableSharedFlow( + replay = 1, + extraBufferCapacity = SPECTRUM_BUFFER_FRAMES, + onBufferOverflow = BufferOverflow.DROP_OLDEST, + ) } } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumBurstDeliveryTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumBurstDeliveryTest.kt new file mode 100644 index 0000000000..d9c9a48c63 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumBurstDeliveryTest.kt @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.playerPool + +import androidx.annotation.OptIn +import androidx.media3.common.C +import androidx.media3.common.util.UnstableApi +import com.vitorpamplona.amethyst.commons.audio.Spectrum +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test +import java.nio.ByteBuffer +import java.nio.ByteOrder + +/** + * The audio thread runs [SpectrumAudioBufferSink.handleBuffer] to completion, emitting EVERY fft + * frame of one decoder buffer synchronously with no suspension point in between. The UI collector + * lives on the main dispatcher and cannot interleave with that burst, so anything that does not fit + * in the flow's buffer at that instant is silently dropped by `tryEmit`. + * + * With a 2-slot buffer that capped the visualizer at two frames per decoder buffer no matter how + * much audio it carried, decoupling the update rate from the ~43 Hz the fft actually produces. + */ +@kotlin.OptIn(ExperimentalCoroutinesApi::class) +@OptIn(UnstableApi::class) +class SpectrumBurstDeliveryTest { + private val fftSize = 64 + + private fun monoPcm(totalSamples: Int): ByteBuffer { + val bb = ByteBuffer.allocate(totalSamples * 2).order(ByteOrder.LITTLE_ENDIAN) + for (i in 0 until totalSamples) bb.putShort(((i % 100) * 300).toShort()) + bb.flip() + return bb + } + + /** Frames the UI receives from ONE decoder buffer holding [framesInBurst] fft frames. */ + private fun deliveredFromOneBurst(framesInBurst: Int): Int { + var count = -1 + runTest { + val mediaId = "test://burst-$framesInBurst" + val sink = SpectrumAudioBufferSink(fftSize = fftSize, binCount = 16) + PcmTapRegistry.bind(mediaId, sink) + sink.flush(48000, 1, C.ENCODING_PCM_16BIT) + + val received = mutableListOf() + val collector = launch { PcmTapRegistry.spectrumFor(mediaId).collect { received.add(it) } } + advanceUntilIdle() // let the collector subscribe + + sink.handleBuffer(monoPcm(fftSize * framesInBurst)) + + advanceUntilIdle() // now let the UI collector run + collector.cancel() + PcmTapRegistry.bind(null, sink) + count = received.size + } + return count + } + + @Test + fun everyFrameOfADecoderBurstReachesTheVisualizer() { + assertEquals("burst of 8", 8, deliveredFromOneBurst(8)) + assertEquals("burst of 20", 20, deliveredFromOneBurst(20)) + assertEquals("burst of 50", 50, deliveredFromOneBurst(50)) + } +} From 65df16bbc3a8abf9a77d847aaa366b0e52b542c6 Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 24 Sep 2026 12:04:23 +0200 Subject: [PATCH 2/6] fix(audio): pace spectrum frames to the display instead of drawing a burst once Buffering the tap fixes delivery but not motion. Frames still arrive in decoder-sized bursts that run ahead of the audio, and writing a whole burst into Compose state collapses it: every write lands before the next vsync, so the burst draws ONCE and shows only its newest frame. The visual would keep stepping at the decoder-buffer rate despite every frame now arriving. Queue the frames and take exactly one per displayed frame. Production (~43 Hz) is slower than the display, so the queue drains and sits near empty; when it is starved the caller simply holds the frame already drawn rather than redrawing identical bins, so this adds no redundant draws over the previous behaviour. SpectrumPacer is a plain class in commons rather than a flow operator so the queue and eviction rules are unit-testable without a Compose or coroutine harness. It is bounded and evicts the stalest frame so a stalled UI cannot bank a growing backlog and drift permanently behind the audio. --- .../amethyst/commons/audio/SpectrumPacer.kt | 59 ++++++++++++++++ .../commons/audio/SpectrumPacerTest.kt | 67 +++++++++++++++++++ .../amethyst/commons/audio/SpectrumCanvas.kt | 15 ++++- 3 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacer.kt create mode 100644 commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacerTest.kt 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 new file mode 100644 index 0000000000..bc8fc2c913 --- /dev/null +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacer.kt @@ -0,0 +1,59 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.audio + +/** + * Spreads a bursty spectrum stream over the frames that draw it. + * + * 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. + * + * Not thread-safe by design: both ends run on the UI dispatcher. + */ +class SpectrumPacer( + private val capacity: Int = DEFAULT_CAPACITY, +) { + private val queue = ArrayDeque(capacity) + + /** Queues a freshly decoded frame, evicting the stalest if the UI has fallen behind. */ + fun offer(frame: Spectrum) { + // Bound the queue so a stalled UI cannot bank an ever-growing backlog of stale spectrum and + // drift permanently behind the audio. The newest frame is always the one worth keeping. + while (queue.size >= capacity) queue.removeFirst() + queue.addLast(frame) + } + + /** The next frame to draw, or null when the queue is empty and the last frame should persist. */ + fun next(): Spectrum? = queue.removeFirstOrNull() + + companion object { + // ~1.5 s of 1024-sample hops at 44.1 kHz: room for any realistic decoder buffer without + // letting a stalled UI accumulate an unbounded backlog. + const val DEFAULT_CAPACITY = 64 + } +} diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacerTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacerTest.kt new file mode 100644 index 0000000000..5dbaf18950 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacerTest.kt @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.audio + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +class SpectrumPacerTest { + private fun frame(id: Float) = Spectrum(floatArrayOf(id)) + + private fun idOf(spectrum: Spectrum?) = spectrum?.bins?.first() + + @Test + fun drainsOneFramePerTickInArrivalOrder() { + val pacer = SpectrumPacer(capacity = 8) + pacer.offer(frame(1f)) + pacer.offer(frame(2f)) + pacer.offer(frame(3f)) + + assertEquals(1f, idOf(pacer.next())) + assertEquals(2f, idOf(pacer.next())) + assertEquals(3f, idOf(pacer.next())) + } + + @Test + fun returnsNullWhenStarvedSoTheCallerCanHoldTheLastFrame() { + val pacer = SpectrumPacer(capacity = 8) + assertNull(pacer.next()) + + pacer.offer(frame(1f)) + assertEquals(1f, idOf(pacer.next())) + assertNull(pacer.next()) + } + + @Test + fun overCapacityDropsTheStalestFrameNotTheNewest() { + val pacer = SpectrumPacer(capacity = 3) + pacer.offer(frame(1f)) + pacer.offer(frame(2f)) + pacer.offer(frame(3f)) + pacer.offer(frame(4f)) // evicts 1 + + assertEquals(2f, idOf(pacer.next())) + assertEquals(3f, idOf(pacer.next())) + assertEquals(4f, idOf(pacer.next())) + assertNull(pacer.next()) + } +} 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 a3c0b51eca..ae54e3b19f 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 @@ -46,9 +46,22 @@ fun SpectrumCanvas( draw: DrawScope.(bins: FloatArray, timeSec: Float, palette: VisualizerPalette) -> Unit, ) { val smoothed = remember { mutableStateOf(FloatArray(0)) } + + // Frames arrive in decoder-sized bursts that run ahead of the audio, so they are queued here 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. + val pacer = remember(spectrum) { SpectrumPacer() } + LaunchedEffect(spectrum) { + spectrum.collect { pacer.offer(it) } + } + LaunchedEffect(spectrum, decay) { var prev = FloatArray(0) - spectrum.collect { frame -> + while (true) { + withFrameMillis { } + // Starved: production (~43 Hz) is slower than the display, so this is the common case + // between frames. Hold what is already drawn rather than redrawing the same bins. + val frame = pacer.next() ?: continue // A fresh array each frame is intentional: mutableStateOf compares by reference, so a new // instance is what signals Compose to redraw. Do NOT switch to in-place mutation. val next = From 22051c546d7c1d94bdcb2a9e60b1007ff586fc65 Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 24 Sep 2026 12:23:33 +0200 Subject: [PATCH 3/6] refactor(audio): extract the per-frame visualizer step into a testable SpectrumTrail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pacing commit left the queue drain, the decay trail and the fresh-array rule inline in SpectrumCanvas, where none of it could be covered — commonsUI has no Compose test harness, and adding one to a KMP module for three lines of glue is not worth the dependency. Move the whole per-displayed-frame step into SpectrumTrail in commons instead: take one queued frame, decay against what is drawn, return a new array, or null when starved. The composable keeps only the plumbing (collect into offer, drain on withFrameMillis), so what is left untested is code with no branches. Covered: starvation holds the drawn frame, the first frame has nothing to decay from, a rising bin attacks instantly, a falling bin releases gradually, each draw gets a distinct array (mutableStateOf compares by reference, so reuse would silently stop redrawing), and a backlog evicts the stalest frame. Also corrects the SpectrumCanvas KDoc: there is now a per-frame loop whatever `animated` is, so the note about it existing to avoid 60fps redraws no longer described the code. It only governs the monotonic clock. --- .../amethyst/commons/audio/SpectrumTrail.kt | 63 ++++++++++++ .../commons/audio/SpectrumTrailTest.kt | 96 +++++++++++++++++++ .../amethyst/commons/audio/SpectrumCanvas.kt | 40 ++++---- 3 files changed, 178 insertions(+), 21 deletions(-) create mode 100644 commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrail.kt create mode 100644 commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrailTest.kt 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 new file mode 100644 index 0000000000..e4bb934759 --- /dev/null +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrail.kt @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +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. + * + * 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. + * + * 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. + * + * Not thread-safe by design: both ends run on the UI dispatcher. + */ +class SpectrumTrail( + private val decay: Float, + capacity: Int = SpectrumPacer.DEFAULT_CAPACITY, +) { + private val pacer = SpectrumPacer(capacity) + private var drawn = FloatArray(0) + + /** Queues a freshly decoded frame, evicting the stalest if the UI has fallen behind. */ + 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. + * + * 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 + val prev = drawn + val next = + FloatArray(frame.bins.size) { i -> + val prior = if (i < prev.size) prev[i] * decay else 0f + if (frame.bins[i] > prior) frame.bins[i] else prior + } + drawn = next + return next + } +} 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 new file mode 100644 index 0000000000..9708e33105 --- /dev/null +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumTrailTest.kt @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.audio + +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. + */ +class SpectrumTrailTest { + private fun frame(vararg bins: Float) = Spectrum(bins) + + @Test + fun yieldsNothingWhenStarvedSoTheDrawnFrameIsHeld() { + val trail = SpectrumTrail(decay = 0.5f) + assertNull(trail.nextOrNull()) + } + + @Test + fun theFirstFrameIsDrawnAsIsWithNothingToDecayFrom() { + val trail = SpectrumTrail(decay = 0.5f) + trail.offer(frame(1f, 0.5f, 0f)) + + assertContentEquals(floatArrayOf(1f, 0.5f, 0f), trail.nextOrNull()) + } + + @Test + fun aRisingBinTakesItsNewValueImmediately() { + val trail = SpectrumTrail(decay = 0.5f) + trail.offer(frame(0.2f)) + trail.nextOrNull() + trail.offer(frame(0.9f)) + + assertContentEquals(floatArrayOf(0.9f), trail.nextOrNull()) + } + + @Test + fun aFallingBinDecaysFromWhatWasDrawnRatherThanSnapping() { + val trail = SpectrumTrail(decay = 0.5f) + trail.offer(frame(1f)) + trail.nextOrNull() + trail.offer(frame(0f)) + + // 1f * 0.5 decay beats the new 0f, so the bar falls gradually. + assertContentEquals(floatArrayOf(0.5f), trail.nextOrNull()) + } + + @Test + fun eachDrawGetsAFreshArraySoComposeSeesTheChange() { + val trail = SpectrumTrail(decay = 0.5f) + trail.offer(frame(1f)) + trail.offer(frame(1f)) + + val first = trail.nextOrNull() + val second = trail.nextOrNull() + + // 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 ae54e3b19f..1326ffac57 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 @@ -34,7 +34,14 @@ import kotlinx.coroutines.flow.Flow * Collects [spectrum] into a decayed [FloatArray] and optionally runs a monotonic time clock, * 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. - * Pass [animated] = false for non-time-varying styles (bars, radial) to avoid 60fps redraws. + * + * 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. + * + * 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. */ @Composable fun SpectrumCanvas( @@ -47,30 +54,21 @@ fun SpectrumCanvas( ) { val smoothed = remember { mutableStateOf(FloatArray(0)) } - // Frames arrive in decoder-sized bursts that run ahead of the audio, so they are queued here 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. - val pacer = remember(spectrum) { SpectrumPacer() } - LaunchedEffect(spectrum) { - spectrum.collect { pacer.offer(it) } + // 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. + val trail = remember(spectrum, decay) { SpectrumTrail(decay) } + LaunchedEffect(spectrum, trail) { + spectrum.collect { trail.offer(it) } } - LaunchedEffect(spectrum, decay) { - var prev = FloatArray(0) + LaunchedEffect(trail) { while (true) { withFrameMillis { } - // Starved: production (~43 Hz) is slower than the display, so this is the common case - // between frames. Hold what is already drawn rather than redrawing the same bins. - val frame = pacer.next() ?: continue - // A fresh array each frame is intentional: mutableStateOf compares by reference, so a new - // instance is what signals Compose to redraw. Do NOT switch to in-place mutation. - val next = - FloatArray(frame.bins.size) { i -> - val prior = if (i < prev.size) prev[i] * decay else 0f - if (frame.bins[i] > prior) frame.bins[i] else prior - } - smoothed.value = next - prev = next + // 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 } } From 991687a33e563c579a24c19c729fb695593611de Mon Sep 17 00:00:00 2001 From: davotoula Date: Thu, 24 Sep 2026 14:47:34 +0200 Subject: [PATCH 4/6] docs(playback): correct the log-level claims guarding the diagnostic traces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both comments described levels no variant actually uses. DEFAULT_LOG_LEVEL gives debug builds INFO (not DEBUG) and release WARN (not ERROR), and the `benchmark` build type counts as `isDebug`, so it gets INFO too rather than being the silent release-like variant the text implied. The practical consequence was the misleading part: both said a debug build is enough to capture the trace. It is not — these are `Log.d`, which needs `minLevel <= DEBUG`, so PlaybackDiag and VideoQuality are silent in a stock debug build and only appear once Amethyst.VERBOSE_LOGS is flipped to true. RelayUsageListener already documents this correctly ("a debug build defaults to LogLevel.INFO"); these two had drifted from it. --- .../amethyst/service/playback/PlaybackDiag.kt | 10 +++++++--- .../composable/controls/VideoQualityControls.kt | 8 +++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/PlaybackDiag.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/PlaybackDiag.kt index c6cd9c9085..ac5c018225 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/PlaybackDiag.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/PlaybackDiag.kt @@ -22,9 +22,13 @@ package com.vitorpamplona.amethyst.service.playback /** * Shared logcat tag for the playback diagnostic trace (source routing, player lifecycle, error - * recovery, HLS liveness learning). Emitted with `Log.d`, so it appears only in a debug build - * (`Log.minLevel = DEBUG`) and is silent in benchmark/release (`ERROR`). To capture a playback - * investigation, install a debug build and run: + * recovery, HLS liveness learning). + * + * Emitted with `Log.d`, which fires only while `Log.minLevel <= DEBUG`. No shipped variant is + * there by default: [com.vitorpamplona.amethyst.Amethyst.DEFAULT_LOG_LEVEL] gives debug AND + * benchmark builds `INFO` (the `benchmark` build type counts as `isDebug`) and release `WARN`, so + * this trace is silent everywhere until `Amethyst.VERBOSE_LOGS` is flipped to true. To capture a + * playback investigation, set that flag, install a debug build and run: * * ``` * adb logcat -s PlaybackDiag diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityControls.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityControls.kt index dbc3491464..8acee68d0a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityControls.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/VideoQualityControls.kt @@ -148,9 +148,11 @@ const val VIDEO_QUALITY_TAG = "VideoQuality" * Traces which rendition adaptive selection landed on, against the full ladder the manifest * offered. * - * The listener is registered only when the trace can actually be emitted — debug builds set - * `Log.minLevel = DEBUG` while benchmark/release set `ERROR` (see [PLAYBACK_DIAG_TAG]) — so the - * release path keeps the "no listener per player" property that dropping the old selector bought. + * The listener is registered only when the trace can actually be emitted. `Log.minLevel` is above + * `DEBUG` in every variant by default — `INFO` for debug and benchmark builds, `WARN` for release + * (see [PLAYBACK_DIAG_TAG]) — so this costs nothing until `Amethyst.VERBOSE_LOGS` is turned on, + * and every build keeps the "no listener per player" property that dropping the old selector + * bought. */ @Composable fun LogVideoQualitySelection(player: Player) { From 33f684773b86c439cc0bd8c845d0467540f2da14 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 27 Sep 2026 08:37:37 +0200 Subject: [PATCH 5/6] fix(audio): pace visualizer frames by the audio time they cover, not the display refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Releasing one spectrum frame per display refresh still looked jerky on device: bursts of fast motion with regular freezes two or three times a second. Measured on a Pixel 9a at 120 Hz with temporary logging: frames reach the UI in clusters of ~15 roughly every 330 ms, because the audio pipeline fills its output buffer in chunks. Released one per 8.3 ms refresh, a cluster played out in ~110 ms (3x too fast) and the screen then froze for ~220 ms until the next one — 161 stalls over 52.8 s, 3.05 a second, averaging 211 ms. Each Spectrum now carries the audio time it describes (fft size / sample rate, ~21 ms at 48 kHz) and SpectrumTrail releases a frame only once the previous one has covered its time, so a cluster spreads across the gap to the next. Time lost while starved is not owed back: after a gap, frames resume from the current frame time rather than being dumped at once to catch up. The backlog is capped at 24 frames (~0.5 s) so 2x playback speed cannot leave the picture drifting behind the sound; ordinary clusters peak at ~15 and are never trimmed. The duration defaults to zero, meaning "show on arrival", so producers that do not pace to audio — the synthetic preview emits one frame per display frame — keep their existing behaviour. Same device afterwards: 46.5 frames/s (48 kHz / 1024 = 46.9), median gap 24 ms, p95 26 ms, 2 gaps over 150 ms in 49.3 s, peak backlog 15. Also corrects the PcmTapRegistry comment from the buffering fix, which blamed a single large decoder buffer. The logs show decoder calls mostly carry one fft frame; the burst is many calls landing within a few ms, which the main-thread collector cannot interleave with either way. --- .../playback/playerPool/PcmTapRegistry.kt | 24 ++-- .../playerPool/SpectrumAudioBufferSinkTest.kt | 25 ++++ .../amethyst/commons/audio/AudioSpectrum.kt | 9 +- .../amethyst/commons/audio/SpectrumPacer.kt | 18 ++- .../amethyst/commons/audio/SpectrumTrail.kt | 63 ++++++-- .../commons/audio/SpectrumTrailTest.kt | 134 ++++++++++++++---- .../amethyst/commons/audio/SpectrumCanvas.kt | 25 ++-- 7 files changed, 226 insertions(+), 72 deletions(-) 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 } } From 5b8e6e5b0a55c0c97f9f3b72f7edb2fe7ec918a8 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 27 Sep 2026 08:49:09 +0200 Subject: [PATCH 6/6] refactor(audio): simplify the visualizer pacing and stop it polling while idle Cleanup pass over the visualizer fix: - Fold SpectrumPacer into SpectrumTrail. It had one caller and only wrapped an ArrayDeque; its unused 64-frame default contradicted the real 24-frame cap. Its three tests were already covered by SpectrumTrailTest. - Size the PcmTapRegistry flow buffer from SpectrumTrail.MAX_BACKLOG_FRAMES instead of a separate 63. Both drop the stalest frame, so the visualizer keeps the same newest frames either way; the trail's cap is the one that actually bounds how far the picture can lag the sound. - Move the cluster-delivery test into SpectrumAudioBufferSinkTest on its existing monoPcm/sineShorts helpers instead of a second PCM encoder. Re-checked against the old 2-slot buffer: it still fails with 2 of 8. - Park the pacing loop off the frame clock while nothing is queued. It woke the main thread every vsync (up to 120 Hz) even when paused, which was new work for the non-animated bars and radial styles. It parks only after a nextOrNull has seen the empty queue, so the starvation reset still runs and the next cluster is not dumped at once to catch up. - Keep the cluster-timing explanation in the SpectrumTrail KDoc only. It had been restated in six places and the copies had already drifted (210 vs 220 ms). Also corrects the SpectrumCanvas KDoc's redraw-rate claim, which only held for bars and radial. --- .../playback/playerPool/PcmTapRegistry.kt | 19 ++-- .../playerPool/SpectrumAudioBufferSinkTest.kt | 38 ++++++++ .../playerPool/SpectrumBurstDeliveryTest.kt | 86 ------------------- .../amethyst/commons/audio/SpectrumPacer.kt | 57 ------------ .../amethyst/commons/audio/SpectrumTrail.kt | 42 +++++---- .../commons/audio/SpectrumPacerTest.kt | 67 --------------- .../commons/audio/SpectrumTrailTest.kt | 27 ++++-- .../amethyst/commons/audio/SpectrumCanvas.kt | 31 ++++--- 8 files changed, 105 insertions(+), 262 deletions(-) delete mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumBurstDeliveryTest.kt delete mode 100644 commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacer.kt delete mode 100644 commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacerTest.kt 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 70b32c1a03..2fd9ce4a6d 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 @@ -27,6 +27,7 @@ import androidx.media3.exoplayer.audio.TeeAudioProcessor import com.vitorpamplona.amethyst.commons.audio.AudioWindow import com.vitorpamplona.amethyst.commons.audio.Fft import com.vitorpamplona.amethyst.commons.audio.Spectrum +import com.vitorpamplona.amethyst.commons.audio.SpectrumTrail import com.vitorpamplona.amethyst.commons.audio.normalizeToPeakInPlace import com.vitorpamplona.amethyst.commons.audio.toLogBins import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -118,11 +119,6 @@ class SpectrumAudioBufferSink( object PcmTapRegistry { private const val MAX_TRACKED_FLOWS = 64 - // 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 - private val lock = Any() // access-order LinkedHashMap → eldest (least-recently-used) entries iterate first for eviction. @@ -178,16 +174,13 @@ object PcmTapRegistry { if (!fedByLiveSink && !stillCollected) iter.remove() } } - // 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.) + // Frames arrive in clusters (see SpectrumTrail) emitted within a few ms on the audio + // thread, while the collector sits on the main dispatcher and cannot run in between; + // a 2-slot buffer kept ~2 frames of each cluster and dropped the rest. Hold as much as + // the visualizer's own backlog, which is the real lag bound, dropping the stalest. MutableSharedFlow( replay = 1, - extraBufferCapacity = SPECTRUM_BUFFER_FRAMES, + extraBufferCapacity = SpectrumTrail.MAX_BACKLOG_FRAMES, onBufferOverflow = BufferOverflow.DROP_OLDEST, ) } 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 012ddf6e72..7cf470a967 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 @@ -24,7 +24,11 @@ import androidx.annotation.OptIn import androidx.media3.common.C import androidx.media3.common.util.UnstableApi import com.vitorpamplona.amethyst.commons.audio.Spectrum +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test @@ -33,6 +37,7 @@ import java.nio.ByteOrder import kotlin.math.PI import kotlin.math.sin +@kotlin.OptIn(ExperimentalCoroutinesApi::class) @OptIn(UnstableApi::class) class SpectrumAudioBufferSinkTest { private val fftSize = 64 @@ -161,4 +166,37 @@ class SpectrumAudioBufferSinkTest { assertEquals(fftSize * 1_000_000_000L / 44100, out.replayCache.last().durationNanos) } + + /** + * The audio thread emits a cluster of fft frames within a few ms, with no suspension point in + * between, while the UI collector sits on the main dispatcher and cannot interleave. Anything the + * registry's flow cannot hold at that instant is dropped; a 2-slot buffer kept 2 of every cluster. + */ + private fun deliveredFromOneCluster(framesInCluster: Int): Int { + var count = -1 + runTest { + val mediaId = "test://cluster-$framesInCluster" + val sink = SpectrumAudioBufferSink(fftSize = fftSize, binCount = binCount) + PcmTapRegistry.bind(mediaId, sink) + sink.flush(48000, 1, C.ENCODING_PCM_16BIT) + + val received = mutableListOf() + val collector = launch { PcmTapRegistry.spectrumFor(mediaId).collect { received.add(it) } } + advanceUntilIdle() // let the collector subscribe + + sink.handleBuffer(monoPcm(sineShorts(k = 2, n = fftSize * framesInCluster))) + + advanceUntilIdle() // only now does the UI collector get to run + collector.cancel() + PcmTapRegistry.bind(null, sink) + count = received.size + } + return count + } + + @Test + fun everyFrameOfAClusterReachesTheVisualizer() { + assertEquals("cluster of 8", 8, deliveredFromOneCluster(8)) + assertEquals("cluster of 20", 20, deliveredFromOneCluster(20)) + } } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumBurstDeliveryTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumBurstDeliveryTest.kt deleted file mode 100644 index d9c9a48c63..0000000000 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/SpectrumBurstDeliveryTest.kt +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.service.playback.playerPool - -import androidx.annotation.OptIn -import androidx.media3.common.C -import androidx.media3.common.util.UnstableApi -import com.vitorpamplona.amethyst.commons.audio.Spectrum -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.launch -import kotlinx.coroutines.test.advanceUntilIdle -import kotlinx.coroutines.test.runTest -import org.junit.Assert.assertEquals -import org.junit.Test -import java.nio.ByteBuffer -import java.nio.ByteOrder - -/** - * The audio thread runs [SpectrumAudioBufferSink.handleBuffer] to completion, emitting EVERY fft - * frame of one decoder buffer synchronously with no suspension point in between. The UI collector - * lives on the main dispatcher and cannot interleave with that burst, so anything that does not fit - * in the flow's buffer at that instant is silently dropped by `tryEmit`. - * - * With a 2-slot buffer that capped the visualizer at two frames per decoder buffer no matter how - * much audio it carried, decoupling the update rate from the ~43 Hz the fft actually produces. - */ -@kotlin.OptIn(ExperimentalCoroutinesApi::class) -@OptIn(UnstableApi::class) -class SpectrumBurstDeliveryTest { - private val fftSize = 64 - - private fun monoPcm(totalSamples: Int): ByteBuffer { - val bb = ByteBuffer.allocate(totalSamples * 2).order(ByteOrder.LITTLE_ENDIAN) - for (i in 0 until totalSamples) bb.putShort(((i % 100) * 300).toShort()) - bb.flip() - return bb - } - - /** Frames the UI receives from ONE decoder buffer holding [framesInBurst] fft frames. */ - private fun deliveredFromOneBurst(framesInBurst: Int): Int { - var count = -1 - runTest { - val mediaId = "test://burst-$framesInBurst" - val sink = SpectrumAudioBufferSink(fftSize = fftSize, binCount = 16) - PcmTapRegistry.bind(mediaId, sink) - sink.flush(48000, 1, C.ENCODING_PCM_16BIT) - - val received = mutableListOf() - val collector = launch { PcmTapRegistry.spectrumFor(mediaId).collect { received.add(it) } } - advanceUntilIdle() // let the collector subscribe - - sink.handleBuffer(monoPcm(fftSize * framesInBurst)) - - advanceUntilIdle() // now let the UI collector run - collector.cancel() - PcmTapRegistry.bind(null, sink) - count = received.size - } - return count - } - - @Test - fun everyFrameOfADecoderBurstReachesTheVisualizer() { - assertEquals("burst of 8", 8, deliveredFromOneBurst(8)) - assertEquals("burst of 20", 20, deliveredFromOneBurst(20)) - assertEquals("burst of 50", 50, deliveredFromOneBurst(50)) - } -} 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 deleted file mode 100644 index e6f8cc2e1d..0000000000 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacer.kt +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.commons.audio - -/** - * The bounded queue between the audio tap and the visualizer. - * - * 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. - */ -class SpectrumPacer( - private val capacity: Int = DEFAULT_CAPACITY, -) { - private val queue = ArrayDeque(capacity) - - /** Queues a freshly decoded frame, evicting the stalest if the UI has fallen behind. */ - fun offer(frame: Spectrum) { - // Bound the queue so a stalled UI cannot bank an ever-growing backlog of stale spectrum and - // drift permanently behind the audio. The newest frame is always the one worth keeping. - while (queue.size >= capacity) queue.removeFirst() - 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() - - companion object { - // ~1.5 s of 1024-sample hops at 44.1 kHz: room for any realistic decoder buffer without - // letting a stalled UI accumulate an unbounded backlog. - const val DEFAULT_CAPACITY = 64 - } -} 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 98e40f7770..08f2afd6be 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,16 +21,20 @@ package com.vitorpamplona.amethyst.commons.audio /** - * 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 + * The per-displayed-frame step of the spectrum visualizer: queue incoming frames, release each as + * its audio time comes due, apply the decay trail against what is already on screen, and return a * fresh array to draw. * - * 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. + * Why pacing is needed at all: the audio pipeline fills its output buffer in chunks, so frames reach + * the UI in clusters — ~15 frames roughly every 330 ms, measured on a Pixel 9a. Drawn on arrival a + * cluster collapses into one frame; released one per display refresh it plays out in ~110 ms at + * 120 Hz and then freezes (161 stalls averaging 211 ms over 52.8 s). 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: after a gap, frames resume from the current frame + * time rather than being dumped at once to catch up. + * + * The backlog is capped at [capacity], evicting the stalest frame, so faster-than-real-time playback + * or a stalled UI cannot leave the picture drifting ever further behind the sound. * * 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. @@ -42,17 +46,23 @@ package com.vitorpamplona.amethyst.commons.audio */ class SpectrumTrail( private val decay: Float, - capacity: Int = MAX_BACKLOG_FRAMES, + private val capacity: Int = MAX_BACKLOG_FRAMES, ) { - private val pacer = SpectrumPacer(capacity) + private val queue = ArrayDeque(capacity) private var drawn = FloatArray(0) // 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) + /** Queues a freshly decoded frame, evicting the stalest if the backlog is at [capacity]. */ + fun offer(frame: Spectrum) { + while (queue.size >= capacity) queue.removeFirst() + queue.addLast(frame) + } + + /** True when nothing is queued, so the caller can stop polling until the next [offer]. */ + fun isEmpty(): Boolean = queue.isEmpty() /** * The array to draw at [frameTimeNanos], or null when nothing new is due and the current one @@ -62,7 +72,7 @@ class SpectrumTrail( * instance is what signals Compose to redraw. Do NOT switch to in-place mutation. */ fun nextOrNull(frameTimeNanos: Long): FloatArray? { - if (pacer.isEmpty()) { + if (queue.isEmpty()) { starved = true return null } @@ -74,11 +84,10 @@ class SpectrumTrail( var result: FloatArray? = null while (dueNanos <= frameTimeNanos) { - val frame = pacer.next() ?: break + val frame = queue.removeFirstOrNull() ?: break result = decayedFrom(frame) dueNanos += frame.durationNanos } - return result } @@ -95,8 +104,7 @@ class SpectrumTrail( 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). + // bounding how far the picture can lag the sound (e.g. at 2x playback speed). const val MAX_BACKLOG_FRAMES = 24 } } diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacerTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacerTest.kt deleted file mode 100644 index 5dbaf18950..0000000000 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/audio/SpectrumPacerTest.kt +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.commons.audio - -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertNull - -class SpectrumPacerTest { - private fun frame(id: Float) = Spectrum(floatArrayOf(id)) - - private fun idOf(spectrum: Spectrum?) = spectrum?.bins?.first() - - @Test - fun drainsOneFramePerTickInArrivalOrder() { - val pacer = SpectrumPacer(capacity = 8) - pacer.offer(frame(1f)) - pacer.offer(frame(2f)) - pacer.offer(frame(3f)) - - assertEquals(1f, idOf(pacer.next())) - assertEquals(2f, idOf(pacer.next())) - assertEquals(3f, idOf(pacer.next())) - } - - @Test - fun returnsNullWhenStarvedSoTheCallerCanHoldTheLastFrame() { - val pacer = SpectrumPacer(capacity = 8) - assertNull(pacer.next()) - - pacer.offer(frame(1f)) - assertEquals(1f, idOf(pacer.next())) - assertNull(pacer.next()) - } - - @Test - fun overCapacityDropsTheStalestFrameNotTheNewest() { - val pacer = SpectrumPacer(capacity = 3) - pacer.offer(frame(1f)) - pacer.offer(frame(2f)) - pacer.offer(frame(3f)) - pacer.offer(frame(4f)) // evicts 1 - - assertEquals(2f, idOf(pacer.next())) - assertEquals(3f, idOf(pacer.next())) - assertEquals(4f, idOf(pacer.next())) - assertNull(pacer.next()) - } -} 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 39a73cb261..8a6ffab71a 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 @@ -22,18 +22,12 @@ package com.vitorpamplona.amethyst.commons.audio import kotlin.test.Test import kotlin.test.assertContentEquals +import kotlin.test.assertFalse import kotlin.test.assertNotSame import kotlin.test.assertNull +import kotlin.test.assertTrue -/** - * 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. - */ +/** Pacing, starvation, backlog and decay rules of [SpectrumTrail]; see its KDoc for why each exists. */ class SpectrumTrailTest { private val ms = 1_000_000L @@ -171,4 +165,19 @@ class SpectrumTrailTest { // mutableStateOf compares by reference; reusing one array would never trigger a redraw. assertNotSame(first, second) } + + @Test + fun reportsEmptyOnlyOnceEveryQueuedFrameIsReleasedSoTheCallerKnowsWhenToPark() { + val trail = SpectrumTrail(decay = 0f) + assertTrue(trail.isEmpty()) + + trail.offer(frame(1f)) + trail.offer(frame(2f)) + assertFalse(trail.isEmpty()) + + trail.nextOrNull(0) + assertFalse(trail.isEmpty()) // frame 2 is queued but not yet due + trail.nextOrNull(20 * ms) + assertTrue(trail.isEmpty()) + } } 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 56a2ec66d9..a89eade52f 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 @@ -29,6 +29,7 @@ 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.channels.Channel import kotlinx.coroutines.flow.Flow /** @@ -36,10 +37,9 @@ 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 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. + * Frames are released through [SpectrumTrail] as the audio they describe comes due. The pacing loop + * only writes state when a frame is due, and parks entirely while nothing is queued (paused, idle), + * so on its own it redraws at the ~43-47 Hz the fft produces and costs nothing when silent. * * 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. @@ -55,21 +55,26 @@ fun SpectrumCanvas( ) { val smoothed = remember { mutableStateOf(FloatArray(0)) } - // 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) } + val arrivals = remember(trail) { Channel(Channel.CONFLATED) } LaunchedEffect(spectrum, trail) { - spectrum.collect { trail.offer(it) } + spectrum.collect { + trail.offer(it) + arrivals.trySend(Unit) + } } LaunchedEffect(trail) { while (true) { - 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 + val next = trail.nextOrNull(withFrameNanos { it }) + if (next != null) { + smoothed.value = next + } else if (trail.isEmpty()) { + // Park off the frame clock until the next offer. This must follow a nextOrNull that + // saw the empty queue: that call records the starvation which stops the next cluster + // from being dumped at once to "catch up". + arrivals.receive() + } } }