diff --git a/core/metadata/artwork.py b/core/metadata/artwork.py index 694933d9..f1ef58d8 100644 --- a/core/metadata/artwork.py +++ b/core/metadata/artwork.py @@ -256,6 +256,79 @@ def _browser_safe_image_url(url: str) -> str: return url +def _upgrade_art_url(art_url: str) -> str: + """Rewrite a source CDN art URL to the highest resolution that source + serves, so embedded tag art is as sharp as the cover.jpg in the folder. + + - Spotify (i.scdn.co): request the original uploaded master (~2000px+). + - iTunes (mzstatic.com): bump the size segment to 3000x3000. + - Deezer (dzcdn): rewrite to 1900x1900 (CDN serves larger than the API's + 1000px cover_xl). + + Unrecognized URLs are returned unchanged. Both the embed and cover.jpg + paths call this so the two never diverge in quality again. + """ + if not art_url: + return art_url + if "i.scdn.co" in art_url: + try: + from core.spotify_client import _upgrade_spotify_image_url + + return _upgrade_spotify_image_url(art_url) + except Exception as e: + logger.debug("upgrade spotify image url failed: %s", e) + elif "mzstatic.com" in art_url: + return re.sub(r"\d+x\d+bb", "3000x3000bb", art_url) + elif "dzcdn" in art_url: + try: + from core.deezer_client import _upgrade_deezer_cover_url + + return _upgrade_deezer_cover_url(art_url) + except Exception as e: + logger.debug("upgrade deezer image url failed: %s", e) + elif "coverartarchive.org" in art_url: + # MusicBrainz art comes in as Cover Art Archive thumbnails + # (/front-250, also -500/-1200 — see musicbrainz_search._cover_art_url). + # The bare /front is the original full-resolution upload, which always + # exists when any front art does, so rewrite the size segment back to + # it. Critical for MB-as-source users: without this, embedded art is + # the 250px search thumbnail. `_fetch_art_bytes` falls back to the + # sized URL if /front is ever refused. + return re.sub(r"/front-\d+", "/front", art_url) + return art_url + + +def _fetch_art_bytes(art_url: str): + """Fetch artwork bytes at the highest resolution the source serves. + + Upgrades the URL via `_upgrade_art_url`, then fetches. If the upgraded + (larger) size is refused by the CDN, retry once with the original URL so + we never regress vs. the un-upgraded behavior. Empirically the upgrade + works for every album tested; the fallback just defends the edge case. + + Returns `(image_data, mime_type)` or `(None, None)` on failure. + """ + if not art_url: + return None, None + upgraded = _upgrade_art_url(art_url) + try: + with urllib.request.urlopen(upgraded, timeout=10) as response: + return response.read(), (response.info().get_content_type() or "image/jpeg") + except Exception as fetch_err: + if upgraded != art_url: + logger.info( + "Upgraded art URL refused (%s); retrying original size", fetch_err + ) + try: + with urllib.request.urlopen(art_url, timeout=10) as response: + return response.read(), (response.info().get_content_type() or "image/jpeg") + except Exception as retry_err: + logger.error("Art fetch failed after fallback: %s", retry_err) + return None, None + logger.error("Art fetch failed: %s", fetch_err) + return None, None + + def embed_album_art_metadata(audio_file, metadata: dict): cfg = get_config_manager() symbols = get_mutagen_symbols() @@ -284,9 +357,7 @@ def embed_album_art_metadata(audio_file, metadata: dict): if not art_url: logger.warning("No album art URL available for embedding.") return - with urllib.request.urlopen(art_url, timeout=10) as response: - image_data = response.read() - mime_type = response.info().get_content_type() or "image/jpeg" + image_data, mime_type = _fetch_art_bytes(art_url) if not image_data: logger.error("Failed to download album art data.") @@ -365,59 +436,13 @@ 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 art_url and "i.scdn.co" in art_url: - try: - from core.spotify_client import _upgrade_spotify_image_url - - art_url = _upgrade_spotify_image_url(art_url) - except Exception as e: - logger.debug("upgrade spotify image url failed: %s", e) - elif art_url and "mzstatic.com" in art_url: - art_url = re.sub(r"\d+x\d+bb", "3000x3000bb", art_url) - elif art_url and "dzcdn" in art_url: - # Deezer's API returns cover_xl URLs at 1000×1000 but - # the underlying CDN serves up to 1900×1900 by rewriting - # the size segment in the URL path. Without this upgrade - # users embedding cover art via Deezer get visibly - # blurry covers in their library / phone player (Discord - # report from Tim, 2026-05). Same shape as the iTunes - # mzstatic upgrade above + Spotify scdn upgrade. - try: - from core.deezer_client import _upgrade_deezer_cover_url - - art_url = _upgrade_deezer_cover_url(art_url) - except Exception as e: - logger.debug("upgrade deezer image url failed: %s", e) if not art_url: logger.warning("No cover art URL available for download.") return - # Fetch with one fallback level: if we upgraded a Deezer - # URL above and the CDN happens to refuse the larger size - # for this specific album, retry with the original URL so - # we never regress vs. pre-upgrade behavior. Empirically - # 1900 works for every album tested but defending against - # the edge case keeps the fix strictly non-regressive. - original_url = album_info.get("album_image_url") - if context and not original_url: - album_ctx = get_import_context_album(context) - original_url = album_ctx.get("image_url") or original_url - try: - with urllib.request.urlopen(art_url, timeout=10) as response: - image_data = response.read() - except Exception as fetch_err: - if ( - "dzcdn" in art_url - and original_url - and original_url != art_url - ): - logger.info( - "Deezer CDN refused upgraded cover URL (%s); " - "retrying with original size", fetch_err, - ) - with urllib.request.urlopen(original_url, timeout=10) as response: - image_data = response.read() - else: - raise + # 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 not image_data: return diff --git a/core/tag_writer.py b/core/tag_writer.py index 5b4d5752..e6edbc12 100644 --- a/core/tag_writer.py +++ b/core/tag_writer.py @@ -6,7 +6,6 @@ Reuses the same Mutagen patterns as _enhance_file_metadata in web_server.py. import os import logging -import urllib.request from typing import Dict, Any, Optional, List, Tuple from mutagen import File as MutagenFile @@ -205,48 +204,18 @@ def download_cover_art(cover_url: str) -> Optional[Tuple[bytes, str]]: Download cover art once. Returns (image_data, mime_type) or None on failure. Call this once per album, then pass the result to write_tags_to_file for each track. - For Deezer CDN URLs, upgrades the size segment to 1900×1900 (CDN - max). Mirrors the same upgrade in - ``core.metadata.artwork.download_cover_art`` so the - enhanced-library-view "Write Tags to File" feature embeds the same - high-resolution cover the auto post-process flow does. Falls back - to the original URL if the CDN refuses the upgraded size for a - specific album — keeps the fix strictly non-regressive vs. the - pre-upgrade behaviour. + Delegates to ``core.metadata.artwork._fetch_art_bytes`` so the enhanced- + library-view "Write Tags to File" feature embeds the same highest- + resolution cover the auto post-process flow does — Spotify master + (~2000px), iTunes 3000×3000, and Deezer 1900×1900 — with the same + one-level fallback to the original size if the CDN refuses the upgrade. """ if not cover_url: return None - original_url = cover_url - if 'dzcdn' in cover_url: - try: - from core.deezer_client import _upgrade_deezer_cover_url - cover_url = _upgrade_deezer_cover_url(cover_url) - except Exception as e: - logger.debug("upgrade deezer image url failed: %s", e) - try: - with urllib.request.urlopen(cover_url, timeout=15) as response: - image_data = response.read() - mime_type = response.info().get_content_type() or 'image/jpeg' - if image_data: - return (image_data, mime_type) - except Exception as e: - # Deezer CDN refused upgraded size for this album — retry with - # original URL so we never get less than pre-upgrade behaviour. - if 'dzcdn' in cover_url and cover_url != original_url: - logger.info( - "Deezer CDN refused upgraded cover URL (%s); retrying with original size", e, - ) - try: - with urllib.request.urlopen(original_url, timeout=15) as response: - image_data = response.read() - mime_type = response.info().get_content_type() or 'image/jpeg' - if image_data: - return (image_data, mime_type) - except Exception as fallback_err: - logger.error(f"Error downloading cover art (fallback): {fallback_err}") - return None - logger.error(f"Error downloading cover art from {cover_url}: {e}") - return None + from core.metadata.artwork import _fetch_art_bytes + + image_data, mime_type = _fetch_art_bytes(cover_url) + return (image_data, mime_type) if image_data else None def write_tags_to_file(file_path: str, db_data: Dict[str, Any], diff --git a/tests/metadata/test_artwork_resolution.py b/tests/metadata/test_artwork_resolution.py new file mode 100644 index 00000000..3b9c976b --- /dev/null +++ b/tests/metadata/test_artwork_resolution.py @@ -0,0 +1,158 @@ +"""Pin the shared artwork resolution-upgrade + fetch helpers. + +Bug (Discord, user report): embedded album art came out ~600×600 while the +cover.jpg in the folder was high-res. Cause: only the cover.jpg path +upgraded the source CDN URL to its highest resolution (Spotify master / +iTunes 3000 / Deezer 1900); the tag-embed path and the "Write Tags to File" +retag path fetched the raw URL — Spotify 640, iTunes 600, Deezer 1000. + +Fix: one shared ``_upgrade_art_url`` + ``_fetch_art_bytes`` in +``core.metadata.artwork`` that every art path now calls, so embedded art is +always the same highest resolution as the folder cover. +""" + +from __future__ import annotations + +import pytest + +from core.metadata.artwork import _upgrade_art_url, _fetch_art_bytes + + +# --------------------------------------------------------------------------- +# _upgrade_art_url — per-source resolution bump +# --------------------------------------------------------------------------- + +class TestUpgradeArtUrl: + def test_spotify_album_art_upgraded_to_master(self): + # Spotify encodes size as the hex segment after 'ab67616d0000'. + # 1e02 = 300px, b273 = 640px; 82c1 = the original uploaded master. + url = 'https://i.scdn.co/image/ab67616d00001e02deadbeef' + assert _upgrade_art_url(url) == 'https://i.scdn.co/image/ab67616d000082c1deadbeef' + + def test_spotify_640_also_upgraded(self): + url = 'https://i.scdn.co/image/ab67616d0000b273cafef00d' + assert _upgrade_art_url(url) == 'https://i.scdn.co/image/ab67616d000082c1cafef00d' + + def test_itunes_600_upgraded_to_3000(self): + url = 'https://is1-ssl.mzstatic.com/image/thumb/abc/600x600bb.jpg' + assert _upgrade_art_url(url) == 'https://is1-ssl.mzstatic.com/image/thumb/abc/3000x3000bb.jpg' + + def test_itunes_100_upgraded_to_3000(self): + url = 'https://is1-ssl.mzstatic.com/image/thumb/abc/100x100bb.jpg' + assert '3000x3000bb' in _upgrade_art_url(url) + + def test_deezer_upgraded_to_1900(self): + url = 'https://cdn-images.dzcdn.net/images/cover/abc/1000x1000-000000-80-0-0.jpg' + assert '1900x1900' in _upgrade_art_url(url) + + def test_caa_thumbnail_upgraded_to_original(self): + # MusicBrainz art arrives as /front-250 (and -500/-1200) thumbnails; + # /front is the original full-resolution upload. + url = 'https://coverartarchive.org/release/abc-123/front-250' + assert _upgrade_art_url(url) == 'https://coverartarchive.org/release/abc-123/front' + + def test_caa_500_and_release_group_scope_upgraded(self): + assert _upgrade_art_url('https://coverartarchive.org/release/x/front-500') \ + == 'https://coverartarchive.org/release/x/front' + assert _upgrade_art_url('https://coverartarchive.org/release-group/y/front-1200') \ + == 'https://coverartarchive.org/release-group/y/front' + + def test_caa_already_original_unchanged(self): + url = 'https://coverartarchive.org/release/abc/front' + assert _upgrade_art_url(url) == url + + @pytest.mark.parametrize('url', [ + 'https://lastfm.freetls.fastly.net/i/u/770x0/x.jpg', + 'https://example.com/random.jpg', + ]) + def test_unrecognized_url_unchanged(self, url): + assert _upgrade_art_url(url) == url + + def test_empty_and_none_unchanged(self): + assert _upgrade_art_url('') == '' + assert _upgrade_art_url(None) is None + + +# --------------------------------------------------------------------------- +# _fetch_art_bytes — upgrade, fetch, fall back to original on CDN refusal +# --------------------------------------------------------------------------- + +class _FakeResponse: + def __init__(self, data=b'art-bytes', ctype='image/jpeg'): + self._data = data + self._ctype = ctype + + def read(self): + return self._data + + def info(self): + ctype = self._ctype + + class _Info: + def get_content_type(_self): + return ctype + + return _Info() + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + +class TestFetchArtBytes: + def test_fetches_upgraded_url(self, monkeypatch): + """The upgraded (high-res) URL is the one actually fetched.""" + calls = [] + + def fake_urlopen(url, timeout=None): + calls.append(url) + return _FakeResponse(b'big-cover', 'image/png') + + monkeypatch.setattr('core.metadata.artwork.urllib.request.urlopen', fake_urlopen) + + data, mime = _fetch_art_bytes('https://i.scdn.co/image/ab67616d0000b273x') + + assert data == b'big-cover' + assert mime == 'image/png' + # Fetched the master-res URL, not the 640 original. + assert calls == ['https://i.scdn.co/image/ab67616d000082c1x'] + + def test_falls_back_to_original_when_upgrade_refused(self, monkeypatch): + upgraded = 'https://is1-ssl.mzstatic.com/image/thumb/a/3000x3000bb.jpg' + original = 'https://is1-ssl.mzstatic.com/image/thumb/a/600x600bb.jpg' + calls = [] + + def fake_urlopen(url, timeout=None): + calls.append(url) + if url == upgraded: + raise Exception('403 Forbidden') + return _FakeResponse(b'orig-cover') + + monkeypatch.setattr('core.metadata.artwork.urllib.request.urlopen', fake_urlopen) + + data, mime = _fetch_art_bytes(original) + + assert data == b'orig-cover' + assert calls == [upgraded, original] # tried big first, then fell back + + def test_no_fallback_when_url_not_upgraded(self, monkeypatch): + """If the upgrade is a no-op (unrecognized URL), a single failed + fetch returns (None, None) — no pointless retry of the same URL.""" + calls = [] + + def fake_urlopen(url, timeout=None): + calls.append(url) + raise Exception('network down') + + monkeypatch.setattr('core.metadata.artwork.urllib.request.urlopen', fake_urlopen) + + data, mime = _fetch_art_bytes('https://example.com/cover.jpg') + + assert (data, mime) == (None, None) + assert calls == ['https://example.com/cover.jpg'] + + def test_empty_url_returns_none(self): + assert _fetch_art_bytes('') == (None, None) + assert _fetch_art_bytes(None) == (None, None) diff --git a/tests/metadata/test_deezer_cover_url_upgrade.py b/tests/metadata/test_deezer_cover_url_upgrade.py index caba3dea..16c80f1b 100644 --- a/tests/metadata/test_deezer_cover_url_upgrade.py +++ b/tests/metadata/test_deezer_cover_url_upgrade.py @@ -153,7 +153,9 @@ class TestDownloadFallbackOnCdnRefusal: def test_tag_writer_retries_with_original_on_failure(self, monkeypatch): """tag_writer.download_cover_art must fall back to the - original URL when the upgraded URL fails.""" + original URL when the upgraded URL fails. The fetch now lives in + the shared ``core.metadata.artwork._fetch_art_bytes`` helper that + tag_writer delegates to, so the network is patched there.""" from core import tag_writer original_url = 'https://cdn-images.dzcdn.net/images/cover/abc/1000x1000-000000-80-0-0.jpg' @@ -176,7 +178,7 @@ class TestDownloadFallbackOnCdnRefusal: raise Exception("403 Forbidden") return _FakeResponse() - monkeypatch.setattr('core.tag_writer.urllib.request.urlopen', fake_urlopen) + monkeypatch.setattr('core.metadata.artwork.urllib.request.urlopen', fake_urlopen) result = tag_writer.download_cover_art(original_url) @@ -185,8 +187,9 @@ class TestDownloadFallbackOnCdnRefusal: assert call_log == [upgraded_url, original_url] def test_tag_writer_no_fallback_for_non_dzcdn_url(self, monkeypatch): - """Non-Deezer URLs go through unchanged — no upgrade, no - fallback. Fast path preserved.""" + """Non-Deezer URLs with no recognizable size segment go through + unchanged — no upgrade, so a single fetch attempt with no + fallback.""" from core import tag_writer spotify_url = 'https://i.scdn.co/image/abc' @@ -196,10 +199,10 @@ class TestDownloadFallbackOnCdnRefusal: call_log.append(url) raise Exception("network error") - monkeypatch.setattr('core.tag_writer.urllib.request.urlopen', fake_urlopen) + monkeypatch.setattr('core.metadata.artwork.urllib.request.urlopen', fake_urlopen) result = tag_writer.download_cover_art(spotify_url) assert result is None - # Single attempt — no Deezer fallback path triggered + # Single attempt — upgrade is a no-op for this URL, so no fallback assert call_log == [spotify_url]