From b202c176f74923d673939c5500de45726fd27403 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Mon, 1 Jun 2026 12:24:51 -0700 Subject: [PATCH] Cover-art sources: skip low-res art (min-resolution guard) + max-res iTunes Follow-up to the preferred-art feature. Real test runs showed a source could win on priority while handing back a small cover: Cover Art Archive is volunteer-uploaded with no size floor, so CAA-first gave a 599x531 (Taylor Swift) and a 600x600 (Kendrick GNX) -- front-1200 only caps the max, so a ~600px upload stays ~600px -- and Deezer/iTunes lower in the order never got a turn. Fix: - Minimum-resolution guard: artwork._min_size_art_validator builds the resolver's validate hook -- it fetches each candidate, caches the bytes (so the winner isn't fetched twice), and accepts art only when its shortest side >= metadata_enhancement.min_art_size (default 1000px; 0 disables). Art that's too small is a miss, so the resolver falls through to the next source instead of winning on priority. Unmeasurable images are accepted (don't over-reject; fallback is still today's art). Wired into both embed_album_art_metadata and download_cover_art. - iTunes art upgraded to /3000x3000bb/ (was the 600px default) so it contributes high-res when it wins. - select_preferred_art_url gains a validate passthrough to the resolver. - config default metadata_enhancement.min_art_size: 1000. Effect: with an order like caa > deezer > spotify > itunes, a ~600px CAA upload is now skipped and Deezer's ~1900px wins -- consistent big art. (Spotify art often maxes ~640px, so it's skipped at the 1000 floor in favor of bigger sources; lower min_art_size to ~640 to allow it.) Tests: tests/metadata/test_art_min_size.py (6 -- incl. the real 599x531 and 600x600 cases, shortest-side logic, unmeasurable-accept, no-bytes-reject, 0-disables) + iTunes max-res upgrade test. Full metadata suite green (617). --- config/settings.py | 7 +++- core/metadata/art_lookup.py | 13 +++++-- core/metadata/artwork.py | 41 ++++++++++++++++++++-- tests/metadata/test_art_lookup.py | 12 +++++++ tests/metadata/test_art_min_size.py | 54 +++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 tests/metadata/test_art_min_size.py diff --git a/config/settings.py b/config/settings.py index a2c0630d..aa10bff5 100644 --- a/config/settings.py +++ b/config/settings.py @@ -609,7 +609,12 @@ class ConfigManager: # Ordered preferred cover-art sources (empty = use the # download's own art, i.e. today's behavior). Resolved + walked # with fallback by core/metadata/art_sources.py. - "album_art_order": [] + "album_art_order": [], + # Minimum cover-art resolution (shortest side, px). A preferred + # source whose art is smaller is skipped so the next source is + # tried — stops a low-res Cover Art Archive upload from winning. + # 0 disables the size gate. + "min_art_size": 1000 }, "musicbrainz": { "embed_tags": True diff --git a/core/metadata/art_lookup.py b/core/metadata/art_lookup.py index 22bb46a3..458dc946 100644 --- a/core/metadata/art_lookup.py +++ b/core/metadata/art_lookup.py @@ -188,7 +188,9 @@ def _itunes_art(artist: str, album: str, metadata: dict) -> Optional[str]: continue got_album, got_artist = _result_album_artist(alb) if _album_matches(artist, album, got_artist, got_album): - return url + # iTunes serves any size via the WxH segment — request the max so + # iTunes contributes high-res art, not the 600px default. + return re.sub(r"/\d+x\d+bb\.", "/3000x3000bb.", url) return None @@ -232,6 +234,7 @@ def select_preferred_art_url( album: Optional[str], metadata: Optional[dict], configured_order, + validate: Optional[Callable[[str, str], bool]] = None, ) -> Optional[str]: """Pick a cover-art URL from the user's configured source order, or None. @@ -240,6 +243,12 @@ def select_preferred_art_url( the cover you'd get today. This is the single entry point the art pipeline calls; it's a no-op (returns ``None`` immediately) unless ``album_art_order`` is an explicit non-empty list, which keeps every existing install untouched. + + ``validate(source, url)`` is an optional gate forwarded to the resolver — the + art pipeline passes one that fetches the candidate and rejects images below a + minimum resolution, so a too-small cover (e.g. a low-res Cover Art Archive + upload) is skipped and the next source is tried instead of winning by + priority alone. """ if not isinstance(configured_order, (list, tuple)) or not configured_order: return None @@ -248,7 +257,7 @@ def select_preferred_art_url( if not order: return None lookup = build_art_lookup(artist or "", album or "", metadata or {}) - url, _src = resolve_cover_art(order, lookup) + url, _src = resolve_cover_art(order, lookup, validate=validate) return url diff --git a/core/metadata/artwork.py b/core/metadata/artwork.py index f9b159fa..9b8386af 100644 --- a/core/metadata/artwork.py +++ b/core/metadata/artwork.py @@ -330,6 +330,35 @@ def _fetch_art_bytes(art_url: str): return None, None +def _min_size_art_validator(min_px): + """Build a ``(validate, cache)`` pair for the preferred-art resolver. + + ``validate(source, url)`` fetches the candidate cover, caches its bytes (so + the winning source isn't fetched twice), and accepts it only when its + shortest side is at least ``min_px``. A too-small cover — e.g. a low-res + Cover Art Archive upload — is rejected so the resolver falls through to the + next source instead of letting it win on priority alone. Images whose + dimensions can't be read are accepted (don't over-reject; the fallback is + still today's art). ``min_px <= 0`` disables the size gate entirely. + """ + cache = {} + + def validate(_source, url): + res = _fetch_art_bytes(url) + cache[url] = res + data = res[0] if res else None + if not data: + return False + if not min_px or min_px <= 0: + return True + dims = get_image_dimensions(data) + if not dims: + return True + return min(dims[0] or 0, dims[1] or 0) >= min_px + + return validate, cache + + def embed_album_art_metadata(audio_file, metadata: dict): cfg = get_config_manager() symbols = get_mutagen_symbols() @@ -348,14 +377,18 @@ def embed_album_art_metadata(audio_file, metadata: dict): art_list_active = isinstance(album_art_order, (list, tuple)) and len(album_art_order) > 0 try: from core.metadata.art_lookup import select_preferred_art_url + _validate, _art_cache = _min_size_art_validator( + cfg.get("metadata_enhancement.min_art_size", 1000)) preferred_url = select_preferred_art_url( metadata.get("album_artist") or metadata.get("artist"), metadata.get("album"), metadata, album_art_order, + validate=_validate, ) if preferred_url: - image_data, mime_type = _fetch_art_bytes(preferred_url) + cached = _art_cache.get(preferred_url) + image_data, mime_type = cached if (cached and cached[0]) else _fetch_art_bytes(preferred_url) except Exception as exc: logger.debug("Preferred art-source selection failed: %s", exc) @@ -443,14 +476,18 @@ def download_cover_art(album_info: dict, target_dir: str, context: dict = None): try: from core.metadata.art_lookup import select_preferred_art_url artist_ctx = get_import_context_artist(context) if context else {} + _validate, _art_cache = _min_size_art_validator( + cfg.get("metadata_enhancement.min_art_size", 1000)) preferred_url = select_preferred_art_url( (artist_ctx or {}).get("name"), album_info.get("album_name"), album_info, cfg.get("metadata_enhancement.album_art_order"), + validate=_validate, ) if preferred_url: - pref_data, _ = _fetch_art_bytes(preferred_url) + cached = _art_cache.get(preferred_url) + pref_data = cached[0] if (cached and cached[0]) else _fetch_art_bytes(preferred_url)[0] if pref_data and len(pref_data) > 1000: image_data = pref_data except Exception as exc: diff --git a/tests/metadata/test_art_lookup.py b/tests/metadata/test_art_lookup.py index 9ca9c1bf..975fe1e9 100644 --- a/tests/metadata/test_art_lookup.py +++ b/tests/metadata/test_art_lookup.py @@ -123,6 +123,18 @@ def test_itunes_art_returns_first_matching_album_image(monkeypatch): assert art_lookup._itunes_art("A", "B", {}) == "http://it/600.jpg" +def test_itunes_art_upgrades_to_max_resolution(monkeypatch): + import core.metadata.registry as registry + client = MagicMock() + client.search_albums.return_value = [ + SimpleNamespace(name="GNX", artists=["Kendrick Lamar"], + image_url="https://is1.mzstatic.com/image/source/600x600bb.jpg")] + monkeypatch.setattr(registry, "get_itunes_client", lambda *a, **k: client) + # The 600x600 default is bumped to the max so iTunes contributes big art. + assert art_lookup._itunes_art("Kendrick Lamar", "GNX", {}) == \ + "https://is1.mzstatic.com/image/source/3000x3000bb.jpg" + + def test_audiodb_art_extracts_thumb(monkeypatch): import core.audiodb_client as adb fake = MagicMock() diff --git a/tests/metadata/test_art_min_size.py b/tests/metadata/test_art_min_size.py new file mode 100644 index 00000000..5b8d9fbd --- /dev/null +++ b/tests/metadata/test_art_min_size.py @@ -0,0 +1,54 @@ +"""Tests for the minimum-resolution guard on preferred cover art. + +A source's art is only accepted when its shortest side meets the threshold, so +a low-res cover (e.g. a small Cover Art Archive upload) is skipped and the +resolver falls through to the next source instead of winning on priority alone. +Reproduces the two real cases that motivated it: Taylor Swift 599x531 and +Kendrick GNX 600x600 — both rejected at the default 1000px floor. +""" + +from __future__ import annotations + +from unittest.mock import patch + +from core.metadata import artwork + + +def _run(min_px, fetch_return, dims): + with patch.object(artwork, "_fetch_art_bytes", return_value=fetch_return), \ + patch.object(artwork, "get_image_dimensions", return_value=dims): + validate, cache = artwork._min_size_art_validator(min_px) + return validate("deezer", "http://x/cover.jpg"), cache + + +def test_rejects_small_square_art(): + ok, cache = _run(1000, (b"img", "image/jpeg"), (600, 600)) # GNX case + assert ok is False + # Bytes are cached even on reject (so a later accepted source reuses fetches). + assert cache["http://x/cover.jpg"] == (b"img", "image/jpeg") + + +def test_rejects_small_non_square_using_shortest_side(): + ok, _ = _run(1000, (b"img", "image/jpeg"), (599, 531)) # Taylor case + assert ok is False + + +def test_accepts_big_art(): + ok, cache = _run(1000, (b"img", "image/jpeg"), (1900, 1900)) + assert ok is True + assert cache["http://x/cover.jpg"] == (b"img", "image/jpeg") + + +def test_accepts_unmeasurable_art_to_avoid_over_rejecting(): + ok, _ = _run(1000, (b"img", "image/jpeg"), None) + assert ok is True + + +def test_rejects_when_fetch_returns_no_bytes(): + ok, _ = _run(1000, (None, None), (4000, 4000)) + assert ok is False + + +def test_zero_threshold_disables_the_gate(): + ok, _ = _run(0, (b"img", "image/jpeg"), (10, 10)) + assert ok is True