diff --git a/TASKS.md b/TASKS.md index 9e8a8fe..b7ff7d8 100644 --- a/TASKS.md +++ b/TASKS.md @@ -358,6 +358,40 @@ Tests land in `tests/test_round17.py`. decode→filter→output pipeline (replaces playback; risky for seek/formats) or leaning on the system (PipeWire filter-chain / EasyEffects). Needs a design decision before building. + **See the backend note below — a GStreamer sink would make this a + drop-in `equalizer-3bands` element instead of a redesign.** + +- [ ] Swap the local sink from Qt Multimedia to GStreamer (`GstSink`). + Assessed 2026-08-15 while fixing the Round 34 audio leak. Not urgent — + the parking brake handles the leak — but the case is real and it should + be the plan whenever the equalizer or gapless comes up, since those are + what make it pay for itself. + + Why: Qt Multimedia's FFmpeg backend has cost us five workarounds now, all + of them in `LocalSink` — the stale-`LoadedMedia` URL tagging, the + `setSource` no-op dance, the resume nudge, the shutdown-ordering segfault + guard, and now the parking brake. GStreamer is what every other Linux + music player uses (Rhythmbox, Lollypop, Amberol), corks properly on + pause, and unlocks three parked/impossible features: a 3-band EQ + (`equalizer-3bands`), true gapless (`playbin3` `about-to-finish`), and + ReplayGain (`rgvolume`). + + Cost: `LocalSink` is ~180 lines and `PlaybackSink` is already a real + seam (proven by `CastSink`), so the sink itself is bounded. The bigger + cost is the tests — 10+ files stub Qt Multimedia *by name* via + `patch.multiple(player_module, QMediaPlayer=…, QAudioOutput=…, + QAudioBufferOutput=…, QMediaDevices=…)`. Also: new system dep + (`python3-gi` + `gstreamer1.0-*` via apt here, `python3-gobject` + + `gstreamer1-plugins-*` via dnf on the Fedora machine — the self-updater + only does `git pull`, so both machines need it installed by hand before + the push lands), GLib bus pumped from a `QTimer` rather than a GLib main + loop, and the visualizer's PCM tee rebuilt on `appsink` instead of + `QAudioBufferOutput`. + + Do it incrementally: write `GstSink` alongside `LocalSink`, put it behind + a preference, run it for a week, delete the Qt one when it's trusted. + Already available here: GStreamer 1.26.2 + Python bindings + libav, + pipewire and good/base plugin sets. - [ ] Smart playlists Phase 2: nested-group editing UI in the criteria dialog (import + evaluation of nested groups already works; imported nested playlists are read-only until then). Big change, on hold. diff --git a/lintunes/__init__.py b/lintunes/__init__.py index ca87981..a8ab46b 100644 --- a/lintunes/__init__.py +++ b/lintunes/__init__.py @@ -1,3 +1,3 @@ """LinTunes — iTunes-style music library manager and player for Linux.""" -__version__ = "0.6.0" +__version__ = "0.6.1" diff --git a/lintunes/player.py b/lintunes/player.py index 7a65a3e..25b880d 100644 --- a/lintunes/player.py +++ b/lintunes/player.py @@ -2,7 +2,7 @@ import random import time from pathlib import Path -from PyQt6.QtCore import QObject, QUrl, pyqtSignal +from PyQt6.QtCore import QObject, QTimer, QUrl, pyqtSignal from PyQt6.QtMultimedia import ( QMediaPlayer, QAudioOutput, QAudioBufferOutput, QMediaDevices, QAudio) @@ -19,6 +19,23 @@ RESTART_THRESHOLD_MS = 3000 # audible until a seek re-primes the sink. Seen on trav's Debian 13 machine. RESUME_NUDGE_THRESHOLD_S = 30 +# The "parking brake". A merely paused/stopped QMediaPlayer keeps its PipeWire +# stream open and RUNNING, still holding the few seconds of audio the FFmpeg +# backend had decoded ahead. Nothing drains or corks it, so when the audio +# graph is later rewired — a USB DAC waking from idle suspend, a device +# appearing — that stale buffer flushes to the speakers: LinTunes "plays by +# itself" for ~4s, mid-song, hours after it was paused, and never records a +# play count. Diagnosed 2026-08-15 (the app sat Paused at 6974ms for 7h while +# PipeWire reported its stream state=running) and reproduced standalone on +# Qt 6.8.2 / FFmpeg 7.1.5. +# +# Qt owns that stream, so the fix has to be ours: once playback has been +# idle this long, tear the pipeline down and rebuild it on resume. Releasing +# the source is verified to remove the PipeWire node outright, so there is no +# buffer left to leak. Players built on GStreamer get this for free by corking +# on pause — if we ever move to a GStreamer sink, all of this goes away. +PARK_AFTER_IDLE_S = 30 + def make_shuffle_order(count: int, start_index: int) -> list[int]: """A random permutation of range(count) that begins at start_index.""" @@ -113,6 +130,17 @@ class LocalSink(PlaybackSink): self._pending_start_url = QUrl() self._paused_at: float | None = None # monotonic time of last pause self._shutdown_done = False + # Parking-brake state (see PARK_AFTER_IDLE_S). While parked there is no + # pipeline at all, so position/duration are served from these caches + # and the media object's own signals are suppressed. + self._parked = False + self._parked_url = QUrl() + self._parked_position_ms = 0 + self._parked_duration_ms = 0 + self._park_timer = QTimer(self) + self._park_timer.setSingleShot(True) + self._park_timer.setInterval(int(PARK_AFTER_IDLE_S * 1000)) + self._park_timer.timeout.connect(self._park) self._audio = QAudioOutput(self) self._media = QMediaPlayer(self) @@ -128,14 +156,19 @@ class LocalSink(PlaybackSink): self._media_devices = QMediaDevices(self) self._media_devices.audioOutputsChanged.connect( self._on_audio_outputs_changed) - self._media.positionChanged.connect(self.position_changed) - self._media.durationChanged.connect(self.duration_changed) + self._media.positionChanged.connect(self._on_position_changed) + self._media.durationChanged.connect(self._on_duration_changed) self._media.playbackStateChanged.connect(self._on_state_changed) self._media.mediaStatusChanged.connect(self._on_media_status) self._media.errorOccurred.connect(self._on_error) def load(self, track, autoplay: bool, start_ms: int): start_ms = max(0, start_ms) + # A new track supersedes anything parked; the source below rebuilds the + # pipeline anyway, and the cached position belongs to the old track. + self._park_timer.stop() + self._parked = False + self._parked_url = QUrl() url = QUrl.fromLocalFile(track.location) if self._media.source() == url: # setSource() no-ops on an unchanged URL, so LoadedMedia never @@ -161,32 +194,49 @@ class LocalSink(PlaybackSink): self._media.play() def play(self): - self._apply_volume() - if (self._paused_at is not None + self._park_timer.stop() + if self._parked: + self._unpark() # rebuilds the pipeline and re-arms the seek + elif (self._paused_at is not None and time.monotonic() - self._paused_at >= RESUME_NUDGE_THRESHOLD_S): # Re-prime a possibly idle-suspended sink (the manual workaround # was "rewind slightly"); seek in place so the listener doesn't - # lose their spot. + # lose their spot. Rarely reached now that a pause this long parks + # instead — a rebuilt pipeline can't be idle-suspended-dead — but + # kept as the fallback for a park that didn't happen. self._media.setPosition(self._media.position()) + self._apply_volume() self._paused_at = None self._media.play() def pause(self): self._paused_at = time.monotonic() self._media.pause() + self._park_timer.start() def stop(self): + # Also parked: Qt's stop() leaves the source loaded, so the stream + # survives here too — this is the path _advance() takes when a playlist + # runs out, which would otherwise sit loaded all night. self._media.stop() + self._park_timer.start() def seek(self, position_ms: int): - self._media.setPosition(max(0, position_ms)) + position_ms = max(0, position_ms) + if self._parked: + # No pipeline to seek; move the resume point instead and keep the + # scrubber honest. + self._parked_position_ms = position_ms + self.position_changed.emit(position_ms) + return + self._media.setPosition(position_ms) def position_ms(self) -> int: - return self._media.position() + return self._parked_position_ms if self._parked else self._media.position() def duration_ms(self) -> int: - return self._media.duration() + return self._parked_duration_ms if self._parked else self._media.duration() def is_playing(self) -> bool: return self._media.playbackState() == QMediaPlayer.PlaybackState.PlayingState @@ -203,13 +253,71 @@ class LocalSink(PlaybackSink): """Push the stored logical volume to the sink. Called again on resume and after device swaps: some backends (seen with Bluetooth sinks) re-create the sink with a stale/zero gain, which played silently until - a seek re-primed it.""" + a seek re-primed it. Forced to silence while parked, so moving the + volume slider can't re-arm a buffer the teardown failed to drop.""" + level = 0.0 if self._parked else self._volume self._audio.setVolume(QAudio.convertVolume( - self._volume, + level, QAudio.VolumeScale.LogarithmicVolumeScale, QAudio.VolumeScale.LinearVolumeScale, )) + # ---- the parking brake ---- + + def _park(self): + """Release the audio pipeline after a long idle (see PARK_AFTER_IDLE_S). + + Two independent layers, because this is guarding someone's sleep: + the gain goes to zero first (so anything already buffered downstream + drains silently even if the teardown misbehaves), then the source is + released, which removes the PipeWire stream outright — verified on + Qt 6.8.2 — leaving nothing that could ever flush to the speakers. + """ + if self._parked or self._shutdown_done or self.is_playing(): + return + if self._media.source().isEmpty(): + return # nothing loaded; nothing to release + self._parked_url = self._media.source() + self._parked_position_ms = self._media.position() + self._parked_duration_ms = self._media.duration() + # Set before teardown so the stop/clear churn below can't reach the UI + # as a jump to 0:00, and so _apply_volume() forces silence. + self._parked = True + self._apply_volume() + self._media.stop() + self._media.setSource(QUrl()) # this is what drops the stream + log_control("player", "park", + f"released the audio pipeline at " + f"{self._parked_position_ms}ms") + + def _unpark(self): + """Rebuild the pipeline parked by _park() and resume where we left off. + + The seek rides the existing custom-start-time machinery: arming + _pending_start_* makes _on_media_status() jump there once the media is + loaded, which is the only point it is reliably seekable. + """ + url = self._parked_url + start = self._parked_position_ms + self._parked = False + self._parked_url = QUrl() + self._apply_volume() # back to the real gain + if url.isEmpty(): + return + self._pending_start_ms = start + self._pending_start_url = url + self._media.setSource(url) + log_control("player", "unpark", + f"rebuilt the audio pipeline at {start}ms") + + def _on_position_changed(self, position): + if not self._parked: + self.position_changed.emit(position) + + def _on_duration_changed(self, duration): + if not self._parked: + self.duration_changed.emit(duration) + def shutdown(self): """Tear down the Qt Multimedia pipeline in a safe order before Qt destroys the objects. Without this, QObject children are destroyed in @@ -219,6 +327,7 @@ class LocalSink(PlaybackSink): if self._shutdown_done: return self._shutdown_done = True + self._park_timer.stop() try: self._media_devices.audioOutputsChanged.disconnect( self._on_audio_outputs_changed) @@ -244,9 +353,13 @@ class LocalSink(PlaybackSink): self._media.play() def _on_state_changed(self, state): + if self._parked: + return # the teardown's own Stopped, not a user-visible change self.state_changed.emit(state == QMediaPlayer.PlaybackState.PlayingState) def _on_media_status(self, status): + if self._parked: + return # statuses from tearing the pipeline down if (status == QMediaPlayer.MediaStatus.LoadedMedia and self._pending_start_ms and self._media.source() == self._pending_start_url): diff --git a/tasks-done.md b/tasks-done.md index cfb58da..594a6cc 100644 --- a/tasks-done.md +++ b/tasks-done.md @@ -1,5 +1,54 @@ ## Done +### Round 34 (2026-08-15) — The parking brake: stop playing audio at 3am (v0.6.1) + +The long-running "LinTunes plays by itself" haunting, finally attributed and +fixed. Five-plus incidents since July, always the same shape: a few seconds of +audio from the middle of a song, at some random hour, hours after the app was +last touched, never recording a play count. + +It was never a control event. The v0.1.4 provenance log had **zero** entries +between the 17:48 pause and the 00:30 incident — no MPRIS, no media key, no +AVRCP ghost — while MPRIS still reported `Paused` at exactly `6974000µs`, the +same position it was paused at seven hours earlier. Meanwhile PipeWire showed +LinTunes owning a stream in state `running`, linked to the DAC, gain 1.0. + +Cause: a paused (or stopped) `QMediaPlayer` on the FFmpeg backend keeps its +PipeWire stream open and never corks or drains it, so the few seconds the +backend had decoded ahead sit there live. When the audio graph is later rewired +— a USB DAC waking from idle suspend, a device appearing — that stale buffer +flushes to the speakers. It explains every observation: always a short burst +(it can only be what was buffered), always mid-song, never a play count (nothing +resumed), and why the Fedora machine never does it (no USB DAC to wake up). +Reproduced standalone on Qt 6.8.2 / FFmpeg 7.1.5 in ~20 lines. + +- [x] `PARK_AFTER_IDLE_S` (30s) + `LocalSink._park()` / `_unpark()`. After that + long idle, release the pipeline: `stop()` then `setSource(QUrl())`, which + is verified to remove the PipeWire node outright — there is no buffer left + that could ever leak. Resume rebuilds the source and rides the existing + custom-start-time machinery to seek back. +- [x] Two independent layers, deliberately: gain goes to 0 *before* the + teardown, so anything still buffered downstream drains silently even if + the release misbehaves. `_apply_volume()` forces silence while parked, so + the volume slider can't re-arm it. +- [x] `stop()` arms the brake too — Qt's `stop()` leaves the source loaded, and + `Player._advance()` calls it when a playlist runs out, so "playlist ended + at 1am" was a second, equally real leak path. +- [x] Parked state serves `position_ms()`/`duration_ms()` from a cache and + suppresses the media object's position/duration/state/status signals, so + the teardown never reaches the UI as a jump to 0:00 and a stray + `EndOfMedia` can't advance the queue. +- [x] `seek()` while parked moves the resume point instead of poking a dead + pipeline; `load()` discards parked state; `shutdown()` disarms the timer. +- [x] Verified end-to-end against the real `LocalSink` (silent WAV, zero gain, + watched via `pw-dump`): stream present while playing, still present right + after pause, **gone** once parked, rebuilt on resume with position and + duration intact. Tests in `tests/test_round34.py` (17). + +Note: this is a workaround for an upstream Qt Multimedia bug, not a LinTunes +design flaw — GStreamer-based players get correct pause behavior for free by +corking. See the backend note under "Parked / deferred". + ### Round 33 (2026-08-14) — Delete songs from the library (v0.6.0) An iTunes 12 habit LinTunes had no answer for: getting rid of a song you don't diff --git a/tests/test_round34.py b/tests/test_round34.py new file mode 100644 index 0000000..642ef99 --- /dev/null +++ b/tests/test_round34.py @@ -0,0 +1,187 @@ +"""Round 34 — the parking brake. + +A paused/stopped QMediaPlayer keeps its PipeWire stream alive, still holding +the audio the FFmpeg backend decoded ahead; a later audio-graph change flushes +that buffer to the speakers hours afterwards (see PARK_AFTER_IDLE_S). These +cover the release, the resume, and the ways the release must NOT fire. +""" +from unittest.mock import MagicMock, patch + +import pytest +from PyQt6.QtCore import QUrl +from PyQt6.QtMultimedia import QMediaPlayer + +from lintunes import player as player_module +from lintunes.player import LocalSink +from lintunes.models import Track + + +def _sink(qapp): + with patch.multiple( + player_module, + QMediaPlayer=MagicMock(), + QAudioOutput=MagicMock(), + QAudioBufferOutput=MagicMock(), + QMediaDevices=MagicMock(), + ): + return LocalSink() + + +def _loaded(qapp, url="file:///music/a.mp3", position=6974, duration=180000): + """A sink whose media object looks like a paused, loaded track.""" + sink = _sink(qapp) + sink._media.source.return_value = QUrl(url) + sink._media.position.return_value = position + sink._media.duration.return_value = duration + sink._media.playbackState.return_value = ( + QMediaPlayer.PlaybackState.PausedState) + return sink + + +class TestParking: + def test_park_releases_the_source(self, qapp): + """Clearing the source is what actually drops the PipeWire stream.""" + sink = _loaded(qapp) + sink._park() + sink._media.stop.assert_called_once() + cleared = sink._media.setSource.call_args[0][0] + assert cleared.isEmpty(), "source must be cleared, not just stopped" + assert sink._parked + + def test_park_silences_first(self, qapp): + """Belt as well as braces: gain hits zero before the teardown, so a + buffer that somehow survives drains inaudibly.""" + sink = _loaded(qapp) + sink.set_volume(0.8) + sink._audio.setVolume.reset_mock() + sink._park() + assert sink._audio.setVolume.call_args_list[0][0][0] == 0.0 + + def test_parked_volume_slider_cannot_re_arm_the_gain(self, qapp): + sink = _loaded(qapp) + sink._park() + sink._audio.setVolume.reset_mock() + sink.set_volume(1.0) + assert sink._audio.setVolume.call_args[0][0] == 0.0 + + def test_park_remembers_where_we_were(self, qapp): + sink = _loaded(qapp, position=6974, duration=180000) + sink._park() + sink._media.position.return_value = 0 # the torn-down pipeline + sink._media.duration.return_value = 0 + assert sink.position_ms() == 6974 + assert sink.duration_ms() == 180000 + + def test_park_is_idempotent_and_skips_an_empty_source(self, qapp): + sink = _sink(qapp) + sink._media.source.return_value = QUrl() + sink._park() + assert not sink._parked + sink._media.setSource.assert_not_called() + + def test_park_never_interrupts_playback(self, qapp): + """A timer that somehow fires while playing must do nothing.""" + sink = _loaded(qapp) + sink._media.playbackState.return_value = ( + QMediaPlayer.PlaybackState.PlayingState) + sink._park() + assert not sink._parked + sink._media.setSource.assert_not_called() + + def test_teardown_does_not_reach_the_ui(self, qapp): + """stop()/setSource() churn must not show up as 0:00 or a state flip.""" + sink = _loaded(qapp) + seen = [] + sink.position_changed.connect(lambda ms: seen.append(("pos", ms))) + sink.duration_changed.connect(lambda ms: seen.append(("dur", ms))) + sink.state_changed.connect(lambda p: seen.append(("state", p))) + sink._park() + sink._on_position_changed(0) + sink._on_duration_changed(0) + sink._on_state_changed(QMediaPlayer.PlaybackState.StoppedState) + assert seen == [] + + def test_ended_is_not_emitted_while_parked(self, qapp): + """A stray EndOfMedia during teardown must not advance the queue.""" + sink = _loaded(qapp) + ended = [] + sink.ended.connect(lambda: ended.append(True)) + sink._park() + sink._on_media_status(QMediaPlayer.MediaStatus.EndOfMedia) + assert ended == [] + + +class TestUnparking: + def test_play_rebuilds_and_re_arms_the_seek(self, qapp): + sink = _loaded(qapp, url="file:///music/a.mp3", position=6974) + sink._park() + sink._media.setSource.reset_mock() + sink.play() + assert not sink._parked + restored = sink._media.setSource.call_args[0][0] + assert restored == QUrl("file:///music/a.mp3") + assert sink._pending_start_ms == 6974 + sink._media.play.assert_called() + + def test_resume_restores_the_real_volume(self, qapp): + sink = _loaded(qapp) + sink.set_volume(0.8) + sink._park() + sink._audio.setVolume.reset_mock() + sink.play() + assert sink._audio.setVolume.call_args[0][0] > 0.0 + + def test_seek_while_parked_moves_the_resume_point(self, qapp): + sink = _loaded(qapp) + sink._park() + seen = [] + sink.position_changed.connect(seen.append) + sink.seek(90000) + sink._media.setPosition.assert_not_called() + assert sink.position_ms() == 90000 + assert seen == [90000] + sink.play() + assert sink._pending_start_ms == 90000 + + def test_loading_a_new_track_discards_parked_state(self, qapp, tmp_path): + sink = _loaded(qapp) + sink._park() + path = tmp_path / "b.mp3" + path.write_bytes(b"x") + sink._media.source.return_value = QUrl() + sink.load(Track(track_id=2, name="B", location=str(path)), + autoplay=False, start_ms=0) + assert not sink._parked + assert sink._parked_url.isEmpty() + + +class TestTimerWiring: + def test_pause_arms_the_timer(self, qapp): + sink = _loaded(qapp) + sink.pause() + assert sink._park_timer.isActive() + + def test_end_of_queue_stop_also_arms_it(self, qapp): + """_advance() calls sink.stop() when a playlist runs out; Qt's stop() + leaves the source loaded, so that path leaks too.""" + sink = _loaded(qapp) + sink.stop() + assert sink._park_timer.isActive() + + def test_play_disarms_the_timer(self, qapp): + sink = _loaded(qapp) + sink.pause() + sink.play() + assert not sink._park_timer.isActive() + + def test_shutdown_disarms_the_timer(self, qapp): + sink = _loaded(qapp) + sink.pause() + sink.shutdown() + assert not sink._park_timer.isActive() + + def test_timer_fires_park(self, qapp): + sink = _loaded(qapp) + assert sink._park_timer.isSingleShot() + sink._park_timer.timeout.emit() + assert sink._parked