From 458658de86ef9323d55155affbd3c976f591634d Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sun, 21 Jun 2026 22:47:09 -0700 Subject: [PATCH] Special-edition cover art: prefer the pinned release own cover over the release-group representative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A MusicBrainz album resolves its art at RELEASE-GROUP scope even for a concrete release (musicbrainz_search _release_to_album -> _cached_art prefers the rg mbid). On the Cover Art Archive a release-group front is a single REPRESENTATIVE cover (CAA picks one release to stand for the group, ~always the standard edition), so a special edition like "Clair Obscur: Expedition 33 (Gustave Edition)" got the standard art baked into cover.jpg + embedded tags at download time. Add core/metadata/caa_art.fetch_release_preferred_art: try the specific release own /release//front first, fall back to the existing release-group/provider URL only when the release has no art of its own (404 -> None). Wire it into both download_cover_art (cover.jpg) and embed_album_art_metadata (tags) so they stay in sync. min_bytes defaults to 0 so the fallback keeps its prior behavior — this can only ever ADD a better edition match, never strip a cover that showed before. Caveat (documented, not fixed here): this only helps when the resolved release IS the right edition. If the upstream match picked the standard release/group, that is the separate canonical-version matching problem. Tests: pure helper (prefers release art, falls back on 404/tiny/exception, no-mbid passes through, nothing-available -> None). 667 metadata/artwork/deezer/hifi tests pass. --- core/metadata/artwork.py | 34 ++++++++++---- core/metadata/caa_art.py | 74 ++++++++++++++++++++++++++++++ tests/test_caa_release_art.py | 85 +++++++++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 core/metadata/caa_art.py create mode 100644 tests/test_caa_release_art.py diff --git a/core/metadata/artwork.py b/core/metadata/artwork.py index 758fe206..0c99fc77 100644 --- a/core/metadata/artwork.py +++ b/core/metadata/artwork.py @@ -438,10 +438,19 @@ def embed_album_art_metadata(audio_file, metadata: dict): if not image_data: art_url = metadata.get("album_art_url") - if not art_url: - logger.warning("No album art URL available for embedding.") + # Prefer the pinned release's OWN cover over a release-group / provider + # representative (usually the standard edition); fall back to art_url + # when the release has no art of its own. Keeps embedded art in sync + # with cover.jpg (same preference + fetch). + from core.metadata.caa_art import fetch_release_preferred_art + image_data, mime_type, used_url = fetch_release_preferred_art( + release_mbid, art_url, fetch_fn=_fetch_art_bytes) + if not image_data: + if not art_url and not release_mbid: + logger.warning("No album art URL available for embedding.") return False - image_data, mime_type = _fetch_art_bytes(art_url) + if release_mbid and used_url and used_url != art_url: + logger.info("Embedding release-specific art (edition match): %s", release_mbid) if not image_data: logger.error("Failed to download album art data.") @@ -560,13 +569,20 @@ def download_cover_art(album_info: dict, target_dir: str, context: dict = None, art_url = images[0].get("url", "") if art_url: logger.info("Using cover art URL from album context") - if not art_url: - logger.warning("No cover art URL available for download.") + # Prefer the pinned release's OWN cover over a release-group / provider + # representative (which is usually the standard edition), falling back + # to art_url when the release has no art of its own. Upgrades to the + # source's highest resolution via _fetch_art_bytes (shared with the + # tag-embed path so cover.jpg and embedded art match). + from core.metadata.caa_art import fetch_release_preferred_art + image_data, _, used_url = fetch_release_preferred_art( + release_mbid, art_url, fetch_fn=_fetch_art_bytes) + if not image_data: + if not art_url and not release_mbid: + logger.warning("No cover art URL available for download.") return - # Upgrade to the source's highest resolution (Spotify master / - # iTunes 3000 / Deezer 1900) with a one-level fallback — shared - # with the tag-embed path so cover.jpg and embedded art match. - image_data, _ = _fetch_art_bytes(art_url) + if release_mbid and used_url and used_url != art_url: + logger.info("Using release-specific cover.jpg (edition match): %s", release_mbid) if not image_data: return diff --git a/core/metadata/caa_art.py b/core/metadata/caa_art.py new file mode 100644 index 00000000..b1a7d949 --- /dev/null +++ b/core/metadata/caa_art.py @@ -0,0 +1,74 @@ +"""Cover Art Archive helper: prefer a pinned release's OWN cover over the +release-group representative. + +On the Cover Art Archive a release-group ``front`` is a single REPRESENTATIVE +cover — CAA designates one release in the group to stand for the whole thing, +which is almost always the standard / most-common edition. So when a download +has pinned a SPECIFIC release (e.g. a "Gustave Edition" the user picked), using +the release-group cover silently swaps in the standard art. + +This helper tries the specific release's own ``/release//front`` first and +only falls back to the caller's existing URL (a release-group representative or a +provider cover) when the release has no art of its own — so it can only ever +*improve* on today's behaviour, never strip a cover that was already showing. + +Pure: the network fetch is injected, so the preference logic is unit-testable. +""" + +from __future__ import annotations + +from typing import Callable, Optional, Tuple + +COVER_ART_ARCHIVE_URL = "https://coverartarchive.org" + + +def caa_front_url(mbid: Optional[str], scope: str = "release", size: int = 1200) -> Optional[str]: + """Build a Cover Art Archive front-cover URL, or None for a falsy mbid. + ``scope`` is 'release' (a specific edition) or 'release-group' (the group's + representative). ``size`` selects the CDN thumbnail (e.g. 250/500/1200); 0 + requests the bare ``/front`` original.""" + if not mbid: + return None + if scope not in ("release", "release-group"): + scope = "release" + suffix = f"-{size}" if size else "" + return f"{COVER_ART_ARCHIVE_URL}/{scope}/{mbid}/front{suffix}" + + +def fetch_release_preferred_art( + release_mbid: Optional[str], + fallback_url: Optional[str], + *, + fetch_fn: Callable[[str], Tuple[Optional[bytes], Optional[str]]], + size: int = 1200, + min_bytes: int = 0, +) -> Tuple[Optional[bytes], Optional[str], Optional[str]]: + """Fetch the best cover, preferring the specific release's own art. + + Tries ``/release//front`` first; on any miss (no such art, + 404, or smaller than ``min_bytes``) falls back to ``fallback_url`` (a + release-group / provider cover). ``fetch_fn(url) -> (bytes|None, mime|None)``; + it is expected to return ``(None, None)`` on a 404, which is how a release + with no art of its own advances to the fallback. ``min_bytes`` defaults to 0 + (accept any non-empty image) to preserve the fallback path's prior behaviour; + callers can raise it to reject placeholder/error images. Returns + ``(bytes|None, mime|None, url_used|None)``. Never raises for a missing cover — + a failed candidate just advances to the next, so coverage never regresses.""" + candidates = [] + release_url = caa_front_url(release_mbid, "release", size) if release_mbid else None + if release_url: + candidates.append(release_url) + if fallback_url and fallback_url not in candidates: + candidates.append(fallback_url) + + for url in candidates: + try: + data, mime = fetch_fn(url) + except Exception: + data, mime = None, None + if data and len(data) > min_bytes: + return data, mime, url + return None, None, None + + +__all__ = ["COVER_ART_ARCHIVE_URL", "caa_front_url", "fetch_release_preferred_art"] diff --git a/tests/test_caa_release_art.py b/tests/test_caa_release_art.py new file mode 100644 index 00000000..29351481 --- /dev/null +++ b/tests/test_caa_release_art.py @@ -0,0 +1,85 @@ +"""Special-edition cover art: prefer the pinned release's OWN cover over the +release-group representative. + +A MusicBrainz release-group 'front' on the Cover Art Archive is a single +representative cover (usually the standard edition), so a special edition (e.g. +"Gustave Edition") was getting the standard art. The download/embed art paths now +try the specific release's own cover first and fall back to the group/provider +URL only when the release has none — so coverage never regresses. +""" + +from __future__ import annotations + +from core.metadata.caa_art import caa_front_url, fetch_release_preferred_art + + +def test_caa_front_url_scopes_and_size(): + assert caa_front_url("abc", "release") == "https://coverartarchive.org/release/abc/front-1200" + assert caa_front_url("rg", "release-group") == "https://coverartarchive.org/release-group/rg/front-1200" + assert caa_front_url("abc", "release", size=250).endswith("/front-250") + assert caa_front_url("abc", "release", size=0).endswith("/abc/front") + assert caa_front_url("", "release") is None + assert caa_front_url(None) is None + # unknown scope coerces to release + assert "/release/x/" in caa_front_url("x", "bogus") + + +def _fetcher(table): + """table: {url: bytes|None}. Returns (bytes, mime) or (None, None).""" + calls = [] + def fetch(url): + calls.append(url) + data = table.get(url) + return (data, "image/jpeg") if data else (None, None) + fetch.calls = calls + return fetch + + +def test_prefers_release_specific_art_over_fallback(): + rel = "https://coverartarchive.org/release/REL/front-1200" + fb = "https://provider.example/standard.jpg" + fetch = _fetcher({rel: b"X" * 5000, fb: b"Y" * 5000}) + data, mime, used = fetch_release_preferred_art("REL", fb, fetch_fn=fetch) + assert data == b"X" * 5000 and used == rel + assert fetch.calls[0] == rel # release tried FIRST + + +def test_falls_back_when_release_has_no_own_art(): + rel = "https://coverartarchive.org/release/REL/front-1200" + fb = "https://provider.example/standard.jpg" + fetch = _fetcher({rel: None, fb: b"Y" * 5000}) # release 404s + data, _, used = fetch_release_preferred_art("REL", fb, fetch_fn=fetch) + assert data == b"Y" * 5000 and used == fb # never regresses: keeps the old cover + assert fetch.calls == [rel, fb] # tried release, then fell back + + +def test_no_release_mbid_uses_fallback_directly(): + fb = "https://provider.example/standard.jpg" + fetch = _fetcher({fb: b"Y" * 5000}) + data, _, used = fetch_release_preferred_art(None, fb, fetch_fn=fetch) + assert data and used == fb + assert fetch.calls == [fb] # no wasted release lookup + + +def test_tiny_image_is_treated_as_a_miss(): + rel = "https://coverartarchive.org/release/REL/front-1200" + fb = "https://provider.example/standard.jpg" + fetch = _fetcher({rel: b"tiny", fb: b"Y" * 5000}) # release art under min_bytes + data, _, used = fetch_release_preferred_art("REL", fb, fetch_fn=fetch, min_bytes=1000) + assert used == fb + + +def test_nothing_available_returns_none(): + fetch = _fetcher({}) + assert fetch_release_preferred_art("REL", None, fetch_fn=fetch) == (None, None, None) + assert fetch_release_preferred_art(None, None, fetch_fn=fetch) == (None, None, None) + + +def test_fetch_exception_is_treated_as_miss_not_fatal(): + fb = "https://provider.example/standard.jpg" + def fetch(url): + if "release" in url: + raise RuntimeError("network boom") + return b"Y" * 5000, "image/jpeg" + data, _, used = fetch_release_preferred_art("REL", fb, fetch_fn=fetch) + assert data == b"Y" * 5000 and used == fb # exception on release → fell back safely