playback: reject non-finite gains before the pow, and filter peaks first

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.
This commit is contained in:
austinried
2026-10-16 13:07:08 +00:00
parent aee631c08d
commit 8f34c958fb
6 changed files with 44 additions and 20 deletions
@@ -60,7 +60,7 @@ class PlaybackService : MediaSessionService() {
private var library: LibraryRepository? = null private var library: LibraryRepository? = null
private var currentSongId: String? = null private var currentSongId: String? = null
private var starred = false private var starred = false
private var songJob: Job? = null private var starJob: Job? = null
private val gainProcessor: ReplayGainProcessor = ReplayGainProcessor { replayGain.applyItem(it) } private val gainProcessor: ReplayGainProcessor = ReplayGainProcessor { replayGain.applyItem(it) }
private val replayGain: ReplayGainController = ReplayGainController { gainProcessor.setLinearGain(it) } private val replayGain: ReplayGainController = ReplayGainController { gainProcessor.setLinearGain(it) }
@@ -139,7 +139,7 @@ class PlaybackService : MediaSessionService() {
mediaItem: MediaItem?, mediaItem: MediaItem?,
reason: Int, reason: Int,
) { ) {
observeCurrentSong(mediaItem?.mediaId) observeStar(mediaItem?.mediaId)
} }
}, },
) )
@@ -174,8 +174,8 @@ class PlaybackService : MediaSessionService() {
pauseAllPlayersAndStopSelf() pauseAllPlayersAndStopSelf()
} }
private fun observeCurrentSong(songId: String?) { private fun observeStar(songId: String?) {
songJob?.cancel() starJob?.cancel()
currentSongId = songId currentSongId = songId
starred = false starred = false
val library = library ?: return val library = library ?: return
@@ -184,7 +184,7 @@ class PlaybackService : MediaSessionService() {
return return
} }
session?.setMediaButtonPreferences(listOf(starButton(false))) session?.setMediaButtonPreferences(listOf(starButton(false)))
songJob = starJob =
scope.launch { scope.launch {
val sourceId = sourceRepository?.activeSourceIdOnce() ?: return@launch val sourceId = sourceRepository?.activeSourceIdOnce() ?: return@launch
library.song(sourceId, songId).collect { song -> library.song(sourceId, songId).collect { song ->
@@ -30,19 +30,20 @@ internal fun replayGainLinearGain(
gain: ReplayGain, gain: ReplayGain,
settings: ReplayGainSettings, settings: ReplayGainSettings,
): Float { ): 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) = val (requestedDb, peak) =
when (settings.mode) { when (settings.mode) {
ReplayGainMode.Off -> return 1f ReplayGainMode.Off -> return 1f
ReplayGainMode.Track -> (gain.trackGain ?: gain.albumGain) to (gain.trackPeak ?: gain.albumPeak) ReplayGainMode.Track -> (gain.trackGain ?: gain.albumGain) to (trackPeak ?: albumPeak)
ReplayGainMode.Album -> (gain.albumGain ?: gain.trackGain) to (gain.albumPeak ?: gain.trackPeak) 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 requested = gainDb + settings.preampDb
val cap = peak?.takeIf { it.isFinite() && it > 0f }?.let(::maxSafeGainDb) val applied = peak?.let { min(requested, maxSafeGainDb(it)) } ?: requested
val applied = if (cap != null) min(requested, cap) else requested return 10f.pow(applied / 20f).takeIf { it.isFinite() && it > 0f } ?: 1f
return 10f.pow(applied / 20f).takeIf { it.isFinite() } ?: 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) private fun maxSafeGainDb(peak: Float): Float = -20f * log10(peak)
@@ -28,8 +28,7 @@ internal class ReplayGainProcessor(
// item transition even when the format is unchanged, and hands over the timeline and the period // 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 // 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 // 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 // thread receives afterwards.
// than an item boundary, hence the ignore.
override fun onFlush(streamMetadata: StreamMetadata) { override fun onFlush(streamMetadata: StreamMetadata) {
streamMetadata.currentItem()?.let(onItem) streamMetadata.currentItem()?.let(onItem)
} }
@@ -12,9 +12,8 @@ internal class ReplayGainRenderersFactory(
context: Context, context: Context,
private val processor: ReplayGainProcessor, private val processor: ReplayGainProcessor,
) : DefaultRenderersFactory(context) { ) : DefaultRenderersFactory(context) {
// Mirrors the stock DefaultRenderersFactory.buildAudioSink, plus the processor. setAudioProcessors // Mirrors the stock DefaultRenderersFactory.buildAudioSink. setAudioProcessors keeps the default
// keeps the default chain and prepends ours, but DefaultAudioSink drops the whole chain when // chain and prepends ours; DefaultAudioSink omits the whole chain when float output is enabled.
// float output is enabled, so enabling that would silently disable replay gain.
public override fun buildAudioSink( public override fun buildAudioSink(
context: Context, context: Context,
enableFloatOutput: Boolean, enableFloatOutput: Boolean,
@@ -21,8 +21,7 @@ class ReplayGainRenderersFactoryTest {
val sink = ReplayGainRenderersFactory(context, processor).buildAudioSink(context, false, false) 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 // DefaultAudioSink exposes no accessor for the chain it was built with.
// way to assert the processor is actually installed rather than merely constructed.
val chain = val chain =
DefaultAudioSink::class.java DefaultAudioSink::class.java
.getDeclaredField("audioProcessorChain") .getDeclaredField("audioProcessorChain")
@@ -160,7 +160,33 @@ class ReplayGainTest {
val settings = ReplayGainSettings(ReplayGainMode.Track, 0f) val settings = ReplayGainSettings(ReplayGainMode.Track, 0f)
assertEquals(1f, replayGainLinearGain(ReplayGain(trackGain = Float.POSITIVE_INFINITY), settings), TOLERANCE) 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.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 { private companion object {