diff --git a/lintunes/__init__.py b/lintunes/__init__.py index 421e480..6f7440a 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.5.1" +__version__ = "0.5.2" diff --git a/lintunes/player.py b/lintunes/player.py index c32f9f5..d1cb199 100644 --- a/lintunes/player.py +++ b/lintunes/player.py @@ -106,8 +106,11 @@ class LocalSink(PlaybackSink): super().__init__(parent) self._volume = 1.0 # logical 0..1; what the volume slider shows # Custom start time (ms) for the loaded track: applied once the media - # reports itself loaded, which is when it is reliably seekable. + # reports itself loaded, which is when it is reliably seekable. Tagged + # with the URL it was armed for, because setSource() fires a LoadedMedia + # for the *outgoing* media that would otherwise consume it (see load()). self._pending_start_ms = 0 + self._pending_start_url = QUrl() self._paused_at: float | None = None # monotonic time of last pause self._shutdown_done = False @@ -132,7 +135,7 @@ class LocalSink(PlaybackSink): self._media.errorOccurred.connect(self._on_error) def load(self, track, autoplay: bool, start_ms: int): - self._pending_start_ms = max(0, start_ms) + start_ms = max(0, start_ms) url = QUrl.fromLocalFile(track.location) if self._media.source() == url: # setSource() no-ops on an unchanged URL, so LoadedMedia never @@ -140,10 +143,19 @@ class LocalSink(PlaybackSink): # a clear+reload races the FFmpeg backend, which snaps the seek # back to 0). The media is already loaded: rewind and seek now. self._media.stop() - if self._pending_start_ms: - self._media.setPosition(self._pending_start_ms) - self._pending_start_ms = 0 + self._pending_start_ms = 0 + self._pending_start_url = QUrl() + if start_ms: + self._media.setPosition(start_ms) else: + # setSource() synchronously emits a *stale* LoadedMedia that still + # reports the outgoing media (the previous track dropping back from + # BufferedMedia) before it starts loading the new one. Tagging the + # armed seek with its URL keeps that event from eating it — the + # real LoadedMedia lands a few ms later, and until Round 32 the + # start time was silently lost on every track change. + self._pending_start_ms = start_ms + self._pending_start_url = url self._media.setSource(url) if autoplay: self._media.play() @@ -236,11 +248,15 @@ class LocalSink(PlaybackSink): def _on_media_status(self, status): if (status == QMediaPlayer.MediaStatus.LoadedMedia - and self._pending_start_ms): + and self._pending_start_ms + and self._media.source() == self._pending_start_url): # The media is only reliably seekable once loaded; jump to the # custom start time now (works whether or not we're autoplaying). + # The source check rejects the stale LoadedMedia that setSource() + # fires for the media being replaced. start = self._pending_start_ms self._pending_start_ms = 0 + self._pending_start_url = QUrl() self._media.setPosition(start) return if status == QMediaPlayer.MediaStatus.BufferedMedia: diff --git a/tasks-done.md b/tasks-done.md index c3322a1..3f21413 100644 --- a/tasks-done.md +++ b/tasks-done.md @@ -1,5 +1,32 @@ ## Done +### Round 32 (2026-08-14) — Custom start times survive a track change (v0.5.2) + +trav reported "Pretty Girls is a Motherfucker" starting at 0:00 despite its +imported 0:40 start time. Import and storage were fine (70 tracks carry a start +time, 72 a stop time); the seek was being thrown away. `QMediaPlayer.setSource()` +synchronously emits *two* status changes before it returns — a `LoadedMedia` +still reporting the **outgoing** source, then `LoadingMedia` for the new one — +and the stale first event consumed the one-shot `_pending_start_ms` armed by +Round 22, leaving nothing for the new track's real `LoadedMedia` ~5 ms later. +So it misfired on every track change; only the first track after launch worked, +which is why Round 22 looked green. + +- [x] The armed seek is tagged with the URL it belongs to + (`_pending_start_url`), and `_on_media_status` consumes it only when + `source()` matches — a status change fired for other media can't eat it. +- [x] `LocalSink.load` restructured so the pending seek is armed only on the + new-source branch; Round 22's same-source replay path still seeks directly + and arms nothing. +- [x] `tests/test_round32.py` models the real Qt event sequence (a `setSource` + side effect that fires the stale `LoadedMedia` before switching `source()`), + which Round 22's plain `MagicMock` never did. All three behavioral tests fail + on the old code. `test_round8` / `test_round22` stubs updated to report the + new source by the time its `LoadedMedia` arrives, as real Qt does. +- [x] Verified against the real files headless: switching from a playing track + to the Psychic Vagina mp3 with `start_ms=40000` now lands at 40.1 s and + climbs; a track with no start time still starts at 0. + ### Round 31 (2026-08-13) — One cast control, not two (v0.5.1) The cast glyph appeared both under the volume slider and inside the visualizer, diff --git a/tests/test_round22.py b/tests/test_round22.py index b44a3ca..4d68d8b 100644 --- a/tests/test_round22.py +++ b/tests/test_round22.py @@ -94,6 +94,10 @@ class TestSameSourceReplay: == [(QUrl.fromLocalFile(t2.location),)] # New source: the seek waits for LoadedMedia as before. assert player._local._pending_start_ms == 30_000 + # Qt reports the new source by the time its LoadedMedia arrives; since + # Round 32 the armed seek is only consumed when the two match (see + # tests/test_round32.py for the stale-event sequence that motivated it). + player._local._media.source.return_value = QUrl.fromLocalFile(t2.location) player._local._on_media_status(player_module.QMediaPlayer.MediaStatus.LoadedMedia) player._local._media.setPosition.assert_called_with(30_000) assert player._local._pending_start_ms == 0 diff --git a/tests/test_round32.py b/tests/test_round32.py new file mode 100644 index 0000000..90c6929 --- /dev/null +++ b/tests/test_round32.py @@ -0,0 +1,168 @@ +"""Round 32: custom start times survive a track change. + +Round 22 armed the start-time seek and consumed it on the next LoadedMedia. +But QMediaPlayer.setSource() synchronously emits *two* status changes before it +returns: a LoadedMedia that still reports the outgoing source (the previous +track dropping back from BufferedMedia), then LoadingMedia for the new one. The +stale first event consumed the armed seek, so the new track's real LoadedMedia +found nothing armed and playback started at 0:00 — for every track change, i.e. +everything except the first track after launch. + +The armed seek is now tagged with the URL it belongs to, so only the matching +source's LoadedMedia can consume it. These tests model the real Qt event +sequence, which Round 22's plain MagicMock never emitted. +""" + +from unittest.mock import MagicMock, patch + +from PyQt6.QtCore import QUrl + +from lintunes.models import Library, Track +from lintunes import player as player_module +from lintunes.player import Player + +def _track(tid, **kw): + return Track(track_id=tid, name=kw.pop("name", f"Track {tid}"), **kw) + + +def _player(qapp, tracks): + library = Library() + for t in tracks: + library.tracks[t.track_id] = t + manager = MagicMock() + manager.library = library + with patch.multiple( + player_module, + QMediaPlayer=MagicMock(), + QAudioOutput=MagicMock(), + QAudioBufferOutput=MagicMock(), + QMediaDevices=MagicMock(), + ): + player = Player(manager) + return player, manager + + +def _status(name): + """The patched QMediaPlayer's enum members are stable MagicMock identities, + which is all `_on_media_status` compares against.""" + return getattr(player_module.QMediaPlayer.MediaStatus, name) + + +def _wire_real_qt_sequence(sink): + """Make setSource() behave like the FFmpeg backend really does: fire a + stale LoadedMedia for the media being replaced, *then* switch the reported + source and fire LoadingMedia — all synchronously, inside the call.""" + def side_effect(url): + if sink._media.source.return_value not in (None, QUrl()): + sink._on_media_status(_status("LoadedMedia")) # outgoing media + sink._media.source.return_value = url + sink._on_media_status(_status("LoadingMedia")) + sink._media.source.return_value = QUrl() + sink._media.setSource.side_effect = side_effect + + +class TestStartTimeAcrossTrackChange: + def test_stale_loadedmedia_does_not_eat_the_armed_start(self, qapp, tmp_path): + for name in ("a.mp3", "b.mp3"): + (tmp_path / name).write_bytes(b"x") + t1 = _track(1, location=str(tmp_path / "a.mp3"), total_time=200_000) + t2 = _track(2, location=str(tmp_path / "b.mp3"), + total_time=202_240, start_time=40_000) + player, _ = _player(qapp, [t1, t2]) + sink = player._local + _wire_real_qt_sequence(sink) + + player.play_queue([1, 2], 0) # a.mp3 becomes the loaded source + sink._on_media_status(_status("LoadedMedia")) + sink._media.setPosition.reset_mock() + + player.next() # switch to the start-timed track + # The stale LoadedMedia fired inside setSource() must not have consumed + # the armed seek, nor issued one against the outgoing media. + sink._media.setPosition.assert_not_called() + assert sink._pending_start_ms == 40_000 + + sink._on_media_status(_status("LoadedMedia")) # the real one + sink._media.setPosition.assert_called_once_with(40_000) + assert sink._pending_start_ms == 0 + + def test_stale_loadedmedia_alone_leaves_the_seek_armed(self, qapp, tmp_path): + (tmp_path / "b.mp3").write_bytes(b"x") + track = _track(1, location=str(tmp_path / "b.mp3"), + total_time=200_000, start_time=30_000) + player, _ = _player(qapp, [track]) + sink = player._local + sink._media.source.return_value = QUrl.fromLocalFile("/somewhere/old.mp3") + + player.play_queue([1], 0) + assert sink._pending_start_ms == 30_000 + # source() still reports the outgoing file: this status change is not ours. + sink._on_media_status(_status("LoadedMedia")) + sink._media.setPosition.assert_not_called() + assert sink._pending_start_ms == 30_000 + + def test_end_of_track_advance_lands_on_the_start_time(self, qapp, tmp_path): + for name in ("a.mp3", "b.mp3"): + (tmp_path / name).write_bytes(b"x") + t1 = _track(1, location=str(tmp_path / "a.mp3"), total_time=200_000) + t2 = _track(2, location=str(tmp_path / "b.mp3"), + total_time=200_000, start_time=40_000) + player, _ = _player(qapp, [t1, t2]) + sink = player._local + _wire_real_qt_sequence(sink) + + player.play_queue([1, 2], 0) + sink._on_media_status(_status("LoadedMedia")) + sink._media.setPosition.reset_mock() + + player._on_sink_ended() # t1 finished naturally + # Nothing may be seeked while the new source is still loading — a seek + # issued here lands on the outgoing pipeline and is thrown away. + sink._media.setPosition.assert_not_called() + sink._on_media_status(_status("LoadedMedia")) + sink._media.setPosition.assert_called_once_with(40_000) + + def test_track_without_start_time_never_seeks(self, qapp, tmp_path): + for name in ("a.mp3", "b.mp3"): + (tmp_path / name).write_bytes(b"x") + t1 = _track(1, location=str(tmp_path / "a.mp3"), total_time=200_000) + t2 = _track(2, location=str(tmp_path / "b.mp3"), total_time=200_000) + player, _ = _player(qapp, [t1, t2]) + sink = player._local + _wire_real_qt_sequence(sink) + + player.play_queue([1, 2], 0) + player.next() + sink._on_media_status(_status("LoadedMedia")) + sink._media.setPosition.assert_not_called() + + +class TestSameSourceReplayStillWorks: + """Round 22's path, re-checked against the restructured load().""" + + def test_unchanged_url_seeks_directly_and_arms_nothing(self, qapp, tmp_path): + (tmp_path / "a.mp3").write_bytes(b"x") + loc = str(tmp_path / "a.mp3") + track = _track(1, location=loc, total_time=200_000, start_time=30_000) + player, _ = _player(qapp, [track]) + sink = player._local + sink._media.source.return_value = QUrl.fromLocalFile(loc) + + player.play_queue([1], 0) + sink._media.setSource.assert_not_called() + sink._media.stop.assert_called() + sink._media.setPosition.assert_called_once_with(30_000) + assert sink._pending_start_ms == 0 + assert sink._pending_start_url == QUrl() + + def test_unchanged_url_without_start_time_does_not_seek(self, qapp, tmp_path): + (tmp_path / "a.mp3").write_bytes(b"x") + loc = str(tmp_path / "a.mp3") + track = _track(1, location=loc, total_time=200_000) + player, _ = _player(qapp, [track]) + sink = player._local + sink._media.source.return_value = QUrl.fromLocalFile(loc) + + player.play_queue([1], 0) + sink._media.stop.assert_called() + sink._media.setPosition.assert_not_called() diff --git a/tests/test_round8.py b/tests/test_round8.py index cf1e1e4..dd460b1 100644 --- a/tests/test_round8.py +++ b/tests/test_round8.py @@ -10,6 +10,8 @@ from unittest.mock import MagicMock, patch +from PyQt6.QtCore import QUrl + from lintunes.models import Library, Track, Playlist, PlaylistType from lintunes.library_manager import LibraryManager from lintunes.gui.library_view import _matches_search @@ -249,6 +251,9 @@ class TestPlayerStartStop: player, _ = self._player(qapp, [track]) player.play_queue([1], 0) assert player._local._pending_start_ms == 30_000 + # By the time the real LoadedMedia arrives Qt reports the new source; + # since Round 32 the armed seek is only consumed when it matches. + player._local._media.source.return_value = QUrl.fromLocalFile(track.location) player._local._on_media_status(player_module.QMediaPlayer.MediaStatus.LoadedMedia) player._local._media.setPosition.assert_called_with(30_000) assert player._local._pending_start_ms == 0 # consumed (one-shot)