Make nightcore and vaporwave rate-independent

.github/workflows/tests.yml

@@ -22,11 +22,21 @@ jobs:
           python-version: ${{ matrix.python-version }}
           cache: pip
 
+      - name: Install FFmpeg
+        # tests/test_effect_filters.py renders real audio to verify the filter
+        # presets. Without FFmpeg those tests skip silently, which would let a
+        # broken filter through CI.
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y --no-install-recommends ffmpeg
+          ffmpeg -version | head -1
+
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
           pip install -r requirements-dev.txt
 
       - name: Run tests
         # The suite needs no credentials, no network and no .env.
-        run: pytest
+        # -ra surfaces anything that skipped, so silent skips stay visible.
+        run: pytest -ra

cogs/effects.py

@@ -17,8 +17,12 @@
 EFFECTS = {
     "bass":       "equalizer=f=54:width_type=o:width=2:g=5",     # gentle low-end lift
     "bassboost":  "equalizer=f=54:width_type=o:width=2:g=10",    # heavy low-end lift
-    "nightcore":  "asetrate=48000*1.25,aresample=48000",         # +pitch, +speed
-    "vaporwave":  "asetrate=48000*0.8,aresample=48000",          # -pitch, -speed
+    # The leading `aresample=48000` is load-bearing: `asetrate` *reinterprets* a
+    # stream's declared rate instead of scaling it, so without normalising first
+    # the speed factor becomes 48000*N/<source rate> and differs per track. A
+    # 44.1 kHz upload ran at 1.36x and a 22 kHz one at 2.72x, not 1.25x.
+    "nightcore":  "aresample=48000,asetrate=48000*1.25,aresample=48000",  # +pitch, +speed
+    "vaporwave":  "aresample=48000,asetrate=48000*0.8,aresample=48000",   # -pitch, -speed
     "treble":     "equalizer=f=8000:width_type=o:width=2:g=5",   # high-end lift
     "echo":       "aecho=0.8:0.88:60:0.4",                       # short echo
     "karaoke":    "pan=stereo|c0=c0-c1|c1=c1-c0",                # cancel centre vocals

tests/test_effect_filters.py

@@ -0,0 +1,94 @@
+"""
+The FFmpeg filter presets, measured against real FFmpeg.
+
+The pitch/speed effects are the ones worth measuring: ``asetrate``
+*reinterprets* a stream's declared sample rate rather than scaling it, so a
+hardcoded constant silently produces a different speed for every source rate.
+Rendering a tone and measuring the output is the only way to catch that - the
+filter string looks perfectly reasonable either way.
+
+Skipped when FFmpeg is not installed.
+"""
+
+import shutil
+import subprocess
+
+import pytest
+
+from cogs.effects import EFFECTS
+
+pytestmark = pytest.mark.skipif(
+    shutil.which("ffmpeg") is None, reason="FFmpeg is not installed"
+)
+
+OUTPUT_RATE = 48000          # what Discord consumes
+SOURCE_SECONDS = 2.0
+# Rates real sources actually use: 48 kHz (YouTube Opus), 44.1 kHz (CD-derived
+# uploads, much of SoundCloud), and lower ones on older or spoken-word uploads.
+SOURCE_RATES = [48000, 44100, 32000, 22050]
+
+
+def rendered_seconds(audio_filter: str, source_rate: int) -> float:
+    """
+    Push a sine through ``audio_filter`` and return how long the result plays.
+
+    Output is raw 16-bit mono at 48 kHz, so duration is just a byte count.
+    """
+    result = subprocess.run(
+        [
+            "ffmpeg", "-hide_banner", "-loglevel", "error",
+            "-f", "lavfi",
+            "-i", f"sine=frequency=440:sample_rate={source_rate}:duration={SOURCE_SECONDS}",
+            "-af", audio_filter,
+            "-f", "s16le", "-ac", "1", "-ar", str(OUTPUT_RATE), "-",
+        ],
+        capture_output=True, check=True,
+    )
+    return len(result.stdout) / (2 * OUTPUT_RATE)
+
+
+def speed_factor(audio_filter: str, source_rate: int) -> float:
+    """How much faster the filtered audio plays than the source."""
+    return SOURCE_SECONDS / rendered_seconds(audio_filter, source_rate)
+
+
+# -- the bug ------------------------------------------------------------
+
+@pytest.mark.parametrize("source_rate", SOURCE_RATES)
+def test_nightcore_speed_does_not_depend_on_the_source_rate(source_rate):
+    assert speed_factor(EFFECTS["nightcore"], source_rate) == pytest.approx(1.25, rel=0.02)
+
+
+@pytest.mark.parametrize("source_rate", SOURCE_RATES)
+def test_vaporwave_speed_does_not_depend_on_the_source_rate(source_rate):
+    assert speed_factor(EFFECTS["vaporwave"], source_rate) == pytest.approx(0.8, rel=0.02)
+
+
+def test_the_hardcoded_form_really_was_rate_dependent():
+    """
+    Documents why the fix exists. The previous filter reinterpreted every source
+    as 48 kHz, so a 22 kHz upload played at 2.7x instead of 1.25x.
+    """
+    previous = "asetrate=48000*1.25,aresample=48000"
+    assert speed_factor(previous, 48000) == pytest.approx(1.25, rel=0.02)
+    assert speed_factor(previous, 44100) == pytest.approx(1.36, rel=0.02)
+    assert speed_factor(previous, 22050) == pytest.approx(2.72, rel=0.02)
+
+
+# -- every preset must be valid FFmpeg ----------------------------------
+
+@pytest.mark.parametrize("name", sorted(EFFECTS))
+def test_every_preset_is_accepted_by_ffmpeg(name):
+    """A typo in a filter string would only surface as silence at playback."""
+    rendered_seconds(EFFECTS[name], 48000)      # check=True raises on rejection
+
+
+@pytest.mark.parametrize("name", sorted(set(EFFECTS) - {"nightcore", "vaporwave"}))
+def test_non_speed_effects_leave_the_duration_alone(name):
+    """Only the two pitch effects are meant to change how long a track runs."""
+    assert speed_factor(EFFECTS[name], 44100) == pytest.approx(1.0, rel=0.05)
+
+
+@pytest.mark.parametrize("name", sorted(EFFECTS))
+def test_every_preset_produces_audio(name):
+    assert rendered_seconds(EFFECTS[name], 44100) > 0