From c72e83bc2f73760b5f3d70e5640cee9ed6a6b666 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Fri, 12 Jun 2026 23:46:29 -0700 Subject: [PATCH] =?UTF-8?q?#863:=20move=20YouTube=20artist=20recovery=20ou?= =?UTF-8?q?t=20of=20the=20(synchronous)=20parse=20into=20the=20async=20dis?= =?UTF-8?q?covery=20worker=20=E2=80=94=20parse=20is=20fast=20again,=20no?= =?UTF-8?q?=20120s=20timeout=20risk?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/discovery/youtube.py | 21 ++++++++++++ web_server.py | 67 +++++++++++---------------------------- 2 files changed, 39 insertions(+), 49 deletions(-) diff --git a/core/discovery/youtube.py b/core/discovery/youtube.py index d9830f38..d0c6f8f9 100644 --- a/core/discovery/youtube.py +++ b/core/discovery/youtube.py @@ -52,6 +52,10 @@ class YoutubeDiscoveryDeps: build_discovery_wing_it_stub: Callable get_database: Callable[[], Any] add_activity_item: Callable + # Recover a YouTube track's artist from its own video page when flat playlist + # extraction left it "Unknown Artist" (#863). Takes a video id, returns a raw + # artist string or ''. Optional — discovery still works without it. + recover_youtube_artist: Callable[[str], str] = None def run_youtube_discovery_worker(url_hash, deps: YoutubeDiscoveryDeps): @@ -94,6 +98,23 @@ def run_youtube_discovery_worker(url_hash, deps: YoutubeDiscoveryDeps): cleaned_title = track['name'] cleaned_artist = track['artists'][0] if track['artists'] else 'Unknown Artist' + # Recover the artist from the track's own video page if flat + # playlist extraction left it Unknown (#863). Done here, in the + # background worker, rather than in the parse request (which would + # block for minutes on a big playlist). Per-track cost is hidden + # behind the discovery progress bar; the recovered artist makes the + # match below actually find the song. + if (cleaned_artist == 'Unknown Artist' and deps.recover_youtube_artist + and track.get('id')): + try: + _rec = deps.recover_youtube_artist(track['id']) + except Exception as _rec_err: + logger.debug(f"Artist recovery failed for {track.get('id')}: {_rec_err}") + _rec = '' + if _rec and _rec != 'Unknown Artist': + cleaned_artist = _rec + track['artists'] = [_rec] # persist so retries/UI see it + logger.info(f"Searching {discovery_source} for: '{cleaned_artist}' - '{cleaned_title}'") # Check discovery cache first diff --git a/web_server.py b/web_server.py index 01d54827..3c86e85b 100644 --- a/web_server.py +++ b/web_server.py @@ -14776,6 +14776,16 @@ def _fetch_youtube_video_artist(video_id, cookie_opts): return (info.get('uploader') or info.get('channel') or '').strip() +def _recover_youtube_artist_cleaned(video_id): + """Fetch + clean a YouTube track's artist from its video page (#863). + Used by the async discovery worker for tracks flat extraction left Unknown.""" + raw = _fetch_youtube_video_artist(video_id, _youtube_cookie_opts()) + if not raw: + return '' + cleaned = clean_youtube_artist(raw) + return cleaned if cleaned and cleaned != 'Unknown Artist' else '' + + def parse_youtube_playlist(url): """ Parse a YouTube Music playlist URL and extract track information using yt-dlp @@ -14848,55 +14858,13 @@ def parse_youtube_playlist(url): tracks.append(track_data) - # ARTIST RECOVERY (#863): current yt-dlp flat extraction returns ONLY - # the title per entry — no uploader/artist — so tracks whose title - # isn't "Artist - Title" came out "Unknown Artist" and discovery - # couldn't match them. Recover the artist from each unresolved track's - # OWN video page (uploader/channel). Bounded by a wall-clock budget so - # a large playlist can't exceed the 120s gunicorn worker timeout; - # whatever isn't reached stays for MusicBrainz/Spotify discovery. - unresolved = [t for t in tracks - if t.get('id') and (not t['artists'] or t['artists'][0] == 'Unknown Artist')] - if unresolved: - cookie_opts = _youtube_cookie_opts() - deadline = time.time() + 75 # leave headroom under the 120s worker timeout - recovered = 0 - # Per-video lookups are several seconds each, so doing them - # sequentially only covered ~a dozen tracks before the budget ran - # out (the back half of a playlist stayed "Unknown Artist"). Run a - # small pool concurrently to cover the whole playlist within the - # same wall-clock budget, without hammering YouTube. - from concurrent.futures import (ThreadPoolExecutor, as_completed, - TimeoutError as _FuturesTimeout) - ex = ThreadPoolExecutor(max_workers=5, thread_name_prefix="yt_artist") - fut_to_track = {ex.submit(_fetch_youtube_video_artist, t['id'], cookie_opts): t - for t in unresolved} - try: - for fut in as_completed(fut_to_track, timeout=max(1.0, deadline - time.time())): - t = fut_to_track[fut] - try: - raw_artist = fut.result() - except Exception: - raw_artist = '' - if raw_artist: - ca = clean_youtube_artist(raw_artist) - if ca and ca != 'Unknown Artist': - t['artists'] = [ca] - t['name'] = clean_youtube_track_title(t.get('name') or '', ca) - t['raw_artist'] = raw_artist - recovered += 1 - except _FuturesTimeout: - logger.warning( - f"[YT Parse] Artist-recovery budget hit; recovered {recovered}/{len(unresolved)}, " - "rest left for discovery to resolve" - ) - finally: - # Don't block the response waiting on in-flight lookups past - # the budget — cancel what hasn't started, let the rest finish - # in the background. - ex.shutdown(wait=False, cancel_futures=True) - if recovered: - logger.info(f"[YT Parse] Recovered artist for {recovered}/{len(unresolved)} unresolved tracks") + # NOTE: current yt-dlp flat extraction returns ONLY the title per entry + # (no uploader/artist), so tracks whose title isn't "Artist - Title" + # land here as "Unknown Artist". The per-video artist recovery does NOT + # run here — it would block this request for minutes on a big playlist + # and risk the 120s worker timeout. It runs in the async discovery + # worker instead (which already iterates every track with a progress + # bar). See run_youtube_discovery_worker / recover_youtube_artist (#863). # Create playlist object matching GUI structure playlist_data = { @@ -24748,6 +24716,7 @@ def _build_youtube_discovery_deps(): build_discovery_wing_it_stub=_build_discovery_wing_it_stub, get_database=get_database, add_activity_item=add_activity_item, + recover_youtube_artist=_recover_youtube_artist_cleaned, )