mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-10 15:11:58 +01:00
playback: apply the gain only from the item flush
Review follow-ups for the apply slice. - Drop the media item transition application. It never supplied a gain the flush had not already supplied, and as a second writer it raced the playback thread: it applied the incoming item's gain while the outgoing item's buffers were still queued, scaling the previous track's tail by the next track's gain (reproduced at +12 dB). The flush is the only writer now. This reverses the earlier decision to keep it as a fallback. That was made on the premise that it was insurance against a flush that could not be resolved. The flush covers every transition, and a period-less flush only happens while the sink is being reset, so the premise does not hold. - Ignore a flush that carries no period rather than reading it as "this item has no gain". Resetting to unity there would have been wrong if a bare flush ever arrived mid-playback; the test that pinned that behaviour now pins the opposite. - Guard the pass-through branch against an empty input, which is the pipeline's own shared buffer and cannot be put into itself. - The controller is written from the playback thread now, so its state updates and the recompute that follows them are synchronized rather than two independent volatile writes. - Add ReplayGainSinkTest, which drives the real DefaultAudioSink and asserts it hands the processor the item being configured. That is the media3 behaviour this whole feature rests on, and it was previously asserted only by comments.
This commit is contained in:
@@ -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?,
|
||||
|
||||
@@ -140,7 +140,6 @@ class PlaybackService : MediaSessionService() {
|
||||
reason: Int,
|
||||
) {
|
||||
observeCurrentSong(mediaItem?.mediaId)
|
||||
replayGain.applyItem(mediaItem)
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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?,
|
||||
|
||||
@@ -93,7 +93,7 @@ class ReplayGainProcessorTest {
|
||||
|
||||
@Test
|
||||
fun aFlushReportsTheItemBeingFlushed() {
|
||||
val reported = mutableListOf<MediaItem?>()
|
||||
val reported = mutableListOf<MediaItem>()
|
||||
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<MediaItem?>()
|
||||
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
|
||||
|
||||
@@ -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<Context>()
|
||||
val reported = mutableListOf<MediaItem>()
|
||||
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
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user