From 8f34c958fb68557b51ce80a214b53abd674ac053 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Fri, 16 Oct 2026 09:12:20 +0000 Subject: [PATCH] playback: reject non-finite gains before the pow, and filter peaks first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups for the apply slice. - A negative-infinite gain produced 0 dB, not unity: 10^(inf/20) is 0.0, which is finite, so the result guard never fired and the track played silent — the opposite of what the guard is for. A large-enough finite negative gain underflows the same way, and an infinite gain paired with a peak survived as the cap. The gain is now rejected before the pow and an underflowed result falls back to unity. - A peak carrying no data shadowed a usable peak in the other slot, so trackPeak=0 with albumPeak=0.5 skipped the cap entirely — the gonic shape on the default mode. Both peaks are filtered before one is chosen. - Drop the claim that a period-less flush comes from a sink reset; it does not, and the guard stands on its own. - Trim two comments back to naming the Media3 types. - Revert the observeStar rename, which was for a path that no longer applies the gain. --- .../com/subtracks/playback/PlaybackService.kt | 10 +++---- .../com/subtracks/playback/ReplayGain.kt | 17 ++++++------ .../subtracks/playback/ReplayGainProcessor.kt | 3 +-- .../playback/ReplayGainRenderersFactory.kt | 5 ++-- .../ReplayGainRenderersFactoryTest.kt | 3 +-- .../com/subtracks/playback/ReplayGainTest.kt | 26 +++++++++++++++++++ 6 files changed, 44 insertions(+), 20 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt index 9c937461..028b2229 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt @@ -60,7 +60,7 @@ class PlaybackService : MediaSessionService() { private var library: LibraryRepository? = null private var currentSongId: String? = null private var starred = false - private var songJob: Job? = null + private var starJob: Job? = null private val gainProcessor: ReplayGainProcessor = ReplayGainProcessor { replayGain.applyItem(it) } private val replayGain: ReplayGainController = ReplayGainController { gainProcessor.setLinearGain(it) } @@ -139,7 +139,7 @@ class PlaybackService : MediaSessionService() { mediaItem: MediaItem?, reason: Int, ) { - observeCurrentSong(mediaItem?.mediaId) + observeStar(mediaItem?.mediaId) } }, ) @@ -174,8 +174,8 @@ class PlaybackService : MediaSessionService() { pauseAllPlayersAndStopSelf() } - private fun observeCurrentSong(songId: String?) { - songJob?.cancel() + private fun observeStar(songId: String?) { + starJob?.cancel() currentSongId = songId starred = false val library = library ?: return @@ -184,7 +184,7 @@ class PlaybackService : MediaSessionService() { return } session?.setMediaButtonPreferences(listOf(starButton(false))) - songJob = + starJob = scope.launch { val sourceId = sourceRepository?.activeSourceIdOnce() ?: return@launch library.song(sourceId, songId).collect { song -> diff --git a/app/src/main/kotlin/com/subtracks/playback/ReplayGain.kt b/app/src/main/kotlin/com/subtracks/playback/ReplayGain.kt index cbb6d780..f3157e5b 100644 --- a/app/src/main/kotlin/com/subtracks/playback/ReplayGain.kt +++ b/app/src/main/kotlin/com/subtracks/playback/ReplayGain.kt @@ -30,19 +30,20 @@ internal fun replayGainLinearGain( gain: ReplayGain, settings: ReplayGainSettings, ): Float { + // gonic sends a peak of 0 when it only has album gain, so a non-positive or non-finite peak means + // "no peak data" and is ignored rather than treated as silence. + val trackPeak = gain.trackPeak?.takeIf { it.isFinite() && it > 0f } + val albumPeak = gain.albumPeak?.takeIf { it.isFinite() && it > 0f } val (requestedDb, peak) = when (settings.mode) { ReplayGainMode.Off -> return 1f - ReplayGainMode.Track -> (gain.trackGain ?: gain.albumGain) to (gain.trackPeak ?: gain.albumPeak) - ReplayGainMode.Album -> (gain.albumGain ?: gain.trackGain) to (gain.albumPeak ?: gain.trackPeak) + ReplayGainMode.Track -> (gain.trackGain ?: gain.albumGain) to (trackPeak ?: albumPeak) + ReplayGainMode.Album -> (gain.albumGain ?: gain.trackGain) to (albumPeak ?: trackPeak) } - val gainDb = requestedDb ?: return 1f + val gainDb = requestedDb?.takeIf { it.isFinite() } ?: return 1f val requested = gainDb + settings.preampDb - val cap = peak?.takeIf { it.isFinite() && it > 0f }?.let(::maxSafeGainDb) - val applied = if (cap != null) min(requested, cap) else requested - return 10f.pow(applied / 20f).takeIf { it.isFinite() } ?: 1f + val applied = peak?.let { min(requested, maxSafeGainDb(it)) } ?: requested + return 10f.pow(applied / 20f).takeIf { it.isFinite() && it > 0f } ?: 1f } -// gonic sends a peak of 0 when it only has album gain, so a non-positive or non-finite peak means -// "no peak data" and is ignored rather than treated as silence. private fun maxSafeGainDb(peak: Float): Float = -20f * log10(peak) diff --git a/app/src/main/kotlin/com/subtracks/playback/ReplayGainProcessor.kt b/app/src/main/kotlin/com/subtracks/playback/ReplayGainProcessor.kt index 414643c9..4b39ed6a 100644 --- a/app/src/main/kotlin/com/subtracks/playback/ReplayGainProcessor.kt +++ b/app/src/main/kotlin/com/subtracks/playback/ReplayGainProcessor.kt @@ -28,8 +28,7 @@ internal class ReplayGainProcessor( // item transition even when the format is unchanged, and hands over the timeline and the period // being flushed (androidx/media#418). That is early enough to set the new item's gain before any // of its buffers are processed, unlike Player.Listener.onMediaItemTransition, which the app - // thread receives afterwards. A flush with no period comes from a DefaultAudioSink reset rather - // than an item boundary, hence the ignore. + // thread receives afterwards. override fun onFlush(streamMetadata: StreamMetadata) { streamMetadata.currentItem()?.let(onItem) } diff --git a/app/src/main/kotlin/com/subtracks/playback/ReplayGainRenderersFactory.kt b/app/src/main/kotlin/com/subtracks/playback/ReplayGainRenderersFactory.kt index 628a9486..6e251b42 100644 --- a/app/src/main/kotlin/com/subtracks/playback/ReplayGainRenderersFactory.kt +++ b/app/src/main/kotlin/com/subtracks/playback/ReplayGainRenderersFactory.kt @@ -12,9 +12,8 @@ internal class ReplayGainRenderersFactory( context: Context, private val processor: ReplayGainProcessor, ) : DefaultRenderersFactory(context) { - // Mirrors the stock DefaultRenderersFactory.buildAudioSink, plus the processor. setAudioProcessors - // keeps the default chain and prepends ours, but DefaultAudioSink drops the whole chain when - // float output is enabled, so enabling that would silently disable replay gain. + // Mirrors the stock DefaultRenderersFactory.buildAudioSink. setAudioProcessors keeps the default + // chain and prepends ours; DefaultAudioSink omits the whole chain when float output is enabled. public override fun buildAudioSink( context: Context, enableFloatOutput: Boolean, diff --git a/app/src/test/kotlin/com/subtracks/playback/ReplayGainRenderersFactoryTest.kt b/app/src/test/kotlin/com/subtracks/playback/ReplayGainRenderersFactoryTest.kt index 36287b96..d736c414 100644 --- a/app/src/test/kotlin/com/subtracks/playback/ReplayGainRenderersFactoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/ReplayGainRenderersFactoryTest.kt @@ -21,8 +21,7 @@ class ReplayGainRenderersFactoryTest { val sink = ReplayGainRenderersFactory(context, processor).buildAudioSink(context, false, false) - // DefaultAudioSink exposes no accessor for the chain it was built with, so this is the only - // way to assert the processor is actually installed rather than merely constructed. + // DefaultAudioSink exposes no accessor for the chain it was built with. val chain = DefaultAudioSink::class.java .getDeclaredField("audioProcessorChain") diff --git a/app/src/test/kotlin/com/subtracks/playback/ReplayGainTest.kt b/app/src/test/kotlin/com/subtracks/playback/ReplayGainTest.kt index 8617f0af..4f5fb900 100644 --- a/app/src/test/kotlin/com/subtracks/playback/ReplayGainTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/ReplayGainTest.kt @@ -160,7 +160,33 @@ class ReplayGainTest { val settings = ReplayGainSettings(ReplayGainMode.Track, 0f) assertEquals(1f, replayGainLinearGain(ReplayGain(trackGain = Float.POSITIVE_INFINITY), settings), TOLERANCE) + assertEquals(1f, replayGainLinearGain(ReplayGain(trackGain = Float.NEGATIVE_INFINITY), settings), TOLERANCE) assertEquals(1f, replayGainLinearGain(ReplayGain(trackGain = Float.NaN), settings), TOLERANCE) + assertEquals(1f, replayGainLinearGain(ReplayGain(trackGain = -Float.MAX_VALUE), settings), TOLERANCE) + } + + @Test + fun playsAtUnityWhenANonFiniteGainWouldOtherwiseBeCapped() { + assertEquals( + 1f, + replayGainLinearGain( + ReplayGain(trackGain = Float.POSITIVE_INFINITY, trackPeak = 0.001f), + ReplayGainSettings(ReplayGainMode.Track, 0f), + ), + TOLERANCE, + ) + } + + @Test + fun thePeakFallsBackWhenThePreferredOneCarriesNoData() { + assertEquals( + 2f, + replayGainLinearGain( + ReplayGain(trackGain = 12f, trackPeak = 0f, albumPeak = 0.5f), + ReplayGainSettings(ReplayGainMode.Track, 0f), + ), + TOLERANCE, + ) } private companion object {