From 743220d0aae96c438ac558fef928cd0d107eb877 Mon Sep 17 00:00:00 2001
From: Alexander Capehart <alex@oxycblt.org>
Date: Thu, 5 Jan 2023 11:03:30 -0700
Subject: [PATCH] replaygain: revert back to copy no-op

Turns out using isActive to indicate that the AudioProcessor is a no-op
is too unreliable due to how they are managed internally.

Instead, I really do just have to use a copy. Once again ExoPlayer
picks the most absurd possible design choices for no good reason.

Resolves #293.
---
 CHANGELOG.md                                  |  1 +
 .../replaygain/ReplayGainAudioProcessor.kt    | 63 +++++++++----------
 .../org/oxycblt/auxio/playback/state/Queue.kt |  1 -
 3 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 173367fb4..387549954 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - Added ability to edit previously played or currently playing items in the queue
 
 #### What's Fixed
+- Fixed unreliable ReplayGain adjustment application in certain situations
 - Fixed crash that would occur in music folders dialog when user does not have a working
 file manager
 
diff --git a/app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt b/app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt
index 77a633140..5c09579b6 100644
--- a/app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/replaygain/ReplayGainAudioProcessor.kt
@@ -229,49 +229,44 @@ class ReplayGainAudioProcessor(private val context: Context) :
         throw AudioProcessor.UnhandledAudioFormatException(inputAudioFormat)
     }
 
-    override fun isActive() = super.isActive() && volume != 1f
-
     override fun queueInput(inputBuffer: ByteBuffer) {
-        val position = inputBuffer.position()
+        val pos = inputBuffer.position()
         val limit = inputBuffer.limit()
-        val size = limit - position
-        val buffer = replaceOutputBuffer(size)
+        val buffer = replaceOutputBuffer(limit - pos)
 
-        for (i in position until limit step 2) {
-            // Ensure we clamp the values to the minimum and maximum values possible
-            // for the encoding. This prevents issues where samples amplified beyond
-            // 1 << 16 will end up becoming truncated during the conversion to a short,
-            // resulting in popping.
-            var sample = inputBuffer.getLeShort(i)
-            sample =
-                (sample * volume)
-                    .toInt()
-                    .coerceAtLeast(Short.MIN_VALUE.toInt())
-                    .coerceAtMost(Short.MAX_VALUE.toInt())
-                    .toShort()
-            buffer.putLeShort(sample)
+        if (volume == 1f) {
+            // Nothing to adjust, just copy the audio data.
+            // isActive is technically a much better way of doing a no-op like this, but since
+            // the adjustment can change during playback I'm largely forced to do this.
+            buffer.put(inputBuffer.slice())
+        } else {
+            for (i in pos until limit step 2) {
+                // 16-bit PCM audio, deserialize a little-endian short.
+                var sample =
+                    inputBuffer
+                        .get(i + 1)
+                        .toInt()
+                        .shl(8)
+                        .or(inputBuffer.get(i).toInt().and(0xFF))
+                        .toShort()
+                // Ensure we clamp the values to the minimum and maximum values possible
+                // for the encoding. This prevents issues where samples amplified beyond
+                // 1 << 16 will end up becoming truncated during the conversion to a short,
+                // resulting in popping.
+                sample =
+                    (sample * volume)
+                        .toInt()
+                        .coerceAtLeast(Short.MIN_VALUE.toInt())
+                        .coerceAtMost(Short.MAX_VALUE.toInt())
+                        .toShort()
+                buffer.put(sample.toByte()).put(sample.toInt().shr(8).toByte())
+            }
         }
 
         inputBuffer.position(limit)
         buffer.flip()
     }
 
-    /**
-     * Always read a little-endian [Short] from the [ByteBuffer] at the given index.
-     * @param at The index to read the [Short] from.
-     */
-    private fun ByteBuffer.getLeShort(at: Int) =
-        get(at + 1).toInt().shl(8).or(get(at).toInt().and(0xFF)).toShort()
-
-    /**
-     * Always write a little-endian [Short] at the end of the [ByteBuffer].
-     * @param short The [Short] to write.
-     */
-    private fun ByteBuffer.putLeShort(short: Short) {
-        put(short.toByte())
-        put(short.toInt().shr(8).toByte())
-    }
-
     /**
      * The resolved ReplayGain adjustment for a file.
      * @param track The track adjustment (in dB), or 0 if it is not present.
diff --git a/app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt b/app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt
index 333d5e189..025485f53 100644
--- a/app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt
+++ b/app/src/main/java/org/oxycblt/auxio/playback/state/Queue.kt
@@ -170,7 +170,6 @@ class Queue {
      * mutated.
      */
     fun move(src: Int, dst: Int): ChangeResult {
-
         if (shuffledMapping.isNotEmpty()) {
             // Move songs only in the shuffled mapping. There is no sane analogous form of
             // this for the ordered mapping.