diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index a216db03..5fa038f3 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -9,9 +9,9 @@ on: workflow_dispatch: inputs: version_tag: - description: 'Version tag (e.g. 2.7.3)' + description: 'Version tag (e.g. 2.7.5)' required: true - default: '2.7.3' + default: '2.7.5' jobs: build-and-push: diff --git a/core/deezer_client.py b/core/deezer_client.py index 6f9d96ec..64ca5141 100644 --- a/core/deezer_client.py +++ b/core/deezer_client.py @@ -117,6 +117,48 @@ def _is_full_track_payload(payload: Optional[Dict[str, Any]]) -> bool: return 'track_position' in payload and 'contributors' in payload +def resolve_album_track_positions(session, base_url, album_ids, cache=None, sleep_s=0.2): + """Build ``{str(track_id): track_position}`` for a set of Deezer album ids. + + Deezer PLAYLIST and SEARCH track objects (and even the album object's embedded + ``tracks.data``) omit ``track_position`` — only ``/album//tracks`` and + ``/track/`` carry it. So numbering playlist tracks by their playlist index + silently poisons the real album track number, which then rides onto the + downloaded file's tag. This resolves the authoritative position per album + (cache-first, best-effort — a failed album just isn't in the map).""" + import time as _time + positions: Dict[str, int] = {} + for aid in album_ids: + aid = str(aid) + at_list = None + if cache: + try: + ct = cache.get_entity('deezer', 'album_tracks', aid) + if ct and ct.get('data'): + at_list = ct['data'] + except Exception: # noqa: BLE001 - cache is best-effort + at_list = None + if at_list is None: + try: + if sleep_s: + _time.sleep(sleep_s) # respect Deezer rate limits + r = session.get(f"{base_url}/album/{aid}/tracks", params={'limit': 500}, timeout=10) + if getattr(r, 'ok', False): + at_list = (r.json() or {}).get('data', []) + if cache and at_list is not None: + try: + cache.store_entity('deezer', 'album_tracks', aid, {'data': at_list}) + except Exception as _cache_err: # noqa: BLE001 + logger.debug("album_tracks cache store failed for %s: %s", aid, _cache_err) + except Exception: # noqa: BLE001 - never let metadata resolution break the fetch + at_list = None + for at in (at_list or []): + tp = at.get('track_position') + if at.get('id') and tp: + positions[str(at['id'])] = tp + return positions + + # ==================== Dataclasses (match iTunesClient / SpotifyClient format) ==================== @dataclass @@ -1357,6 +1399,16 @@ class DeezerClient: raw_tracks.extend(page_tracks) + # Real album track positions — playlist tracks don't carry track_position, + # so numbering by playlist index would poison the downloaded file's tag. + album_ids = {str(t.get('album', {}).get('id')) for t in raw_tracks if t.get('album', {}).get('id')} + try: + from core.metadata.cache import get_metadata_cache + _cache = get_metadata_cache() + except Exception: + _cache = None + track_positions = resolve_album_track_positions(self.session, self.BASE_URL, album_ids, _cache) + # Normalize tracks tracks: List[Dict[str, Any]] = [] for i, t in enumerate(raw_tracks, start=1): @@ -1368,7 +1420,8 @@ class DeezerClient: 'artists': [artist_name], 'album': t.get('album', {}).get('title', ''), 'duration_ms': t.get('duration', 0) * 1000, - 'track_number': i, + # REAL album position; the playlist index is a last resort only. + 'track_number': track_positions.get(str(t.get('id'))) or i, }) result = { diff --git a/core/deezer_download_client.py b/core/deezer_download_client.py index 727c7006..165adcfd 100644 --- a/core/deezer_download_client.py +++ b/core/deezer_download_client.py @@ -417,6 +417,12 @@ class DeezerDownloadClient(DownloadSourcePlugin): if aid: album_ids.add(str(aid)) album_release_dates = {} + # Deezer PLAYLIST tracks do NOT carry `track_position` (only `/track/` + # and `/album//tracks` do), so numbering them by their playlist index + # poisons the real album track number — which then rides into the wishlist + # and onto the downloaded file's tag (e.g. 'Apologize' tagged track 1 instead + # of 16). Resolve the REAL position from each album's track list (cache-first). + track_positions: Dict[str, int] = {} # str(track_id) -> album track_position try: from core.metadata.cache import get_metadata_cache cache = get_metadata_cache() @@ -429,24 +435,32 @@ class DeezerDownloadClient(DownloadSourcePlugin): cached = cache.get_entity('deezer', 'album', aid) if cached and cached.get('release_date'): album_release_dates[aid] = cached['release_date'] - continue except Exception as e: logger.debug("cache get_entity album release_date: %s", e) # Cache miss — fetch from API - try: - time.sleep(0.3) # Respect rate limits - a_resp = self._session.get(f'https://api.deezer.com/album/{aid}', timeout=10) - if a_resp.ok: - a_data = a_resp.json() - album_release_dates[aid] = a_data.get('release_date', '') - # Store in metadata cache for future use - if cache: - try: - cache.store_entity('deezer', 'album', aid, a_data) - except Exception as e: - logger.debug("cache store_entity album release_date: %s", e) - except Exception as e: - logger.debug("fetch deezer album release_date %s: %s", aid, e) + if aid not in album_release_dates: + try: + time.sleep(0.3) # Respect rate limits + a_resp = self._session.get(f'https://api.deezer.com/album/{aid}', timeout=10) + if a_resp.ok: + a_data = a_resp.json() + album_release_dates[aid] = a_data.get('release_date', '') + # Store in metadata cache for future use + if cache: + try: + cache.store_entity('deezer', 'album', aid, a_data) + except Exception as e: + logger.debug("cache store_entity album release_date: %s", e) + except Exception as e: + logger.debug("fetch deezer album release_date %s: %s", aid, e) + # Real album track positions (separate endpoint — playlist tracks AND the + # album object's embedded tracks both omit track_position). Cache-first. + try: + from core.deezer_client import resolve_album_track_positions + track_positions = resolve_album_track_positions( + self._session, 'https://api.deezer.com', album_ids, cache) + except Exception as e: + logger.debug("resolve deezer album track positions: %s", e) tracks = [] for i, t in enumerate(raw_tracks, start=1): @@ -467,7 +481,9 @@ class DeezerDownloadClient(DownloadSourcePlugin): 'id': album_id, }, 'duration_ms': t.get('duration', 0) * 1000, - 'track_number': i, + # REAL album position (resolved above); the playlist index is a last + # resort only when the album lookup failed, never the default. + 'track_number': track_positions.get(str(t.get('id'))) or t.get('track_position') or i, }) return { diff --git a/core/discovery/sync.py b/core/discovery/sync.py index aef0acaf..cee281d6 100644 --- a/core/discovery/sync.py +++ b/core/discovery/sync.py @@ -173,6 +173,32 @@ async def _database_only_find_track(spotify_track, candidate_pool=None): logger.debug("sync match cache fast-path failed: %s", e) # --- End cache fast-path --- + # Durable manual library match (#787) — survives a library rescan (the + # sync_match_cache above does not), so a user's Find & Add pairing keeps + # sticking across auto-syncs instead of being re-matched from scratch (#895 + # follow-up). Self-heals a stale library id via the stored file path. + if spotify_id: + try: + from core.artists.map import get_current_profile_id + m = db.find_manual_library_match_by_source_track_id( + get_current_profile_id(), str(spotify_id), active_server) + if m: + lib_id = m.get('library_track_id') + dt = db.get_track_by_id(lib_id) if lib_id is not None else None + if not dt and m.get('library_file_path'): + new_id = db.find_track_id_by_file_path(m['library_file_path']) + dt = db.get_track_by_id(new_id) if new_id else None + if dt: + class DatabaseTrackDurable: + def __init__(self, db_t): + self.ratingKey = db_t.id + self.title = db_t.title + self.id = db_t.id + logger.debug(f"Durable manual match hit: '{original_title}' → {lib_id}") + return DatabaseTrackDurable(dt), 1.0 + except Exception as e: + logger.debug("durable manual match fast-path failed: %s", e) + # Try each artist for artist in spotify_track.artists: if isinstance(artist, str): diff --git a/core/hifi_client.py b/core/hifi_client.py index a1164e8f..0b380132 100644 --- a/core/hifi_client.py +++ b/core/hifi_client.py @@ -140,6 +140,81 @@ def compute_new_default_pushes(all_defaults, offered, legacy_baseline, existing) return to_add, new_offered +_EXTINF_RE = re.compile(r'#EXTINF:\s*([0-9]+(?:\.[0-9]+)?)') + + +def sum_hls_segment_seconds(playlist_text: str) -> float: + """Total audio seconds an HLS media playlist actually provides — the sum of its + ``#EXTINF`` segment durations. This is the authoritative "how much audio is really + here" signal: a PREVIEW manifest serves only ~30s of segments even though the track + is full-length, so summing EXTINF catches it before we waste the download. Returns + 0.0 when the playlist has no EXTINF lines (master playlists, legacy manifests) — the + caller treats 0 as 'unknown', never as 'preview'.""" + total = 0.0 + for m in _EXTINF_RE.finditer(playlist_text or ''): + try: + total += float(m.group(1)) + except (TypeError, ValueError): + continue + return total + + +def is_short_audio(actual_seconds: float, expected_seconds: float, threshold: float = 0.8) -> bool: + """True when ``actual`` is meaningfully shorter than ``expected`` — i.e. a preview + clip or a truncated/corrupt download. Conservative: returns False whenever either + value is missing/zero (unknown ⇒ never reject), and only trips below ``threshold`` + of the expected length (previews are ~15% of full, so the margin is huge).""" + try: + a, e = float(actual_seconds or 0), float(expected_seconds or 0) + except (TypeError, ValueError): + return False + if a <= 0 or e <= 0: + return False + return a < e * threshold + + +def is_fake_lossless_bitrate(size_bytes, claimed_seconds, sample_rate, bits_per_sample, + channels, min_ratio: float = 0.30) -> bool: + """True when a 'lossless' file's data is FAR too small for its claimed length — the + fingerprint of a ~30s preview whose STREAMINFO/container was faked to the full + duration (so every length header reads 'full' and only the bitrate gives it away). + Real FLAC is ~40-75% of raw PCM; a preview padded to full length implies single-digit + %. Conservative: 0 / bad inputs return False (never reject on unknowns).""" + try: + sz, secs = float(size_bytes or 0), float(claimed_seconds or 0) + sr, bits, ch = int(sample_rate or 0), int(bits_per_sample or 0), int(channels or 0) + except (TypeError, ValueError): + return False + if sz <= 0 or secs <= 0 or sr <= 0 or bits <= 0 or ch <= 0: + return False + return (sz * 8 / secs) < (sr * bits * ch) * min_ratio + + +def parse_ffmpeg_time(stderr_text) -> float: + """The last ``time=HH:MM:SS.xx`` ffmpeg prints while decoding — the REAL decoded + length (immune to a faked container/STREAMINFO duration). 0.0 if not found.""" + last = 0.0 + for m in re.finditer(r'time=(\d+):(\d+):(\d+(?:\.\d+)?)', stderr_text or ''): + last = int(m.group(1)) * 3600 + int(m.group(2)) * 60 + float(m.group(3)) + return last + + +def is_preview_download(real_seconds, reference_seconds, *, is_lossless, size_bytes, + sample_rate, bits_per_sample, channels): + """Is a finished file a preview/truncated fake? Two independent signals, so it fires + even when the fakery declares full length at every layer: + 1. DECODED length far below the reference (the ground truth, when a decoder ran); + 2. for lossless, an impossibly-low implied bitrate (no decoder needed). + Returns ``(is_fake, reason)``.""" + if real_seconds and is_short_audio(real_seconds, reference_seconds): + return True, "decoded %.0fs of %.0fs" % (real_seconds, reference_seconds) + if is_lossless and is_fake_lossless_bitrate(size_bytes, reference_seconds, sample_rate, + bits_per_sample, channels): + kbps = (float(size_bytes) * 8 / reference_seconds / 1000) if reference_seconds else 0 + return True, "%.0fkbps lossless over %.0fs (far too low — a ~30s preview)" % (kbps, reference_seconds) + return False, "" + + # Run the new-default push at most once per process. _pushed_new_defaults = False @@ -662,6 +737,7 @@ class HiFiClient(DownloadSourcePlugin): logger.warning(f"Failed to parse HLS playlist for track {track_id}: {e}") return None + media_text = playlist_text # the playlist that actually carries the EXTINF segments if '#EXT-X-STREAM-INF' in playlist_text and segment_uris: playlist_uri = segment_uris[0] try: @@ -669,6 +745,7 @@ class HiFiClient(DownloadSourcePlugin): variant_resp = self.session.get(playlist_uri, allow_redirects=True, timeout=30) variant_resp.raise_for_status() variant_text = variant_resp.text + media_text = variant_text init_uri, segment_uris = self._parse_hls_playlist(variant_text, playlist_uri) except Exception as e: logger.warning(f"Failed to fetch variant playlist for track {track_id}: {e}") @@ -687,6 +764,9 @@ class HiFiClient(DownloadSourcePlugin): 'extension': q_info['extension'], 'codec': q_info['codec'], 'quality': quality, + # Real audio length the manifest provides (sum of EXTINF) — used to reject + # preview manifests before downloading. 0.0 = unknown (don't reject). + 'manifest_duration': sum_hls_segment_seconds(media_text), } def _get_legacy_track_manifest(self, track_id: int, quality: str = 'lossless') -> Optional[Dict]: @@ -711,15 +791,57 @@ class HiFiClient(DownloadSourcePlugin): 'quality': quality, } + @staticmethod + def _probe_audio_seconds(path) -> float: + """Real decoded audio length of a finished file, via mutagen (already a dep). + 0.0 on any failure — the caller treats 0 as 'unknown' and never rejects on it.""" + try: + from mutagen import File as _MutagenFile + mf = _MutagenFile(str(path)) + info = getattr(mf, 'info', None) if mf is not None else None + if info is not None: + return float(getattr(info, 'length', 0) or 0) + except Exception as _probe_err: # noqa: BLE001 + logger.debug("mutagen audio-length probe failed for %s: %s", path, _probe_err) + return 0.0 + + @staticmethod + def _find_ffmpeg(): + ff = shutil.which('ffmpeg') + if ff: + return ff + cand = Path(__file__).parent.parent / 'tools' / ('ffmpeg.exe' if os.name == 'nt' else 'ffmpeg') + return str(cand) if cand.exists() else None + + def _probe_real_seconds(self, path) -> float: + """REAL decoded audio length via ffmpeg — decodes the actual frames, so it sees + through a faked STREAMINFO/container duration (a 30s preview claiming full + length decodes to 30s). 0.0 if ffmpeg is unavailable or on error.""" + ff = self._find_ffmpeg() + if not ff: + return 0.0 + try: + proc = subprocess.run( + [ff, '-hide_banner', '-nostdin', '-i', str(path), '-map', '0:a:0', '-f', 'null', '-'], + capture_output=True, text=True, timeout=180) + return parse_ffmpeg_time(proc.stderr) + except Exception: + return 0.0 + + @staticmethod + def _flac_props(path): + """(sample_rate, bits_per_sample, channels) for the bitrate sanity check, or None.""" + try: + from mutagen.flac import FLAC + si = FLAC(str(path)).info + return (si.sample_rate, si.bits_per_sample, si.channels) + except Exception: + return None + def _demux_flac(self, input_path: Path, output_path: Path) -> None: - ffmpeg = shutil.which('ffmpeg') + ffmpeg = self._find_ffmpeg() if not ffmpeg: - tools_dir = Path(__file__).parent.parent / 'tools' - ffmpeg_candidate = tools_dir / ('ffmpeg.exe' if os.name == 'nt' else 'ffmpeg') - if ffmpeg_candidate.exists(): - ffmpeg = str(ffmpeg_candidate) - else: - raise RuntimeError('ffmpeg is required to demux FLAC from MP4. Install ffmpeg and retry.') + raise RuntimeError('ffmpeg is required to demux FLAC from MP4. Install ffmpeg and retry.') try: result = subprocess.run( @@ -836,6 +958,15 @@ class HiFiClient(DownloadSourcePlugin): MIN_AUDIO_SIZE = 100 * 1024 + # Expected track length (for the preview/truncation guards). Best-effort: a 0 + # here just disables the duration checks for this track, never rejects. + expected_s = 0.0 + try: + info = self.get_track_info(track_id) or {} + expected_s = float(info.get('duration_s') or 0) + except Exception: + expected_s = 0.0 + for q_key in chain: if self.shutdown_check and self.shutdown_check(): logger.info("Shutdown detected, aborting HiFi download") @@ -852,6 +983,18 @@ class HiFiClient(DownloadSourcePlugin): logger.warning(f"No HLS manifest at quality {q_key}, trying next") continue + # Preview guard #1 (pre-download): a preview manifest serves only ~30s of + # segments for a full-length track. A preview means THIS SOURCE only has a + # preview of the track — lower quality tiers are the SAME preview — so abort + # HiFi entirely and let the orchestrator fall through to the next SOURCE + # (soulseek/youtube/…), rather than landing a lower-tier preview. + manifest_s = float(manifest_info.get('manifest_duration') or 0) + if is_short_audio(manifest_s, expected_s): + logger.warning( + "HiFi has only a PREVIEW of '%s' (manifest %.0fs of %.0fs at %s) — " + "failing HiFi so the next source is tried", display_name, manifest_s, expected_s, q_key) + return None + extension = manifest_info['extension'] safe_name = re.sub(r'[<>:"/\\|?*]', '_', display_name) out_filename = f"{safe_name}.{extension}" @@ -931,6 +1074,31 @@ class HiFiClient(DownloadSourcePlugin): out_path.unlink(missing_ok=True) continue + # Preview guard #2 (post-download): the real catch. HiFi previews fake the + # FULL length in every header — manifest EXTINF, m4a moov, FLAC + # total_samples — so only the DECODED audio (or, for lossless, the + # bitrate) reveals the ~30s truth. Reference = the largest length any + # header claims (so the file's own faked claim becomes the bar its real + # audio must clear); is_preview_download decodes + bitrate-checks. + ref_s = max(expected_s, self._probe_audio_seconds(out_path)) + real_s = self._probe_real_seconds(out_path) + props = self._flac_props(out_path) if is_flac else None + fake, why = is_preview_download( + real_s, ref_s, is_lossless=is_flac, size_bytes=final_size, + sample_rate=(props[0] if props else 0), + bits_per_sample=(props[1] if props else 0), + channels=(props[2] if props else 0)) + if fake: + # A preview at this tier means the SOURCE only has a preview — every + # lower tier is the same 30s clip (and the lossy ones dodge the + # bitrate check). Abort HiFi so the orchestrator tries the next + # SOURCE, instead of cascading down into an accepted lower-tier preview. + logger.warning( + "HiFi has only a PREVIEW of '%s' (%s at %s) — failing HiFi so the " + "next source is tried", display_name, why, q_key) + out_path.unlink(missing_ok=True) + return None + logger.info(f"HiFi download complete ({q_key}): {out_path} " f"({final_size / (1024*1024):.1f} MB)") return str(out_path) 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/core/playlists/item_naming.py b/core/playlists/item_naming.py new file mode 100644 index 00000000..94e56790 --- /dev/null +++ b/core/playlists/item_naming.py @@ -0,0 +1,94 @@ +"""Optional custom naming for the FILES inside an organize-by-playlist folder. + +By default a playlist entry keeps the real library filename (the materialized +folder is a view onto Artist/Album/track.ext). A user can opt into a flat +filename template — e.g. ``$position - $artist - $title`` — so the folder sorts +and plays the way they want (most commonly: in playlist order on a dumb DAP). + +It is a **filename** template, never a path: + - it may NOT contain a path separator (``/`` or ``\\``) — it names the file, + not a folder tree, and + - it MUST contain ``$title`` — so every file has a real, non-empty name. + +Both rules are validated up front (so the Settings UI can reject a bad value +with a reason) AND re-checked at apply time, where an invalid/empty template or +an empty render falls back to the library filename. So a bad value can never +produce a broken name — the worst case is "no change from today". + +Pure logic: no DB, no config, no filesystem. The caller supplies the metadata. +""" + +from __future__ import annotations + +from typing import Optional, Tuple + +from core.imports.paths import sanitize_filename + +# Tokens a user may use in the template (for docs / UI hints). +PLAYLIST_ITEM_TOKENS = ("$position", "$artist", "$album", "$track", "$title") + + +def validate_playlist_item_template(template: Optional[str]) -> Tuple[bool, str]: + """Return ``(ok, reason)``. An empty template is VALID and means "feature off" + (keep the library filename). ``reason`` is '' when ok.""" + t = (template or "").strip() + if not t: + return True, "" # empty == disabled, not an error + if "/" in t or "\\" in t: + return False, ("Playlist file naming can't contain a folder separator " + "( / or \\ ) — it names the file, not a path.") + if "$title" not in t: + return False, "Playlist file naming must include $title so every file has a name." + return True, "" + + +def render_playlist_item_name( + template: Optional[str], + *, + title: str, + artist: str = "", + album: str = "", + track: object = None, + position: object = None, + ext: str = "", + fallback_name: str = "", +) -> str: + """Render ``template`` to a sanitized filename WITH ``ext`` appended. + + Falls back to ``fallback_name`` (the library filename) when the template is + empty/invalid or renders to nothing after sanitizing — so the result is + never broken. ``position`` is used verbatim (the caller pre-pads it for + correct sorting); ``track`` is zero-padded to two digits when numeric.""" + ok, _ = validate_playlist_item_template(template) + t = (template or "").strip() + if not ok or not t: + return fallback_name + + pos_str = "" if position is None else str(position) + if track is None: + trk_str = "" + else: + try: + trk_str = f"{int(track):02d}" + except (TypeError, ValueError): + trk_str = str(track) + + # No token is a prefix of another, so replacement order is irrelevant. + out = t + out = out.replace("$position", pos_str) + out = out.replace("$artist", str(artist or "")) + out = out.replace("$album", str(album or "")) + out = out.replace("$track", trk_str) + out = out.replace("$title", str(title or "")) + + out = sanitize_filename(out).strip() + if not out: + return fallback_name + return out + (ext or "") + + +__all__ = [ + "PLAYLIST_ITEM_TOKENS", + "validate_playlist_item_template", + "render_playlist_item_name", +] diff --git a/core/playlists/materialize.py b/core/playlists/materialize.py index f48a9001..dff26886 100644 --- a/core/playlists/materialize.py +++ b/core/playlists/materialize.py @@ -72,16 +72,25 @@ def playlist_dir_for(playlists_root: str, playlist_name: str) -> str: return candidate -def _desired_entries(playlist_dir: str, real_paths: Sequence[str]) -> "list[tuple[str, str]]": - """Map each real file to a flat destination inside ``playlist_dir``, preserving - the source filename. On a basename collision between two *different* sources, - disambiguate with a numeric suffix rather than silently overwriting.""" +def _desired_entries( + playlist_dir: str, + real_paths: Sequence[str], + dest_names: Optional[Sequence[Optional[str]]] = None, +) -> "list[tuple[str, str]]": + """Map each real file to a flat destination inside ``playlist_dir``. + + By default the source filename is preserved. ``dest_names`` (parallel to + ``real_paths``) lets a caller override the name per entry — e.g. a custom + playlist file-naming template; a falsy override falls back to the source + basename. On a name collision between two *different* sources, disambiguate + with a numeric suffix rather than silently overwriting.""" entries: list[tuple[str, str]] = [] used: dict[str, str] = {} # dest basename -> source real path - for real in real_paths: + for i, real in enumerate(real_paths): if not real: continue - base = os.path.basename(real) + override = dest_names[i] if (dest_names is not None and i < len(dest_names)) else None + base = override or os.path.basename(real) name = base stem, ext = os.path.splitext(base) counter = 1 @@ -156,6 +165,7 @@ def rebuild_playlist_folder( real_paths: Sequence[str], mode: str = DEFAULT_MODE, *, + dest_names: Optional[Sequence[Optional[str]]] = None, prune_stale: bool = True, symlink_fn: Callable[[str, str], None] = os.symlink, copy_fn: Callable[[str, str], object] = shutil.copy2, @@ -163,13 +173,15 @@ def rebuild_playlist_folder( """(Re)build ``playlists_root//`` so it contains exactly one entry per real file in ``real_paths`` — adding missing entries, leaving correct ones untouched, and (when ``prune_stale``) removing entries no longer present. + ``dest_names`` (parallel to ``real_paths``) optionally overrides each entry's + filename (custom playlist naming); falsy entries keep the source basename. Idempotent and safe to re-run any time. Filesystem ops are injectable.""" mode = normalize_mode(mode) pdir = playlist_dir_for(playlists_root, playlist_name) summary = RebuildSummary(playlist_dir=pdir, mode_requested=mode) os.makedirs(pdir, exist_ok=True) - entries = _desired_entries(pdir, real_paths) + entries = _desired_entries(pdir, real_paths, dest_names) keep = {dest for _real, dest in entries} for real, dest in entries: diff --git a/core/playlists/materialize_service.py b/core/playlists/materialize_service.py index c8121b86..c00124eb 100644 --- a/core/playlists/materialize_service.py +++ b/core/playlists/materialize_service.py @@ -155,11 +155,20 @@ def _rebuild_one_from_db(db, config_manager, playlist: dict): source IDs), resolving to disk, and rebuilding WITH prune. Because it's driven by current membership, a track that has LEFT the playlist drops out of the set and its symlink is pruned. Returns ``(playlist_name, RebuildSummary)``.""" + import os as _os + from core.library.path_resolver import resolve_library_file_path + from core.playlists.item_naming import render_playlist_item_name root = docker_resolve_path(config_manager.get("playlists.materialize_path", "./Playlists")) mode = normalize_mode(config_manager.get("playlists.materialize_mode", "symlink")) - real_paths: List[str] = [] + item_template = ((config_manager.get("file_organization.templates", {}) or {}) + .get("playlist_item", "") or "").strip() + + # Resolve owned tracks to real paths IN PLAYLIST ORDER, keeping the metadata + # so an optional custom filename template ($position/$artist/$title/...) can + # be applied. $position is the playlist index, which is exactly this order. + resolved: List[dict] = [] seen = set() for t in (db.get_mirrored_playlist_tracks(playlist["id"]) or []): title = (t.get("track_name") or "").strip() @@ -175,9 +184,32 @@ def _rebuild_one_from_db(db, config_manager, playlist: dict): real = resolve_library_file_path(getattr(db_track, "file_path", None), config_manager=config_manager) if real and real not in seen: seen.add(real) - real_paths.append(real) + resolved.append({ + "real": real, + "title": title, + "artist": artist, + "album": (t.get("album_name") or t.get("album") or "").strip(), + "track": getattr(db_track, "track_number", None), + }) + + real_paths: List[str] = [r["real"] for r in resolved] + + dest_names = None + if item_template: + width = max(2, len(str(len(resolved)))) # zero-pad $position for correct sorting + dest_names = [ + render_playlist_item_name( + item_template, + title=r["title"], artist=r["artist"], album=r["album"], track=r["track"], + position=f"{i:0{width}d}", + ext=_os.path.splitext(r["real"])[1], + fallback_name=_os.path.basename(r["real"]), + ) + for i, r in enumerate(resolved, start=1) + ] + name = playlist.get("name") or "Unnamed Playlist" - return name, rebuild_playlist_folder(root, name, real_paths, mode) # prune_stale=True + return name, rebuild_playlist_folder(root, name, real_paths, mode, dest_names=dest_names) # prune_stale=True def rebuild_organized_playlists_from_db(db, config_manager, *, profile_id: int = 1): diff --git a/core/wishlist/routes.py b/core/wishlist/routes.py index a467676c..c9536828 100644 --- a/core/wishlist/routes.py +++ b/core/wishlist/routes.py @@ -501,6 +501,10 @@ def add_album_track_to_wishlist( source_type=source_type, source_context=enhanced_source_context, profile_id=runtime.profile_id, + # Explicit user click in the album modal — must bypass + clear the + # ignore-list, even if the user previously cancelled this track + # (otherwise the add is silently dropped — carlosjfcasero, #897). + user_initiated=True, ) if success: diff --git a/core/wishlist/service.py b/core/wishlist/service.py index df3be19a..1f76f2d9 100644 --- a/core/wishlist/service.py +++ b/core/wishlist/service.py @@ -100,6 +100,7 @@ class WishlistService: source_type: str = "manual", source_context: Dict[str, Any] = None, profile_id: int = 1, + user_initiated: bool = False, ) -> bool: """ Directly add a track to the wishlist. @@ -110,6 +111,8 @@ class WishlistService: source_type: Source type ('playlist', 'album', 'manual') source_context: Additional context information profile_id: Profile to add to + user_initiated: True for an explicit user add — bypasses + clears the + ignore-list while keeping the real source_type (#874/#897). """ if track_data is None: track_data = spotify_track_data @@ -124,6 +127,7 @@ class WishlistService: source_type=source_type, source_info=source_context or {}, profile_id=profile_id, + user_initiated=user_initiated, ) def add_spotify_track_to_wishlist( diff --git a/database/music_database.py b/database/music_database.py index 8691e8d1..7fb473bd 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -7288,6 +7288,18 @@ class MusicDatabase: variations.append(normalized_name.title()) variations.append(normalized_name) + # Leading-"The" toggle — a leading "The" is noise for artist identity + # ("The Black Eyed Peas" == "Black Eyed Peas"). Without this, a request for + # one variant never fetches a library track filed under the other, so it + # "fails to match" and re-downloads a duplicate. Search BOTH forms; the + # confidence scorer still decides (50/50 title/artist), so this only widens + # the candidate fetch — it can't merge genuinely different artists on its own. + stripped = artist_name.strip() + if stripped.lower().startswith("the ") and stripped[4:].strip(): + variations.append(stripped[4:].strip()) # "The Black Eyed Peas" -> "Black Eyed Peas" + elif stripped: + variations.append("The " + stripped) # "Black Eyed Peas" -> "The Black Eyed Peas" + # Add more aliases here in the future if "korn" in name_lower: if "KoЯn" not in variations: @@ -8913,8 +8925,15 @@ class MusicDatabase: source_info: Dict[str, Any] = None, profile_id: int = 1, track_data: Dict[str, Any] = None, + user_initiated: bool = False, ) -> bool: - """Add a failed track to the wishlist for retry""" + """Add a failed track to the wishlist for retry. + + ``user_initiated`` marks an explicit user add (e.g. the library album + "add to wishlist" modal). Like ``source_type == 'manual'`` it bypasses + the ignore-list gate AND clears any stale ignore — but unlike changing + ``source_type`` it preserves the real provenance ('album'), which the + wishlist categorisation (Albums vs Singles) relies on (#874/#897).""" try: if track_data is not None and spotify_track_data is None: spotify_track_data = track_data @@ -8944,7 +8963,7 @@ class MusicDatabase: # clear any stale ignore so it sticks. Fail-open: any error here # must never block a legitimate wishlist add. try: - if source_type == 'manual': + if source_type == 'manual' or user_initiated: self.remove_from_wishlist_ignore(track_id, profile_id=profile_id) elif self.is_track_ignored(track_id, profile_id=profile_id): logger.info("Skipping wishlist add — track is on the ignore-list (#874): %s", track_id) diff --git a/pr_description.md b/pr_description.md index 365c2dfa..17de1921 100644 --- a/pr_description.md +++ b/pr_description.md @@ -1,37 +1,43 @@ -# soulsync 2.7.4 — `dev` → `main` +# soulsync 2.7.5 — `dev` → `main` -patch release on top of 2.7.3. headline is **re-identify** — re-file an already-imported track under the right release without re-downloading it. +patch release on top of 2.7.4. a fix-heavy cycle — matching & artwork accuracy, the HiFi preview mess — plus a few quality-of-life features (M3U import, per-playlist file naming, ignore-list management). --- ## what's new -### re-identify a track (#889) -filed a track under the wrong release (single vs ep vs album)? there's now a ⇄ button in the library Enhanced view that lets you fix it. search any configured source (tabs, defaults to your active one), see the same song across its single / ep / album with type badges, pick the right one, and soulsync re-files the file you already have under that release — correct year, in-album track number, and art. opt to replace the original entry or keep both. +### matching & metadata accuracy +- **deezer track numbers** — a track grabbed from a deezer playlist/wishlist now gets its REAL in-album track number (e.g. "Apologize" = 16 on *Shock Value*), not the playlist index. deezer's playlist/search results don't carry the position, so we resolve it from the album endpoint. +- **special-edition cover art** — a special edition (e.g. *Clair Obscur: Expedition 33 (Gustave Edition)*) no longer ends up with the standard edition's art. musicbrainz albums were resolving cover art at release-GROUP scope (a single representative cover, ~always the standard), so a pinned release now prefers its OWN cover and only falls back to the group/provider art when the release has none of its own. +- **"The" dedup** — wanting "I Gotta Feeling" by "The Black Eyed Peas" when you own it under "Black Eyed Peas" (or vice-versa) no longer fails to match and re-downloads a duplicate. the matcher now searches both forms across a leading "The"; the confidence scorer still has the final say, so it can't merge genuinely different artists. -built additively over 5 phases (hint store → import seam → multi-source search → modal → button), all riding the existing import pipeline so a no-hint import is byte-identical to before. and it can't lose your file: replace deletes the old entry only *after* the re-import lands, and never if you pick the release it's already in. +### HiFi previews (#895) +HiFi was serving 30-second preview files dressed up as full songs (full length faked in the header). soulsync now rejects them three ways — short preview manifests, faked-header files that decode to ~30s, and a lossless-bitrate sanity check — and ABORTS the HiFi source instead of cascading down into a lower-tier copy of the same preview. (this was also an upstream Tidal-ban outage; the guard means you get a clean fail + fallback instead of a broken file.) -### cleaner libraries & imports -- **#890** — track titles no longer keep the "01 - " prefix from the filename when there's no embedded title tag (which made the real track read as a false "missing"). stripped conservatively so "7 Rings" / "1-800-273-8255" / "1979" are left alone. -- **#891** — a Library Reorganize now sweeps the leftover cover.jpg / .lrc / sidecars from the old folder so it actually empties, plus an opt-in "Remove Residual Files" toggle on the Empty Folder Cleaner for the image-only folders you already have. -- **Sokhi's batch** — same-album songs group under one canonical release id (no more split discographies / mixed cover art); a single can match its parent album; a mid-enrichment crash on an art-less file no longer leaves it untagged; and a sequel digit glued to a CJK title no longer matches the wrong album. +### playlists +- **M3U / M3U8 import (#893)** — the "import from file" tool now reads M3U/M3U8 playlists (the most common file-playlist format, and the one soulsync itself exports). parses extended `#EXTINF` (artist/title/duration) and simple path-only playlists, and round-trips with soulsync's own export. +- **organize-by-playlist file naming** — an opt-in template (`$position - $artist - $title`) renames the files INSIDE each playlist folder so they sort/play the way you want (e.g. in playlist order on a dumb player). filename only — validated to reject "/" and require `$title` — defaults to empty (keep the library filename), and works for both symlink and copy modes. +- **find & add is remembered** — a manual match you set on a synced playlist is no longer forgotten on the next auto-sync. replace-mode re-matched from scratch and ignored your durable pick; the matcher now consults the durable manual-match table, not just the volatile cache a library rescan wipes. -### quality & sources -- **#886** — AAC (.m4a) as an opt-in soulseek quality tier, ranked above mp3 / below flac. off by default; existing profiles unchanged until you enable it. -- **#887** — enrichment on Spotify Free now reads "Running (Spotify Free)" instead of wrongly showing "Not Authenticated". -- **#884** — NZBGet imports from the finished location, not the incomplete "….#NZBID" folder. -- **#885** — setting the timezone to Australia/Sydney no longer makes the cache-maintenance job loop every 5 seconds. +### ignore-list (#897) +- the wishlist ignore-list now has a "🚫 Ignored" button right on the wishlist page — it was buried in a modal most people never opened. +- manually re-adding a previously-cancelled track no longer gets silently blocked by the ignore-list. an explicit user add now bypasses + clears the ignore while keeping the real source type (so the Albums/Singles split is unaffected). -### polish -- the artist-detail header no longer bleeds the blurred artist photo behind it. +### docker / packaging (#899) +the Unraid template pointed its `TemplateURL` / `Icon` at a dead third-party repo — now points at the canonical files in this repo (raw URLs, not the HTML `/blob/` ones), and maps `/app/MusicVideos` so music-video downloads land on a share instead of an anonymous volume. + +--- + +## a brief recap of what came before +2.7.4 was **re-identify** (re-file an imported track under the right release without re-downloading) plus library/import cleanups (#889/#890/#891). 2.7.3 added the Quality Upgrade Finder and the wishlist ignore-list (#874). 2.7.2 brought playlist-folder mirroring + server-playlist M3U export; 2.7.1 added download verification + a review queue (#852); 2.7.0 made multi-user real — per-profile accounts, opt-in login, reverse-proxy support. --- ## tests -strictly additive across the board — every new behavior is opt-in or gated so default flows are unchanged. ~100 new tests this cycle (re-identify seam, title-strip danger cases, the shared residual-file classifier, aac tier, tz scheduler, spotify-free status). full imports / matching / reorganize / auto-import suites green, ruff clean. +additive + gated — every new behavior is opt-in or defaults to today's behavior. new seam/regression tests across deezer track positions, the HiFi preview guards, the "The" dedup, M3U parsing, the ignore-list manual-add bypass, the playlist item-naming template, and the release-scope cover-art helper. relevant suites green; `ruff check .` clean app-wide. ## post-merge -- [ ] tag `v2.7.4` on `main` -- [ ] docker-publish with `version_tag: 2.7.4` +- [ ] tag `v2.7.5` on `main` +- [ ] docker-publish with `version_tag: 2.7.5` - [ ] discord announce (auto-fired by the workflow) -- [ ] reply on #889 / #890 / #891 +- [ ] reply on #893 / #895 / #897 / #899 diff --git a/services/sync_service.py b/services/sync_service.py index b48aa051..172dbd5d 100644 --- a/services/sync_service.py +++ b/services/sync_service.py @@ -599,38 +599,63 @@ class PlaylistSyncService: spotify_id = getattr(spotify_track, 'id', '') or '' active_server = config_manager.get_active_media_server() - # --- Sync match cache fast-path --- + # --- User-confirmed match fast-path (Find & Add / manual match) --- if spotify_id: + cache_db = MusicDatabase() + + def _materialize(server_track_id): + """Turn a stored library track id into the actual server item the + sync needs (DB row for Jellyfin/Navidrome/SoulSync, Plex fetchItem).""" + if server_track_id is None: + return None + dbt = cache_db.get_track_by_id(server_track_id) + if not dbt: + return None + if server_type in ("jellyfin", "navidrome", "soulsync"): + class DbTrackFromCache: + def __init__(self, db_t): + self.ratingKey = db_t.id + self.title = db_t.title + self.id = db_t.id + return DbTrackFromCache(dbt) + try: + at = media_client.server.fetchItem(int(server_track_id)) + return at if (at and hasattr(at, 'ratingKey')) else None + except Exception: + return None + + # 1) Volatile sync_match_cache — fast, but wiped on every library rescan. try: - cache_db = MusicDatabase() cached = cache_db.read_sync_match_cache(spotify_id, active_server) if cached: - server_track_id = cached['server_track_id'] - db_track_check = cache_db.get_track_by_id(server_track_id) - if db_track_check: - if server_type in ("jellyfin", "navidrome", "soulsync"): - class DbTrackFromCache: - def __init__(self, db_t): - self.ratingKey = db_t.id - self.title = db_t.title - self.id = db_t.id - actual_track = DbTrackFromCache(db_track_check) - else: - try: - actual_track = media_client.server.fetchItem(int(server_track_id)) - if not (actual_track and hasattr(actual_track, 'ratingKey')): - actual_track = None - except Exception: - actual_track = None - - if actual_track: - logger.debug(f"Sync cache hit: '{original_title}' → server track {server_track_id}") - return actual_track, cached['confidence'] - - logger.debug(f"Sync cache stale for '{original_title}' — track {server_track_id} gone") + actual_track = _materialize(cached['server_track_id']) + if actual_track: + logger.debug(f"Sync cache hit: '{original_title}' → server track {cached['server_track_id']}") + return actual_track, cached['confidence'] + logger.debug(f"Sync cache stale for '{original_title}' — track {cached['server_track_id']} gone") except Exception as cache_err: logger.debug(f"Sync cache lookup error: {cache_err}") - # --- End cache fast-path --- + + # 2) Durable manual library match (#787) — SURVIVES a rescan (the cache + # above does not). Without this, a user's Find & Add pairing is + # re-matched from scratch on the next auto-sync after a library scan, + # so they have to Find & Add the same track again (#895 follow-up). + # Self-heals a stale library id via the stored file path. + try: + from core.artists.map import get_current_profile_id + m = cache_db.find_manual_library_match_by_source_track_id( + get_current_profile_id(), str(spotify_id), active_server) + if m: + actual_track = _materialize(m.get('library_track_id')) + if not actual_track and m.get('library_file_path'): + new_id = cache_db.find_track_id_by_file_path(m['library_file_path']) + actual_track = _materialize(new_id) + if actual_track: + logger.debug(f"Durable manual match hit: '{original_title}' → {m.get('library_track_id')}") + return actual_track, 1.0 + except Exception as durable_err: + logger.debug(f"Durable manual match lookup error: {durable_err}") + # --- End match fast-path --- # Try each artist (same as modal logic) for artist in spotify_track.artists: diff --git a/templates/soulsync.xml b/templates/soulsync.xml index 0b2964a7..fda29e51 100644 --- a/templates/soulsync.xml +++ b/templates/soulsync.xml @@ -13,8 +13,8 @@ Music discovery and automation platform. Find new music, curate playlists, sync libraries, and integrate with popular streaming services, Soulseek (slskd), and media servers. MediaApp:Music http://[IP]:[PORT:8008] - https://raw.githubusercontent.com/snuffomega/SoulSync_unraid/main/soulsync.xml - https://raw.githubusercontent.com/snuffomega/SoulSync_unraid/main/soulsync.png + https://raw.githubusercontent.com/Nezreka/SoulSync/main/templates/soulsync.xml + https://raw.githubusercontent.com/Nezreka/SoulSync/main/templates/soulsync.png @@ -29,6 +29,7 @@ /mnt/user/appdata/soulsync/data /mnt/user/downloads/ /mnt/user/library/ + /mnt/user/media/music-videos/ 99 100 America/New_York diff --git a/tests/discovery/test_sync_database_only_matcher.py b/tests/discovery/test_sync_database_only_matcher.py index b28b63d2..d987e788 100644 --- a/tests/discovery/test_sync_database_only_matcher.py +++ b/tests/discovery/test_sync_database_only_matcher.py @@ -24,11 +24,13 @@ def test_matcher_signature_accepts_candidate_pool(): def _run(track, **kw): fake_db = MagicMock() fake_db.read_sync_match_cache.return_value = None + fake_db.find_manual_library_match_by_source_track_id.return_value = None fake_db.check_track_exists.return_value = (None, 0.0) fake_cm = MagicMock() fake_cm.get_active_media_server.return_value = "plex" with patch("database.music_database.MusicDatabase", return_value=fake_db), \ - patch("config.settings.config_manager", fake_cm): + patch("config.settings.config_manager", fake_cm), \ + patch("core.artists.map.get_current_profile_id", return_value=1): return asyncio.run(sync_mod._database_only_find_track(track, **kw)) @@ -42,10 +44,62 @@ def test_returns_match_when_db_has_it(): track = SimpleNamespace(name="HUMBLE.", artists=["Kendrick Lamar"], id="sp2") fake_db = MagicMock() fake_db.read_sync_match_cache.return_value = None + fake_db.find_manual_library_match_by_source_track_id.return_value = None fake_db.check_track_exists.return_value = (SimpleNamespace(id="t1", title="HUMBLE."), 0.95) fake_cm = MagicMock() fake_cm.get_active_media_server.return_value = "plex" with patch("database.music_database.MusicDatabase", return_value=fake_db), \ - patch("config.settings.config_manager", fake_cm): + patch("config.settings.config_manager", fake_cm), \ + patch("core.artists.map.get_current_profile_id", return_value=1): match, conf = asyncio.run(sync_mod._database_only_find_track(track, candidate_pool={})) assert conf == 0.95 and match.id == "t1" + + +# ── durable manual match (#787) survives a rescan that wipes sync_match_cache ── +# (#895 follow-up: Find & Add was forgotten on the next auto-sync after a library +# scan, because the matcher only consulted the volatile cache.) + +def _run_with_db(track, fake_db): + fake_cm = MagicMock() + fake_cm.get_active_media_server.return_value = "plex" + with patch("database.music_database.MusicDatabase", return_value=fake_db), \ + patch("config.settings.config_manager", fake_cm), \ + patch("core.artists.map.get_current_profile_id", return_value=1): + return asyncio.run(sync_mod._database_only_find_track(track, candidate_pool={})) + + +def test_durable_match_used_when_volatile_cache_is_empty(): + track = SimpleNamespace(name="Valió la Pena - Salsa Version", artists=["Marc Anthony"], id="sp16") + dt = SimpleNamespace(id="t99", title="Valió la Pena (Salsa Version)") + db = MagicMock() + db.read_sync_match_cache.return_value = None # cache wiped by a rescan + db.check_track_exists.return_value = (None, 0.0) # fuzzy would FAIL + db.find_manual_library_match_by_source_track_id.return_value = { + "library_track_id": "t99", "library_file_path": "/m/x.flac"} + db.get_track_by_id.side_effect = lambda i: dt if str(i) == "t99" else None + match, conf = _run_with_db(track, db) + assert conf == 1.0 and match.id == "t99" # manual pick honored, not re-matched + + +def test_durable_match_self_heals_a_stale_library_id(): + track = SimpleNamespace(name="X", artists=["Y"], id="sp1") + dt = SimpleNamespace(id="newid", title="X") + db = MagicMock() + db.read_sync_match_cache.return_value = None + db.check_track_exists.return_value = (None, 0.0) + db.find_manual_library_match_by_source_track_id.return_value = { + "library_track_id": "staleid", "library_file_path": "/m/x.flac"} + db.get_track_by_id.side_effect = lambda i: dt if str(i) == "newid" else None # stale id misses + db.find_track_id_by_file_path.return_value = "newid" # re-resolve via path + match, conf = _run_with_db(track, db) + assert conf == 1.0 and match.id == "newid" + db.find_track_id_by_file_path.assert_called_once_with("/m/x.flac") + + +def test_no_durable_match_falls_through_to_fuzzy(): + track = SimpleNamespace(name="X", artists=["Y"], id="sp1") + db = MagicMock() + db.read_sync_match_cache.return_value = None + db.find_manual_library_match_by_source_track_id.return_value = None + db.check_track_exists.return_value = (None, 0.0) + assert _run_with_db(track, db) == (None, 0.0) diff --git a/tests/playlists/test_playlist_item_naming.py b/tests/playlists/test_playlist_item_naming.py new file mode 100644 index 00000000..fd9b38ab --- /dev/null +++ b/tests/playlists/test_playlist_item_naming.py @@ -0,0 +1,121 @@ +"""Custom file naming for organize-by-playlist folders. + +The materialized playlist folder used to be stuck with the library filename. +A user can now opt into a flat filename template (e.g. "$position - $artist - +$title"). It's a FILENAME, not a path — validated so it can't make folders or +broken names, and it falls back to the library filename on anything invalid. +""" + +from __future__ import annotations + +import os + +import pytest + +from core.playlists.item_naming import ( + render_playlist_item_name, + validate_playlist_item_template, +) +from core.playlists.materialize import rebuild_playlist_folder + + +# ── validation ────────────────────────────────────────────────────────────── + +def test_empty_template_is_valid_means_off(): + assert validate_playlist_item_template("") == (True, "") + assert validate_playlist_item_template(" ") == (True, "") + assert validate_playlist_item_template(None) == (True, "") + + +def test_slash_is_rejected_no_folder_structure(): + ok, why = validate_playlist_item_template("$artist/$title") + assert ok is False and "separator" in why + ok, why = validate_playlist_item_template("$artist\\$title") + assert ok is False + + +def test_must_contain_title(): + ok, why = validate_playlist_item_template("$position - $artist") + assert ok is False and "$title" in why + + +def test_valid_flat_template_passes(): + assert validate_playlist_item_template("$position - $artist - $title") == (True, "") + + +# ── rendering ─────────────────────────────────────────────────────────────── + +def test_renders_tokens_and_keeps_extension(): + out = render_playlist_item_name( + "$position - $artist - $title", + title="One More Time", artist="Daft Punk", position="01", ext=".flac", + fallback_name="x.flac") + assert out == "01 - Daft Punk - One More Time.flac" + + +def test_track_is_zero_padded_album_is_optional(): + out = render_playlist_item_name( + "$track - $title", title="Genesis", track=5, ext=".mp3", fallback_name="x.mp3") + assert out == "05 - Genesis.mp3" + + +def test_invalid_template_falls_back_to_library_name(): + # slash / missing-title / empty all fall back — never a broken name + for bad in ("$artist/$title", "$artist - $position", ""): + assert render_playlist_item_name( + bad, title="T", artist="A", ext=".flac", fallback_name="orig.flac") == "orig.flac" + + +def test_garbage_title_still_yields_a_safe_name_not_broken(): + # a title made of separators is sanitized to a safe (ugly) name with the + # extension intact — never a broken name and never a path. + out = render_playlist_item_name( + "$title", title="/////", ext=".flac", fallback_name="orig.flac") + assert out.endswith(".flac") and "/" not in out and "\\" not in out + + +def test_rendered_name_can_never_contain_a_separator(): + out = render_playlist_item_name( + "$artist - $title", title="AC/DC Song", artist="AC/DC", ext=".flac", fallback_name="x.flac") + assert "/" not in out and "\\" not in out + + +# ── end-to-end through the real folder builder ────────────────────────────── + +def _touch(p): + os.makedirs(os.path.dirname(p), exist_ok=True) + with open(p, "wb") as f: + f.write(b"\x00") + + +def test_rebuild_uses_dest_names_when_given(tmp_path): + lib = tmp_path / "lib" + a = str(lib / "Artist A" / "05 - Song A.flac") + b = str(lib / "Artist B" / "02 - Song B.flac") + _touch(a); _touch(b) + root = str(tmp_path / "Playlists") + summary = rebuild_playlist_folder( + root, "My Mix", [a, b], "copy", + dest_names=["01 - Artist A - Song A.flac", "02 - Artist B - Song B.flac"]) + got = sorted(os.listdir(summary.playlist_dir)) + assert got == ["01 - Artist A - Song A.flac", "02 - Artist B - Song B.flac"] + + +def test_rebuild_without_dest_names_keeps_basename(tmp_path): + # back-compat: default behavior unchanged + a = str(tmp_path / "lib" / "05 - Song A.flac") + _touch(a) + summary = rebuild_playlist_folder(str(tmp_path / "PL"), "Mix", [a], "copy") + assert os.listdir(summary.playlist_dir) == ["05 - Song A.flac"] + + +def test_rebuild_disambiguates_colliding_dest_names(tmp_path): + # two different sources, same templated name (e.g. template "$title" + dup title) + a = str(tmp_path / "lib" / "a" / "x.flac") + b = str(tmp_path / "lib" / "b" / "y.flac") + _touch(a); _touch(b) + summary = rebuild_playlist_folder( + str(tmp_path / "PL"), "Mix", [a, b], "copy", + dest_names=["Song.flac", "Song.flac"]) + got = sorted(os.listdir(summary.playlist_dir)) + assert got == ["Song (2).flac", "Song.flac"] diff --git a/tests/sync/test_sync_durable_match.py b/tests/sync/test_sync_durable_match.py new file mode 100644 index 00000000..0c0abb4e --- /dev/null +++ b/tests/sync/test_sync_durable_match.py @@ -0,0 +1,57 @@ +"""The REAL playlist sync matcher (PlaylistSyncService._find_track_in_media_server) +must honor a durable Find & Add / manual match when the volatile sync_match_cache +has been wiped by a library rescan — otherwise the manual pick is re-matched from +scratch on the next auto-sync (#895 follow-up). Jellyfin server-type avoids Plex +fetchItem mocking; the durable block is server-agnostic.""" + +from __future__ import annotations + +import asyncio +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from services.sync_service import PlaylistSyncService + + +def _service(): + svc = PlaylistSyncService(spotify_client=MagicMock(), download_orchestrator=MagicMock(), + media_server_engine=MagicMock()) + client = MagicMock(); client.is_connected.return_value = True + svc._get_active_media_client = lambda: (client, "jellyfin") + svc._cancelled = False + return svc + + +def _run(svc, db): + cm = MagicMock(); cm.get_active_media_server.return_value = "jellyfin" + track = SimpleNamespace(name="Valió la Pena - Salsa Version", artists=["Marc Anthony"], id="sp16") + with patch("database.music_database.MusicDatabase", return_value=db), \ + patch("config.settings.config_manager", cm), \ + patch("core.artists.map.get_current_profile_id", return_value=1): + return asyncio.run(svc._find_track_in_media_server(track, candidate_pool={})) + + +def test_durable_match_used_when_volatile_cache_wiped(): + dt = SimpleNamespace(id="t99", title="Valió la Pena (Salsa Version)") + db = MagicMock() + db.read_sync_match_cache.return_value = None # rescan wiped the cache + db.check_track_exists.return_value = (None, 0.0) # fuzzy would FAIL + db.find_manual_library_match_by_source_track_id.return_value = { + "library_track_id": "t99", "library_file_path": "/m/x.flac"} + db.get_track_by_id.side_effect = lambda i: dt if str(i) == "t99" else None + match, conf = _run(svc=_service(), db=db) + assert conf == 1.0 and match.id == "t99" # manual pick honored across the rescan + + +def test_durable_match_self_heals_stale_library_id(): + dt = SimpleNamespace(id="newid", title="X") + db = MagicMock() + db.read_sync_match_cache.return_value = None + db.check_track_exists.return_value = (None, 0.0) + db.find_manual_library_match_by_source_track_id.return_value = { + "library_track_id": "staleid", "library_file_path": "/m/x.flac"} + db.get_track_by_id.side_effect = lambda i: dt if str(i) == "newid" else None # stale id gone + db.find_track_id_by_file_path.return_value = "newid" + match, conf = _run(svc=_service(), db=db) + assert conf == 1.0 and match.id == "newid" + db.find_track_id_by_file_path.assert_called_once_with("/m/x.flac") diff --git a/tests/test_artist_the_prefix_variations.py b/tests/test_artist_the_prefix_variations.py new file mode 100644 index 00000000..29670912 --- /dev/null +++ b/tests/test_artist_the_prefix_variations.py @@ -0,0 +1,90 @@ +"""Leading-"The" duplicate fix. + +A user wanted "I Gotta Feeling" by "The Black Eyed Peas" but owned it under +"Black Eyed Peas" (or vice-versa). The dedup gate (check_track_exists) fetches +candidates via _get_artist_variations(), which had no "The" toggle — so the +owned track was never fetched, the request "failed to match", and a duplicate +was downloaded. The toggle widens the fetch to both forms; the scorer still +decides, so it can't merge genuinely different artists. +""" + +from __future__ import annotations + +from database.music_database import MusicDatabase + + +def _variations(name): + db = object.__new__(MusicDatabase) # no DB / network init needed + return db._get_artist_variations(name) + + +def test_leading_the_is_stripped_to_search_the_bare_form(): + v = _variations("The Black Eyed Peas") + assert "Black Eyed Peas" in v # owned-bare form now gets fetched + assert "The Black Eyed Peas" in v # original kept + + +def test_bare_name_also_searches_the_the_prefixed_form(): + v = _variations("Black Eyed Peas") + assert "The The Black Eyed Peas" not in v # no double-"The" + assert "The Black Eyed Peas" in v # the "The"-variant gets fetched + assert "Black Eyed Peas" in v + + +def test_the_band_named_just_the_never_produces_an_empty_search(): + # "The" alone must not collapse to an empty artist search (which would match + # the entire library). Adding "The The" is harmless — the scorer still gates. + v = _variations("The") + assert "" not in v + assert "The" in v + + +def test_a_leading_the_word_is_required_not_a_mid_word_the(): + # "Theory of a Deadman" starts with "The" but not the WORD "The" — it must not + # be stripped mid-word. (It does still get the harmless "The "-prefixed widen.) + v = _variations("Theory of a Deadman") + assert "ory of a Deadman" not in v # NOT mangled mid-word + assert "Theory of a Deadman" in v + + +def test_the_toggle_lands_the_match_through_the_real_scorer(): + # End-to-end on the confidence scorer: requesting one variant against the + # other owned variant must clear the 0.8 dedup threshold (50/50 title/artist + # → 1.0*0.5 + 0.882*0.5 = 0.94), so it's recognized as already owned. + db = object.__new__(MusicDatabase) + + class _Track: + title = "I Gotta Feeling" + artist_name = "Black Eyed Peas" + track_artist = "Black Eyed Peas" + album = "The E.N.D." + + conf = db._calculate_track_confidence("I Gotta Feeling", "The Black Eyed Peas", _Track()) + assert conf >= 0.8, conf + + # …and the reverse direction too. + class _Track2: + title = "I Gotta Feeling" + artist_name = "The Black Eyed Peas" + track_artist = "The Black Eyed Peas" + album = "The E.N.D." + + conf2 = db._calculate_track_confidence("I Gotta Feeling", "Black Eyed Peas", _Track2()) + assert conf2 >= 0.8, conf2 + + +def test_toggle_does_not_falsely_merge_different_the_artists(): + # "The Police" and "Police" are arguably the same band, but "The Weeknd" vs + # "Weeknd" etc. — the toggle only WIDENS the fetch; the scorer still gates. + # A clearly different artist must not score as a match just because both + # share no "The". (Title differs too — this is the real safety net.) + db = object.__new__(MusicDatabase) + + class _Other: + title = "Some Other Song" + artist_name = "Black Eyed Peas" + track_artist = "Black Eyed Peas" + album = "Whatever" + + conf = db._calculate_track_confidence("I Gotta Feeling", "The Killers", _Other()) + assert conf < 0.8, conf 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 diff --git a/tests/test_deezer_track_positions.py b/tests/test_deezer_track_positions.py new file mode 100644 index 00000000..b0ed79b7 --- /dev/null +++ b/tests/test_deezer_track_positions.py @@ -0,0 +1,60 @@ +"""Deezer playlist tracks must carry the REAL album track_position, not their +playlist index — otherwise the downloaded file is tagged with the wrong track +number (e.g. 'Apologize' from Shock Value tagged track 1 instead of 16).""" + +from __future__ import annotations + +from core.deezer_client import resolve_album_track_positions + + +class _Resp: + def __init__(self, data, ok=True): + self._d, self.ok = data, ok + + def json(self): + return self._d + + +class _Session: + """Fake requests session returning /album//tracks payloads.""" + + def __init__(self, by_album, fail_for=()): + self.by_album, self.fail_for, self.calls = by_album, set(fail_for), [] + + def get(self, url, params=None, timeout=None): + aid = url.rstrip('/').split('/')[-2] # …/album//tracks + self.calls.append(aid) + if aid in self.fail_for: + return _Resp(None, ok=False) + return _Resp({'data': self.by_album.get(aid, [])}) + + +def test_maps_track_id_to_real_album_position(): + sess = _Session({'119606': [ + {'id': 100, 'track_position': 16}, {'id': 101, 'track_position': 2}]}) + pos = resolve_album_track_positions(sess, 'https://api.deezer.com', {'119606'}, sleep_s=0) + assert pos == {'100': 16, '101': 2} # real positions, not 1/2 enumerate + + +def test_cache_first_skips_the_network(): + class _Cache: + def __init__(self): self.stored = {} + def get_entity(self, src, kind, aid): + return {'data': [{'id': 7, 'track_position': 9}]} if kind == 'album_tracks' else None + def store_entity(self, *a, **k): pass + sess = _Session({}) + pos = resolve_album_track_positions(sess, 'https://api.deezer.com', {'42'}, cache=_Cache(), sleep_s=0) + assert pos == {'7': 9} and sess.calls == [] # served from cache, no HTTP + + +def test_failed_album_is_simply_absent_not_fatal(): + sess = _Session({'1': [{'id': 5, 'track_position': 3}]}, fail_for={'2'}) + pos = resolve_album_track_positions(sess, 'https://api.deezer.com', {'1', '2'}, sleep_s=0) + assert pos == {'5': 3} # album 2 failed → just missing + + +def test_zero_position_is_ignored(): + # Deezer sometimes returns 0/None for odd entries — don't poison the map with them + sess = _Session({'1': [{'id': 5, 'track_position': 0}, {'id': 6}, {'id': 7, 'track_position': 4}]}) + pos = resolve_album_track_positions(sess, 'https://api.deezer.com', {'1'}, sleep_s=0) + assert pos == {'7': 4} diff --git a/tests/test_hifi_preview_guard.py b/tests/test_hifi_preview_guard.py new file mode 100644 index 00000000..ed085933 --- /dev/null +++ b/tests/test_hifi_preview_guard.py @@ -0,0 +1,218 @@ +"""HiFi sometimes serves a PREVIEW manifest (~30s of segments) for a full-length +track, which slipped through the old 100KB-only size floor. The duration guards +catch it: a preview manifest is way shorter than the real track length.""" + +from __future__ import annotations + +from core.hifi_client import sum_hls_segment_seconds, is_short_audio + + +_FULL = """#EXTM3U +#EXT-X-VERSION:6 +#EXT-X-MAP:URI="init.mp4" +#EXTINF:10.0, +seg0.mp4 +#EXTINF:10.0, +seg1.mp4 +#EXTINF:9.5, +seg2.mp4 +#EXT-X-ENDLIST +""" + +_PREVIEW = """#EXTM3U +#EXTINF:15.0, +p0.mp4 +#EXTINF:15.0, +p1.mp4 +#EXT-X-ENDLIST +""" + + +def test_sums_extinf_segment_durations(): + assert sum_hls_segment_seconds(_FULL) == 29.5 # 10 + 10 + 9.5 + assert sum_hls_segment_seconds(_PREVIEW) == 30.0 # the preview's true length + + +def test_no_extinf_is_unknown_zero(): + assert sum_hls_segment_seconds("#EXTM3U\nseg.mp4\n") == 0.0 + assert sum_hls_segment_seconds("") == 0.0 + + +def test_preview_is_flagged_short_against_full_track(): + # Save Your Tears ~215s; a 30s preview manifest is obviously short + assert is_short_audio(30.0, 215.0) is True + + +def test_full_length_download_is_not_flagged(): + assert is_short_audio(213.0, 215.0) is False # ~1% trim → fine + assert is_short_audio(215.0, 215.0) is False + + +def test_unknown_durations_never_reject(): + assert is_short_audio(0, 215) is False # couldn't probe → don't reject + assert is_short_audio(30, 0) is False # expected unknown → don't reject + assert is_short_audio(0, 0) is False + + +def test_legitimately_short_track_is_kept(): + # a real 40s interlude: actual ≈ expected → not a preview + assert is_short_audio(40.0, 41.0) is False + + +def test_threshold_boundary(): + assert is_short_audio(79, 100) is True # below 80% + assert is_short_audio(85, 100) is False # above 80% + + +# ── integration: the guards actually wire into _download_sync ──────────────── +import pytest +import core.hifi_client as hc + + +class _Cfg: + """Stub config so _download_sync just takes its defaults (no DB).""" + def get(self, key, default=None): + return default + + +def _bare_client(tmp_path): + c = object.__new__(hc.HiFiClient) # skip __init__ (DB / network) + c.download_path = tmp_path + c._engine = None + c.shutdown_check = None + return c + + +def test_download_sync_skips_preview_manifests_and_never_downloads(tmp_path, monkeypatch): + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 215} # real track length + tiers = [] + c._get_hls_manifest = lambda tid, quality='lossless': ( + tiers.append(quality) or + {'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', + 'manifest_duration': 30.0}) # preview at EVERY tier + c._download_segment_with_retry = lambda url: pytest.fail("downloaded a preview segment!") + + result = c._download_sync('dl1', 12345, 'The Weeknd - Save Your Tears') + assert result is None # → orchestrator falls back + # A preview means the SOURCE only has a preview — every lower tier is the same clip, + # so it must ABORT on the first preview, not cascade down into a lower-tier preview. + assert tiers == ['lossless'] + + +def test_download_sync_proceeds_past_the_gate_for_a_full_manifest(tmp_path, monkeypatch): + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 215} + c._get_hls_manifest = lambda tid, quality='lossless': { + 'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', + 'manifest_duration': 215.0} # full length → must NOT skip + seg_calls = [] + + def _seg(url): + seg_calls.append(url) + raise RuntimeError("stop after the gate") + c._download_segment_with_retry = _seg + + c._download_sync('dl1', 12345, 'x') + assert seg_calls # it got PAST the preview gate to download + + +def test_download_sync_aborts_on_a_faked_full_length_file_no_tier_cascade(tmp_path, monkeypatch): + # The real #895 case: manifest + container claim FULL length, but the finished file + # decodes to 30s. It must abort HiFi (return None) on the first tier — NOT drop to the + # lossy 'high' tier (the same 30s preview, which dodges the bitrate check). + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 215} + tiers = [] + c._get_hls_manifest = lambda tid, quality='lossless': ( + tiers.append(quality) or + {'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', 'manifest_duration': 215.0}) + c._download_segment_with_retry = lambda url: b'\x00' * 200_000 # > MIN_AUDIO_SIZE + c._demux_flac = lambda i, o: o.write_bytes(b'\x00' * 200_000) # produce the 'flac' + c._probe_real_seconds = lambda p: 30.0 # decodes to 30s + c._probe_audio_seconds = lambda p: 215.0 # faked container claim + c._flac_props = lambda p: (44100, 16, 2) + + result = c._download_sync('dl1', 12345, 'The Weeknd - Save Your Tears') + assert result is None and tiers == ['lossless'] # aborted, did NOT try 'high' + + +def test_download_sync_does_not_reject_when_track_length_unknown(tmp_path, monkeypatch): + monkeypatch.setattr(hc, 'config_manager', _Cfg()) + c = _bare_client(tmp_path) + c.get_track_info = lambda tid: {'duration_s': 0} # expected unknown + c._get_hls_manifest = lambda tid, quality='lossless': { + 'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', + 'manifest_duration': 30.0} # short, but expected is unknown + seg_calls = [] + c._download_segment_with_retry = lambda url: (seg_calls.append(url), (_ for _ in ()).throw(RuntimeError("stop")))[0] + + c._download_sync('dl1', 12345, 'x') + assert seg_calls # unknown length → no rejection, proceeds + + +# ── faked-header previews: claim full length everywhere, only ~30s of real audio ── +# (real numbers measured from issue #895's files: every "lossless" FLAC was a 30s +# preview with STREAMINFO total_samples faked to the full length.) +from core.hifi_client import is_fake_lossless_bitrate, is_preview_download, parse_ffmpeg_time + + +def test_real_issue895_files_are_all_flagged_by_bitrate(): + # (size_bytes, claimed_seconds) for the actual files — 16-bit/44.1kHz stereo FLAC. + samples = [ + (4_080_000, 216), # Save Your Tears (151 kbps claimed) + (6_770_000, 150), # I Ain't Worried (362 kbps — the highest, nearest the line) + (2_240_000, 326), # Lose Yourself (55 kbps) + (4_190_000, 285), # The Real Slim Shady + (4_910_000, 170), # APT + ] + for size, secs in samples: + assert is_fake_lossless_bitrate(size, secs, 44100, 16, 2) is True, (size, secs) + + +def test_real_full_lossless_is_not_flagged(): + # a genuine 16/44.1 lossless track is ~700-1100 kbps → well above the floor + assert is_fake_lossless_bitrate(25_000_000, 216, 44100, 16, 2) is False # ~926 kbps + assert is_fake_lossless_bitrate(12_000_000, 216, 44100, 16, 2) is False # ~444 kbps, still real + + +def test_bitrate_check_is_conservative_on_unknowns(): + assert is_fake_lossless_bitrate(0, 216, 44100, 16, 2) is False + assert is_fake_lossless_bitrate(4_080_000, 0, 44100, 16, 2) is False + assert is_fake_lossless_bitrate(4_080_000, 216, 0, 0, 0) is False + + +def test_is_preview_download_decode_path(): + # decoded 30s of a claimed 216s → fake, regardless of bitrate + fake, why = is_preview_download(30.0, 216.0, is_lossless=False, size_bytes=99_000_000, + sample_rate=44100, bits_per_sample=16, channels=2) + assert fake and "decoded 30s of 216s" in why + + +def test_is_preview_download_bitrate_path_when_no_decoder(): + # real_seconds=0 (no ffmpeg) → fall back to the lossless bitrate check + fake, why = is_preview_download(0.0, 216.0, is_lossless=True, size_bytes=4_080_000, + sample_rate=44100, bits_per_sample=16, channels=2) + assert fake and "kbps lossless" in why + + +def test_is_preview_download_passes_a_real_file(): + fake, _ = is_preview_download(214.0, 216.0, is_lossless=True, size_bytes=25_000_000, + sample_rate=44100, bits_per_sample=16, channels=2) + assert fake is False + + +def test_is_preview_download_lossy_no_decoder_is_not_flagged(): + # a lossy tier (mp3/m4a) with no decode info → can't bitrate-check → don't reject + fake, _ = is_preview_download(0.0, 216.0, is_lossless=False, size_bytes=2_000_000, + sample_rate=44100, bits_per_sample=16, channels=2) + assert fake is False + + +def test_parse_ffmpeg_time_reads_the_last_progress_line(): + stderr = "frame= ... time=00:00:12.34 bitrate=...\nframe= ... time=00:00:30.05 bitrate=..." + assert abs(parse_ffmpeg_time(stderr) - 30.05) < 0.01 + assert parse_ffmpeg_time("no time here") == 0.0 diff --git a/tests/test_playlist_materialize_service.py b/tests/test_playlist_materialize_service.py index fbb3332e..17648790 100644 --- a/tests/test_playlist_materialize_service.py +++ b/tests/test_playlist_materialize_service.py @@ -59,8 +59,12 @@ class _RebuildDB: class _Cfg: - def __init__(self, root, mode="symlink"): - self._d = {"playlists.materialize_path": root, "playlists.materialize_mode": mode} + def __init__(self, root, mode="symlink", item_template=""): + self._d = { + "playlists.materialize_path": root, + "playlists.materialize_mode": mode, + "file_organization.templates": {"playlist_item": item_template}, + } def get(self, key, default=None): return self._d.get(key, default) @@ -266,3 +270,30 @@ def test_rebuild_from_db_only_organized_and_owned(tmp_path: Path): assert name == "Mix" and s.linked == 1 # only A owned; Gone skipped assert (tmp_path / "Playlists" / "Mix" / "A.mp3").exists() assert not (tmp_path / "Playlists" / "Off").exists() + + +def test_playlist_item_template_renames_entries(tmp_path: Path): + """The custom-naming opt-in: a configured playlist_item template renames the + files INSIDE the playlist folder (real library file untouched), with $position + coming straight from playlist order.""" + a = (tmp_path / "Music" / "Artist X" / "07 - A.flac") + a.parent.mkdir(parents=True, exist_ok=True) + a.write_bytes(b"audio") + db = _RebuildDB({"A": str(a)}) + cfg = _Cfg(str(tmp_path / "Playlists"), mode="copy", item_template="$position - $title") + results = rebuild_organized_playlists_from_db(db, cfg, profile_id=1) + assert results[0][0] == "Mix" + mix = tmp_path / "Playlists" / "Mix" + assert sorted(p.name for p in mix.iterdir()) == ["01 - A.flac"] # templated, NOT "07 - A.flac" + + +def test_empty_playlist_item_template_keeps_library_filename(tmp_path: Path): + """Back-compat: with no template configured, entries keep the library filename.""" + a = (tmp_path / "Music" / "Artist X" / "07 - A.flac") + a.parent.mkdir(parents=True, exist_ok=True) + a.write_bytes(b"audio") + db = _RebuildDB({"A": str(a)}) + cfg = _Cfg(str(tmp_path / "Playlists"), mode="copy") # item_template="" (default) + rebuild_organized_playlists_from_db(db, cfg, profile_id=1) + mix = tmp_path / "Playlists" / "Mix" + assert sorted(p.name for p in mix.iterdir()) == ["07 - A.flac"] # unchanged diff --git a/tests/wishlist/test_wishlist_ignore.py b/tests/wishlist/test_wishlist_ignore.py index 783511f2..023b6113 100644 --- a/tests/wishlist/test_wishlist_ignore.py +++ b/tests/wishlist/test_wishlist_ignore.py @@ -146,6 +146,26 @@ def test_gate_blocks_auto_readd_but_manual_bypasses_and_clears(db): assert db.is_track_ignored("t1") is False +def test_user_initiated_add_bypasses_and_clears_keeping_source_type(db): + # #897 / carlosjfcasero: a user manually adds an album track they had + # previously cancelled. It must bypass the gate AND clear the ignore — but + # WITHOUT pretending to be source_type='manual' (the album modal sends + # source_type='album', which the Albums/Singles categorisation relies on, + # and which an automatic path like repair_worker also legitimately uses). + track = _track("t7") + db.add_to_wishlist_ignore("t7", "Owned Song", "Owned Artist", REASON_CANCELLED) + # An automatic 'album' add (e.g. repair_worker) is still correctly blocked. + assert db.add_to_wishlist(track, source_type="album") is False + assert db.is_track_ignored("t7") is True + # The explicit user click (user_initiated) goes through and clears the ignore, + # while the stored source_type stays 'album'. + assert db.add_to_wishlist(track, source_type="album", user_initiated=True) is True + assert db.is_track_ignored("t7") is False + # Provenance preserved: the stored row is still source_type='album', NOT 'manual'. + row = next(r for r in db.get_wishlist_tracks() if str(r.get("spotify_track_id")) == "t7") + assert row.get("source_type") == "album" + + def test_gate_failopen_when_ignore_table_errors(db, monkeypatch): # If the ignore check raises, the add must still succeed (never block). monkeypatch.setattr(db, "is_track_ignored", lambda *a, **k: (_ for _ in ()).throw(RuntimeError("boom"))) diff --git a/web_server.py b/web_server.py index 46e7fec4..7be627af 100644 --- a/web_server.py +++ b/web_server.py @@ -40,7 +40,7 @@ logger = setup_logging(_log_level, _log_path) # App version — single source of truth for backup metadata, system-info, update check, etc. # Semver: MAJOR.MINOR.PATCH. Bump at each dev→main release. -_SOULSYNC_BASE_VERSION = "2.7.4" +_SOULSYNC_BASE_VERSION = "2.7.5" def _build_version_string(): """Append short commit hash to version when available (e.g. 2.35+abc1234).""" diff --git a/webui/index.html b/webui/index.html index b682f8aa..5f1a3d3d 100644 --- a/webui/index.html +++ b/webui/index.html @@ -1917,8 +1917,8 @@
📄
Drop your file here
or click to browse
-
Supported: CSV, TSV, TXT
- +
Supported: CSV, TSV, TXT, M3U, M3U8
+
@@ -1929,6 +1929,10 @@ TXT One track per line (e.g. Artist - Title). Format and separator can be adjusted after upload.
+
+ M3U / M3U8 + Standard playlist files. Artist, title and duration are read automatically from #EXTINF lines (or the file name for simple playlists). +
@@ -5619,6 +5623,13 @@ Variables: $playlist, $albumartist, $artist, $artistletter, $title, $year, $quality (filename only). Use ${var} to append text +
+ + + Renames the files inside each "Organize by Playlist" folder (your real library files are never touched). Filename only — no folders. Variables: $position (playlist order), $artist, $album, $track, $title. Must include $title and cannot contain "/". Example: $position - $artist - $title → 01 - Daft Punk - One More Time.flac. Empty = keep the original library filename. +
+
+

Import from File

-

Import track lists from CSV, TSV, or plain text files. Drag and drop a file or click to browse. SoulSync parses the file, lets you preview and map columns, then creates a mirrored playlist for discovery and download.

+

Import track lists from CSV, TSV, M3U/M3U8, or plain text files. Drag and drop a file or click to browse. SoulSync parses the file, lets you preview and map columns, then creates a mirrored playlist for discovery and download.

  • CSV/TSV: Auto-detects columns; map Artist, Title, and Album from dropdowns
    • +
  • M3U/M3U8: Read automatically — artist, title and duration come from #EXTINF lines (or the file name for simple playlists). Round-trips with SoulSync's own M3U export
  • Text files: One track per line; choose Artist-Title or Title-Artist order and separator (dash, tab, pipe, etc.)
  • Preview parsed tracks before importing
  • Name your playlist and it becomes a mirrored playlist for sync
    • @@ -1308,7 +1309,7 @@ const DOCS_SECTIONS = [

Import from Text File

-

Import track lists from CSV, TSV, or TXT files. Upload a file with columns for artist, album, and track title:

+

Import track lists from CSV, TSV, TXT, or M3U/M3U8 files. Upload a file with columns for artist, album, and track title (M3U playlists are read automatically):

  1. Click Import from File and select your text file
  2. Choose the separator (comma, tab, or pipe)
    3. diff --git a/webui/static/helper.js b/webui/static/helper.js index a6261903..52eb7528 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -722,8 +722,8 @@ const HELPER_CONTENT = { }, '.sync-tab-button[data-tab="import-file"]': { title: 'Import from File', - description: 'Import track lists from CSV, TSV, or plain text files. Drag and drop or browse for a file, map columns, then create a playlist for sync.', - tips: ['Supports CSV, TSV, and plain text (one track per line)', 'Column mapping for CSV/TSV files', 'Creates a mirrored playlist for persistent state'], + description: 'Import track lists from CSV, TSV, M3U/M3U8, or plain text files. Drag and drop or browse for a file, map columns, then create a playlist for sync.', + tips: ['Supports CSV, TSV, M3U/M3U8, and plain text (one track per line)', 'M3U/M3U8 is read automatically (artist, title, duration from #EXTINF)', 'Column mapping for CSV/TSV files', 'Creates a mirrored playlist for persistent state'], docsId: 'sync-import-file' }, '.sync-tab-button[data-tab="mirrored"]': { @@ -3404,16 +3404,18 @@ function closeHelperSearch() { const WHATS_NEW = { // Convention: keep only the CURRENT release here, plus a single brief // "Earlier versions" summary entry. Don't accumulate old per-version blocks. - '2.7.4': [ - { date: 'June 2026 — 2.7.4 release' }, - { title: 'Re-identify a track (#889)', desc: 'filed a track under the wrong release? a new ⇄ button in the library Enhanced view lets you re-identify it — search any source (tabs, defaults to your active one), see the same song across its single / EP / album with type badges, pick the right one, and soulsync re-files the file you already have under that release with the correct year, in-album track number, and art. replace the original or keep both.', page: 'artists' }, - { title: 'Track titles no longer keep the "01 - " (#890)', desc: 'files with no embedded title tag used to import as "01 - Song Title" (the filename stem) — which never matched the canonical "Song Title", so the real track showed as a false "missing". the number prefix is now stripped, conservatively, so titles like "7 Rings" or "1-800-273-8255" are left alone.', page: 'library' }, - { title: 'Clear dead cover-art folders (#891)', desc: 'a Library Reorganize that moves an album now sweeps the leftover cover.jpg / .lrc / sidecars from the old folder so it actually empties. plus an opt-in "Remove Residual Files" toggle on the Empty Folder Cleaner clears the image-/sidecar-only folders you already have.', page: 'tools' }, - { title: 'AAC as an opt-in quality tier (#886)', desc: 'soulseek downloads can now include AAC (.m4a) as a selectable quality tier, ranked above MP3 and below FLAC. purely additive — off by default; every existing profile behaves exactly as before until you enable it.', page: 'settings' }, - { title: 'Spotify Free enrichment status (#887)', desc: 'if you run enrichment on Spotify Free (no spotify auth), the dashboard button now reads "Running (Spotify Free)" instead of wrongly showing "Not Authenticated".', page: 'dashboard' }, - { title: 'Cleaner album imports (Sokhi)', desc: 'songs from the same album now group under one canonical release id (no more split discographies or mixed cover art), a single can be matched to its parent album, a mid-enrichment crash on an art-less file no longer leaves it untagged, and a sequel digit glued to a CJK title no longer matches the wrong album.', page: 'library' }, - { title: 'More fixes', desc: 'NZBGet imports from the finished location instead of the incomplete "….#NZBID" folder (#884); setting your timezone to Australia/Sydney no longer makes the cache-maintenance job loop every 5 seconds (#885); and the artist-detail header no longer bleeds the blurred artist photo behind it.', page: 'downloads' }, - { title: 'Earlier versions', desc: '2.7.3 added the Quality Upgrade Finder (find + upgrade tracks you own in worse quality than available), a wishlist ignore-list (#874), quarantine duplicate-grouping (#876), the "Track 01" position fix, and Tidal discovery/favorites fixes (#867, #880). 2.7.2 brought playlist-folder mirroring + server-playlist M3U export and ReplayGain / Empty-Folder maintenance jobs; 2.7.1 added download verification + a review queue and closed the websocket login-bypass (#852); 2.7.0 made multi-user real — per-profile streaming accounts, opt-in login, reverse-proxy support.' }, + '2.7.5': [ + { date: 'June 2026 — 2.7.5 release' }, + { title: 'Special-edition cover art', desc: 'a special edition (e.g. *Clair Obscur: Expedition 33 (Gustave Edition)*) no longer ends up with the standard edition\'s art. musicbrainz albums resolved cover art at release-GROUP scope (one representative cover, ~always the standard), so a pinned release now prefers its OWN cover and only falls back to the group/provider art when the release has none.', page: 'library' }, + { title: 'Deezer track numbers', desc: 'a track grabbed from a deezer playlist/wishlist now gets its REAL in-album track number (e.g. "Apologize" = 16 on Shock Value), not the playlist index — resolved from deezer\'s album endpoint, which playlist/search results don\'t carry.', page: 'downloads' }, + { title: '"The" duplicate fix', desc: 'wanting "I Gotta Feeling" by "The Black Eyed Peas" when you own it under "Black Eyed Peas" (or vice-versa) no longer fails to match and re-downloads a duplicate. the matcher now searches both forms across a leading "The"; the scorer still has the final say.', page: 'downloads' }, + { title: 'HiFi previews rejected (#895)', desc: 'HiFi was serving 30-second preview files dressed up as full songs (faked length in the header). soulsync now catches them — short manifests, faked-header files that decode to ~30s, and a lossless-bitrate check — and aborts the HiFi source instead of cascading into a lower-tier copy of the same preview.', page: 'downloads' }, + { title: 'Import M3U / M3U8 playlists (#893)', desc: 'the "import from file" tool now reads M3U/M3U8 playlists — extended #EXTINF (artist/title/duration) and simple path-only files — and round-trips with soulsync\'s own M3U export.', page: 'sync' }, + { title: 'Name files in playlist folders', desc: 'an opt-in template ($position - $artist - $title) renames the files INSIDE each Organize-by-Playlist folder so they sort/play the way you want. filename only (rejects "/", requires $title), defaults to keeping the library filename, works for symlink + copy modes.', page: 'settings' }, + { title: 'Manage the ignore-list (#897)', desc: 'the wishlist ignore-list now has a "🚫 Ignored" button right on the wishlist page (it was buried in a modal). and manually re-adding a previously-cancelled track no longer gets silently blocked — an explicit add bypasses + clears the ignore.', page: 'downloads' }, + { title: 'Find & Add is remembered', desc: 'a manual match you set on a synced playlist is no longer forgotten on the next auto-sync. replace-mode re-matched from scratch; the matcher now honors the durable manual-match, not just the volatile cache a rescan wipes.', page: 'sync' }, + { title: 'Unraid template fixes (#899)', desc: 'the Unraid template now points its TemplateURL / Icon at the canonical files in this repo (raw URLs, not the dead third-party ones), and maps /app/MusicVideos so music-video downloads land on a share.', page: 'settings' }, + { title: 'Earlier versions', desc: '2.7.4 added re-identify (re-file an imported track under the right release without re-downloading) + library/import cleanups (#889/#890/#891). 2.7.3 added the Quality Upgrade Finder and the wishlist ignore-list (#874); 2.7.2 brought playlist-folder mirroring + server-playlist M3U export; 2.7.1 added download verification + a review queue (#852); 2.7.0 made multi-user real — per-profile streaming accounts, opt-in login, reverse-proxy support.' }, ], }; @@ -3444,38 +3446,42 @@ const WHATS_NEW = { // usage_note?: 'optional hint shown at the bottom' } const VERSION_MODAL_SECTIONS = [ { - title: "Re-identify a track (#889)", - description: "filed a track under the wrong release? re-identify it from the library without re-downloading — soulsync re-files the file you already have under the release you pick.", + title: "Matching & metadata accuracy", + description: "a batch of fixes so downloads land with the right numbers, art, and no duplicates.", features: [ - "a ⇄ button in the Enhanced library view opens a search across any configured source (tabs, defaults to your active one)", - "see the same song across its single / EP / album, each with a type badge, and pick the right collection", - "it re-files under that release with the correct year, in-album track number, and album art", - "replace the original entry or keep both — and it can never delete the file if you pick the release it's already in", - ], - usage_note: "Library → an artist → Enhanced view → ⇄ on a track", - }, - { - title: "Cleaner libraries & imports", - description: "a batch of fixes that keep the library tidy and matchable.", - features: [ - "#890 — track titles no longer keep the \"01 - \" from a filename (which caused false \"missing\" tracks); stripped conservatively so \"7 Rings\" / \"1-800-273-8255\" are left alone", - "#891 — a reorganize now sweeps leftover cover.jpg / .lrc from the old folder, plus an opt-in \"Remove Residual Files\" toggle clears image-only folders you already have", - "Sokhi — same-album songs group under one canonical release (no split discographies / mixed cover art); a single can match its parent album; a mid-enrichment crash no longer leaves a file untagged; a CJK-title sequel-digit no longer matches the wrong album", + "special-edition cover art — a pinned release (e.g. a \"Gustave Edition\") now uses its OWN cover instead of musicbrainz's release-group representative (usually the standard edition)", + "deezer track numbers — a track from a deezer playlist/wishlist gets its REAL in-album track number (e.g. \"Apologize\" = 16 on Shock Value), resolved from the album endpoint, not the playlist index", + "\"The\" dedup — \"The Black Eyed Peas\" and \"Black Eyed Peas\" now match across a leading \"The\", so a track you already own doesn't fail to match and re-download a duplicate", ], }, { - title: "Quality & sources", - description: "more control over downloads, plus a couple of source fixes.", + title: "HiFi previews (#895)", + description: "HiFi was serving 30-second preview files disguised as full songs (full length faked in the header).", features: [ - "#886 — AAC (.m4a) as an opt-in soulseek quality tier, ranked above MP3 and below FLAC; off by default so nothing changes until you enable it", - "#887 — enrichment on Spotify Free now reads \"Running (Spotify Free)\" instead of \"Not Authenticated\"", - "#884 — NZBGet imports from the finished location, not the incomplete \"….#NZBID\" folder", - "#885 — Australia/Sydney timezone no longer makes the cache-maintenance job loop every 5 seconds", + "rejects them three ways — short preview manifests, faked-header files that decode to ~30s, and a lossless-bitrate sanity check", + "aborts the HiFi source instead of cascading into a lower-tier copy of the same preview — you get a clean fail + fallback, not a broken file", ], }, { - title: "Earlier in 2.7.3 / 2.7.2 / 2.7.1 / 2.7.0", - description: "2.7.3 added the Quality Upgrade Finder (find + upgrade tracks you own in worse quality than available), a wishlist ignore-list (#874), quarantine duplicate-grouping (#876), the \"Track 01\" position fix, and Tidal discovery/favorites fixes (#867, #880). 2.7.2 brought playlist-folder mirroring + server-playlist M3U export and ReplayGain / Empty-Folder maintenance jobs; 2.7.1 added download verification (acoustid checks every download) + a review queue and closed the websocket login-bypass (#852); 2.7.0 made multi-user real — per-profile streaming accounts, opt-in login, reverse-proxy support.", + title: "Playlists", + description: "import from more formats, name files your way, and keep your manual matches.", + features: [ + "#893 — import M3U / M3U8 playlists in the \"import from file\" tool (extended #EXTINF + simple path-only files); round-trips with soulsync's own M3U export", + "organize-by-playlist file naming — an opt-in template ($position - $artist - $title) renames the files inside each playlist folder so they sort/play in order; filename only, defaults to keeping the library name, works for symlink + copy", + "find & add is remembered — a manual match on a synced playlist survives the next auto-sync; the matcher honors the durable manual-match, not just the volatile cache a rescan wipes", + ], + }, + { + title: "Ignore-list & packaging", + description: "manage the wishlist ignore-list, and a couple of Unraid template fixes.", + features: [ + "#897 — a \"🚫 Ignored\" button now lives right on the wishlist page (it was buried in a modal), and manually re-adding a previously-cancelled track no longer gets silently blocked", + "#899 — the Unraid template points TemplateURL / Icon at the canonical files in this repo (not a dead third-party repo) and maps /app/MusicVideos so music-video downloads land on a share", + ], + }, + { + title: "Earlier in 2.7.4 / 2.7.3 / 2.7.2 / 2.7.1 / 2.7.0", + description: "2.7.4 added re-identify (re-file an imported track under the right release without re-downloading) plus library/import cleanups (#889/#890/#891). 2.7.3 added the Quality Upgrade Finder and the wishlist ignore-list (#874); 2.7.2 brought playlist-folder mirroring + server-playlist M3U export and ReplayGain / Empty-Folder maintenance jobs; 2.7.1 added download verification (acoustid checks every download) + a review queue and closed the websocket login-bypass (#852); 2.7.0 made multi-user real — per-profile streaming accounts, opt-in login, reverse-proxy support.", features: [], }, ]; diff --git a/webui/static/settings.js b/webui/static/settings.js index abfa3e80..8c550783 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -1274,6 +1274,7 @@ async function loadSettingsData() { document.getElementById('template-album-path').value = settings.file_organization?.templates?.album_path || '$albumartist/$albumartist - $album/$track - $title'; document.getElementById('template-single-path').value = settings.file_organization?.templates?.single_path || '$artist/$artist - $title/$title'; document.getElementById('template-playlist-path').value = settings.file_organization?.templates?.playlist_path || '$playlist/$artist - $title'; + document.getElementById('template-playlist-item').value = settings.file_organization?.templates?.playlist_item || ''; document.getElementById('template-video-path').value = settings.file_organization?.templates?.video_path || '$artist/$title-video'; document.getElementById('disc-label').value = settings.file_organization?.disc_label || 'Disc'; document.getElementById('collab-artist-mode').value = settings.file_organization?.collab_artist_mode || 'first'; @@ -2936,6 +2937,21 @@ async function saveSettings(quiet = false) { } } + // Validate the optional "Playlist File Naming" template before saving: it's a + // filename (no path separator) and must include $title — mirrors the server-side + // rule so a broken value can't be stored. Empty = feature off (allowed). + const _plItemTpl = (document.getElementById('template-playlist-item')?.value || '').trim(); + if (_plItemTpl) { + if (_plItemTpl.includes('/') || _plItemTpl.includes('\\')) { + showToast('Playlist File Naming can\'t contain a folder separator ( / or \\ ) — it names the file, not a path.', 'error'); + return; + } + if (!_plItemTpl.includes('$title')) { + showToast('Playlist File Naming must include $title so every file has a name.', 'error'); + return; + } + } + const settings = { active_media_server: activeServer, spotify: { @@ -3141,6 +3157,7 @@ async function saveSettings(quiet = false) { album_path: document.getElementById('template-album-path').value, single_path: document.getElementById('template-single-path').value, playlist_path: document.getElementById('template-playlist-path').value, + playlist_item: document.getElementById('template-playlist-item').value, video_path: document.getElementById('template-video-path').value } }, diff --git a/webui/static/stats-automations.js b/webui/static/stats-automations.js index 395ba528..bf07d547 100644 --- a/webui/static/stats-automations.js +++ b/webui/static/stats-automations.js @@ -41,8 +41,8 @@ function _initImportFileTab() { function _importFileRead(file) { const ext = file.name.split('.').pop().toLowerCase(); - if (!['csv', 'tsv', 'txt'].includes(ext)) { - showToast('Unsupported file type. Use CSV, TSV, or TXT.', 'error'); + if (!['csv', 'tsv', 'txt', 'm3u', 'm3u8'].includes(ext)) { + showToast('Unsupported file type. Use CSV, TSV, TXT, M3U, or M3U8.', 'error'); return; } @@ -50,7 +50,8 @@ function _importFileRead(file) { reader.onload = (e) => { _importFileState.rawText = e.target.result; _importFileState.fileName = file.name; - _importFileState.fileType = (ext === 'txt') ? 'text' : 'csv'; + _importFileState.fileType = (ext === 'm3u' || ext === 'm3u8') ? 'm3u' + : (ext === 'txt') ? 'text' : 'csv'; _importFileParseAndPreview(); }; reader.readAsText(file); @@ -103,6 +104,66 @@ function _importFileParseCsv(text, delimiter) { return { headers, rows }; } +// Parse an M3U / M3U8 playlist into track objects. Handles both the simple form +// (one media path per line) and the extended form (#EXTINF:, - +// followed by the path). SoulSync's own export is extended M3U and replaces the path +// of an un-located track with a "# MISSING: ..." comment — those are exactly the +// tracks a user imports to go match/download, so a pending #EXTINF is flushed even +// when no path line follows it. Returns { tracks, playlistName }. +function _importFileParseM3u(text) { + const lines = (text || '').split(/\r?\n/); + const tracks = []; + let playlistName = ''; + let pending = null; // { duration_ms, artist, title } from the last #EXTINF + + function splitArtistTitle(s) { + const i = s.indexOf(' - '); + return i !== -1 + ? { artist: s.slice(0, i).trim(), title: s.slice(i + 3).trim() } + : { artist: '', title: s.trim() }; + } + function pushTrack(artist, title, duration_ms) { + if (!title && !artist) return; + tracks.push({ track_name: title || '', artist_name: artist || '', album_name: '', duration_ms: duration_ms || 0 }); + } + function flushPending() { + if (pending) { pushTrack(pending.artist, pending.title, pending.duration_ms); pending = null; } + } + + for (const raw of lines) { + const line = raw.trim(); + if (!line) continue; + if (line.charAt(0) === '#') { + if (/^#EXTINF:/i.test(line)) { + flushPending(); // a prior #EXTINF whose path was missing still counts + const rest = line.slice(8); // after "#EXTINF:" + const comma = rest.indexOf(','); + const secs = parseFloat(comma === -1 ? rest : rest.slice(0, comma)); + const meta = comma === -1 ? '' : rest.slice(comma + 1).trim(); + const { artist, title } = splitArtistTitle(meta); + pending = { duration_ms: (!isNaN(secs) && secs > 0) ? Math.round(secs * 1000) : 0, artist, title }; + } else if (/^#PLAYLIST:/i.test(line)) { + playlistName = line.slice(line.indexOf(':') + 1).trim(); + } + // ignore #EXTM3U, #GENERATED, "# MISSING:", #EXTALB, and other directives + continue; + } + // A non-# line is a media path/URL — the entry for the pending #EXTINF, if any. + if (pending && (pending.title || pending.artist)) { + pushTrack(pending.artist, pending.title, pending.duration_ms); + pending = null; + } else { + // Simple M3U with no #EXTINF: derive artist/title from the file name. + pending = null; + const base = (line.split(/[\\/]/).pop() || line).replace(/\.[^.]+$/, ''); + const { artist, title } = splitArtistTitle(base); + pushTrack(artist, title, 0); + } + } + flushPending(); // trailing #EXTINF with no following path + return { tracks, playlistName }; +} + function _importFileAutoMapColumns(headers) { const map = {}; const lowerHeaders = headers.map(h => h.toLowerCase().trim()); @@ -139,7 +200,14 @@ function _importFileParseAndPreview() { const state = _importFileState; const text = state.rawText; - if (state.fileType === 'text') { + if (state.fileType === 'm3u') { + // M3U/M3U8 is self-describing — no column mapping or order/separator needed. + const { tracks, playlistName } = _importFileParseM3u(text); + state.rows = tracks; // already track objects + state.headers = []; + state.columnMap = {}; + state.m3uPlaylistName = playlistName || ''; + } else if (state.fileType === 'text') { // Plain text: one track per line const lines = text.split(/\r?\n/).filter(l => l.trim()); state.rows = lines; @@ -163,7 +231,15 @@ function _importFileBuildTracks() { const state = _importFileState; state.parsedTracks = []; - if (state.fileType === 'text') { + if (state.fileType === 'm3u') { + // Tracks were already parsed by _importFileParseM3u; just copy them through. + state.parsedTracks = state.rows.map(t => ({ + track_name: t.track_name || '', + artist_name: t.artist_name || '', + album_name: t.album_name || '', + duration_ms: t.duration_ms || 0 + })); + } else if (state.fileType === 'text') { const orderEl = document.getElementById('import-file-text-order'); const sepEl = document.getElementById('import-file-text-separator'); const order = orderEl ? orderEl.value : 'artist-title'; @@ -249,10 +325,12 @@ function _importFileRenderPreview() { _importFileRenderColumnMapping(); } - // Pre-fill playlist name from filename (strip extension) + // Pre-fill playlist name: prefer the M3U's own #PLAYLIST: directive, else the filename. const nameInput = document.getElementById('import-file-playlist-name'); if (nameInput && !nameInput.value) { - nameInput.value = state.fileName.replace(/\.[^.]+$/, ''); + nameInput.value = (state.fileType === 'm3u' && state.m3uPlaylistName) + ? state.m3uPlaylistName + : state.fileName.replace(/\.[^.]+$/, ''); } // Update button state const btn = document.getElementById('import-file-import-btn');