From 7ce754f95a58c6ff982a8f39c482d015ea8a77ae Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Thu, 15 Oct 2026 07:19:45 +0000 Subject: [PATCH] playback: apply the item's gain in one tested step Review follow-ups. - `ReplayGainController.applyItem` owns reading the gain off a `MediaItem`, so the transition, the controller and the extras read are exercised together rather than only in isolation. - drop the leftover `setTrack(null)` on a missing active source: now that the transition sets the gain from the item, that line only raced it and could reset a playing item to unity. - tests for a zero gain surviving the extras round trip (zero is a real gain, not an absent one), for the latest item's gain being the one left applied, and for a missing item applying unity. --- .../subtracks/playback/Media3PlayerHandle.kt | 4 ++ .../com/subtracks/playback/PlaybackService.kt | 8 +-- .../com/subtracks/playback/MediaItemTest.kt | 13 +++++ .../playback/ReplayGainControllerTest.kt | 49 +++++++++++++++++++ 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt b/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt index abc7b0b5..37460c29 100644 --- a/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt +++ b/app/src/main/kotlin/com/subtracks/playback/Media3PlayerHandle.kt @@ -266,6 +266,10 @@ 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 12fcf237..66f02cb0 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackService.kt @@ -140,7 +140,7 @@ class PlaybackService : MediaSessionService() { reason: Int, ) { observeCurrentSong(mediaItem?.mediaId) - replayGain.setTrack(mediaItem?.replayGain()) + replayGain.applyItem(mediaItem) } }, ) @@ -187,11 +187,7 @@ class PlaybackService : MediaSessionService() { session?.setMediaButtonPreferences(listOf(starButton(false))) songJob = scope.launch { - val sourceId = sourceRepository?.activeSourceIdOnce() - if (sourceId == null) { - replayGain.setTrack(null) - return@launch - } + val sourceId = sourceRepository?.activeSourceIdOnce() ?: return@launch library.song(sourceId, songId).collect { song -> if (currentSongId != songId) return@collect starred = song?.starred != null diff --git a/app/src/test/kotlin/com/subtracks/playback/MediaItemTest.kt b/app/src/test/kotlin/com/subtracks/playback/MediaItemTest.kt index 731bf948..93cff3c2 100644 --- a/app/src/test/kotlin/com/subtracks/playback/MediaItemTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/MediaItemTest.kt @@ -76,4 +76,17 @@ class MediaItemTest { assertEquals(ReplayGain(), item.replayGain()) } + + @Test + fun aZeroGainIsNotMistakenForNoGain() { + val item = + queueMediaItem( + song.copy(replayGain = ReplayGain(trackGain = 0f, trackPeak = 0.5f)), + local = null, + stream = "http://server/stream?s=s1", + transcode = false, + ) + + assertEquals(ReplayGain(trackGain = 0f, trackPeak = 0.5f), item.replayGain()) + } } diff --git a/app/src/test/kotlin/com/subtracks/playback/ReplayGainControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/ReplayGainControllerTest.kt index aee366b8..b569c2a3 100644 --- a/app/src/test/kotlin/com/subtracks/playback/ReplayGainControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/ReplayGainControllerTest.kt @@ -1,9 +1,12 @@ package com.subtracks.playback +import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.prefs.ReplayGainMode import org.junit.Assert.assertEquals import org.junit.Test +import org.junit.runner.RunWith +@RunWith(AndroidJUnit4::class) class ReplayGainControllerTest { private val applied = mutableListOf() private val controller = ReplayGainController { applied += it } @@ -43,6 +46,52 @@ class ReplayGainControllerTest { assertEquals(1f, applied.last(), TOLERANCE) } + @Test + fun theLatestItemLeavesItsGainApplied() { + controller.setSettings(ReplayGainSettings(ReplayGainMode.Track, 0f)) + + controller.applyItem(item("a", trackGain = 6.0206f)) + controller.applyItem(item("b", trackGain = -6.0206f)) + + 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?, + ) = queueMediaItem( + QueueItem( + id = id, + title = id, + artist = null, + album = null, + coverArtId = null, + replayGain = ReplayGain(trackGain = trackGain), + ), + local = null, + stream = "http://server/$id", + transcode = false, + ) + private companion object { const val TOLERANCE = 0.001f }