Special-edition cover art: prefer the pinned release own cover over the release-group representative
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/<mbid>/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.
This commit is contained in:
parent
9aaafaf341
commit
458658de86
3 changed files with 184 additions and 9 deletions
|
|
@ -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
|
||||
|
|
|
|||
74
core/metadata/caa_art.py
Normal file
74
core/metadata/caa_art.py
Normal file
|
|
@ -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/<mbid>/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/<release_mbid>/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"]
|
||||
85
tests/test_caa_release_art.py
Normal file
85
tests/test_caa_release_art.py
Normal file
|
|
@ -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
|
||||
Loading…
Reference in a new issue