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."""