From ebab03b700560a89f96945dc1930f060258330ef Mon Sep 17 00:00:00 2001 From: Martin Bauer Date: Sun, 20 Sep 2026 16:57:00 +0200 Subject: [PATCH] Cache every cover, not only the ones pulled out of tags The downscaling landed and the device kept serving 1920px art, because _cover_for has two branches and only one of them went through the cache. An album with a cover.jpg already sitting beside its audio - which most of this library has - had its `cover` point straight at that file, so FileResponse served whatever the internet had given it. The cache directory was full of tidy 640px files that half the albums never used. Both branches now store through the cache, in _cover_for and in _cover_for_episode (sidecar and shared-folder art alike). The undownscaled bytes still come back alongside the path, because colour extraction wants the real thing. That re-points `cover` for every album, but only for albums that are actually rescanned, and the scanner reuses anything whose fingerprint is unchanged - so _INDEX_VERSION goes to 5. An index from 4 is discarded and rebuilt, which is what that mechanism is for and is cheap by design. Two tests asserted `cover == `, which is precisely the behaviour being changed. The episode one was also checking something real - that an episode's own sidecar art beats the show's shared cover - and both covers now live under their own album id, so it reads the colour back out of the stored file instead of comparing paths. Co-Authored-By: Claude Opus 5 --- python-backend/musicmouse/library/cache.py | 15 ++++-- python-backend/musicmouse/library/scanner.py | 18 +++++-- python-backend/tests/test_library.py | 49 ++++++++++++++++++-- 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/python-backend/musicmouse/library/cache.py b/python-backend/musicmouse/library/cache.py index 953e5e6..dae85ad 100644 --- a/python-backend/musicmouse/library/cache.py +++ b/python-backend/musicmouse/library/cache.py @@ -5,8 +5,13 @@ different amounts to produce:: / ├── index.json cheap: tags and structure. Thrown away freely. - ├── covers/.jpg medium: art pulled out of an ID3 APIC frame, - │ downscaled to MAX_COVER_PX on the way in + ├── covers/.jpg medium: the album's art - out of an ID3 APIC + │ frame, or copied from a cover.jpg in the + │ folder - downscaled to MAX_COVER_PX on + │ the way in. An album's `cover` always + │ points here and never into the library, + │ so nothing can serve full-size art by + │ accident. └── analysis/.json expensive: minutes of DSP per track analysis/.beats.json analysis/.curve.json @@ -41,7 +46,11 @@ __all__ = ["Fingerprint", "LibraryCache"] #: worked out - not just when the JSON shape does. A cached entry is reused whenever its #: files are untouched, so otherwise a change to that logic is invisible until somebody #: edits their music folder. -_INDEX_VERSION = 4 +# 5: every album's `cover` now points into this cache, downscaled, including the ones +# that used to point straight at a `cover.jpg` in the library folder. Bumping this is +# what re-points them: an index from version 4 is discarded and the next scan rebuilds +# it, which is cheap by design - see the module docstring. +_INDEX_VERSION = 5 @dataclass(frozen=True, slots=True) diff --git a/python-backend/musicmouse/library/scanner.py b/python-backend/musicmouse/library/scanner.py index d7c188c..7da4050 100644 --- a/python-backend/musicmouse/library/scanner.py +++ b/python-backend/musicmouse/library/scanner.py @@ -119,10 +119,20 @@ def _most_common(values: list[str]) -> str: def _cover_for( folder: Path, paths: list[Path], identifier: str, cache: LibraryCache ) -> tuple[Path | None, bytes | None]: + """The album's art, as a path *into the cache* plus the original bytes. + + Every branch goes through :meth:`LibraryCache.store_cover`, including the one that + finds a ``cover.jpg`` already sitting in the folder. Returning that file directly + would be the obvious thing and was the original behaviour, and it quietly undid the + downscaling: a folder cover is whatever the internet gave it, often 1920px or more, + and the browser then decoded all of it to fill a 185px card. The bytes come back + undownscaled either way, because `colors_from_cover` wants the real art. + """ for name in _COVER_NAMES: candidate = folder / name if candidate.is_file(): - return candidate, candidate.read_bytes() + art = candidate.read_bytes() + return cache.store_cover(identifier, art), art for path in paths[:3]: # Podcast feeds sometimes art only some episodes; a couple of tries is enough. art = _embedded_art(path) @@ -143,14 +153,16 @@ def _cover_for_episode( for extension in _SIDECAR_COVER_EXTENSIONS: candidate = path.with_suffix(extension) if candidate.is_file(): - return candidate, candidate.read_bytes() + art = candidate.read_bytes() + return cache.store_cover(identifier, art), art art = _embedded_art(path) if art is not None: return cache.store_cover(identifier, art), art for name in _COVER_NAMES: candidate = folder / name if candidate.is_file(): - return candidate, candidate.read_bytes() + shared = candidate.read_bytes() + return cache.store_cover(identifier, shared), shared return None, None diff --git a/python-backend/tests/test_library.py b/python-backend/tests/test_library.py index 5e3d303..8125332 100644 --- a/python-backend/tests/test_library.py +++ b/python-backend/tests/test_library.py @@ -247,9 +247,14 @@ async def test_a_cover_file_is_found_and_used(config_dir: Path) -> None: Image.new("RGB", (32, 32), (200, 40, 30)).save(folder / "cover.jpg") album = album_named(await build(config_dir), "Kinderparty Lieder") - assert album.cover == folder / "cover.jpg" + # Into the cache, not at the library file: a folder cover is whatever size it came + # in at, and serving it directly is what used to put 1920px art in a 185px card. + assert album.cover is not None + assert album.cover.parent == config_dir / ".cache" / "covers" + assert album.cover.stem == album.id # A solid red cover has one usable colour; the rest fall back to the synthesised - # palette rather than repeating it. + # palette rather than repeating it - and that still reads the real art, not the + # downscaled copy. assert album.colors[0] != colors_from_id(album.id)[0] @@ -264,9 +269,19 @@ async def test_an_episodes_sidecar_cover_wins_over_the_shows_shared_one(config_d library = await build(config_dir) - assert album_named(library, "Neu").cover == podcast / "20260101 - Neu.jpg" + # Both covers now live in the cache under their own album id, so which file won is + # no longer visible in the path - read the colour back out instead. + def cover_colour(title: str) -> tuple[int, int, int]: + cover = album_named(library, title).cover + assert cover is not None + with Image.open(cover) as image: + return image.convert("RGB").getpixel((0, 0)) # type: ignore[return-value] + + red, green, blue = cover_colour("Neu") + assert red > 150 and green < 90, "the episode's own sidecar art should win" # No sidecar for this one, so it still falls back to the show's shared cover. - assert album_named(library, "Alt").cover == podcast / "cover.jpg" + red, green, blue = cover_colour("Alt") + assert red < 60 and blue > red, "falls back to the show's shared cover" # --------------------------------------------------------------------------- cache @@ -779,6 +794,32 @@ class TestCoverDownscaling: with Image.open(path) as image: assert max(image.size) == MAX_COVER_PX + def test_a_cover_jpg_in_the_library_folder_is_cached_downscaled_too( + self, tmp_path: Path + ) -> None: + """The branch that quietly undid all of this: an album with art already sitting + beside it used to have `cover` point straight at that file, so it was served at + whatever size the internet gave it - often 1920px into a 185px card.""" + from PIL import Image + + from musicmouse.library.cache import MAX_COVER_PX, LibraryCache + from musicmouse.library.scanner import _cover_for + + cache = LibraryCache(tmp_path / "cache") + cache.prepare() + folder = tmp_path / "album" + folder.mkdir() + (folder / "cover.jpg").write_bytes(self._jpeg((1920, 1920))) + + path, art = _cover_for(folder, [], "album1", cache) + + assert path is not None + assert path.parent == cache.covers, "cover must point into the cache, not the library" + with Image.open(path) as image: + assert max(image.size) == MAX_COVER_PX + # The untouched original still comes back, because colour extraction wants it. + assert art == (folder / "cover.jpg").read_bytes() + def test_covers_written_by_an_older_version_are_migrated(self, tmp_path: Path) -> None: """The scanner reuses an unchanged album without re-reading its tags, so a cover stored before the limit existed would otherwise never be rewritten."""