Resume in place when an audio effect changes

cogs/effects.py

@@ -12,8 +12,9 @@
 _EFFECT_COOLDOWN = 3.0
 
 
-# FFmpeg audio-filter presets. Each restarts the current track through
-# ``-af <filter>`` (from the beginning).
+# FFmpeg audio-filter presets. Applying one respawns the stream through
+# ``-af <filter>``, resuming at the current playback position — see
+# ``MusicPlayer.apply_effect``.
 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

cogs/music.py

@@ -124,9 +124,10 @@ def _added_embed(track: dict) -> discord.Embed:
     @same_voice_channel()
     async def pause(self, ctx):
         """Pause the current track."""
-        vc = ctx.voice_client
-        if vc and vc.is_playing():
-            vc.pause()
+        # Routed through the player so it can stop its playback clock; that
+        # clock is what lets an effect change resume in the right place.
+        player = players.get(ctx.guild.id)
+        if player and player.pause():
             await ctx.send(embed=success_embed("Paused ⏸"))
         else:
             await ctx.send(embed=error_embed("Nothing is playing right now."))
@@ -135,9 +136,8 @@ async def pause(self, ctx):
     @same_voice_channel()
     async def resume(self, ctx):
         """Resume a paused track."""
-        vc = ctx.voice_client
-        if vc and vc.is_paused():
-            vc.resume()
+        player = players.get(ctx.guild.id)
+        if player and player.resume():
             await ctx.send(embed=success_embed("Resumed ▶️"))
         else:
             await ctx.send(embed=error_embed("Nothing is paused."))

services/media.py

@@ -331,9 +331,21 @@ def spawn_stream(track: dict) -> AudioStream:
     return AudioStream.launch(cmd)
 
 
-def make_pipe_source(stdin, *, volume: float = 0.5, ffmpeg_filter: str = ""):
-    """Build a ``discord.PCMVolumeTransformer`` that reads audio from a pipe."""
+def make_pipe_source(stdin, *, volume: float = 0.5, ffmpeg_filter: str = "",
+                     seek_seconds: float = 0.0):
+    """
+    Build a ``discord.PCMVolumeTransformer`` that reads audio from a pipe.
+
+    ``seek_seconds`` starts playback partway in, which is what lets an effect
+    change resume where the listener was. It is passed as an *input* option so
+    FFmpeg discards packets without decoding them; on a pipe that is a
+    read-and-discard rather than a real seek, but it costs almost nothing
+    because yt-dlp delivers at network speed rather than in realtime.
+    """
     import discord
     options = f"-vn -af {ffmpeg_filter}" if ffmpeg_filter else "-vn"
-    source = discord.FFmpegPCMAudio(stdin, pipe=True, options=options)
+    before = f"-ss {seek_seconds:.3f}" if seek_seconds > 0 else None
+    source = discord.FFmpegPCMAudio(
+        stdin, pipe=True, before_options=before, options=options,
+    )
     return discord.PCMVolumeTransformer(source, volume=volume)

tests/test_effect_filters.py

@@ -16,6 +16,7 @@
 import pytest
 
 from cogs.effects import EFFECTS
