From 5b8e6e5b0a55c0c97f9f3b72f7edb2fe7ec918a8 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sun, 27 Sep 2026 08:49:09 +0200 Subject: [PATCH] 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() + } } }