Merge PR #1025: fill missing album-artist metadata for audio downloads

Adds _AlbumArtistPostProcessor, a yt-dlp pre_process postprocessor that
fills album_artist from the '<artist> - Topic' channel/uploader signal,
falling back to the first credited artist, when album metadata exists
but no album artist is set.

Closes #1025.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

app/tests/test_ytdl_utils.py

@@ -15,6 +15,8 @@ from unittest.mock import MagicMock, patch
 fake_yt_dlp = types.ModuleType("yt_dlp")
 fake_networking = types.ModuleType("yt_dlp.networking")
 fake_impersonate = types.ModuleType("yt_dlp.networking.impersonate")
+fake_postprocessor = types.ModuleType("yt_dlp.postprocessor")
+fake_postprocessor_common = types.ModuleType("yt_dlp.postprocessor.common")
 fake_utils = types.ModuleType("yt_dlp.utils")
 
 
@@ -24,18 +26,27 @@ class _ImpersonateTarget:
         return value
 
 
+class _PostProcessor:
+    def __init__(self, downloader=None):
+        self._downloader = downloader
+
+
 fake_impersonate.ImpersonateTarget = _ImpersonateTarget
 fake_networking.impersonate = fake_impersonate
+fake_postprocessor_common.PostProcessor = _PostProcessor
 # The inner ``key`` group mirrors the real ``STR_FORMAT_RE_TMPL`` so that
 # ``_OUTTMPL_FIELD_RE`` (compiled at import time) has the named group that
 # ``_resolve_outtmpl_fields`` reads via ``match.group('key')``.
 fake_utils.STR_FORMAT_RE_TMPL = r"(?P<prefix>)%\((?P<has_key>(?P<key>{}))\)(?P<format>[-0-9.]*{})"
 fake_utils.STR_FORMAT_TYPES = "diouxXeEfFgGcrsa"
 fake_yt_dlp.networking = fake_networking
+fake_yt_dlp.postprocessor = fake_postprocessor
 fake_yt_dlp.utils = fake_utils
 sys.modules.setdefault("yt_dlp", fake_yt_dlp)
 sys.modules.setdefault("yt_dlp.networking", fake_networking)
 sys.modules.setdefault("yt_dlp.networking.impersonate", fake_impersonate)
