trav/lintunes
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

v0.1.1: perf timings + single-save tag writes + coalesced browser rebuild

Browse Source 
Diagnosing the Ctrl+I save hang on the Debian machine (GUI thread blocks
past mutter's 5s check-alive):

- lintunes/perf.py: timed() context manager logging wall-clock ms at INFO
  on lintunes.perf — instruments tag/artwork saves, file moves,
  edit_track(s)_fields, the browser rebuild, smart recompute, and the
  debounced JSON flush, so the slow machine can tell us which stage
  eats the time (journalctl --user or a terminal run).
- tagging.write_tags: ONE parse + ONE save per edit. grouping/
  compilation/bpm ride the main save via registered Easy keys
  (GRP1/TCMP/TBPM on EasyID3, cpil on EasyMP4) instead of
  _write_extra_tags re-parsing and re-saving the audio file.
- LibraryView: browser rebuilds coalesce through a 0ms single-shot
  timer — an N-track Get Info edit rebuilds the genre/artist/album
  cascade once instead of N times over the whole library.

Moving the writes off the GUI thread is deliberately deferred until the
Debian timings say which stage dominates.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
trav
2026-07-03 18:25:56 -04:00
parent 2d02ccefd6
commit 332b47a018
7 changed files with 325 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
TASKS.md
View File

@ -3,7 +3,33 @@
Legend: `[ ]` todo · `[~]` in progress · `[x]` done.
When a round closes, move its finished items to `tasks-done.md`.
## Round 19 (current) — version display, git self-update, BPM fix
## Round 20 (current) — Ctrl+I save hang on the Debian machine (v0.1.1)
Plan reference: `~/.claude/plans/i-m-having-some-trouble-eager-gosling.md`.
Tests in `tests/test_round20.py`. Fix-only round → patch bump **0.1.1**.
Background: on the Debian machine, OK in Get Info froze the GUI thread past
mutter's ~5 s check-alive → "Force Quit / Wait" dialog. Happens on
single-track edits. Whole save path is synchronous on the GUI thread.
- [x] Perf instrumentation: new `lintunes/perf.py` (`timed()` context
manager, INFO on the `lintunes.perf` logger → stderr/journal). Times
tag saves, artwork saves, file moves, edit_track(s)_fields, browser
rebuild, smart recompute, and the debounced JSON flush.
- [x] `tagging.write_tags` now parses + saves the file ONCE per edit:
grouping/compilation/bpm ride the same save via registered Easy keys
(GRP1/TCMP/TBPM on EasyID3, cpil on EasyMP4) instead of
`_write_extra_tags` doing a second full parse+save.
- [x] `LibraryView` coalesces browser rebuilds through a 0 ms single-shot
timer: an N-track Get Info edit rebuilds the genre/artist/album
cascade once, not N times (was O(edited × library)).
- [ ] **Diagnose on the Debian machine**: self-update, reproduce a Ctrl+I
edit, read `lintunes.perf` timings (terminal run or
`journalctl --user`) to see which stage eats the ~5 s — likely the
audio-file rewrite or the 15 MB library.json flush. Then decide on
moving that stage off the GUI thread (deliberately deferred).
## Round 19 — version display, git self-update, BPM fix
Plan reference: `~/.claude/plans/some-fixes-features-for-lintues-merry-journal.md`.
Tests land in `tests/test_round19.py`. First versioned release: **0.1.0**.

2
lintunes/__init__.py
View File

@ -1,3 +1,3 @@
"""LinTunes — iTunes-style music library manager and player for Linux."""
__version__ = "0.1.0"
__version__ = "0.1.1"

19
lintunes/gui/library_view.py
View File

@ -8,6 +8,7 @@ from PyQt6.QtCore import Qt, QTimer, pyqtSignal
from lintunes.gui.table_settings import TableSettingsMixin
from lintunes.gui.track_table import TrackTableView
from lintunes.perf import timed
_ARTICLE_RE = re.compile(r"^the\s+", re.IGNORECASE)
@ -162,6 +163,13 @@ class LibraryView(TableSettingsMixin, QWidget):
manager.track_updated.connect(self.table.model_.refresh_track)
manager.track_fields_edited.connect(self._on_track_fields_edited)
# Coalesces browser rebuilds: a multi-track edit emits
# track_fields_edited once per track, and rebuilding the whole
# genre/artist/album cascade per signal is O(edited × library).
self._browser_rebuild_timer = QTimer(self)
self._browser_rebuild_timer.setSingleShot(True)
self._browser_rebuild_timer.setInterval(0)
self._browser_rebuild_timer.timeout.connect(self._rebuild_browser)
self.reload()
@ -295,8 +303,17 @@ class LibraryView(TableSettingsMixin, QWidget):
rebuild the lists and re-filter so the renamed value sorts to its new
spot and clicking it still finds its tracks (the plain row repaint from
track_updated leaves the browser stale). Non-browser edits (name, bpm…)
are handled by the proxy's live re-sort, so they need nothing here."""
are handled by the proxy's live re-sort, so they need nothing here.
The rebuild is deferred to a 0 ms single-shot timer so an N-track edit
(which fires this once per track) rebuilds the browser once, not N
times."""
if _BROWSER_FIELDS.intersection(changed_fields):
self._browser_rebuild_timer.start()
def _rebuild_browser(self):
with timed("library browser rebuild (%d tracks)",
len(self._all_tracks)):
self._populate_genres()
self._populate_artists()
self._populate_albums()

96
lintunes/library_manager.py
View File

@ -10,6 +10,7 @@ log = logging.getLogger(__name__)
from lintunes import smart, tagging
from lintunes.importers.file_importer import organized_destination, unique_path
from lintunes.models import Library, Playlist, PlaylistType
from lintunes.perf import timed
from lintunes.storage import json_storage
from lintunes.undo import Command, UndoStack
@ -371,16 +372,18 @@ class LibraryManager(QObject):
if hasattr(track, k) and getattr(track, k) != v}
if not changed:
return
old = {k: getattr(track, k) for k in changed}
if not self._write_track_tags(track, changed):
# File write failed: leave the library untouched so memory and file
# don't diverge, and don't record an un-undoable phantom edit.
return
new_loc = self._maybe_move_file(track, changed)
if new_loc is not None:
old = {**old, "location": track.location}
changed = {**changed, "location": new_loc}
self._apply_track_fields(track_id, changed)
with timed("edit_track_fields %r", track.name):
old = {k: getattr(track, k) for k in changed}
if not self._write_track_tags(track, changed):
# File write failed: leave the library untouched so memory and
# file don't diverge, and don't record an un-undoable phantom
# edit.
return
new_loc = self._maybe_move_file(track, changed)
if new_loc is not None:
old = {**old, "location": track.location}
changed = {**changed, "location": new_loc}
self._apply_track_fields(track_id, changed)
self.undo_stack.push(Command(
"Edit Info",
undo=lambda: self._revert_track_fields(track_id, old),
@ -395,21 +398,22 @@ class LibraryManager(QObject):
skipped entirely (no file write, no undo entry).
"""
changes = [] # (track_id, new_fields, old_fields)
for track_id in track_ids:
track = self.library.tracks.get(track_id)
if not track:
continue
changed = {k: v for k, v in fields.items()
if hasattr(track, k) and getattr(track, k) != v}
if not changed or not self._write_track_tags(track, changed):
continue
old = {k: getattr(track, k) for k in changed}
new_loc = self._maybe_move_file(track, changed)
if new_loc is not None:
old = {**old, "location": track.location}
changed = {**changed, "location": new_loc}
self._apply_track_fields(track_id, changed)
changes.append((track_id, changed, old))
with timed("edit_tracks_fields (%d tracks)", len(track_ids)):
for track_id in track_ids:
track = self.library.tracks.get(track_id)
if not track:
continue
changed = {k: v for k, v in fields.items()
if hasattr(track, k) and getattr(track, k) != v}
if not changed or not self._write_track_tags(track, changed):
continue
old = {k: getattr(track, k) for k in changed}
new_loc = self._maybe_move_file(track, changed)
if new_loc is not None:
old = {**old, "location": track.location}
changed = {**changed, "location": new_loc}
self._apply_track_fields(track_id, changed)
changes.append((track_id, changed, old))
if not changes:
return
self.undo_stack.push(Command(
@ -518,12 +522,13 @@ class LibraryManager(QObject):
empty. Same-filesystem rename, so this is safe even while the player
has the file open (the fd stays valid; the path is re-read at next
play). Returns the actual destination (collision-suffixed if taken)."""
dest = unique_path(dest)
dest.parent.mkdir(parents=True, exist_ok=True)
src.rename(dest)
root = self.organize_root()
if root is not None:
self._prune_empty_dirs(src.parent, root)
with timed("move file %s", src.name):
dest = unique_path(dest)
dest.parent.mkdir(parents=True, exist_ok=True)
src.rename(dest)
root = self.organize_root()
if root is not None:
self._prune_empty_dirs(src.parent, root)
return dest
@staticmethod
@ -616,18 +621,22 @@ class LibraryManager(QObject):
self._pending_recompute_fields = set()
self._recomputing = True
try:
for pid, playlist in list(self.library.playlists.items()):
if not playlist.is_smart:
continue
criteria = playlist.smart_criteria
if criteria is None or criteria.unsupported or not criteria.live_update:
continue
if fields is not None and not self._criteria_touched(criteria, fields):
continue
self.recompute_smart_playlist(pid)
with timed("smart playlist recompute"):
self._recompute_touched(fields)
finally:
self._recomputing = False
def _recompute_touched(self, fields: set[str] | None):
for pid, playlist in list(self.library.playlists.items()):
if not playlist.is_smart:
continue
criteria = playlist.smart_criteria
if criteria is None or criteria.unsupported or not criteria.live_update:
continue
if fields is not None and not self._criteria_touched(criteria, fields):
continue
self.recompute_smart_playlist(pid)
@staticmethod
def _criteria_touched(criteria, fields: set[str]) -> bool:
if criteria.referenced_attrs() & fields:
@ -660,13 +669,16 @@ class LibraryManager(QObject):
def flush(self):
self._save_timer.stop()
if self._dirty_tracks:
json_storage.save_tracks(self.library, self.data_dir)
with timed("flush library.json (%d tracks)",
len(self.library.tracks)):
json_storage.save_tracks(self.library, self.data_dir)
self._dirty_tracks = False
self._record_sig(self.data_dir / "library.json")
for pid in list(self._dirty_playlists):
playlist = self.library.playlists.get(pid)
if playlist:
json_storage.save_playlist(playlist, self.data_dir)
with timed("flush playlist %s", pid):
json_storage.save_playlist(playlist, self.data_dir)
self._record_sig(self.data_dir / "playlists" / f"{pid}.json")
self._dirty_playlists.clear()
for pid in list(self._deleted_playlists):

22
lintunes/perf.py Normal file
View File

@ -0,0 +1,22 @@
"""Wall-clock timing for the save/edit hot paths.
Logs at INFO on the ``lintunes.perf`` logger, so a normal desktop launch
records where the time went (visible via ``journalctl --user`` or a
terminal run). Used to diagnose GUI-thread stalls on slow machines.
"""
import logging
import time
from contextlib import contextmanager
log = logging.getLogger("lintunes.perf")
@contextmanager
def timed(label: str, *args):
"""``with timed("write_tags %s", path):`` — logs the block's duration."""
start = time.perf_counter()
try:
yield
finally:
ms = (time.perf_counter() - start) * 1000
log.info("%s: %.0f ms", label % args if args else label, ms)

128
lintunes/tagging.py
View File

@ -9,11 +9,13 @@ from pathlib import Path
import mutagen
from mutagen.easyid3 import EasyID3
from mutagen.id3 import ID3, COMM
from mutagen.id3 import ID3, COMM, GRP1, TCMP, TBPM
from mutagen.mp3 import MP3
from mutagen.mp4 import MP4
from mutagen.flac import FLAC
from lintunes.perf import timed
# EasyID3 has no 'comment' key by default; COMM frames need explicit handling.
def _comm_get(id3, _key):
@ -32,10 +34,55 @@ def _comm_delete(id3, _key):
EasyID3.RegisterKey("comment", _comm_get, _comm_set, _comm_delete)
# Register grouping/compilation/bpm as Easy keys (iTunes-style frames) so
# write_tags can set every field on one object and save the file ONCE —
# a second parse+save per edit is real I/O on big files / slow disks.
def _register_id3_frame_key(key: str, frame_cls):
frame_id = frame_cls.__name__
def getter(id3, _key):
frame = id3.get(frame_id)
return list(frame.text) if frame else []
def setter(id3, _key, value):
id3.delall(frame_id)
id3.add(frame_cls(encoding=3, text=value))
def deleter(id3, _key):
id3.delall(frame_id)
EasyID3.RegisterKey(key, getter, setter, deleter)
_register_id3_frame_key("grouping", GRP1) # iTunes GRP1, not TIT1
_register_id3_frame_key("compilation", TCMP)
_register_id3_frame_key("bpm", TBPM)
# EasyMP4 is missing 'composer' (©wrt) out of the box
from mutagen.easymp4 import EasyMP4Tags # noqa: E402
if "composer" not in EasyMP4Tags.Set:
EasyMP4Tags.RegisterTextKey("composer", "\xa9wrt")
if "grouping" not in EasyMP4Tags.Set:
EasyMP4Tags.RegisterTextKey("grouping", "\xa9grp")
if "bpm" not in EasyMP4Tags.Set:
EasyMP4Tags.RegisterIntKey("bpm", "tmpo")
def _cpil_get(tags, _key):
return ["1"] if tags.get("cpil") else []
def _cpil_set(tags, _key, value):
tags["cpil"] = bool(value and str(value[0]) not in ("", "0"))
def _cpil_delete(tags, _key):
tags.pop("cpil", None)
if "compilation" not in EasyMP4Tags.Set:
EasyMP4Tags.RegisterKey("compilation", _cpil_get, _cpil_set, _cpil_delete)
# Easy-interface tag names shared by EasyID3 / EasyMP4 / FLAC (Vorbis)
_TEXT_FIELDS = {
@ -96,7 +143,10 @@ def read_tags(path: str | Path) -> dict:
def write_tags(path: str | Path, fields: dict):
"""Write the given Track-style fields to the file's tags."""
"""Write the given Track-style fields to the file's tags.
One parse and ONE save per call: grouping/compilation/bpm are covered by
the Easy keys registered above, so no second raw-tag pass is needed."""
path = str(path)
audio = mutagen.File(path, easy=True)
if audio is None:
@ -130,61 +180,28 @@ def write_tags(path: str | Path, fields: dict):
else:
audio.tags.pop(tag_key, None)
audio.save()
_write_extra_tags(path, fields)
if "grouping" in fields:
_set_or_pop(audio.tags, "grouping",
str(fields["grouping"]) if fields["grouping"] else None)
if "bpm" in fields:
_set_or_pop(audio.tags, "bpm",
str(fields["bpm"]) if fields["bpm"] else None)
if "compilation" in fields:
_set_or_pop(audio.tags, "compilation",
"1" if fields["compilation"] else None)
with timed("tag save %s", path):
audio.save()
def _write_extra_tags(path: str, fields: dict):
"""Fields the easy interfaces don't cover uniformly: grouping, compilation, bpm."""
wants = {k for k in ("grouping", "compilation", "bpm") if k in fields}
if not wants:
def _set_or_pop(tags, key, value):
if value:
tags[key] = [value]
return
audio = mutagen.File(path)
if isinstance(audio, MP3):
from mutagen.id3 import GRP1, TCMP, TBPM
if "grouping" in fields:
audio.tags.delall("GRP1")
if fields["grouping"]:
audio.tags.add(GRP1(encoding=3, text=[fields["grouping"]]))
if "compilation" in fields:
audio.tags.delall("TCMP")
if fields["compilation"]:
audio.tags.add(TCMP(encoding=3, text=["1"]))
if "bpm" in fields:
audio.tags.delall("TBPM")
if fields["bpm"]:
audio.tags.add(TBPM(encoding=3, text=[str(fields["bpm"])]))
audio.save()
elif isinstance(audio, MP4):
if "grouping" in fields:
_mp4_set(audio, "\xa9grp", fields["grouping"])
if "compilation" in fields:
audio.tags["cpil"] = bool(fields["compilation"])
if "bpm" in fields:
_mp4_set(audio, "tmpo", int(fields["bpm"]) if fields["bpm"] else None)
audio.save()
elif isinstance(audio, FLAC):
if "grouping" in fields:
_vorbis_set(audio, "grouping", fields["grouping"])
if "compilation" in fields:
_vorbis_set(audio, "compilation", "1" if fields["compilation"] else "")
if "bpm" in fields:
_vorbis_set(audio, "bpm", str(fields["bpm"]) if fields["bpm"] else "")
audio.save()
def _mp4_set(audio, key, value):
if value:
audio.tags[key] = [value]
else:
audio.tags.pop(key, None)
def _vorbis_set(audio, key, value):
if value:
audio.tags[key] = [value]
else:
audio.tags.pop(key, None)
try:
del tags[key] # FLAC's VCommentDict has no pop-with-default
except KeyError:
pass
def _kind_for(audio) -> str:
@ -230,7 +247,8 @@ def write_artwork(path: str | Path, image_bytes: bytes, mime: str = "image/jpeg"
data=image_bytes))
else:
raise ValueError(f"Don't know how to embed artwork in {path}")
audio.save()
with timed("artwork save %s", path):
audio.save()
def read_embedded_artwork(path: str | Path) -> bytes | None:

130
tests/test_round20.py Normal file
View File

@ -0,0 +1,130 @@
"""Round 20: Ctrl+I save-path performance.
- ``tagging.write_tags`` now does ONE mutagen parse + ONE save per call:
grouping/compilation/bpm ride the same save (via registered Easy keys)
instead of a second full parse+save of the audio file.
- ``LibraryView`` coalesces browser rebuilds through a 0 ms single-shot
timer, so an N-track edit rebuilds the genre/artist/album cascade once.
- ``perf.timed`` logs wall-clock durations on the ``lintunes.perf`` logger.
"""
import logging
import mutagen
import pytest
from lintunes import tagging
from lintunes.library_manager import LibraryManager
from lintunes.models import Track
from lintunes.models.library import Library
from lintunes.perf import timed
def _manager(tmp_path, tracks=()):
library = Library()
for track in tracks:
library.tracks[track.track_id] = track
return LibraryManager(library, tmp_path)
EXTRAS = {"grouping": "Test Group", "compilation": True, "bpm": 128}
# ---- single parse + save in write_tags ----
class TestSingleSave:
@pytest.mark.parametrize("fixture", ["mp3_file", "m4a_file", "flac_file"])
def test_one_parse_per_write(self, fixture, request, monkeypatch):
path = request.getfixturevalue(fixture)
opens = []
real_file = mutagen.File
def counting_file(*args, **kwargs):
opens.append(args)
return real_file(*args, **kwargs)
monkeypatch.setattr(tagging.mutagen, "File", counting_file)
tagging.write_tags(path, {"name": "Title", **EXTRAS})
assert len(opens) == 1
def test_mp3_extras_use_itunes_frames(self, mp3_file):
tagging.write_tags(mp3_file, EXTRAS)
raw = mutagen.File(mp3_file)
assert [str(t) for t in raw.tags["GRP1"].text] == ["Test Group"]
assert str(raw.tags["TCMP"].text[0]) == "1"
assert str(raw.tags["TBPM"].text[0]) == "128"
def test_m4a_extras_use_itunes_atoms(self, m4a_file):
tagging.write_tags(m4a_file, EXTRAS)
raw = mutagen.File(m4a_file)
assert raw.tags["\xa9grp"] == ["Test Group"]
assert bool(raw.tags["cpil"])
assert list(raw.tags["tmpo"]) == [128]
def test_flac_extras_use_vorbis_comments(self, flac_file):
tagging.write_tags(flac_file, EXTRAS)
raw = mutagen.File(flac_file)
assert raw["grouping"] == ["Test Group"]
assert raw["compilation"] == ["1"]
assert raw["bpm"] == ["128"]
@pytest.mark.parametrize("fixture", ["mp3_file", "m4a_file", "flac_file"])
def test_clearing_extras_removes_them(self, fixture, request):
path = request.getfixturevalue(fixture)
tagging.write_tags(path, EXTRAS)
tagging.write_tags(path, {"grouping": "", "compilation": False,
"bpm": 0})
easy = mutagen.File(path, easy=True)
for key in ("grouping", "compilation", "bpm"):
assert not easy.tags.get(key), key
# ---- coalesced browser rebuild ----
class TestCoalescedBrowserRebuild:
def _view_with_counter(self, qapp, tmp_path, monkeypatch, tracks):
from lintunes.gui.library_view import LibraryView
manager = _manager(tmp_path, tracks)
view = LibraryView(manager)
rebuilds = []
# _rebuild_browser looks methods up on self, so patching one of them
# counts rebuilds without disturbing the timer wiring.
monkeypatch.setattr(view, "_populate_genres",
lambda: rebuilds.append(1))
return manager, view, rebuilds
def test_multi_edit_rebuilds_once(self, qapp, tmp_path, monkeypatch):
tracks = [Track(track_id=i, name=f"T{i}", artist="Old")
for i in (1, 2, 3)]
manager, view, rebuilds = self._view_with_counter(
qapp, tmp_path, monkeypatch, tracks)
manager.edit_tracks_fields([1, 2, 3], {"artist": "New"})
assert rebuilds == [] # deferred to the event loop
qapp.processEvents()
assert len(rebuilds) == 1
assert all(t.artist == "New" for t in tracks)
def test_single_edit_still_rebuilds(self, qapp, tmp_path, monkeypatch):
track = Track(track_id=1, name="T", genre="Pop")
manager, view, rebuilds = self._view_with_counter(
qapp, tmp_path, monkeypatch, [track])
manager.edit_track_fields(1, {"genre": "Rock"})
qapp.processEvents()
assert len(rebuilds) == 1
def test_non_browser_edit_skips_rebuild(self, qapp, tmp_path, monkeypatch):
track = Track(track_id=1, name="T")
manager, view, rebuilds = self._view_with_counter(
qapp, tmp_path, monkeypatch, [track])
manager.edit_track_fields(1, {"name": "Renamed"})
qapp.processEvents()
assert rebuilds == []
# ---- perf logging ----
def test_timed_logs_duration(caplog):
with caplog.at_level(logging.INFO, logger="lintunes.perf"):
with timed("unit test %s", "label"):
pass
messages = [r.getMessage() for r in caplog.records]
assert any("unit test label" in m and "ms" in m for m in messages)