Embed highest-resolution album art across all art paths
User report: embedded album art came out ~600x600 while the cover.jpg in
the folder was high-res. The cover.jpg path upgraded the source CDN URL
to its highest resolution, but the tag-embed path fetched the raw URL —
so iTunes art embedded at its 600x600 default, Spotify at 640, Deezer at
1000. The "Write Tags to File" retag path had the same gap (Deezer-only
upgrade), and MusicBrainz art was worse still: every Cover Art Archive
URL is built as the /front-250 thumbnail, so MB-sourced downloads
embedded 250x250.
Factor the resolution upgrade + fetch into two shared helpers in
core/metadata/artwork.py and route every art path through them:
_upgrade_art_url(url) — bump to the source's highest resolution:
- Spotify (i.scdn.co) -> original master (~2000px+)
- iTunes (mzstatic.com) -> 3000x3000
- Deezer (dzcdn) -> 1900x1900
- Cover Art Archive -> /front original (was /front-250)
_fetch_art_bytes(url) — upgrade, fetch, and fall back once to the
original size if the CDN refuses the larger one (non-regressive).
Now consistent across: embed-into-tags (post-process), folder cover.jpg
(post-process), and the enhanced-library "Write Tags to File" retag flow.
The YouTube path already upgraded via Album.from_spotify_album, unchanged.
De-duplicates the per-source upgrade code that was copied across sites
and drops the now-unused urllib import from tag_writer.
Not covered (follow-up): Last.fm / Amazon / Tidal / Qobuz have no
explicit upgrade yet — some already serve full-res, others may hand over
a capped size that passes through unchanged.
Tests: new tests/metadata/test_artwork_resolution.py pins every upgrade
(Spotify 300/640->master, iTunes 100/600->3000, Deezer->1900, CAA
thumbnail->original, unrecognized/empty unchanged) and the fetch
fallback. Updated the two tag_writer fallback tests to patch the network
at its new home in artwork.
This commit is contained in:
parent
ff974c0b5c
commit
c9ad4f496f
4 changed files with 254 additions and 99 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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],
|
||||
|
|
|
|||
158
tests/metadata/test_artwork_resolution.py
Normal file
158
tests/metadata/test_artwork_resolution.py
Normal file
|
|
@ -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)
|
||||
|
|
@ -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]
|
||||
|
|
|
|||
Loading…
Reference in a new issue