diff --git a/app/tests/test_ytdl_utils.py b/app/tests/test_ytdl_utils.py index f529574..73b40fe 100644 --- a/app/tests/test_ytdl_utils.py +++ b/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)%\((?P(?P{}))\)(?P[-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,98 @@ 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_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"eg|h'), "a_b_c_d_e_f_g_h") diff --git a/app/ytdl.py b/app/ytdl.py index 8a2dc42..daeb594 100644 --- a/app/ytdl.py +++ b/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 @@ -52,6 +53,40 @@ _LIVE_MAX_CHECK_INTERVAL = 3600 _LIVE_PROBE_MAX_FAILURES = 5 +class _AlbumArtistPostProcessor(PostProcessor): + """Fill missing album-artist metadata from yt-dlp's main track artist.""" + + @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 + + 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._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``. @@ -545,6 +580,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), @@ -622,7 +663,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: