From 9fdd61b648ed9b8c958ec62d34eec5a28980fc29 Mon Sep 17 00:00:00 2001 From: Emma Thorpe Date: Mon, 24 Aug 2026 19:21:57 +0100 Subject: [PATCH 1/2] feat: report which albums have never been played Stage four, read-only. An album is cold when it has files, not one of its tracks has ever been scrobbled across the whole nine-year history, and its newest file landed over a year ago. The age floor is measured from the file rather than the release date: what matters is how long a record has been available to play, not how old it is. Album-level rather than track-level. A record with two played tracks is a record that gets played, and picking the other ten off it leaves gaps in an album rather than reclaiming anything worth having. Track file sizes are now indexed so the report can rank by the disk actually recoverable, which is the point of the exercise. That needed a column on a table that already exists, so the store gained an in-place column migration; a full rebuild would mean re-downloading sixty thousand scrobbles. The report refuses to produce anything when the backfill is unfinished, when any artist failed to index, when any artist has no albums, or when there is no history to judge against. Each of those makes played music look unplayed, which is the one failure that costs a library, and they are checked rather than trusted because the report they gate is the one that ends in deletion. Nothing is written to Lidarr. Every call there remains a GET, and unmonitoring waits until the list this produces has been looked at -- no flag protects against a list that is wrong. Also prunes playlists no longer produced, which #12 made necessary by renaming moods: high-energy-rock would otherwise have stayed on the device for ever. It is driven by a record of what was written last time rather than by deleting every M3U that is not currently ours, so a playlist put in that directory by hand is left alone. --- README.md | 43 ++++++- music_curator.py | 229 +++++++++++++++++++++++++++++++++--- tests/test_music_curator.py | 118 ++++++++++++++++++- 3 files changed, 371 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 3ae5d07..ec8b2e3 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ which keeps an MP3 copy of a lossless library for an iPod. This one answers the question that mirror cannot: which of it is worth carrying, and which of it has not been played in years. -**This is stage three.** It ingests the scrobble history, indexes the library +**This is stage four, read-only.** It ingests the scrobble history, indexes the library from Lidarr, matches one to the other, and writes playlists into the mirror — by listening history and by mood. Nothing is written back to Lidarr — every call there is a `GET`. See "Where this @@ -20,7 +20,8 @@ is going" below. - Indexes every artist, album and track Lidarr knows about, with file paths and the date each file landed. - Ties the two together and reports how well it managed. -- Writes M3U playlists into the mirror, from the listening history. +- Writes M3U playlists into the mirror, from the listening history and by mood. +- Reports which albums have never been played. It does not act on that. ## Matching @@ -276,6 +277,40 @@ or `--mirror` looks like, since the paths then map to nothing at all. `_playlists/` survives music-mirror's prune: it only deletes `*.mp3`, and its empty-directory sweep skips a directory holding M3Us. +## The cold report + +Which albums have files, have never had a single track played in the whole +history, and have sat there long enough to have had the chance. Ranked by the +disk they occupy, because that is the point of the exercise. + +**Album-level, not track-level.** A record with two played tracks is a record +that gets played; picking the other ten off it leaves gaps rather than +reclaiming anything worth having. + +The age floor is measured from the newest file in the album, not from the +release date — what matters is how long it has been available to play, not how +old the record is. `--cold-after` sets it, defaulting to a year. + +Artists whose *every* album is cold are counted separately. That is a different +proposition from one cold record by somebody otherwise played, and Lidarr can +only tag at artist level anyway. + +### What stops it + +The report refuses to produce anything at all when: + +- the scrobble backfill is unfinished +- any artist failed to index, or any artist has no albums indexed +- the library has not been indexed, or there is no history to judge against + +Each of those makes played music look unplayed, which is the single failure that +costs a library. They are checked rather than trusted, because the report they +gate is the one that ends in deletion. + +**Nothing is written to Lidarr.** Every call there is still a `GET`. Unmonitoring +comes once the list has been looked at, because no flag protects against a list +that is wrong. + ## How the ingest works Two halves, both taking their bounds from the database rather than from a saved @@ -335,6 +370,7 @@ music-curator --report-only # report on the store, fetch nothing | `--playlist-limit` | `MUSIC_CURATOR_PLAYLIST_LIMIT` | `100` | Most tracks in any one playlist | | `--vibes` | `MUSIC_CURATOR_VIBES` | built-in | JSON file of mood definitions | | `--tag-limit` | `MUSIC_CURATOR_TAG_LIMIT` | `0` | Cap artist tag lookups per pass | +| `--cold-after` | `MUSIC_CURATOR_COLD_AFTER` | `365` | Days a file must sit unplayed to count as cold | | `--skip-index` | — | off | Match against the index already held | | `--report-only` | — | off | Report without fetching | @@ -385,7 +421,8 @@ nix shell nixpkgs#python3Packages.pytest -c pytest | Lidarr index and the scrobble-to-track matcher | done | | M3U playlists from the listening history | done | | Genre and mood playlists from Last.fm tags | done | -| Cold-music report, unmonitoring what is not played | last | +| Cold-music report | done | +| Unmonitoring what is not played | last | The cull will unmonitor cold albums in Lidarr and tag their artists. It will never delete files: Lidarr's `AlbumResource` has no tags at all, so tagging diff --git a/music_curator.py b/music_curator.py index 31e38c4..14552e9 100644 --- a/music_curator.py +++ b/music_curator.py @@ -85,12 +85,17 @@ SAFE_VIBE_NAME = re.compile(r"^[a-z0-9][a-z0-9-]*$") # back by whatever serves it, and a playlist nobody can read is not a playlist. GROUP_READ = 0o040 -SCHEMA_VERSION = "2" +SCHEMA_VERSION = "3" # Versions this build upgrades in place. Everything added since version 1 is a -# new table, and the schema script only ever creates what is missing, so running -# it is the whole migration -- an existing history is not re-downloaded. -MIGRATABLE_FROM = {"1"} +# new table or a new column, both of which are applied in place, so an existing +# history is never re-downloaded -- a full backfill is thousands of requests. +MIGRATABLE_FROM = {"1", "2"} + +# Columns added to existing tables after the fact. CREATE TABLE IF NOT EXISTS +# will not add a column to a table that already exists, so these are applied +# separately and only when missing. +ADDED_COLUMNS = (("lidarr_track", "size", "INTEGER"),) SCHEMA = """ -- One row per scrobble. The primary key collapses two plays of the same track @@ -160,7 +165,8 @@ CREATE TABLE IF NOT EXISTS lidarr_track ( has_file INTEGER NOT NULL DEFAULT 0, path TEXT, added INTEGER, - duration INTEGER + duration INTEGER, + size INTEGER ); CREATE INDEX IF NOT EXISTS lidarr_track_recording ON lidarr_track (recording_mbid); CREATE INDEX IF NOT EXISTS lidarr_track_norm ON lidarr_track (norm_artist, norm_title); @@ -497,8 +503,18 @@ class Store: # thousand name pairs into Python to compare them one at a time. self.connection.create_function("normalise", 1, normalise, deterministic=True) self.connection.executescript(SCHEMA) + self._add_missing_columns() self._check_version() + def _add_missing_columns(self): + """Apply column additions to tables that predate them.""" + for table, column, kind in ADDED_COLUMNS: + held = {row[1] for row in self.connection.execute(f"PRAGMA table_info({table})")} + if column not in held: + logger.info("adding %s.%s to the store", table, column) + with self.connection: + self.connection.execute(f"ALTER TABLE {table} ADD COLUMN {column} {kind}") + def _check_version(self): held = self.get_state("schema_version") if held is None or held in MIGRATABLE_FROM: @@ -580,8 +596,8 @@ class Store: self.connection.executemany( "INSERT INTO lidarr_track" " (id, artist_id, album_id, recording_mbid, title, norm_artist, norm_title," - " has_file, path, added, duration)" - " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + " has_file, path, added, duration, size)" + " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", tracks, ) @@ -1040,6 +1056,7 @@ def index_library(client, store): handle.get("path"), parse_added(handle.get("dateAdded")), track.get("duration"), + handle.get("size"), ) ) @@ -1473,11 +1490,12 @@ def sync_tags(client, store, now, limit=0): def build_vibe_playlists(store, vibes, mirror_root, library_root, limit, now): """Write one playlist per mood. Returns how many tracks were listed.""" if not store.scalar("SELECT COUNT(*) FROM artist_tag"): - return 0 + return 0, [] directory = Path(mirror_root) / PLAYLIST_DIRECTORY week = now // ROTATION_PERIOD_SECONDS total = 0 + produced = [] for vibe in vibes: tags = [str(tag).strip().casefold() for tag in vibe["tags"]] @@ -1535,10 +1553,11 @@ def build_vibe_playlists(store, vibes, mirror_root, library_root, limit, now): entries.append({**dict(row), "mirror": mirror}) write_playlist(directory / f"{vibe['name']}.m3u", entries, Path(mirror_root)) + produced.append(f"{vibe['name']}.m3u") total += len(entries) logger.info("playlist %-20s %4d tracks -- by tag", vibe["name"], len(entries)) - return total + return total, produced def library_root_of(store): @@ -1641,6 +1660,22 @@ def write_playlist(path, entries, reference=None): temporary.unlink(missing_ok=True) +def prune_playlists(directory, produced, store): + """Delete playlists this build no longer produces. + + Driven by a record of what was written last time rather than by "every M3U + that is not one of ours", so a playlist put there by hand is left alone. A + renamed mood otherwise leaves its old file on the device for ever. + """ + previous = set(json.loads(store.get_state("playlist_files") or "[]")) + for name in sorted(previous - set(produced)): + stale = directory / name + if stale.is_file(): + logger.info("removing playlist %s, no longer produced", name) + stale.unlink(missing_ok=True) + store.set_state("playlist_files", json.dumps(sorted(produced))) + + def build_playlists(store, mirror_root, library_root, limit, now): """Write every playlist into the mirror. Returns how many tracks were listed. @@ -1658,6 +1693,7 @@ def build_playlists(store, mirror_root, library_root, limit, now): total = 0 missing = 0 + produced = [] for name, description, sql in PLAYLISTS: entries = [] for row in store.connection.execute(sql, parameters): @@ -1667,6 +1703,7 @@ def build_playlists(store, mirror_root, library_root, limit, now): continue entries.append({**dict(row), "mirror": mirror}) write_playlist(directory / f"{name}.m3u", entries, Path(mirror_root)) + produced.append(f"{name}.m3u") total += len(entries) logger.info("playlist %-20s %4d tracks -- %s", name, len(entries), description) @@ -1678,7 +1715,7 @@ def build_playlists(store, mirror_root, library_root, limit, now): " place, since the paths are then being mapped to nothing.", missing, ) - return total + return total, produced def coverage_report(store): @@ -1852,7 +1889,156 @@ def coverage_report(store): logger.info(" unmatched %2d: %-60s %d plays", position, label[:60], row["plays"]) -def report(store, now): +# An album has to have sat unplayed for at least this long before it counts as +# cold. Measured from the newest file in it, not from the release date: what +# matters is how long it has been available to play, not how old the record is. +COLD_AFTER_DAYS = 365 + + +def human_bytes(count): + """Return a size that can be read at a glance.""" + size = float(count or 0) + for unit in ("B", "KiB", "MiB", "GiB", "TiB"): + if size < 1024 or unit == "TiB": + return f"{size:.1f} {unit}" + size /= 1024 + + +def cull_is_safe(store): + """Return why a cull must not run, or None if it may. + + Every one of these makes played music look unplayed, which is the single + failure that costs a library. They are checked rather than trusted because + the report they gate is the one that ends in deletion. + """ + if store.get_state("backfill_complete") != "yes": + return "the scrobble history is still being backfilled" + if int(store.get_state("index_skipped") or 0): + return "some artists could not be indexed, so their tracks are missing" + if int(store.get_state("index_albums_skipped") or 0): + return "some artists have no albums indexed" + if not store.scalar("SELECT COUNT(*) FROM lidarr_track"): + return "the library has not been indexed" + if not store.scalar("SELECT COUNT(*) FROM scrobble"): + return "there is no listening history to judge against" + return None + + +COLD_ALBUMS = """ +WITH played AS ( + SELECT DISTINCT sk.track_id AS track_id + FROM scrobble_key sk + WHERE sk.track_id IS NOT NULL +), +album AS ( + SELECT t.album_id AS album_id, + COUNT(*) AS tracks, + SUM(COALESCE(t.size, 0)) AS bytes, + MAX(COALESCE(t.added, 0)) AS newest, + SUM(CASE WHEN p.track_id IS NULL THEN 0 ELSE 1 END) AS plays + FROM lidarr_track t + LEFT JOIN played p ON p.track_id = t.id + WHERE t.has_file = 1 + GROUP BY t.album_id +) +SELECT al.id AS album_id, + al.title AS title, + ar.id AS artist_id, + ar.name AS artist, + album.tracks AS tracks, + album.bytes AS bytes, + album.newest AS newest + FROM album + JOIN lidarr_album al ON al.id = album.album_id + JOIN lidarr_artist ar ON ar.id = al.artist_id + WHERE album.plays = 0 + AND album.newest > 0 + AND album.newest < :cutoff +""" + + +def cold_albums(store, cutoff): + """Return every album with files, none of them ever played, old enough to judge.""" + return store.connection.execute(COLD_ALBUMS, {"cutoff": cutoff}).fetchall() + + +def cold_report(store, now, cold_after_days=COLD_AFTER_DAYS): + """Report what has never been played. Writes nothing, anywhere. + + Album-level rather than track-level: a record with two played tracks is a + record that gets played, and picking the other ten off it leaves gaps rather + than reclaiming anything worth having. + """ + refusal = cull_is_safe(store) + if refusal is not None: + logger.warning("no cold report: %s", refusal) + return [] + + cutoff = now - cold_after_days * 86400 + rows = cold_albums(store, cutoff) + if not rows: + logger.info("--- cold albums --- none: everything with a file has been played") + return [] + + total_bytes = sum(row["bytes"] or 0 for row in rows) + total_tracks = sum(row["tracks"] or 0 for row in rows) + albums_with_files = store.scalar( + "SELECT COUNT(DISTINCT album_id) FROM lidarr_track WHERE has_file = 1" + ) + + logger.info("--- cold albums (never played, files older than %d days) ---", cold_after_days) + logger.info( + "%d of %d albums, %d tracks, %s", + len(rows), + albums_with_files, + total_tracks, + human_bytes(total_bytes), + ) + + # An artist every one of whose albums is cold is a different proposition + # from one cold record by someone otherwise played, and Lidarr can only tag + # at artist level anyway. + cold_by_artist = {} + for row in rows: + held = cold_by_artist.setdefault(row["artist_id"], {"name": row["artist"], "albums": 0, + "bytes": 0}) + held["albums"] += 1 + held["bytes"] += row["bytes"] or 0 + + owned = dict( + store.connection.execute( + "SELECT al.artist_id, COUNT(DISTINCT al.id) FROM lidarr_album al" + " JOIN lidarr_track t ON t.album_id = al.id AND t.has_file = 1" + " GROUP BY al.artist_id" + ).fetchall() + ) + entirely = [ + held for artist_id, held in cold_by_artist.items() + if held["albums"] == owned.get(artist_id) + ] + logger.info( + "%d artists are cold in their entirety (%s), %d have some cold records", + len(entirely), + human_bytes(sum(held["bytes"] for held in entirely)), + len(cold_by_artist) - len(entirely), + ) + + for position, held in enumerate( + sorted(cold_by_artist.values(), key=lambda held: -held["bytes"])[:20], start=1 + ): + logger.info( + " cold %2d: %-40s %2d albums, %s", + position, + held["name"][:40], + held["albums"], + human_bytes(held["bytes"]), + ) + + logger.info("nothing was changed in Lidarr: this report does not write") + return rows + + +def report(store, now, cold_after=COLD_AFTER_DAYS): """Log what the store holds. The MBID coverage line is the one to watch: scrobbles carrying a MusicBrainz @@ -1886,6 +2072,7 @@ def report(store, now): logger.info(" top artist %2d: %-40s %d", position, row["artist"][:40], row["plays"]) coverage_report(store) + cold_report(store, now, cold_after) recent = store.connection.execute( "SELECT artist, track, COUNT(*) AS plays FROM scrobble WHERE uts >= ?" @@ -1921,9 +2108,14 @@ def run_once( " written; set --library-root" ) else: - build_playlists(store, mirror, root, playlist_limit, now) + _, from_history = build_playlists(store, mirror, root, playlist_limit, now) sync_tags(client, store, now, limit=tag_limit) - build_vibe_playlists(store, vibes, mirror, root, playlist_limit, now) + _, from_tags = build_vibe_playlists( + store, vibes, mirror, root, playlist_limit, now + ) + prune_playlists( + Path(mirror) / PLAYLIST_DIRECTORY, from_history + from_tags, store + ) logger.info( "pass complete in %.1fs: %d scrobbles added, %d loved", @@ -2027,6 +2219,13 @@ def build_parser(): default=int(os.getenv("MUSIC_CURATOR_TAG_LIMIT", "0")), help="cap artist tag lookups per pass; 0 for no cap (env MUSIC_CURATOR_TAG_LIMIT)", ) + parser.add_argument( + "--cold-after", + type=int, + default=int(os.getenv("MUSIC_CURATOR_COLD_AFTER", str(COLD_AFTER_DAYS))), + help="days a file must sit unplayed before its album counts as cold" + " (env MUSIC_CURATOR_COLD_AFTER)", + ) parser.add_argument( "--skip-index", action="store_true", @@ -2065,7 +2264,7 @@ def main(argv=None, clock=time.time): store = Store(database) if args.report_only: - report(store, int(clock())) + report(store, int(clock()), args.cold_after) store.close() lock.close() return 0 @@ -2109,7 +2308,7 @@ def main(argv=None, clock=time.time): logger.error("%s", error) if interval is None: return 1 - report(store, now) + report(store, now, args.cold_after) if interval is None or stopping: return 0 diff --git a/tests/test_music_curator.py b/tests/test_music_curator.py index df74267..969784c 100644 --- a/tests/test_music_curator.py +++ b/tests/test_music_curator.py @@ -2,6 +2,7 @@ import json import os import stat import urllib.error +from datetime import datetime, timezone from pathlib import Path import pytest @@ -924,9 +925,10 @@ def test_a_track_with_no_mirror_file_is_left_out(tmp_path): ) music_curator.match_library(store) - total = music_curator.build_playlists(store, mirror, str(source), 100, NOW) + total, produced = music_curator.build_playlists(store, mirror, str(source), 100, NOW) assert total == 0 + assert produced, "the playlists are still written, they are simply empty" assert (mirror / "_playlists" / "all-time.m3u").read_text() == "#EXTM3U\n" @@ -1342,3 +1344,117 @@ def test_a_playlist_is_chowned_to_match_the_mirror(tmp_path, monkeypatch): assert all(tuple(owner) == (568, 568) for _, *owner in attempted) # The temporary file, before the rename, never the finished playlist. assert all(path.endswith(".part") or path.endswith("_playlists") for path, *_ in attempted) + + +def test_a_renamed_mood_does_not_leave_its_old_playlist_behind(tmp_path): + """Otherwise the dead file stays on the device for ever.""" + api, source, mirror = playlist_library(tmp_path) + store = store_at(tmp_path) + directory = mirror / "_playlists" + directory.mkdir(parents=True, exist_ok=True) + (directory / "high-energy-rock.m3u").write_text("#EXTM3U\n") + (directory / "screamo.m3u").write_text("#EXTM3U\n") + store.set_state( + "playlist_files", json.dumps(["high-energy-rock.m3u", "screamo.m3u"]) + ) + + music_curator.prune_playlists(directory, ["screamo.m3u"], store) + + assert not (directory / "high-energy-rock.m3u").exists() + assert (directory / "screamo.m3u").exists() + + +def test_pruning_leaves_a_playlist_it_never_wrote(tmp_path): + """A hand-made playlist in that directory is not ours to delete.""" + api, source, mirror = playlist_library(tmp_path) + store = store_at(tmp_path) + directory = mirror / "_playlists" + directory.mkdir(parents=True, exist_ok=True) + (directory / "lyras-own-mix.m3u").write_text("#EXTM3U\n") + store.set_state("playlist_files", json.dumps(["screamo.m3u"])) + + music_curator.prune_playlists(directory, [], store) + + assert (directory / "lyras-own-mix.m3u").exists() + + +def cold_store(tmp_path, played=(), added="2020-05-01T12:00:00Z"): + """A library where nothing is played unless named, indexed and matched.""" + library = [ + { + "name": "Played Band", + "albums": [{"title": "Known", "tracks": [{"title": "Hit", "added": added}]}], + }, + { + "name": "Silent Band", + "albums": [ + {"title": "Unknown", "tracks": [{"title": "Never Heard", "added": added}]} + ], + }, + ] + store = store_at(tmp_path) + ingest(store, FakeLastfm([scrobble_of(a, t) for a, t in played])) + music_curator.index_library( + music_curator.Lidarr("http://lidarr", "key", transport=FakeLidarr(library)), store + ) + music_curator.match_library(store) + return store + + +def test_an_album_with_no_plays_is_cold(tmp_path): + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + + cold = music_curator.cold_report(store, NOW) + + assert [row["artist"] for row in cold] == ["Silent Band"] + + +def test_an_album_with_any_play_is_not_cold(tmp_path): + """A record with one played track is a record that gets played; picking the + rest off it leaves gaps rather than reclaiming anything.""" + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + + cold = music_curator.cold_report(store, NOW) + + assert "Played Band" not in [row["artist"] for row in cold] + + +def test_a_recent_arrival_is_too_young_to_judge(tmp_path): + """It has not had a chance to be played yet.""" + recent = datetime.fromtimestamp(NOW - 30 * 86400, tz=timezone.utc).isoformat() + store = cold_store(tmp_path, played=[("Played Band", "Hit")], added=recent) + + assert music_curator.cold_report(store, NOW) == [] + + +def test_the_cull_refuses_to_run_on_an_incomplete_index(tmp_path): + """A missing artist makes their played music look unplayed, which is + precisely how this would delete something you like.""" + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + store.set_state("index_skipped", "1") + + assert music_curator.cull_is_safe(store) is not None + assert music_curator.cold_report(store, NOW) == [] + + +def test_the_cull_refuses_while_the_backfill_is_unfinished(tmp_path): + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + store.set_state("backfill_complete", "no") + + assert music_curator.cold_report(store, NOW) == [] + + +def test_the_cold_report_writes_nothing_to_lidarr(tmp_path): + """Stage four is read-only until the list it produces has been looked at.""" + store = cold_store(tmp_path, played=[("Played Band", "Hit")]) + before = store.scalar("SELECT COUNT(*) FROM lidarr_album WHERE monitored = 1") + + music_curator.cold_report(store, NOW) + + assert store.scalar("SELECT COUNT(*) FROM lidarr_album WHERE monitored = 1") == before + + +def test_human_bytes_reads_at_a_glance(): + assert music_curator.human_bytes(0) == "0.0 B" + assert music_curator.human_bytes(1536) == "1.5 KiB" + assert music_curator.human_bytes(3 * 1024**3) == "3.0 GiB" -- 2.54.0 From 35d5e98642e454085bcc6a70c6fa284ae809c45f Mon Sep 17 00:00:00 2001 From: Emma Thorpe Date: Tue, 25 Aug 2026 10:43:43 +0100 Subject: [PATCH 2/2] fix: write playlists as .m3u8 so Rockbox reads them as UTF-8 Rockbox's is_m3u8_name() treats every playlist extension as UTF-8 except an explicit ".m3u", which it instead decodes through the user's configured codepage: /* Default to M3U8 unless explicitly told otherwise. */ return (!dot || strcasecmp(dot, ".m3u") != 0); The one extension being used was therefore the only one that mangles accented filenames, and this library holds Motley Crue, Beyonce and Sigur Ros. Renaming the output is the whole fix. No byte order mark is written. One would promote a .m3u file to UTF-8 as well, but it is unnecessary at this extension and upsets players that do not expect to find one. Kept with the pruning change rather than raised separately, because the rename depends on it: without pruning, seventeen dead .m3u files would sit on the device for ever, and every one of them full of paths that still resolve. --- README.md | 8 +++- music_curator.py | 18 +++++-- tests/test_music_curator.py | 96 +++++++++++++++++++++++++++---------- 3 files changed, 91 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index ec8b2e3..a735239 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,13 @@ overstates the problem and would over-block the cull. ## Playlists -Written into `/_playlists/` as extended M3U, rebuilt every pass. Six +Written into `/_playlists/` as extended M3U, rebuilt every pass. + +The extension is **`.m3u8`**, not `.m3u`. Rockbox's `is_m3u8_name()` treats +every extension as UTF-8 *except* an explicit `.m3u`, which it decodes through +the user's configured codepage instead — so a plain `.m3u` mangles every +accented filename. No byte order mark is written: Rockbox does not need one at +this extension, and a BOM upsets players that do not expect it. Six rules, capped at `--playlist-limit` tracks each: | Playlist | Rule | diff --git a/music_curator.py b/music_curator.py index 14552e9..dd7696a 100644 --- a/music_curator.py +++ b/music_curator.py @@ -69,6 +69,12 @@ BACKOFF_CEILING_SECONDS = 60.0 MIRROR_SUFFIX = ".mp3" PLAYLIST_DIRECTORY = "_playlists" +# .m3u8, not .m3u. Rockbox's is_m3u8_name() treats every extension as UTF-8 +# except an explicit ".m3u", which it decodes through the user's configured +# codepage instead -- so a plain .m3u mangles every accented filename, and this +# library holds Mötley Crüe, Beyoncé and Sigur Rós. +PLAYLIST_SUFFIX = ".m3u8" + # Tags are re-fetched this often. They move slowly, and the first pass over a # library already costs one request per artist. TAG_REFRESH_SECONDS = 90 * 86400 @@ -1552,8 +1558,10 @@ def build_vibe_playlists(store, vibes, mirror_root, library_root, limit, now): continue entries.append({**dict(row), "mirror": mirror}) - write_playlist(directory / f"{vibe['name']}.m3u", entries, Path(mirror_root)) - produced.append(f"{vibe['name']}.m3u") + write_playlist( + directory / f"{vibe['name']}{PLAYLIST_SUFFIX}", entries, Path(mirror_root) + ) + produced.append(f"{vibe['name']}{PLAYLIST_SUFFIX}") total += len(entries) logger.info("playlist %-20s %4d tracks -- by tag", vibe["name"], len(entries)) @@ -1641,7 +1649,7 @@ def write_playlist(path, entries, reference=None): if fresh and reference is not None: match_ownership(path.parent, reference) - handle, temporary = tempfile.mkstemp(dir=path.parent, suffix=".m3u.part") + handle, temporary = tempfile.mkstemp(dir=path.parent, suffix=".m3u8.part") os.close(handle) temporary = Path(temporary) try: @@ -1702,8 +1710,8 @@ def build_playlists(store, mirror_root, library_root, limit, now): missing += 1 continue entries.append({**dict(row), "mirror": mirror}) - write_playlist(directory / f"{name}.m3u", entries, Path(mirror_root)) - produced.append(f"{name}.m3u") + write_playlist(directory / f"{name}{PLAYLIST_SUFFIX}", entries, Path(mirror_root)) + produced.append(f"{name}{PLAYLIST_SUFFIX}") total += len(entries) logger.info("playlist %-20s %4d tracks -- %s", name, len(entries), description) diff --git a/tests/test_music_curator.py b/tests/test_music_curator.py index 969784c..788ff77 100644 --- a/tests/test_music_curator.py +++ b/tests/test_music_curator.py @@ -875,23 +875,23 @@ def test_playlists_are_written_into_the_mirror(tmp_path): music_curator.build_playlists(store, mirror, str(source), 100, NOW) - written = sorted(p.name for p in (mirror / "_playlists").glob("*.m3u")) + written = sorted(p.name for p in (mirror / "_playlists").glob("*.m3u8")) assert written == [ - "all-time.m3u", - "deep-cuts.m3u", - "heavy-rotation.m3u", - "neglected.m3u", - "unheard-favourites.m3u", - "unheard.m3u", + "all-time.m3u8", + "deep-cuts.m3u8", + "heavy-rotation.m3u8", + "neglected.m3u8", + "unheard-favourites.m3u8", + "unheard.m3u8", ] - played = (mirror / "_playlists" / "all-time.m3u").read_text().splitlines() + played = (mirror / "_playlists" / "all-time.m3u8").read_text().splitlines() assert played[0] == "#EXTM3U" assert played[1].startswith("#EXTINF:") assert "Played Band - Hit" in played[1] # Relative to the playlist file, so the same file works from any mount. assert played[2] == "../Played Band/Known/Hit.mp3" - assert (mirror / "_playlists" / "all-time.m3u").parent.joinpath(played[2]).resolve().is_file() + assert (mirror / "_playlists" / "all-time.m3u8").parent.joinpath(played[2]).resolve().is_file() def test_an_unplayed_track_lands_in_the_unheard_playlist(tmp_path): @@ -905,7 +905,7 @@ def test_an_unplayed_track_lands_in_the_unheard_playlist(tmp_path): music_curator.build_playlists(store, mirror, str(source), 100, NOW) - unheard = (mirror / "_playlists" / "unheard.m3u").read_text() + unheard = (mirror / "_playlists" / "unheard.m3u8").read_text() assert "Never Heard" in unheard assert "Album Track" in unheard # The one thing that was played must not be in it. @@ -929,7 +929,7 @@ def test_a_track_with_no_mirror_file_is_left_out(tmp_path): assert total == 0 assert produced, "the playlists are still written, they are simply empty" - assert (mirror / "_playlists" / "all-time.m3u").read_text() == "#EXTM3U\n" + assert (mirror / "_playlists" / "all-time.m3u8").read_text() == "#EXTM3U\n" def test_the_library_root_is_derived_from_the_artist_folders(tmp_path): @@ -952,7 +952,7 @@ def test_the_playlist_limit_is_honoured(tmp_path): music_curator.build_playlists(store, mirror, str(source), 1, NOW) - unheard = (mirror / "_playlists" / "unheard.m3u").read_text().splitlines() + unheard = (mirror / "_playlists" / "unheard.m3u8").read_text().splitlines() assert len([line for line in unheard if line.startswith("#EXTINF")]) == 1 @@ -968,7 +968,7 @@ def test_the_rotation_moves_weekly_not_every_pass(tmp_path): def unheard_at(when): music_curator.build_playlists(store, mirror, str(source), 100, when) - return (mirror / "_playlists" / "unheard.m3u").read_text() + return (mirror / "_playlists" / "unheard.m3u8").read_text() same_week = unheard_at(NOW), unheard_at(NOW + 3600) assert same_week[0] == same_week[1] @@ -984,7 +984,7 @@ def test_playlists_are_group_readable(tmp_path): music_curator.build_playlists(store, mirror, str(source), 100, NOW) - for playlist in (mirror / "_playlists").glob("*.m3u"): + for playlist in (mirror / "_playlists").glob("*.m3u8"): assert playlist.stat().st_mode & stat.S_IRGRP, playlist @@ -1061,7 +1061,7 @@ def test_a_vibe_selects_by_tag(tmp_path): music_curator.build_vibe_playlists(store, vibes, mirror, str(source), 100, NOW) - written = (mirror / "_playlists" / "screamo.m3u").read_text() + written = (mirror / "_playlists" / "screamo.m3u8").read_text() assert "Played Band" in written assert "Silent Band" not in written @@ -1075,7 +1075,7 @@ def test_a_weakly_tagged_artist_is_below_the_threshold(tmp_path): music_curator.build_vibe_playlists(store, vibes, mirror, str(source), 100, NOW) - assert (mirror / "_playlists" / "screamo.m3u").read_text() == "#EXTM3U\n" + assert (mirror / "_playlists" / "screamo.m3u8").read_text() == "#EXTM3U\n" def test_a_vibe_can_be_restricted_by_release_year(tmp_path): @@ -1090,12 +1090,12 @@ def test_a_vibe_can_be_restricted_by_release_year(tmp_path): # FakeLidarr dates every album 2019, so neither window should catch it. music_curator.build_vibe_playlists(store, inside, mirror, str(source), 100, NOW) music_curator.build_vibe_playlists(store, outside, mirror, str(source), 100, NOW) - assert (mirror / "_playlists" / "eighties.m3u").read_text() == "#EXTM3U\n" - assert (mirror / "_playlists" / "nineties.m3u").read_text() == "#EXTM3U\n" + assert (mirror / "_playlists" / "eighties.m3u8").read_text() == "#EXTM3U\n" + assert (mirror / "_playlists" / "nineties.m3u8").read_text() == "#EXTM3U\n" modern = [{"name": "modern", "tags": ["synthpop"], "years": [2000, 2030]}] music_curator.build_vibe_playlists(store, modern, mirror, str(source), 100, NOW) - assert "Played Band" in (mirror / "_playlists" / "modern.m3u").read_text() + assert "Played Band" in (mirror / "_playlists" / "modern.m3u8").read_text() def test_the_built_in_vibes_are_all_usable_filenames(): @@ -1281,7 +1281,7 @@ def test_an_excluded_tag_drops_the_artist(tmp_path): music_curator.build_vibe_playlists(store, vibes, mirror, str(source), 100, NOW) - written = (mirror / "_playlists" / "eighties.m3u").read_text() + written = (mirror / "_playlists" / "eighties.m3u8").read_text() assert "Silent Band" in written assert "Played Band" not in written @@ -1353,15 +1353,15 @@ def test_a_renamed_mood_does_not_leave_its_old_playlist_behind(tmp_path): directory = mirror / "_playlists" directory.mkdir(parents=True, exist_ok=True) (directory / "high-energy-rock.m3u").write_text("#EXTM3U\n") - (directory / "screamo.m3u").write_text("#EXTM3U\n") + (directory / "screamo.m3u8").write_text("#EXTM3U\n") store.set_state( - "playlist_files", json.dumps(["high-energy-rock.m3u", "screamo.m3u"]) + "playlist_files", json.dumps(["high-energy-rock.m3u", "screamo.m3u8"]) ) - music_curator.prune_playlists(directory, ["screamo.m3u"], store) + music_curator.prune_playlists(directory, ["screamo.m3u8"], store) assert not (directory / "high-energy-rock.m3u").exists() - assert (directory / "screamo.m3u").exists() + assert (directory / "screamo.m3u8").exists() def test_pruning_leaves_a_playlist_it_never_wrote(tmp_path): @@ -1371,7 +1371,7 @@ def test_pruning_leaves_a_playlist_it_never_wrote(tmp_path): directory = mirror / "_playlists" directory.mkdir(parents=True, exist_ok=True) (directory / "lyras-own-mix.m3u").write_text("#EXTM3U\n") - store.set_state("playlist_files", json.dumps(["screamo.m3u"])) + store.set_state("playlist_files", json.dumps(["screamo.m3u8"])) music_curator.prune_playlists(directory, [], store) @@ -1458,3 +1458,49 @@ def test_human_bytes_reads_at_a_glance(): assert music_curator.human_bytes(0) == "0.0 B" assert music_curator.human_bytes(1536) == "1.5 KiB" assert music_curator.human_bytes(3 * 1024**3) == "3.0 GiB" + + +def test_playlists_are_written_as_m3u8(tmp_path): + """Rockbox's is_m3u8_name() treats every extension as UTF-8 except an + explicit ".m3u", which it decodes through the configured codepage instead. + A library with accented names needs the other extension.""" + api, source, mirror = playlist_library(tmp_path) + store = store_at(tmp_path) + music_curator.index_library( + music_curator.Lidarr("http://lidarr", "key", transport=api), store + ) + music_curator.match_library(store) + + music_curator.build_playlists(store, mirror, str(source), 100, NOW) + + written = sorted(p.suffix for p in (mirror / "_playlists").iterdir()) + assert set(written) == {".m3u8"} + + +def test_playlists_are_written_as_utf8(tmp_path): + playlist = tmp_path / "_playlists" / "accents.m3u8" + music_curator.write_playlist( + playlist, + [{"artist": "Mötley Crüe", "title": "Kickstart My Heart", + "duration": 283000, "mirror": tmp_path / "x.mp3"}], + ) + + # Decodes as UTF-8, and carries no BOM: Rockbox does not need one at this + # extension, and a BOM confuses players that do not expect it. + raw = playlist.read_bytes() + assert not raw.startswith(b"\xef\xbb\xbf") + assert "Mötley Crüe" in raw.decode("utf-8") + + +def test_the_old_m3u_playlists_are_pruned_after_the_rename(tmp_path): + """Without this the seventeen dead .m3u files stay on the device for ever.""" + api, source, mirror = playlist_library(tmp_path) + store = store_at(tmp_path) + directory = mirror / "_playlists" + directory.mkdir(parents=True, exist_ok=True) + (directory / "screamo.m3u").write_text("#EXTM3U\n") + store.set_state("playlist_files", json.dumps(["screamo.m3u"])) + + music_curator.prune_playlists(directory, ["screamo.m3u8"], store) + + assert not (directory / "screamo.m3u").exists() -- 2.54.0