+sys.modules.setdefault("yt_dlp.postprocessor", fake_postprocessor)
+sys.modules.setdefault("yt_dlp.postprocessor.common", fake_postprocessor_common)
 sys.modules.setdefault("yt_dlp.utils", fake_utils)
 
 from ytdl import (
@@ -43,6 +54,7 @@ from ytdl import (
     DownloadInfo,
     _compact_persisted_entry,
     _convert_srt_to_txt_file,
+    _AlbumArtistPostProcessor,
     _output_dir_escapes,
     _resolve_outtmpl_fields,
     _sanitize_entry_for_pickle,
@@ -54,6 +66,132 @@ from ytdl import (
 _has_real_ytdlp = hasattr(sys.modules.get("yt_dlp"), "YoutubeDL")
 
 
+class AlbumArtistPostProcessorTests(unittest.TestCase):
+    def setUp(self):
+        self.postprocessor = _AlbumArtistPostProcessor()
+
+    def test_fills_album_artist_from_artist(self):
+        info = {'album': 'CrasH Talk', 'artist': 'ScHoolboy Q'}
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertEqual(result['album_artist'], 'ScHoolboy Q')
+
+    def test_uses_main_artist_for_featured_track(self):
+        info = {
+            'album': 'CrasH Talk',
+            'artists': ['ScHoolboy Q · Travis Scott'],
+        }
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertEqual(result['album_artist'], 'ScHoolboy Q')
+
+    def test_uses_topic_channel_artist_for_joint_album(self):
+        info = {
+            'album': 'Watch the Throne',
+            'artists': ['JAY-Z', 'Kanye West'],
+            'channel': 'JAY-Z & Kanye West - Topic',
+        }
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertEqual(result['album_artist'], 'JAY-Z & Kanye West')
+
+    def test_uses_topic_uploader_and_strips_suffix_for_compilation(self):
+        info = {
+            'album': 'Compilation',
+            'artist': 'Track Artist',
+            'channel': 'Regular Channel',
+            'uploader': 'Various Artists - Topic',
+        }
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertEqual(result['album_artist'], 'Various Artists')
+
+    def test_regular_channel_falls_back_to_main_artist(self):
+        info = {
+            'album': 'Album',
+            'artist': 'Track Artist',
+            'channel': 'Label Channel',
+        }
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertEqual(result['album_artist'], 'Track Artist')
+
+    def test_preserves_explicit_various_artists(self):
+        info = {
+            'album': 'Revenge of the Dreamers III',
+            'artist': 'J. Cole',
+            'album_artist': 'Various Artists',
+        }
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertEqual(result['album_artist'], 'Various Artists')
+
+    def test_preserves_existing_album_artists_list(self):
+        info = {
+            'album': 'Album',
+            'artist': 'Track Artist',
+            'album_artists': ['Album Artist'],
+        }
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertEqual(result['album_artists'], ['Album Artist'])
+        self.assertNotIn('album_artist', result)
+
+    def test_uses_first_artist_when_artist_list_has_multiple_entries(self):
+        info = {'album': 'Album', 'artists': ['Main Artist', 'Featured Artist']}
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertEqual(result['album_artist'], 'Main Artist')
+
+    def test_does_not_fill_without_album(self):
+        info = {'artist': 'Standalone Artist'}
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertNotIn('album_artist', result)
+        self.assertNotIn('album_artists', result)
+
+    def test_does_not_fill_without_artist(self):
+        info = {'album': 'Instrumental Album'}
+
+        _, result = self.postprocessor.run(info)
+
+        self.assertNotIn('album_artist', result)
+        self.assertNotIn('album_artists', result)
+
+
+class AlbumArtistRegistrationTests(unittest.TestCase):
+    def test_audio_download_registers_pre_process_postprocessor(self):
+        download = _make_test_download()
+        download.info.download_type = 'audio'
+        fake_ydl = MagicMock()
+
+        with patch('ytdl.yt_dlp.YoutubeDL', return_value=fake_ydl):
+            result = download._make_youtube_dl({'quiet': True})
+
+        self.assertIs(result, fake_ydl)
+        postprocessor, = fake_ydl.add_post_processor.call_args.args
+        self.assertIsInstance(postprocessor, _AlbumArtistPostProcessor)
+        self.assertEqual(fake_ydl.add_post_processor.call_args.kwargs, {'when': 'pre_process'})
+
+    def test_video_download_does_not_register_postprocessor(self):
+        download = _make_test_download()
+        fake_ydl = MagicMock()
+
+        with patch('ytdl.yt_dlp.YoutubeDL', return_value=fake_ydl):
+            download._make_youtube_dl({'quiet': True})
+
+        fake_ydl.add_post_processor.assert_not_called()
+
+
 class SanitizePathComponentTests(unittest.TestCase):
     def test_replaces_windows_invalid_chars(self):
         self.assertEqual(_sanitize_path_component('a:b*c?d"e<f>g|h'), "a_b_c_d_e_f_g_h")

app/ytdl.py

@@ -19,6 +19,7 @@ import types
 from typing import Any, Optional
 
 import yt_dlp.networking.impersonate
+from yt_dlp.postprocessor.common import PostProcessor
 from yt_dlp.utils import STR_FORMAT_RE_TMPL, STR_FORMAT_TYPES
 import bg_tasks
 from dl_formats import get_format, get_opts, AUDIO_FORMATS, merge_ytdl_option_layers
@@ -53,6 +54,52 @@ _LIVE_MAX_CHECK_INTERVAL = 3600
 _LIVE_PROBE_MAX_FAILURES = 5
 
 
+class _AlbumArtistPostProcessor(PostProcessor):
+    """Fill missing album-artist metadata from yt-dlp's album-level signals."""
+
+    _TOPIC_SUFFIX = ' - Topic'
+
+    @staticmethod
+    def _has_value(value: Any) -> bool:
+        if isinstance(value, str):
+            return bool(value.strip())
+        if isinstance(value, (list, tuple)):
+            return any(_AlbumArtistPostProcessor._has_value(item) for item in value)
+        return value is not None
+
+    @staticmethod
+    def _main_artist(info) -> Optional[str]:
+        artists = info.get('artists')
+        candidates = artists if isinstance(artists, list) else [info.get('artist')]
+        for candidate in candidates:
+            if not isinstance(candidate, str) or not candidate.strip():
+                continue
+            # YouTube Music uses a spaced middle dot between credited artists.
+            # The first credit is the primary artist for normal albums.
+            return candidate.split(' · ', 1)[0].strip()
+        return None
+
+    @classmethod
+    def _topic_artist(cls, info) -> Optional[str]:
+        for field in ('channel', 'uploader'):
+            value = info.get(field)
+            if not isinstance(value, str) or not value.endswith(cls._TOPIC_SUFFIX):
+                continue
+            if artist := value[:-len(cls._TOPIC_SUFFIX)].strip():
+                return artist
+        return None
+
+    def run(self, info):
+        if not self._has_value(info.get('album')):
+            return [], info
+        if self._has_value(info.get('album_artist')) or self._has_value(info.get('album_artists')):
+            return [], info
+
+        if artist := self._topic_artist(info) or self._main_artist(info):
+            info['album_artist'] = artist
+        return [], info
+
+
 def _is_within_directory(real_base: str, real_target: str) -> bool:
     """True if ``real_target`` is inside (or equal to) ``real_base``.
 
@@ -546,6 +593,12 @@ class Download:
 
         return put_status
 
+    def _make_youtube_dl(self, params):
+        ydl = yt_dlp.YoutubeDL(params=params)
+        if getattr(self.info, 'download_type', '') == 'audio':
+            ydl.add_post_processor(_AlbumArtistPostProcessor(ydl), when='pre_process')
+        return ydl
+
     def _download(self):
         # Run in our own process group so cancel() can SIGKILL the whole
         # group (yt-dlp + any ffmpeg children it spawned for merge/postproc),
@@ -623,7 +676,7 @@ class Download:
                     [(start, end)],
                 )
 
-            ret = yt_dlp.YoutubeDL(params=ytdl_params).download([self.info.url])
+            ret = self._make_youtube_dl(ytdl_params).download([self.info.url])
             self.status_queue.put({'status': 'finished' if ret == 0 else 'error'})
             log.info(f"Finished download for: {self.info.title}")
         except yt_dlp.utils.YoutubeDLError as exc: