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).
This commit is contained in:
parent
6bc2836f47
commit
b202c176f7
5 changed files with 122 additions and 5 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
54
tests/metadata/test_art_min_size.py
Normal file
54
tests/metadata/test_art_min_size.py
Normal file
|
|
@ -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
|
||||
Loading…
Reference in a new issue