v0.5.2: custom start times survive a track change
Playing a track with an iTunes start time (e.g. "Pretty Girls is a Motherfucker", 0:40) began at 0:00 whenever another track was already loaded. QMediaPlayer.setSource() synchronously emits two status changes before returning: a LoadedMedia still reporting the *outgoing* source, then LoadingMedia for the new one. The stale first event consumed the one-shot _pending_start_ms armed in Round 22 and seeked the dying pipeline, so the new track's real LoadedMedia ~5 ms later found nothing armed. Only the first track after launch worked. The armed seek now carries the URL it belongs to and is consumed only when source() matches. tests/test_round32.py models the real Qt event sequence, which Round 22's plain MagicMock never emitted; the test_round8/test_round22 stubs now report the new source by the time its LoadedMedia arrives, as Qt does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -1,3 +1,3 @@
|
||||
"""LinTunes — iTunes-style music library manager and player for Linux."""
|
||||
|
||||
__version__ = "0.5.1"
|
||||
__version__ = "0.5.2"
|
||||
|
||||
+22
-6
@@ -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:
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user