+from services import media
 
 pytestmark = pytest.mark.skipif(
     shutil.which("ffmpeg") is None, reason="FFmpeg is not installed"
@@ -92,3 +93,54 @@ def test_non_speed_effects_leave_the_duration_alone(name):
 @pytest.mark.parametrize("name", sorted(EFFECTS))
 def test_every_preset_produces_audio(name):
     assert rendered_seconds(EFFECTS[name], 44100) > 0
+
+
+# -- resuming in place, through the real audio path ---------------------
+
+def played_seconds(source) -> float:
+    """Drain a discord AudioSource and return how much audio it yielded."""
+    frames = 0
+    while source.read():
+        frames += 1
+    source.cleanup()
+    return frames * 0.02        # discord consumes 20 ms frames
+
+
+@pytest.fixture
+def tone_file(tmp_path):
+    """A 30-second MP3 on disk, to feed the pipe source from a real file."""
+    path = tmp_path / "tone.mp3"
+    subprocess.run(
+        [
+            "ffmpeg", "-hide_banner", "-loglevel", "error", "-y",
+            "-f", "lavfi",
+            "-i", "sine=frequency=440:sample_rate=44100:duration=30",
+            str(path),
+        ],
+        check=True,
+    )
+    return path
+
+
+@pytest.mark.parametrize("seek,expected_remaining", [
+    (0, 30.0),
+    (10, 20.0),
+    (25, 5.0),
+])
+def test_seeking_starts_playback_partway_in(tone_file, seek, expected_remaining):
+    """
+    The whole point of #9: an effect change respawns FFmpeg, and the new
+    process has to pick up where the listener was rather than at 0:00.
+    """
+    with open(tone_file, "rb") as handle:
+        source = media.make_pipe_source(handle, seek_seconds=seek)
+        assert played_seconds(source) == pytest.approx(expected_remaining, abs=0.3)
+
+
+def test_seeking_and_an_effect_apply_together(tone_file):
+    """A resumed track keeps the effect that triggered the respawn."""
+    with open(tone_file, "rb") as handle:
+        source = media.make_pipe_source(
+            handle, ffmpeg_filter=EFFECTS["bassboost"], seek_seconds=20,
+        )
+        assert played_seconds(source) == pytest.approx(10.0, abs=0.3)

tests/test_player_seek.py

@@ -0,0 +1,245 @@
+"""
+Playback position tracking, and resuming in place across an effect change.
+
+An FFmpeg filter chain is fixed for the life of the process, so changing an
+effect means respawning the stream. These cover the clock that makes the
+respawn land where the listener actually was, rather than back at 0:00.
+
+Time is driven by a fake clock so the tests are exact rather than timing-
+dependent.
+"""
+
+from unittest.mock import MagicMock
+
+import pytest
+
+import discord
+from services import media
+from utils import player as player_module
+from utils.player import SEEK_TAIL_MARGIN
+from tests.conftest import make_track
+
+
+class FakeClock:
+    def __init__(self, now: float = 1000.0):
+        self.now = now
+
+    def __call__(self) -> float:
+        return self.now
+
+    def advance(self, seconds: float) -> None:
+        self.now += seconds
+
+
+@pytest.fixture
+def clock(monkeypatch):
+    c = FakeClock()
+    monkeypatch.setattr(player_module.time, "monotonic", c)
+    return c
+
+
+@pytest.fixture
+def playing(player, fake_guild, clock):
+    """A player mid-track, three minutes into a five-minute song."""
+    vc = MagicMock()
+    vc.is_playing.return_value = True
+    vc.is_paused.return_value = False
+    fake_guild.voice_client = vc
+
+    player.current = make_track("Song", duration=300)
+    player._start_ts = clock.now
+    clock.advance(180)
+    return player
+
+
+# -- elapsed -----------------------------------------------------------
+
+def test_elapsed_is_zero_before_anything_plays(player, clock):
+    assert player.elapsed == 0.0
+
+
+def test_elapsed_tracks_wall_time(playing, clock):
+    assert playing.elapsed == pytest.approx(180)
+    clock.advance(45)
+    assert playing.elapsed == pytest.approx(225)
+
+
+def test_elapsed_excludes_time_spent_paused(playing, clock):
+    """Otherwise a track paused for a coffee break resumes far too late."""
+    playing.pause()
+    clock.advance(600)                    # ten minutes paused
+    playing.voice.is_playing.return_value = False
+    playing.voice.is_paused.return_value = True
+    playing.resume()
+
+    assert playing.elapsed == pytest.approx(180), "paused time must not count"
+
+    clock.advance(20)
+    assert playing.elapsed == pytest.approx(200)
+
+
+def test_elapsed_is_frozen_while_still_paused(playing, clock):
+    playing.pause()
+    clock.advance(60)
+    assert playing.elapsed == pytest.approx(180)
+    clock.advance(60)
+    assert playing.elapsed == pytest.approx(180)
+
+
+def test_repeated_pauses_accumulate(playing, clock):
+    for _ in range(3):
+        playing.voice.is_playing.return_value = True
+        playing.voice.is_paused.return_value = False
+        playing.pause()
+        clock.advance(10)
+        playing.voice.is_playing.return_value = False
+        playing.voice.is_paused.return_value = True
+        playing.resume()
+        clock.advance(5)
+
+    # 180 before, then 3 x 5s of actual playback; the 3 x 10s paused is excluded.
+    assert playing.elapsed == pytest.approx(195)
+
+
+# -- pause / resume guards ---------------------------------------------
+
+def test_pause_refuses_when_nothing_is_playing(player, fake_guild):
+    fake_guild.voice_client = None
+    assert player.pause() is False
+
+
+def test_resume_refuses_when_nothing_is_paused(playing):
+    assert playing.resume() is False, "already playing"
+
+
+def test_double_pause_does_not_lose_the_clock(playing, clock):
+    playing.pause()
+    clock.advance(30)
+    playing.pause()                       # a second !pause while paused
+    clock.advance(30)
+    playing.voice.is_playing.return_value = False
+    playing.voice.is_paused.return_value = True
+    playing.resume()
+    assert playing.elapsed == pytest.approx(180)
+
+
+# -- where a respawn should pick up ------------------------------------
+
+def test_seek_target_is_the_current_position(playing):
+    assert playing._seek_target() == pytest.approx(180)
+
+
+def test_live_streams_are_never_seeked(playing):
+    """A live stream reports no duration and has no position to seek to."""
+    playing.current["duration"] = None
+    assert playing._seek_target() == 0.0
+
+
+def test_a_track_with_unknown_duration_is_never_seeked(playing):
+    del playing.current["duration"]
+    assert playing._seek_target() == 0.0
+
+
+def test_the_last_seconds_of_a_track_are_not_seeked_into(playing, clock):
+    """Resuming past the end would produce silence, not audio."""
+    clock.advance(300 - 180 - SEEK_TAIL_MARGIN)      # right at the margin
+    assert playing._seek_target() == 0.0
+
+
+def test_a_position_past_the_end_is_not_seeked_to(playing, clock):
+    clock.advance(500)
+    assert playing._seek_target() == 0.0
+
+
+def test_the_very_start_is_not_seeked_to(player, fake_guild, clock):
+    """Seeking to 0 is a no-op that only costs a slower start."""
+    vc = MagicMock()
+    vc.is_playing.return_value = True
+    fake_guild.voice_client = vc
+    player.current = make_track("Song", duration=300)
+    player._start_ts = clock.now
+    assert player._seek_target() == 0.0
+
+
+# -- apply_effect ------------------------------------------------------
+
+def test_applying_an_effect_captures_the_position(playing):
+    assert playing.apply_effect("bassboost", "equalizer=f=54:g=10") is True
+    assert playing._resume_at == pytest.approx(180)
+    assert playing._replay is True
+    assert playing.effect_name == "bassboost"
+    playing.voice.stop.assert_called_once()
+
+
+def test_applying_an_effect_to_a_live_stream_does_not_seek(playing):
+    playing.current["duration"] = None
+    assert playing.apply_effect("bass", "equalizer=f=54:g=5") is True
+    assert playing._resume_at == 0.0, "a live stream must restart, not seek"
+
+
+def test_clearing_an_effect_also_resumes_in_place(playing):
+    assert playing.apply_effect(None, "") is True
+    assert playing._resume_at == pytest.approx(180)
+    assert playing.effect_filter == ""
+
+
+def test_apply_effect_refuses_when_nothing_is_playing(player, fake_guild):
+    fake_guild.voice_client = None
+    assert player.apply_effect("bass", "x") is False
+
+
+def test_apply_effect_works_while_paused(playing, clock):
+    playing.pause()
+    playing.voice.is_playing.return_value = False
+    playing.voice.is_paused.return_value = True
+    clock.advance(90)
+    assert playing.apply_effect("echo", "aecho=0.8:0.88:60:0.4") is True
+    assert playing._resume_at == pytest.approx(180), "paused time must not shift the resume point"
+
+
+# -- the seek reaching FFmpeg ------------------------------------------
+
+class FakeFFmpeg(discord.AudioSource):
+    """Records how discord.FFmpegPCMAudio was configured."""
+    last = None
+
+    def __init__(self, source, **kwargs):
+        FakeFFmpeg.last = kwargs
+
+    def read(self):
+        return b""
+
+
+@pytest.fixture
+def captured_ffmpeg(monkeypatch):
+    FakeFFmpeg.last = None
+    monkeypatch.setattr(discord, "FFmpegPCMAudio", FakeFFmpeg)
+    return FakeFFmpeg
+
+
+def test_a_seek_becomes_an_ffmpeg_input_option(captured_ffmpeg):
+    """
+    -ss must be an *input* option. As an output option FFmpeg decodes and
+    discards every frame instead of skipping packets.
+    """
+    media.make_pipe_source(MagicMock(), seek_seconds=182.5)
+    assert captured_ffmpeg.last["before_options"] == "-ss 182.500"
+
+
+@pytest.mark.parametrize("seek", [0, 0.0, -1])
+def test_no_seek_means_no_input_option(captured_ffmpeg, seek):
+    media.make_pipe_source(MagicMock(), seek_seconds=seek)
+    assert captured_ffmpeg.last["before_options"] is None
+
+
+def test_the_filter_and_the_seek_coexist(captured_ffmpeg):
+    media.make_pipe_source(
+        MagicMock(), ffmpeg_filter="equalizer=f=54:g=10", seek_seconds=60,
+    )
+    assert captured_ffmpeg.last["before_options"] == "-ss 60.000"
+    assert captured_ffmpeg.last["options"] == "-vn -af equalizer=f=54:g=10"
+
+
+def test_no_filter_still_strips_video(captured_ffmpeg):
+    media.make_pipe_source(MagicMock())
+    assert captured_ffmpeg.last["options"] == "-vn"