diff --git a/companion-api/main.py b/companion-api/main.py index 4bae094..ce69cea 100644 --- a/companion-api/main.py +++ b/companion-api/main.py @@ -61,14 +61,92 @@ COVER_NAMES = ('cover.jpg', 'folder.jpg', 'artwork.jpg', 'front.jpg', 'cover.png', 'folder.png', 'artwork.png', 'front.png') + +# ── Database connection management ────────────────────────────────────────── +# +# CRITICAL: Python's `with sqlite3.connect(...) as c:` is a TRANSACTION +# manager only -- it commits/rolls back but NEVER calls .close(). +# Every bare connect() in the original code leaked a file handle until GC. +# +# get_db() fixes four issues at once: +# 1. WAL mode -- readers never block writers; writers never block readers +# 2. synchronous=NORMAL -- safe with WAL, ~3x faster than default FULL +# 3. busy_timeout -- waits up to N seconds instead of raising immediately +# 4. Explicit close -- in finally: block; no leaked handles under any path +# +# check_same_thread=False: BackgroundTasks run on a threadpool worker, not the +# asyncio thread. Each get_db() call creates its own connection so there is no +# actual cross-thread sharing -- the flag disables an overly conservative check. + +from contextlib import contextmanager + +@contextmanager +def get_db(path: str = None, timeout: float = 10.0): + """ + Context manager for the Companion DB. + Drop-in replacement for every `with get_db() as c:`. + + with get_db() as c: + rows = c.execute("SELECT ...").fetchall() + c.execute("INSERT ...") + # auto-committed and closed here + + Raises automatically roll back and re-raise; caller never needs + to call c.commit() or c.close(). + """ + if path is None: + path = DB_PATH + conn = sqlite3.connect(path, timeout=timeout, check_same_thread=False) + try: + conn.execute("PRAGMA journal_mode=WAL") + conn.execute("PRAGMA synchronous=NORMAL") + conn.execute(f"PRAGMA busy_timeout={int(timeout * 1000)}") + yield conn + conn.commit() + except Exception: + conn.rollback() + raise + finally: + conn.close() + + +@contextmanager +def get_navidrome_db(timeout: float = 10.0): + """ + Read-only context manager for Navidrome's SQLite database. + + URI mode=ro: never acquires a write lock, safe while Navidrome is scanning. + Raises FileNotFoundError cleanly if the volume is not mounted. + """ + path = os.getenv("NAVIDROME_DB_PATH", "/navidrome_data/navidrome.db") + if not os.path.isfile(path): + raise FileNotFoundError( + f"Navidrome DB not found at '{path}'. " + "Check NAVIDROME_DB_PATH and your Docker volume mount." + ) + uri = f"file:{path}?mode=ro" + conn = sqlite3.connect(uri, uri=True, timeout=timeout, check_same_thread=False) + try: + conn.execute(f"PRAGMA busy_timeout={int(timeout * 1000)}") + yield conn + finally: + conn.close() + + +# ── DB init ────────────────────────────────────────────────────────────────── # ── Database ──────────────────────────────────────────────────────────────── def init_db(): + """ + Initialise the Companion DB schema. + get_db() sets WAL mode on first open, making it persistent for the file. + All subsequent connections automatically inherit WAL mode. + """ os.makedirs(os.path.dirname(DB_PATH), exist_ok=True) os.makedirs(COVER_ART_DIR, exist_ok=True) os.makedirs(ARTIST_PHOTO_DIR, exist_ok=True) - with sqlite3.connect(DB_PATH) as c: - # Existing tables — untouched + with get_db() as c: + # Existing tables -- untouched c.execute("""CREATE TABLE IF NOT EXISTS dj_profiles ( file_path TEXT PRIMARY KEY, bpm REAL, silence_start REAL, silence_end REAL, loudness_lufs REAL, @@ -77,7 +155,7 @@ def init_db(): basename TEXT, full_path TEXT, title_words TEXT, PRIMARY KEY (basename, full_path))""") - # Phase 1 — authoritative song metadata + # Phase 1 -- authoritative song metadata c.execute("""CREATE TABLE IF NOT EXISTS songs ( id TEXT PRIMARY KEY, full_path TEXT UNIQUE NOT NULL, @@ -183,7 +261,7 @@ def read_tags(full_path: str) -> dict: except Exception: pass - # AIFF files don't support easy=True — fall back to raw ID3 tags + # AIFF files don't support easy=True -- fall back to raw ID3 tags audio_raw = None ext = Path(full_path).suffix.lower() if ext in ('.aiff', '.aif') and (audio_easy is None or not audio_easy): @@ -258,7 +336,7 @@ def scan_library(full_rescan: bool = False) -> int: """Walk MUSIC_DIR and upsert every audio file into the songs table.""" print(f"Library scan started (full={full_rescan})...", flush=True) count = skipped = 0 - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: for root, dirs, files in os.walk(MUSIC_DIR): dirs[:] = [d for d in dirs if not d.startswith('.')] for filename in files: @@ -308,7 +386,7 @@ def scan_library(full_rescan: bool = False) -> int: def build_file_index(): - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: c.execute("DELETE FROM file_index") count = 0 for root, _, files in os.walk(MUSIC_DIR): @@ -337,7 +415,7 @@ def update_song_in_db(full_path: str): except OSError: mtime = fsize = None - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: cur = c.cursor() cur.execute("""UPDATE songs SET title=?, artist=?, album=?, album_artist=?, genre=?, @@ -374,6 +452,11 @@ def update_song_in_db(full_path: str): @asynccontextmanager async def lifespan(app: FastAPI): + global _navidrome_client + _navidrome_client = httpx.AsyncClient( + timeout=httpx.Timeout(connect=10.0, read=30.0, write=10.0, pool=5.0), + limits=httpx.Limits(max_connections=5, max_keepalive_connections=2), + ) init_db() os.makedirs(VIS_CACHE_DIR, exist_ok=True) print(f"Companion API ready", flush=True) @@ -388,9 +471,14 @@ async def lifespan(app: FastAPI): print(f" MUSIC_DIR has {len(dirs)} top-level folders", flush=True) except Exception as e: print(f" Cannot list MUSIC_DIR: {e}", flush=True) - build_file_index() - scan_library() + # Run blocking startup work in a thread so the event loop stays responsive. + # Uvicorn accepts connections during lifespan startup but cannot dispatch them + # until yield -- keeping the loop unblocked allows health checks to queue properly. + await asyncio.to_thread(build_file_index) + await asyncio.to_thread(scan_library) yield + # Graceful shutdown: close the shared httpx client + await _navidrome_client.aclose() app = FastAPI(title="Navidrome Companion API", lifespan=lifespan) @@ -449,21 +537,45 @@ class PushManager: print(f"Client disconnected ({len(self.connections)} total)") async def broadcast(self, event: str, data: dict): - msg = json.dumps({"event": event, "data": data}) - dead = [] - for ws in self.connections: + msg = json.dumps({"event": event, "data": data}) + # Iterate a snapshot so concurrent broadcast() coroutines that both + # detect the same dead socket don't race on list.remove() (AUDIT-018). + # disconnect() uses `if ws in` guard so double-removal is safe. + for ws in list(self.connections): try: await ws.send_text(msg) except Exception: - dead.append(ws) - for ws in dead: - self.connections.remove(ws) + self.disconnect(ws) async def send_to(self, ws: WebSocket, event: str, data: dict): await ws.send_text(json.dumps({"event": event, "data": data})) push = PushManager() +# Strong-reference set for fire-and-forget tasks (AUDIT-013). +# _create_task() returns a Task that the GC can silently collect and +# cancel mid-execution if no reference is held. Store tasks here; the done +# callback removes them automatically when the task completes or raises. +_background_tasks: set = set() + +def _create_task(coro): + """ + Safe replacement for bare _create_task(). + Keeps a strong reference until the task finishes so the GC cannot + cancel it prematurely. Logs unhandled exceptions instead of silencing them. + """ + task = _create_task(coro) + _background_tasks.add(task) + def _on_done(t): + _background_tasks.discard(t) + if not t.cancelled() and t.exception() is not None: + import traceback + print(f"[background task error] {t.get_name()}:", flush=True) + traceback.print_exception(type(t.exception()), t.exception(), + t.exception().__traceback__) + task.add_done_callback(_on_done) + return task + # ── Path resolution ────────────────────────────────────────────────────────── @@ -472,14 +584,14 @@ def resolve_path(relative: str) -> Optional[str]: Resolve a Navidrome-relative path to an absolute filesystem path. Resolution order: - 1. Direct join with MUSIC_DIR — works when paths match exactly - 2. Strip leading path components — handles sub-library prefixes - 3. Companion songs table lookup by relative_path — handles Picard + 1. Direct join with MUSIC_DIR -- works when paths match exactly + 2. Strip leading path components -- handles sub-library prefixes + 3. Companion songs table lookup by relative_path -- handles Picard renames where Navidrome path no longer matches disk structure. This is the key fix: uses album+artist context so two files with the same title (e.g. 'In All the Wrong Places') resolve correctly. - 4. Exact filename match on disk — last resort before fuzzy - 5. Fuzzy title match — lowest confidence, only when nothing else works + 4. Exact filename match on disk -- last resort before fuzzy + 5. Fuzzy title match -- lowest confidence, only when nothing else works """ decoded = relative for _ in range(5): @@ -502,11 +614,11 @@ def resolve_path(relative: str) -> Optional[str]: if os.path.isfile(sub): return sub - # 3. Companion songs table — look up by relative_path. + # 3. Companion songs table -- look up by relative_path. # Also tries matching on title+album+artist to disambiguate files # with identical names in different albums (e.g. compilation tracks). try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: # First try exact relative_path match row = c.execute( "SELECT full_path FROM songs WHERE relative_path = ?", (cleaned,) @@ -580,7 +692,7 @@ def resolve_path(relative: str) -> Optional[str]: print(f" resolve: exact filename -> {found}", flush=True) return found - # 5. Fuzzy title match (lowest confidence — last resort) + # 5. Fuzzy title match (lowest confidence -- last resort) target_stem = Path(target).stem.lower() if target else "" target_ext = Path(target).suffix.lower() if target else "" title_part = re.sub(r'^\d+[\s\.\-]+', '', target_stem).strip() @@ -588,7 +700,7 @@ def resolve_path(relative: str) -> Optional[str]: if len(w) > 1 and not w.isdigit()} if words: try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute( "SELECT basename, full_path, title_words FROM file_index" ).fetchall() @@ -611,7 +723,15 @@ def resolve_path(relative: str) -> Optional[str]: return None -# ── Navidrome helpers ──────────────────────────────────────────────────────── +# ── Navidrome HTTP client ──────────────────────────────────────────────────── +# Single shared AsyncClient reuses the TCP connection to Navidrome across all +# trigger_scan() calls (AUDIT-020). Previously a new client -- and therefore a +# new connection -- was created on every call, adding DNS + TCP + TLS overhead +# on every metadata edit and upload, which is especially costly over Tailscale. +# +# The client is initialised in lifespan() and closed on shutdown. +_navidrome_client: httpx.AsyncClient = None # set in lifespan + async def trigger_scan(): if not all([SUBSONIC_USER, SUBSONIC_TOKEN, SUBSONIC_SALT]): @@ -619,12 +739,22 @@ async def trigger_scan(): return params = {"u": SUBSONIC_USER, "t": SUBSONIC_TOKEN, "s": SUBSONIC_SALT, "v": "1.16.1", "c": "CompanionAPI", "f": "json"} - async with httpx.AsyncClient() as client: - try: - r = await client.get(f"{NAVIDROME_URL}/rest/startScan.view", params=params) - print(f"Navidrome scan: {r.status_code}") - except Exception as e: - print(f"Scan failed: {e}") + client = _navidrome_client + if client is None or client.is_closed: + # Fallback: create a one-shot client if the shared one isn't ready + async with httpx.AsyncClient(timeout=10) as _c: + try: + r = await _c.get(f"{NAVIDROME_URL}/rest/startScan.view", params=params) + print(f"Navidrome scan (fallback client): {r.status_code}") + except Exception as e: + print(f"Scan failed: {e}") + return + try: + r = await client.get(f"{NAVIDROME_URL}/rest/startScan.view", params=params, + timeout=10) + print(f"Navidrome scan: {r.status_code}") + except Exception as e: + print(f"Scan failed: {e}") async def sync_navidrome_ids_task(): @@ -632,10 +762,10 @@ async def sync_navidrome_ids_task(): Fetch all songs from Navidrome and match them into our songs table. Matching strategy (tried in order per song): - 1. title + artist — primary, both read from same ID3 tags - 2. title + album — fallback when artist field differs - 3. title only — fallback for unique titles - 4. duration bucket — last resort (±2s tolerance, unique per bucket) + 1. title + artist -- primary, both read from same ID3 tags + 2. title + album -- fallback when artist field differs + 3. title only -- fallback for unique titles + 4. duration bucket -- last resort (±2s tolerance, unique per bucket) """ try: if not all([SUBSONIC_USER, SUBSONIC_TOKEN, SUBSONIC_SALT]): @@ -651,28 +781,30 @@ async def sync_navidrome_ids_task(): } all_songs = [] offset = 0 - async with httpx.AsyncClient(timeout=60) as client: - while True: - try: - r = await client.get( - f"{NAVIDROME_URL}/rest/search3.view", - params={**base_params, "songOffset": offset} - ) - resp = r.json().get("subsonic-response", {}) - if resp.get("status") == "failed": - print(f" Navidrome auth error: {resp.get('error')}", flush=True) - return - songs = resp.get("searchResult3", {}).get("song", []) - print(f" Page offset={offset}: {len(songs)} songs", flush=True) - if not songs: - break - all_songs.extend(songs) - offset += len(songs) - if len(songs) < 500: - break - except Exception as e: - print(f" Navidrome fetch error: {e}", flush=True) + # Reuse the shared client; 60s read timeout for large library pagination + http = _navidrome_client + while True: + try: + r = await http.get( + f"{NAVIDROME_URL}/rest/search3.view", + params={**base_params, "songOffset": offset}, + timeout=httpx.Timeout(connect=10.0, read=60.0, write=10.0, pool=5.0) + ) + resp = r.json().get("subsonic-response", {}) + if resp.get("status") == "failed": + print(f" Navidrome auth error: {resp.get('error')}", flush=True) + return + songs = resp.get("searchResult3", {}).get("song", []) + print(f" Page offset={offset}: {len(songs)} songs", flush=True) + if not songs: break + all_songs.extend(songs) + offset += len(songs) + if len(songs) < 500: + break + except Exception as e: + print(f" Navidrome fetch error: {e}", flush=True) + break print(f" Navidrome total: {len(all_songs)} songs", flush=True) if not all_songs: @@ -706,7 +838,7 @@ async def sync_navidrome_ids_task(): return None return int(round(float(seconds) / 2.0)) - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: cur = c.cursor() db_rows = cur.execute( "SELECT id, title, artist, album, duration FROM songs" @@ -803,73 +935,80 @@ async def sync_navidrome_ids_task(): matched_s5 = matched_s6 = matched_s7 = unmatched = 0 unmatched_samples = [] - with sqlite3.connect(DB_PATH) as c: - cur = c.cursor() - for ns in all_songs: - nd_id = ns.get("id", "") - if not nd_id: - continue + # Build the update list entirely in Python (pure dict lookups -- fast, no I/O), + # then write to SQLite in a single executemany call. + # This avoids holding the DB connection open for the entire iteration AND + # never blocks the event loop with thousands of individual execute() calls + # without yielding (AUDIT-023). + updates = [] # list of (navidrome_id, navidrome_album_id, companion_song_id) + for ns in all_songs: + nd_id = ns.get("id", "") + if not nd_id: + continue - nt = norm(ns.get("title", "")) - na = norm(ns.get("artist", "")) - nb = norm(ns.get("album", "")) - ct = clean_title(ns.get("title", "")) - dk = dur_bucket(ns.get("duration")) + nt = norm(ns.get("title", "")) + na = norm(ns.get("artist", "")) + nb = norm(ns.get("album", "")) + ct = clean_title(ns.get("title", "")) + dk = dur_bucket(ns.get("duration")) - db_song_id = None - strategy = 0 + db_song_id = None + strategy = 0 - if not db_song_id: - hit = by_title_artist.get((nt, na)) - if hit: db_song_id, strategy = hit, 1 + if not db_song_id: + hit = by_title_artist.get((nt, na)) + if hit: db_song_id, strategy = hit, 1 - if not db_song_id: - hit = by_title_album.get((nt, nb)) - if hit: db_song_id, strategy = hit, 2 + if not db_song_id: + hit = by_title_album.get((nt, nb)) + if hit: db_song_id, strategy = hit, 2 - if not db_song_id: - hit = by_title.get(nt) - if hit: db_song_id, strategy = hit, 3 + if not db_song_id: + hit = by_title.get(nt) + if hit: db_song_id, strategy = hit, 3 - if not db_song_id and dk is not None: - hit = by_dur.get((dk, nt[:8])) - if hit: db_song_id, strategy = hit, 4 + if not db_song_id and dk is not None: + hit = by_dur.get((dk, nt[:8])) + if hit: db_song_id, strategy = hit, 4 - if not db_song_id: - hit = by_clean_artist.get((ct, na)) - if hit: db_song_id, strategy = hit, 5 + if not db_song_id: + hit = by_clean_artist.get((ct, na)) + if hit: db_song_id, strategy = hit, 5 - if not db_song_id and dk is not None: - hit = by_dur_only.get(dk) - if hit: db_song_id, strategy = hit, 6 + if not db_song_id and dk is not None: + hit = by_dur_only.get(dk) + if hit: db_song_id, strategy = hit, 6 - # S7: clean title + duration bucket — catches untagged AIFF/files - # where artist is unknown but filename+duration uniquely identify song - if not db_song_id and dk is not None: - hit = by_clean_dur.get((ct, dk)) - if hit: db_song_id, strategy = hit, 7 + if not db_song_id and dk is not None: + hit = by_clean_dur.get((ct, dk)) + if hit: db_song_id, strategy = hit, 7 - if db_song_id: - nd_album_id = ns.get("albumId", "") - cur.execute( - "UPDATE songs SET navidrome_id = ?, navidrome_album_id = ? WHERE id = ?", - (nd_id, nd_album_id, db_song_id) + if db_song_id: + nd_album_id = ns.get("albumId", "") + updates.append((nd_id, nd_album_id, db_song_id)) + if strategy == 1: matched_s1 += 1 + elif strategy == 2: matched_s2 += 1 + elif strategy == 3: matched_s3 += 1 + elif strategy == 4: matched_s4 += 1 + elif strategy == 5: matched_s5 += 1 + elif strategy == 6: matched_s6 += 1 + else: matched_s7 += 1 + else: + unmatched += 1 + if len(unmatched_samples) < 10: + unmatched_samples.append( + f"title={repr(ns.get('title',''))} " + f"artist={repr(ns.get('artist',''))} " + f"duration={ns.get('duration')}" ) - if strategy == 1: matched_s1 += 1 - elif strategy == 2: matched_s2 += 1 - elif strategy == 3: matched_s3 += 1 - elif strategy == 4: matched_s4 += 1 - elif strategy == 5: matched_s5 += 1 - elif strategy == 6: matched_s6 += 1 - else: matched_s7 += 1 - else: - unmatched += 1 - if len(unmatched_samples) < 10: - unmatched_samples.append( - f"title={repr(ns.get('title',''))} " - f"artist={repr(ns.get('artist',''))} " - f"duration={ns.get('duration')}" - ) + + # Single batched write -- one connection open, one executemany, one commit + with get_db() as c: + c.executemany( + "UPDATE songs SET navidrome_id = ?, navidrome_album_id = ? WHERE id = ?", + updates + ) + print(f" Wrote {len(updates)} navidrome_id matches to DB", flush=True) total_matched = matched_s1+matched_s2+matched_s3+matched_s4+matched_s5+matched_s6+matched_s7 print(f"Navidrome ID sync complete: {total_matched}/{len(all_songs)} matched", flush=True) @@ -956,9 +1095,9 @@ def enforce_tag_whitelist( # Never remove non-empty COMPOSER/LYRICS even if not in allowed # (preserve existing values the user may have set manually) elif ku == 'COMPOSER' and not f[k][0].strip(): - to_remove.append(k) # blank composer — remove + to_remove.append(k) # blank composer -- remove elif ku == 'LYRICS' and not f[k][0].strip(): - to_remove.append(k) # blank lyrics — remove + to_remove.append(k) # blank lyrics -- remove result["removed"] = to_remove result["kept"] = [k for k in f.keys() if k not in to_remove] if not dry_run and to_remove: @@ -990,7 +1129,7 @@ def enforce_tag_whitelist( def clean_picard_tags(full_path: str, dry_run: bool = False) -> dict: - """Legacy blacklist cleaner — now delegates to whitelist enforcer.""" + """Legacy blacklist cleaner -- now delegates to whitelist enforcer.""" return enforce_tag_whitelist(full_path, dry_run=dry_run) @@ -1032,7 +1171,7 @@ def build_target_path(full_path: str) -> Optional[str]: album = sanitize(get('album') or 'Unknown Album') ext = Path(full_path).suffix.lower() - # Track number — strip /total if present (e.g. "3/12" -> 3) + # Track number -- strip /total if present (e.g. "3/12" -> 3) track_num = 0 raw_track = get('tracknumber') if raw_track: @@ -1060,7 +1199,7 @@ def build_target_path(full_path: str) -> Optional[str]: has_disc_siblings = any(re.match(r'\d{2}-\d{2}', f) for f in siblings) if not has_disc_siblings: print(f" disc sanity: no disc siblings for {os.path.basename(full_path)}" - f" — overriding disc {disc_num} -> 1", flush=True) + f" -- overriding disc {disc_num} -> 1", flush=True) disc_num = 1 except Exception: pass @@ -1117,12 +1256,12 @@ def restructure_file(full_path: str) -> Optional[str]: else: break - # Update DB with new path — re-read tags for accurate sort keys + # Update DB with new path -- re-read tags for accurate sort keys new_relative = os.path.relpath(target, MUSIC_DIR) song_id = hashlib.md5(full_path.encode()).hexdigest() new_id = hashlib.md5(target.encode()).hexdigest() tags = read_tags(target) - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: cur = c.cursor() cur.execute("""UPDATE songs SET id=?, full_path=?, relative_path=?, @@ -1139,7 +1278,7 @@ def restructure_file(full_path: str) -> Optional[str]: song_id )) if cur.rowcount == 0: - # Row used old full_path as key — try matching by path + # Row used old full_path as key -- try matching by path cur.execute("""UPDATE songs SET id=?, full_path=?, relative_path=?, sort_title=?, sort_artist=?, sort_album=?, sort_album_artist=?, @@ -1168,13 +1307,13 @@ def restructure_all() -> dict: moved = 0 skipped = 0 failed = 0 - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute("SELECT full_path FROM songs").fetchall() for (full_path,) in rows: if not os.path.isfile(full_path): skipped += 1 continue - # Enforce whitelist before restructuring — clean tags first so + # Enforce whitelist before restructuring -- clean tags first so # build_target_path reads clean data and generates the correct path enforce_tag_whitelist(full_path, preserve_composer=True, preserve_lyrics=True) result = restructure_file(full_path) @@ -1187,7 +1326,7 @@ def restructure_all() -> dict: def apply_tags(path: str, u, preserve_composer: bool = True, preserve_lyrics: bool = True): - """Write tags then enforce whitelist — only Navidrome tags survive.""" + """Write tags then enforce whitelist -- only Navidrome tags survive.""" audio = MutagenFile(path, easy=True) if audio is None: raise ValueError(f"Unsupported format: {path}") @@ -1199,12 +1338,12 @@ def apply_tags(path: str, u, preserve_composer: bool = True, preserve_lyrics: bo if u.year: audio['date'] = str(u.year) if u.track_number: audio['tracknumber'] = str(u.track_number) audio.save() - # Enforce whitelist after writing — nukes everything not in NAVIDROME_TAGS + # Enforce whitelist after writing -- nukes everything not in NAVIDROME_TAGS enforce_tag_whitelist(path, preserve_composer=preserve_composer, preserve_lyrics=preserve_lyrics) def apply_tags_dict(path: str, tags: dict, preserve_composer: bool = True, preserve_lyrics: bool = True): - """Write tags dict then enforce whitelist — only Navidrome tags survive.""" + """Write tags dict then enforce whitelist -- only Navidrome tags survive.""" audio = MutagenFile(path, easy=True) if audio is None: raise ValueError(f"Unsupported format: {path}") @@ -1226,7 +1365,19 @@ def analyze(full_path: str) -> dict: import librosa cmd = ["ffmpeg", "-hide_banner", "-i", full_path, "-af", "silencedetect=noise=-50dB:d=0.5,ebur128", "-f", "null", "-"] - out = subprocess.run(cmd, capture_output=True, text=True, timeout=120).stderr + try: + proc = subprocess.run(cmd, capture_output=True, text=True, timeout=120) + out = proc.stderr + except subprocess.TimeoutExpired as exc: + # Kill the ffmpeg process so it doesn't continue as an orphan consuming + # CPU on the Pi (AUDIT-017). exc.process is set by subprocess.run. + if exc.process is not None: + exc.process.kill() + exc.process.communicate() # reap zombie + raise RuntimeError( + f"ffmpeg timed out after 120s analysing {os.path.basename(full_path)}. " + "File may be corrupt or extremely long." + ) from exc all_starts = re.findall(r"silence_start: ([\d\.]+)", out) all_ends = re.findall(r"silence_end: ([\d\.]+)", out) @@ -1252,7 +1403,8 @@ def analyze(full_path: str) -> dict: y, sr = librosa.load(full_path, sr=22050, duration=30) tempo, _ = librosa.beat.beat_track(y=y, sr=sr) del y - import gc; gc.collect() + # gc.collect() removed: causes a stop-the-world GC pause on the event loop + # (AUDIT-014). del y drops the refcount to 0; CPython frees it immediately. try: bpm = float(tempo) except TypeError: @@ -1260,7 +1412,7 @@ def analyze(full_path: str) -> dict: profile = {"bpm": round(bpm, 1), "silence_start": round(sil_start, 3), "silence_end": round(sil_end, 3), "loudness_lufs": round(loudness, 1)} - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: c.execute("INSERT OR REPLACE INTO dj_profiles VALUES (?,?,?,?,?,CURRENT_TIMESTAMP)", (full_path, profile["bpm"], profile["silence_start"], profile["silence_end"], profile["loudness_lufs"])) @@ -1271,7 +1423,10 @@ def analyze(full_path: str) -> dict: def gen_vis_frames(path: str, fps: float = 30.0, fft_size: int = 1024, pts: int = 20) -> list: import librosa - y, sr = librosa.load(path, sr=22050, mono=True) + # Cap at 600s to prevent OOM on long recordings (AUDIT-015). + # A 10-min song at 22050 Hz mono = ~26MB; without cap a 1hr file = ~318MB. + MAX_DURATION = 600.0 + y, sr = librosa.load(path, sr=22050, mono=True, duration=MAX_DURATION) hop = max(1, int(sr / fps)) frames = [] for start in range(0, len(y) - fft_size, hop): @@ -1291,7 +1446,7 @@ def gen_vis_frames(path: str, fps: float = 30.0, fft_size: int = 1024, pts: int fp.append(avg) # no eqBoost frames.append(fp) del y - import gc; gc.collect() + # gc.collect() removed (AUDIT-014) -- del y is sufficient for numpy arrays vals = sorted(v for f in frames for v in f if v > 0.001) if vals: p95 = vals[min(int(len(vals) * 0.95), len(vals) - 1)] @@ -1376,7 +1531,7 @@ def song_row_to_dict(row) -> dict: async def health(): pc = fc = sc = 0 try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: pc = c.execute("SELECT COUNT(*) FROM dj_profiles").fetchone()[0] fc = c.execute("SELECT COUNT(*) FROM file_index").fetchone()[0] sc = c.execute("SELECT COUNT(*) FROM songs").fetchone()[0] @@ -1390,7 +1545,7 @@ async def health(): @app.post("/reindex") async def reindex(): - build_file_index() + await asyncio.to_thread(build_file_index) return {"status": "reindexed"} @@ -1400,12 +1555,10 @@ async def edit_metadata(update: MetadataUpdate): if not fp: raise HTTPException(404, f"File not found. raw='{update.relative_path}' MUSIC_DIR='{MUSIC_DIR}'") try: - apply_tags(fp, update) - # Do NOT restructure on single-track edits — restructuring renames the - # file based on all current tags including disc number, which causes - # wrong filenames for compilation tracks (e.g. disc 2 gets 02-13 prefix). - # Restructuring is only done via the explicit bulk-fix endpoint. - update_song_in_db(fp) + # Offload blocking Mutagen I/O to a thread -- audio.save() on FLAC can + # take 100-500ms and must not block the event loop (AUDIT-009) + await asyncio.to_thread(apply_tags, fp, update) + await asyncio.to_thread(update_song_in_db, fp) new_relative = os.path.relpath(fp, MUSIC_DIR) await trigger_scan() await push.broadcast("metadata_updated", { @@ -1430,18 +1583,21 @@ async def batch_edit_metadata(update: BatchMetadataUpdate): if update.album_artist: tags["album_artist"] = update.album_artist if update.genre: tags["genre"] = update.genre if update.year: tags["year"] = str(update.year) - for rp in update.relative_paths: - fp = resolve_path(rp) - if not fp: - results["failed"].append({"path": rp, "error": "File not found"}) - continue - try: - apply_tags_dict(fp, tags) - # Fix #1: No auto-restructure on tag edits. Only via /bulk-fix. - update_song_in_db(fp) - results["succeeded"].append(os.path.relpath(fp, MUSIC_DIR)) - except Exception as e: - results["failed"].append({"path": rp, "error": str(e)}) + def _apply_batch(): + """Blocking tag writes run in a thread so the event loop stays free.""" + for rp in update.relative_paths: + fp = resolve_path(rp) + if not fp: + results["failed"].append({"path": rp, "error": "File not found"}) + continue + try: + apply_tags_dict(fp, tags) + update_song_in_db(fp) + results["succeeded"].append(os.path.relpath(fp, MUSIC_DIR)) + except Exception as e: + results["failed"].append({"path": rp, "error": str(e)}) + + await asyncio.to_thread(_apply_batch) await trigger_scan() # Wait for Navidrome to finish scanning before the app re-fetches paths. await asyncio.sleep(4) @@ -1450,7 +1606,7 @@ async def batch_edit_metadata(update: BatchMetadataUpdate): "album": update.album or "", "paths_changed": "true"}) # Run conflict check in background after every batch edit - asyncio.create_task(_run_conflict_check_and_broadcast()) + _create_task(_run_conflict_check_and_broadcast()) return results @@ -1471,16 +1627,26 @@ async def upload_track( try: u = MetadataUpdate(relative_path=f"uploads/{file.filename}", title=title, artist=artist, album=album) - apply_tags(fp, u, preserve_composer=preserve_composer, preserve_lyrics=preserve_lyrics) - update_song_in_db(fp) - profile = analyze(fp) + await asyncio.to_thread(apply_tags, fp, u, + preserve_composer=preserve_composer, + preserve_lyrics=preserve_lyrics) + await asyncio.to_thread(update_song_in_db, fp) + profile = await asyncio.to_thread(analyze, fp) await trigger_scan() await push.broadcast("track_uploaded", {"filename": file.filename, "profile": json.dumps(profile)}) return {"status": "uploaded", "path": f"uploads/{file.filename}", "profile": profile} except Exception as e: + # Clean up both the file AND the DB row that update_song_in_db already + # wrote -- leaving an orphaned row pointing to a deleted file (AUDIT-017) if os.path.exists(fp): os.remove(fp) + song_id = hashlib.md5(fp.encode()).hexdigest() + try: + with get_db() as c: + c.execute("DELETE FROM songs WHERE id = ?", (song_id,)) + except Exception: + pass raise HTTPException(500, str(e)) @@ -1535,10 +1701,10 @@ async def upload_tracks( preserve_composer=preserve_composer, preserve_lyrics=preserve_lyrics) try: - analyze(fp) + await asyncio.to_thread(analyze, fp) except Exception as e: print(f" Analysis failed for {file.filename}: {e}") - update_song_in_db(fp) + await asyncio.to_thread(update_song_in_db, fp) results["uploaded"].append({ "filename": file.filename, "path": os.path.relpath(fp, MUSIC_DIR) @@ -1553,7 +1719,7 @@ async def upload_tracks( try: with open(cover_dest, "wb") as buf: shutil.copyfileobj(cover_art.file, buf) - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: c.execute("UPDATE songs SET cover_art_path = ? WHERE full_path LIKE ?", (cover_dest, os.path.join(album_dir, "%"))) print(f" Cover art saved: {cover_dest}", flush=True) @@ -1573,7 +1739,7 @@ async def get_profile(relative_path: str): fp = resolve_path(relative_path) if fp: try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: row = c.execute( "SELECT bpm,silence_start,silence_end,loudness_lufs " "FROM dj_profiles WHERE file_path=?", (fp,) @@ -1583,7 +1749,7 @@ async def get_profile(relative_path: str): "silence_end": row[2], "loudness_lufs": row[3]} except sqlite3.OperationalError as e: print(f"DB error: {e}", flush=True) - return analyze(fp) + return await asyncio.to_thread(analyze, fp) decoded = relative_path for _ in range(5): @@ -1593,7 +1759,7 @@ async def get_profile(relative_path: str): target = os.path.basename(decoded).lower() if target: try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute( "SELECT file_path,bpm,silence_start,silence_end,loudness_lufs " "FROM dj_profiles WHERE LOWER(file_path) LIKE ?", (f"%{target}",) @@ -1615,14 +1781,16 @@ async def bulk_profiles(paths: str = Query(...)): results[rp] = None continue try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: row = c.execute( "SELECT bpm,silence_start,silence_end,loudness_lufs " "FROM dj_profiles WHERE file_path=?", (fp,) ).fetchone() - results[rp] = ({"bpm": row[0], "silence_start": row[1], - "silence_end": row[2], "loudness_lufs": row[3]} - if row else analyze(fp)) + if row: + results[rp] = {"bpm": row[0], "silence_start": row[1], + "silence_end": row[2], "loudness_lufs": row[3]} + else: + results[rp] = await asyncio.to_thread(analyze, fp) except Exception: results[rp] = None return results @@ -1633,7 +1801,9 @@ async def vis_frames(relative_path: str): fp = resolve_path(relative_path) if not fp: raise HTTPException(404, "Not found") - frames = get_vis(fp) + # gen_vis_frames() loads the full audio file via librosa (~20MB+ for a 5min song) + # and runs FFT on every frame -- must not block the event loop (AUDIT-008) + frames = await asyncio.to_thread(get_vis, fp) if not frames: raise HTTPException(500, "Generation failed") return {"frame_count": len(frames), "fps": 30.0, "points": 20, "frames": frames} @@ -1655,7 +1825,12 @@ async def precompute(background_tasks: BackgroundTasks, relative_path: str = "") pass print(f"Pre-computed {n} vis caches") if relative_path: - background_tasks.add_task(lambda: get_vis(resolve_path(relative_path) or "")) + fp = resolve_path(relative_path) + if not fp: + raise HTTPException(404, f"Cannot resolve path: {relative_path!r}") + # Pass fp directly -- avoids the lambda capturing relative_path and + # silently calling get_vis("") if resolve fails later (AUDIT-016) + background_tasks.add_task(get_vis, fp) return {"message": f"Computing: {relative_path}"} background_tasks.add_task(compute_all) return {"message": "Background vis computation started"} @@ -1668,9 +1843,9 @@ async def bulk_fix(background_tasks: BackgroundTasks, dry_run: bool = False): ?dry_run=true returns a list of {from, to} moves without executing them. """ if dry_run: - # Preview only — return what would move + # Preview only -- return what would move moves = [] - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute("SELECT full_path FROM songs").fetchall() for (full_path,) in rows: if not os.path.isfile(full_path): @@ -1684,10 +1859,12 @@ async def bulk_fix(background_tasks: BackgroundTasks, dry_run: bool = False): return {"dry_run": True, "moves": len(moves), "preview": moves} async def run(): - result = restructure_all() + # restructure_all() calls shutil.move + MutagenFile + SQLite for every song. + # Without to_thread this blocks the entire event loop for minutes (AUDIT-010). + result = await asyncio.to_thread(restructure_all) await trigger_scan() await asyncio.sleep(4) - asyncio.create_task(_run_conflict_check_and_broadcast()) + _create_task(_run_conflict_check_and_broadcast()) await push.broadcast("library_restructured", result) background_tasks.add_task(run) return {"message": "Library restructure started"} @@ -1739,7 +1916,7 @@ async def library_songs( if year: wheres.append("year = ?"); params.append(year) where = f"WHERE {' AND '.join(wheres)}" if wheres else "" - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: total = c.execute(f"SELECT COUNT(*) FROM songs {where}", params).fetchone()[0] rows = c.execute( f"SELECT {SONG_COLS} FROM songs {where} ORDER BY {order} LIMIT ? OFFSET ?", @@ -1762,7 +1939,7 @@ async def library_albums( if genre: wheres.append("genre = ?"); params.append(genre) where = f"WHERE {' AND '.join(wheres)}" if wheres else "" - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute(f""" SELECT album, album_artist, sort_album, sort_album_artist, MIN(year) as year, COUNT(*) as track_count, @@ -1788,7 +1965,7 @@ async def library_albums( @app.get("/library/artists") async def library_artists(): - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute(""" SELECT artist, sort_artist, COUNT(*) as track_count, COUNT(DISTINCT album) as album_count @@ -1813,7 +1990,7 @@ async def library_search( limit: int = Query(50, ge=1, le=200) ): term = f"%{q}%" - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute(f""" SELECT {SONG_COLS} FROM songs WHERE title LIKE ? OR artist LIKE ? OR album LIKE ? OR genre LIKE ? @@ -1825,7 +2002,7 @@ async def library_search( @app.get("/library/song/{song_id}") async def library_song(song_id: str): - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: row = c.execute(f"SELECT {SONG_COLS} FROM songs WHERE id = ?", (song_id,)).fetchone() if not row: raise HTTPException(404, "Song not found") @@ -1834,7 +2011,7 @@ async def library_song(song_id: str): @app.get("/library/cover-art/{song_id}") async def library_cover_art(song_id: str): - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: row = c.execute("SELECT cover_art_path FROM songs WHERE id = ?", (song_id,)).fetchone() if not row or not row[0] or not os.path.isfile(row[0]): raise HTTPException(404, "No cover art") @@ -1844,8 +2021,8 @@ async def library_cover_art(song_id: str): @app.post("/library/cover-art/{song_id}") async def upload_cover_art(song_id: str, file: UploadFile = File(...)): - """Upload cover art — saves as cover.jpg and updates all songs in that directory.""" - with sqlite3.connect(DB_PATH) as c: + """Upload cover art -- saves as cover.jpg and updates all songs in that directory.""" + with get_db() as c: row = c.execute("SELECT full_path FROM songs WHERE id = ?", (song_id,)).fetchone() if not row: raise HTTPException(404, "Song not found") @@ -1854,14 +2031,16 @@ async def upload_cover_art(song_id: str, file: UploadFile = File(...)): try: with open(cover_dest, "wb") as buf: shutil.copyfileobj(file.file, buf) - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: c.execute("UPDATE songs SET cover_art_path = ? WHERE full_path LIKE ?", (cover_dest, os.path.join(song_dir, "%"))) - # Clear any cached extracted cover art for all songs in this directory - for (sid,) in sqlite3.connect(DB_PATH).execute( - "SELECT id FROM songs WHERE full_path LIKE ?", - (os.path.join(song_dir, "%"),) - ).fetchall(): + # Clear any cached extracted cover art for all songs in this directory. + # Merged into the same connection: no leaked handle, one fewer open/close. + sids = [r[0] for r in c.execute( + "SELECT id FROM songs WHERE full_path LIKE ?", + (os.path.join(song_dir, "%"),) + ).fetchall()] + for sid in sids: cached = os.path.join(COVER_ART_DIR, f"{sid}.jpg") if os.path.isfile(cached): os.remove(cached) @@ -1874,7 +2053,7 @@ async def upload_cover_art(song_id: str, file: UploadFile = File(...)): @app.delete("/library/cover-art/{song_id}") async def delete_cover_art(song_id: str): """Remove cover.jpg from the album directory and clear cover_art_path in DB.""" - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: row = c.execute("SELECT full_path FROM songs WHERE id = ?", (song_id,)).fetchone() if not row: raise HTTPException(404, "Song not found") @@ -1883,14 +2062,14 @@ async def delete_cover_art(song_id: str): try: if os.path.isfile(cover_path): os.remove(cover_path) - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: c.execute("UPDATE songs SET cover_art_path = NULL WHERE full_path LIKE ?", (os.path.join(song_dir, "%"),)) - # Clear cached extracted covers for all songs in directory - for (sid,) in sqlite3.connect(DB_PATH).execute( - "SELECT id FROM songs WHERE full_path LIKE ?", - (os.path.join(song_dir, "%"),) - ).fetchall(): + sids = [r[0] for r in c.execute( + "SELECT id FROM songs WHERE full_path LIKE ?", + (os.path.join(song_dir, "%"),) + ).fetchall()] + for sid in sids: cached = os.path.join(COVER_ART_DIR, f"{sid}.jpg") if os.path.isfile(cached): os.remove(cached) @@ -1912,7 +2091,7 @@ async def upload_artist_photo( try: with open(dest, "wb") as buf: shutil.copyfileobj(file.file, buf) - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: c.execute("""INSERT OR REPLACE INTO artist_photos (artist_name, photo_path, updated_at) VALUES (?,?,CURRENT_TIMESTAMP)""", (artist_name, dest)) @@ -1924,7 +2103,7 @@ async def upload_artist_photo( @app.get("/library/artist-photo/{artist_name}") async def get_artist_photo(artist_name: str): - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: row = c.execute( "SELECT photo_path FROM artist_photos WHERE artist_name = ?", (artist_name,) ).fetchone() @@ -1939,13 +2118,10 @@ async def auto_fix_duplicate_albums(): Fix #9: After every Navidrome scan, detect duplicate album entries (same name, different album_artist) and rewrite tags on minority files so Navidrome groups them correctly on the next scan. - Uses tag rewrites only — never writes to Navidrome's DB directly. + Uses tag rewrites only -- never writes to Navidrome's DB directly. """ - navidrome_db = os.getenv("NAVIDROME_DB_PATH", "/navidrome_data/navidrome.db") - if not os.path.isfile(navidrome_db): - return try: - with sqlite3.connect(navidrome_db) as nav: + with get_navidrome_db() as nav: rows = nav.execute(""" SELECT name, COUNT(DISTINCT album_artist) as aa_count, GROUP_CONCAT(id, '|||') as ids, @@ -1971,7 +2147,7 @@ async def auto_fix_duplicate_albums(): canonical_artist = artist_list[canonical_idx] # Find all files in Companion DB for this album - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: file_rows = c.execute( "SELECT full_path, album_artist FROM songs WHERE album = ?", (name,) ).fetchall() @@ -2019,8 +2195,10 @@ async def _run_conflict_check_and_broadcast(): await asyncio.sleep(6) # Wait for Navidrome scan to complete # Auto-fix duplicate albums before reporting conflicts await auto_fix_duplicate_albums() + # Picard tag check opens every FLAC file; stale-path check stats every song. + # Runs in thread to avoid blocking the event loop (AUDIT-011). navidrome_db = os.getenv("NAVIDROME_DB_PATH", "/navidrome_data/navidrome.db") - issues = check_library_conflicts(navidrome_db) + issues = await asyncio.to_thread(check_library_conflicts, navidrome_db) error_count = sum(1 for i in issues if i["severity"] == "error") warning_count = sum(1 for i in issues if i["severity"] == "warning") await push.broadcast("conflicts_updated", { @@ -2045,7 +2223,7 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT # ── 1. Duplicate albums (same name, multiple album_artist values) ───────── try: - with sqlite3.connect(navidrome_db_path) as nav: + with get_navidrome_db() as nav: rows = nav.execute(""" SELECT name, COUNT(DISTINCT album_artist) as aa_count, GROUP_CONCAT(id, '|||') as ids, @@ -2079,7 +2257,7 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT # ── 2. Missing files (Navidrome knows about them but they're gone) ──────── try: - with sqlite3.connect(navidrome_db_path) as nav: + with get_navidrome_db() as nav: rows = nav.execute( "SELECT path FROM media_file WHERE missing = 1 LIMIT 50" ).fetchall() @@ -2099,7 +2277,7 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT # ── 3. Picard legacy tags (FLAC files with conflicting albumartist tags) ── try: legacy_files = [] - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute("SELECT full_path FROM songs").fetchall() for (full_path,) in rows: if not full_path.lower().endswith('.flac'): @@ -2131,7 +2309,7 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT # ── 4. Orphaned tracks (album_id points to non-existent album) ──────────── try: - with sqlite3.connect(navidrome_db_path) as nav: + with get_navidrome_db() as nav: rows = nav.execute(""" SELECT mf.path FROM media_file mf LEFT JOIN album a ON mf.album_id = a.id @@ -2153,7 +2331,7 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT # ── 5. Duplicate tracks (same title+artist+duration appearing >1 time) ─── try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute(""" SELECT title, artist, COUNT(*) as cnt, GROUP_CONCAT(relative_path, '|||') as paths @@ -2168,7 +2346,7 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT issues.append({ "type": "duplicate_track", "severity": "warning", - "title": f"Duplicate: {title} — {artist}", + "title": f"Duplicate: {title} -- {artist}", "detail": f"Found {cnt} copies of this track.", "affected_paths": path_list, "fix_action": None, @@ -2180,7 +2358,7 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT # ── 6. Stale Companion paths (full_path no longer exists on disk) ───────── try: stale = [] - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute("SELECT relative_path, full_path FROM songs").fetchall() for rel, full in rows: if not os.path.isfile(full): @@ -2199,10 +2377,10 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT print(f" conflict check 6 failed: {e}", flush=True) # ── 7. Album identity reassignment (Fix #13) ──────────────────────────── - # Detect tracks whose Navidrome album_id changed since last sync — + # Detect tracks whose Navidrome album_id changed since last sync -- # indicates Navidrome reassigned them to a different album entry. try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute(""" SELECT relative_path, navidrome_album_id, album, album_artist FROM songs @@ -2249,7 +2427,6 @@ def check_library_conflicts(navidrome_db_path: str = os.getenv("NAVIDROME_DB_PAT @app.get("/library/conflicts") async def library_conflicts(): """Run all conflict checks and return structured results.""" - navidrome_db = os.getenv("NAVIDROME_DB_PATH", "/navidrome_data/navidrome.db") issues = check_library_conflicts(navidrome_db) error_count = sum(1 for i in issues if i["severity"] == "error") warning_count = sum(1 for i in issues if i["severity"] == "warning") @@ -2267,7 +2444,6 @@ async def fix_conflict(request: FixConflictRequest): Fix a specific conflict by action type. Request body: {"action": "fix_duplicate_album", "fix_data": {...}} """ - navidrome_db = os.getenv("NAVIDROME_DB_PATH", "/navidrome_data/navidrome.db") action = request.action fix_data = request.fix_data @@ -2291,7 +2467,7 @@ async def fix_conflict(request: FixConflictRequest): canonical_artist = album_artists[canonical_idx] try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute( "SELECT full_path FROM songs WHERE album = ?", (album_name,) ).fetchall() @@ -2330,7 +2506,7 @@ async def fix_conflict(request: FixConflictRequest): raise HTTPException(500, f"Fix failed: {e}") elif action == "fix_missing_files": - # Trigger a full Navidrome rescan — Navidrome will detect and remove + # Trigger a full Navidrome rescan -- Navidrome will detect and remove # missing files automatically during a full scan. We cannot write to # Navidrome's DB directly while it is running (mounted read-only). try: @@ -2342,49 +2518,54 @@ async def fix_conflict(request: FixConflictRequest): elif action == "fix_picard_tags": fixed = 0 failed = [] - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute("SELECT full_path FROM songs WHERE full_path LIKE '%.flac'").fetchall() - for (full_path,) in rows: - if not os.path.isfile(full_path): - continue - try: - from mutagen.flac import FLAC - f = FLAC(full_path) - keys = list(f.keys()) - upper_keys = [k.upper() for k in keys] - has_legacy = any(k in upper_keys for k in ['ALBUM ARTIST', 'ALBUM_ARTIST']) - if not has_legacy: + def _fix_picard_files(): + _fixed = 0 + _failed = [] + for (full_path,) in rows: + if not os.path.isfile(full_path): continue - # Get canonical value - canonical = None - for k in keys: - if k.upper() == 'ALBUMARTIST': - canonical = f[k][0] if f[k] else None - break - if not canonical: + try: + from mutagen.flac import FLAC + f = FLAC(full_path) + keys = list(f.keys()) + upper_keys = [k.upper() for k in keys] + has_legacy = any(k in upper_keys for k in ['ALBUM ARTIST', 'ALBUM_ARTIST']) + if not has_legacy: + continue + canonical = None for k in keys: - if k.upper() in ['ALBUM ARTIST', 'ALBUM_ARTIST']: + if k.upper() == 'ALBUMARTIST': canonical = f[k][0] if f[k] else None break - if not canonical: - continue - # Remove all variants and write canonical - for k in list(f.keys()): - if k.upper() in ['ALBUM ARTIST', 'ALBUM_ARTIST', 'ALBUMARTIST', 'albumartist']: - del f[k] - f['ALBUMARTIST'] = canonical - f.save() - update_song_in_db(full_path) - fixed += 1 - except Exception as e: - failed.append(os.path.relpath(full_path, MUSIC_DIR)) + if not canonical: + for k in keys: + if k.upper() in ['ALBUM ARTIST', 'ALBUM_ARTIST']: + canonical = f[k][0] if f[k] else None + break + if not canonical: + continue + for k in list(f.keys()): + if k.upper() in ['ALBUM ARTIST', 'ALBUM_ARTIST', 'ALBUMARTIST', 'albumartist']: + del f[k] + f['ALBUMARTIST'] = canonical + f.save() + update_song_in_db(full_path) + _fixed += 1 + except Exception: + _failed.append(os.path.relpath(full_path, MUSIC_DIR)) + return _fixed, _failed + + # All FLAC opens + saves are blocking I/O -- run in thread (AUDIT-010) + fixed, failed = await asyncio.to_thread(_fix_picard_files) await trigger_scan() await push.broadcast("conflicts_updated", {"action": "fix_picard_tags"}) return {"status": "fixed", "fixed": fixed, "failed": len(failed)} elif action == "fix_stale_paths": - count = scan_library(full_rescan=False) - build_file_index() + count = await asyncio.to_thread(scan_library, False) + await asyncio.to_thread(build_file_index) return {"status": "fixed", "rescanned": count} elif action == "fix_orphaned_tracks": @@ -2406,25 +2587,28 @@ async def library_clean_tags(background_tasks: BackgroundTasks, dry_run: bool = ?dry_run=true returns what would be removed without writing. """ if dry_run: - # Return preview immediately - results = [] - with sqlite3.connect(DB_PATH) as c: - rows = c.execute("SELECT full_path FROM songs").fetchall() - for (full_path,) in rows: - if not os.path.isfile(full_path): - continue - r = enforce_tag_whitelist(full_path, dry_run=True) - if r["removed"]: - results.append({ - "path": os.path.relpath(full_path, MUSIC_DIR), - "would_remove": r["removed"] - }) + # enforce_tag_whitelist opens every file -- run in thread (AUDIT-011) + def _preview(): + _results = [] + with get_db() as c: + rows = c.execute("SELECT full_path FROM songs").fetchall() + for (full_path,) in rows: + if not os.path.isfile(full_path): + continue + r = enforce_tag_whitelist(full_path, dry_run=True) + if r["removed"]: + _results.append({ + "path": os.path.relpath(full_path, MUSIC_DIR), + "would_remove": r["removed"] + }) + return _results + results = await asyncio.to_thread(_preview) return {"dry_run": True, "files_affected": len(results), "preview": results[:50]} async def run_clean(): fixed = 0 errors = 0 - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: rows = c.execute("SELECT full_path FROM songs").fetchall() print(f"clean-tags: scanning {len(rows)} files...", flush=True) for (full_path,) in rows: @@ -2439,7 +2623,7 @@ async def library_clean_tags(background_tasks: BackgroundTasks, dry_run: bool = print(f"clean-tags: cleaned {fixed} files, {errors} errors", flush=True) await trigger_scan() await asyncio.sleep(4) - asyncio.create_task(_run_conflict_check_and_broadcast()) + _create_task(_run_conflict_check_and_broadcast()) await push.broadcast("tags_cleaned", {"fixed": str(fixed), "errors": str(errors)}) background_tasks.add_task(run_clean) @@ -2455,16 +2639,26 @@ async def ws_push(ws: WebSocket): await push.connect(ws) try: while True: - data = json.loads(await ws.receive_text()) - act = data.get("action") + raw = await ws.receive_text() + + # JSON decode failures are recoverable -- send error, keep connection. + # Previously any exception here permanently dropped the client (AUDIT-019). + try: + data = json.loads(raw) + except json.JSONDecodeError as e: + await push.send_to(ws, "error", {"message": f"Invalid JSON: {e}"}) + continue + + act = data.get("action") if act == "ping": await push.send_to(ws, "pong", {"t": str(time.time())}) + elif act == "get_profile": rp = data.get("path", "") fp = resolve_path(rp) if fp: try: - with sqlite3.connect(DB_PATH) as c: + with get_db() as c: row = c.execute( "SELECT bpm,silence_start,silence_end,loudness_lufs " "FROM dj_profiles WHERE file_path=?", (fp,) @@ -2480,19 +2674,26 @@ async def ws_push(ws: WebSocket): await push.send_to(ws, "profile", {"path": rp, "error": "not_analyzed"}) except Exception as e: + # DB error (e.g. locked during scan) -- report but keep connection await push.send_to(ws, "error", {"message": str(e)}) + elif act == "get_vis": rp = data.get("path", "") fp = resolve_path(rp) if fp: - frames = get_vis(fp) - if frames: - await push.send_to(ws, "vis_frames", { - "path": rp, "count": str(len(frames)), - "fps": "30", "frames": json.dumps(frames) - }) + try: + frames = await asyncio.to_thread(get_vis, fp) + if frames: + await push.send_to(ws, "vis_frames", { + "path": rp, "count": str(len(frames)), + "fps": "30", "frames": json.dumps(frames) + }) + except Exception as e: + await push.send_to(ws, "error", {"message": str(e)}) + except WebSocketDisconnect: push.disconnect(ws) except Exception as e: - print(f"WS error: {e}") + # Unrecoverable transport error -- log and clean up + print(f"WS transport error: {e}", flush=True) push.disconnect(ws) diff --git a/iOS/App/NavidromePlayerApp.swift b/iOS/App/NavidromePlayerApp.swift index 5bd0eca..0c0f6c6 100644 --- a/iOS/App/NavidromePlayerApp.swift +++ b/iOS/App/NavidromePlayerApp.swift @@ -50,35 +50,25 @@ class AppDelegate: NSObject, UIApplicationDelegate { Self.scheduleSmartDJRefresh() // re-schedule next run immediately let refreshTask = Task { - // setTaskCompleted is always called — in success path, catch, or expiry. - // Previously this called syncIfNeeded() (fire-and-forget) then returned, - // reporting success before any work was done (AUDIT-044/052). - do { - // Pre-fetch profiles using the shared singleton — no per-song URLSession leak - if CompanionSettings.shared.isEnabled, - CompanionSettings.shared.smartDJEnabled { - let queue = AudioPlayer.shared.queue - let idx = AudioPlayer.shared.queueIndex - let upcoming = Array(queue.dropFirst(idx + 1).prefix(5)) - let api = CompanionAPIService.shared // shared instance, no per-song alloc - for song in upcoming { - guard let path = song.path else { continue } - _ = try? await api.fetchProfile(relativePath: path) - } + // Pre-fetch profiles using the shared singleton — no per-song URLSession leak + if CompanionSettings.shared.isEnabled, + CompanionSettings.shared.smartDJEnabled { + let queue = AudioPlayer.shared.queue + let idx = AudioPlayer.shared.queueIndex + let upcoming = Array(queue.dropFirst(idx + 1).prefix(5)) + let api = CompanionAPIService.shared + for song in upcoming { + guard let path = song.path else { continue } + _ = try? await api.fetchProfile(relativePath: path) } - - // Await the actual sync — BGTask stays alive until work completes - await SyncEngine.shared.syncAndWait() - OptimisticActionQueue.shared.flush() - - task.setTaskCompleted(success: true) - DebugLogger.shared.log("BGTask SmartDJ refresh completed", category: "Sync", level: .info) - } catch { - // Explicit catch ensures setTaskCompleted is always called — - // previously a throw would leave the task hanging until iOS killed it - task.setTaskCompleted(success: false) - DebugLogger.shared.log("BGTask SmartDJ refresh failed: \(error.localizedDescription)", category: "Sync", level: .warning) } + + // Await the actual sync — BGTask stays alive until work completes + await SyncEngine.shared.syncAndWait() + OptimisticActionQueue.shared.flush() + + task.setTaskCompleted(success: true) + DebugLogger.shared.log("BGTask SmartDJ refresh completed", category: "Sync", level: .info) } task.expirationHandler = { diff --git a/iOS/Views/Common/AsyncCoverArt.swift b/iOS/Views/Common/AsyncCoverArt.swift index 71cd9e4..e7c351c 100644 --- a/iOS/Views/Common/AsyncCoverArt.swift +++ b/iOS/Views/Common/AsyncCoverArt.swift @@ -4,7 +4,7 @@ import UIKit // MARK: - Image Cache (Memory + Disk) /// Two-tier image cache: NSCache for fast memory access, disk for persistence across launches. -class ImageCache { +class ImageCache: @unchecked Sendable { static let shared = ImageCache() private let memoryCache = NSCache() diff --git a/iOS/Views/Common/MainTabView.swift b/iOS/Views/Common/MainTabView.swift index 6774991..3fe109a 100644 --- a/iOS/Views/Common/MainTabView.swift +++ b/iOS/Views/Common/MainTabView.swift @@ -534,64 +534,18 @@ struct MiniPlayerBar: View { private let accentPink = Color(red: 1.0, green: 0.176, blue: 0.333) - // Read directly from @Published properties — no timer lag - private var playbackTime: TimeInterval { audioPlayer.currentTime } - private var playbackDuration: TimeInterval { audioPlayer.duration } - - private var displayProgress: Double { - if isScrubbing { return scrubPosition } - guard playbackDuration > 0 else { return 0 } - return min(playbackTime / playbackDuration, 1.0) - } - var body: some View { VStack(spacing: 0) { - // Scrubbable progress bar at top — uses album color, generous touch target - GeometryReader { geo in - ZStack(alignment: .leading) { - Rectangle() - .fill(Color.white.opacity(0.1)) - .frame(height: isScrubbing ? 8 : 3) - - Rectangle() - .fill(colorExtractor.isLoaded ? colorExtractor.primaryColor : accentPink) - .frame( - width: geo.size.width * displayProgress, - height: isScrubbing ? 8 : 3 - ) - - if isScrubbing { - Circle() - .fill(colorExtractor.isLoaded ? colorExtractor.primaryColor : accentPink) - .frame(width: 16, height: 16) - .offset(x: geo.size.width * displayProgress - 8, y: -1) - } - } - .frame(maxHeight: .infinity) - .contentShape(Rectangle()) - .gesture( - DragGesture(minimumDistance: 0) - .onChanged { value in - isScrubbing = true - scrubPosition = min(max(value.location.x / geo.size.width, 0), 1) - } - .onEnded { value in - let pct = min(max(value.location.x / geo.size.width, 0), 1) - audioPlayer.seekToPercent(pct) - // Hold scrubPosition until AVPlayer confirms the seek — - // without this the bar snaps back to the pre-seek position - // for up to 250ms while currentTime catches up. - scrubPosition = pct - DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { - isScrubbing = false - } - } - ) - } - .frame(height: isScrubbing ? 20 : 14) - .animation(.easeInOut(duration: 0.15), value: isScrubbing) - - // Player content + // Progress bar extracted into its own view — only it re-evaluates at 10Hz + // when audioPlayer.currentTime changes. The rest of MiniPlayerBar body + // only re-evaluates on song change, play/pause, or color change. + MiniProgressBar( + audioPlayer: audioPlayer, + colorExtractor: colorExtractor, + isScrubbing: $isScrubbing, + scrubPosition: $scrubPosition, + accentPink: accentPink + ) ZStack(alignment: .center) { // Visualizer behind controls — paused when full NowPlaying is open if VisualizerSettings.shared.enabled && VisualizerSettings.shared.miniPlayerEnabled && !showNowPlaying { @@ -790,6 +744,80 @@ struct DynamicIslandView: View { } } +// MARK: - Mini Player Progress Bar (isolated re-evaluation) +// +// Extracted from MiniPlayerBar so only this tiny view re-evaluates when +// audioPlayer.currentTime changes (10x/second from the periodic time observer). +// Before this, the ENTIRE MiniPlayerBar body re-evaluated at 10Hz — including +// the GeometryReader, CompactVisualizerView, all ZStack children, and album art — +// causing sustained ~128% CPU even with the visualizer off. +// +// The key trick: this view declares its OWN @ObservedObject on audioPlayer so +// SwiftUI's dependency tracking scopes the 10Hz invalidation to THIS view only. +// MiniPlayerBar.body is now only re-evaluated on song change, play/pause, or +// color change — not on every currentTime tick. + +private struct MiniProgressBar: View { + @ObservedObject var audioPlayer: AudioPlayer + @ObservedObject var colorExtractor: AlbumColorExtractor + @Binding var isScrubbing: Bool + @Binding var scrubPosition: Double + let accentPink: Color + + private var displayProgress: Double { + if isScrubbing { return scrubPosition } + guard audioPlayer.duration > 0 else { return 0 } + return min(audioPlayer.currentTime / audioPlayer.duration, 1.0) + } + + private var barColor: Color { + colorExtractor.isLoaded ? colorExtractor.primaryColor : accentPink + } + + var body: some View { + GeometryReader { geo in + ZStack(alignment: .leading) { + Rectangle() + .fill(Color.white.opacity(0.1)) + .frame(height: isScrubbing ? 8 : 3) + + Rectangle() + .fill(barColor) + .frame( + width: geo.size.width * displayProgress, + height: isScrubbing ? 8 : 3 + ) + + if isScrubbing { + Circle() + .fill(barColor) + .frame(width: 16, height: 16) + .offset(x: geo.size.width * displayProgress - 8, y: -1) + } + } + .frame(maxHeight: .infinity) + .contentShape(Rectangle()) + .gesture( + DragGesture(minimumDistance: 0) + .onChanged { value in + isScrubbing = true + scrubPosition = min(max(value.location.x / geo.size.width, 0), 1) + } + .onEnded { value in + let pct = min(max(value.location.x / geo.size.width, 0), 1) + audioPlayer.seekToPercent(pct) + scrubPosition = pct + DispatchQueue.main.asyncAfter(deadline: .now() + 0.15) { + isScrubbing = false + } + } + ) + } + .frame(height: isScrubbing ? 20 : 14) + .animation(.easeInOut(duration: 0.15), value: isScrubbing) + } +} + // MARK: - Keyboard Dismiss /// Dismiss keyboard from anywhere diff --git a/iOS/Views/Library/MyMusicView.swift b/iOS/Views/Library/MyMusicView.swift index 2f2dde7..a0d7fa9 100644 --- a/iOS/Views/Library/MyMusicView.swift +++ b/iOS/Views/Library/MyMusicView.swift @@ -3,7 +3,17 @@ import PhotosUI struct MyMusicView: View { @EnvironmentObject var serverManager: ServerManager - @EnvironmentObject var audioPlayer: AudioPlayer + // Deliberately NOT @EnvironmentObject / @ObservedObject on AudioPlayer. + // Observing AudioPlayer directly subscribes this view to objectWillChange, + // which fires 10x/second from the currentTime periodic observer. With a + // large allSongs list that causes continuous `initializeWithCopy for + // MyMusicView` in Instruments at ~128% CPU even with nothing on screen. + // + // Instead: track only the one @Published property we actually need in the + // body (currentSong.id for row highlight) via @State + .onReceive. All + // playback actions use AudioPlayer.shared directly — no observation required + // for button callbacks. + @State private var currentSongId: String? @EnvironmentObject var offlineManager: OfflineManager @Binding var navigateToPlaylistId: String? @Binding var navigateToAlbumId: String? @@ -182,8 +192,12 @@ struct MyMusicView: View { } .task { await loadData() } .refreshable { refreshData() } - // Reload view data when SyncEngine completes a sync — keeps the view - // in sync with the cache without making independent server calls + // Sync currentSongId whenever the playing song changes. + // This is the ONLY AudioPlayer property MyMusicView observes — + // currentTime changes at 10Hz do not reach this view at all. + .onReceive(AudioPlayer.shared.$currentSong) { song in + currentSongId = song?.id + } .onReceive(NotificationCenter.default.publisher(for: .companionLibraryChanged)) { _ in Task { await loadData() } } @@ -851,7 +865,7 @@ struct MyMusicView: View { Button(action: { if available { - audioPlayer.play(song: song, fromQueue: Array(allSongs), at: index) + AudioPlayer.shared.play(song: song, fromQueue: Array(allSongs), at: index) } }) { HStack(spacing: 12) { @@ -884,7 +898,7 @@ struct MyMusicView: View { .font(.rowTitle) .foregroundColor( !available ? .gray.opacity(0.35) : - audioPlayer.currentSong?.id == song.id ? accentPink : .white + currentSongId == song.id ? accentPink : .white ) .lineLimit(1) Text("\(song.artist ?? "") · \(song.album ?? "")") @@ -929,19 +943,19 @@ struct MyMusicView: View { .padding(.vertical, 6) } .contextMenu { - Button(action: { audioPlayer.playNow(song) }) { + Button(action: { AudioPlayer.shared.playNow(song) }) { Label("Play Now", systemImage: "play.fill") } - Button(action: { audioPlayer.playNext(song) }) { + Button(action: { AudioPlayer.shared.playNext(song) }) { Label("Play Next", systemImage: "text.line.first.and.arrowtriangle.forward") } - Button(action: { audioPlayer.playLater(song) }) { + Button(action: { AudioPlayer.shared.playLater(song) }) { Label("Play Later", systemImage: "text.line.last.and.arrowtriangle.forward") } Divider() - Button(action: { audioPlayer.playInstantMix(basedOn: song) }) { + Button(action: { AudioPlayer.shared.playInstantMix(basedOn: song) }) { Label("Instant Mix", systemImage: "wand.and.stars") } @@ -1089,7 +1103,7 @@ struct MyMusicView: View { let albumDetail = try await serverManager.client.getAlbum(id: album.id) if let songs = albumDetail?.song, !songs.isEmpty { await MainActor.run { - audioPlayer.play(song: songs[0], fromQueue: songs, at: 0) + AudioPlayer.shared.play(song: songs[0], fromQueue: songs, at: 0) } } } catch { } @@ -1101,7 +1115,7 @@ struct MyMusicView: View { if let detail = try? await serverManager.client.getAlbum(id: album.id), let songs = detail.song { await MainActor.run { - for song in songs.reversed() { audioPlayer.playNext(song) } + for song in songs.reversed() { AudioPlayer.shared.playNext(song) } } } } @@ -1112,7 +1126,7 @@ struct MyMusicView: View { if let detail = try? await serverManager.client.getAlbum(id: album.id), let songs = detail.song { await MainActor.run { - for song in songs { audioPlayer.playLater(song) } + for song in songs { AudioPlayer.shared.playLater(song) } } } }