diff --git a/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt b/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt index 37460c29..abc7b0b5 100644 --- a/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt +++ b/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt @@ -266,10 +266,6 @@ private fun Bundle.putGain( private fun Bundle.gain(key: String): Float? = if (containsKey(key)) getFloat(key) else null -internal fun ReplayGainController.applyItem(mediaItem: MediaItem?) { - setTrack(mediaItem.replayGain()) -} - internal fun queueMediaItem( item: QueueItem, local: String?, diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt index d808a4e8..9c937461 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt @@ -140,7 +140,6 @@ class PlaybackService : MediaSessionService() { reason: Int, ) { observeCurrentSong(mediaItem?.mediaId) - replayGain.applyItem(mediaItem) } }, ) diff --git a/app/src/main/kotlin/com/subtracks/playback/ReplayGainController.kt b/app/src/main/kotlin/com/subtracks/playback/ReplayGainController.kt index 6d254a38..cd14f58c 100644 --- a/app/src/main/kotlin/com/subtracks/playback/ReplayGainController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/ReplayGainController.kt @@ -1,23 +1,23 @@ package com.subtracks.playback +import androidx.media3.common.MediaItem import com.subtracks.data.prefs.ReplayGainMode internal class ReplayGainController( private val apply: (Float) -> Unit, ) { - // Written from the playback thread by ReplayGainProcessor.onFlush and from the app thread by - // the media item transition fallback. - @Volatile private var track = ReplayGain() - - @Volatile private var settings = ReplayGainSettings(ReplayGainMode.Track, 0f) - fun setTrack(value: ReplayGain?) { - track = value ?: ReplayGain() + // Called from the playback thread, by ReplayGainProcessor.onFlush. + @Synchronized + fun applyItem(item: MediaItem) { + track = item.replayGain() applyGain() } + // Called from the app thread, when the preferences change. + @Synchronized fun setSettings(value: ReplayGainSettings) { settings = value applyGain() diff --git a/app/src/main/kotlin/com/subtracks/playback/ReplayGainProcessor.kt b/app/src/main/kotlin/com/subtracks/playback/ReplayGainProcessor.kt index 111a1930..519a1930 100644 --- a/app/src/main/kotlin/com/subtracks/playback/ReplayGainProcessor.kt +++ b/app/src/main/kotlin/com/subtracks/playback/ReplayGainProcessor.kt @@ -13,7 +13,7 @@ import kotlin.math.roundToInt @OptIn(UnstableApi::class) internal class ReplayGainProcessor( - private val onItem: (MediaItem?) -> Unit = {}, + private val onItem: (MediaItem) -> Unit = {}, ) : BaseAudioProcessor() { @Volatile private var linearGain = 1f @@ -30,7 +30,9 @@ internal class ReplayGainProcessor( // Player.Listener.onMediaItemTransition, which the app thread receives afterwards. // See androidx/media#418. override fun onFlush(streamMetadata: StreamMetadata) { - onItem(streamMetadata.currentItem()) + // A flush with no period comes from a sink reset rather than an item boundary, so leave the + // gain alone rather than resetting it to unity. + streamMetadata.currentItem()?.let(onItem) } override fun queueInput(inputBuffer: ByteBuffer) { @@ -48,7 +50,9 @@ internal class ReplayGainProcessor( } else -> { - output.put(inputBuffer) + // An empty input would be the pipeline's own shared buffer, which cannot be put into + // itself. + if (inputBuffer.hasRemaining()) output.put(inputBuffer) } } inputBuffer.position(inputBuffer.limit()) diff --git a/app/src/test/kotlin/com/subtracks/playback/ReplayGainControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/ReplayGainControllerTest.kt index b569c2a3..817bea3b 100644 --- a/app/src/test/kotlin/com/subtracks/playback/ReplayGainControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/ReplayGainControllerTest.kt @@ -12,16 +12,16 @@ class ReplayGainControllerTest { private val controller = ReplayGainController { applied += it } @Test - fun appliesTheGainForTheCurrentTrack() { + fun appliesTheItemsGain() { controller.setSettings(ReplayGainSettings(ReplayGainMode.Track, 0f)) - controller.setTrack(ReplayGain(trackGain = -6.0206f)) + controller.applyItem(item("a", trackGain = -6.0206f)) assertEquals(0.5f, applied.last(), TOLERANCE) } @Test fun recomputesWhenTheSettingsChange() { - controller.setTrack(ReplayGain(trackGain = -6.0206f)) + controller.applyItem(item("a", trackGain = -6.0206f)) controller.setSettings(ReplayGainSettings(ReplayGainMode.Track, 6.0206f)) assertEquals(1f, applied.last(), TOLERANCE) @@ -29,7 +29,7 @@ class ReplayGainControllerTest { @Test fun switchingOffRestoresUnityGain() { - controller.setTrack(ReplayGain(trackGain = -6.0206f)) + controller.applyItem(item("a", trackGain = -6.0206f)) controller.setSettings(ReplayGainSettings(ReplayGainMode.Track, 0f)) assertEquals(0.5f, applied.last(), TOLERANCE) @@ -39,9 +39,12 @@ class ReplayGainControllerTest { } @Test - fun aTrackWithoutGainPlaysAtUnity() { + fun anItemWithoutGainPlaysAtUnity() { controller.setSettings(ReplayGainSettings(ReplayGainMode.Track, 0f)) - controller.setTrack(null) + controller.applyItem(item("a", trackGain = -6.0206f)) + assertEquals(0.5f, applied.last(), TOLERANCE) + + controller.applyItem(item("b", trackGain = null)) assertEquals(1f, applied.last(), TOLERANCE) } @@ -56,25 +59,6 @@ class ReplayGainControllerTest { assertEquals(0.5f, applied.last(), TOLERANCE) } - @Test - fun anItemWithoutGainPlaysAtUnity() { - controller.setSettings(ReplayGainSettings(ReplayGainMode.Track, 0f)) - - controller.applyItem(item("a", trackGain = null)) - - assertEquals(1f, applied.last(), TOLERANCE) - } - - @Test - fun noItemPlaysAtUnity() { - controller.setSettings(ReplayGainSettings(ReplayGainMode.Track, 0f)) - controller.applyItem(item("a", trackGain = 6.0206f)) - - controller.applyItem(null) - - assertEquals(1f, applied.last(), TOLERANCE) - } - private fun item( id: String, trackGain: Float?, diff --git a/app/src/test/kotlin/com/subtracks/playback/ReplayGainProcessorTest.kt b/app/src/test/kotlin/com/subtracks/playback/ReplayGainProcessorTest.kt index e752ed82..f5b761e4 100644 --- a/app/src/test/kotlin/com/subtracks/playback/ReplayGainProcessorTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/ReplayGainProcessorTest.kt @@ -93,7 +93,7 @@ class ReplayGainProcessorTest { @Test fun aFlushReportsTheItemBeingFlushed() { - val reported = mutableListOf() + val reported = mutableListOf() val item = item("a", trackGain = -6.0206f) ReplayGainProcessor { reported += it }.flush(streamMetadata(item)) @@ -102,12 +102,18 @@ class ReplayGainProcessorTest { } @Test - fun aFlushWithoutAPeriodReportsNoItem() { - val reported = mutableListOf() + fun aFlushWithoutAPeriodLeavesTheGainAlone() { + lateinit var processor: ReplayGainProcessor + val controller = ReplayGainController { processor.setLinearGain(it) } + processor = ReplayGainProcessor { controller.applyItem(it) } + controller.setSettings(ReplayGainSettings(ReplayGainMode.Track, 0f)) + processor.configure(AudioFormat(SAMPLE_RATE, CHANNELS, C.ENCODING_PCM_16BIT)) - ReplayGainProcessor { reported += it }.flush(StreamMetadata.DEFAULT) + processor.flush(streamMetadata(item("a", trackGain = -6.0206f))) + processor.flush(StreamMetadata.DEFAULT) + processor.queueInput(sixteenBit(1000)) - assertEquals(listOf(null), reported) + assertEquals(listOf(500), readShorts(processor.getOutput())) } @Test diff --git a/app/src/test/kotlin/com/subtracks/playback/ReplayGainSinkTest.kt b/app/src/test/kotlin/com/subtracks/playback/ReplayGainSinkTest.kt new file mode 100644 index 00000000..469e29d7 --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/playback/ReplayGainSinkTest.kt @@ -0,0 +1,66 @@ +package com.subtracks.playback + +import android.content.Context +import androidx.annotation.OptIn +import androidx.media3.common.C +import androidx.media3.common.Format +import androidx.media3.common.MediaItem +import androidx.media3.common.MimeTypes +import androidx.media3.common.util.UnstableApi +import androidx.media3.exoplayer.audio.AudioSink +import androidx.media3.exoplayer.source.MediaSource.MediaPeriodId +import androidx.media3.exoplayer.source.SinglePeriodTimeline +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 +import java.nio.ByteBuffer +import java.nio.ByteOrder + +// Pins the media3 behaviour the whole feature rests on: DefaultAudioSink hands the processor the +// timeline and the period being configured, on every configuration, which is what lets the gain be +// set before a new item's buffers. See androidx/media#418. +@OptIn(UnstableApi::class) +@RunWith(AndroidJUnit4::class) +class ReplayGainSinkTest { + @Test + fun theSinkHandsTheItemBeingConfiguredToTheProcessor() { + val context = ApplicationProvider.getApplicationContext() + val reported = mutableListOf() + val processor = ReplayGainProcessor { reported += it } + val sink = ReplayGainRenderersFactory(context, processor).buildAudioSink(context, false, false) + + val item = MediaItem.Builder().setMediaId("b").build() + val timeline = SinglePeriodTimeline(DURATION_US, false, false, false, null, item) + sink.configure( + AudioSink.AudioSinkConfig + .Builder(pcmFormat()) + .setTimeline(timeline) + .setMediaPeriodId(MediaPeriodId(timeline.getUidOfPeriod(0))) + .build(), + ) + sink.handleBuffer(sixteenBit(1000), 0, 1) + + assertEquals(listOf(item), reported) + } + + private fun pcmFormat() = + Format + .Builder() + .setSampleMimeType(MimeTypes.AUDIO_RAW) + .setPcmEncoding(C.ENCODING_PCM_16BIT) + .setChannelCount(2) + .setSampleRate(44_100) + .build() + + private fun sixteenBit(vararg samples: Int): ByteBuffer = + ByteBuffer.allocate(samples.size * 2).order(ByteOrder.nativeOrder()).apply { + samples.forEach { putShort(it.toShort()) } + flip() + } + + private companion object { + const val DURATION_US = 1_000_000L + } +}