Fix AcoustID verification: MusicBrainz metadata fallback and quarantine reliability
This commit is contained in:
parent
d73f91ea1c
commit
3f0854e070
2 changed files with 107 additions and 7 deletions
|
|
@ -9,11 +9,13 @@ the file is flagged as incorrect.
|
|||
"""
|
||||
|
||||
import re
|
||||
import threading
|
||||
from difflib import SequenceMatcher
|
||||
from typing import Optional, Dict, Any, Tuple, List
|
||||
from enum import Enum
|
||||
from utils.logging_config import get_logger
|
||||
from core.acoustid_client import AcoustIDClient
|
||||
from core.musicbrainz_client import MusicBrainzClient
|
||||
|
||||
logger = get_logger("acoustid_verification")
|
||||
|
||||
|
|
@ -94,6 +96,91 @@ def _find_best_title_artist_match(
|
|||
return best_rec, best_title_sim, best_artist_sim
|
||||
|
||||
|
||||
# Shared MusicBrainz client for enrichment lookups
|
||||
_mb_client = None
|
||||
_mb_client_lock = threading.Lock()
|
||||
|
||||
MAX_MB_ENRICHMENT_LOOKUPS = 3
|
||||
|
||||
|
||||
def _get_mb_client() -> MusicBrainzClient:
|
||||
"""Get or create a shared MusicBrainz client instance."""
|
||||
global _mb_client
|
||||
if _mb_client is None:
|
||||
with _mb_client_lock:
|
||||
if _mb_client is None:
|
||||
_mb_client = MusicBrainzClient()
|
||||
return _mb_client
|
||||
|
||||
|
||||
def _enrich_recordings_from_musicbrainz(
|
||||
recordings: List[Dict[str, Any]],
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""
|
||||
Enrich recordings that are missing title/artist by looking up their
|
||||
MBIDs via MusicBrainz.
|
||||
|
||||
AcoustID often returns recordings with title=None, artist=None even though
|
||||
the MBIDs are valid. This resolves the metadata so verification can compare
|
||||
title/artist instead of skipping.
|
||||
|
||||
Args:
|
||||
recordings: List of recording dicts from fingerprint_and_lookup()
|
||||
|
||||
Returns:
|
||||
The same list, with title/artist filled in where possible.
|
||||
"""
|
||||
# Fast path: if any recording already has title AND artist, no enrichment needed
|
||||
if any(rec.get('title') and rec.get('artist') for rec in recordings):
|
||||
return recordings
|
||||
|
||||
logger.info(f"Enriching {len(recordings)} recordings via MusicBrainz (all missing title/artist)...")
|
||||
|
||||
mb = _get_mb_client()
|
||||
enriched_count = 0
|
||||
|
||||
for rec in recordings[:MAX_MB_ENRICHMENT_LOOKUPS]:
|
||||
mbid = rec.get('mbid')
|
||||
if not mbid:
|
||||
continue
|
||||
|
||||
try:
|
||||
data = mb.get_recording(mbid, includes=['artist-credits'])
|
||||
if not data:
|
||||
logger.debug(f"MusicBrainz returned no data for recording {mbid}")
|
||||
continue
|
||||
|
||||
title = data.get('title')
|
||||
artist_credit = data.get('artist-credit', [])
|
||||
|
||||
# Build artist string from artist-credit array
|
||||
# Each entry has {"artist": {"name": "..."}, "joinphrase": "..."}
|
||||
artist_parts = []
|
||||
for credit in artist_credit:
|
||||
name = credit.get('artist', {}).get('name', '')
|
||||
joinphrase = credit.get('joinphrase', '')
|
||||
if name:
|
||||
artist_parts.append(name + joinphrase)
|
||||
artist = ''.join(artist_parts).strip() if artist_parts else None
|
||||
|
||||
if title:
|
||||
rec['title'] = title
|
||||
logger.debug(f"Enriched {mbid}: title='{title}'")
|
||||
if artist:
|
||||
rec['artist'] = artist
|
||||
logger.debug(f"Enriched {mbid}: artist='{artist}'")
|
||||
|
||||
if title or artist:
|
||||
enriched_count += 1
|
||||
|
||||
except Exception as e:
|
||||
logger.debug(f"Failed to enrich recording {mbid}: {e}")
|
||||
continue
|
||||
|
||||
logger.info(f"Enriched {enriched_count}/{min(len(recordings), MAX_MB_ENRICHMENT_LOOKUPS)} recordings from MusicBrainz")
|
||||
return recordings
|
||||
|
||||
|
||||
class AcoustIDVerification:
|
||||
"""
|
||||
Verification service that compares audio fingerprint identity
|
||||
|
|
@ -175,6 +262,9 @@ class AcoustIDVerification:
|
|||
logger.info(msg)
|
||||
return VerificationResult.SKIP, msg
|
||||
|
||||
# Enrich recordings that are missing title/artist via MusicBrainz lookup
|
||||
recordings = _enrich_recordings_from_musicbrainz(recordings)
|
||||
|
||||
# Step 4: Find best title/artist match among AcoustID results
|
||||
best_rec, title_sim, artist_sim = _find_best_title_artist_match(
|
||||
recordings, expected_track_name, expected_artist_name
|
||||
|
|
|
|||
|
|
@ -9145,9 +9145,9 @@ def _move_to_quarantine(file_path: str, context: dict, reason: str) -> str:
|
|||
from pathlib import Path
|
||||
from datetime import datetime
|
||||
|
||||
# Get quarantine directory (parallel to Transfer folder)
|
||||
transfer_dir = docker_resolve_path(config_manager.get('soulseek.transfer_path', './Transfer'))
|
||||
quarantine_dir = Path(transfer_dir).parent / "Quarantine"
|
||||
# Get quarantine directory (inside download folder — always writable, even in Docker)
|
||||
download_dir = docker_resolve_path(config_manager.get('soulseek.download_path', './downloads'))
|
||||
quarantine_dir = Path(download_dir) / "Quarantine"
|
||||
quarantine_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Create quarantine entry with timestamp
|
||||
|
|
@ -9353,10 +9353,20 @@ def _post_process_matched_download(context_key, context, file_path):
|
|||
|
||||
if verification_result == VerificationResult.FAIL:
|
||||
# Move to quarantine instead of Transfer
|
||||
quarantine_path = _move_to_quarantine(file_path, context, verification_msg)
|
||||
print(f"🚫 File quarantined due to verification failure: {quarantine_path}")
|
||||
try:
|
||||
quarantine_path = _move_to_quarantine(file_path, context, verification_msg)
|
||||
print(f"🚫 File quarantined due to verification failure: {quarantine_path}")
|
||||
except Exception as quarantine_error:
|
||||
# Quarantine failed — delete the known-wrong file instead
|
||||
# NEVER save a file we've confirmed is wrong
|
||||
logger.error(f"Quarantine failed ({quarantine_error}), deleting wrong file: {file_path}")
|
||||
print(f"🚫 Quarantine failed, deleting wrong file: {file_path}")
|
||||
try:
|
||||
os.remove(file_path)
|
||||
except Exception as del_error:
|
||||
logger.error(f"Could not delete wrong file either: {del_error}")
|
||||
|
||||
# Set flag so the _with_verification wrapper knows we quarantined
|
||||
# These always execute for FAIL — whether quarantine succeeded or not
|
||||
context['_acoustid_quarantined'] = True
|
||||
context['_acoustid_failure_msg'] = verification_msg
|
||||
|
||||
|
|
@ -9378,7 +9388,7 @@ def _post_process_matched_download(context_key, context, file_path):
|
|||
if task_id and batch_id:
|
||||
_on_download_completed(batch_id, task_id, success=False)
|
||||
|
||||
return # Don't continue with normal processing
|
||||
return # NEVER continue processing a known-wrong file
|
||||
else:
|
||||
print(f"⚠️ AcoustID verification skipped: missing track/artist info")
|
||||
else:
|
||||
|
|
|
|||
Loading…
Reference in a new issue