mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 07:29:14 +01:00
playback: reject non-finite replay gain
Review follow-ups for the apply slice. The parse guard in the base branch keeps non-finite gains and peaks out of the library, but nothing here should depend on that: a value already stored, or written by a future caller, must not be able to silence a track or blast it at full scale. - the gain maths ignores a non-finite peak and falls back to unity when the result is not finite. - the processor falls back to unity for a non-finite gain, so `roundToInt` can never be handed a NaN. - the preamp is sanitised on both read and write, since `coerceIn` passes NaN straight through. - the renderers factory is pulled out of `PlaybackService` into `ReplayGainRenderersFactory` so a test can assert the sink really is built with the processor in its chain; the wiring was otherwise untested. - a null active source clears the gain rather than leaving the previous track's applied.
This commit is contained in:
@@ -163,13 +163,15 @@ class UserPreferences(
|
|||||||
|
|
||||||
private val replayGainModeKey = stringPreferencesKey("replay_gain_mode")
|
private val replayGainModeKey = stringPreferencesKey("replay_gain_mode")
|
||||||
|
|
||||||
fun replayGainPreampDb(): Flow<Float> =
|
fun replayGainPreampDb(): Flow<Float> = store.data.map { prefs -> prefs[replayGainPreampKey].sanitizedPreampDb() }
|
||||||
store.data.map { prefs -> (prefs[replayGainPreampKey] ?: 0f).coerceIn(MIN_REPLAY_GAIN_PREAMP_DB, MAX_REPLAY_GAIN_PREAMP_DB) }
|
|
||||||
|
|
||||||
suspend fun setReplayGainPreampDb(value: Float) {
|
suspend fun setReplayGainPreampDb(value: Float) {
|
||||||
store.edit { prefs -> prefs[replayGainPreampKey] = value.coerceIn(MIN_REPLAY_GAIN_PREAMP_DB, MAX_REPLAY_GAIN_PREAMP_DB) }
|
store.edit { prefs -> prefs[replayGainPreampKey] = value.sanitizedPreampDb() }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun Float?.sanitizedPreampDb(): Float =
|
||||||
|
this?.takeIf { it.isFinite() }?.coerceIn(MIN_REPLAY_GAIN_PREAMP_DB, MAX_REPLAY_GAIN_PREAMP_DB) ?: 0f
|
||||||
|
|
||||||
private val replayGainPreampKey = floatPreferencesKey("replay_gain_preamp_db")
|
private val replayGainPreampKey = floatPreferencesKey("replay_gain_preamp_db")
|
||||||
|
|
||||||
fun offlineMode(): Flow<Boolean> = store.data.map { prefs -> prefs[offlineModeKey] ?: false }
|
fun offlineMode(): Flow<Boolean> = store.data.map { prefs -> prefs[offlineModeKey] ?: false }
|
||||||
|
|||||||
@@ -20,10 +20,7 @@ import androidx.media3.datasource.cache.CacheDataSource
|
|||||||
import androidx.media3.datasource.cache.LeastRecentlyUsedCacheEvictor
|
import androidx.media3.datasource.cache.LeastRecentlyUsedCacheEvictor
|
||||||
import androidx.media3.datasource.cache.SimpleCache
|
import androidx.media3.datasource.cache.SimpleCache
|
||||||
import androidx.media3.datasource.okhttp.OkHttpDataSource
|
import androidx.media3.datasource.okhttp.OkHttpDataSource
|
||||||
import androidx.media3.exoplayer.DefaultRenderersFactory
|
|
||||||
import androidx.media3.exoplayer.ExoPlayer
|
import androidx.media3.exoplayer.ExoPlayer
|
||||||
import androidx.media3.exoplayer.audio.AudioSink
|
|
||||||
import androidx.media3.exoplayer.audio.DefaultAudioSink
|
|
||||||
import androidx.media3.exoplayer.source.DefaultMediaSourceFactory
|
import androidx.media3.exoplayer.source.DefaultMediaSourceFactory
|
||||||
import androidx.media3.session.CommandButton
|
import androidx.media3.session.CommandButton
|
||||||
import androidx.media3.session.MediaSession
|
import androidx.media3.session.MediaSession
|
||||||
@@ -113,24 +110,10 @@ class PlaybackService : MediaSessionService() {
|
|||||||
.combine(preferences.replayGainPreampDb()) { mode, preamp -> ReplayGainSettings(mode, preamp) }
|
.combine(preferences.replayGainPreampDb()) { mode, preamp -> ReplayGainSettings(mode, preamp) }
|
||||||
.collect(replayGain::setSettings)
|
.collect(replayGain::setSettings)
|
||||||
}
|
}
|
||||||
val renderersFactory =
|
|
||||||
object : DefaultRenderersFactory(this) {
|
|
||||||
override fun buildAudioSink(
|
|
||||||
context: Context,
|
|
||||||
enableFloatOutput: Boolean,
|
|
||||||
enableAudioOutputPlaybackParameters: Boolean,
|
|
||||||
): AudioSink =
|
|
||||||
DefaultAudioSink
|
|
||||||
.Builder(context)
|
|
||||||
.setEnableFloatOutput(enableFloatOutput)
|
|
||||||
.setEnableAudioOutputPlaybackParameters(enableAudioOutputPlaybackParameters)
|
|
||||||
.setAudioProcessors(arrayOf(gainProcessor))
|
|
||||||
.build()
|
|
||||||
}
|
|
||||||
val exoPlayer =
|
val exoPlayer =
|
||||||
ExoPlayer
|
ExoPlayer
|
||||||
.Builder(this)
|
.Builder(this)
|
||||||
.setRenderersFactory(renderersFactory)
|
.setRenderersFactory(ReplayGainRenderersFactory(this, gainProcessor))
|
||||||
.setAudioAttributes(
|
.setAudioAttributes(
|
||||||
AudioAttributes
|
AudioAttributes
|
||||||
.Builder()
|
.Builder()
|
||||||
@@ -204,7 +187,11 @@ class PlaybackService : MediaSessionService() {
|
|||||||
session?.setMediaButtonPreferences(listOf(starButton(false)))
|
session?.setMediaButtonPreferences(listOf(starButton(false)))
|
||||||
songJob =
|
songJob =
|
||||||
scope.launch {
|
scope.launch {
|
||||||
val sourceId = sourceRepository?.activeSourceIdOnce() ?: return@launch
|
val sourceId = sourceRepository?.activeSourceIdOnce()
|
||||||
|
if (sourceId == null) {
|
||||||
|
replayGain.setTrack(null)
|
||||||
|
return@launch
|
||||||
|
}
|
||||||
library.song(sourceId, songId).collect { song ->
|
library.song(sourceId, songId).collect { song ->
|
||||||
if (currentSongId != songId) return@collect
|
if (currentSongId != songId) return@collect
|
||||||
starred = song?.starred != null
|
starred = song?.starred != null
|
||||||
|
|||||||
@@ -38,11 +38,11 @@ internal fun replayGainLinearGain(
|
|||||||
}
|
}
|
||||||
val gainDb = requestedDb ?: return 1f
|
val gainDb = requestedDb ?: return 1f
|
||||||
val requested = gainDb + settings.preampDb
|
val requested = gainDb + settings.preampDb
|
||||||
val applied = if (peak != null && peak > 0f) min(requested, maxSafeGainDb(peak)) else requested
|
val cap = peak?.takeIf { it.isFinite() && it > 0f }?.let(::maxSafeGainDb)
|
||||||
return 10f.pow(applied / 20f)
|
val applied = if (cap != null) min(requested, cap) else requested
|
||||||
|
return 10f.pow(applied / 20f).takeIf { it.isFinite() } ?: 1f
|
||||||
}
|
}
|
||||||
|
|
||||||
// A peak of 1.0 is full scale, so anything above it leaves no headroom and this goes negative. A
|
// gonic sends a peak of 0 when it only has album gain, so a non-positive or non-finite peak means
|
||||||
// peak of zero or less carries no peak data (gonic sends 0 when only album gain is known), which
|
// "no peak data" and is ignored rather than treated as silence.
|
||||||
// is why the caller ignores it rather than treating it as silence.
|
|
||||||
private fun maxSafeGainDb(peak: Float): Float = -20f * log10(peak)
|
private fun maxSafeGainDb(peak: Float): Float = -20f * log10(peak)
|
||||||
|
|||||||
@@ -21,7 +21,7 @@ internal class ReplayGainProcessor : BaseAudioProcessor() {
|
|||||||
|
|
||||||
override fun queueInput(inputBuffer: ByteBuffer) {
|
override fun queueInput(inputBuffer: ByteBuffer) {
|
||||||
val output = replaceOutputBuffer(inputBuffer.remaining())
|
val output = replaceOutputBuffer(inputBuffer.remaining())
|
||||||
val gain = linearGain
|
val gain = linearGain.takeIf { it.isFinite() } ?: 1f
|
||||||
when (inputAudioFormat.encoding) {
|
when (inputAudioFormat.encoding) {
|
||||||
C.ENCODING_PCM_16BIT -> {
|
C.ENCODING_PCM_16BIT -> {
|
||||||
while (inputBuffer.remaining() >= 2) {
|
while (inputBuffer.remaining() >= 2) {
|
||||||
|
|||||||
@@ -0,0 +1,29 @@
|
|||||||
|
package com.subtracks.playback
|
||||||
|
|
||||||
|
import android.content.Context
|
||||||
|
import androidx.annotation.OptIn
|
||||||
|
import androidx.media3.common.util.UnstableApi
|
||||||
|
import androidx.media3.exoplayer.DefaultRenderersFactory
|
||||||
|
import androidx.media3.exoplayer.audio.AudioSink
|
||||||
|
import androidx.media3.exoplayer.audio.DefaultAudioSink
|
||||||
|
|
||||||
|
@OptIn(UnstableApi::class)
|
||||||
|
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.
|
||||||
|
public override fun buildAudioSink(
|
||||||
|
context: Context,
|
||||||
|
enableFloatOutput: Boolean,
|
||||||
|
enableAudioOutputPlaybackParameters: Boolean,
|
||||||
|
): AudioSink =
|
||||||
|
DefaultAudioSink
|
||||||
|
.Builder(context)
|
||||||
|
.setEnableFloatOutput(enableFloatOutput)
|
||||||
|
.setEnableAudioOutputPlaybackParameters(enableAudioOutputPlaybackParameters)
|
||||||
|
.setAudioProcessors(arrayOf(processor))
|
||||||
|
.build()
|
||||||
|
}
|
||||||
@@ -213,6 +213,9 @@ class UserPreferencesTest {
|
|||||||
prefs.setReplayGainPreampDb(-99f)
|
prefs.setReplayGainPreampDb(-99f)
|
||||||
assertEquals(MIN_REPLAY_GAIN_PREAMP_DB, prefs.replayGainPreampDb().first(), 0f)
|
assertEquals(MIN_REPLAY_GAIN_PREAMP_DB, prefs.replayGainPreampDb().first(), 0f)
|
||||||
|
|
||||||
|
prefs.setReplayGainPreampDb(Float.NaN)
|
||||||
|
assertEquals(0f, prefs.replayGainPreampDb().first(), 0f)
|
||||||
|
|
||||||
file.delete()
|
file.delete()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -57,6 +57,24 @@ class ReplayGainProcessorTest {
|
|||||||
assertEquals(listOf<Byte>(1, 2, 3), readBytes(processor.getOutput()))
|
assertEquals(listOf<Byte>(1, 2, 3), readBytes(processor.getOutput()))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun passesSamplesThroughWhenTheGainIsNaN() {
|
||||||
|
val processor = processor(gain = Float.NaN)
|
||||||
|
|
||||||
|
processor.queueInput(sixteenBit(1234, -4321))
|
||||||
|
|
||||||
|
assertEquals(listOf(1234, -4321), readShorts(processor.getOutput()))
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun passesSamplesThroughWhenTheGainIsInfinite() {
|
||||||
|
val processor = processor(gain = Float.POSITIVE_INFINITY)
|
||||||
|
|
||||||
|
processor.queueInput(sixteenBit(1234, -4321))
|
||||||
|
|
||||||
|
assertEquals(listOf(1234, -4321), readShorts(processor.getOutput()))
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun keepsTheInputFormatSoTheSinkKeepsItInThePipeline() {
|
fun keepsTheInputFormatSoTheSinkKeepsItInThePipeline() {
|
||||||
val processor = ReplayGainProcessor()
|
val processor = ReplayGainProcessor()
|
||||||
|
|||||||
@@ -0,0 +1,34 @@
|
|||||||
|
package com.subtracks.playback
|
||||||
|
|
||||||
|
import android.content.Context
|
||||||
|
import androidx.annotation.OptIn
|
||||||
|
import androidx.media3.common.audio.AudioProcessorChain
|
||||||
|
import androidx.media3.common.util.UnstableApi
|
||||||
|
import androidx.media3.exoplayer.audio.DefaultAudioSink
|
||||||
|
import androidx.test.core.app.ApplicationProvider
|
||||||
|
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Test
|
||||||
|
import org.junit.runner.RunWith
|
||||||
|
|
||||||
|
@OptIn(UnstableApi::class)
|
||||||
|
@RunWith(AndroidJUnit4::class)
|
||||||
|
class ReplayGainRenderersFactoryTest {
|
||||||
|
@Test
|
||||||
|
fun installsTheGainProcessorInTheAudioSink() {
|
||||||
|
val context = ApplicationProvider.getApplicationContext<Context>()
|
||||||
|
val processor = ReplayGainProcessor()
|
||||||
|
|
||||||
|
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.
|
||||||
|
val chain =
|
||||||
|
DefaultAudioSink::class.java
|
||||||
|
.getDeclaredField("audioProcessorChain")
|
||||||
|
.apply { isAccessible = true }
|
||||||
|
.get(sink) as AudioProcessorChain
|
||||||
|
|
||||||
|
assertEquals(true, chain.audioProcessors.any { it === processor })
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -143,6 +143,26 @@ class ReplayGainTest {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun aNonFinitePeakDoesNotCap() {
|
||||||
|
assertEquals(
|
||||||
|
3.9810717f,
|
||||||
|
replayGainLinearGain(
|
||||||
|
ReplayGain(trackGain = 12f, trackPeak = Float.POSITIVE_INFINITY),
|
||||||
|
ReplayGainSettings(ReplayGainMode.Track, 0f),
|
||||||
|
),
|
||||||
|
TOLERANCE,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun playsAtUnityWhenTheGainIsNotFinite() {
|
||||||
|
val settings = ReplayGainSettings(ReplayGainMode.Track, 0f)
|
||||||
|
|
||||||
|
assertEquals(1f, replayGainLinearGain(ReplayGain(trackGain = Float.POSITIVE_INFINITY), settings), TOLERANCE)
|
||||||
|
assertEquals(1f, replayGainLinearGain(ReplayGain(trackGain = Float.NaN), settings), TOLERANCE)
|
||||||
|
}
|
||||||
|
|
||||||
private companion object {
|
private companion object {
|
||||||
const val TOLERANCE = 0.001f
|
const val TOLERANCE = 0.001f
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user