diff --git a/config/settings.py b/config/settings.py index fe03096e..71e5428e 100644 --- a/config/settings.py +++ b/config/settings.py @@ -705,6 +705,12 @@ class ConfigManager: }, "import": { "staging_path": "./Staging", + # Master toggle for quality-filtering on import. On by default: + # downloaded files that don't meet the quality profile are + # quarantined instead of imported (same gate the download + # pipeline uses). Off → import everything regardless of quality; + # the library Quality Upgrade Scanner still flags them. + "quality_filter_enabled": True, "replace_lower_quality": False, # Use the top Staging folder as the artist (Artist/Album layouts, # mixtapes). On by default to preserve the long-standing import diff --git a/core/amazon_download_client.py b/core/amazon_download_client.py index 18e4bad1..004a0e8d 100644 --- a/core/amazon_download_client.py +++ b/core/amazon_download_client.py @@ -32,6 +32,8 @@ from config.settings import config_manager from core.amazon_client import AmazonClient, AmazonClientError from core.download_plugins.base import DownloadSourcePlugin from core.download_plugins.types import AlbumResult, DownloadStatus, TrackResult +from core.quality.model import AudioQuality +from core.quality.source_map import quality_tier_for_source from utils.logging_config import get_logger logger = get_logger("amazon_download_client") @@ -78,7 +80,7 @@ class AmazonDownloadClient(DownloadSourcePlugin): self.download_path = Path(download_path) self.download_path.mkdir(parents=True, exist_ok=True) - self._quality = config_manager.get("amazon_download.quality", "flac") + self._quality = quality_tier_for_source("amazon", default="flac") self._allow_fallback = config_manager.get("amazon_download.allow_fallback", True) self._client = AmazonClient(preferred_codec=self._quality) @@ -133,11 +135,17 @@ class AmazonDownloadClient(DownloadSourcePlugin): album_map: Dict[str, AlbumResult] = {} album_order: List[str] = [] preferred = self._client.preferred_codec + # Search results only carry the codec (real sample_rate arrives at + # stream time). Claim the format honestly — FLAC for the lossless + # codec, lossy otherwise — so audio_quality derives a real format + # instead of the display label ("Lossless"), and the post-download + # probe pins the actual sample_rate/bit_depth. + amazon_q = AudioQuality(format='flac' if _codec_key(preferred) == 'flac' else 'aac') for item in items: quality = _quality_label(preferred) if item.is_track: - track_results.append(TrackResult( + tr = TrackResult( username="amazon", filename=f"{item.asin}||{item.artist_name} - {item.title}", size=0, @@ -155,7 +163,9 @@ class AmazonDownloadClient(DownloadSourcePlugin): "album_asin": item.album_asin, "isrc": item.isrc, }, - )) + ) + tr.set_quality(amazon_q) + track_results.append(tr) elif item.is_album: album_asin = item.album_asin or item.asin if album_asin not in album_map: @@ -173,6 +183,7 @@ class AmazonDownloadClient(DownloadSourcePlugin): title=item.title, album=item.album_name, ) + placeholder.set_quality(amazon_q) album_map[album_asin] = AlbumResult( username="amazon", album_path=album_asin, diff --git a/core/deezer_download_client.py b/core/deezer_download_client.py index 165adcfd..f2368ac1 100644 --- a/core/deezer_download_client.py +++ b/core/deezer_download_client.py @@ -21,6 +21,7 @@ from typing import Any, Dict, List, Optional, Tuple import requests from core.download_plugins.types import AlbumResult, DownloadStatus, TrackResult +from core.quality.source_map import quality_from_deezer, quality_tier_for_source from utils.logging_config import get_logger logger = get_logger("deezer_download") @@ -119,7 +120,7 @@ class DeezerDownloadClient(DownloadSourcePlugin): self._authenticated = False # Quality preference - self._quality = config_manager.get('deezer_download.quality', 'flac') + self._quality = quality_tier_for_source('deezer', default='flac') # Try to authenticate on init if ARL is configured arl = config_manager.get('deezer_download.arl', '') @@ -597,7 +598,7 @@ class DeezerDownloadClient(DownloadSourcePlugin): bitrate = 128 quality = 'mp3' - results.append(TrackResult( + tr = TrackResult( username='deezer_dl', filename=f"{track_id}||{artist} - {title}", size=est_size, @@ -611,7 +612,10 @@ class DeezerDownloadClient(DownloadSourcePlugin): title=title, album=album, track_number=item.get('track_position'), - )) + ) + # Stamp CD-quality FLAC (16/44.1) so lossless ranks correctly. + tr.set_quality(quality_from_deezer(self._quality)) + results.append(tr) logger.info(f"Deezer search for '{query}' returned {len(results)} results") return results, [] diff --git a/core/discovery/quality_scanner.py b/core/discovery/quality_scanner.py index 7c689a38..4d2a2bdf 100644 --- a/core/discovery/quality_scanner.py +++ b/core/discovery/quality_scanner.py @@ -21,7 +21,11 @@ from core.metadata.registry import get_client_for_source from core.metadata.types import Album from core.wishlist.payloads import ensure_wishlist_track_format -logger = logging.getLogger(__name__) +# Use the project logger namespace ("soulsync.*") so the scanner's progress and +# diagnostics actually surface in the app log — plain getLogger(__name__) lands +# under "core.discovery.quality_scanner", which the app log view doesn't show. +from utils.logging_config import get_logger +logger = get_logger("discovery.quality_scanner") # Per-source typed converter dispatch — same registry pattern as diff --git a/core/download_engine/engine.py b/core/download_engine/engine.py index 8aaa07e8..1d555432 100644 --- a/core/download_engine/engine.py +++ b/core/download_engine/engine.py @@ -25,6 +25,7 @@ big-bang switchover. from __future__ import annotations +import asyncio import threading from typing import Any, Dict, Iterator, List, Optional, Tuple @@ -391,6 +392,16 @@ class DownloadEngine: (tracks, albums) tuple, or ``([], [])`` when every source in the chain is exhausted. + Priority mode is deliberately quality-AGNOSTIC at search time — source + order is king and the first source that returns any tracks wins, exactly + matching pre-quality-system behaviour byte-for-byte (#896 review #3). + Quality-gating the priority path would deprioritise e.g. a soulseek + mp3 whose bitrate slskd omitted (``bitrate=None`` → "unsatisfied"), + changing which source wins and adding latency for users who never opted + in. Cross-source quality pooling is the job of best_quality mode + (``search_all_sources``); final per-result ranking still happens in the + orchestrator's match/quality filter. RAW tracks are returned. + Replaces orchestrator's hand-rolled hybrid search loop. The chain is ordered (most-preferred first). """ @@ -406,9 +417,10 @@ class DownloadEngine: try: logger.info(f"Trying {source_name} (priority {i+1}): {query}") tracks, albums = await plugin.search(query, timeout, progress_callback) - if tracks: - logger.info(f"{source_name} found {len(tracks)} tracks") - return (tracks, albums) + if not tracks: + continue + logger.info(f"{source_name} found {len(tracks)} tracks") + return (tracks, albums) except Exception as e: logger.warning(f"{source_name} search failed: {e}") @@ -418,6 +430,75 @@ class DownloadEngine: ) return ([], []) + async def search_all_sources(self, query: str, source_chain, + timeout=None, progress_callback=None, + exclude_sources=None): + """Best-quality mode: pool RAW tracks from EVERY configured source in + ``source_chain`` instead of stopping at the first satisfying one. + + Unlike :meth:`search_with_fallback`, no source short-circuits the + search — the caller (orchestrator/worker) ranks the combined pool + best→worst by actual audio quality. ``exclude_sources`` drops sources + whose per-source retry budget is already spent (so their candidates + never re-enter the pool). Unconfigured / unregistered / raising sources + are skipped exactly like the fallback path. Returns + ``(combined_tracks, combined_albums)``. + """ + excluded = {s.lower() for s in (exclude_sources or []) if s} + pooled_tracks = [] + pooled_albums = [] + # Per-source contribution for an honest pool log — e.g. a release-level + # source like usenet/torrent that returns nothing for a track-title + # query should read "usenet=0", not silently hide behind the chain name. + contributions = [] + + # Decide which sources to actually query, recording why the rest were + # skipped. Searches then run CONCURRENTLY so the pool waits only for the + # slowest source (e.g. usenet/Prowlarr, which can be slow) rather than + # the sum of every source's latency. + to_search = [] # (source_name, plugin) + for source_name in source_chain: + if source_name.lower() in excluded: + contributions.append(f"{source_name}=excluded") + continue + plugin = self._plugins.get(source_name) + if plugin is None: + logger.info(f"Skipping {source_name} (not available)") + contributions.append(f"{source_name}=unavailable") + continue + if hasattr(plugin, 'is_configured') and not plugin.is_configured(): + logger.info(f"Skipping {source_name} (not configured)") + contributions.append(f"{source_name}=unconfigured") + continue + to_search.append((source_name, plugin)) + + async def _one(plugin): + return await plugin.search(query, timeout, progress_callback) + + results = await asyncio.gather( + *[_one(plugin) for _, plugin in to_search], + return_exceptions=True, + ) + + for (source_name, _), result in zip(to_search, results): + if isinstance(result, Exception): + logger.warning(f"{source_name} search failed: {result}") + contributions.append(f"{source_name}=error") + continue + tracks, albums = result + n = len(tracks) if tracks else 0 + if tracks: + pooled_tracks.extend(tracks) + if albums: + pooled_albums.extend(albums) + contributions.append(f"{source_name}={n}") + + logger.info( + "Best-quality pool: %d candidates [%s] for: %s", + len(pooled_tracks), ', '.join(contributions), query, + ) + return (pooled_tracks, pooled_albums) + async def download_with_fallback(self, username: str, filename: str, file_size: int, source_chain) -> Optional[str]: """Try each source in ``source_chain`` until one accepts the diff --git a/core/download_orchestrator.py b/core/download_orchestrator.py index 1984c0a6..664a0f4f 100644 --- a/core/download_orchestrator.py +++ b/core/download_orchestrator.py @@ -28,6 +28,7 @@ from config.settings import config_manager from core.download_engine import DownloadEngine from core.download_plugins.registry import DownloadPluginRegistry, build_default_registry from core.download_plugins.types import TrackResult, AlbumResult, DownloadStatus +from core.quality.selection import load_search_mode logger = get_logger("download_orchestrator") @@ -102,12 +103,14 @@ class DownloadOrchestrator: deezer_dl = self.client('deezer_dl') if deezer_arl and deezer_dl: deezer_dl.reconnect(deezer_arl) - deezer_dl._quality = config_manager.get('deezer_download.quality', 'flac') + from core.quality.source_map import quality_tier_for_source + deezer_dl._quality = quality_tier_for_source('deezer', default='flac') # Reload Amazon quality preference (T2Tunes needs no reconnect — public proxy) amazon = self.client('amazon') if amazon: - quality = config_manager.get('amazon_download.quality', 'flac') + from core.quality.source_map import quality_tier_for_source + quality = quality_tier_for_source('amazon', default='flac') amazon._quality = quality amazon._allow_fallback = config_manager.get('amazon_download.allow_fallback', True) if hasattr(amazon, '_client') and amazon._client: @@ -342,6 +345,11 @@ class DownloadOrchestrator: if not chain: logger.warning("Hybrid search exhausted: no eligible sources after exclusion filter") return [], [] + if load_search_mode() == 'best_quality': + logger.info(f"Best-quality search ({' → '.join(chain)}): {query}") + return await self.engine.search_all_sources( + query, chain, timeout, progress_callback, + ) logger.info(f"Hybrid search ({' → '.join(chain)}): {query}") return await self.engine.search_with_fallback(query, chain, timeout, progress_callback) @@ -412,9 +420,17 @@ class DownloadOrchestrator: if scored: scored.sort(key=lambda x: x._match_confidence, reverse=True) - filtered_results = scored + # Match filter done (right track); now prefer the best quality + # among the confidence-passing survivors so streaming isn't + # quality-blind like Soulseek already isn't. Stable ranking + # keeps confidence order within an equal quality tier; the + # `or scored` fail-safe never leaves us with nothing to try. + from core.quality.selection import rank_for_profile + ranked, _ = rank_for_profile(scored) + filtered_results = ranked or scored logger.info(f"Streaming validation: {len(scored)}/{len(tracks)} passed " - f"(best: {scored[0]._match_confidence:.2f})") + f"(best: {scored[0]._match_confidence:.2f}, " + f"quality pick: {filtered_results[0].audio_quality.label()})") else: logger.warning(f"No streaming results passed validation for: {query}") return None diff --git a/core/download_plugins/types.py b/core/download_plugins/types.py index 59d52913..5c364da8 100644 --- a/core/download_plugins/types.py +++ b/core/download_plugins/types.py @@ -13,10 +13,11 @@ import from a neutral package per Cin's contract-first standard. from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Any, Dict, List, Optional from core.imports.filename import parse_filename_metadata +from core.quality.model import AudioQuality @dataclass @@ -32,6 +33,36 @@ class SearchResult: upload_speed: int queue_length: int result_type: str = "track" # "track" or "album" + # Rich quality metadata — populated by sources that provide it. + # None means "unknown", not "absent". + sample_rate: Optional[int] = None # Hz (e.g. 44100, 96000, 192000) + bit_depth: Optional[int] = None # bits per sample (16, 24) + + @property + def audio_quality(self) -> AudioQuality: + """Unified quality descriptor derived from this result's fields.""" + return AudioQuality( + format=self.quality.lower() if self.quality else 'unknown', + bitrate=self.bitrate, + sample_rate=self.sample_rate, + bit_depth=self.bit_depth, + ) + + def set_quality(self, aq: AudioQuality) -> None: + """Merge a mapped :class:`AudioQuality` onto this result's fields. + + Used by streaming sources to stamp their claimed tier (Tidal/HiFi + tier strings, Qobuz API values, …) so ``audio_quality`` ranks + correctly. Mapper-provided fields win; a ``None`` from the mapper + leaves any already-reported value (e.g. a probed bitrate) intact. + """ + self.quality = aq.format + if aq.bitrate is not None: + self.bitrate = aq.bitrate + if aq.sample_rate is not None: + self.sample_rate = aq.sample_rate + if aq.bit_depth is not None: + self.bit_depth = aq.bit_depth @property def quality_score(self) -> float: @@ -127,6 +158,19 @@ class AlbumResult: queue_length: int = 0 result_type: str = "album" + @property + def audio_quality(self) -> AudioQuality: + """Unified quality descriptor derived from dominant track quality.""" + sample_rates = [t.sample_rate for t in self.tracks if t.sample_rate] + bit_depths = [t.bit_depth for t in self.tracks if t.bit_depth] + bitrates = [t.bitrate for t in self.tracks if t.bitrate] + return AudioQuality( + format=self.dominant_quality.lower() if self.dominant_quality else 'unknown', + bitrate=max(bitrates) if bitrates else None, + sample_rate=max(sample_rates) if sample_rates else None, + bit_depth=max(bit_depths) if bit_depths else None, + ) + @property def quality_score(self) -> float: """Calculate album quality score based on dominant quality and track count""" diff --git a/core/downloads/cancel.py b/core/downloads/cancel.py index b375f8fa..d8d0b820 100644 --- a/core/downloads/cancel.py +++ b/core/downloads/cancel.py @@ -83,9 +83,26 @@ def clear_completed_local() -> int: """ cleared = 0 with tasks_lock: + # Protect tasks belonging to a still-active batch. A batch is "active" + # while any of its queued tasks is non-terminal (still searching / + # downloading / queued / post-processing). Pruning a batch's completed + # or failed tasks mid-run would yank them out of the Downloads page — + # and failed/cancelled rows aren't recoverable from library_history — + # so the user would never see them until the batch ended. Keep the whole + # active batch intact; it gets cleaned by a later run once it finishes. + protected_task_ids: set = set() + for batch in download_batches.values(): + queue = batch.get('queue', []) if isinstance(batch, dict) else [] + batch_active = any( + download_tasks.get(tid, {}).get('status') not in _TERMINAL_STATUSES + for tid in queue if tid in download_tasks + ) + if batch_active: + protected_task_ids.update(queue) + task_ids_to_remove = [ tid for tid, task in download_tasks.items() - if task.get('status') in _TERMINAL_STATUSES + if task.get('status') in _TERMINAL_STATUSES and tid not in protected_task_ids ] for tid in task_ids_to_remove: del download_tasks[tid] diff --git a/core/downloads/candidates.py b/core/downloads/candidates.py index 1edca30d..1dfa231d 100644 --- a/core/downloads/candidates.py +++ b/core/downloads/candidates.py @@ -47,6 +47,55 @@ from core.runtime_state import ( logger = logging.getLogger(__name__) +def _priority_sort_key(r): + """Today's confidence-first key: never download a high-quality WRONG file.""" + return ( + getattr(r, 'confidence', 0) or 0, + getattr(r, 'quality_score', 0) or 0, + getattr(r, 'upload_speed', 0) or 0, + -(getattr(r, 'queue_length', 0) or 0), + getattr(r, 'free_upload_slots', 0) or 0, + getattr(r, 'size', 0) or 0, + ) + + +def _quality_first_sort_key(r, targets): + """Best-quality key: the user's profile quality rank dominates; all the + priority-mode signals (confidence, speed, …) become tiebreakers. + + Every candidate reaching this point already passed match filtering, so it + is "correct enough" — ordering by quality among correct candidates is safe. + Candidates with no usable quality info, or that match no target, sort last + (never dropped). Lower target index = better target, so it's negated to fit + the descending (reverse=True) sort. + """ + from core.quality.model import rank_candidate + + aq = getattr(r, 'audio_quality', None) + if aq is None or not targets: + target_idx, tier = (len(targets) if targets else 0), 0.0 + else: + try: + target_idx, tier = rank_candidate(aq, targets) + except Exception: + target_idx, tier = len(targets), 0.0 + return (-target_idx, tier) + _priority_sort_key(r) + + +def order_candidates(candidates, *, quality_first=False, targets=None): + """Return *candidates* ordered best-first for the download walk. + + ``quality_first=False`` (priority mode) → confidence-first, byte-for-byte + today's behaviour. ``quality_first=True`` (best-quality mode) → the user's + profile quality rank dominates, confidence/peer signals break ties. + """ + if quality_first: + key = lambda r: _quality_first_sort_key(r, targets or []) + else: + key = _priority_sort_key + return sorted(candidates, key=key, reverse=True) + + @dataclass class CandidatesDeps: """Bundle of cross-cutting deps the candidate-fallback logic needs.""" @@ -59,25 +108,25 @@ class CandidatesDeps: on_download_completed: Callable -def attempt_download_with_candidates(task_id, candidates, track, batch_id=None, deps: CandidatesDeps = None): +def attempt_download_with_candidates(task_id, candidates, track, batch_id=None, + deps: CandidatesDeps = None, *, + quality_first=False, quality_targets=None): """ Attempts to download with fallback candidate logic (matches GUI's retry_parallel_download_with_fallback). Returns True if successful, False if all candidates fail. + + ``quality_first`` (best-quality search mode) orders the walk by the user's + profile quality rank instead of confidence-first; ``quality_targets`` is the + profile target list used for that ranking. Defaults preserve priority-mode + behaviour exactly. """ - # Sort candidates by match confidence first, then peer quality. Upstream - # Soulseek validation already considers peer speed/slots/queue when scores - # are close; preserve that signal here instead of flattening ties back to - # arbitrary slskd response order. - candidates.sort( - key=lambda r: ( - getattr(r, 'confidence', 0) or 0, - getattr(r, 'quality_score', 0) or 0, - getattr(r, 'upload_speed', 0) or 0, - -(getattr(r, 'queue_length', 0) or 0), - getattr(r, 'free_upload_slots', 0) or 0, - getattr(r, 'size', 0) or 0, - ), - reverse=True, + # Sort candidates. Priority mode: confidence-first, then peer quality — + # upstream Soulseek validation already considers peer speed/slots/queue when + # scores are close; preserve that signal instead of flattening ties back to + # arbitrary slskd response order. Best-quality mode: profile quality rank + # dominates (all candidates here already passed match filtering). + candidates = order_candidates( + candidates, quality_first=quality_first, targets=quality_targets, ) with tasks_lock: diff --git a/core/downloads/lifecycle.py b/core/downloads/lifecycle.py index cd7e38fc..8ad2d773 100644 --- a/core/downloads/lifecycle.py +++ b/core/downloads/lifecycle.py @@ -26,6 +26,7 @@ Lifted verbatim from web_server.py. Dependencies injected via from __future__ import annotations import logging +import os import shutil import time import traceback @@ -44,6 +45,27 @@ from core.runtime_state import ( logger = logging.getLogger(__name__) +# A task that has been in 'post_processing' longer than this is treated as stuck. +# Post-processing (AcoustID + quality + import) is serialized, so a large batch +# legitimately backs up — keep this generous so genuinely-slow imports aren't +# cut off mid-flight (the old 5-min cutoff falsely "completed" queued tasks). +_POST_PROCESSING_STUCK_TIMEOUT = 1800 # 30 minutes + + +def _resolve_stuck_post_processing_status(task: dict) -> str: + """Decide the terminal status for a task stuck in post_processing. + + Only call it 'completed' if the import actually produced a file on disk + (``final_file_path`` is set at the end of successful post-processing). Without + a real file, force-completing is a lie — the task shows as a downloaded track + that isn't anywhere. Mark those 'failed' so they're retryable and honest. + """ + final_path = task.get('final_file_path') + if final_path and os.path.exists(final_path): + return 'completed' + return 'failed' + + def _safe_batch_dirname(batch_id: str) -> str: safe = ''.join(ch if ch.isalnum() or ch in ('-', '_') else '_' for ch in str(batch_id or 'batch')) return safe or 'batch' @@ -435,9 +457,15 @@ def on_download_completed(batch_id: str, task_id: str, success: bool, deps: Life retrying_count += 1 elif task_status == 'post_processing': task_age = current_time - task.get('status_change_time', current_time) - if task_age > 300: # 5 minutes (post-processing should be fast) - logger.info(f"⏰ [Stuck Detection] Task {queue_task_id} stuck in post_processing for {task_age:.0f}s - forcing completion") - task['status'] = 'completed' # Assume it worked if file verification is taking too long + if task_age > _POST_PROCESSING_STUCK_TIMEOUT: + new_status = _resolve_stuck_post_processing_status(task) + if new_status == 'completed': + logger.info(f"⏰ [Stuck Detection] Task {queue_task_id} stuck in post_processing for {task_age:.0f}s but file exists — completing") + task['status'] = 'completed' + else: + logger.warning(f"⏰ [Stuck Detection] Task {queue_task_id} stuck in post_processing for {task_age:.0f}s with no output file — marking failed") + task['status'] = 'failed' + task['error_message'] = 'Post-processing timed out without producing a file' finished_count += 1 else: retrying_count += 1 @@ -667,9 +695,15 @@ def check_batch_completion_v2(batch_id: str, deps: LifecycleDeps) -> Optional[bo retrying_count += 1 elif task_status == 'post_processing': task_age = current_time - task.get('status_change_time', current_time) - if task_age > 300: # 5 minutes (post-processing should be fast) - logger.info(f"⏰ [Stuck Detection V2] Task {task_id} stuck in post_processing for {task_age:.0f}s - forcing completion") - task['status'] = 'completed' # Assume it worked if file verification is taking too long + if task_age > _POST_PROCESSING_STUCK_TIMEOUT: + new_status = _resolve_stuck_post_processing_status(task) + if new_status == 'completed': + logger.info(f"⏰ [Stuck Detection V2] Task {task_id} stuck in post_processing for {task_age:.0f}s but file exists — completing") + task['status'] = 'completed' + else: + logger.warning(f"⏰ [Stuck Detection V2] Task {task_id} stuck in post_processing for {task_age:.0f}s with no output file — marking failed") + task['status'] = 'failed' + task['error_message'] = 'Post-processing timed out without producing a file' finished_count += 1 else: retrying_count += 1 diff --git a/core/downloads/monitor.py b/core/downloads/monitor.py index 60ab4585..81a8ef22 100644 --- a/core/downloads/monitor.py +++ b/core/downloads/monitor.py @@ -760,7 +760,8 @@ class WebUIDownloadMonitor: # used_sources keys are formatted as "{username}_{filename}", so startswith is exact. is_tidal = any(s.startswith('tidal_') for s in tried_sources) if is_tidal: - tidal_quality = config_manager.get('tidal_download.quality', 'lossless') + from core.quality.source_map import quality_tier_for_source + tidal_quality = quality_tier_for_source('tidal', default='lossless') allow_fb = config_manager.get('tidal_download.allow_fallback', True) if tidal_quality == 'hires' and not allow_fb: task['error_message'] = ( diff --git a/core/downloads/status.py b/core/downloads/status.py index f425cceb..8735d4c4 100644 --- a/core/downloads/status.py +++ b/core/downloads/status.py @@ -94,6 +94,10 @@ class StatusDeps: run_async: Optional[Callable] = None on_download_completed: Optional[Callable[[str, str, bool], None]] = None get_persistent_download_history: Optional[Callable[[int], list[dict]]] = None + # Returns ALL library_history rows with verification_status in + # ('unverified', 'force_imported') — no recency limit, so historical + # entries are never buried by the general history tail cap. + get_unverified_download_history: Optional[Callable[[], list[dict]]] = None # Streaming sources the engine fallback applies to. Soulseek goes through @@ -796,6 +800,13 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: 'progress': progress, 'error': task.get('error_message'), 'verification_status': task.get('verification_status'), + # library_history row id (set at import) so the Unverified review + # queue can act on a still-live completed task before it becomes + # a persistent-history row. + 'history_id': task.get('history_id'), + # Real probed audio quality (mutagen-read from the actual file), + # surfaced so the Downloads page can show what was downloaded. + 'quality': task.get('quality') or '', 'retry_info': task.get('retry_info'), 'retry_trigger': task.get('retry_trigger'), 'batch_id': batch_id, @@ -812,6 +823,32 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: 'is_persistent_history': False, }) + # --- Unverified history (unconditional, no limit) --- + # Always load every library_history row that still needs human confirmation + # (verification_status IN ('unverified', 'force_imported')). This is NOT + # gated on len(items) < limit so that historical entries from past batches + # are visible even during a large active batch that would otherwise exhaust + # the limit before the history tail is read. Dedup against live tasks by + # identity so a track currently in post-processing isn't shown twice. + if deps.get_unverified_download_history is not None: + try: + unverified_entries = deps.get_unverified_download_history() or [] + except Exception as exc: + logger.debug("[Downloads] unverified history lookup failed: %s", exc) + unverified_entries = [] + + for entry in unverified_entries: + item = _build_history_download_item(entry) + identity = _download_identity(item.get('title'), item.get('artist'), item.get('album')) + if identity in live_identities: + continue + items.append(item) + live_identities.add(identity) + + # --- General recent-history tail (capped, recency-ordered) --- + # Fills in the completed/verified tail so the full Downloads list looks + # populated. Gated on len(items) < limit so a busy batch doesn't trigger + # an extra DB round-trip when we're already at capacity. if deps.get_persistent_download_history is not None and len(items) < limit: history_limit = min(limit - len(items), _PERSISTENT_HISTORY_TAIL_LIMIT) try: @@ -832,7 +869,14 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: live_identities.add(identity) appended_history += 1 - # Sort: active first (by priority), then by timestamp desc within each group + # Sort: active first (by priority), then by timestamp desc within each group. + # NOTE: the array order is presentation-only — the Downloads page filters + # client-side per tab. What matters is that EVERY live task is present: an + # earlier `items[:limit]` truncation (active-first) starved completed/failed/ + # unverified rows off the end during a busy batch, so those tabs stayed empty + # until the batch drained. `limit` now bounds only the persistent-history + # tail (handled above); live in-memory tasks are always returned in full + # (they're already bounded by the 5-min cleanup automation). items.sort(key=lambda x: (x['priority'], -x['timestamp'])) # Build batch summaries for the batch context panel @@ -860,7 +904,7 @@ def build_unified_downloads_response(limit: int, deps: StatusDeps) -> dict: return { 'success': True, - 'downloads': items[:limit], + 'downloads': items, 'total': len(items), 'batches': batch_summaries, 'timestamp': time.time(), diff --git a/core/downloads/task_worker.py b/core/downloads/task_worker.py index 71e49933..1b7b2bc1 100644 --- a/core/downloads/task_worker.py +++ b/core/downloads/task_worker.py @@ -54,6 +54,24 @@ def _cand_user_file(candidate): return getattr(candidate, 'username', None), getattr(candidate, 'filename', None) +def _best_quality_ordering(): + """Return ``(quality_first, targets)`` for the active search mode. + + In best-quality mode the candidate walk is ordered by the user's profile + quality rank (best→worst) instead of confidence-first. Fails closed to + priority-mode ordering on any error so a profile/DB hiccup never blocks a + download. See docs/superpowers/specs/2026-06-14-best-quality-search-mode-design.md. + """ + try: + from core.quality.selection import load_search_mode, load_profile_targets + if load_search_mode() == 'best_quality': + targets, _ = load_profile_targets() + return True, targets + except Exception as exc: + logger.debug("[Modal Worker] best-quality ordering unavailable: %s", exc) + return False, None + + def _try_cached_candidates(task_id, batch_id, track, deps): """Quarantine-retry fast path: attempt the already-found candidates before re-searching anything. @@ -91,7 +109,10 @@ def _try_cached_candidates(task_id, batch_id, track, deps): f"[Modal Worker] Quarantine retry: trying {len(remaining)} cached " f"candidate(s) before re-searching (task {task_id})" ) - return deps.attempt_download_with_candidates(task_id, remaining, track, batch_id) + _qf, _qt = _best_quality_ordering() + return deps.attempt_download_with_candidates( + task_id, remaining, track, batch_id, quality_first=_qf, quality_targets=_qt, + ) def _private_album_bundle_staging_miss_reason(batch_id: Optional[str], deps: Any) -> Optional[str]: @@ -369,6 +390,11 @@ def download_track_worker(task_id: str, batch_id: Optional[str], deps: TaskWorke logger.info(f"[Modal Worker] Generated {len(search_queries)} smart search queries for '{track.name}': {search_queries}") logger.info(f"[Modal Worker] About to start search loop for task {task_id} (track: '{track.name}')") + # Best-quality search mode: the orchestrator already pooled candidates + # across every source for each query, so order the candidate walk by the + # user's profile quality rank (best→worst). Computed once per task. + _best_quality, _quality_targets = _best_quality_ordering() + # 2. Sequential Query Search (matches GUI's start_search_worker_parallel logic) search_diagnostics = [] # Track what happened per query for detailed error messages all_raw_results = [] # Collect raw results across queries for candidate review modal @@ -495,7 +521,10 @@ def download_track_worker(task_id: str, batch_id: Optional[str], deps: TaskWorke download_tasks[task_id]['cached_candidates'] = candidates # Try to download with these candidates - success = deps.attempt_download_with_candidates(task_id, candidates, track, batch_id) + success = deps.attempt_download_with_candidates( + task_id, candidates, track, batch_id, + quality_first=_best_quality, quality_targets=_quality_targets, + ) if success: # Download initiated successfully - let the download monitoring system handle completion if batch_id: @@ -529,7 +558,10 @@ def download_track_worker(task_id: str, batch_id: Optional[str], deps: TaskWorke # === HYBRID FALLBACK: If primary source failed, try remaining sources directly === # The orchestrator's hybrid search stops at the first source with results, even if # those results all fail quality filtering. Try remaining sources individually. - if getattr(deps.download_orchestrator, 'mode', '') == 'hybrid': + # + # Best-quality mode already searched EVERY source per query (the pool), so this + # block would only re-search the same sources — skip it there. + if not _best_quality and getattr(deps.download_orchestrator, 'mode', '') == 'hybrid': try: orch = deps.download_orchestrator hybrid_order = getattr(orch, 'hybrid_order', None) or [] diff --git a/core/hifi_client.py b/core/hifi_client.py index 0b380132..73ce0c69 100644 --- a/core/hifi_client.py +++ b/core/hifi_client.py @@ -36,9 +36,34 @@ import requests as http_requests from utils.logging_config import get_logger from config.settings import config_manager from core.download_plugins.types import TrackResult, AlbumResult, DownloadStatus +from core.quality.source_map import quality_from_tidal_tier, quality_tier_for_source logger = get_logger("hifi_client") +# A media playlist whose total runtime is below this fraction of the track's +# real duration is a preview (some Monochrome instances only have 30s Tidal +# DOWNLOAD access — a 220s track comes back as ~30s of segments + ENDLIST). +_PREVIEW_DURATION_RATIO = 0.85 +_EXTINF_RE = re.compile(r'#EXTINF:\s*([0-9.]+)') + + +def hls_total_seconds(playlist_text: str) -> float: + """Sum the ``#EXTINF`` segment durations in an HLS media playlist.""" + return sum(float(x) for x in _EXTINF_RE.findall(playlist_text or '')) + + +def is_preview_playlist(playlist_s: float, track_s: float, + ratio: float = _PREVIEW_DURATION_RATIO) -> bool: + """True when the playlist runtime is far shorter than the track's real + duration (a preview). Returns False when either duration is unknown, so a + missing reference never false-positives — the post-download audio guard is + the safety net. + """ + if not playlist_s or not track_s or track_s <= 0: + return False + return playlist_s < track_s * ratio + + # HLS quality presets mapping to /trackManifests/ format parameters HLS_QUALITY_MAP = { 'hires': { @@ -697,7 +722,8 @@ class HiFiClient(DownloadSourcePlugin): return init_uri, segment_uris - def _get_hls_manifest(self, track_id: int, quality: str = 'lossless') -> Optional[Dict]: + def _get_hls_manifest(self, track_id: int, quality: str = 'lossless', + expected_duration_s: float = 0) -> Optional[Dict]: q_info = HLS_QUALITY_MAP.get(quality, HLS_QUALITY_MAP['lossless']) formats = q_info['formats'] @@ -751,6 +777,20 @@ class HiFiClient(DownloadSourcePlugin): logger.warning(f"Failed to fetch variant playlist for track {track_id}: {e}") return None + # Preview detection — some instances only have 30s Tidal DOWNLOAD + # access, returning a playlist far shorter than the real track. Decline + # it (and rotate the instance) so the orchestrator falls through to a + # real source instead of fetching a 30s file that gets quarantined. + playlist_s = hls_total_seconds(media_text) + if is_preview_playlist(playlist_s, expected_duration_s): + logger.warning( + f"HiFi manifest for track {track_id} ({quality}) is a " + f"{playlist_s:.0f}s preview of a {expected_duration_s:.0f}s track — " + f"declining this instance" + ) + self._rotate_instance(self._current_instance) + return None + if init_uri: logger.info(f"HiFi HLS manifest for track {track_id}: " f"init segment + {len(segment_uris)} segments ({quality})") @@ -873,13 +913,18 @@ class HiFiClient(DownloadSourcePlugin): loop = asyncio.get_event_loop() tracks = await loop.run_in_executor(None, lambda: self.search_raw(query)) - quality_key = config_manager.get('hifi_download.quality', 'lossless') + quality_key = quality_tier_for_source('hifi', default='lossless') q_info = HLS_QUALITY_MAP.get(quality_key, HLS_QUALITY_MAP['lossless']) + # HiFi is Tidal-backed; stamp the configured tier so the global + # ranker sees real sample_rate/bit_depth, not just 'flac'. + tier_quality = quality_from_tidal_tier(quality_key) + results = [] for t in tracks: try: tr = self._to_track_result(t, q_info) + tr.set_quality(tier_quality) results.append(tr) except Exception as e: logger.debug(f"Skipping track result conversion: {e}") @@ -950,7 +995,7 @@ class HiFiClient(DownloadSourcePlugin): ) def _download_sync(self, download_id: str, track_id: int, display_name: str) -> Optional[str]: - quality_key = config_manager.get('hifi_download.quality', 'lossless') + quality_key = quality_tier_for_source('hifi', default='lossless') chain = ['hires', 'lossless', 'high', 'low'] start = chain.index(quality_key) if quality_key in chain else 1 allow_fallback = config_manager.get('hifi_download.allow_fallback', True) @@ -958,21 +1003,26 @@ 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 track length, drives every preview/truncation guard here: + # * _get_hls_manifest's pre-download is_preview_playlist check + # * the pre-download is_short_audio manifest check + # * the post-download is_preview_download faked-header decode check + # Best-effort: a 0 here just disables the duration checks, 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 + expected_duration_s = expected_s # alias for _get_hls_manifest's param name for q_key in chain: if self.shutdown_check and self.shutdown_check(): logger.info("Shutdown detected, aborting HiFi download") return None - manifest_info = self._get_hls_manifest(track_id, quality=q_key) + manifest_info = self._get_hls_manifest(track_id, quality=q_key, + expected_duration_s=expected_duration_s) if ( not manifest_info or ( diff --git a/core/imports/file_ops.py b/core/imports/file_ops.py index 9923c39f..747d65e2 100644 --- a/core/imports/file_ops.py +++ b/core/imports/file_ops.py @@ -230,6 +230,9 @@ def get_audio_quality_string(file_path): if ext == ".flac": from mutagen.flac import FLAC audio = FLAC(file_path) + sr = getattr(audio.info, "sample_rate", 0) or 0 + if sr: + return f"FLAC {audio.info.bits_per_sample}bit/{sr / 1000:g}kHz" return f"FLAC {audio.info.bits_per_sample}bit" if ext == ".mp3": @@ -263,6 +266,106 @@ def get_audio_quality_string(file_path): return "" +def probe_audio_quality(file_path: str): + """Read the actual file and return an AudioQuality with real measured values. + + Uses mutagen to extract sample_rate, bit_depth, and bitrate from the + downloaded file — these are ground-truth values, not estimates. + Returns None when the file cannot be read. + """ + from core.quality.model import AudioQuality + try: + ext = os.path.splitext(file_path)[1].lower().lstrip('.') + + if ext == 'flac': + from mutagen.flac import FLAC + audio = FLAC(file_path) + return AudioQuality( + format='flac', + bitrate=audio.info.bitrate // 1000 if audio.info.bitrate else None, + sample_rate=audio.info.sample_rate, + bit_depth=audio.info.bits_per_sample, + ) + + if ext == 'mp3': + from mutagen.mp3 import MP3 + audio = MP3(file_path) + return AudioQuality( + format='mp3', + bitrate=audio.info.bitrate // 1000, + sample_rate=audio.info.sample_rate, + ) + + if ext in ('m4a', 'aac', 'mp4'): + from mutagen.mp4 import MP4 + audio = MP4(file_path) + # .m4a can carry AAC (lossy) OR ALAC (lossless) — only the real + # codec tells them apart, which is why extension-based classification + # defaults to 'aac' and we correct it here from the probed file. + codec = (getattr(audio.info, 'codec', '') or '').lower() + if 'alac' in codec: + return AudioQuality( + format='alac', + bitrate=audio.info.bitrate // 1000 if audio.info.bitrate else None, + sample_rate=audio.info.sample_rate, + bit_depth=getattr(audio.info, 'bits_per_sample', None) or None, + ) + return AudioQuality( + format='aac', + bitrate=audio.info.bitrate // 1000, + sample_rate=audio.info.sample_rate, + ) + + if ext == 'ogg': + from mutagen.oggvorbis import OggVorbis + audio = OggVorbis(file_path) + return AudioQuality( + format='ogg', + bitrate=audio.info.bitrate // 1000, + sample_rate=audio.info.sample_rate, + ) + + if ext == 'opus': + from mutagen.oggopus import OggOpus + audio = OggOpus(file_path) + return AudioQuality( + format='opus', + bitrate=audio.info.bitrate // 1000, + sample_rate=audio.info.sample_rate, + ) + + if ext in ('wav', 'aiff', 'aif'): + # AIFF must use mutagen.aiff.AIFF — WAVE() can't parse it and would + # raise, making the file fail open and silently bypass the quality + # filter. Both are uncompressed PCM, so they share the 'wav' tier. + if ext == 'wav': + from mutagen.wave import WAVE + audio = WAVE(file_path) + else: + from mutagen.aiff import AIFF + audio = AIFF(file_path) + return AudioQuality( + format='wav', + bitrate=audio.info.bitrate // 1000 if audio.info.bitrate else None, + sample_rate=audio.info.sample_rate, + bit_depth=getattr(audio.info, 'bits_per_sample', None), + ) + + if ext == 'wma': + from mutagen.asf import ASF + audio = ASF(file_path) + return AudioQuality( + format='wma', + bitrate=audio.info.bitrate // 1000 if audio.info.bitrate else None, + sample_rate=getattr(audio.info, 'sample_rate', None), + ) + + return None + except Exception as e: + logger.debug("probe_audio_quality failed for %s: %s", file_path, e) + return None + + def get_quality_tier_from_extension(file_path): """Classify a file extension into a quality tier.""" if not file_path: diff --git a/core/imports/guards.py b/core/imports/guards.py index f4519a5d..12bc9afb 100644 --- a/core/imports/guards.py +++ b/core/imports/guards.py @@ -105,30 +105,72 @@ def move_to_quarantine(file_path: str, context: dict, reason: str, automation_en def check_flac_bit_depth(file_path: str, context: dict) -> Optional[str]: - """Return a rejection message if a FLAC file violates the configured bit depth.""" - if not context.get("_audio_quality", "").startswith("FLAC"): + """Legacy wrapper — delegates to check_quality_target. + + Kept for callers that still pass trigger='bit_depth'; the new guard + covers bit_depth as part of the full quality target check. + """ + return check_quality_target(file_path, context) + + +def check_quality_target(file_path: str, context: dict) -> Optional[str]: + """Return a rejection message when the downloaded file does not satisfy + the user's quality priority list. + + Probes the actual file with mutagen (ground-truth sample_rate, + bit_depth, bitrate) and checks it against the profile's + ``ranked_targets``. Falls back gracefully when fallback_enabled=True. + + Works for all formats and all download sources — no Soulseek-specific + logic here. + """ + from core.imports.file_ops import probe_audio_quality + from core.quality.selection import targets_from_profile, quality_meets_profile + + # Master toggle (Settings → Import). When OFF, the quality check is skipped + # entirely and files import regardless of quality — the user opted out of + # quality-filtering on import. Default ON preserves existing behaviour. The + # library Quality Upgrade scanner still flags below-profile files either way. + if _get_config_manager().get("import.quality_filter_enabled", True) is False: + logger.debug( + "[QualityGuard] import.quality_filter_enabled=False — skipping quality " + "filter for %s", os.path.basename(file_path), + ) return None - quality_profile = MusicDatabase().get_quality_profile() - flac_config = quality_profile.get("qualities", {}).get("flac", {}) - flac_pref = flac_config.get("bit_depth", "any") - if flac_pref == "any": + aq = probe_audio_quality(file_path) + if aq is None: + logger.debug("[QualityGuard] Could not probe %s — skipping check", os.path.basename(file_path)) return None - actual_bits = context["_audio_quality"].replace("FLAC ", "").replace("bit", "") - if actual_bits == flac_pref: + profile = MusicDatabase().get_quality_profile() + targets, fallback_enabled = targets_from_profile(profile) + + if not targets: return None - flac_fallback = flac_config.get("bit_depth_fallback", True) downsample_enabled = _get_config_manager().get("lossy_copy.downsample_hires", False) + + matched = quality_meets_profile(aq, targets) + track_info = context.get("track_info", {}) track_name = track_info.get("name", os.path.basename(file_path)) + actual_label = aq.label() - if flac_fallback or downsample_enabled: - if downsample_enabled: - logger.info("[FLAC Downsample] Accepted %s-bit FLAC (will be downsampled to %s-bit): %s", actual_bits, flac_pref, track_name) - else: - logger.warning("[FLAC Fallback] Accepted %s-bit FLAC (preferred %s-bit): %s", actual_bits, flac_pref, track_name) + if matched: + logger.info("[QualityGuard] %s meets profile: %s", track_name, actual_label) return None - return f"FLAC bit depth mismatch: file is {actual_bits}-bit, preference is {flac_pref}-bit" + # No target matched + best_label = targets[0].label if targets else "?" + if fallback_enabled or downsample_enabled: + logger.warning( + "[QualityGuard] %s did not match any target (got %s, wanted %s) — accepting via fallback", + track_name, actual_label, best_label, + ) + return None + + return ( + f"Quality mismatch: file is {actual_label}, " + f"does not satisfy any configured target (best wanted: {best_label})" + ) diff --git a/core/imports/pipeline.py b/core/imports/pipeline.py index 700bc51a..e23f9590 100644 --- a/core/imports/pipeline.py +++ b/core/imports/pipeline.py @@ -34,9 +34,11 @@ from core.imports.context import ( ) from core.imports.file_integrity import check_audio_integrity, expected_duration_for_check, resolve_duration_tolerance from core.imports.filename import extract_track_number_from_filename -from core.imports.guards import check_flac_bit_depth, move_to_quarantine +from core.imports.guards import check_flac_bit_depth, check_quality_target, move_to_quarantine +from core.imports.silence import detect_broken_audio from core.imports.quarantine import ( approve_quarantine_entry, + delete_quarantine_entry, entry_id_from_quarantined_filename, list_quarantine_entries, ) @@ -160,7 +162,9 @@ def import_rejection_reason(context: dict) -> str | None: f"{context.get('_acoustid_failure_msg', 'fingerprint mismatch')}" ) if context.get('_bitdepth_rejected'): - return "rejected by bit-depth filter" + return "rejected by quality filter" + if context.get('_silence_rejected'): + return "rejected by audio guard (incomplete or silent audio)" if context.get('_race_guard_failed'): return "source file disappeared before import completed" return None @@ -353,6 +357,116 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta f"drift={integrity.checks.get('length_drift_s', 'n/a')})" ) + # Audio-completeness guard — runs right where the length is verified, + # BEFORE the AcoustID/quality gates, so a truncated file (container + # claims the full length but only ~30s actually decodes, e.g. HiFi/ + # Monochrome HLS assembly) or a mostly-silent file is caught regardless + # of its quality verdict and gets the right reason. Same quarantine + + # next-candidate retry pattern as the integrity check. + # + # Opt-in (default OFF): this is the one check that fully DECODES the file + # with ffmpeg, so it is the most CPU-expensive step in post-processing. + # Most preview/truncation cases are already caught at the source + # (HiFi/Qobuz have their own guards), so it stays off unless the user + # turns it on under Settings → Post-processing. + _audio_guard_enabled = config_manager.get('post_processing.audio_completeness_check', False) + _skip_audio = (not _audio_guard_enabled) or _should_skip_quarantine_check(context, 'silence') + audio_reason = None if _skip_audio else detect_broken_audio(file_path) + if audio_reason: + logger.error(f"[AudioGuard] Rejected {_basename}: {audio_reason}") + context['_silence_rejected'] = True + try: + quarantine_path = move_to_quarantine( + file_path, context, audio_reason, automation_engine, + trigger='silence', + ) + _mark_task_quarantined(context, quarantine_path) + logger.warning("File quarantined — incomplete/silent audio: %s", quarantine_path) + except Exception as quarantine_error: + logger.error(f"Quarantine failed ({quarantine_error}), deleting file: {file_path}") + try: + os.remove(file_path) + except Exception as del_error: + logger.debug("delete broken file fallback: %s", del_error) + + with matched_context_lock: + matched_downloads_context.pop(context_key, None) + + task_id = context.get('task_id') + batch_id = context.get('batch_id') + # Try the next-best candidate before giving up — same pattern as + # the integrity / AcoustID / quality failures. + if _requeue_quarantined_task_for_retry(task_id, batch_id, 'silence'): + logger.info( + "Incomplete/silent audio for task %s — retrying next-best candidate: %s", + task_id, audio_reason, + ) + return + if task_id: + with tasks_lock: + if task_id in download_tasks: + download_tasks[task_id]['status'] = 'failed' + download_tasks[task_id]['error_message'] = f"Audio guard: {audio_reason}" + if task_id and batch_id: + _notify_download_completed(batch_id, task_id, success=False) + return + + # QUALITY GATE — runs BEFORE AcoustID so (1) a wrong-quality file is + # rejected without paying for an AcoustID fingerprint, and (2) the real + # audio quality is recorded on the context (→ quarantine sidecar) for + # EVERY trigger, so it's known when reviewing/approving any quarantined + # file. force_import still never fires on a quality mismatch. + context['_audio_quality'] = get_audio_quality_string(file_path) + if context['_audio_quality']: + logger.info(f"Audio quality detected: {context['_audio_quality']}") + + _skip_quality = _should_skip_quarantine_check(context, 'bit_depth') or \ + _should_skip_quarantine_check(context, 'quality') + rejection_reason = None if _skip_quality else check_quality_target(file_path, context) + if _skip_quality: + logger.info(f"[QualityGuard] Skipped (user approval) for {_basename}") + if rejection_reason: + try: + quarantine_path = move_to_quarantine( + file_path, + context, + rejection_reason, + automation_engine, + trigger='quality', + ) + _mark_task_quarantined(context, quarantine_path) + logger.info(f"File quarantined due to quality mismatch: {quarantine_path}") + except Exception as quarantine_error: + logger.error(f"Quarantine failed ({quarantine_error}), deleting file: {file_path}") + try: + os.remove(file_path) + except Exception as e: + logger.debug("delete quarantine fallback: %s", e) + + context['_bitdepth_rejected'] = True + task_id = context.get('task_id') + batch_id = context.get('batch_id') + with matched_context_lock: + matched_downloads_context.pop(context_key, None) + + # Try the next-best candidate before giving up — same pattern + # as AcoustID and integrity failures. + if _requeue_quarantined_task_for_retry(task_id, batch_id, 'quality'): + logger.info( + "Quality mismatch for task %s — retrying next-best candidate: %s", + task_id, rejection_reason, + ) + return + + if task_id: + with tasks_lock: + if task_id in download_tasks: + download_tasks[task_id]['status'] = 'failed' + download_tasks[task_id]['error_message'] = f"Quality filter: {rejection_reason}" + if task_id and batch_id: + _notify_download_completed(batch_id, task_id, success=False) + return + _skip_acoustid = _should_skip_quarantine_check(context, 'acoustid') if _skip_acoustid: logger.info(f"[AcoustID] Skipped (user approval) for {_basename}") @@ -390,14 +504,33 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta logger.info(f"AcoustID verification result: {verification_result.value} - {verification_msg}") context['_acoustid_result'] = verification_result.value - if verification_result == VerificationResult.FAIL: + # Fail-closed mode: when the user requires a hard AcoustID + # PASS, a SKIP (ran but couldn't confirm — no fingerprint + # match / cross-script metadata) is treated like a FAIL: + # quarantine + try the next candidate, instead of importing + # an unverified file. ERROR (rate-limit / infra) is NOT + # blocked — that would stall the whole pipeline during an + # outage; those still import with their existing flag. + require_verified = config_manager.get('acoustid.require_verified', False) + _skip_as_fail = ( + require_verified + and verification_result == VerificationResult.SKIP + ) + if _skip_as_fail: + verification_msg = ( + f"AcoustID could not confirm the track and 'require verified' " + f"is on — rejecting unverified file ({verification_msg})" + ) + logger.warning("[AcoustID] Require-verified: SKIP treated as FAIL — %s", verification_msg) + + if verification_result == VerificationResult.FAIL or _skip_as_fail: try: quarantine_path = move_to_quarantine( file_path, context, verification_msg, automation_engine, - trigger='acoustid', + trigger='acoustid_unverified' if _skip_as_fail else 'acoustid', ) _mark_task_quarantined(context, quarantine_path) logger.error(f"File quarantined due to verification failure: {quarantine_path}") @@ -577,48 +710,6 @@ def post_process_matched_download(context_key, context, file_path, runtime, meta album_info.get('album_name', 'None'), ) - context['_audio_quality'] = get_audio_quality_string(file_path) - if context['_audio_quality']: - logger.info(f"Audio quality detected: {context['_audio_quality']}") - - _skip_bit_depth = _should_skip_quarantine_check(context, 'bit_depth') - rejection_reason = None if _skip_bit_depth else check_flac_bit_depth(file_path, context) - if _skip_bit_depth: - logger.info(f"[BitDepth] Skipped (user approval) for {_basename}") - if rejection_reason: - try: - quarantine_path = move_to_quarantine( - file_path, - context, - rejection_reason, - automation_engine, - trigger='bit_depth', - ) - _mark_task_quarantined(context, quarantine_path) - logger.info(f"File quarantined due to bit depth filter: {quarantine_path}") - except Exception as quarantine_error: - logger.error(f"Quarantine failed ({quarantine_error}), deleting file: {file_path}") - try: - os.remove(file_path) - except Exception as e: - logger.debug("delete quarantine fallback: %s", e) - - context['_bitdepth_rejected'] = True - with matched_context_lock: - if context_key in matched_downloads_context: - del matched_downloads_context[context_key] - - task_id = context.get('task_id') - batch_id = context.get('batch_id') - if task_id: - with tasks_lock: - if task_id in download_tasks: - download_tasks[task_id]['status'] = 'failed' - download_tasks[task_id]['error_message'] = f"Bit depth filter: {rejection_reason}" - if task_id and batch_id: - _notify_download_completed(batch_id, task_id, success=False) - return - file_ext = os.path.splitext(file_path)[1] clean_track_name = get_import_clean_title( context, @@ -987,6 +1078,22 @@ def post_process_matched_download_with_verification(context_key, context, file_p if original_batch_id: context['batch_id'] = original_batch_id + # Quality / audio-guard quarantine. Unlike acoustid/integrity (whose + # retry+fail is driven by THIS wrapper below), the inner pipeline fully + # owns the quality and audio-guard outcome: it quarantines the file and + # then either re-queues the next-best candidate or marks the task failed + # and notifies. The wrapper must NOT continue to the "assume success" + # fall-through — that marked the quarantined file Completed, so the same + # track showed in BOTH Completed and Quarantine (e.g. an MP3-VBR rejected + # by a FLAC-only profile). It must also NOT mark it failed here, which + # would clobber a successful next-candidate retry. Just return. + if context.get('_bitdepth_rejected') or context.get('_silence_rejected'): + logger.info( + f"Task {task_id} quarantined by the quality/audio guard — inner " + f"pipeline already handled retry/fail; wrapper not marking completed" + ) + return + if context.get('_race_guard_failed'): logger.info(f"Race guard: source file gone for task {task_id} — marking as failed") with tasks_lock: @@ -1000,6 +1107,33 @@ def post_process_matched_download_with_verification(context_key, context, file_p return if context.get('_acoustid_quarantined'): + # Race-condition guard: if the user approved an alternative quarantine + # entry for this track while this download was already in flight, the + # pipeline will have created a new quarantine entry for the just-finished + # file. Delete that entry and exit — the user's choice already won. + _approved_alt = False + if task_id: + with tasks_lock: + _ct = download_tasks.get(task_id) + if isinstance(_ct, dict) and _ct.get('_quarantine_approved_alternative'): + _approved_alt = True + if _approved_alt: + _eid = context.get('_quarantine_entry_id') + if _eid: + try: + from core.imports.guards import _get_config_manager + _dl_dir = _get_config_manager().get('soulseek.download_path', './downloads') + _qdir = os.path.join(docker_resolve_path(_dl_dir), 'ss_quarantine') + delete_quarantine_entry(_qdir, _eid) + logger.info( + f"[Quarantine] Discarded late entry {_eid} — task {task_id} " + f"was cancelled by prior quarantine approval" + ) + except Exception as _dqe: + logger.debug(f"[Quarantine] Could not clean up late entry {_eid}: {_dqe}") + _notify_download_completed(batch_id, task_id, success=False) + return + failure_msg = context.get('_acoustid_failure_msg', 'AcoustID verification failed') # Before failing outright, try the next-best candidate. The wrong # file was just quarantined; re-running the worker (with the bad @@ -1116,6 +1250,10 @@ def post_process_matched_download_with_verification(context_key, context, file_p _mark_task_completed(task_id, context.get('track_info')) if context.get('_verification_status'): download_tasks[task_id]['verification_status'] = context['_verification_status'] + if context.get('_history_id'): + download_tasks[task_id]['history_id'] = context['_history_id'] + if context.get('_audio_quality'): + download_tasks[task_id]['quality'] = context['_audio_quality'] with matched_context_lock: if context_key in matched_downloads_context: del matched_downloads_context[context_key] @@ -1130,6 +1268,10 @@ def post_process_matched_download_with_verification(context_key, context, file_p download_tasks[task_id]['metadata_enhanced'] = True if context.get('_verification_status'): download_tasks[task_id]['verification_status'] = context['_verification_status'] + if context.get('_history_id'): + download_tasks[task_id]['history_id'] = context['_history_id'] + if context.get('_audio_quality'): + download_tasks[task_id]['quality'] = context['_audio_quality'] redownload_ctx = download_tasks[task_id].get('_redownload_context') with matched_context_lock: diff --git a/core/imports/quarantine.py b/core/imports/quarantine.py index 7dd43be6..dd249f4d 100644 --- a/core/imports/quarantine.py +++ b/core/imports/quarantine.py @@ -168,16 +168,15 @@ def quarantine_group_key( fetch — NOT the downloaded file's own tags. That matters: the file's metadata is frequently *wrong* (that's why it failed acoustid / integrity), whereas the target is fixed and identical across every - alternative for one song (they're all Soulseek uploads of the *same* - source track), so grouping by it is an exact relationship, not a fuzzy - metadata guess. + alternative for one song. - Prefers a stable target-track id from the sidecar `context.track_info` - when present — isrc, then source id, then uri — since those are exact - and constant across siblings. Falls back to the normalized - artist|track name only for legacy/thin sidecars that carry no context. - Keys are kind-prefixed so an id-based key never collides with a - name-based one. + Uses ISRC when available (truly universal across sources and batches). + Falls back to normalized artist|track name, which is stable across + different batches and sources. + + Source-specific IDs (Spotify track id, Qobuz id, uri) are intentionally + NOT used: the same song imported from different playlists or sources gets + different source IDs, so id-based keys break cross-batch sibling matching. Returns ``None`` when nothing identifies the target (no usable id and both name fields empty). Callers treat a ``None`` key as "its own @@ -191,12 +190,6 @@ def quarantine_group_key( isrc = str(ti.get("isrc") or "").strip().lower() if isrc: return f"isrc:{isrc}" - tid = str(ti.get("id") or "").strip() - if tid: - return f"id:{tid}" - uri = str(ti.get("uri") or "").strip() - if uri: - return f"uri:{uri}" artist = " ".join(str(expected_artist or "").split()).lower() track = " ".join(str(expected_track or "").split()).lower() if not artist and not track: @@ -302,6 +295,10 @@ def list_quarantine_entries(quarantine_dir: str) -> List[Dict[str, Any]]: "source_username": source_username, "source_filename": source_filename, "thumb_url": _extract_context_thumb(ctx), + # Real probed audio quality (recorded on the context before the + # quality/AcoustID gates) so the review UI shows what the file + # actually is when deciding to approve/delete. + "quality": ctx.get("_audio_quality", "") if isinstance(ctx, dict) else "", } ) diff --git a/core/imports/side_effects.py b/core/imports/side_effects.py index 867329c3..f2e21efa 100644 --- a/core/imports/side_effects.py +++ b/core/imports/side_effects.py @@ -252,7 +252,7 @@ def record_library_history_download(context: Dict[str, Any]) -> None: origin, origin_context = derive_download_origin(context) db = get_database() - db.add_library_history_entry( + _history_id = db.add_library_history_entry( event_type="download", title=title, artist_name=artist_name, @@ -270,6 +270,10 @@ def record_library_history_download(context: Dict[str, Any]) -> None: origin_context=origin_context, verification_status=context.get("_verification_status"), ) + # Stash the row id so the live download task can link to its + # library_history row (the Unverified review queue needs it). + if isinstance(_history_id, int) and _history_id > 0: + context["_history_id"] = _history_id except Exception as e: logger.debug("library history record failed: %s", e) diff --git a/core/imports/silence.py b/core/imports/silence.py new file mode 100644 index 00000000..ecbe048f --- /dev/null +++ b/core/imports/silence.py @@ -0,0 +1,277 @@ +"""Audio-completeness guard — detect files whose container duration looks +right but whose REAL audio is far shorter, or mostly silence. + +Motivating bug: HiFi/Monochrome HLS assembly can yield a file whose container +claims the full track length (e.g. 3:08) while only ~30s of audio actually +decodes — the rest is missing. The duration-agreement and quality guards both +pass (mutagen reads the container's 3:08 and the format/bitrate are fine), so +nothing catches it until you listen. ffmpeg's ``time=`` even reports 0 with no +error on such a file, so the robust signal is to DECODE the audio and compare +the real duration (sample count / sample rate, via ``astats``) against the +container duration. A separate ``silencedetect`` pass also flags genuine +silence-padding. + +The parsers here are pure and unit-tested; the ffmpeg invocations are +integration glue that fails open (returns None) when ffmpeg or mutagen can't +run, so a tooling problem never blocks a legitimate import. +""" + +from __future__ import annotations + +import re +import subprocess +from typing import Optional + +from utils.logging_config import get_logger + +logger = get_logger("imports.silence") + +# Real decoded audio must cover at least this fraction of the container +# duration. A legit file decodes to ~100% (encoder padding aside); a truncated +# file decodes to a small fraction (the Blossom file: 30s of a 188s container +# = 16%). 0.85 leaves generous headroom against false positives. +DEFAULT_MIN_DURATION_RATIO = 0.85 + +_SAMPLES_RE = re.compile(r"Number of samples:\s*([0-9]+)") + +# Defaults: treat audio below -50 dB lasting >= 2s as silence, and reject when +# more than half the track is silent. A normal song — even with quiet intros/ +# outros — sits far below 0.5; a 30s-real + padded-silence file sits near 0.85. +DEFAULT_NOISE_DB = -50 +DEFAULT_MIN_SILENCE_S = 2.0 +DEFAULT_THRESHOLD = 0.5 + +_SILENCE_DURATION_RE = re.compile(r"silence_duration:\s*([0-9]+(?:\.[0-9]+)?)") + + +def silence_ratio_from_output(ffmpeg_stderr: str, total_duration_s: float) -> float: + """Fraction of *total_duration_s* covered by detected silence. + + Sums every ``silence_duration: X`` reported by ffmpeg ``silencedetect`` + and divides by the track length. Capped at 1.0; returns 0.0 when the + duration is unknown/zero or no silence was reported. + """ + if not total_duration_s or total_duration_s <= 0: + return 0.0 + total_silence = sum(float(m) for m in _SILENCE_DURATION_RE.findall(ffmpeg_stderr or "")) + if total_silence <= 0: + return 0.0 + return min(total_silence / total_duration_s, 1.0) + + +def is_mostly_silent_reason( + ffmpeg_stderr: str, + total_duration_s: float, + *, + threshold: float = DEFAULT_THRESHOLD, +) -> Optional[str]: + """Return a rejection reason when the silent fraction meets *threshold*.""" + ratio = silence_ratio_from_output(ffmpeg_stderr, total_duration_s) + if ratio >= threshold: + pct = round(ratio * 100) + audible_s = round(total_duration_s * (1 - ratio)) + return ( + f"Audio is mostly silent: {pct}% silence (only ~{audible_s}s audible of " + f"{round(total_duration_s)}s) — likely a truncated/preview file padded " + f"to full length" + ) + return None + + +def _ffmpeg_available() -> bool: + try: + subprocess.run( + ["ffmpeg", "-version"], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + timeout=10, check=True, + ) + return True + except (FileNotFoundError, subprocess.SubprocessError, OSError): + return False + + +def _probe_duration_s(file_path: str) -> Optional[float]: + try: + from mutagen import File as MutagenFile + audio = MutagenFile(file_path) + if audio and audio.info and getattr(audio.info, "length", None): + return float(audio.info.length) + except Exception as exc: # pragma: no cover - defensive + logger.debug("silence guard duration probe failed for %s: %s", file_path, exc) + return None + + +def detect_mostly_silent( + file_path: str, + *, + threshold: float = DEFAULT_THRESHOLD, + noise_db: int = DEFAULT_NOISE_DB, + min_silence_s: float = DEFAULT_MIN_SILENCE_S, +) -> Optional[str]: + """Run ffmpeg ``silencedetect`` over *file_path* and return a rejection + reason when the file is mostly silence, else None. + + Fails open: returns None when ffmpeg/mutagen are unavailable or error, so + a tooling problem never quarantines a legitimate file. + """ + if not _ffmpeg_available(): + logger.debug("silence guard skipped — ffmpeg not available") + return None + + total_duration_s = _probe_duration_s(file_path) + if not total_duration_s: + return None + + try: + proc = subprocess.run( + [ + "ffmpeg", "-hide_banner", "-nostats", "-i", file_path, + "-af", f"silencedetect=noise={noise_db}dB:d={min_silence_s}", + "-f", "null", "-", + ], + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, + timeout=120, + ) + except (subprocess.SubprocessError, OSError) as exc: + logger.debug("silence guard ffmpeg run failed for %s: %s", file_path, exc) + return None + + stderr = proc.stderr.decode("utf-8", errors="replace") if proc.stderr else "" + return is_mostly_silent_reason(stderr, total_duration_s, threshold=threshold) + + +# ── Truncation: real decoded duration vs container duration ──────────────── + +def measured_duration_from_astats(astats_stderr: str, sample_rate: int) -> Optional[float]: + """Real decoded audio duration in seconds from ffmpeg ``astats`` output. + + ``astats`` reports the per-channel ``Number of samples``; dividing by the + sample rate gives the true decoded length. Returns None when the sample + count or sample rate is unavailable. + """ + if not sample_rate or sample_rate <= 0: + return None + m = _SAMPLES_RE.search(astats_stderr or "") + if not m: + return None + return int(m.group(1)) / float(sample_rate) + + +def incomplete_audio_reason( + measured_s: Optional[float], + container_s: Optional[float], + *, + min_ratio: float = DEFAULT_MIN_DURATION_RATIO, +) -> Optional[str]: + """Return a rejection reason when the real decoded duration falls short of + the container duration (a truncated file whose metadata over-states length). + """ + if not measured_s or not container_s or container_s <= 0: + return None + if measured_s >= container_s * min_ratio: + return None + pct = round(measured_s / container_s * 100) + return ( + f"Incomplete audio: only ~{round(measured_s)}s actually decodes of a " + f"{round(container_s)}s file ({pct}%) — truncated/broken download " + f"(container duration over-states the real audio)" + ) + + +def _measured_audio_duration_s(file_path: str, sample_rate: int) -> Optional[float]: + try: + proc = subprocess.run( + ["ffmpeg", "-hide_banner", "-nostats", "-i", file_path, + "-af", "astats=metadata=1", "-f", "null", "-"], + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, timeout=120, + ) + except (subprocess.SubprocessError, OSError) as exc: + logger.debug("astats run failed for %s: %s", file_path, exc) + return None + stderr = proc.stderr.decode("utf-8", errors="replace") if proc.stderr else "" + return measured_duration_from_astats(stderr, sample_rate) + + +def detect_incomplete_audio( + file_path: str, + *, + min_ratio: float = DEFAULT_MIN_DURATION_RATIO, +) -> Optional[str]: + """Decode the file and reject when the real audio is far shorter than the + container claims. Fails open when ffmpeg/mutagen are unavailable. + """ + if not _ffmpeg_available(): + return None + try: + from mutagen import File as MutagenFile + audio = MutagenFile(file_path) + if not (audio and audio.info): + return None + container_s = float(getattr(audio.info, "length", 0) or 0) + sample_rate = int(getattr(audio.info, "sample_rate", 0) or 0) + except Exception as exc: # pragma: no cover - defensive + logger.debug("container probe failed for %s: %s", file_path, exc) + return None + + measured_s = _measured_audio_duration_s(file_path, sample_rate) + return incomplete_audio_reason(measured_s, container_s, min_ratio=min_ratio) + + +def detect_broken_audio( + file_path: str, + *, + min_ratio: float = DEFAULT_MIN_DURATION_RATIO, + threshold: float = DEFAULT_THRESHOLD, + noise_db: int = DEFAULT_NOISE_DB, + min_silence_s: float = DEFAULT_MIN_SILENCE_S, +) -> Optional[str]: + """Combined post-download audio guard: reject a file that is truncated + (real audio far shorter than the container) or mostly silence. Returns the + first failure reason, or None when the audio looks complete. + + Runs a SINGLE ffmpeg decode pass with both the ``astats`` (truncation) and + ``silencedetect`` (silence) filters chained — one decode of the file feeds + both checks instead of two full decodes. Halves the CPU cost versus running + ``detect_incomplete_audio`` and ``detect_mostly_silent`` back to back. + + Fails open: returns None when ffmpeg/mutagen are unavailable or error, so a + tooling problem never quarantines a legitimate file. + """ + if not _ffmpeg_available(): + logger.debug("audio guard skipped — ffmpeg not available") + return None + + try: + from mutagen import File as MutagenFile + audio = MutagenFile(file_path) + if not (audio and audio.info): + return None + container_s = float(getattr(audio.info, "length", 0) or 0) + sample_rate = int(getattr(audio.info, "sample_rate", 0) or 0) + except Exception as exc: # pragma: no cover - defensive + logger.debug("container probe failed for %s: %s", file_path, exc) + return None + + try: + proc = subprocess.run( + [ + "ffmpeg", "-hide_banner", "-nostats", "-i", file_path, + "-af", f"astats=metadata=1,silencedetect=noise={noise_db}dB:d={min_silence_s}", + "-f", "null", "-", + ], + stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, timeout=120, + ) + except (subprocess.SubprocessError, OSError) as exc: + logger.debug("audio guard ffmpeg run failed for %s: %s", file_path, exc) + return None + + stderr = proc.stderr.decode("utf-8", errors="replace") if proc.stderr else "" + + # Truncation check first (real audio far shorter than the container). + measured_s = measured_duration_from_astats(stderr, sample_rate) + reason = incomplete_audio_reason(measured_s, container_s, min_ratio=min_ratio) + if reason: + return reason + + # Then silence-padding (mostly-silent file). + return is_mostly_silent_reason(stderr, container_s, threshold=threshold) diff --git a/core/library/path_resolver.py b/core/library/path_resolver.py index 712ec3c2..6ff3e6b5 100644 --- a/core/library/path_resolver.py +++ b/core/library/path_resolver.py @@ -138,10 +138,25 @@ def _collect_base_dirs( except Exception as e: logger.debug("music paths read failed: %s", e) + # Normalize to absolute forms so resolution does NOT depend on the calling + # thread's CWD. A relative config like "./Transfer" otherwise only resolves + # when os.path.isdir("./Transfer") happens to be true from the current CWD — + # which fails in background workers whose CWD isn't the app root, leaving + # base_dirs empty and every track "unresolved". For each candidate we try + # the raw form first (cheap, preserves an already-absolute path), then its + # os.path.abspath() form so "./Transfer" → "/app/Transfer". + expanded: list[str] = [] + for c in candidates: + if not c: + continue + expanded.append(c) + if not os.path.isabs(c): + expanded.append(os.path.abspath(c)) + # De-duplicate while preserving order, drop empties / non-existent dirs. seen: set[str] = set() out: list[str] = [] - for c in candidates: + for c in expanded: if not c or c in seen: continue seen.add(c) @@ -224,10 +239,23 @@ def resolve_library_file_path_with_diagnostic( if not base_dirs: return None, attempt - # Skip index 0 to avoid drive-letter / leading-slash artifacts - # (e.g. "E:" or "" from a leading "/"). + # Try progressively shorter path suffixes against each base dir. + # + # Start at index 0 so a clean RELATIVE library path is tried in FULL first. + # SoulSync's own library scanner stores paths like + # "Asketa/Another Side/01 - Track.flac" (no leading slash) — index 0 is the + # artist folder and dropping it (the old range(1, ...)) meant the artist + # segment was never joined, so nothing under transfer/ ever resolved and + # every track looked unreadable to the quality scanner. + # + # For ABSOLUTE media-server paths ("/music/Artist/Album/track.flac") index 0 + # is the empty leading segment and i=0 yields os.path.join(base, "", ...) == + # base/Artist/... which simply won't exist and harmlessly falls through to + # i=1 ("music/...") etc. A Windows drive part ("E:") at i=0 likewise just + # fails on POSIX and falls through. So starting at 0 is safe for every form + # and only ADDS the relative-full-path match that was missing. for base in base_dirs: - for i in range(1, len(path_parts)): + for i in range(0, len(path_parts)): candidate = os.path.join(base, *path_parts[i:]) if os.path.exists(candidate): return candidate, attempt diff --git a/core/qobuz_client.py b/core/qobuz_client.py index cf4fbf9b..73a0cae1 100644 --- a/core/qobuz_client.py +++ b/core/qobuz_client.py @@ -29,6 +29,7 @@ from config.settings import config_manager # Import Soulseek data structures for drop-in replacement compatibility from core.download_plugins.types import TrackResult, AlbumResult, DownloadStatus +from core.quality.source_map import quality_from_qobuz, quality_tier_for_source logger = get_logger("qobuz_client") @@ -987,7 +988,7 @@ class QobuzClient(DownloadSourcePlugin): return ([], []) # Get configured quality for display - quality_key = config_manager.get('qobuz.quality', 'lossless') + quality_key = quality_tier_for_source('qobuz', default='lossless') quality_info = QOBUZ_QUALITY_MAP.get(quality_key, QOBUZ_QUALITY_MAP['lossless']) track_results = [] @@ -1072,6 +1073,14 @@ class QobuzClient(DownloadSourcePlugin): track_number=track.get('track_number'), ) + # Stamp real API quality so the global ranker sees actual + # sample_rate/bit_depth. Hi-res only when the track itself is + # hires-streamable; otherwise it streams as CD-quality FLAC. + if hires_streamable and max_bit_depth >= 24: + track_result.set_quality(quality_from_qobuz(max_sample_rate, max_bit_depth)) + else: + track_result.set_quality(quality_from_qobuz(44.1, 16)) + return track_result # ===================== Download ===================== @@ -1181,7 +1190,7 @@ class QobuzClient(DownloadSourcePlugin): try: # Determine quality - quality_key = config_manager.get('qobuz.quality', 'lossless') + quality_key = quality_tier_for_source('qobuz', default='lossless') quality_info = QOBUZ_QUALITY_MAP.get(quality_key, QOBUZ_QUALITY_MAP['lossless']) # Quality fallback chain: hires_max → hires → lossless → mp3 diff --git a/core/quality/__init__.py b/core/quality/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/core/quality/model.py b/core/quality/model.py new file mode 100644 index 00000000..4fe1c0f9 --- /dev/null +++ b/core/quality/model.py @@ -0,0 +1,284 @@ +"""Source-agnostic audio quality model. + +Every download source maps its result into ``AudioQuality``. +The ``QualityTarget`` list in the user's profile defines the +priority order (1st choice, 2nd choice, …). ``rank_candidate`` +scores any ``AudioQuality`` against that list so the same +logic drives Soulseek, Tidal, Deezer, torrent — no per-source +quality pipelines needed. + +Soulseek attribute type codes (Soulseek protocol spec): + 0 = bitrate (kbps) + 1 = duration (seconds) + 2 = VBR flag + 4 = sample rate (Hz) — FLAC / WAV only + 5 = bit depth — FLAC / WAV only +""" + +from __future__ import annotations + +from dataclasses import dataclass, field +from typing import List, Optional, Tuple + + +@dataclass +class AudioQuality: + """Unified audio quality descriptor — source-agnostic.""" + + format: str # 'flac', 'mp3', 'aac', 'ogg', 'wav', 'unknown' + bitrate: Optional[int] = None # kbps + sample_rate: Optional[int] = None # Hz (e.g. 44100, 96000, 192000) + bit_depth: Optional[int] = None # bits per sample (16, 24, 32) + + def tier_score(self) -> float: + """Continuous score for ranking within a matched target bucket. + Higher = better. Used as a tiebreaker after target-list matching. + """ + # NOTE: this only orders the *fallback* path (nothing matched a ranked + # target) and tie-breaks candidates of the SAME format within one + # matched target. Cross-format PRIORITY is decided solely by the user's + # ranked-target list (target index), never by these numbers. + format_base: dict[str, float] = { + 'flac': 100.0, + 'alac': 98.0, # lossless (Apple) + 'wav': 95.0, + 'ogg': 70.0, + 'opus': 65.0, + 'aac': 60.0, + 'mp3': 50.0, + 'wma': 30.0, + } + base = format_base.get(self.format.lower(), 10.0) + + if self.format.lower() in ('flac', 'wav'): + sr = self.sample_rate or 44100 + bd = self.bit_depth or 16 + # sample-rate contribution: 44.1 kHz = 0, 192 kHz = +20 + sr_score = min(sr / 192_000, 1.0) * 20 + # bit-depth contribution: 16-bit = 0, 24-bit = +10 + bd_score = max(bd - 16, 0) / 8 * 10 + return base + sr_score + bd_score + else: + br = self.bitrate or 0 + return base + min(br / 320, 1.0) * 10 + + def matches_target(self, target: QualityTarget) -> bool: + """True when this quality satisfies every constraint in *target*.""" + if target.format and target.format.lower() != self.format.lower(): + return False + if target.min_bitrate and (self.bitrate or 0) < target.min_bitrate: + return False + if target.min_sample_rate: + if self.sample_rate is not None: + if self.sample_rate < target.min_sample_rate: + return False + else: + # No sample-rate metadata (common on slskd FLAC). Use the kbps + # heuristic when a bitrate is present; otherwise we CANNOT + # confirm the spec, so fail the strict target rather than + # over-claim it — an unknown-spec FLAC must not outrank a known + # 16/44 FLAC under a hi-res target (#896 review #4). It falls to + # the plain-flac bucket instead. + # 16-bit/44.1 kHz ≈ 1411 kbps; 24-bit/96 kHz ≈ 4608 kbps. + if self.format.lower() == 'flac' and self.bitrate: + required_kbps = _sample_rate_to_min_kbps(target.min_sample_rate, target.bit_depth or 24) + if self.bitrate < required_kbps: + return False + else: + return False + if target.bit_depth: + if self.bit_depth is not None: + if self.bit_depth < target.bit_depth: + return False + else: + # No bit-depth metadata. A hi-res (>=24-bit) target needs proof: + # use the kbps heuristic if a bitrate is present, else fail + # rather than over-claim. The 16-bit baseline still matches an + # unknown-spec FLAC (any FLAC is at least CD quality). #896 review #4. + if self.format.lower() == 'flac' and target.bit_depth >= 24: + if self.bitrate: + if self.bitrate < 1450: + return False + else: + return False + return True + + def label(self) -> str: + """Human-readable label, e.g. 'FLAC 24-bit/192kHz' or 'MP3 320kbps'.""" + fmt = self.format.upper() + if self.format.lower() in ('flac', 'wav'): + bd = f"{self.bit_depth}-bit/" if self.bit_depth else "" + sr = f"{self.sample_rate // 1000}kHz" if self.sample_rate else "" + detail = f" {bd}{sr}".rstrip() + return f"{fmt}{detail}" if detail.strip() else fmt + else: + br = f" {self.bitrate}kbps" if self.bitrate else "" + return f"{fmt}{br}" + + @classmethod + def from_slskd_file(cls, file_data: dict, extension: str) -> 'AudioQuality': + """Build from a raw slskd API file entry. + + slskd exposes Soulseek protocol file attributes as: + ``{"attributes": [{"type": 4, "value": 96000}, {"type": 5, "value": 24}, ...]}`` + """ + attrs = {a['type']: a['value'] for a in file_data.get('attributes', [])} + return cls( + format=extension.lower().lstrip('.'), + bitrate=file_data.get('bitRate') or attrs.get(0), + sample_rate=attrs.get(4), + bit_depth=attrs.get(5), + ) + + @classmethod + def from_tier(cls, fmt: str, bitrate: int, sample_rate: Optional[int] = None, bit_depth: Optional[int] = None) -> 'AudioQuality': + """Build from a hardcoded quality tier (Tidal, Deezer, Qobuz).""" + return cls(format=fmt, bitrate=bitrate, sample_rate=sample_rate, bit_depth=bit_depth) + + @classmethod + def from_extension_and_bitrate(cls, extension: str, bitrate: Optional[int]) -> 'AudioQuality': + """Minimal constructor when only format + bitrate are known (torrent, YouTube).""" + return cls(format=extension.lower().lstrip('.'), bitrate=bitrate) + + +@dataclass +class QualityTarget: + """One ranked entry in the user's quality priority list.""" + + label: str = "" + format: Optional[str] = None # 'flac', 'mp3', 'aac', … + bit_depth: Optional[int] = None # 16, 24 + min_sample_rate: Optional[int] = None # Hz + min_bitrate: Optional[int] = None # kbps (lossy) + + def to_dict(self) -> dict: + return {k: v for k, v in self.__dict__.items() if v not in (None, "")} + + @classmethod + def from_dict(cls, d: dict) -> 'QualityTarget': + known = {f.name for f in cls.__dataclass_fields__.values()} # type: ignore[attr-defined] + return cls(**{k: v for k, v in d.items() if k in known}) + + +# ── Default priority list ────────────────────────────────────────────────────── + +DEFAULT_RANKED_TARGETS: List[QualityTarget] = [ + QualityTarget(label='FLAC 24-bit/192kHz', format='flac', bit_depth=24, min_sample_rate=192_000), + QualityTarget(label='FLAC 24-bit/96kHz', format='flac', bit_depth=24, min_sample_rate=96_000), + QualityTarget(label='FLAC 24-bit/48kHz', format='flac', bit_depth=24, min_sample_rate=48_000), + QualityTarget(label='FLAC 24-bit/44.1kHz',format='flac', bit_depth=24, min_sample_rate=44_100), + QualityTarget(label='FLAC 16-bit', format='flac', bit_depth=16), + QualityTarget(label='MP3 320kbps', format='mp3', min_bitrate=320), + QualityTarget(label='MP3 256kbps', format='mp3', min_bitrate=256), + QualityTarget(label='MP3 192kbps', format='mp3', min_bitrate=192), +] + + +# ── Ranking helpers ──────────────────────────────────────────────────────────── + +def rank_candidate(aq: AudioQuality, targets: List[QualityTarget]) -> Tuple[int, float]: + """Return *(target_index, tier_score)* for sorting. + + Lower ``target_index`` → higher priority match. + Candidates that satisfy no target get ``index = len(targets)`` + (they sort last but are not discarded — the caller decides that). + """ + for i, target in enumerate(targets): + if aq.matches_target(target): + return (i, aq.tier_score()) + return (len(targets), aq.tier_score()) + + +def filter_and_rank( + candidates: list, + targets: List[QualityTarget], + *, + fallback_enabled: bool = True, +) -> list: + """Sort *candidates* (any objects with an ``audio_quality`` attribute) + by quality priority. + + Returns the subset that matched the *highest-priority* satisfied target, + sorted by ``tier_score`` descending within that group. + Falls back to all candidates sorted by score when ``fallback_enabled`` + and nothing matches, or when targets list is empty. + """ + if not targets: + candidates_copy = list(candidates) + candidates_copy.sort(key=lambda c: c.audio_quality.tier_score(), reverse=True) + return candidates_copy + + scored = [(rank_candidate(c.audio_quality, targets), c) for c in candidates] + + # Best target index that any candidate reached + best_idx = min((s[0][0] for s in scored), default=len(targets)) + + if best_idx < len(targets): + winners = [c for (idx, _), c in scored if idx == best_idx] + winners.sort(key=lambda c: c.audio_quality.tier_score(), reverse=True) + return winners + + if fallback_enabled: + all_sorted = list(candidates) + all_sorted.sort(key=lambda c: c.audio_quality.tier_score(), reverse=True) + return all_sorted + + return [] + + +# ── Migration helper ─────────────────────────────────────────────────────────── + +def v2_qualities_to_ranked_targets(qualities: dict) -> List[dict]: + """Convert old v2 ``qualities`` dict to a ranked-targets list. + + Preserves the user's existing priority order while upgrading to the + richer target format. + """ + _FORMAT_MAP = { + 'flac': {'format': 'flac', 'bit_depth': None}, + 'mp3_320': {'format': 'mp3', 'min_bitrate': 320}, + 'mp3_256': {'format': 'mp3', 'min_bitrate': 256}, + 'mp3_192': {'format': 'mp3', 'min_bitrate': 192}, + # AAC (#886): opt-in tier. Match on format alone — Soulseek AAC/.m4a + # rarely carries a bitrate attribute, so a min_bitrate gate would + # reject every bitrate-less AAC. Priority order (above MP3, below FLAC) + # is preserved by the caller's priority sort, not by min_bitrate. + 'aac': {'format': 'aac'}, + } + enabled = [ + (cfg.get('priority', 999), name, cfg) + for name, cfg in qualities.items() + if cfg.get('enabled', False) + ] + enabled.sort() + targets = [] + for _, name, cfg in enabled: + base = _FORMAT_MAP.get(name, {}).copy() + if not base: + continue + if name == 'flac': + bd = cfg.get('bit_depth', 'any') + if bd == '24': + base['bit_depth'] = 24 + base['label'] = 'FLAC 24-bit' + elif bd == '16': + base['bit_depth'] = 16 + base['label'] = 'FLAC 16-bit' + else: + base['label'] = 'FLAC (any)' + else: + base['label'] = name.upper().replace('_', ' ') + targets.append(base) + return targets + + +# ── Internal helpers ─────────────────────────────────────────────────────────── + +def _sample_rate_to_min_kbps(sample_rate: int, bit_depth: int) -> int: + """Approximate minimum kbps for a lossless file at the given spec. + Used as heuristic when actual sample-rate metadata is absent. + """ + # kbps = sample_rate * channels * bit_depth / 1000 * compression_ratio + # Assume stereo (2 ch) and ~0.6 FLAC compression ratio + raw_kbps = sample_rate * 2 * bit_depth / 1000 + return int(raw_kbps * 0.55) # conservative compressed estimate diff --git a/core/quality/selection.py b/core/quality/selection.py new file mode 100644 index 00000000..98eed250 --- /dev/null +++ b/core/quality/selection.py @@ -0,0 +1,128 @@ +"""Quality-aware candidate selection shared by the search engine and the +download orchestrator. + +``rank_with_targets`` is the pure core: it ranks candidates against a target +list and reports whether any candidate met a *real* target (strict, fallback +off). The engine uses that ``satisfied`` flag to decide whether the current +source is good enough or it should fall through to the next source in the +hybrid chain. + +``rank_for_profile`` is the thin DB-backed wrapper that loads the user's +quality profile (with v2->v3 migration) and delegates. +""" + +from __future__ import annotations + +from typing import List, Tuple + +from core.quality.model import ( + QualityTarget, + filter_and_rank, + v2_qualities_to_ranked_targets, +) + + +def rank_with_targets( + candidates: list, + targets: List[QualityTarget], + *, + fallback_enabled: bool = True, +) -> Tuple[list, bool]: + """Rank *candidates* against *targets*. + + Returns ``(ranked, satisfied)`` where ``satisfied`` is True when at least + one candidate meets a real target. When no targets are configured the + profile imposes no constraint, so any non-empty result counts as + satisfied (the first source wins, quality-sorted). + """ + if not candidates: + return [], False + + if not targets: + ranked = filter_and_rank(candidates, targets, fallback_enabled=True) + return ranked, bool(ranked) + + strict = filter_and_rank(candidates, targets, fallback_enabled=False) + if strict: + return strict, True + + if fallback_enabled: + return filter_and_rank(candidates, targets, fallback_enabled=True), False + return [], False + + +def targets_from_profile(profile: dict) -> Tuple[List[QualityTarget], bool]: + """Convert a quality-profile dict into ``(targets, fallback_enabled)`` with + v2->v3 migration applied. The single conversion path shared by the import + guard, the download ranker and the library quality scanner.""" + raw_targets = profile.get('ranked_targets') + if not raw_targets and 'qualities' in profile: + raw_targets = v2_qualities_to_ranked_targets(profile['qualities']) + + targets = [QualityTarget.from_dict(t) for t in (raw_targets or [])] + fallback_enabled = profile.get('fallback_enabled', True) + return targets, fallback_enabled + + +def load_profile_targets() -> Tuple[List[QualityTarget], bool]: + """Load the user's quality profile from the DB and return + ``(targets, fallback_enabled)`` with v2->v3 migration applied. + + Callers that rank across many sources should load once and reuse via + :func:`rank_with_targets` rather than calling :func:`rank_for_profile` + per source. + """ + from database.music_database import MusicDatabase + + return targets_from_profile(MusicDatabase().get_quality_profile()) + + +def quality_meets_profile(aq, targets: List[QualityTarget]) -> bool: + """Strict: True iff *aq* satisfies at least one ranked *target*. + + The shared definition of "good enough" for both the import guard and the + library scanner — bit depth + sample rate are minimums (see + :meth:`AudioQuality.matches_target`). Fallback is NOT consulted here; it's a + download-time last-resort concession, not part of what counts as meeting the + profile. ``targets`` empty → no constraint (True). ``aq`` None (probe + failed) → True, so an unreadable file is never falsely flagged. + """ + if not targets: + return True + if aq is None: + return True + from core.quality.model import rank_candidate + + idx, _ = rank_candidate(aq, targets) + return idx < len(targets) + + +_VALID_SEARCH_MODES = ("priority", "best_quality") + + +def load_search_mode() -> str: + """Return the download search strategy from the user's quality profile. + + ``'priority'`` (default) keeps today's behaviour — the first source in the + hybrid chain that meets a quality target wins. ``'best_quality'`` pools + candidates across all sources and works them best→worst by actual audio + quality. Any missing/unknown value resolves to ``'priority'`` so existing + installs are unaffected. + """ + from database.music_database import MusicDatabase + + try: + profile = MusicDatabase().get_quality_profile() + mode = profile.get("search_mode", "priority") + except Exception: + return "priority" + return mode if mode in _VALID_SEARCH_MODES else "priority" + + +def rank_for_profile(candidates: list) -> Tuple[list, bool]: + """Load the user's quality profile and rank *candidates* against it. + + Returns ``(ranked, satisfied)`` — see :func:`rank_with_targets`. + """ + targets, fallback_enabled = load_profile_targets() + return rank_with_targets(candidates, targets, fallback_enabled=fallback_enabled) diff --git a/core/quality/source_map.py b/core/quality/source_map.py new file mode 100644 index 00000000..9a834f4d --- /dev/null +++ b/core/quality/source_map.py @@ -0,0 +1,203 @@ +"""Per-source quality mappers — turn each download source's tier string or +API values into a unified :class:`~core.quality.model.AudioQuality`. + +Every streaming source describes quality differently (Tidal/HiFi use tier +strings, Qobuz reports real kHz + bit depth, Deezer uses config codes, +Amazon mixes real values with HD/UHD tiers). Centralising the knowledge +here keeps the per-client code to a single call and keeps the tier tables +in one auditable place. + +Each value is a *claim*: the download client populates its ``TrackResult`` +from it so the global ranker can choose a source, and the post-download +quality guard later verifies the real file. Over-claiming is the danger — +an unknown tier maps to ``format='unknown'`` rather than pretending to be +lossless. +""" + +from __future__ import annotations + +from typing import Optional + +from core.quality.model import AudioQuality +from core.quality.selection import load_profile_targets + + +# ── Extension → format string (source-agnostic) ──────────────────────────── +# +# The single source of truth for mapping a file extension to the unified +# AudioQuality ``format``. Every extension-based download source (Soulseek, +# torrent/usenet file lists, …) classifies through this, so the ranked-target +# system behaves identically across sources and adding a format here lights it +# up everywhere at once. Unknown extensions → 'unknown' (never matches a +# target, so it only ever comes through via the fallback toggle). +# +# AIFF/AIF are uncompressed PCM like WAV → the same 'wav' tier. ``.m4a`` +# defaults to 'aac'; an ALAC-in-m4a file can't be told apart by extension +# alone, so probe_audio_quality corrects it from the real codec post-download. +_EXTENSION_FORMAT_MAP = { + 'flac': 'flac', + 'alac': 'alac', + 'wav': 'wav', 'wave': 'wav', + 'aiff': 'wav', 'aif': 'wav', 'aifc': 'wav', + 'mp3': 'mp3', + 'm4a': 'aac', 'mp4': 'aac', 'aac': 'aac', + 'ogg': 'ogg', 'oga': 'ogg', + 'opus': 'opus', + 'wma': 'wma', +} + +# Audio extensions worth probing/classifying at all — derived from the map so +# the allow-list and the classifier never drift apart. +AUDIO_EXTENSIONS = {f'.{e}' for e in _EXTENSION_FORMAT_MAP} + + +def format_from_extension(ext: str) -> str: + """Map a file extension (with or without leading dot) to the unified + AudioQuality format string. Unknown → 'unknown'.""" + return _EXTENSION_FORMAT_MAP.get(str(ext or '').lower().lstrip('.'), 'unknown') + + +# ── Tidal / HiFi (Monochrome is Tidal-backed) ────────────────────────────── +# +# Tidal exposes UPPER_SNAKE tier strings (``HI_RES_LOSSLESS``); HiFi's config +# uses lowercase keys (``hires``/``lossless``). We normalise both into the +# same lookup so one mapper serves both sources. + +_TIDAL_HIRES = AudioQuality(format='flac', sample_rate=96000, bit_depth=24) +_TIDAL_LOSSLESS = AudioQuality(format='flac', sample_rate=44100, bit_depth=16) +_TIDAL_HIGH = AudioQuality(format='aac', bitrate=320) +_TIDAL_LOW = AudioQuality(format='aac', bitrate=96) + +TIDAL_TIER_MAP = { + 'HI_RES_LOSSLESS': _TIDAL_HIRES, + 'HI_RES': _TIDAL_HIRES, + 'HIRES': _TIDAL_HIRES, + 'LOSSLESS': _TIDAL_LOSSLESS, + 'HIGH': _TIDAL_HIGH, + 'LOW': _TIDAL_LOW, +} + + +def quality_from_tidal_tier(tier: str) -> AudioQuality: + """Map a Tidal/HiFi quality tier string to an AudioQuality. + + Case-insensitive; accepts both ``HI_RES`` and ``hires`` spellings. + Unrecognised tiers map to ``format='unknown'`` so they never + over-claim lossless quality. + """ + key = (tier or '').strip().upper() + return TIDAL_TIER_MAP.get(key, AudioQuality(format='unknown')) + + +# ── Qobuz (real API values) ──────────────────────────────────────────────── + +def quality_from_qobuz(sampling_rate_khz: float, bit_depth: int) -> AudioQuality: + """Qobuz reports ``maximum_sampling_rate`` in kHz (e.g. 44.1, 96, 192) + and ``maximum_bit_depth``. These are real values from the API. + """ + sample_rate = int(round(sampling_rate_khz * 1000)) if sampling_rate_khz else None + return AudioQuality(format='flac', sample_rate=sample_rate, bit_depth=bit_depth) + + +# ── Deezer (config code) ─────────────────────────────────────────────────── + +DEEZER_CODE_MAP = { + 'flac': AudioQuality(format='flac', sample_rate=44100, bit_depth=16), + 'mp3_320': AudioQuality(format='mp3', bitrate=320), + 'mp3_128': AudioQuality(format='mp3', bitrate=128), +} + + +def quality_from_deezer(code: str) -> AudioQuality: + """Map a Deezer download quality code to AudioQuality. + + Deezer FLAC is always CD-quality (16-bit/44.1 kHz). + """ + return DEEZER_CODE_MAP.get((code or '').lower(), AudioQuality(format='unknown')) + + +# ── Amazon Music (real sampleRate preferred, HD/UHD tier fallback) ───────── + +_AMAZON_TIER_MAP = { + 'UHD': AudioQuality(format='flac', sample_rate=96000, bit_depth=24), + 'HD': AudioQuality(format='flac', sample_rate=44100, bit_depth=16), +} + + +def quality_from_amazon( + tier: str, + sample_rate: Optional[int] = None, + bit_depth: Optional[int] = None, +) -> AudioQuality: + """Amazon Music is FLAC; prefer the real ``sampleRate``/``bitDepth`` from + the stream info when present, otherwise fall back to the HD/UHD tier. + """ + base = _AMAZON_TIER_MAP.get((tier or '').strip().upper(), AudioQuality(format='flac')) + return AudioQuality( + format='flac', + sample_rate=sample_rate if sample_rate is not None else base.sample_rate, + bit_depth=bit_depth if bit_depth is not None else base.bit_depth, + ) + + +# ── Profile-driven download tier (replaces per-source quality settings) ───── +# +# Each source's selectable download tiers, ordered best → worst, with the +# AudioQuality the tier delivers. ``quality_tier_for_source`` walks these to +# request the LOWEST tier that satisfies the user's top global target — so the +# global quality profile, not a per-source dropdown, decides what each source +# fetches. + +_SOURCE_TIER_LADDERS: dict[str, list[tuple[str, AudioQuality]]] = { + 'tidal': [ + ('hires', AudioQuality('flac', sample_rate=96000, bit_depth=24)), + ('lossless', AudioQuality('flac', sample_rate=44100, bit_depth=16)), + ('high', AudioQuality('aac', bitrate=320)), + ('low', AudioQuality('aac', bitrate=96)), + ], + 'hifi': [ + ('hires', AudioQuality('flac', sample_rate=96000, bit_depth=24)), + ('lossless', AudioQuality('flac', sample_rate=44100, bit_depth=16)), + ('high', AudioQuality('aac', bitrate=320)), + ('low', AudioQuality('aac', bitrate=96)), + ], + 'qobuz': [ + ('hires_max', AudioQuality('flac', sample_rate=192000, bit_depth=24)), + ('hires', AudioQuality('flac', sample_rate=96000, bit_depth=24)), + ('lossless', AudioQuality('flac', sample_rate=44100, bit_depth=16)), + ('mp3', AudioQuality('mp3', bitrate=320)), + ], + 'deezer': [ + ('flac', AudioQuality('flac', sample_rate=44100, bit_depth=16)), + ('mp3_320', AudioQuality('mp3', bitrate=320)), + ('mp3_128', AudioQuality('mp3', bitrate=128)), + ], + 'amazon': [ + ('flac', AudioQuality('flac', sample_rate=48000, bit_depth=24)), + ('opus', AudioQuality('aac', bitrate=320)), + ], +} + + +def quality_tier_for_source(source_name: str, *, default: Optional[str] = None) -> Optional[str]: + """Return the source tier key to request, derived from the global profile. + + Picks the lowest tier in the source's ladder that satisfies the user's + top (most-preferred) target — respecting the quality ceiling and saving + bandwidth. Falls back to the source's max tier when none can satisfy it + (best effort), or to the source's max when no targets are configured. + Returns *default* for an unknown source. + """ + ladder = _SOURCE_TIER_LADDERS.get(source_name) + if not ladder: + return default + + targets, _ = load_profile_targets() + if not targets: + return ladder[0][0] + + top = targets[0] + for key, aq in reversed(ladder): # low → high + if aq.matches_target(top): + return key + return ladder[0][0] # best effort: max tier diff --git a/core/repair_jobs/__init__.py b/core/repair_jobs/__init__.py index 35a236bd..477eab1d 100644 --- a/core/repair_jobs/__init__.py +++ b/core/repair_jobs/__init__.py @@ -40,6 +40,7 @@ _JOB_MODULES = [ 'core.repair_jobs.metadata_gap_filler', 'core.repair_jobs.album_completeness', 'core.repair_jobs.fake_lossless_detector', + 'core.repair_jobs.quality_upgrade_scanner', 'core.repair_jobs.library_reorganize', 'core.repair_jobs.mbid_mismatch_detector', 'core.repair_jobs.single_album_dedup', diff --git a/core/repair_jobs/quality_upgrade.py b/core/repair_jobs/quality_upgrade.py index f0ad8c53..b720f7be 100644 --- a/core/repair_jobs/quality_upgrade.py +++ b/core/repair_jobs/quality_upgrade.py @@ -14,9 +14,10 @@ is queued until you review and Apply the finding — at which point the matched track (carrying its album context) is added to the wishlist, exactly like every other acquisition path. -The quality decision (``meets_preferred_quality``) is a pure function so it can be -unit-tested without a database or network. Transcode/"fake lossless" detection is -intentionally NOT done here — that's the separate Fake Lossless Detector job. +Quality is judged using the real file (mutagen-measured bit depth / sample rate / +bitrate) checked against the user's v3 ranked profile targets — fully profile-driven, +no hardcoded thresholds. Transcode/"fake lossless" detection is the separate Fake +Lossless Detector job. """ from __future__ import annotations @@ -39,30 +40,25 @@ from core.discovery.quality_scanner import ( ) from core.library.file_tags import read_embedded_tags from core.library.path_resolver import resolve_library_file_path +# v3 quality: probe the real file + check it against the ranked profile targets, +# the SAME definition the download import guard uses. Module-level so they're +# monkeypatchable in tests. +from core.imports.file_ops import probe_audio_quality +from core.quality.model import rank_candidate +from core.quality.selection import targets_from_profile, quality_meets_profile from utils.logging_config import get_logger logger = get_logger("repair_jobs.quality_upgrade") -# Quality ranks — higher is better. Lossless tops everything; lossy tiers fall out -# of bitrate. 0 means "below the lowest tracked tier / unknown". -RANK_LOSSLESS = 4 -RANK_320 = 3 -RANK_256 = 2 -RANK_192 = 1 -RANK_BELOW = 0 +def _to_bool(val) -> bool: + """Coerce a setting value to bool. Handles Python bool, string 'true'/'false', and int.""" + if isinstance(val, bool): + return val + if isinstance(val, str): + return val.lower() == 'true' + return bool(val) if val is not None else False -LOSSLESS_EXTENSIONS = {'.flac', '.alac', '.ape', '.wav', '.aiff', '.aif', '.dsf', '.dff', '.m4a'} -# NB: .m4a is ambiguous (ALAC vs AAC); we treat the *format* as lossy-capable and -# rely on bitrate below — a true ALAC .m4a reports a lossless-scale bitrate. - -# Quality-profile bucket key -> rank. -_PROFILE_KEY_RANK = { - 'flac': RANK_LOSSLESS, - 'mp3_320': RANK_320, - 'mp3_256': RANK_256, - 'mp3_192': RANK_192, -} # Per-source file-tag key holding that source's own track ID (written by enrichment). _SOURCE_TRACK_ID_TAG = { @@ -79,93 +75,6 @@ _SOURCE_TRACK_ID_TAG = { _DURATION_TOLERANCE_MS = 5000 -def _normalize_kbps(bitrate: Optional[int]) -> Optional[int]: - """Library bitrate may be stored in bps (e.g. 320000) or kbps (320). - Normalize to kbps. Returns None when unknown/zero.""" - if not bitrate: - return None - try: - b = int(bitrate) - except (TypeError, ValueError): - return None - if b <= 0: - return None - return b // 1000 if b > 4000 else b - - -def classify_track_quality(file_path: str, bitrate: Optional[int]) -> Optional[int]: - """Rank a file by format + bitrate. Returns a RANK_* value, or None when it - can't be judged (a lossy file with no known bitrate).""" - ext = os.path.splitext(file_path or '')[1].lower() - kbps = _normalize_kbps(bitrate) - - # Lossless containers: a real lossless file has a high bitrate; a low one is a - # lossy stream in a lossless container — but flagging that is the Fake Lossless - # Detector's job, so here we treat the lossless *format* as top rank. - if ext in {'.flac', '.alac', '.ape', '.wav', '.aiff', '.aif', '.dsf', '.dff'}: - return RANK_LOSSLESS - # .m4a / lossy: judge purely by bitrate. A lossless-scale bitrate (ALAC in m4a, - # or a mislabeled lossless) ranks as lossless. - if kbps is None: - return None - if kbps >= 800: - return RANK_LOSSLESS - if kbps >= 280: - return RANK_320 - if kbps >= 200: - return RANK_256 - if kbps >= 150: - return RANK_192 - return RANK_BELOW - - -def preferred_quality_floor(quality_profile: Dict[str, Any]) -> Optional[int]: - """The lowest acceptable quality rank from the profile's ENABLED buckets — the - floor a track must meet. Returns None when nothing is enabled (caller should - then flag nothing, rather than flagging everything).""" - qualities = (quality_profile or {}).get('qualities', {}) or {} - enabled_ranks = [ - _PROFILE_KEY_RANK[key] - for key, cfg in qualities.items() - if isinstance(cfg, dict) and cfg.get('enabled') and key in _PROFILE_KEY_RANK - ] - if not enabled_ranks: - return None - return min(enabled_ranks) - - -def meets_preferred_quality(file_path: str, bitrate: Optional[int], - quality_profile: Dict[str, Any]) -> bool: - """Pure decision: does this track already meet the user's preferred quality? - - A track meets quality when its format+bitrate rank is at least the profile's - floor (the worst quality the user still accepts). This honors a profile that - enables, say, FLAC *and* MP3-320: a 320 kbps MP3 passes, a 128 kbps MP3 does - not. With nothing enabled, everything passes (we never flag the whole library - on an empty profile).""" - floor = preferred_quality_floor(quality_profile) - if floor is None: - return True - - file_rank = classify_track_quality(file_path, bitrate) - if file_rank is None: - # Lossy file with unknown bitrate: only judgeable when the floor is - # lossless (then any lossy file is below it). Otherwise don't flag. - ext = os.path.splitext(file_path or '')[1].lower() - if floor == RANK_LOSSLESS and ext not in LOSSLESS_EXTENSIONS: - return False - return True - - return file_rank >= floor - - -def _rank_label(rank: Optional[int]) -> str: - return { - RANK_LOSSLESS: 'Lossless', RANK_320: 'MP3 320', RANK_256: 'MP3 256', - RANK_192: 'MP3 192', RANK_BELOW: 'low bitrate', - }.get(rank, 'unknown') - - def _norm_isrc(value: Any) -> str: """Canonicalize an ISRC for comparison: uppercase, strip dashes/spaces.""" if not value: @@ -173,14 +82,14 @@ def _norm_isrc(value: Any) -> str: return str(value).upper().replace('-', '').replace(' ', '').strip() -def _read_file_ids(file_path: str) -> Dict[str, str]: +def _read_file_ids(file_path: str, resolved_path: Optional[str] = None) -> Dict[str, str]: """Read the identifiers enrichment embedded in the file's tags. Enrichment matches every track to the metadata sources and writes the IDs (ISRC + per-source track IDs) into the file — so an already-enriched track carries its exact identity. Returns a dict with a normalized ``isrc`` plus any ``_track_id`` tags present; empty dict when unreadable / not enriched.""" - resolved = resolve_library_file_path(file_path) if file_path else None + resolved = resolved_path or (resolve_library_file_path(file_path) if file_path else None) if not resolved and file_path and os.path.isfile(file_path): resolved = file_path if not resolved: @@ -436,45 +345,61 @@ def _find_best_match(engine: Any, source_priority: List[str], title: str, artist @register_job class QualityUpgradeJob(RepairJob): job_id = 'quality_upgrade' - display_name = 'Quality Upgrade Finder' - description = 'Finds library tracks below your preferred quality and proposes a better version' + display_name = 'Quality Upgrade Finder (active — proposes a replacement)' + description = 'Finds library tracks below your quality profile and actively searches a better version to add to the wishlist' help_text = ( - 'Scans your library (or just your watchlist artists) and compares each ' - "track against your Quality Profile using BOTH the file format and its " - 'bitrate — so a 128 kbps MP3 is no longer treated the same as a 320 kbps ' - 'one, and enabling MP3-320/256 in your profile actually counts.\n\n' - 'For every track below your preferred quality it resolves the exact better ' - 'version using the most precise identity available, in order: the source ' - "track ID enrichment wrote into the file → the file's ISRC → the album's " - 'tracklist (by stored album ID or album search) → a name/artist search. The ' - 'fuzzy steps also reject candidates whose length is off (wrong live/edit cut). ' - 'It skips tracks it already proposed, so re-runs are cheap. Nothing is queued ' - 'automatically: applying a finding adds that matched track — with its album ' - 'context — to the wishlist, the same as any other download.\n\n' + 'ACTIVE quality job. For every library track below your quality profile it ' + 'goes one step further than the Quality Check (flag-only) scanner: it ' + 'actively SEARCHES your metadata source for the exact better version and ' + 'attaches it to the finding, so Apply adds that track straight to your ' + 'wishlist.\n\n' + 'Quality is judged the SAME way as the download/import pipeline — it reads ' + 'the REAL file with mutagen (measured bit depth / sample rate / bitrate) and ' + 'checks it against your v3 quality profile targets (strict: fallback is ' + 'ignored, that\'s a download-time concession, not "good enough" for an ' + 'upgrade). So a 128 kbps MP3, a 16-bit FLAC where you want 24-bit, etc. are ' + 'all caught accurately.\n\n' + 'For every below-profile track it resolves the better version by the most ' + 'precise identity available, in order: the source track ID enrichment wrote ' + "into the file → the file's ISRC → the album's tracklist (by stored album ID " + 'or album search) → a name/artist search (with a duration guard against wrong ' + 'live/edit cuts). It skips tracks it already proposed, so re-runs are cheap. ' + 'Nothing is queued automatically — applying a finding adds the matched track ' + '(with album context) to the wishlist, like any other download.\n\n' 'Settings:\n' '- Scope: "watchlist" (watchlisted artists only) or "all" (whole library)\n' - '- Min confidence: minimum match confidence (0-1) to surface a finding\n\n' - 'Note: detecting fake/transcoded lossless files is handled by the separate ' - 'Fake Lossless Detector job.' + '- Min confidence: minimum match confidence (0-1) to surface a finding\n' + '- Deep audio verify (default OFF): also run the ffmpeg decode guard ' + '(truncation + silence) per track — catches broken/incomplete files the ' + 'header hides, but decodes every file (seconds per track, CPU-heavy).\n\n' + 'Sibling job: "Quality Check (flag only)" finds the same below-profile tracks ' + 'but only flags them for you to decide per finding (re-download / delete / ' + 'ignore) instead of searching a replacement. Fake/transcoded lossless ' + 'detection is the separate Fake Lossless Detector job.' ) icon = 'repair-icon-lossy' default_enabled = False default_interval_hours = 168 - default_settings = {'scope': 'watchlist', 'min_confidence': 0.7} - setting_options = {'scope': ['watchlist', 'all']} + default_settings = {'scope': 'all', 'min_confidence': 0.7, 'deep_audio_verify': False, 'require_top_target': False} + setting_options = {'scope': ['all', 'watchlist'], 'deep_audio_verify': [True, False], 'require_top_target': [True, False]} auto_fix = False def _get_settings(self, context: JobContext) -> Dict[str, Any]: - cfg = context.config_manager - scope = 'watchlist' - min_conf = 0.7 - if cfg: - scope = cfg.get(self.get_config_key('settings.scope'), 'watchlist') or 'watchlist' + merged = dict(self.default_settings) + if context.config_manager: try: - min_conf = float(cfg.get(self.get_config_key('settings.min_confidence'), 0.7)) - except (TypeError, ValueError): - min_conf = 0.7 - return {'scope': scope, 'min_confidence': min_conf} + cfg = context.config_manager.get(f'repair.jobs.{self.job_id}.settings', {}) + if isinstance(cfg, dict): + merged.update(cfg) + except Exception as e: + logger.debug("settings read failed: %s", e) + try: + merged['min_confidence'] = float(merged.get('min_confidence', 0.7)) + except (TypeError, ValueError): + merged['min_confidence'] = 0.7 + merged['deep_audio_verify'] = _to_bool(merged.get('deep_audio_verify')) + merged['require_top_target'] = _to_bool(merged.get('require_top_target')) + return merged def _load_tracks(self, db: Any, scope: str) -> List[dict]: conn = db._get_connection() @@ -530,13 +455,27 @@ class QualityUpgradeJob(RepairJob): settings = self._get_settings(context) scope = settings['scope'] min_conf = settings['min_confidence'] + deep_verify = settings['deep_audio_verify'] + require_top = settings['require_top_target'] + # v3 quality: judge the REAL file (mutagen-measured bit depth / sample rate + # / bitrate) against the profile's ranked targets — the SAME definition the + # download import guard uses. Strict: fallback is ignored (a download-time + # concession, not "good enough" for an upgrade). targets_from_profile / + # quality_meets_profile / probe_audio_quality are imported at module level. db = context.db quality_profile = db.get_quality_profile() - if preferred_quality_floor(quality_profile) is None: - logger.info("[Quality Upgrade] No quality buckets enabled in profile — nothing to flag") + targets, _fallback = targets_from_profile(quality_profile) + if not targets: + logger.info("[Quality Upgrade] No quality targets in profile — nothing to flag") return result + logger.info( + "[Quality Upgrade] scope=%s require_top=%s · all targets: %s", + scope, require_top, + [t.label for t in targets] if targets else '(none — all pass)', + ) + try: tracks = self._load_tracks(db, scope) except Exception as e: @@ -583,13 +522,53 @@ class QualityUpgradeJob(RepairJob): result.findings_skipped_dedup += 1 continue - if meets_preferred_quality(file_path, bitrate, quality_profile): + # v3 quality decision — probe the REAL file. Resolve the library path + # first (the DB stores a possibly-relative path). Pass config_manager so + # the resolver can find the transfer/music folders and expand relative paths. + resolved_path = resolve_library_file_path( + file_path, + transfer_folder=context.transfer_folder, + config_manager=context.config_manager, + ) if file_path else None + if not resolved_path and file_path and os.path.isfile(file_path): + resolved_path = file_path + + measured_aq = probe_audio_quality(resolved_path) if resolved_path else None + + # Optional ffmpeg deep verify (default off): a truncated/silent file is + # treated as "needs a replacement" just like a below-profile one. + broken_reason = None + if deep_verify and resolved_path: + try: + from core.imports.silence import detect_broken_audio + broken_reason = detect_broken_audio(resolved_path) + except Exception as e: + logger.debug("[Quality Upgrade] deep verify failed for %s: %s", file_path, e) + + if measured_aq is None and not broken_reason: + # Can't read the file → can't judge it; leave it alone. result.skipped += 1 if context.update_progress and (i + 1) % 25 == 0: context.update_progress(i + 1, total) continue - # Below preferred quality — find a better version to propose. + if not broken_reason and measured_aq is not None: + if require_top: + # ranking-based: skip only if the file already sits at rank 0 + # (the top configured target). Any lower rank → flag for upgrade. + idx, _ = rank_candidate(measured_aq, targets) + already_best = (idx == 0) + else: + # default: skip if the file meets ANY configured target (i.e. + # it's not below the acceptable floor). + already_best = quality_meets_profile(measured_aq, targets) + if already_best: + result.skipped += 1 + if context.update_progress and (i + 1) % 25 == 0: + context.update_progress(i + 1, total) + continue + + # Below profile (or broken) — find a better version to propose. if engine is None: from core.matching_engine import MusicMatchingEngine engine = MusicMatchingEngine() @@ -602,17 +581,20 @@ class QualityUpgradeJob(RepairJob): logger.info("[Quality Upgrade] Spotify rate-limited — stopping scan early") return result - current_rank = classify_track_quality(file_path, bitrate) - current_label = _rank_label(current_rank) + current_label = measured_aq.label() if measured_aq is not None else 'broken/unreadable' + if broken_reason: + current_label = f'{current_label} (broken: {broken_reason})' if measured_aq is not None else f'broken ({broken_reason})' if context.report_progress: + _why = 'broken audio' if broken_reason else 'low quality' context.report_progress( scanned=i + 1, total=total, - log_line=f'Low quality ({current_label}): {artist_name} - {title}', + log_line=f'{_why} ({current_label}): {artist_name} - {title}', log_type='info') # Read the identifiers enrichment embedded in the file once (ISRC + # per-source track IDs), used by the two most-exact tiers below. - file_ids = _read_file_ids(file_path) + # Pass resolved_path so the inner resolver doesn't redo the lookup. + file_ids = _read_file_ids(file_path, resolved_path=resolved_path) # Tiered match, best identity first, loosest last: # 0. The active source's OWN track ID, embedded in the file by @@ -685,8 +667,9 @@ class QualityUpgradeJob(RepairJob): file_path=file_path, title=f'Upgrade: {artist_name} - {title} ({current_label})', description=( - f'"{title}" by {artist_name} is {current_label}, below your preferred ' - f'quality. Best match: "{_track_name(best)}" via {source} ' + f'"{title}" by {artist_name} is {current_label}' + + (f', below your preferred quality ({targets[0].label})' if require_top and targets else ', below your preferred quality') + + f'. Best match: "{_track_name(best)}" via {source} ' f'({_MATCH_NOTE.get(matched_via, "matched") if matched_via != "search" else f"confidence {conf:.0%}"}). ' 'Apply to add it to the wishlist.'), details={ @@ -715,6 +698,13 @@ class QualityUpgradeJob(RepairJob): if context.update_progress: context.update_progress(total, total) - logger.info("[Quality Upgrade] %d scanned, %d upgrades found, %d met/skip", + logger.info("[Quality Upgrade] %d scanned, %d upgrades found, %d already met profile / skipped", result.scanned, result.findings_created, result.skipped) + if result.scanned > 0 and result.findings_created == 0 and result.errors == 0: + logger.info( + "[Quality Upgrade] All tracks already satisfy the configured targets. " + "If you expected upgrades, check your quality profile — the current " + "top target is: %s", + targets[0].label if targets else '(none)', + ) return result diff --git a/core/repair_jobs/quality_upgrade_scanner.py b/core/repair_jobs/quality_upgrade_scanner.py new file mode 100644 index 00000000..25b36e56 --- /dev/null +++ b/core/repair_jobs/quality_upgrade_scanner.py @@ -0,0 +1,416 @@ +"""Quality Upgrade Scanner Job — flags library tracks below the user's profile. + +Walks the music folders ON DISK (transfer + download + every configured library +path) exactly like the Orphan / Fake-Lossless detectors — those reliably "see" +files because they os.walk real directories instead of trying to resolve the +DB's stored (often relative) paths. For each audio file it probes the ACTUAL +measured audio quality (bit depth / sample rate / bitrate via the same +`probe_audio_quality` the download import guard uses) and checks it against the +user's v3 ranked targets with `quality_meets_profile` (strict — fallback +ignored, that's a download-time concession, not a definition of "good enough"). + +Every file that satisfies none of the targets becomes a finding the user can: + - 'redownload': add the track to the wishlist and delete the low-quality file + - 'delete': remove the low-quality file (+ DB row when known) + - 'ignore': dismiss the finding (handled in the UI via the dismiss endpoint) + +Each walked file is matched back to its DB track (by path suffix) so the finding +carries the real title/artist/album + track id; when no DB row matches, the +file's own tags are used and the finding is filed as a loose 'file'. +""" + +import os + +from core.repair_jobs import register_job +from core.repair_jobs.base import JobContext, JobResult, RepairJob +from utils.logging_config import get_logger + +logger = get_logger("repair_job.quality_upgrade") + +AUDIO_EXTENSIONS = {'.mp3', '.flac', '.ogg', '.opus', '.m4a', '.aac', '.wav', '.wma', '.aiff', '.aif'} + + +@register_job +class QualityUpgradeScannerJob(RepairJob): + job_id = 'quality_upgrade_scanner' + display_name = 'Quality Check (flag only — you decide per finding)' + description = 'Flags library tracks below your quality profile; you choose re-download / delete / ignore per finding' + help_text = ( + 'FLAG-ONLY quality job. Walks your music library folder on disk (so it also ' + 'catches loose files not in the DB) and checks every track against your v3 ' + 'quality profile — then just FLAGS what is below profile. Unlike the active ' + '"Quality Upgrade Finder", it does NOT search a replacement; you decide what ' + 'to do per finding: Re-download / Delete / Ignore.\n\n' + 'Two-stage check (same as the download/import pipeline):\n' + '1. Real-audio guard (optional, ffmpeg) — decodes the file (truncation + ' + 'silence detection) to catch broken/incomplete audio the header hides.\n' + '2. Quality gate — measured bit depth / sample rate / bitrate vs your ' + 'profile targets.\n\n' + 'Settings:\n' + '- Deep audio verify (default OFF): run the ffmpeg decode guard. Off = fast ' + 'header-only quality pass (milliseconds/track). On = full decode ' + '(seconds/track, CPU-heavy) but catches broken/silent audio.\n' + '- library_tracks_only (default off): only check files matched to a ' + 'library DB track (skip loose/orphan files).\n\n' + 'The scan only reports — it never deletes or re-downloads on its own. ' + 'Use the sibling "Quality Upgrade Finder" instead if you want it to actively ' + 'find and queue a better version for you.' + ) + icon = 'repair-icon-lossless' + default_enabled = False + default_interval_hours = 168 + # library_tracks_only: when ON, only check files that match a library DB + # track (skips loose/orphan files). Default OFF — the scan checks EVERY + # audio file in the Music Library output folder, which is what users expect + # ("check my library folder"). DB matching after a reset is unreliable and + # would wrongly skip everything. Turn ON to ignore non-DB files. + # + # deep_audio_verify default OFF: the ffmpeg decode is the CPU-heavy step. Most + # users want the fast header-only quality pass; turn it on for a deep scan that + # also catches broken/silent audio. (Matches the download pipeline's default.) + default_settings = {'library_tracks_only': False, 'deep_audio_verify': False, 'require_top_target': False} + setting_options = {'library_tracks_only': [True, False], + 'deep_audio_verify': [True, False], + 'require_top_target': [True, False]} + auto_fix = False # User chooses fix action per finding + + def scan(self, context: JobContext) -> JobResult: + result = JobResult() + + # Load the user's v3 ranked targets — the SAME definition the download + # import guard uses. Strict: a track is below-profile when its measured + # quality satisfies NONE of the targets (fallback is not consulted). + from core.quality.selection import targets_from_profile, quality_meets_profile + try: + profile = context.db.get_quality_profile() + except Exception as e: + logger.warning("Could not load quality profile: %s", e) + return result + targets, _fallback = targets_from_profile(profile) + if not targets: + logger.info("Quality profile has no targets — nothing to check against") + return result + + logger.info("Quality upgrade scan — profile targets (strict): %s", + [t.label for t in targets]) + + from core.imports.file_ops import probe_audio_quality + # Same real-file AudioGuard the download/import pipeline runs: ffmpeg + # DECODES the file (astats + silencedetect) to catch truncated or + # mostly-silent audio the header can't reveal. + from core.imports.silence import detect_broken_audio + + # --- Collect the music folders to walk (real dirs, abspath'd) --- + base_dirs = self._collect_music_dirs(context) + if not base_dirs: + logger.warning( + "[QualityScan] No existing music folder to walk (transfer=%r, cwd=%r). " + "Set soulseek.transfer_path to the real mount or add your library under " + "Settings → Library → Music Paths.", + context.transfer_folder, os.getcwd()) + return result + logger.info("[QualityScan] Walking %d folder(s): %r", len(base_dirs), base_dirs) + + # --- Gather audio files (dedup by real path) --- + audio_files = [] + seen = set() + for base in base_dirs: + for root, _dirs, files in os.walk(base): + if context.check_stop(): + return result + for fname in files: + if os.path.splitext(fname)[1].lower() in AUDIO_EXTENSIONS: + fpath = os.path.join(root, fname) + rp = os.path.realpath(fpath) + if rp in seen: + continue + seen.add(rp) + audio_files.append(fpath) + + total = len(audio_files) + logger.info("[QualityScan] Found %d audio file(s) to check", total) + if context.report_progress: + context.report_progress(phase=f'Checking {total} files...', total=total) + if context.update_progress: + context.update_progress(0, total) + + # --- DB suffix index so a walked file maps back to its track row --- + db_index = self._build_db_suffix_index(context) + # Only check files that are part of the LIBRARY (have a DB track row). + # The transfer/download folders also hold pre-import leftovers (e.g. + # residue after a DB reset) — those are orphans, not library tracks, and + # belong to the Orphan File Detector, not a quality upgrade scan. Default + # ON so the scan reflects the user's actual library, not download junk. + _settings = self._get_settings(context) + library_only = _settings.get('library_tracks_only', False) + # Deep verify = run the ffmpeg AudioGuard (real decode) per file, exactly + # like the download pipeline. Slower than a header read (seconds vs ms) but + # it verifies the REAL audio, not just the metadata. OFF by default (the + # decode is the CPU-heavy step); turn on for a deep scan. + deep_verify = _settings.get('deep_audio_verify', False) + # require_top_target: flag files that meet a lower target but not the + # highest-priority one (e.g. 16-bit FLAC when 24-bit is preferred). + require_top = _settings.get('require_top_target', False) + check_targets = targets[:1] if require_top and len(targets) > 1 else targets + + probe_failed = 0 + not_in_library = 0 + for i, fpath in enumerate(audio_files): + if context.check_stop(): + return result + if i % 20 == 0 and context.wait_if_paused(): + return result + + fname = os.path.basename(fpath) + + # Map to a DB track up front (cheap suffix lookup). When scoping to + # the library, skip anything with no DB row BEFORE probing — no point + # reading hundreds of orphan files. + meta = self._match_db(fpath, db_index) + if library_only and meta is None: + not_in_library += 1 + result.skipped += 1 + continue + if meta is None: + meta = self._read_file_tags(fpath) + + result.scanned += 1 + if context.report_progress and i % 25 == 0: + context.report_progress( + scanned=i + 1, total=total, + phase=f'Checking {i + 1} / {total}', + log_line=f'Checking: {fname}', + log_type='info', + ) + + # === Real-file verification — the SAME two stages the download / + # import pipeline runs on every file === + # 1) AudioGuard: ffmpeg DECODES the audio (astats / silencedetect) + # to catch truncated or mostly-silent files the header hides. + # 2) Quality gate: measured quality (mutagen) vs the ranked profile. + try: + broken_reason = detect_broken_audio(fpath) if deep_verify else None + except Exception as e: + logger.debug("AudioGuard failed for %s: %s", fname, e) + broken_reason = None + + try: + aq = probe_audio_quality(fpath) + except Exception as e: + logger.debug("Probe failed for %s: %s", fname, e) + aq = None + + if broken_reason: + issue = 'broken_audio' + current_label = aq.label() if aq is not None else 'unknown' + elif aq is None: + # Header unreadable → can't judge quality; leave it unflagged. + probe_failed += 1 + result.skipped += 1 + continue + elif not quality_meets_profile(aq, check_targets): + issue = 'below_profile' + current_label = aq.label() + else: + # Decodes fully AND meets the profile → genuinely good. + if context.update_progress and (i + 1) % 25 == 0: + context.update_progress(i + 1, total) + continue + + # Build the finding (broken audio OR below profile). + target_labels = [t.label for t in targets] + disp_title = meta.get('title') or os.path.splitext(fname)[0] + disp_artist = meta.get('artist') or 'Unknown' + if issue == 'broken_audio': + _title = f'Broken/incomplete audio: {disp_title}' + _desc = (f'"{disp_title}" by {disp_artist} failed real-audio ' + f'verification (ffmpeg): {broken_reason}') + _severity = 'warning' + else: + _pref = targets[0].label if require_top and len(targets) > 1 else None + _title = f'{"Upgradeable" if _pref else "Below quality"}: {disp_title} ({current_label})' + _desc = (f'"{disp_title}" by {disp_artist} is {current_label}' + + (f', below your preferred quality ({_pref}).' if _pref else + f', which does not meet your quality profile ' + f'({", ".join(target_labels[:3])}' + f'{"…" if len(target_labels) > 3 else ""}).')) + _severity = 'info' + + if context.report_progress: + context.report_progress(log_line=_title, log_type='error') + if context.create_finding: + inserted = context.create_finding( + job_id=self.job_id, + finding_type='quality_upgrade', + severity=_severity, + entity_type='track' if meta.get('track_id') else 'file', + entity_id=str(meta['track_id']) if meta.get('track_id') else None, + file_path=fpath, + title=_title, + description=_desc, + details={ + 'quality_issue': issue, + 'broken_audio_reason': broken_reason or '', + 'current_quality': current_label, + 'current_format': aq.format if aq is not None else '', + 'current_bitrate': aq.bitrate if aq is not None else None, + 'current_sample_rate': aq.sample_rate if aq is not None else None, + 'current_bit_depth': aq.bit_depth if aq is not None else None, + 'target_qualities': target_labels, + 'expected_title': disp_title, + 'expected_artist': disp_artist, + 'album_title': meta.get('album', ''), + 'track_number': meta.get('track_number'), + 'album_thumb_url': meta.get('album_thumb_url'), + 'artist_thumb_url': meta.get('artist_thumb_url'), + }, + ) + if inserted: + result.findings_created += 1 + else: + result.findings_skipped_dedup += 1 + + if context.update_progress and (i + 1) % 25 == 0: + context.update_progress(i + 1, total) + + if context.update_progress: + context.update_progress(total, total) + + if probe_failed: + logger.warning("[QualityScan] %d/%d files could not be probed (unreadable)", + probe_failed, total) + if not_in_library: + logger.info( + "[QualityScan] %d/%d files skipped — not in the library DB (orphan " + "leftovers in transfer/downloads; disable 'library_tracks_only' to " + "include them)", not_in_library, total) + logger.info("Quality upgrade scan: %d checked, %d below profile, %d skipped", + result.scanned, result.findings_created, result.skipped) + return result + + def _get_settings(self, context: JobContext) -> dict: + merged = dict(self.default_settings) + if context.config_manager: + try: + cfg = context.config_manager.get(f'repair.jobs.{self.job_id}.settings', {}) + if isinstance(cfg, dict): + merged.update(cfg) + except Exception as e: + logger.debug("settings read failed: %s", e) + for key in ('library_tracks_only', 'deep_audio_verify', 'require_top_target'): + val = merged.get(key) + if not isinstance(val, bool): + merged[key] = str(val).lower() == 'true' if val is not None else False + return merged + + def _collect_music_dirs(self, context: JobContext) -> list: + """The music-library directories to walk, as absolute paths (dedup). + + Only the user's MUSIC LIBRARY is scanned — that's the "Output Folder + (Music Library)" setting (soulseek.transfer_path) plus any custom + library paths (library.music_paths, for media-server setups). The + download/staging folders are deliberately NOT walked: they hold raw, + pre-import downloads and leftovers, not the finished library, and the + user expects quality checks to run on their library only. Whatever + custom path the user configured for the output folder is respected, + because it's read live from config here. + """ + cm = context.config_manager + raw = [context.transfer_folder] + if cm: + try: + raw.append(cm.get('soulseek.transfer_path', './Transfer')) + mp = cm.get('library.music_paths', []) or [] + if isinstance(mp, list): + raw.extend([p for p in mp if isinstance(p, str) and p.strip()]) + except Exception as e: + logger.debug("music dir config read failed: %s", e) + out, seen = [], set() + for d in raw: + if not d: + continue + ad = os.path.abspath(d) + if ad in seen: + continue + seen.add(ad) + if os.path.isdir(ad): + out.append(ad) + return out + + def _build_db_suffix_index(self, context: JobContext) -> dict: + """Map normalized path suffixes (last 1-3 components, lowercased) → + track metadata, so a walked absolute file can be matched to its DB row + even when the DB stores a different (relative) path prefix.""" + index = {} + conn = None + try: + conn = context.db._get_connection() + cursor = conn.cursor() + cursor.execute(""" + SELECT t.id, t.title, + COALESCE(NULLIF(t.track_artist, ''), ar.name) AS artist, + t.file_path, t.track_number, + al.title AS album_title, al.thumb_url, ar.thumb_url + FROM tracks t + LEFT JOIN artists ar ON ar.id = t.artist_id + LEFT JOIN albums al ON al.id = t.album_id + WHERE t.file_path IS NOT NULL AND t.file_path != '' + """) + for row in cursor.fetchall(): + fp = (row[3] or '').replace('\\', '/') + if not fp: + continue + parts = fp.split('/') + meta = { + 'track_id': row[0], + 'title': row[1] or '', + 'artist': row[2] or '', + 'track_number': row[4], + 'album': row[5] or '', + 'album_thumb_url': row[6] or None, + 'artist_thumb_url': row[7] or None, + } + for depth in range(1, min(4, len(parts) + 1)): + suffix = '/'.join(parts[-depth:]).lower() + index.setdefault(suffix, meta) + except Exception as e: + logger.error("Error building DB suffix index: %s", e) + finally: + if conn: + conn.close() + return index + + def _match_db(self, fpath: str, db_index: dict): + """Match a walked file to a DB track via path suffix. Returns the track + meta dict, or None when the file isn't part of the library.""" + parts = fpath.replace('\\', '/').split('/') + for depth in range(min(3, len(parts)), 0, -1): + suffix = '/'.join(parts[-depth:]).lower() + hit = db_index.get(suffix) + if hit: + return hit + return None + + def _read_file_tags(self, fpath: str) -> dict: + """Read title/artist/album from the file's own tags (for loose files + when library_tracks_only is off).""" + meta = {'track_id': None} + try: + from mutagen import File as MutagenFile + audio = MutagenFile(fpath, easy=True) + if audio: + meta['title'] = (audio.get('title') or [None])[0] or '' + meta['artist'] = (audio.get('artist') or audio.get('albumartist') or [None])[0] or '' + meta['album'] = (audio.get('album') or [None])[0] or '' + except Exception as e: + logger.debug("tag read failed for %s: %s", os.path.basename(fpath), e) + return meta + + def estimate_scope(self, context: JobContext) -> int: + count = 0 + for base in self._collect_music_dirs(context): + for _root, _dirs, files in os.walk(base): + for fname in files: + if os.path.splitext(fname)[1].lower() in AUDIO_EXTENSIONS: + count += 1 + return count diff --git a/core/repair_worker.py b/core/repair_worker.py index 9a552a5f..64f9864a 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -994,9 +994,9 @@ class RepairWorker: 'unwanted_content': self._fix_unwanted_content, 'unknown_artist': self._fix_unknown_artist, 'acoustid_mismatch': self._fix_acoustid_mismatch, + 'quality_upgrade': self._fix_quality_upgrade, 'missing_discography_track': self._fix_discography_backfill, 'library_retag': self._fix_library_retag, - 'quality_upgrade': self._fix_quality_upgrade, } handler = handlers.get(finding_type) if not handler: @@ -1024,9 +1024,43 @@ class RepairWorker: return {'success': False, 'error': str(e)} def _fix_quality_upgrade(self, entity_type, entity_id, file_path, details): - """Add the matched higher-quality version to the wishlist (with album - context). Applying a Quality Upgrade finding is the user-approved step - that the old auto-acting Quality Scanner did without review.""" + """Apply a Quality Upgrade finding (user-approved; the old Quality + Scanner did this without review). Action via ``details['_fix_action']``: + + 'redownload' (default): add the matched higher-quality version to the + wishlist (with album context) for a profile-gated re-download. + The low-quality file stays in place — it's replaced only after the + better version actually imports (safe pattern; auto-delete-on- + import is handled separately). + 'delete': remove the low-quality file + its DB row outright. + 'ignore' is handled in the UI by dismissing the finding — never here. + """ + fix_action = details.get('_fix_action', 'redownload') + + if fix_action == 'delete': + if file_path: + resolved = _resolve_file_path( + file_path, self.transfer_folder, + config_manager=self._config_manager) + if resolved and os.path.exists(resolved): + try: + os.remove(resolved) + self._cleanup_empty_parents(resolved) + except Exception as e: + logger.warning("Could not delete low-quality file %s: %s", + resolved, e) + if entity_id: + try: + conn = self.db._get_connection() + conn.cursor().execute("DELETE FROM tracks WHERE id = ?", (entity_id,)) + conn.commit() + conn.close() + except Exception as e: + return {'success': False, 'error': f'DB delete failed: {e}'} + return {'success': True, 'action': 'deleted_file', + 'message': f'Deleted low-quality file: ' + f'{os.path.basename(file_path or "")}'} + track_data = details.get('matched_track_data') if not track_data: return {'success': False, 'error': 'No matched track in finding'} @@ -3384,7 +3418,8 @@ class RepairWorker: 'album_tag_inconsistency', 'incomplete_album', 'path_mismatch', 'missing_lossy_copy', 'missing_replaygain', 'empty_folder', - 'missing_discography_track', 'acoustid_mismatch') + 'missing_discography_track', 'acoustid_mismatch', + 'quality_upgrade') placeholders = ','.join(['?'] * len(fixable_types)) where_parts = [f"finding_type IN ({placeholders})", "status = 'pending'"] params = list(fixable_types) diff --git a/core/soulseek_client.py b/core/soulseek_client.py index 28c92393..18e20a63 100644 --- a/core/soulseek_client.py +++ b/core/soulseek_client.py @@ -24,6 +24,8 @@ from core.download_plugins.album_bundle import ( get_poll_timeout, ) from core.download_plugins.base import DownloadSourcePlugin +from core.quality.model import QualityTarget, filter_and_rank, v2_qualities_to_ranked_targets +from core.quality.source_map import AUDIO_EXTENSIONS, format_from_extension from utils.async_helpers import run_async logger = get_logger("soulseek_client") @@ -409,7 +411,7 @@ class SoulseekClient(DownloadSourcePlugin): # Audio file extensions to filter for - audio_extensions = {'.mp3', '.flac', '.ogg', '.aac', '.wma', '.wav', '.m4a'} + audio_extensions = AUDIO_EXTENSIONS for response_data in responses_data: username = response_data.get('username', '') @@ -426,26 +428,28 @@ class SoulseekClient(DownloadSourcePlugin): if f'.{file_ext}' not in audio_extensions: continue - # .m4a is the usual AAC container — bucket it as 'aac' (the - # quality filter treats AAC as an opt-in tier; off by default). - quality = 'aac' if file_ext == 'm4a' else ( - file_ext if file_ext in ['flac', 'mp3', 'ogg', 'aac', 'wma'] else 'unknown') + # Source-agnostic extension → format (shared with every other + # extension-based source). Ranked targets do the rest. + quality = format_from_extension(file_ext) # Create TrackResult # Convert duration from seconds to milliseconds (slskd returns seconds, Spotify uses ms) raw_duration = file_data.get('length') duration_ms = raw_duration * 1000 if raw_duration else None + slskd_attrs = {a['type']: a['value'] for a in file_data.get('attributes', [])} track = TrackResult( username=username, filename=filename, size=size, - bitrate=file_data.get('bitRate'), + bitrate=file_data.get('bitRate') or slskd_attrs.get(0), duration=duration_ms, quality=quality, free_upload_slots=response_data.get('freeUploadSlots', 0), upload_speed=response_data.get('uploadSpeed', 0), - queue_length=response_data.get('queueLength', 0) + queue_length=response_data.get('queueLength', 0), + sample_rate=slskd_attrs.get(4), + bit_depth=slskd_attrs.get(5), ) all_tracks.append(track) @@ -1136,7 +1140,7 @@ class SoulseekClient(DownloadSourcePlugin): Returns: List of TrackResult objects for audio files """ - audio_extensions = {'.mp3', '.flac', '.ogg', '.aac', '.wma', '.wav', '.m4a'} + audio_extensions = AUDIO_EXTENSIONS results = [] if files: logger.debug(f"Browse raw file sample: {files[0]}") @@ -1150,15 +1154,17 @@ class SoulseekClient(DownloadSourcePlugin): ext = Path(filename).suffix.lower() if ext not in audio_extensions: continue - _qext = ext.lstrip('.') - quality = 'aac' if _qext == 'm4a' else ( - _qext if _qext in ['flac', 'mp3', 'ogg', 'aac', 'wma'] else 'unknown') + quality = format_from_extension(ext) raw_duration = file_data.get('length') duration_ms = raw_duration * 1000 if raw_duration else None + slskd_attrs = {a['type']: a['value'] for a in file_data.get('attributes', [])} results.append(TrackResult( username=username, filename=filename, size=file_data.get('size', 0), - bitrate=file_data.get('bitRate'), duration=duration_ms, quality=quality, - free_upload_slots=free_slots, upload_speed=upload_speed, queue_length=queue_length + bitrate=file_data.get('bitRate') or slskd_attrs.get(0), + duration=duration_ms, quality=quality, + free_upload_slots=free_slots, upload_speed=upload_speed, queue_length=queue_length, + sample_rate=slskd_attrs.get(4), + bit_depth=slskd_attrs.get(5), )) return results @@ -2008,189 +2014,57 @@ class SoulseekClient(DownloadSourcePlugin): return kept def filter_results_by_quality_preference(self, results: List[TrackResult]) -> List[TrackResult]: - """ - Filter candidates based on user's quality profile with bitrate density constraints. - Uses priority waterfall logic: tries highest priority quality first, falls back to lower priorities. - Returns candidates matching quality profile constraints, sorted by confidence and effective bitrate. + """Filter and rank candidates using the global quality target list. - Issue #652: also drops candidates whose `(username, filename)` - matches a previously-quarantined download. Without this pre-filter - the auto-wishlist processor's ranking is deterministic — the same - `(uploader, file)` keeps winning the quality picker, downloading, - failing AcoustID, quarantining, and re-queueing in an infinite - loop. Users wake up to hundreds of duplicate `.quarantined` files - for the same source URL. + Replaces the old bucket+heuristic approach with ``core.quality.model`` + so every download source shares the same ranking logic. + + Issue #652: also drops candidates whose ``(username, filename)`` + matches a previously-quarantined download to break infinite retry loops. """ from database.music_database import MusicDatabase if not results: return [] - # Drop sources already quarantined — bypass the quality picker - # entirely so the same bad upload doesn't get re-selected on the - # next wishlist cycle. Filesystem read is bounded (~few hundred - # sidecars in practical use × <1ms each). + # Issue #652: drop candidates on the quarantine record BEFORE ranking, + # so a previously-quarantined source can't win the quality picker by + # superior bitrate and re-trigger the same failed download in a loop. results = self._drop_quarantined_sources(results) if not results: return [] - # Get quality profile from database db = MusicDatabase() profile = db.get_quality_profile() - logger.debug(f"Quality Filter: Using profile preset '{profile.get('preset', 'custom')}', filtering {len(results)} candidates") + # Build ranked target list — v3 profiles carry it directly; + # v2 profiles are converted on the fly (no DB write needed here). + raw_targets = profile.get('ranked_targets') + if not raw_targets and 'qualities' in profile: + raw_targets = v2_qualities_to_ranked_targets(profile['qualities']) - # Categorize candidates by quality with bitrate density constraints - quality_buckets = { - 'flac': [], - 'mp3_320': [], - 'mp3_256': [], - 'mp3_192': [], - 'aac': [], - 'other': [] - } + targets = [QualityTarget.from_dict(t) for t in (raw_targets or [])] + fallback_enabled = profile.get('fallback_enabled', True) - # Track all candidates that pass checks (for fallback) - density_filtered_all = [] + # Every format (AAC included) follows the SAME universal rule: a + # candidate passes only if it matches a ranked target; if nothing + # matches, the fallback toggle decides. No per-format special-casing. - for candidate in results: - if not candidate.quality: - quality_buckets['other'].append(candidate) - continue + logger.debug( + "Quality Filter: profile='%s', %d targets, %d candidates", + profile.get('preset', 'custom'), len(targets), len(results), + ) - track_format = candidate.quality.lower() - track_bitrate = candidate.bitrate or 0 + ranked = filter_and_rank(results, targets, fallback_enabled=fallback_enabled) - # Determine quality key - if track_format == 'flac': - quality_key = 'flac' - elif track_format == 'mp3': - if track_bitrate >= 320: - quality_key = 'mp3_320' - elif track_bitrate >= 256: - quality_key = 'mp3_256' - elif track_bitrate >= 192: - quality_key = 'mp3_192' - else: - quality_buckets['other'].append(candidate) - continue - elif track_format in ('aac', 'm4a'): - # Opt-in AAC tier. ADDITIVE: when AAC isn't enabled in the - # profile (the default, and every profile that predates this), - # route exactly where AAC went before — the 'other' bucket — so - # behaviour is byte-identical. Only a user who turns AAC on lets - # it become a first-class, selectable tier. - aac_cfg = profile['qualities'].get('aac') - if not (aac_cfg and aac_cfg.get('enabled')): - quality_buckets['other'].append(candidate) - continue - quality_key = 'aac' - else: - quality_buckets['other'].append(candidate) - continue - - quality_config = profile['qualities'].get(quality_key, {}) - min_kbps = quality_config.get('min_kbps', 0) - max_kbps = quality_config.get('max_kbps', 99999) - - effective_kbps = self._calculate_effective_kbps(candidate.size, candidate.duration) - - if effective_kbps is not None: - # Primary: bitrate density check - if min_kbps <= effective_kbps <= max_kbps: - if quality_config.get('enabled', False): - quality_buckets[quality_key].append(candidate) - density_filtered_all.append(candidate) - else: - logger.debug(f"Quality Filter: {quality_key} rejected - {effective_kbps:.0f} kbps outside {min_kbps}-{max_kbps} kbps range") - else: - # Fallback: duration unavailable, use generous raw-size sanity check - file_size_mb = candidate.size / (1024 * 1024) - size_min, size_max = self._FALLBACK_SIZE_LIMITS.get(quality_key, (0, 500)) - if size_min <= file_size_mb <= size_max: - if quality_config.get('enabled', False): - quality_buckets[quality_key].append(candidate) - density_filtered_all.append(candidate) - logger.debug(f"Quality Filter: {quality_key} accepted via size fallback ({file_size_mb:.1f} MB, no duration available)") - else: - logger.debug(f"Quality Filter: {quality_key} rejected via size fallback - {file_size_mb:.1f} MB outside {size_min}-{size_max} MB safety limits") - - # Sort each bucket: effective bitrate first (prefer highest audio quality), - # then peer quality score as tiebreaker (prefer fastest peer at same quality) - for bucket in quality_buckets.values(): - bucket.sort(key=lambda x: (self._calculate_effective_kbps(x.size, x.duration) or 0, x.quality_score), reverse=True) - - # Enforce FLAC bit depth preference from quality profile - flac_config = profile['qualities'].get('flac', {}) - bit_depth_pref = flac_config.get('bit_depth', 'any') - bit_depth_fallback = flac_config.get('bit_depth_fallback', True) - - if bit_depth_pref != 'any' and quality_buckets['flac']: - # 16-bit/44.1kHz FLAC theoretical max is 1411 kbps; 24-bit starts at ~2116 kbps - # Real-world compressed: 16-bit = 800-1400 kbps, 24-bit = 1500+ kbps - DEPTH_THRESHOLD = 1450 - - if bit_depth_pref == '24': - hi_res = [c for c in quality_buckets['flac'] - if (self._calculate_effective_kbps(c.size, c.duration) or 0) > DEPTH_THRESHOLD] - if hi_res: - logger.info(f"Quality Filter: Bit depth 24-bit preference — {len(hi_res)}/{len(quality_buckets['flac'])} FLAC candidates are hi-res") - quality_buckets['flac'] = hi_res - elif not bit_depth_fallback: - logger.info("Quality Filter: No 24-bit FLAC found and fallback disabled — rejecting all FLAC") - quality_buckets['flac'] = [] - else: - logger.info("Quality Filter: No 24-bit FLAC found — falling back to 16-bit") - - elif bit_depth_pref == '16': - lo_res = [c for c in quality_buckets['flac'] - if (self._calculate_effective_kbps(c.size, c.duration) or 0) <= DEPTH_THRESHOLD] - if lo_res: - logger.info(f"Quality Filter: Bit depth 16-bit preference — {len(lo_res)}/{len(quality_buckets['flac'])} FLAC candidates are standard") - quality_buckets['flac'] = lo_res - elif not bit_depth_fallback: - logger.info("Quality Filter: No 16-bit FLAC found and fallback disabled — rejecting all FLAC") - quality_buckets['flac'] = [] - else: - logger.info("Quality Filter: No 16-bit FLAC found — falling back to 24-bit") - - # Debug logging - for quality, bucket in quality_buckets.items(): - if bucket: - logger.debug(f"Quality Filter: Found {len(bucket)} '{quality}' candidates (after bitrate + bit depth filtering)") - - # Waterfall priority logic: try qualities in priority order - # Build priority list from enabled qualities - quality_priorities = [] - for quality_name, quality_config in profile['qualities'].items(): - if quality_config.get('enabled', False): - priority = quality_config.get('priority', 999) - quality_priorities.append((priority, quality_name)) - - # Sort by priority (lower number = higher priority) - quality_priorities.sort() - - # Try each quality in priority order - for priority, quality_name in quality_priorities: - candidates_for_quality = quality_buckets.get(quality_name, []) - if candidates_for_quality: - logger.info(f"Quality Filter: Returning {len(candidates_for_quality)} '{quality_name}' candidates (priority {priority})") - return candidates_for_quality - - # If no enabled qualities matched, check if fallback is enabled - if profile.get('fallback_enabled', True): - logger.warning("Quality Filter: No enabled qualities matched, falling back to density-filtered candidates") - if density_filtered_all: - density_filtered_all.sort(key=lambda x: (x.quality_score, self._calculate_effective_kbps(x.size, x.duration) or 0), reverse=True) - logger.info(f"Quality Filter: Returning {len(density_filtered_all)} fallback candidates (bitrate-filtered, any quality)") - return density_filtered_all - else: - logger.warning("Quality Filter: All candidates failed bitrate checks, returning empty (respecting constraints)") - return [] + if ranked: + best_label = ranked[0].audio_quality.label() + logger.info("Quality Filter: returning %d candidate(s), best=%s", len(ranked), best_label) else: - logger.warning("Quality Filter: No enabled qualities matched and fallback is disabled, returning empty") - return [] - + logger.warning("Quality Filter: no candidates passed quality constraints") + + return ranked + async def get_session_info(self) -> Optional[Dict[str, Any]]: """Get slskd session information including version""" if not self.base_url: diff --git a/core/tidal_download_client.py b/core/tidal_download_client.py index 4192606a..5e2f4524 100644 --- a/core/tidal_download_client.py +++ b/core/tidal_download_client.py @@ -33,6 +33,7 @@ from config.settings import config_manager # Import Soulseek data structures for drop-in replacement compatibility from core.download_plugins.types import TrackResult, AlbumResult, DownloadStatus +from core.quality.source_map import quality_from_tidal_tier, quality_tier_for_source logger = get_logger("tidal_download_client") @@ -455,13 +456,17 @@ class TidalDownloadClient(DownloadSourcePlugin): if successful_query and successful_query != query: logger.info(f"Tidal fallback query succeeded: '{successful_query}' (original: '{query}')") - quality_key = config_manager.get('tidal_download.quality', 'lossless') + quality_key = quality_tier_for_source('tidal', default='lossless') quality_info = QUALITY_MAP.get(quality_key, QUALITY_MAP['lossless']) + # Stamp the configured tier (what will actually be downloaded) so + # the global ranker sees real sample_rate/bit_depth. + tier_quality = quality_from_tidal_tier(quality_key) track_results = [] for track in tidal_tracks: try: track_result = self._tidal_to_track_result(track, quality_info) + track_result.set_quality(tier_quality) track_results.append(track_result) except Exception as e: logger.debug(f"Skipping track conversion error: {e}") @@ -774,7 +779,7 @@ class TidalDownloadClient(DownloadSourcePlugin): logger.error("Tidal session not authenticated") return None - quality_key = config_manager.get('tidal_download.quality', 'lossless') + quality_key = quality_tier_for_source('tidal', default='lossless') chain = ['hires', 'lossless', 'high', 'low'] start = chain.index(quality_key) if quality_key in chain else 1 allow_fallback = config_manager.get('tidal_download.allow_fallback', True) diff --git a/database/music_database.py b/database/music_database.py index 2eb1ed80..c173015b 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -677,6 +677,12 @@ class MusicDatabase: cursor.execute(f"ALTER TABLE library_history ADD COLUMN {_col} TEXT") logger.info(f"Added {_col} column to library_history") + # Index on verification_status — MUST come after the ALTER above: + # on a fresh DB the base CREATE TABLE has no verification_status + # column, so indexing it before the migration adds it raises + # "no such column: verification_status" and aborts DB init. + cursor.execute("CREATE INDEX IF NOT EXISTS idx_lh_verification_status ON library_history (verification_status)") + # One-time backfill: derive verification_status for history rows # written before the column existed (or by pipeline exits that # missed it) from the acoustid_result those imports already @@ -8742,7 +8748,7 @@ class MusicDatabase: # Quality profile management methods def get_quality_profile(self) -> dict: - """Get the quality profile configuration, returns default if not set""" + """Get the quality profile configuration, returns default if not set.""" import json profile_json = self.get_preference('quality_profile') @@ -8750,21 +8756,85 @@ class MusicDatabase: if profile_json: try: profile = json.loads(profile_json) - # Migrate v1 profiles (min_mb/max_mb) to v2 (min_kbps/max_kbps) - if profile.get('version', 1) < 2: - logger.info("Migrating quality profile from v1 (file size) to v2 (bitrate density)") + version = profile.get('version', 1) + if version < 2: + logger.info("Migrating quality profile v1 → v3") return self._get_default_quality_profile() + if version == 2: + logger.info("Migrating quality profile v2 → v3 (adding ranked_targets)") + return self._migrate_v2_to_v3(profile) return profile except json.JSONDecodeError: logger.error("Failed to parse quality profile JSON, returning default") return self._get_default_quality_profile() + # 24-bit FLAC ladder seeded on migration for users who had a streaming + # source on Hi-Res under the old (now removed) per-source quality dropdowns. + _HIRES_24BIT_TARGETS = [ + {"label": "FLAC 24-bit/192kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 192000}, + {"label": "FLAC 24-bit/96kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 96000}, + {"label": "FLAC 24-bit/48kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 48000}, + {"label": "FLAC 24-bit/44.1kHz","format": "flac", "bit_depth": 24, "min_sample_rate": 44100}, + ] + + def _had_hires_source_preference(self) -> bool: + """True if the user had any streaming source set to a Hi-Res tier under + the old per-source quality dropdowns (tidal_download/qobuz/hifi_download + .quality = 'hires'|'hires_max'), which #896 removed in favour of the + global profile. Used to preserve their intent on migration.""" + try: + from config.settings import config_manager + except Exception: + return False + hires = {'hires', 'hires_max'} + for key in ('tidal_download.quality', 'qobuz.quality', 'hifi_download.quality'): + try: + if str(config_manager.get(key) or '').strip().lower() in hires: + return True + except Exception: + continue + return False + + def _migrate_v2_to_v3(self, profile: dict) -> dict: + """Add ranked_targets to a v2 profile without losing its qualities dict.""" + from core.quality.model import v2_qualities_to_ranked_targets + profile = dict(profile) + profile['version'] = 3 + if 'ranked_targets' not in profile: + ranked = v2_qualities_to_ranked_targets(profile.get('qualities', {})) + # #896 review #5: the per-source quality dropdowns are gone — sources + # now derive their tier from this profile. If the user had a source on + # Hi-Res, seed 24-bit FLAC targets at the top so they keep Hi-Res + # instead of silently dropping to lossless. Skip when the profile + # already expresses 24-bit (don't duplicate the ladder). + already_24bit = any( + t.get('format') == 'flac' and (t.get('bit_depth') or 0) >= 24 + for t in ranked + ) + if not already_24bit and self._had_hires_source_preference(): + ranked = [dict(t) for t in self._HIRES_24BIT_TARGETS] + ranked + profile['ranked_targets'] = ranked + return profile + def _get_default_quality_profile(self) -> dict: - """Return the default v2 quality profile (balanced preset)""" + """Return the default v3 quality profile (balanced preset).""" return { - "version": 2, + "version": 3, "preset": "balanced", + "fallback_enabled": True, + "search_mode": "priority", + "ranked_targets": [ + {"label": "FLAC 24-bit/192kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 192000}, + {"label": "FLAC 24-bit/96kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 96000}, + {"label": "FLAC 24-bit/48kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 48000}, + {"label": "FLAC 24-bit/44.1kHz","format": "flac", "bit_depth": 24, "min_sample_rate": 44100}, + {"label": "FLAC 16-bit", "format": "flac", "bit_depth": 16}, + {"label": "MP3 320kbps", "format": "mp3", "min_bitrate": 320}, + {"label": "MP3 256kbps", "format": "mp3", "min_bitrate": 256}, + {"label": "MP3 192kbps", "format": "mp3", "min_bitrate": 192}, + ], + # Keep qualities dict for backwards compat with any old code paths still reading it "qualities": { "flac": { "enabled": True, @@ -8801,141 +8871,119 @@ class MusicDatabase: "priority": 1.5 } }, - "fallback_enabled": True } + # Presets whose per-preset customizations we remember across switches. + _KNOWN_PRESETS = ('audiophile', 'balanced', 'space_saver') + def set_quality_profile(self, profile: dict) -> bool: - """Save quality profile configuration""" + """Save quality profile configuration. + + Besides the single active profile (read by the download pipeline), we also + stash the profile under its preset name so switching presets and coming + back restores the user's edits instead of the factory defaults. 'custom' + and unknown preset names are not stashed.""" import json try: profile_json = json.dumps(profile) self.set_preference('quality_profile', profile_json) + + preset_name = profile.get('preset') + if preset_name in self._KNOWN_PRESETS: + store = self._load_preset_store() + store[preset_name] = profile + self.set_preference('quality_profile_presets', json.dumps(store)) + logger.info(f"Quality profile saved: preset={profile.get('preset', 'custom')}") return True except Exception as e: logger.error(f"Failed to save quality profile: {e}") return False - def get_quality_preset(self, preset_name: str) -> dict: - """Get a predefined quality preset""" + def _load_preset_store(self) -> dict: + """Per-preset customizations, keyed by preset name. {} if none saved.""" + import json + raw = self.get_preference('quality_profile_presets') + if raw: + try: + parsed = json.loads(raw) + if isinstance(parsed, dict): + return parsed + except json.JSONDecodeError: + logger.error("Failed to parse quality_profile_presets, ignoring") + return {} + + def reset_quality_preset(self, preset_name: str) -> dict: + """Forget a preset's saved customizations and return its factory defaults.""" + import json + store = self._load_preset_store() + if preset_name in store: + del store[preset_name] + self.set_preference('quality_profile_presets', json.dumps(store)) + return self.get_quality_preset(preset_name, customized=False) + + def get_quality_preset(self, preset_name: str, *, customized: bool = True) -> dict: + """Get a quality preset (v3 format with ranked_targets). + + With ``customized`` (default), a preset the user has edited is returned in + its saved form; otherwise the hard-coded factory defaults are returned.""" + if customized: + saved = self._load_preset_store().get(preset_name) + if saved: + return saved + return self._factory_quality_preset(preset_name) + + def _factory_quality_preset(self, preset_name: str) -> dict: + """The hard-coded factory defaults for a preset (ignores customizations).""" + # Strict 24-bit FLAC ladder — no 16-bit, no lossy. This is what + # "audiophile" means: only true hi-res passes. + _FLAC_24BIT_TARGETS = [ + {"label": "FLAC 24-bit/192kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 192000}, + {"label": "FLAC 24-bit/96kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 96000}, + {"label": "FLAC 24-bit/48kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 48000}, + {"label": "FLAC 24-bit/44.1kHz","format": "flac", "bit_depth": 24, "min_sample_rate": 44100}, + ] + # Lossless ladder used by "balanced" — hi-res first, then CD-quality 16-bit. + _FLAC_HI_RES_TARGETS = _FLAC_24BIT_TARGETS + [ + {"label": "FLAC 16-bit", "format": "flac", "bit_depth": 16}, + ] + _MP3_TARGETS = [ + {"label": "MP3 320kbps", "format": "mp3", "min_bitrate": 320}, + {"label": "MP3 256kbps", "format": "mp3", "min_bitrate": 256}, + {"label": "MP3 192kbps", "format": "mp3", "min_bitrate": 192}, + ] + # Legacy v2 ``qualities`` dict carried alongside ranked_targets for + # backwards compat — read by the settings UI and the #886 AAC opt-in + # toggle. AAC ships OFF in every preset; its priority sits it above MP3 + # but below FLAC (space_saver puts it at 0.5, still above its MP3 tiers). + def _quals(*, flac_en, flac_prio, mp3_320_en, mp3_256_en, mp3_192_en, + mp3_320_prio=2, mp3_256_prio=3, mp3_192_prio=4, aac_prio=1.5): + return { + "flac": {"enabled": flac_en, "min_kbps": 500, "max_kbps": 10000, "priority": flac_prio, "bit_depth": "any"}, + "mp3_320": {"enabled": mp3_320_en, "min_kbps": 280, "max_kbps": 500, "priority": mp3_320_prio}, + "mp3_256": {"enabled": mp3_256_en, "min_kbps": 200, "max_kbps": 400, "priority": mp3_256_prio}, + "mp3_192": {"enabled": mp3_192_en, "min_kbps": 150, "max_kbps": 300, "priority": mp3_192_prio}, + "aac": {"enabled": False, "min_kbps": 128, "max_kbps": 400, "priority": aac_prio}, + } + presets = { "audiophile": { - "version": 2, - "preset": "audiophile", - "qualities": { - "flac": { - "enabled": True, - "min_kbps": 500, - "max_kbps": 10000, - "priority": 1, - "bit_depth": "any" - }, - "mp3_320": { - "enabled": False, - "min_kbps": 280, - "max_kbps": 500, - "priority": 2 - }, - "mp3_256": { - "enabled": False, - "min_kbps": 200, - "max_kbps": 400, - "priority": 3 - }, - "mp3_192": { - "enabled": False, - "min_kbps": 150, - "max_kbps": 300, - "priority": 4 - }, - "aac": { - "enabled": False, - "min_kbps": 128, - "max_kbps": 400, - "priority": 1.5 - } - }, - "fallback_enabled": False + "version": 3, "preset": "audiophile", "fallback_enabled": False, + "ranked_targets": _FLAC_24BIT_TARGETS, + "qualities": _quals(flac_en=True, flac_prio=1, mp3_320_en=False, mp3_256_en=False, mp3_192_en=False), }, "balanced": { - "version": 2, - "preset": "balanced", - "qualities": { - "flac": { - "enabled": True, - "min_kbps": 500, - "max_kbps": 10000, - "priority": 1, - "bit_depth": "any" - }, - "mp3_320": { - "enabled": True, - "min_kbps": 280, - "max_kbps": 500, - "priority": 2 - }, - "mp3_256": { - "enabled": True, - "min_kbps": 200, - "max_kbps": 400, - "priority": 3 - }, - "mp3_192": { - "enabled": False, - "min_kbps": 150, - "max_kbps": 300, - "priority": 4 - }, - "aac": { - "enabled": False, - "min_kbps": 128, - "max_kbps": 400, - "priority": 1.5 - } - }, - "fallback_enabled": True + "version": 3, "preset": "balanced", "fallback_enabled": True, + "ranked_targets": _FLAC_HI_RES_TARGETS + _MP3_TARGETS, + "qualities": _quals(flac_en=True, flac_prio=1, mp3_320_en=True, mp3_256_en=True, mp3_192_en=False), }, "space_saver": { - "version": 2, - "preset": "space_saver", - "qualities": { - "flac": { - "enabled": False, - "min_kbps": 500, - "max_kbps": 10000, - "priority": 4, - "bit_depth": "any" - }, - "mp3_320": { - "enabled": True, - "min_kbps": 280, - "max_kbps": 500, - "priority": 1 - }, - "mp3_256": { - "enabled": True, - "min_kbps": 200, - "max_kbps": 400, - "priority": 2 - }, - "mp3_192": { - "enabled": True, - "min_kbps": 150, - "max_kbps": 300, - "priority": 3 - }, - # Space-saver favours small files, where AAC shines — but it - # still ships OFF (opt-in). Priority 0.5 puts it above MP3. - "aac": { - "enabled": False, - "min_kbps": 128, - "max_kbps": 400, - "priority": 0.5 - } - }, - "fallback_enabled": True - } + "version": 3, "preset": "space_saver", "fallback_enabled": True, + "ranked_targets": _MP3_TARGETS, + "qualities": _quals(flac_en=False, flac_prio=4, mp3_320_en=True, mp3_256_en=True, mp3_192_en=True, + mp3_320_prio=1, mp3_256_prio=2, mp3_192_prio=3, aac_prio=0.5), + }, } return presets.get(preset_name, presets["balanced"]) @@ -13483,7 +13531,10 @@ class MusicDatabase: download_source, source_track_id, source_track_title, source_filename, acoustid_result, source_artist, origin, origin_context, verification_status)) conn.commit() - return True + # Return the new row id (truthy on success) so callers can link the + # live download task to its library_history row — e.g. the Unverified + # review queue needs the id for its play/approve/delete actions. + return cursor.lastrowid except Exception as e: logger.debug(f"Error adding library history entry: {e}") return False @@ -13681,6 +13732,26 @@ class MusicDatabase: logger.error(f"Error querying library history: {e}") return [], 0 + def get_library_history_unverified(self) -> list[dict]: + """Return every library_history row that still needs human confirmation. + + Fetches all rows where verification_status is 'unverified' or + 'force_imported', ordered newest-first. No row limit — the full + set must always be visible on the Downloads → Unverified tab. + """ + try: + conn = self._get_connection() + cursor = conn.cursor() + cursor.execute(""" + SELECT * FROM library_history + WHERE verification_status IN ('unverified', 'force_imported') + ORDER BY created_at DESC + """) + return [dict(row) for row in cursor.fetchall()] + except Exception as e: + logger.error("Error querying unverified library history: %s", e) + return [] + def get_library_history_stats(self): """Return counts per event_type and per download_source.""" try: diff --git a/tests/downloads/test_candidate_ordering.py b/tests/downloads/test_candidate_ordering.py new file mode 100644 index 00000000..c66fde84 --- /dev/null +++ b/tests/downloads/test_candidate_ordering.py @@ -0,0 +1,69 @@ +"""order_candidates — the candidate sort used by attempt_download_with_candidates. + +Default (priority mode) sorts confidence-first, then peer quality — today's +behaviour, locked here as a regression guard. quality_first=True (best-quality +mode) makes the user's profile quality rank dominate, with confidence as the +tiebreaker. Both keep correctly-matched candidates; ordering only changes which +is tried first. +""" + +from core.downloads.candidates import order_candidates +from core.quality.model import AudioQuality, QualityTarget + + +class _Cand: + def __init__(self, name, aq, confidence, quality_score=0, + upload_speed=0, queue_length=0, free_upload_slots=0, size=0): + self.name = name + self.audio_quality = aq + self.confidence = confidence + self.quality_score = quality_score + self.upload_speed = upload_speed + self.queue_length = queue_length + self.free_upload_slots = free_upload_slots + self.size = size + + +FLAC_HI = AudioQuality('flac', sample_rate=96000, bit_depth=24) +FLAC_CD = AudioQuality('flac', sample_rate=44100, bit_depth=16) + +TARGETS = [ + QualityTarget(label='FLAC 24', format='flac', bit_depth=24, min_sample_rate=96000), + QualityTarget(label='FLAC 16', format='flac', bit_depth=16), +] + + +def test_priority_mode_is_confidence_first(): + hi = _Cand('hi-flac', FLAC_HI, confidence=0.80) + lo = _Cand('cd-flac', FLAC_CD, confidence=0.95) + + ordered = order_candidates([hi, lo], quality_first=False, targets=TARGETS) + + assert [c.name for c in ordered] == ['cd-flac', 'hi-flac'] # higher confidence wins + + +def test_quality_first_lets_better_quality_win_over_confidence(): + hi = _Cand('hi-flac', FLAC_HI, confidence=0.80) + lo = _Cand('cd-flac', FLAC_CD, confidence=0.95) + + ordered = order_candidates([hi, lo], quality_first=True, targets=TARGETS) + + assert [c.name for c in ordered] == ['hi-flac', 'cd-flac'] # 24-bit beats higher-confidence 16-bit + + +def test_quality_first_uses_confidence_as_tiebreak_within_same_quality(): + a = _Cand('a', FLAC_HI, confidence=0.70) + b = _Cand('b', FLAC_HI, confidence=0.90) + + ordered = order_candidates([a, b], quality_first=True, targets=TARGETS) + + assert [c.name for c in ordered] == ['b', 'a'] # same quality → confidence breaks tie + + +def test_quality_first_ranks_unmatched_quality_last(): + matched = _Cand('matched', FLAC_CD, confidence=0.50) + off_list = _Cand('off', AudioQuality('mp3', bitrate=320), confidence=0.99) + + ordered = order_candidates([off_list, matched], quality_first=True, targets=TARGETS) + + assert [c.name for c in ordered] == ['matched', 'off'] # off-list sorts last despite high confidence diff --git a/tests/downloads/test_downloads_cancel.py b/tests/downloads/test_downloads_cancel.py index 7d90f0ee..0dfac0c5 100644 --- a/tests/downloads/test_downloads_cancel.py +++ b/tests/downloads/test_downloads_cancel.py @@ -176,15 +176,30 @@ def test_clear_completed_drops_empty_batches(): assert download_batches['b3']['queue'] == ['t3'] -def test_clear_completed_prunes_terminal_task_ids_from_batch_queues(): - """Batch with mix of terminal + active tasks gets queue trimmed, not deleted.""" +def test_clear_completed_keeps_terminal_tasks_in_active_batch(): + """A completed/failed task in a batch that still has active work is KEPT, so + the Downloads page shows it for the whole run (the 5-min Clean Completed + automation must not yank terminal tasks out from under a live batch).""" download_tasks['t1'] = {'status': 'completed'} - download_tasks['t2'] = {'status': 'downloading'} + download_tasks['t2'] = {'status': 'downloading'} # batch still active download_batches['b1'] = {'queue': ['t1', 't2']} - cancel.clear_completed_local() + cleared = cancel.clear_completed_local() + assert cleared == 0 # nothing removed — batch is live assert 'b1' in download_batches - assert download_batches['b1']['queue'] == ['t2'] + assert download_batches['b1']['queue'] == ['t1', 't2'] # queue intact + + +def test_clear_completed_clears_terminal_tasks_once_batch_is_finished(): + """When every task in a batch is terminal, the batch is done — its tasks are + cleared and the empty batch dropped.""" + download_tasks['t1'] = {'status': 'completed'} + download_tasks['t2'] = {'status': 'failed'} + download_batches['b1'] = {'queue': ['t1', 't2']} + + cleared = cancel.clear_completed_local() + assert cleared == 2 + assert 'b1' not in download_batches def test_clear_completed_drops_batch_locks_for_deleted_batches(): diff --git a/tests/downloads/test_downloads_lifecycle.py b/tests/downloads/test_downloads_lifecycle.py index f9da4861..8b4480d0 100644 --- a/tests/downloads/test_downloads_lifecycle.py +++ b/tests/downloads/test_downloads_lifecycle.py @@ -480,11 +480,33 @@ def test_stuck_searching_task_forced_to_not_found(): assert download_tasks['t1']['status'] == 'not_found' -def test_stuck_post_processing_task_forced_to_completed(): - """Task post_processing > 5min gets forced to completed.""" +def test_stuck_post_processing_without_file_forced_to_failed(): + """Task stuck in post_processing past the timeout with NO output file must + be marked FAILED, not falsely completed — otherwise it shows as a phantom + download with nothing on disk (big batches back up post-processing).""" + download_tasks['t1'] = { + 'status': 'post_processing', 'track_info': {'name': 'X'}, + 'status_change_time': 0, # ancient → past any timeout + } + download_batches['b1'] = { + 'queue': ['t1'], 'queue_index': 1, 'active_count': 1, + 'max_concurrent': 1, 'permanently_failed_tracks': [], + 'cancelled_tracks': set(), + } + deps, _ = _build_deps() + lc.on_download_completed('b1', 't1', True, deps) + assert download_tasks['t1']['status'] == 'failed' + + +def test_stuck_post_processing_with_existing_file_completed(tmp_path): + """If the import really finished (final_file_path exists on disk), a stuck + post_processing task is legitimately completed.""" + real_file = tmp_path / 'track.flac' + real_file.write_bytes(b'x') download_tasks['t1'] = { 'status': 'post_processing', 'track_info': {'name': 'X'}, 'status_change_time': 0, + 'final_file_path': str(real_file), } download_batches['b1'] = { 'queue': ['t1'], 'queue_index': 1, 'active_count': 1, diff --git a/tests/downloads/test_downloads_status.py b/tests/downloads/test_downloads_status.py index 7050e3e6..ecce784a 100644 --- a/tests/downloads/test_downloads_status.py +++ b/tests/downloads/test_downloads_status.py @@ -667,15 +667,44 @@ def test_unified_response_includes_batch_summaries(): assert bs['queued'] == 1 -def test_unified_response_respects_limit(): +def test_unified_response_returns_all_live_tasks_even_past_limit(): + """`limit` bounds the persistent-history tail, NOT live in-memory tasks. + + Live tasks are already bounded by the 5-min cleanup, and the Downloads page + filters them client-side per tab — truncating them (active-first) starved + completed/failed/unverified rows out of the response during a busy batch so + they never showed until the batch drained. + """ deps, _ = _build_deps() for i in range(20): download_tasks[f't{i}'] = { 'track_index': i, 'status': 'completed', 'track_info': {}, } out = st.build_unified_downloads_response(5, deps) - assert len(out['downloads']) == 5 - assert out['total'] == 20 # total still reflects all + assert len(out['downloads']) == 20 # all live tasks returned + assert out['total'] == 20 + + +def test_unified_response_does_not_truncate_terminal_tasks_behind_active(): + """A busy batch (many queued/active tasks) must not push completed/failed + rows off the end of the response — they're what the Completed/Failed tabs + show during the run.""" + deps, _ = _build_deps() + for i in range(120): + download_tasks[f'q{i}'] = { + 'track_index': i, 'status': 'queued', + 'track_info': {'name': f'Q{i}'}, 'status_change_time': i, + } + download_tasks['done'] = { + 'status': 'completed', 'track_info': {'name': 'DONE'}, 'status_change_time': 999, + } + download_tasks['fail'] = { + 'status': 'failed', 'track_info': {'name': 'FAIL'}, 'status_change_time': 999, + } + out = st.build_unified_downloads_response(100, deps) + titles = {d['title'] for d in out['downloads']} + assert 'DONE' in titles + assert 'FAIL' in titles def test_unified_response_includes_persistent_download_history(): @@ -763,6 +792,68 @@ def test_unified_response_caps_persistent_history_tail(): assert out['total'] == 50 +def test_unverified_history_always_loaded_even_when_at_limit(): + """Unverified entries must appear even during a large batch that fills the limit.""" + for i in range(200): + download_tasks[f'live-{i}'] = { + 'track_index': i, + 'status': 'completed', + 'track_info': {'name': f'Track {i}', 'artist': f'Artist {i}', 'album': 'Album'}, + 'verification_status': 'verified', + } + + deps, _ = _build_deps( + persistent_history=lambda limit: [], + ) + deps.get_unverified_download_history = lambda: [ + { + 'id': 999, + 'title': 'Old Unverified', + 'artist_name': 'Forgotten Artist', + 'album_name': 'Old Album', + 'created_at': '2026-01-01 00:00:00', + 'verification_status': 'unverified', + } + ] + + out = st.build_unified_downloads_response(200, deps) + + titles = {d['title'] for d in out['downloads']} + assert 'Old Unverified' in titles, "historical unverified entry must appear regardless of limit" + + unverified = [d for d in out['downloads'] if d.get('verification_status') == 'unverified'] + assert len(unverified) == 1 + assert unverified[0]['task_id'] == 'history-999' + assert unverified[0]['is_persistent_history'] is True + + +def test_unverified_history_deduped_against_live_task(): + """A live task that's still unverified must not appear twice.""" + download_tasks['live-unv'] = { + 'track_index': 0, + 'status': 'completed', + 'track_info': {'name': 'Live Unverified', 'artist': 'Artsy', 'album': 'Alb'}, + 'verification_status': 'unverified', + } + + deps, _ = _build_deps() + deps.get_unverified_download_history = lambda: [ + { + 'id': 77, + 'title': 'Live Unverified', + 'artist_name': 'Artsy', + 'album_name': 'Alb', + 'created_at': '2026-06-15 10:00:00', + 'verification_status': 'unverified', + } + ] + + out = st.build_unified_downloads_response(200, deps) + matches = [d for d in out['downloads'] if d['title'] == 'Live Unverified'] + assert len(matches) == 1, "same track must not be duplicated" + assert matches[0]['is_persistent_history'] is False + + # --------------------------------------------------------------------------- # #836 — a rejected slskd transfer must not hang the task at 'downloading' # forever. The monitor normally retries; when it can't make progress, a backstop diff --git a/tests/downloads/test_downloads_task_worker.py b/tests/downloads/test_downloads_task_worker.py index 39320940..ed15a2c3 100644 --- a/tests/downloads/test_downloads_task_worker.py +++ b/tests/downloads/test_downloads_task_worker.py @@ -352,7 +352,7 @@ def test_first_query_success_returns_after_storing_source(): _seed_task() rec = _Recorder() - def _attempt_success(task_id, candidates, track, batch_id): + def _attempt_success(task_id, candidates, track, batch_id, **kwargs): download_tasks[task_id]['filename'] = 'song.flac' download_tasks[task_id]['username'] = 'u1' return True @@ -478,7 +478,7 @@ def test_hybrid_fallback_tries_secondary_sources(): }, ) - def _attempt_yt_success(task_id, candidates, track, batch_id): + def _attempt_yt_success(task_id, candidates, track, batch_id, **kwargs): return True deps, _ = _build_deps( @@ -528,7 +528,7 @@ def test_quarantine_retry_tries_cached_candidates_without_searching(): ) attempted = [] - def _attempt(task_id, candidates, track, batch_id): + def _attempt(task_id, candidates, track, batch_id, **kwargs): attempted.append([getattr(c, 'filename', None) for c in candidates]) return True @@ -555,7 +555,7 @@ def test_quarantine_retry_skips_cached_from_exhausted_source(): ) attempted = [] - def _attempt(task_id, candidates, track, batch_id): + def _attempt(task_id, candidates, track, batch_id, **kwargs): attempted.append([getattr(c, 'username', None) for c in candidates]) return True diff --git a/tests/downloads/test_orchestrator_search_mode.py b/tests/downloads/test_orchestrator_search_mode.py new file mode 100644 index 00000000..991fc486 --- /dev/null +++ b/tests/downloads/test_orchestrator_search_mode.py @@ -0,0 +1,66 @@ +"""orchestrator.search routes to the right engine method per search_mode: + +- priority (default) → engine.search_with_fallback (first source wins) +- best_quality + hybrid → engine.search_all_sources (pool every source) +- single-source mode → the single client's search (toggle is a no-op) +""" + +import asyncio + +import core.download_orchestrator as orch_mod +from core.download_orchestrator import DownloadOrchestrator + + +def _run(coro): + loop = asyncio.new_event_loop() + try: + return loop.run_until_complete(coro) + finally: + loop.close() + + +class _SpyEngine: + def __init__(self): + self.calls = [] + + async def search_with_fallback(self, query, chain, timeout=None, progress_callback=None): + self.calls.append(('fallback', tuple(chain))) + return (['fb'], []) + + async def search_all_sources(self, query, chain, timeout=None, + progress_callback=None, exclude_sources=None): + self.calls.append(('all', tuple(chain))) + return (['pool'], []) + + +def _hybrid_orch(): + orch = DownloadOrchestrator.__new__(DownloadOrchestrator) + orch.mode = 'hybrid' + orch.hybrid_order = ['soulseek', 'hifi'] + orch.hybrid_primary = 'soulseek' + orch.hybrid_secondary = 'hifi' + orch.engine = _SpyEngine() + # _resolve_source_chain normalizes names through the registry; stub it so the + # test doesn't need a full registry. + orch._resolve_source_chain = lambda: ['soulseek', 'hifi'] + return orch + + +def test_priority_mode_uses_search_with_fallback(monkeypatch): + monkeypatch.setattr(orch_mod, 'load_search_mode', lambda: 'priority') + orch = _hybrid_orch() + + tracks, _ = _run(orch.search('q')) + + assert orch.engine.calls == [('fallback', ('soulseek', 'hifi'))] + assert tracks == ['fb'] + + +def test_best_quality_mode_uses_search_all_sources(monkeypatch): + monkeypatch.setattr(orch_mod, 'load_search_mode', lambda: 'best_quality') + orch = _hybrid_orch() + + tracks, _ = _run(orch.search('q')) + + assert orch.engine.calls == [('all', ('soulseek', 'hifi'))] + assert tracks == ['pool'] diff --git a/tests/downloads/test_soulseek_aac_quality.py b/tests/downloads/test_soulseek_aac_quality.py index 1dd9fe2f..a4b609d7 100644 --- a/tests/downloads/test_soulseek_aac_quality.py +++ b/tests/downloads/test_soulseek_aac_quality.py @@ -38,14 +38,14 @@ def _cand(quality, size_mb, bitrate=None): queue_length=0, artist='A', title='Song', album='B', track_number=1) -def _q(enabled_flac=True, enabled_mp3=True, aac=None): +def _q(enabled_flac=True, enabled_mp3=True, aac=None, fallback=True): qualities = { 'flac': {'enabled': enabled_flac, 'min_kbps': 500, 'max_kbps': 10000, 'priority': 1, 'bit_depth': 'any'}, 'mp3_320': {'enabled': enabled_mp3, 'min_kbps': 280, 'max_kbps': 500, 'priority': 2}, } - if aac is not None: # None => omit the tier entirely (pre-existing profile) + if aac is not None: # None => omit the tier entirely qualities['aac'] = {'enabled': aac, 'min_kbps': 128, 'max_kbps': 400, 'priority': 1.5} - return {'preset': 'custom', 'qualities': qualities, 'fallback_enabled': True} + return {'preset': 'custom', 'qualities': qualities, 'fallback_enabled': fallback} def _filter(candidates, profile): @@ -56,51 +56,44 @@ def _filter(candidates, profile): return c.filter_results_by_quality_preference(candidates) -# ── additive proof: AAC off == today (dropped) ──────────────────────────────── -def test_aac_dropped_when_tier_absent_pre_existing_profile(): - # A profile saved before this feature has no 'aac' key at all. - out = _filter([_cand('aac', 5)], _q(aac=None)) - assert out == [] # AAC went to 'other' -> never returned, exactly as before +# ── AAC follows the UNIVERSAL rule (no per-format special-casing) ────────────── +# AAC is just another format: it passes only if it matches a ranked target; +# when nothing matches, the fallback toggle decides — exactly like every format. + +def test_aac_not_targeted_fallback_off_is_dropped(): + out = _filter([_cand('aac', 5)], _q(aac=None, fallback=False)) + assert out == [] # no AAC target + fallback off → nothing comes through -def test_aac_dropped_when_tier_present_but_disabled(): - out = _filter([_cand('aac', 5)], _q(aac=False)) - assert out == [] +def test_aac_not_targeted_fallback_on_comes_through(): + out = _filter([_cand('aac', 5)], _q(aac=None, fallback=True)) + assert len(out) == 1 and out[0].quality == 'aac' # fallback grabs it + + +def test_aac_disabled_tier_behaves_like_not_targeted(): + # An explicitly-disabled aac tier is not a target → same as absent. + assert _filter([_cand('aac', 5)], _q(aac=False, fallback=False)) == [] def test_flac_mp3_selection_unchanged_when_aac_absent(): - # The headline no-regression guard: a normal FLAC/MP3 mix is unaffected. flac, mp3 = _cand('flac', 30), _cand('mp3', 5, bitrate=320) out = _filter([mp3, flac], _q(aac=None)) - assert out and out[0].quality == 'flac' # FLAC still wins, as before + assert out and out[0].quality == 'flac' # FLAC still wins -# ── opt-in: AAC on makes it a real tier ─────────────────────────────────────── -def test_aac_selected_when_enabled(): +# ── AAC as a real ranked target ─────────────────────────────────────────────── +def test_aac_selected_when_targeted(): out = _filter([_cand('aac', 5)], _q(aac=True)) assert len(out) == 1 and out[0].quality == 'aac' -def test_flac_beats_aac_when_both_present(): +def test_flac_beats_aac_when_listed_higher(): flac, aac = _cand('flac', 30), _cand('aac', 5) out = _filter([aac, flac], _q(aac=True)) - assert out[0].quality == 'flac' # priority 1 < 1.5 + assert out[0].quality == 'flac' # flac target ranked above aac (priority 1 < 1.5) -def test_aac_beats_mp3_when_both_present(): +def test_aac_beats_mp3_when_listed_higher(): mp3, aac = _cand('mp3', 5, bitrate=320), _cand('aac', 5) out = _filter([mp3, aac], _q(aac=True)) - assert out[0].quality == 'aac' # priority 1.5 < 2 - - -def test_default_and_presets_ship_aac_disabled_above_mp3(): - from database.music_database import MusicDatabase - db = MusicDatabase.__new__(MusicDatabase) # no DB init - profiles = [db._get_default_quality_profile()] - profiles += [db.get_quality_preset(p) for p in ('audiophile', 'balanced', 'space_saver')] - for prof in profiles: - aac = prof['qualities']['aac'] - assert aac['enabled'] is False # opt-in everywhere - # above MP3: lower priority number than the best MP3 tier present - mp3_prios = [v['priority'] for k, v in prof['qualities'].items() if k.startswith('mp3')] - assert aac['priority'] < min(mp3_prios) + assert out[0].quality == 'aac' # aac target (1.5) ranked above mp3 (2) diff --git a/tests/imports/test_import_guards.py b/tests/imports/test_import_guards.py index ee71a1dd..22586d5a 100644 --- a/tests/imports/test_import_guards.py +++ b/tests/imports/test_import_guards.py @@ -1,47 +1,64 @@ +"""``check_flac_bit_depth`` is a thin legacy wrapper that delegates to the +unified ``check_quality_target`` guard, which probes the REAL file (mutagen) +and ranks it against the v3 ranked-targets list. + +The old per-quality ``bit_depth_fallback`` config and the "reject a higher bit +depth" semantics are gone by design: bit depth is now a MINIMUM, so a 24-bit +file satisfies a 16-bit target. These tests pin the wrapper's current +behaviour (deeper coverage of ``check_quality_target`` lives in +``tests/imports/test_quality_guard.py``). +""" + from types import SimpleNamespace -from core.imports import guards +import core.imports.guards as guards +import core.imports.file_ops as file_ops +from core.quality.model import AudioQuality class _FakeDB: - def __init__(self, quality_profile): - self._quality_profile = quality_profile + def __init__(self, profile): + self._profile = profile def get_quality_profile(self): - return self._quality_profile + return self._profile -def test_check_flac_bit_depth_rejects_strict_mismatch(monkeypatch): +_WANT_FLAC24 = { + "fallback_enabled": False, + "ranked_targets": [ + {"label": "FLAC 24-bit/96kHz", "format": "flac", "bit_depth": 24, "min_sample_rate": 96000}, + ], +} + + +def _patch(monkeypatch, aq, profile): + monkeypatch.setattr(file_ops, "probe_audio_quality", lambda fp: aq) + monkeypatch.setattr(guards, "MusicDatabase", lambda: _FakeDB(profile)) + + # Key-aware config stub: the import quality filter is ON (its default), so + # the guard runs; everything else (downsample, etc.) is OFF. A blanket False + # would wrongly disable the filter itself via import.quality_filter_enabled. + def _cfg_get(key, default=None): + if key == "import.quality_filter_enabled": + return True + return False + monkeypatch.setattr( - guards, - "MusicDatabase", - lambda: _FakeDB({"qualities": {"flac": {"bit_depth": "16", "bit_depth_fallback": False}}}), - ) - monkeypatch.setattr( - guards, - "_get_config_manager", - lambda: SimpleNamespace(get=lambda _key, default=None: False), - ) - - context = {"_audio_quality": "FLAC 24bit", "track_info": {"name": "Song One"}} - - assert guards.check_flac_bit_depth("/tmp/Song One.flac", context) == ( - "FLAC bit depth mismatch: file is 24-bit, preference is 16-bit" + guards, "_get_config_manager", + lambda: SimpleNamespace(get=_cfg_get), ) -def test_check_flac_bit_depth_allows_fallback_when_enabled(monkeypatch): - monkeypatch.setattr( - guards, - "MusicDatabase", - lambda: _FakeDB({"qualities": {"flac": {"bit_depth": "16", "bit_depth_fallback": True}}}), - ) - monkeypatch.setattr( - guards, - "_get_config_manager", - lambda: SimpleNamespace(get=lambda _key, default=None: False), - ) +def test_check_flac_bit_depth_rejects_below_target(monkeypatch): + # 16-bit file, target wants 24-bit, fallback off → rejected. + _patch(monkeypatch, AudioQuality("flac", sample_rate=44100, bit_depth=16), _WANT_FLAC24) + reason = guards.check_flac_bit_depth("/tmp/Song One.flac", {}) + assert reason is not None + assert "FLAC 24-bit/96kHz" in reason - context = {"_audio_quality": "FLAC 24bit", "track_info": {"name": "Song One"}} - assert guards.check_flac_bit_depth("/tmp/Song One.flac", context) is None +def test_check_flac_bit_depth_accepts_when_meeting_target(monkeypatch): + # 24-bit/96k file meets the 24-bit target → accepted. + _patch(monkeypatch, AudioQuality("flac", sample_rate=96000, bit_depth=24), _WANT_FLAC24) + assert guards.check_flac_bit_depth("/tmp/Song One.flac", {}) is None diff --git a/tests/imports/test_import_pipeline.py b/tests/imports/test_import_pipeline.py index a8b550aa..06bffb6d 100644 --- a/tests/imports/test_import_pipeline.py +++ b/tests/imports/test_import_pipeline.py @@ -235,6 +235,48 @@ def test_post_process_matched_download_forwards_separate_metadata_runtime(tmp_pa # write to the task directly — it stashes on context and the wrapper applies it) # --------------------------------------------------------------------------- +def test_quality_gate_runs_before_acoustid(tmp_path, monkeypatch): + """The quality check must run BEFORE AcoustID: a wrong-quality file is + quarantined with trigger='quality' and AcoustID is never fingerprinted (so + quality is known on every quarantine entry, and no wasted AcoustID call).""" + src = tmp_path / "source.flac" + src.write_bytes(b"fLaC") + + # Reach the quality gate: bypass integrity + silence guards. + from core.imports.file_integrity import IntegrityResult + monkeypatch.setattr(import_pipeline, "check_audio_integrity", + lambda *_a, **_kw: IntegrityResult(ok=True, checks={})) + monkeypatch.setattr(import_pipeline, "detect_broken_audio", lambda *_a, **_kw: None) + + # Wrong quality → rejection. + monkeypatch.setattr(import_pipeline, "get_audio_quality_string", lambda fp: "FLAC 16bit/44.1kHz") + monkeypatch.setattr(import_pipeline, "check_quality_target", lambda fp, ctx: "Quality mismatch: FLAC 16bit") + + triggers = [] + monkeypatch.setattr(import_pipeline, "move_to_quarantine", + lambda fp, ctx, reason, eng, trigger=None: triggers.append(trigger) or "/q/x.flac.quarantined") + monkeypatch.setattr(import_pipeline, "_mark_task_quarantined", lambda *a, **k: None) + monkeypatch.setattr(import_pipeline, "_requeue_quarantined_task_for_retry", lambda *a, **k: False) + + # Spy: AcoustID must NOT be constructed when quality already rejected. + acoustid_constructed = [] + fake_mod = types.SimpleNamespace( + AcoustIDVerification=lambda *a, **k: acoustid_constructed.append(True), + VerificationResult=types.SimpleNamespace(FAIL="fail"), + ) + monkeypatch.setitem(sys.modules, "core.acoustid_verification", fake_mod) + + runtime = types.SimpleNamespace(automation_engine=None, on_download_completed=None, + web_scan_manager=None, repair_worker=None) + context = {"track_info": {}, "task_id": None, "batch_id": None} + + import_pipeline.post_process_matched_download("ctx", context, str(src), runtime) + + assert triggers == ["quality"] # quarantined for quality + assert acoustid_constructed == [] # AcoustID never ran + assert context.get("_audio_quality") == "FLAC 16bit/44.1kHz" # recorded for the sidecar + + def test_mark_task_quarantined_stashes_entry_id_when_task_id_absent(): ctx = {} # wrapper popped task_id before the inner pipeline ran import_pipeline._mark_task_quarantined(ctx, "/q/20260514_120000_song.flac.quarantined") diff --git a/tests/imports/test_import_rejection_reason.py b/tests/imports/test_import_rejection_reason.py index 9377ff6a..4d630cde 100644 --- a/tests/imports/test_import_rejection_reason.py +++ b/tests/imports/test_import_rejection_reason.py @@ -48,9 +48,11 @@ def test_acoustid_quarantine_without_message_still_flags(): def test_bitdepth_rejection_detected(): + # The _bitdepth_rejected flag now signals any quality-target rejection + # (bit depth, sample rate, format, bitrate) — the unified quality guard. reason = import_rejection_reason({'_bitdepth_rejected': True}) assert reason is not None - assert 'bit-depth' in reason.lower() + assert 'quality filter' in reason.lower() def test_race_guard_failure_detected(): diff --git a/tests/imports/test_quality_guard.py b/tests/imports/test_quality_guard.py new file mode 100644 index 00000000..12ac06db --- /dev/null +++ b/tests/imports/test_quality_guard.py @@ -0,0 +1,127 @@ +"""Quality guard + quarantine isolation. + +Locks two invariants the global-quality work depends on: + +1. ``check_quality_target`` rejects with a 'what the file IS vs what was + WANTED' reason (surfaced in the track-detail modal), and accepts when a + target is met or fallback is on. +2. A quality mismatch is isolated from the AcoustID/force_import path: it + uses ``trigger='quality'`` and the ``'quality'`` bypass flag, which must + NOT bypass the AcoustID check and vice-versa. force_imported stays + reserved for AcoustID version-mismatch acceptance. +""" + +import json +import types + +import pytest + +import core.imports.guards as guards +import core.imports.file_ops as file_ops +from core.imports.pipeline import _should_skip_quarantine_check +from core.quality.model import AudioQuality + + +class _FakeDB: + def __init__(self, profile): + self._p = profile + + def get_quality_profile(self): + return self._p + + +def _patch_guard(monkeypatch, probe_aq, profile, downsample=False, quality_filter=True): + monkeypatch.setattr(file_ops, 'probe_audio_quality', lambda fp: probe_aq) + monkeypatch.setattr(guards, 'MusicDatabase', lambda: _FakeDB(profile)) + + def _cfg_get(k, d=None): + if 'downsample' in k: + return downsample + if k == 'import.quality_filter_enabled': + return quality_filter + return d + + monkeypatch.setattr( + guards, '_get_config_manager', + lambda: types.SimpleNamespace(get=_cfg_get), + ) + + +_WANT_FLAC24 = { + 'fallback_enabled': False, + 'ranked_targets': [ + {'label': 'FLAC 24-bit/96kHz', 'format': 'flac', 'bit_depth': 24, 'min_sample_rate': 96000}, + ], +} +_WANT_FLAC24_FALLBACK = {**_WANT_FLAC24, 'fallback_enabled': True} + + +# ── check_quality_target ─────────────────────────────────────────────────── + +def test_rejects_with_wanted_vs_got_reason(monkeypatch): + _patch_guard(monkeypatch, AudioQuality('flac', sample_rate=44100, bit_depth=16), _WANT_FLAC24) + reason = guards.check_quality_target('/x/song.flac', {}) + assert reason is not None + assert 'FLAC 16-bit' in reason # what the file IS + assert 'FLAC 24-bit/96kHz' in reason # what was WANTED + + +def test_accepts_when_target_met(monkeypatch): + _patch_guard(monkeypatch, AudioQuality('flac', sample_rate=96000, bit_depth=24), _WANT_FLAC24) + assert guards.check_quality_target('/x/song.flac', {}) is None + + +def test_accepts_via_fallback(monkeypatch): + _patch_guard(monkeypatch, AudioQuality('flac', sample_rate=44100, bit_depth=16), _WANT_FLAC24_FALLBACK) + assert guards.check_quality_target('/x/song.flac', {}) is None + + +def test_master_toggle_off_skips_filter(monkeypatch): + # import.quality_filter_enabled = False → a below-target file is accepted + # (imported) regardless of quality, even with fallback off. + _patch_guard( + monkeypatch, AudioQuality('flac', sample_rate=44100, bit_depth=16), + _WANT_FLAC24, quality_filter=False, + ) + assert guards.check_quality_target('/x/song.flac', {}) is None + + +def test_skips_when_unprobeable(monkeypatch): + _patch_guard(monkeypatch, None, _WANT_FLAC24) + assert guards.check_quality_target('/x/song.flac', {}) is None + + +# ── force_import isolation ───────────────────────────────────────────────── + +def test_quality_bypass_does_not_skip_acoustid(): + ctx = {'_skip_quarantine_check': 'quality'} + assert _should_skip_quarantine_check(ctx, 'quality') is True + assert _should_skip_quarantine_check(ctx, 'acoustid') is False + + +def test_acoustid_bypass_does_not_skip_quality(): + ctx = {'_skip_quarantine_check': 'acoustid'} + assert _should_skip_quarantine_check(ctx, 'acoustid') is True + assert _should_skip_quarantine_check(ctx, 'quality') is False + + +def test_quality_quarantine_persists_quality_trigger(monkeypatch, tmp_path): + # A quality reject writes trigger='quality' (not 'acoustid') into the + # sidecar, so Approve never routes it through the force_import path. + monkeypatch.setattr( + guards, '_get_config_manager', + lambda: types.SimpleNamespace(get=lambda k, d=None: str(tmp_path) if 'download_path' in k else d), + ) + src = tmp_path / 'song.flac' + src.write_bytes(b'FLACfake') + qpath = guards.move_to_quarantine( + str(src), {}, 'Quality mismatch: file is FLAC 16-bit, wanted FLAC 24-bit/96kHz', + automation_engine=None, trigger='quality', + ) + sidecars = list((tmp_path / 'ss_quarantine').glob('*.json')) + assert len(sidecars) == 1 + meta = json.loads(sidecars[0].read_text(encoding='utf-8')) + assert meta['trigger'] == 'quality' + assert meta['trigger'] != 'acoustid' + assert 'wanted FLAC 24-bit/96kHz' in meta['quarantine_reason'] + assert qpath.endswith('.quarantined') diff --git a/tests/imports/test_quarantine_management.py b/tests/imports/test_quarantine_management.py index 7234c497..86864f74 100644 --- a/tests/imports/test_quarantine_management.py +++ b/tests/imports/test_quarantine_management.py @@ -470,9 +470,14 @@ def test_group_key_prefers_isrc_over_everything(): assert quarantine_group_key("Artist", "Track", ctx) == "isrc:usrc12345678" -def test_group_key_falls_back_to_source_id_then_uri(): - assert quarantine_group_key("A", "T", {"track_info": {"id": "abc123"}}) == "id:abc123" - assert quarantine_group_key("A", "T", {"track_info": {"uri": "spotify:track:z"}}) == "uri:spotify:track:z" +def test_group_key_uses_isrc_and_ignores_source_specific_ids(): + # ISRC is the universal target identity → wins. + assert quarantine_group_key("A", "T", {"track_info": {"isrc": "USABC1234567"}}) == "isrc:usabc1234567" + # Source-specific ids / uris are intentionally NOT used (they differ across + # sources/batches and break cross-batch sibling matching) — with no ISRC the + # key falls back to the normalized artist|track name, NOT the id/uri. + assert quarantine_group_key("A", "T", {"track_info": {"id": "abc123"}}) == "nm:a|t" + assert quarantine_group_key("A", "T", {"track_info": {"uri": "spotify:track:z"}}) == "nm:a|t" def test_group_key_falls_back_to_normalized_name_without_context(): diff --git a/tests/imports/test_silence_guard.py b/tests/imports/test_silence_guard.py new file mode 100644 index 00000000..fe0d9235 --- /dev/null +++ b/tests/imports/test_silence_guard.py @@ -0,0 +1,104 @@ +"""Audio-completeness guard — catches files whose container duration is +correct but whose real audio is far shorter (HiFi/Monochrome truncated files: +container claims 3:08 but only ~30s actually decodes) or mostly silence. Pure +parsers are tested here; the ffmpeg call is integration. +""" + +import pytest + +from core.imports.silence import ( + silence_ratio_from_output, + is_mostly_silent_reason, + measured_duration_from_astats, + incomplete_audio_reason, +) + + +_ONE_LONG_TAIL = """ +Input #0, flac, from 'song.flac': +[silencedetect @ 0x55] silence_start: 31.512 +[silencedetect @ 0x55] silence_end: 210.300 | silence_duration: 178.788 +""" + +_TWO_GAPS = """ +[silencedetect @ 0x55] silence_start: 0 +[silencedetect @ 0x55] silence_end: 1.5 | silence_duration: 1.5 +[silencedetect @ 0x55] silence_start: 200 +[silencedetect @ 0x55] silence_end: 203 | silence_duration: 3.0 +""" + +_NO_SILENCE = "Input #0, flac\n[some other ffmpeg chatter]\n" + + +def test_ratio_single_long_trailing_silence(): + r = silence_ratio_from_output(_ONE_LONG_TAIL, total_duration_s=210.3) + assert r == pytest.approx(178.788 / 210.3, rel=1e-3) + assert r > 0.8 + + +def test_ratio_sums_multiple_silences(): + r = silence_ratio_from_output(_TWO_GAPS, total_duration_s=210.0) + assert r == pytest.approx(4.5 / 210.0, rel=1e-3) + + +def test_ratio_no_silence_is_zero(): + assert silence_ratio_from_output(_NO_SILENCE, total_duration_s=210.0) == 0.0 + + +def test_ratio_zero_duration_is_zero(): + assert silence_ratio_from_output(_ONE_LONG_TAIL, total_duration_s=0) == 0.0 + + +def test_ratio_capped_at_one(): + # Defensive: bogus silence longer than the track can't exceed 1.0. + out = "[silencedetect @ 0x] silence_end: 999 | silence_duration: 999\n" + assert silence_ratio_from_output(out, total_duration_s=210.0) == 1.0 + + +def test_reason_when_mostly_silent(): + reason = is_mostly_silent_reason(_ONE_LONG_TAIL, total_duration_s=210.3, threshold=0.5) + assert reason is not None + assert "silent" in reason.lower() + + +def test_no_reason_for_normal_song(): + assert is_mostly_silent_reason(_TWO_GAPS, total_duration_s=210.0, threshold=0.5) is None + + +# ── truncation: real decoded duration vs container duration ──────────────── + +_ASTATS_TRUNCATED = """ +[Parsed_astats_0 @ 0x55] Number of samples: 1318912 +[Parsed_astats_0 @ 0x55] Number of NaNs: 0 +""" + +_ASTATS_FULL = "[Parsed_astats_0 @ 0x55] Number of samples: 9261000\n" + + +def test_measured_duration_from_samples(): + # 1318912 samples / 44100 Hz ≈ 29.9s (the real Blossom file) + assert measured_duration_from_astats(_ASTATS_TRUNCATED, 44100) == pytest.approx(29.9, abs=0.1) + + +def test_measured_duration_none_without_samples(): + assert measured_duration_from_astats("no stats here", 44100) is None + + +def test_measured_duration_none_without_sample_rate(): + assert measured_duration_from_astats(_ASTATS_TRUNCATED, 0) is None + + +def test_incomplete_reason_for_truncated_file(): + # 30s of real audio in a 188s container → truncated. + reason = incomplete_audio_reason(29.9, 188.4, min_ratio=0.85) + assert reason is not None + assert "30s" in reason and "188s" in reason + + +def test_no_incomplete_reason_for_full_file(): + assert incomplete_audio_reason(187.5, 188.4, min_ratio=0.85) is None + + +def test_no_incomplete_reason_when_unmeasurable(): + assert incomplete_audio_reason(None, 188.4, min_ratio=0.85) is None + assert incomplete_audio_reason(30.0, 0, min_ratio=0.85) is None diff --git a/tests/library/test_path_resolver.py b/tests/library/test_path_resolver.py index 713fc8ae..de77b738 100644 --- a/tests/library/test_path_resolver.py +++ b/tests/library/test_path_resolver.py @@ -74,6 +74,25 @@ def test_finds_file_via_transfer_folder_suffix_walk(tmp_path: Path) -> None: assert result == str(actual) +def test_finds_file_via_relative_db_path_full_suffix(tmp_path: Path) -> None: + """SoulSync's own library scanner stores RELATIVE paths with NO leading + slash, e.g. "Artist/Album/track.flac". Index 0 is the artist folder, so the + resolver must try the FULL path first — the old range(1, ...) dropped the + artist segment and every such track came back unresolved (quality scanner: + "could not be probed"). Regression guard for that fix.""" + transfer = tmp_path / "Transfer" + (transfer / "Asketa" / "Another Side").mkdir(parents=True) + actual = transfer / "Asketa" / "Another Side" / "01 - Another Side.flac" + actual.write_bytes(b"audio") + + result = resolve_library_file_path( + "Asketa/Another Side/01 - Another Side.flac", + transfer_folder=str(transfer), + ) + + assert result == str(actual) + + def test_finds_file_via_download_folder_when_transfer_misses(tmp_path: Path) -> None: transfer = tmp_path / "Transfer" transfer.mkdir() diff --git a/tests/quality/test_engine_fallback.py b/tests/quality/test_engine_fallback.py new file mode 100644 index 00000000..9e8d2128 --- /dev/null +++ b/tests/quality/test_engine_fallback.py @@ -0,0 +1,127 @@ +"""Engine ``search_with_fallback`` is the PRIORITY-mode search path and is +deliberately quality-agnostic: the first source in the chain that returns any +tracks wins (source order is king), byte-for-byte like the pre-quality-system +hybrid loop (#896 review #3). It skips unconfigured/unavailable sources, +swallows per-source exceptions, and returns RAW tracks (match-filtering + +final quality ranking happen later). Cross-source quality pooling lives in +best_quality mode (``search_all_sources``), not here. +""" + +import asyncio + +import pytest + +from core.download_engine.engine import DownloadEngine +from core.quality.model import AudioQuality + + +class _Cand: + def __init__(self, aq, name): + self.audio_quality = aq + self.name = name + + +class _FakePlugin: + def __init__(self, tracks, configured=True, raises=False): + self._tracks = tracks + self._configured = configured + self._raises = raises + self.searched = False + + def is_configured(self): + return self._configured + + async def search(self, query, timeout=None, progress_callback=None): + self.searched = True + if self._raises: + raise RuntimeError("boom") + return (self._tracks, []) + + +FLAC = AudioQuality('flac', sample_rate=44100, bit_depth=16) +MP3 = AudioQuality('mp3', bitrate=320) +MP3_NO_BITRATE = AudioQuality('mp3') # slskd often omits bitrate + + +def _engine_with(plugins): + eng = object.__new__(DownloadEngine) + eng._plugins = plugins + return eng + + +def test_returns_first_source_with_tracks_source_priority_king(): + first = _FakePlugin([_Cand(MP3, 'a-mp3')]) + second = _FakePlugin([_Cand(FLAC, 'b-flac')]) + eng = _engine_with({'first': first, 'second': second}) + + tracks, _ = asyncio.run(eng.search_with_fallback('q', ['first', 'second'])) + + assert [t.name for t in tracks] == ['a-mp3'] # first source wins regardless of quality + assert second.searched is False # never queried — priority is king + + +def test_metadata_less_mp3_still_wins_in_priority_mode(): + """An mp3 whose bitrate slskd omitted must NOT be deprioritised in priority + mode — the whole point of #896 review #3.""" + first = _FakePlugin([_Cand(MP3_NO_BITRATE, 'a-mp3')]) + second = _FakePlugin([_Cand(FLAC, 'b-flac')]) + eng = _engine_with({'first': first, 'second': second}) + + tracks, _ = asyncio.run(eng.search_with_fallback('q', ['first', 'second'])) + + assert [t.name for t in tracks] == ['a-mp3'] + assert second.searched is False + + +def test_skips_to_next_source_when_first_empty(): + first = _FakePlugin([]) + second = _FakePlugin([_Cand(FLAC, 'b-flac')]) + eng = _engine_with({'first': first, 'second': second}) + + tracks, _ = asyncio.run(eng.search_with_fallback('q', ['first', 'second'])) + + assert [t.name for t in tracks] == ['b-flac'] + assert second.searched is True + + +def test_returns_raw_tracks_not_pruned(): + # All of the winning source's tracks come back so the orchestrator's match + # filter can pick the correct one. + first = _FakePlugin([_Cand(MP3, 'one'), _Cand(FLAC, 'two')]) + eng = _engine_with({'first': first}) + + tracks, _ = asyncio.run(eng.search_with_fallback('q', ['first'])) + + assert {t.name for t in tracks} == {'one', 'two'} + + +def test_skips_unconfigured_source(): + first = _FakePlugin([_Cand(FLAC, 'a')], configured=False) + second = _FakePlugin([_Cand(MP3, 'b')]) + eng = _engine_with({'first': first, 'second': second}) + + tracks, _ = asyncio.run(eng.search_with_fallback('q', ['first', 'second'])) + + assert [t.name for t in tracks] == ['b'] + assert first.searched is False + + +def test_swallows_per_source_exception_and_continues(): + first = _FakePlugin([], raises=True) + second = _FakePlugin([_Cand(MP3, 'b')]) + eng = _engine_with({'first': first, 'second': second}) + + tracks, _ = asyncio.run(eng.search_with_fallback('q', ['first', 'second'])) + + assert [t.name for t in tracks] == ['b'] + + +def test_returns_empty_when_all_sources_empty(): + first = _FakePlugin([]) + second = _FakePlugin([]) + eng = _engine_with({'first': first, 'second': second}) + + tracks, albums = asyncio.run(eng.search_with_fallback('q', ['first', 'second'])) + + assert tracks == [] + assert albums == [] diff --git a/tests/quality/test_model.py b/tests/quality/test_model.py new file mode 100644 index 00000000..a740f22c --- /dev/null +++ b/tests/quality/test_model.py @@ -0,0 +1,130 @@ +"""AudioQuality.matches_target + v2->v3 migration. + +Locks the bitrate-as-threshold behaviour: lossy formats match on a MINIMUM +bitrate (>=, a range), and lossless matches on bit depth + sample rate — NOT +on exact bitrate, so a FLAC's wildly-varying bitrate (stereo vs mono, FLAC +compression) never falsely rejects it. +""" + +import pytest + +from core.quality.model import ( + AudioQuality, + QualityTarget, + v2_qualities_to_ranked_targets, +) + + +# ── lossy: bitrate is a minimum threshold (a range), never exact ─────────── + +def test_mp3_meets_minimum_bitrate(): + t = QualityTarget(format='mp3', min_bitrate=320) + assert AudioQuality('mp3', bitrate=320).matches_target(t) is True + assert AudioQuality('mp3', bitrate=400).matches_target(t) is True # above floor ok + + +def test_mp3_below_minimum_bitrate_rejected(): + t = QualityTarget(format='mp3', min_bitrate=320) + assert AudioQuality('mp3', bitrate=300).matches_target(t) is False + + +def test_mp3_matches_lower_threshold(): + t = QualityTarget(format='mp3', min_bitrate=192) + assert AudioQuality('mp3', bitrate=320).matches_target(t) is True + + +# ── lossless: matched on bit depth + sample rate, NOT exact bitrate ──────── + +def test_flac_matches_on_depth_and_rate_regardless_of_bitrate(): + t = QualityTarget(format='flac', bit_depth=24, min_sample_rate=96000) + # An unusual/low bitrate (e.g. a mono or highly-compressed FLAC) must + # still match when bit depth + sample rate satisfy the target. + weird = AudioQuality('flac', bitrate=300, sample_rate=96000, bit_depth=24) + assert weird.matches_target(t) is True + + +def test_flac_below_target_sample_rate_rejected(): + t = QualityTarget(format='flac', bit_depth=24, min_sample_rate=96000) + assert AudioQuality('flac', sample_rate=44100, bit_depth=24).matches_target(t) is False + + +def test_flac_below_target_bit_depth_rejected(): + t = QualityTarget(format='flac', bit_depth=24) + assert AudioQuality('flac', sample_rate=96000, bit_depth=16).matches_target(t) is False + + +def test_format_mismatch_rejected(): + t = QualityTarget(format='flac', bit_depth=16) + assert AudioQuality('mp3', bitrate=320).matches_target(t) is False + + +# ── metadata-less FLAC must not over-claim a hi-res target (#896 review #4) ─ + +def test_metadata_less_flac_does_not_overclaim_hires_target(): + """A FLAC with no sample_rate/bit_depth metadata (common on slskd) must NOT + satisfy a strict hi-res target — otherwise it outranks and discards a real + 16/44 FLAC. Unknown spec fails the high tier, falling to a plain flac bucket.""" + hires = QualityTarget(format='flac', bit_depth=24, min_sample_rate=192000) + assert AudioQuality('flac').matches_target(hires) is False + + +def test_metadata_less_flac_does_not_overclaim_bit_depth_only_hires(): + """Same guard when the hi-res target constrains only bit depth.""" + hires = QualityTarget(format='flac', bit_depth=24) + assert AudioQuality('flac').matches_target(hires) is False + + +def test_metadata_less_flac_matches_plain_flac_target(): + """A bare FLAC still matches a plain 'flac (any)' target (the baseline).""" + assert AudioQuality('flac').matches_target(QualityTarget(format='flac')) is True + + +def test_metadata_less_flac_matches_16bit_baseline_target(): + """A bare FLAC satisfies the 16-bit baseline (any FLAC is >= CD quality).""" + assert AudioQuality('flac').matches_target(QualityTarget(format='flac', bit_depth=16)) is True + + +# ── ranked targets work for EVERY format, not just flac/mp3 (universal) ──── + +def test_opus_target_matches_opus_candidate(): + t = QualityTarget(format='opus', min_bitrate=128) + assert AudioQuality('opus', bitrate=160).matches_target(t) is True + assert AudioQuality('opus', bitrate=96).matches_target(t) is False + assert AudioQuality('mp3', bitrate=320).matches_target(t) is False # wrong format + + +def test_wav_target_matches_on_bit_depth_and_sample_rate(): + t = QualityTarget(format='wav', bit_depth=24, min_sample_rate=96000) + assert AudioQuality('wav', sample_rate=96000, bit_depth=24).matches_target(t) is True + assert AudioQuality('wav', sample_rate=44100, bit_depth=16).matches_target(t) is False + + +def test_wma_and_alac_targets_match_their_formats(): + assert AudioQuality('wma', bitrate=192).matches_target(QualityTarget(format='wma', min_bitrate=128)) is True + assert AudioQuality('alac', sample_rate=96000, bit_depth=24).matches_target( + QualityTarget(format='alac', bit_depth=24)) is True + + +def test_only_listed_format_passes_others_rank_last(): + """An Opus-only target list: only opus matches; everything else ranks last + (index == len(targets)), so with fallback off the caller drops them.""" + from core.quality.model import rank_candidate + targets = [QualityTarget(format='opus')] + assert rank_candidate(AudioQuality('opus', bitrate=160), targets)[0] == 0 + assert rank_candidate(AudioQuality('flac', sample_rate=96000, bit_depth=24), targets)[0] == 1 + assert rank_candidate(AudioQuality('mp3', bitrate=320), targets)[0] == 1 + + +# ── v2 -> v3 migration preserves the user's priority order ───────────────── + +def test_v2_to_v3_preserves_order_and_maps_fields(): + qualities = { + 'flac': {'enabled': True, 'priority': 1, 'bit_depth': '24'}, + 'mp3_320': {'enabled': True, 'priority': 2}, + 'mp3_192': {'enabled': False, 'priority': 3}, # disabled → dropped + } + targets = v2_qualities_to_ranked_targets(qualities) + formats = [t['format'] for t in targets] + assert formats == ['flac', 'mp3'] # disabled mp3_192 omitted + assert targets[0]['bit_depth'] == 24 + assert targets[1]['min_bitrate'] == 320 diff --git a/tests/quality/test_quality_meets_profile.py b/tests/quality/test_quality_meets_profile.py new file mode 100644 index 00000000..ad8965ae --- /dev/null +++ b/tests/quality/test_quality_meets_profile.py @@ -0,0 +1,60 @@ +"""quality_meets_profile / targets_from_profile — the shared strict +quality-decision core used by BOTH the import quality guard and the library +quality scanner. A file "meets" the profile iff its real measured quality +satisfies at least one ranked target (bit depth + sample rate are minimums; +fallback is NOT consulted — that's a download-time concession, not a definition +of quality). +""" + +from core.quality.model import AudioQuality, QualityTarget +from core.quality.selection import quality_meets_profile, targets_from_profile + + +FLAC_HI = AudioQuality('flac', sample_rate=96000, bit_depth=24) +FLAC_CD = AudioQuality('flac', sample_rate=44100, bit_depth=16) +MP3 = AudioQuality('mp3', bitrate=320) + +WANT_24BIT = [ + QualityTarget(label='FLAC 24/96', format='flac', bit_depth=24, min_sample_rate=96000), + QualityTarget(label='FLAC 24/44.1', format='flac', bit_depth=24, min_sample_rate=44100), +] + + +def test_24bit_meets_24bit_target(): + assert quality_meets_profile(FLAC_HI, WANT_24BIT) is True + + +def test_16bit_does_not_meet_24bit_target(): + assert quality_meets_profile(FLAC_CD, WANT_24BIT) is False + + +def test_mp3_does_not_meet_flac_target(): + assert quality_meets_profile(MP3, WANT_24BIT) is False + + +def test_no_targets_means_no_constraint(): + assert quality_meets_profile(FLAC_CD, []) is True + + +def test_unprobeable_file_is_not_flagged(): + # aq=None (probe failed) → don't act (avoid false re-downloads). + assert quality_meets_profile(None, WANT_24BIT) is True + + +def test_targets_from_profile_reads_v3_ranked_targets(): + profile = { + 'version': 3, + 'fallback_enabled': False, + 'ranked_targets': [ + {'label': 'FLAC 24/96', 'format': 'flac', 'bit_depth': 24, 'min_sample_rate': 96000}, + ], + } + targets, fallback = targets_from_profile(profile) + assert [t.label for t in targets] == ['FLAC 24/96'] + assert fallback is False + + +def test_targets_from_profile_migrates_v2_qualities(): + profile = {'qualities': {'flac': {'enabled': True, 'priority': 1}}} + targets, _ = targets_from_profile(profile) + assert any(t.format == 'flac' for t in targets) diff --git a/tests/quality/test_quality_presets.py b/tests/quality/test_quality_presets.py new file mode 100644 index 00000000..db148a96 --- /dev/null +++ b/tests/quality/test_quality_presets.py @@ -0,0 +1,81 @@ +"""Quality presets — the built-in ranked-target ladders behind the preset +buttons. `audiophile` must be STRICT hi-res (no 16-bit, no lossy, fallback off) +so a user who wants "24-bit only" gets it in one click; `balanced` keeps the +fuller ladder (16-bit + MP3) with fallback on. +""" + +from database.music_database import MusicDatabase + + +def _preset(name): + # _factory_quality_preset is pure (no self/DB) — call unbound to avoid setup. + return MusicDatabase._factory_quality_preset(None, name) + + +def _labels(profile): + return [t['label'] for t in profile['ranked_targets']] + + +def test_audiophile_is_strict_24bit_only(): + p = _preset('audiophile') + assert p['fallback_enabled'] is False + labels = _labels(p) + assert all('24-bit' in l for l in labels) # only 24-bit FLAC + assert 'FLAC 16-bit' not in labels + assert not any('MP3' in l for l in labels) + + +def test_balanced_still_includes_16bit_and_mp3(): + p = _preset('balanced') + labels = _labels(p) + assert 'FLAC 16-bit' in labels + assert any('MP3' in l for l in labels) + assert p['fallback_enabled'] is True + + +# ── v2 → v3 migration seeds Hi-Res from the old per-source dropdowns (#896 #5) ── + +class _FakeCfg: + def __init__(self, values): + self._v = values + + def get(self, key, default=None): + return self._v.get(key, default) + + +_V2_LOSSLESS = { + 'version': 2, 'preset': 'balanced', + 'qualities': { + 'flac': {'enabled': True, 'priority': 1, 'bit_depth': 'any'}, + 'mp3_320': {'enabled': True, 'priority': 2}, + }, +} + + +def _migrate(profile, cfg_values, monkeypatch): + monkeypatch.setattr('config.settings.config_manager', _FakeCfg(cfg_values), raising=False) + db = MusicDatabase.__new__(MusicDatabase) + return db._migrate_v2_to_v3(profile) + + +def test_v2_to_v3_seeds_hires_when_a_source_was_hires(monkeypatch): + """A user who had Tidal on Hi-Res keeps it: the migrated profile gains 24-bit + targets at the top so quality_tier_for_source resolves to 'hires', not a + silent drop to lossless.""" + out = _migrate(dict(_V2_LOSSLESS), {'tidal_download.quality': 'hires'}, monkeypatch) + top = out['ranked_targets'][0] + assert top['format'] == 'flac' and top['bit_depth'] == 24 + # the user's existing lossy fallback is preserved below the seeded ladder + assert any(t.get('format') == 'mp3' for t in out['ranked_targets']) + + +def test_v2_to_v3_no_seed_without_hires_preference(monkeypatch): + out = _migrate(dict(_V2_LOSSLESS), {'tidal_download.quality': 'lossless'}, monkeypatch) + assert not any(t.get('bit_depth') == 24 for t in out['ranked_targets']) + + +def test_v2_to_v3_no_duplicate_when_profile_already_24bit(monkeypatch): + v2 = dict(_V2_LOSSLESS) + v2['qualities'] = {'flac': {'enabled': True, 'priority': 1, 'bit_depth': '24'}} + out = _migrate(v2, {'qobuz.quality': 'hires_max'}, monkeypatch) + assert sum(1 for t in out['ranked_targets'] if t.get('bit_depth') == 24) == 1 diff --git a/tests/quality/test_search_all_sources.py b/tests/quality/test_search_all_sources.py new file mode 100644 index 00000000..cf1f24c4 --- /dev/null +++ b/tests/quality/test_search_all_sources.py @@ -0,0 +1,113 @@ +"""Engine.search_all_sources pools candidates from EVERY configured source +(best-quality mode), instead of stopping at the first satisfying one like +search_with_fallback. Ranking happens later in the orchestrator — this just +combines raw tracks. Excluded/exhausted and raising sources are skipped. +""" + +import asyncio + + +class _Cand: + def __init__(self, name): + self.name = name + + +class _FakePlugin: + def __init__(self, tracks, configured=True, raises=False): + self._tracks = tracks + self._configured = configured + self._raises = raises + self.searched = False + + def is_configured(self): + return self._configured + + async def search(self, query, timeout=None, progress_callback=None): + self.searched = True + if self._raises: + raise RuntimeError("boom") + return (list(self._tracks), []) + + +def _engine_with(plugins): + from core.download_engine.engine import DownloadEngine + eng = object.__new__(DownloadEngine) + eng._plugins = plugins + return eng + + +def test_pools_candidates_from_all_sources(): + a = _FakePlugin([_Cand('a1'), _Cand('a2')]) + b = _FakePlugin([_Cand('b1')]) + eng = _engine_with({'a': a, 'b': b}) + + tracks, albums = asyncio.run(eng.search_all_sources('q', ['a', 'b'])) + + assert {t.name for t in tracks} == {'a1', 'a2', 'b1'} + assert a.searched and b.searched + assert albums == [] + + +def test_skips_excluded_sources(): + a = _FakePlugin([_Cand('a1')]) + b = _FakePlugin([_Cand('b1')]) + eng = _engine_with({'a': a, 'b': b}) + + tracks, _ = asyncio.run( + eng.search_all_sources('q', ['a', 'b'], exclude_sources=['b']) + ) + + assert {t.name for t in tracks} == {'a1'} + assert b.searched is False + + +def test_skips_unconfigured_and_swallows_raising_source(): + a = _FakePlugin([_Cand('a1')], configured=False) + b = _FakePlugin([_Cand('b1')], raises=True) + c = _FakePlugin([_Cand('c1')]) + eng = _engine_with({'a': a, 'b': b, 'c': c}) + + tracks, _ = asyncio.run(eng.search_all_sources('q', ['a', 'b', 'c'])) + + assert {t.name for t in tracks} == {'c1'} # a skipped, b raised, c survives + + +def test_searches_run_concurrently_and_wait_for_slow_source(): + import time + + class _SlowPlugin: + def __init__(self, name, delay): + self._name = name + self._delay = delay + self.searched = False + + def is_configured(self): + return True + + async def search(self, query, timeout=None, progress_callback=None): + self.searched = True + await asyncio.sleep(self._delay) + return ([_Cand(self._name)], []) + + slow = _SlowPlugin('usenet', 0.20) # a slow release-level source + fast = _SlowPlugin('hifi', 0.05) + eng = _engine_with({'usenet': slow, 'hifi': fast}) + + start = time.monotonic() + tracks, _ = asyncio.run(eng.search_all_sources('q', ['usenet', 'hifi'])) + elapsed = time.monotonic() - start + + # Both included — the pool waits for the slow source. + assert {t.name for t in tracks} == {'usenet', 'hifi'} + # Concurrent: total ≈ max(0.20, 0.05), well under the sequential sum 0.25. + assert elapsed < 0.20 + 0.04 + + +def test_empty_when_all_sources_empty(): + a = _FakePlugin([]) + eng = _engine_with({'a': a}) + + tracks, albums = asyncio.run(eng.search_all_sources('q', ['a'])) + + assert tracks == [] + assert albums == [] diff --git a/tests/quality/test_search_mode.py b/tests/quality/test_search_mode.py new file mode 100644 index 00000000..1f8eb3f0 --- /dev/null +++ b/tests/quality/test_search_mode.py @@ -0,0 +1,35 @@ +"""core.quality.selection.load_search_mode — reads the download search +strategy from the quality profile. + +'priority' → today's behaviour: first satisfying source wins. +'best_quality' → pool all sources, work best→worst by actual quality. + +Default and any unknown value resolve to 'priority' so every existing +install keeps its current behaviour. +""" + +import core.quality.selection as selection +import database.music_database as music_database + + +def _patch_profile(monkeypatch, profile): + class _FakeDB: + def get_quality_profile(self): + return profile + + monkeypatch.setattr(music_database, "MusicDatabase", _FakeDB) + + +def test_defaults_to_priority_when_key_absent(monkeypatch): + _patch_profile(monkeypatch, {"version": 3, "ranked_targets": []}) + assert selection.load_search_mode() == "priority" + + +def test_returns_best_quality_when_set(monkeypatch): + _patch_profile(monkeypatch, {"version": 3, "search_mode": "best_quality"}) + assert selection.load_search_mode() == "best_quality" + + +def test_unknown_value_falls_back_to_priority(monkeypatch): + _patch_profile(monkeypatch, {"version": 3, "search_mode": "nonsense"}) + assert selection.load_search_mode() == "priority" diff --git a/tests/quality/test_selection.py b/tests/quality/test_selection.py new file mode 100644 index 00000000..1bcb6bd9 --- /dev/null +++ b/tests/quality/test_selection.py @@ -0,0 +1,64 @@ +"""core.quality.selection — quality-aware ranking + the satisfied flag that +drives the engine's source fall-through. + +A source is "satisfied" when at least one of its candidates meets a real +target (strict, fallback off). The engine uses that to decide whether to +stop on the current source or escalate to the next. +""" + +import pytest + +from core.quality.model import AudioQuality, QualityTarget +from core.quality.selection import rank_with_targets + + +class _Cand: + """Minimal candidate: filter_and_rank only needs ``.audio_quality``.""" + def __init__(self, aq, name=""): + self.audio_quality = aq + self.name = name + + def __repr__(self): + return f"_Cand({self.name})" + + +FLAC_HIRES = AudioQuality('flac', sample_rate=96000, bit_depth=24) +FLAC_CD = AudioQuality('flac', sample_rate=44100, bit_depth=16) +MP3_320 = AudioQuality('mp3', bitrate=320) + +WANT_HIRES = [QualityTarget(label='FLAC 24', format='flac', bit_depth=24, min_sample_rate=96000)] +WANT_FLAC_ONLY = [QualityTarget(label='FLAC 16', format='flac', bit_depth=16)] + + +def test_satisfied_when_a_candidate_meets_a_target(): + cands = [_Cand(MP3_320, 'mp3'), _Cand(FLAC_HIRES, 'hires')] + ranked, satisfied = rank_with_targets(cands, WANT_HIRES, fallback_enabled=True) + assert satisfied is True + assert ranked[0].name == 'hires' # the matching candidate wins + + +def test_unsatisfied_but_fallback_returns_sorted_when_enabled(): + cands = [_Cand(MP3_320, 'mp3')] + ranked, satisfied = rank_with_targets(cands, WANT_FLAC_ONLY, fallback_enabled=True) + assert satisfied is False # no FLAC → no target met + assert [c.name for c in ranked] == ['mp3'] # but fallback keeps it + + +def test_unsatisfied_and_fallback_off_returns_empty(): + cands = [_Cand(MP3_320, 'mp3')] + ranked, satisfied = rank_with_targets(cands, WANT_FLAC_ONLY, fallback_enabled=False) + assert satisfied is False + assert ranked == [] + + +def test_empty_targets_accepts_everything_satisfied(): + cands = [_Cand(MP3_320, 'mp3'), _Cand(FLAC_CD, 'cd')] + ranked, satisfied = rank_with_targets(cands, [], fallback_enabled=True) + assert satisfied is True # no constraint → first source wins + assert ranked[0].name == 'cd' # still quality-sorted + + +def test_no_candidates_is_unsatisfied(): + ranked, satisfied = rank_with_targets([], WANT_FLAC_ONLY, fallback_enabled=True) + assert satisfied is False + assert ranked == [] diff --git a/tests/quality/test_source_map.py b/tests/quality/test_source_map.py new file mode 100644 index 00000000..b4d71c43 --- /dev/null +++ b/tests/quality/test_source_map.py @@ -0,0 +1,147 @@ +"""Tests for core.quality.source_map — each download source's tier/string +mapped into a unified AudioQuality. + +These mappers express each source's *claimed* capability tier. The values +are later verified post-download by the quality guard, so over-claiming is +the failure mode to avoid: an unknown tier must NOT pretend to be lossless. +""" + +import pytest + +from core.quality.source_map import ( + quality_from_tidal_tier, + quality_from_qobuz, + quality_from_deezer, + quality_from_amazon, + format_from_extension, + AUDIO_EXTENSIONS, +) + + +# ── Shared extension → format (used by every extension-based source) ──────── + +@pytest.mark.parametrize("ext,fmt", [ + ("flac", "flac"), (".flac", "flac"), + ("mp3", "mp3"), + ("m4a", "aac"), ("aac", "aac"), ("mp4", "aac"), + ("ogg", "ogg"), ("oga", "ogg"), + ("opus", "opus"), + ("wav", "wav"), ("wave", "wav"), + ("aiff", "wav"), ("aif", "wav"), # PCM → wav tier + ("wma", "wma"), + ("alac", "alac"), + ("xyz", "unknown"), ("", "unknown"), (None, "unknown"), +]) +def test_format_from_extension(ext, fmt): + assert format_from_extension(ext) == fmt + + +def test_audio_extensions_cover_all_known_formats(): + for e in (".flac", ".mp3", ".m4a", ".ogg", ".opus", ".wav", ".aiff", ".wma"): + assert e in AUDIO_EXTENSIONS + + +# ── Tidal / HiFi (Tidal-backed) ──────────────────────────────────────────── + +@pytest.mark.parametrize("tier", ["HI_RES_LOSSLESS", "HI_RES", "hi_res", "hires", "HIRES"]) +def test_tidal_hires_is_flac_24_96(tier): + aq = quality_from_tidal_tier(tier) + assert aq.format == "flac" + assert aq.bit_depth == 24 + assert aq.sample_rate == 96000 + + +@pytest.mark.parametrize("tier", ["LOSSLESS", "lossless"]) +def test_tidal_lossless_is_flac_16_44(tier): + aq = quality_from_tidal_tier(tier) + assert aq.format == "flac" + assert aq.bit_depth == 16 + assert aq.sample_rate == 44100 + + +def test_tidal_high_is_lossy_aac_320(): + aq = quality_from_tidal_tier("HIGH") + assert aq.format == "aac" + assert aq.bitrate == 320 + assert aq.bit_depth is None # lossy: no bit depth + + +def test_tidal_low_is_lossy_aac_96(): + aq = quality_from_tidal_tier("LOW") + assert aq.format == "aac" + assert aq.bitrate == 96 + + +def test_tidal_unknown_tier_does_not_overclaim(): + # An unrecognised tier must not masquerade as lossless. + aq = quality_from_tidal_tier("SOMETHING_NEW") + assert aq.format == "unknown" + assert aq.bit_depth is None + assert aq.sample_rate is None + + +# ── Qobuz (real API values) ──────────────────────────────────────────────── + +def test_qobuz_hires_khz_to_hz(): + aq = quality_from_qobuz(96.0, 24) + assert aq.format == "flac" + assert aq.sample_rate == 96000 + assert aq.bit_depth == 24 + + +def test_qobuz_cd_quality_fractional_khz(): + aq = quality_from_qobuz(44.1, 16) + assert aq.format == "flac" + assert aq.sample_rate == 44100 + assert aq.bit_depth == 16 + + +def test_qobuz_192k(): + aq = quality_from_qobuz(192.0, 24) + assert aq.sample_rate == 192000 + assert aq.bit_depth == 24 + + +# ── Deezer (config code) ─────────────────────────────────────────────────── + +def test_deezer_flac_is_16_44(): + aq = quality_from_deezer("flac") + assert aq.format == "flac" + assert aq.bit_depth == 16 + assert aq.sample_rate == 44100 + + +def test_deezer_mp3_320(): + aq = quality_from_deezer("mp3_320") + assert aq.format == "mp3" + assert aq.bitrate == 320 + assert aq.bit_depth is None + + +def test_deezer_mp3_128(): + aq = quality_from_deezer("mp3_128") + assert aq.format == "mp3" + assert aq.bitrate == 128 + + +# ── Amazon (real sampleRate preferred, tier fallback) ────────────────────── + +def test_amazon_prefers_real_sample_rate(): + aq = quality_from_amazon("HD", sample_rate=88200, bit_depth=24) + assert aq.format == "flac" + assert aq.sample_rate == 88200 + assert aq.bit_depth == 24 + + +def test_amazon_hd_tier_fallback(): + aq = quality_from_amazon("HD") + assert aq.format == "flac" + assert aq.sample_rate == 44100 + assert aq.bit_depth == 16 + + +def test_amazon_uhd_tier_fallback(): + aq = quality_from_amazon("UHD") + assert aq.format == "flac" + assert aq.bit_depth == 24 + assert aq.sample_rate == 96000 diff --git a/tests/quality/test_source_tier.py b/tests/quality/test_source_tier.py new file mode 100644 index 00000000..86abb050 --- /dev/null +++ b/tests/quality/test_source_tier.py @@ -0,0 +1,62 @@ +"""quality_tier_for_source — derive a source's requested download tier from +the GLOBAL quality profile instead of a per-source setting. + +Rule: pick the LOWEST source tier that satisfies the user's top (most +preferred) target — respecting the user's quality ceiling and saving +bandwidth — or the source's max tier when none can satisfy it (best effort). +""" + +import pytest + +import core.quality.source_map as sm +from core.quality.model import QualityTarget + + +def _patch_targets(monkeypatch, targets, fallback=True): + monkeypatch.setattr(sm, 'load_profile_targets', lambda: (targets, fallback)) + + +T_FLAC24_96 = [QualityTarget(label='', format='flac', bit_depth=24, min_sample_rate=96000)] +T_FLAC24_192 = [QualityTarget(label='', format='flac', bit_depth=24, min_sample_rate=192000)] +T_FLAC16 = [QualityTarget(label='', format='flac', bit_depth=16)] +T_MP3_320 = [QualityTarget(label='', format='mp3', min_bitrate=320)] + + +def test_tidal_hires_when_top_wants_24_96(monkeypatch): + _patch_targets(monkeypatch, T_FLAC24_96) + assert sm.quality_tier_for_source('tidal') == 'hires' + + +def test_tidal_lossless_respects_16bit_ceiling(monkeypatch): + # User caps at 16-bit → request lossless, NOT hires (saves bandwidth). + _patch_targets(monkeypatch, T_FLAC16) + assert sm.quality_tier_for_source('tidal') == 'lossless' + + +def test_tidal_best_effort_max_when_unsatisfiable(monkeypatch): + # Source maxes at 24/96 but user wants 24/192 → best effort = max tier. + _patch_targets(monkeypatch, T_FLAC24_192) + assert sm.quality_tier_for_source('tidal') == 'hires' + + +def test_no_targets_requests_max(monkeypatch): + _patch_targets(monkeypatch, []) + assert sm.quality_tier_for_source('tidal') == 'hires' + assert sm.quality_tier_for_source('deezer') == 'flac' + + +def test_deezer_flac_and_mp3(monkeypatch): + _patch_targets(monkeypatch, T_FLAC16) + assert sm.quality_tier_for_source('deezer') == 'flac' + _patch_targets(monkeypatch, T_MP3_320) + assert sm.quality_tier_for_source('deezer') == 'mp3_320' + + +def test_qobuz_hires_max(monkeypatch): + _patch_targets(monkeypatch, T_FLAC24_192) + assert sm.quality_tier_for_source('qobuz') == 'hires_max' + + +def test_unknown_source_returns_default(monkeypatch): + _patch_targets(monkeypatch, T_FLAC16) + assert sm.quality_tier_for_source('nope', default='x') == 'x' diff --git a/tests/quality/test_track_result_quality.py b/tests/quality/test_track_result_quality.py new file mode 100644 index 00000000..047fd0df --- /dev/null +++ b/tests/quality/test_track_result_quality.py @@ -0,0 +1,39 @@ +"""TrackResult.set_quality() — merge a mapped AudioQuality onto a result so +its derived ``audio_quality`` reflects the source's real/claimed tier. +""" + +from core.download_plugins.types import TrackResult +from core.quality.model import AudioQuality + + +def _tr(**kw): + base = dict( + username='x', filename='f', size=0, bitrate=None, duration=None, + quality='', free_upload_slots=0, upload_speed=0, queue_length=0, + ) + base.update(kw) + return TrackResult(**base) + + +def test_set_quality_populates_lossless_fields(): + tr = _tr(quality='flac', bitrate=1411) + tr.set_quality(AudioQuality('flac', sample_rate=96000, bit_depth=24)) + assert tr.quality == 'flac' + assert tr.sample_rate == 96000 + assert tr.bit_depth == 24 + # derived descriptor must reflect the merge + assert tr.audio_quality.sample_rate == 96000 + assert tr.audio_quality.bit_depth == 24 + + +def test_set_quality_keeps_existing_bitrate_when_mapper_has_none(): + tr = _tr(quality='flac', bitrate=950) + tr.set_quality(AudioQuality('flac', sample_rate=44100, bit_depth=16)) + assert tr.bitrate == 950 # mapper bitrate is None → preserve probed/reported + + +def test_set_quality_overwrites_bitrate_for_lossy(): + tr = _tr(quality='mp3', bitrate=128) + tr.set_quality(AudioQuality('mp3', bitrate=320)) + assert tr.bitrate == 320 + assert tr.bit_depth is None diff --git a/tests/repair_jobs/test_quality_upgrade.py b/tests/repair_jobs/test_quality_upgrade.py index d1ff35fa..d265d4bb 100644 --- a/tests/repair_jobs/test_quality_upgrade.py +++ b/tests/repair_jobs/test_quality_upgrade.py @@ -38,6 +38,25 @@ NOTHING_ENABLED = {'qualities': {'flac': {'enabled': False}, 'mp3_320': {'enable # --- pure quality decision ------------------------------------------------- +# +# The old extension-only classifier (meets_preferred_quality / classify_track_ +# quality / preferred_quality_floor / RANK_*) was deleted in favour of the +# shared v3 path: probe → AudioQuality → quality_meets_profile against the +# profile's ranked targets. These pin the same behavioural contract through the +# new API. (Bps→kbps normalisation now lives in probe_audio_quality and is +# covered by its own tests; the deleted-internals tests for it were removed.) + +def meets(path, bitrate, profile): + """Does a file of this format+bitrate satisfy the profile? Mirrors the + scanner's decision: build the measured AudioQuality and check it against the + v3 ranked targets derived from the profile (empty targets → nothing flagged).""" + from core.quality.model import AudioQuality + from core.quality.selection import quality_meets_profile, targets_from_profile + + ext = path.rsplit('.', 1)[-1].lower() + targets, _ = targets_from_profile(profile) + return quality_meets_profile(AudioQuality(format=ext, bitrate=bitrate), targets) + def test_balanced_profile_accepts_320_mp3_REGRESSION(): """The headline bug: with FLAC+320+256 enabled, a 320 kbps MP3 is acceptable. @@ -69,30 +88,6 @@ def test_nothing_enabled_flags_nothing(): assert meets('song.mp3', 64, NOTHING_ENABLED) is True -def test_bitrate_in_bps_is_normalized(): - """Library bitrate stored as bps (320000) classifies the same as 320 kbps.""" - assert qu.classify_track_quality('song.mp3', 320000) == qu.RANK_320 - assert meets('song.mp3', 320000, BALANCED) is True - - -def test_unknown_lossy_bitrate_not_flagged_under_lossy_floor(): - """A lossy file with no bitrate can't be judged against a lossy floor → don't - flag (avoid false positives); but under a lossless floor it's clearly below.""" - assert meets('song.mp3', None, BALANCED) is True - assert meets('song.mp3', None, LOSSLESS_ONLY) is False - - -def test_floor_is_worst_enabled_not_best(): - # FLAC+320+256 enabled → floor is MP3-256 (rank 2), not FLAC. - assert qu.preferred_quality_floor(BALANCED) == qu.RANK_256 - assert qu.preferred_quality_floor(LOSSLESS_ONLY) == qu.RANK_LOSSLESS - assert qu.preferred_quality_floor(NOTHING_ENABLED) is None - - -def meets(path, bitrate, profile): - return qu.meets_preferred_quality(path, bitrate, profile) - - # --- scan produces a finding (seam) ---------------------------------------- class _FakeConn: @@ -149,7 +144,25 @@ def _row(track_id=1, title='Song One', path='/music/a.mp3', bitrate=128, duratio return (track_id, title, path, bitrate, duration, artist, album, album_id, track_number) +def _stub_quality(monkeypatch, *, meets: bool): + """Stub the v3 quality path so scan() works on fake (non-existent) file paths. + + The job now probes the REAL file (mutagen) and checks it against the v3 + ranked targets. Tests use fake paths, so we resolve the path to itself, + return a dummy measured quality, and force the meets/below verdict. + """ + from core.quality.model import AudioQuality, QualityTarget + monkeypatch.setattr(qu, 'targets_from_profile', + lambda profile: ([QualityTarget(label='MP3 320', format='mp3', min_bitrate=320)], False)) + monkeypatch.setattr(qu, 'resolve_library_file_path', lambda p, **kw: p) + monkeypatch.setattr(qu, 'probe_audio_quality', + lambda p: AudioQuality(format='mp3', bitrate=128)) + monkeypatch.setattr(qu, 'quality_meets_profile', lambda aq, targets: meets) + + def _stub_engine(monkeypatch): + # Below-profile by default — the finding-creating tests want a flagged track. + _stub_quality(monkeypatch, meets=False) monkeypatch.setattr(qu, 'get_primary_source', lambda: 'spotify') monkeypatch.setattr(qu, 'get_source_priority', lambda src: ['spotify']) monkeypatch.setattr( @@ -168,7 +181,7 @@ def test_scan_creates_finding_for_low_quality_track(monkeypatch): fake_match = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X', 'images': []}} # No track-id / ISRC / album hit → exercise the search tier. - monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {}) + monkeypatch.setattr(qu, '_read_file_ids', lambda fp, **kw: {}) monkeypatch.setattr(qu, '_match_via_track_id', lambda *a, **k: (None, None)) monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (None, None)) monkeypatch.setattr(qu, '_find_best_match', @@ -212,7 +225,7 @@ def test_scan_prefers_track_id_tier(monkeypatch): """The source's own track ID (from file tags) wins over every other tier.""" db = _FakeDB([_row()], BALANCED) _stub_engine(monkeypatch) - monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {'spotify_track_id': 'sp9', 'isrc': 'X'}) + monkeypatch.setattr(qu, '_read_file_ids', lambda fp, **kw: {'spotify_track_id': 'sp9', 'isrc': 'X'}) fake = {'id': 'sp9', 'name': 'Song One', 'album': {'name': 'Album X'}} monkeypatch.setattr(qu, '_match_via_track_id', lambda ids, sp: (fake, 'spotify')) monkeypatch.setattr(qu, '_normalize_track_match', lambda t, s: dict(fake)) @@ -275,7 +288,7 @@ def test_scan_prefers_isrc_exact_match_over_fuzzy(monkeypatch): and do NOT run the album/search tiers.""" db = _FakeDB([_row()], BALANCED) _stub_engine(monkeypatch) - monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {'isrc': 'USRC17607839'}) + monkeypatch.setattr(qu, '_read_file_ids', lambda fp, **kw: {'isrc': 'USRC17607839'}) monkeypatch.setattr(qu, '_match_via_track_id', lambda *a, **k: (None, None)) fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} monkeypatch.setattr(qu, '_match_via_isrc', lambda isrc, sp: (fake, 'spotify')) @@ -297,7 +310,7 @@ def test_scan_falls_back_to_search_without_ids(monkeypatch): """No track-ID / ISRC / album hit → fall back to fuzzy search.""" db = _FakeDB([_row()], BALANCED) _stub_engine(monkeypatch) - monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {}) # un-enriched + monkeypatch.setattr(qu, '_read_file_ids', lambda fp, **kw: {}) # un-enriched monkeypatch.setattr(qu, '_match_via_track_id', lambda *a, **k: (None, None)) monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (None, None)) fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} @@ -316,7 +329,7 @@ def test_scan_uses_album_tier_when_no_ids(monkeypatch): 'album', and the fuzzy search is never reached.""" db = _FakeDB([_row()], BALANCED) _stub_engine(monkeypatch) - monkeypatch.setattr(qu, '_read_file_ids', lambda fp: {}) + monkeypatch.setattr(qu, '_read_file_ids', lambda fp, **kw: {}) monkeypatch.setattr(qu, '_match_via_track_id', lambda *a, **k: (None, None)) fake = {'id': 'sp1', 'name': 'Song One', 'artists': ['Artist A'], 'album': {'name': 'Album X'}} monkeypatch.setattr(qu, '_match_via_album', lambda *a, **k: (fake, 'spotify')) @@ -346,8 +359,9 @@ def test_find_track_in_album_exact_title_with_track_number(monkeypatch): def test_scan_skips_tracks_meeting_quality(monkeypatch): - # A 320 kbps MP3 meets the balanced profile → no finding, no metadata calls. + # A track that meets the profile → no finding, no metadata calls. db = _FakeDB([_row(track_id=2, title='Good Song', bitrate=320)], BALANCED) + _stub_quality(monkeypatch, meets=True) def _boom(*a, **k): # must never be called for an acceptable track raise AssertionError("matching should not run for an acceptable track") diff --git a/tests/test_hifi_preview_detection.py b/tests/test_hifi_preview_detection.py new file mode 100644 index 00000000..ae0bed7e --- /dev/null +++ b/tests/test_hifi_preview_detection.py @@ -0,0 +1,70 @@ +"""HiFi/Monochrome preview detection. + +Some Monochrome instances only have 30-second Tidal preview access for +usage=DOWNLOAD: the HLS variant playlist for a 220s track contains only ~30s +of segments (with #EXT-X-ENDLIST). Detect that at manifest time so HiFi +declines and the orchestrator falls through to a real source — instead of +downloading the 30s file and quarantining it after the fact. +""" + +import sys +import types + +import pytest + +# hifi_client imports config/db at module load; stub the heavy bits if needed. +from core.hifi_client import hls_total_seconds, is_preview_playlist + + +_PREVIEW_PLAYLIST = """#EXTM3U +#EXT-X-VERSION:6 +#EXT-X-PLAYLIST-TYPE:VOD +#EXT-X-TARGETDURATION:4 +#EXTINF:3.994, +seg1.mp4 +#EXTINF:3.994, +seg2.mp4 +#EXTINF:3.994, +seg3.mp4 +#EXTINF:3.994, +seg4.mp4 +#EXTINF:3.994, +seg5.mp4 +#EXTINF:3.994, +seg6.mp4 +#EXTINF:3.994, +seg7.mp4 +#EXTINF:1.9, +seg8.mp4 +#EXT-X-ENDLIST +""" + + +def test_total_seconds_sums_extinf(): + assert hls_total_seconds(_PREVIEW_PLAYLIST) == pytest.approx(29.86, abs=0.1) + + +def test_total_seconds_empty(): + assert hls_total_seconds("") == 0.0 + assert hls_total_seconds("#EXTM3U\nno segments\n") == 0.0 + + +def test_preview_when_playlist_far_shorter_than_track(): + # 30s playlist for a 220s track → preview. + assert is_preview_playlist(playlist_s=29.9, track_s=220) is True + + +def test_not_preview_when_playlist_matches_track(): + assert is_preview_playlist(playlist_s=218.0, track_s=220) is False + + +def test_not_preview_when_track_duration_unknown(): + # No reference → can't decide; don't false-positive (post-download guard + # is the safety net). + assert is_preview_playlist(playlist_s=29.9, track_s=0) is False + assert is_preview_playlist(playlist_s=0, track_s=220) is False + + +def test_short_real_track_not_flagged(): + # A genuinely ~30s track whose playlist is ~30s is not a preview. + assert is_preview_playlist(playlist_s=29.0, track_s=31) is False diff --git a/tests/test_hifi_preview_guard.py b/tests/test_hifi_preview_guard.py index ed085933..a523e3c1 100644 --- a/tests/test_hifi_preview_guard.py +++ b/tests/test_hifi_preview_guard.py @@ -85,10 +85,13 @@ def _bare_client(tmp_path): def test_download_sync_skips_preview_manifests_and_never_downloads(tmp_path, monkeypatch): monkeypatch.setattr(hc, 'config_manager', _Cfg()) + # Pin the starting tier so the chain is deterministic and independent of the + # global quality profile (which now drives quality_tier_for_source). + monkeypatch.setattr(hc, 'quality_tier_for_source', lambda *a, **k: 'lossless') 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': ( + c._get_hls_manifest = lambda tid, quality='lossless', **kw: ( tiers.append(quality) or {'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', 'manifest_duration': 30.0}) # preview at EVERY tier @@ -105,7 +108,7 @@ def test_download_sync_proceeds_past_the_gate_for_a_full_manifest(tmp_path, monk 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': { + c._get_hls_manifest = lambda tid, quality='lossless', **kw: { 'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', 'manifest_duration': 215.0} # full length → must NOT skip seg_calls = [] @@ -124,10 +127,11 @@ def test_download_sync_aborts_on_a_faked_full_length_file_no_tier_cascade(tmp_pa # 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()) + monkeypatch.setattr(hc, 'quality_tier_for_source', lambda *a, **k: 'lossless') c = _bare_client(tmp_path) c.get_track_info = lambda tid: {'duration_s': 215} tiers = [] - c._get_hls_manifest = lambda tid, quality='lossless': ( + c._get_hls_manifest = lambda tid, quality='lossless', **kw: ( 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 @@ -144,7 +148,7 @@ def test_download_sync_does_not_reject_when_track_length_unknown(tmp_path, monke 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': { + c._get_hls_manifest = lambda tid, quality='lossless', **kw: { 'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', 'manifest_duration': 30.0} # short, but expected is unknown seg_calls = [] diff --git a/tests/test_integrity_failure_marks_task_failed.py b/tests/test_integrity_failure_marks_task_failed.py index e91fe8d6..ecf68974 100644 --- a/tests/test_integrity_failure_marks_task_failed.py +++ b/tests/test_integrity_failure_marks_task_failed.py @@ -144,6 +144,60 @@ def test_race_guard_failure_marker_marks_task_failed(_isolate_state): assert ('b2', 't2', False) in completion_calls +def test_quality_quarantine_does_not_mark_completed(_isolate_state): + """When the inner pipeline quality-quarantines the file (``_bitdepth_rejected``), + the wrapper must NOT fall through to "assume success" and mark the task + Completed — that made the quarantined file appear in BOTH Completed and + Quarantine. The inner pipeline already owns the retry/fail for quality, so + the wrapper just returns.""" + completion_calls = [] + runtime = _build_runtime(completion_calls) + + _seed_task('t5', 'b5') + + context = {'task_id': 't5', 'batch_id': 'b5', 'context_key': 'test::ctx5'} + + def _inner(*a, **kw): + # Inner pipeline quarantined for quality and handled its own retry/fail; + # it sets the marker and leaves NO _final_processed_path. + context['_bitdepth_rejected'] = True + + with patch.object(import_pipeline, 'post_process_matched_download', _inner), \ + patch.object(import_pipeline, '_mark_task_completed', + lambda task_id, ti: runtime_state.download_tasks[task_id].update( + {'status': 'completed'})): + import_pipeline.post_process_matched_download_with_verification( + 'test::ctx5', context, '/fake/source.mp3', 't5', 'b5', runtime, + ) + + assert runtime_state.download_tasks['t5']['status'] != 'completed' + assert ('b5', 't5', True) not in completion_calls + + +def test_silence_quarantine_does_not_mark_completed(_isolate_state): + """Same contract for the audio guard (``_silence_rejected``).""" + completion_calls = [] + runtime = _build_runtime(completion_calls) + + _seed_task('t6', 'b6') + + context = {'task_id': 't6', 'batch_id': 'b6', 'context_key': 'test::ctx6'} + + def _inner(*a, **kw): + context['_silence_rejected'] = True + + with patch.object(import_pipeline, 'post_process_matched_download', _inner), \ + patch.object(import_pipeline, '_mark_task_completed', + lambda task_id, ti: runtime_state.download_tasks[task_id].update( + {'status': 'completed'})): + import_pipeline.post_process_matched_download_with_verification( + 'test::ctx6', context, '/fake/source.flac', 't6', 'b6', runtime, + ) + + assert runtime_state.download_tasks['t6']['status'] != 'completed' + assert ('b6', 't6', True) not in completion_calls + + def test_no_failure_markers_still_assumes_success(_isolate_state): """The pre-existing "assume success" fallback must STILL fire when no failure markers are set — some legitimate flows complete without diff --git a/tests/tools/test_amazon_download_client.py b/tests/tools/test_amazon_download_client.py index f831f268..75c6a54b 100644 --- a/tests/tools/test_amazon_download_client.py +++ b/tests/tools/test_amazon_download_client.py @@ -224,7 +224,10 @@ class TestSearch: assert t.artist == "Kendrick Lamar" assert t.title == "Track 0" assert t.album == "GNX" - assert t.quality == "Lossless" + # Quality is now stamped as a real format token (was the display + # label "Lossless", which broke audio_quality format derivation). + assert t.quality == "flac" + assert t.audio_quality.format == "flac" assert t.duration == 200_000 def test_track_source_metadata(self, tmp_path): diff --git a/web_server.py b/web_server.py index bcd38861..4e330c92 100644 --- a/web_server.py +++ b/web_server.py @@ -4542,19 +4542,23 @@ def get_quality_presets(): @app.route('/api/quality-profile/preset/', methods=['POST']) def apply_quality_preset(preset_name): - """Apply a predefined quality preset""" + """Switch to a quality preset, restoring its saved edits if it has any.""" try: from database.music_database import MusicDatabase db = MusicDatabase() - preset = db.get_quality_preset(preset_name) + current = db.get_quality_profile() + preset = dict(db.get_quality_preset(preset_name)) + # search_mode is a global search strategy, not a per-preset audio setting — + # carry the user's current choice across preset switches. + preset['search_mode'] = current.get('search_mode', preset.get('search_mode', 'priority')) success = db.set_quality_profile(preset) if success: - add_activity_item("", "Quality Preset Applied", f"Applied '{preset_name}' preset", "Now") + add_activity_item("", "Quality Preset Applied", f"Switched to '{preset_name}' preset", "Now") return jsonify({ "success": True, - "message": f"Applied '{preset_name}' preset", + "message": f"Switched to '{preset_name}' preset", "profile": preset }) else: @@ -4564,6 +4568,33 @@ def apply_quality_preset(preset_name): logger.error(f"Error applying quality preset: {e}") return jsonify({"success": False, "error": str(e)}), 500 + +@app.route('/api/quality-profile/preset//reset', methods=['POST']) +def reset_quality_preset(preset_name): + """Discard a preset's saved edits and restore its factory defaults.""" + try: + from database.music_database import MusicDatabase + db = MusicDatabase() + + current = db.get_quality_profile() + preset = dict(db.reset_quality_preset(preset_name)) + preset['search_mode'] = current.get('search_mode', preset.get('search_mode', 'priority')) + success = db.set_quality_profile(preset) + + if success: + add_activity_item("", "Quality Preset Reset", f"Reset '{preset_name}' to defaults", "Now") + return jsonify({ + "success": True, + "message": f"Reset '{preset_name}' to defaults", + "profile": preset + }) + else: + return jsonify({"success": False, "error": "Failed to reset preset"}), 500 + + except Exception as e: + logger.error(f"Error resetting quality preset: {e}") + return jsonify({"success": False, "error": str(e)}), 500 + # =============================== # == END QUALITY PROFILE API == # =============================== @@ -7609,6 +7640,17 @@ def approve_quarantine_item(entry_id): _t = download_tasks.get(_task_id) if isinstance(_t, dict): _batch_id = _t.get('batch_id') + else: + # Manager-tab approve: no task_id in the request. Find the task that + # owns this quarantine entry so the re-import marks it completed in + # the downloads list without waiting for the batch to finish. + with tasks_lock: + for _tid, _t in download_tasks.items(): + if isinstance(_t, dict) and _t.get('quarantine_entry_id') == entry_id: + _task_id = _tid + _batch_id = _t.get('batch_id') + break + if _task_id: context['task_id'] = _task_id if _batch_id: context['batch_id'] = _batch_id @@ -7637,11 +7679,35 @@ def approve_quarantine_item(entry_id): logger.info(f"[Quarantine] Auto-removed {len(removed_siblings)} sibling alternative(s) of {entry_id}: {removed_siblings}") except Exception as sib_exc: logger.warning(f"[Quarantine] Sibling cleanup for {entry_id} failed: {sib_exc}") + # Cancel any still-running quarantine-retry task for the same track so + # the engine doesn't keep fetching new candidates after the user has + # already accepted one. Match by track title from the restored context. + cancelled_retry_task = None + try: + ti = context.get('track_info') if isinstance(context.get('track_info'), dict) else {} + _approved_name = (ti.get('name') or '').strip().lower() + if _approved_name: + with tasks_lock: + for _tid, _t in download_tasks.items(): + if not _t.get('_quarantine_retry'): + continue + if _t.get('status') in ('completed', 'cancelled', 'failed'): + continue + _tti = _t.get('track_info') if isinstance(_t.get('track_info'), dict) else {} + if (_tti.get('name') or '').strip().lower() == _approved_name: + _t['status'] = 'cancelled' + _t['_quarantine_approved_alternative'] = True + cancelled_retry_task = _tid + logger.info(f"[Quarantine] Cancelled in-flight retry task {_tid} for '{_approved_name}' (user approved alternative)") + break + except Exception as _crt_exc: + logger.debug(f"[Quarantine] Retry-cancel scan failed: {_crt_exc}") return jsonify({ "success": True, "trigger_bypassed": "all", "original_trigger": trigger, "removed_siblings": removed_siblings, + "cancelled_retry_task": cancelled_retry_task, }) except Exception as e: logger.error(f"[Quarantine] Error approving {entry_id}: {e}") @@ -7752,7 +7818,8 @@ def get_verification_config(): queue collapses to quarantine-only in the UI.""" try: enabled = bool(config_manager.get('acoustid.enabled', False)) - return jsonify({"success": True, "acoustid_enabled": enabled}) + require_verified = bool(config_manager.get('acoustid.require_verified', False)) + return jsonify({"success": True, "acoustid_enabled": enabled, "require_verified": require_verified}) except Exception as e: return jsonify({"success": True, "acoustid_enabled": True, "error": str(e)}) @@ -11927,6 +11994,9 @@ def _sync_tracks_to_server(track_rows, server_type): return result +_resolve_library_diag_logged = False + + def _resolve_library_file_path(file_path): """Resolve a library file path to an actual file on disk.""" if not file_path: @@ -11963,22 +12033,58 @@ def _resolve_library_file_path(file_path): except Exception as e: logger.debug("library music paths read failed: %s", e) - path_parts = file_path.replace('\\', '/').split('/') - - # Try progressively shorter path suffixes against each candidate directory - # (skip index 0 to avoid drive letter issues). find_on_disk matches each - # component exactly when present, else folds typographic confusables (#833: - # curly U+2019 apostrophe in DB metadata vs ASCII U+0027 on disk) — exact - # matches always win, so paths that already resolved are unaffected. - from core.library.path_resolve import find_on_disk - for base_dir in [transfer_dir, download_dir] + list(library_dirs): - if not base_dir or not os.path.isdir(base_dir): + # Build the set of candidate base directories (absolute forms only, so + # os.path.join produces unambiguous paths). Config often stores RELATIVE + # paths ("./Transfer") — take os.path.abspath() so they resolve from the + # current working directory of the server process. + library_dir_list = list(library_dirs) + raw_bases = [transfer_dir, download_dir] + library_dir_list + abs_bases = [] + seen_abs = set() + for b in raw_bases: + if not b: continue - for i in range(1, len(path_parts)): - found = find_on_disk(base_dir, path_parts[i:]) + for form in [os.path.abspath(b), b, '/' + b.replace('./', '', 1).lstrip('/')]: + a = os.path.abspath(form) if not os.path.isabs(form) else form + if a and a not in seen_abs and os.path.isdir(a): + seen_abs.add(a) + abs_bases.append(a) + + # --- Fast path: direct join --- + # When the DB stores a clean relative path ("Artist/Album/Track.flac") this + # is all that's needed. No component-by-component descent, no confusable + # folding. This handles the common Docker case where CWD is /app, config + # says ./Transfer, and files live at /app/Transfer/Artist/Album/Track.flac. + clean_rel = file_path.replace('\\', '/') + for abs_base in abs_bases: + candidate = os.path.join(abs_base, clean_rel) + if os.path.exists(candidate): + logger.debug("[PathResolve] direct join: %r → %r", file_path, candidate) + return candidate + + # --- Slow path: confusable-tolerant suffix scan --- + # Handles paths with typographic apostrophes/dashes that differ between the + # DB metadata and the actual on-disk filename (#833). + path_parts = clean_rel.split('/') + from core.library.path_resolve import find_on_disk + for abs_base in abs_bases: + for i in range(0, len(path_parts)): + found = find_on_disk(abs_base, path_parts[i:]) if found: return found + # Couldn't resolve — log the bases we searched ONCE so a path/mount mismatch + # is diagnosable (e.g. files live under a dir that isn't transfer/download/ + # a configured library path). + global _resolve_library_diag_logged + if not _resolve_library_diag_logged: + _resolve_library_diag_logged = True + logger.warning( + "[PathResolve] Could not resolve %r — tried direct-join + suffix-scan under %r (cwd=%r). " + "If files live elsewhere, set soulseek.transfer_path to the absolute mount or add " + "the dir under Settings > Library music paths.", + file_path, abs_bases, os.getcwd(), + ) return None @@ -15200,6 +15306,14 @@ def _get_file_path_from_template_raw(template: str, context: dict) -> tuple: return '', _sanitize_filename(full_path) +def _probe_audio_quality(file_path): + """Probe real measured audio quality (bit depth / sample rate / bitrate) + as an AudioQuality, for the library quality scanner. Delegates to the same + core the download import guard uses. Returns None on any error.""" + from core.imports.file_ops import probe_audio_quality + return probe_audio_quality(file_path) + + def _get_audio_quality_string(file_path): """ Read audio file and return a quality descriptor string. @@ -18304,9 +18418,9 @@ def _build_candidates_deps(): ) -def _attempt_download_with_candidates(task_id, candidates, track, batch_id=None): +def _attempt_download_with_candidates(task_id, candidates, track, batch_id=None, **kwargs): return _downloads_candidates.attempt_download_with_candidates( - task_id, candidates, track, batch_id, _build_candidates_deps() + task_id, candidates, track, batch_id, _build_candidates_deps(), **kwargs ) @@ -18757,6 +18871,7 @@ def _build_status_deps(): page=1, limit=limit, )[0], + get_unverified_download_history=lambda: get_database().get_library_history_unverified(), ) diff --git a/webui/index.html b/webui/index.html index 0534c2c9..2d0e6fe3 100644 --- a/webui/index.html +++ b/webui/index.html @@ -4224,7 +4224,8 @@
@@ -4744,21 +4745,8 @@