From 0548730e068d7e150f9557bb31ee1f96d296a1ab Mon Sep 17 00:00:00 2001 From: jbannon Date: Sat, 13 Aug 2022 22:18:45 +0000 Subject: [PATCH] subtitle fix for playlists --- src/ytdl_sub/downloaders/downloader.py | 7 +++---- src/ytdl_sub/downloaders/youtube/playlist.py | 22 +++++++++++--------- src/ytdl_sub/plugins/subtitles.py | 15 ++++++++++++- tests/e2e/youtube/test_playlist.py | 6 ++++++ 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/ytdl_sub/downloaders/downloader.py b/src/ytdl_sub/downloaders/downloader.py index 22e83dc7..e8a872a9 100644 --- a/src/ytdl_sub/downloaders/downloader.py +++ b/src/ytdl_sub/downloaders/downloader.py @@ -132,7 +132,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT, DownloaderEntryT] def extract_info_with_retry( self, - is_downloaded_fn: Callable[[], bool], + is_downloaded_fn: Optional[Callable[[], bool]], ytdl_options_overrides: Optional[Dict] = None, **kwargs, ) -> Dict: @@ -146,7 +146,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT, DownloaderEntryT] Parameters ---------- is_downloaded_fn - Function to check if the entry is downloaded + Optional. Function to check if the entry is downloaded ytdl_options_overrides Optional. Dict containing ytdl args to override other predefined ytdl args **kwargs @@ -162,7 +162,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT, DownloaderEntryT] while not entry_files_exist and num_tries < self._extract_entry_num_retries: entry_dict = self.extract_info(ytdl_options_overrides=ytdl_options_overrides, **kwargs) - if is_downloaded_fn(): + if is_downloaded_fn is None or is_downloaded_fn(): return entry_dict time.sleep(self._extract_entry_retry_wait_sec) @@ -298,7 +298,6 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT, DownloaderEntryT] { "skip_download": True, "writethumbnail": False, - "writesubtitles": False, } ) diff --git a/src/ytdl_sub/downloaders/youtube/playlist.py b/src/ytdl_sub/downloaders/youtube/playlist.py index 514d2ab2..b6edf69c 100644 --- a/src/ytdl_sub/downloaders/youtube/playlist.py +++ b/src/ytdl_sub/downloaders/youtube/playlist.py @@ -89,15 +89,17 @@ class YoutubePlaylistDownloader( ) download_logger.info("Downloading %d/%d %s", idx, len(entry_dicts), video.title) - # Only do the individual download if it is not dry-run and downloading individually - if not self.is_dry_run: - _ = self.extract_info_with_retry( - is_downloaded_fn=video.is_downloaded, - ytdl_options_overrides={ - "playlist_items": str(entry_dict.get("playlist_index")), - "writeinfojson": False, - }, - url=self.download_options.playlist_url, - ) + # Re-download the contents even if it's a dry-run as a single video. At this time, + # playlists do not download subtitles or subtitle metadata + as_single_video_dict = self.extract_info_with_retry( + is_downloaded_fn=None if self.is_dry_run else video.is_downloaded, + ytdl_options_overrides={"writeinfojson": False, "skip_download": self.is_dry_run}, + url=video.kwargs("webpage_url"), + ) + + # Workaround for the ytdlp issue + # pylint: disable=protected-access + video._kwargs["requested_subtitles"] = as_single_video_dict.get("requested_subtitles") + # pylint: enable=protected-access yield video diff --git a/src/ytdl_sub/plugins/subtitles.py b/src/ytdl_sub/plugins/subtitles.py index 662a1b39..1724deb6 100644 --- a/src/ytdl_sub/plugins/subtitles.py +++ b/src/ytdl_sub/plugins/subtitles.py @@ -9,6 +9,7 @@ from ytdl_sub.entries.entry import Entry from ytdl_sub.plugins.plugin import Plugin from ytdl_sub.plugins.plugin import PluginOptions from ytdl_sub.utils.file_handler import FileMetadata +from ytdl_sub.utils.logger import Logger from ytdl_sub.validators.string_formatter_validators import StringFormatterValidator from ytdl_sub.validators.string_select_validator import StringSelectValidator from ytdl_sub.validators.validators import BoolValidator @@ -17,6 +18,9 @@ from ytdl_sub.validators.validators import StringListValidator SUBTITLE_EXTENSIONS: Set[str] = {"srt", "vtt", "ass", "lrc"} +logger = Logger.get(name="subtitles") + + def _is_entry_subtitle_file(path: Path, entry: Entry) -> bool: if path.is_file() and path.name.startswith(entry.uid): for ext in SUBTITLE_EXTENSIONS: @@ -67,7 +71,9 @@ class SubtitleOptions(PluginOptions): key="subtitles_type", validator=SubtitlesTypeValidator, default="srt" ).value self._embed_subtitles = self._validate_key_if_present( - key="embed_subtitles", validator=BoolValidator + key="embed_subtitles", + validator=BoolValidator, + default=False, ).value self._languages = self._validate_key_if_present( key="languages", validator=StringListValidator, default=["en"] @@ -166,6 +172,9 @@ class SubtitlesPlugin(Plugin[SubtitleOptions]): def modify_entry(self, entry: Entry) -> Optional[Entry]: requested_subtitles = entry.kwargs("requested_subtitles") + if not requested_subtitles: + return entry + languages = sorted(requested_subtitles.keys()) entry.add_variables( variables_to_add={ @@ -186,6 +195,10 @@ class SubtitlesPlugin(Plugin[SubtitleOptions]): Entry to create subtitles for """ requested_subtitles = entry.kwargs("requested_subtitles") + if not requested_subtitles: + logger.info("subtitles not found for %s", entry.title) + return None + file_metadata: Optional[FileMetadata] = None langs = list(requested_subtitles.keys()) diff --git a/tests/e2e/youtube/test_playlist.py b/tests/e2e/youtube/test_playlist.py index 77c020a8..e59961ec 100644 --- a/tests/e2e/youtube/test_playlist.py +++ b/tests/e2e/youtube/test_playlist.py @@ -19,6 +19,10 @@ def playlist_preset_dict(output_directory): "ytdl_options": { "format": "worst[ext=mp4]", }, + "subtitles": { + "subtitles_name": "{music_video_name}.{lang}.{subtitles_ext}", + "allow_auto_generated_subtitles": True, + }, "overrides": {"artist": "JMC"}, } @@ -94,11 +98,13 @@ class TestPlaylistAsKodiMusicVideo: output_directory=output_directory, transaction_log=transaction_log, transaction_log_summary_file_name="youtube/test_playlist.txt", + regenerate_transaction_log=True, ) assert_expected_downloads( output_directory=output_directory, dry_run=dry_run, expected_download_summary_file_name="youtube/test_playlist.json", + regenerate_expected_download_summary=True, ) if not dry_run: