From 59a56090e8e1696f2c02e6377de1a820f72b139b Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Mon, 28 Mar 2022 19:58:25 -0600 Subject: [PATCH] playback: add positive replaygain values Implement support for positive ReplayGain values. Turns out the blocker for this with the new AudioProcessor was that I did not properly clamp PCM data when I manipulated the data, resulting in target samples like 75491 being truncated to lower values like 9955, which resulted in popping. This is a niche addition, but also puts Auxio in a category that no other (FOSS) android music player currently occupies. Yay. Resolves #115. --- CHANGELOG.md | 3 + README.md | 2 +- .../system/ReplayGainAudioProcessor.kt | 89 ++++++++++++------- .../auxio/settings/SettingsListFragment.kt | 8 +- .../org/oxycblt/auxio/ui/EdgeAppBarLayout.kt | 3 +- .../org/oxycblt/auxio/util/PrimitiveUtil.kt | 7 ++ 6 files changed, 75 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c5844921..6d9eddcae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## dev [v2.2.3, v2.3.0, or v3.0.0] +#### What's New +- Added ReplayGain support for below-reference volume tracks [i.e positive ReplayGain values] + #### What's Fixed - Fixed incorrect ellipsizing on song items diff --git a/README.md b/README.md index 6257a2a02..daa3d0e65 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ I primarily built Auxio for myself, but you can use it too, I guess. - Customizable UI & Behavior - Advanced media indexer that prioritizes correct metadata - Reliable playback state persistence -- ReplayGain support (On MP3, MP4, FLAC, OGG, and OPUS) +- Full ReplayGain support (On MP3, MP4, FLAC, OGG, and OPUS) - Material You (Android 12+ only) - Edge-to-edge - Embedded covers support diff --git a/app/src/main/java/org/oxycblt/auxio/playback/system/ReplayGainAudioProcessor.kt b/app/src/main/java/org/oxycblt/auxio/playback/system/ReplayGainAudioProcessor.kt index 0a9e222de..9a09641f4 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/system/ReplayGainAudioProcessor.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/system/ReplayGainAudioProcessor.kt @@ -17,7 +17,6 @@ package org.oxycblt.auxio.playback.system -import androidx.core.math.MathUtils import com.google.android.exoplayer2.C import com.google.android.exoplayer2.Format import com.google.android.exoplayer2.audio.AudioProcessor @@ -27,10 +26,10 @@ import com.google.android.exoplayer2.metadata.id3.TextInformationFrame import com.google.android.exoplayer2.metadata.vorbis.VorbisComment import java.nio.ByteBuffer import kotlin.math.pow -import okhttp3.internal.and import org.oxycblt.auxio.music.Album import org.oxycblt.auxio.playback.state.PlaybackStateManager import org.oxycblt.auxio.settings.SettingsManager +import org.oxycblt.auxio.util.clamp import org.oxycblt.auxio.util.logD import org.oxycblt.auxio.util.logW import org.oxycblt.auxio.util.unlikelyToBeNull @@ -41,8 +40,6 @@ import org.oxycblt.auxio.util.unlikelyToBeNull * manipulates the bitstream itself to modify the volume, which allows the use of positive * ReplayGain values. * - * TODO: Positive ReplayGain values (implementation is not good enough yet, results in popping) - * * TODO: Pre-amp values * * @author OxygenCobalt @@ -55,6 +52,11 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() { private val settingsManager = SettingsManager.getInstance() private var volume = 1f + set(value) { + field = value + // Processed bytes are no longer valid, flush the stream + flush() + } /// --- REPLAYGAIN PARSING --- @@ -110,9 +112,7 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() { } // Final adjustment along the volume curve. - // Currently, we clamp it to a fixed value as 0f - volume = MathUtils.clamp(10f.pow(adjust / 20f), 0f, 1f) - flush() + volume = 10f.pow(adjust / 20f) } private fun parseReplayGain(metadata: Metadata): Gain? { @@ -170,7 +170,6 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() { albumGain += tag.value / 256f found = true } - return if (found) { Gain(trackGain, albumGain) } else { @@ -212,20 +211,27 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() { val buffer = replaceOutputBuffer(size) if (volume == 1f) { - // Nothing to do, just copy the bytes normally so that we're more efficient. + // Nothing to do, just copy the bytes into the output buffer. for (i in position until limit) { buffer.put(inputBuffer[i]) } } else { - // Note: If an encoding value exceeds the actual data capacity of the encoding, - // it is truncated. This is not ideal, but since many of these formats are bitwise - // (and the jvm cannot into unsigned types), we can't do smarter clamping with them. + // AudioProcessor supplies us with the raw bytes and the encoding. It's our job + // to decode and manipulate it. However, the way we muck the bytes into integer + // types (and vice versa) introduces the possibility for bits to be dropped along + // the way. This is very bad and can result in popping, corrupted audio streams. + // Fix this by clamping the values to the possible range of *signed* values, as + // the PCM data is unsigned and still uses the bit that the JVM interprets as a sign. when (inputAudioFormat.encoding) { C.ENCODING_PCM_8BIT -> { // 8-bit PCM, decode a single byte and multiply it for (i in position until limit) { val sample = inputBuffer.get(i).toInt().and(0xFF) - val targetSample = (sample * volume).toInt().toByte() + val targetSample = + (sample * volume) + .toInt() + .clamp(Byte.MIN_VALUE.toInt(), Byte.MAX_VALUE.toInt()) + .toByte() buffer.put(targetSample) } } @@ -233,7 +239,11 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() { // 16-bit PCM (little endian). for (i in position until limit step 2) { val sample = inputBuffer.getLeShort(i) - val targetSample = (sample * volume).toInt().toShort() + val targetSample = + (sample * volume) + .toInt() + .clamp(Short.MIN_VALUE.toInt(), Short.MAX_VALUE.toInt()) + .toShort() buffer.putLeShort(targetSample) } } @@ -241,33 +251,40 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() { // 16-bit PCM (big endian) for (i in position until limit step 2) { val sample = inputBuffer.getBeShort(i) - val targetSample = (sample * volume).toInt().toShort() + val targetSample = + (sample * volume) + .toInt() + .clamp(Short.MIN_VALUE.toInt(), Short.MAX_VALUE.toInt()) + .toShort() buffer.putBeSort(targetSample) } } C.ENCODING_PCM_24BIT -> { // 24-bit PCM (little endian), decode the data three bytes at a time. + // I don't know if the clamping we do here is valid or not. Since the bit + // values should not cross over into the sign, we should be able to do a + // simple unsigned clamp, but I'm not sure. for (i in position until limit step 3) { val sample = inputBuffer.getLeInt24(i) - val targetSample = (sample * volume).toInt() + val targetSample = (sample * volume).toInt().clamp(0, 0xFF_FF_FF) buffer.putLeInt24(targetSample) } } C.ENCODING_PCM_32BIT -> { - // 32-bit PCM (little endian) + // 32-bit PCM (little endian). for (i in position until limit step 4) { - var sample = inputBuffer.getLeLong32(i) - sample = (sample * volume).toLong() - buffer.putLeLong32(sample) + var sample = inputBuffer.getLeInt32(i) + sample = (sample * volume).toInt().clamp(Int.MIN_VALUE, Int.MAX_VALUE) + buffer.putLeInt32(sample) } } C.ENCODING_PCM_FLOAT -> { // PCM float. Here we can actually clamp values since the value isn't // bitwise. for (i in position until limit step 4) { - var sample = inputBuffer.getFloat(i) - sample = MathUtils.clamp((sample * volume), 0f, 1f) - buffer.putFloat(sample) + val sample = inputBuffer.getFloat(i) + val targetSample = (sample * volume).clamp(0f, 1f) + buffer.putFloat(targetSample) } } C.ENCODING_INVALID, Format.NO_VALUE -> {} @@ -297,7 +314,11 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() { } private fun ByteBuffer.getLeInt24(at: Int): Int { - return get(at + 2).toInt().shl(16).or(get(at + 1).toInt().shl(8)).or(get(at).and(0xFF)) + return get(at + 2) + .toInt() + .shl(16) + .or(get(at + 1).toInt().shl(8)) + .or(get(at).toInt().and(0xFF)) } private fun ByteBuffer.putLeInt24(int: Int) { @@ -306,20 +327,20 @@ class ReplayGainAudioProcessor : BaseAudioProcessor() { put(int.shr(16).toByte()) } - private fun ByteBuffer.getLeLong32(at: Int): Long { + private fun ByteBuffer.getLeInt32(at: Int): Int { return get(at + 3) - .toLong() + .toInt() .shl(24) - .or(get(at + 2).toLong().shl(16)) - .or(get(at + 1).toLong().shl(8)) - .or(get(at).toLong().and(0xFF)) + .or(get(at + 2).toInt().shl(16)) + .or(get(at + 1).toInt().shl(8)) + .or(get(at).toInt().and(0xFF)) } - private fun ByteBuffer.putLeLong32(long: Long) { - put(long.toByte()) - put(long.shr(8).toByte()) - put(long.shr(16).toByte()) - put(long.shr(24).toByte()) + private fun ByteBuffer.putLeInt32(int: Int) { + put(int.toByte()) + put(int.shr(8).toByte()) + put(int.shr(16).toByte()) + put(int.shr(24).toByte()) } companion object { diff --git a/app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt b/app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt index b68b5c030..baf2e2181 100644 --- a/app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt @@ -78,7 +78,13 @@ class SettingsListFragment : PreferenceFragmentCompat() { override fun onDisplayPreferenceDialog(preference: Preference) { if (preference is IntListPreference) { // Creating our own preference dialog is hilariously difficult. For one, we need - // to override this random method within the class in order to + // to override this random method within the class in order to launch the dialog in + // the first (because apparently you can't just implement some interface that + // automatically provides this behavior), then we also need to use a deprecated method + // to adequately supply + // a "target fragment" (otherwise we will crash since the dialog requires one), and then + // we need to actually show the dialog, making sure we use the parent FragmentManager + // as again, it will crash if we don't do it right. Fragments were a mistake. val dialog = IntListPreferenceDialog.from(preference) dialog.setTargetFragment(this, 0) dialog.show(parentFragmentManager, IntListPreferenceDialog.TAG) diff --git a/app/src/main/java/org/oxycblt/auxio/ui/EdgeAppBarLayout.kt b/app/src/main/java/org/oxycblt/auxio/ui/EdgeAppBarLayout.kt index 30decb333..b8640a1fa 100644 --- a/app/src/main/java/org/oxycblt/auxio/ui/EdgeAppBarLayout.kt +++ b/app/src/main/java/org/oxycblt/auxio/ui/EdgeAppBarLayout.kt @@ -70,6 +70,7 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr override fun onDetachedFromWindow() { super.onDetachedFromWindow() viewTreeObserver.removeOnPreDrawListener(onPreDraw) + scrollingChild = null } override fun setLiftOnScrollTargetViewId(liftOnScrollTargetViewId: Int) { @@ -88,7 +89,7 @@ constructor(context: Context, attrs: AttributeSet? = null, @AttrRes defStyleAttr if (liftOnScrollTargetViewId != ResourcesCompat.ID_NULL) { scrollingChild = (parent as ViewGroup).findViewById(liftOnScrollTargetViewId) } else { - logW("liftOnScrollTargetViewId was not specified. ignoring scroll events") + logW("liftOnScrollTargetViewId was not specified, ignoring scroll events") } } diff --git a/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt b/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt index 779280904..8bd7a18a6 100644 --- a/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt +++ b/app/src/main/java/org/oxycblt/auxio/util/PrimitiveUtil.kt @@ -20,6 +20,7 @@ package org.oxycblt.auxio.util import android.database.Cursor import android.database.sqlite.SQLiteDatabase import android.os.Looper +import androidx.core.math.MathUtils import androidx.fragment.app.Fragment import org.oxycblt.auxio.BuildConfig @@ -51,3 +52,9 @@ fun unlikelyToBeNull(value: T?): T { /** Require the fragment is attached to an activity. */ fun Fragment.requireAttached() = check(!isDetached) { "Fragment is detached from activity" } + +fun Int.clamp(min: Int, max: Int): Int = MathUtils.clamp(this, min, max) + +fun Long.clamp(min: Long, max: Long): Long = MathUtils.clamp(this, min, max) + +fun Float.clamp(min: Float, max: Float): Float = MathUtils.clamp(this, min, max)