From 978dc36713c45e80b77878b936e727d71fbf73a7 Mon Sep 17 00:00:00 2001 From: Jesse Bannon Date: Wed, 16 Nov 2022 12:54:18 -0800 Subject: [PATCH] [BACKEND] Optimize metadata fetching (#335) * [BACKEND] Do not fetch unnecessary metadata when grabbing .info.json files" * update with more optimized ytdl options --- .github/workflows/ci.yaml | 2 +- Makefile | 12 ++- src/ytdl_sub/downloaders/downloader.py | 95 ++++++++++--------- .../downloaders/youtube/merge_playlist.py | 2 +- src/ytdl_sub/entries/variables/kwargs.py | 1 + .../subscriptions/subscription_download.py | 7 +- .../subscription_ytdl_options.py | 26 ++++- .../plugins/test_audio_extract_playlist.json | 6 +- 8 files changed, 93 insertions(+), 58 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0fb9eace..eee5c1e8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -50,7 +50,7 @@ jobs: - name: Run linters run: | source /opt/env/bin/activate - ./tools/linter check + make check_lint test-unit: runs-on: ubuntu-22.04 diff --git a/Makefile b/Makefile index 960a4232..12bced16 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,14 @@ +lint: + @-isort . + @-black . + @-pylint src/ + @-pydocstyle src/* +check_lint: + isort . --check-only --diff \ + && black . --check \ + && pylint src/ \ + && pydocstyle src/* wheel: clean pip3 install build python3 -m build @@ -21,4 +31,4 @@ clean: docker/root/defaults/examples \ coverage.xml -.PHONY: wheel docker docs clean +.PHONY: lint check_lint wheel docker_stage docker docs clean diff --git a/src/ytdl_sub/downloaders/downloader.py b/src/ytdl_sub/downloaders/downloader.py index 45f6fe7f..2af2b909 100644 --- a/src/ytdl_sub/downloaders/downloader.py +++ b/src/ytdl_sub/downloaders/downloader.py @@ -37,6 +37,7 @@ from ytdl_sub.entries.variables.kwargs import DOWNLOAD_INDEX from ytdl_sub.entries.variables.kwargs import PLAYLIST_ENTRY from ytdl_sub.entries.variables.kwargs import REQUESTED_SUBTITLES from ytdl_sub.entries.variables.kwargs import SOURCE_ENTRY +from ytdl_sub.entries.variables.kwargs import SPONSORBLOCK_CHAPTERS from ytdl_sub.entries.variables.kwargs import UPLOAD_DATE_INDEX from ytdl_sub.thread.log_entries_downloaded_listener import LogEntriesDownloadedListener from ytdl_sub.utils.exceptions import FileNotDownloadedException @@ -122,7 +123,8 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): self, download_options: DownloaderOptionsT, enhanced_download_archive: EnhancedDownloadArchive, - ytdl_options_builder: YTDLOptionsBuilder, + download_ytdl_options: YTDLOptionsBuilder, + metadata_ytdl_options: YTDLOptionsBuilder, overrides: Overrides, ): """ @@ -132,41 +134,56 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): Options validator for this downloader enhanced_download_archive Download archive - ytdl_options_builder - YTDL options builder + download_ytdl_options + YTDL options builder for downloading media + metadata_ytdl_options + YTDL options builder for downloading metadata overrides Override variables """ DownloadArchiver.__init__(self=self, enhanced_download_archive=enhanced_download_archive) self.download_options = download_options self.overrides = overrides - - self._ytdl_options_builder = ytdl_options_builder.clone().add( - self.ytdl_option_defaults(), before=True - ) - + self._download_ytdl_options_builder = download_ytdl_options + self._metadata_ytdl_options_builder = metadata_ytdl_options self.parents: List[EntryParent] = [] self.downloaded_entries: Dict[str, Entry] = {} @property - def ytdl_options(self) -> Dict: + def download_ytdl_options(self) -> Dict: """ Returns ------- - YTLD options dict + YTLD options dict for downloading """ - return self._ytdl_options_builder.clone().to_dict() + return ( + self._download_ytdl_options_builder.clone() + .add(self.ytdl_option_defaults(), before=True) + .to_dict() + ) + @property + def metadata_ytdl_options(self) -> Dict: + """ + Returns + ------- + YTDL options dict for fetching metadata + """ + return ( + self._metadata_ytdl_options_builder.clone() + .add(self.ytdl_option_defaults(), before=True) + .to_dict() + ) + + @classmethod @contextmanager - def ytdl_downloader(self, ytdl_options_overrides: Optional[Dict] = None) -> ytdl.YoutubeDL: + def ytdl_downloader(cls, ytdl_options_overrides: Dict) -> ytdl.YoutubeDL: """ Context manager to interact with yt_dlp. """ - ytdl_options = self._ytdl_options_builder.clone().add(ytdl_options_overrides).to_dict() - - download_logger.debug("ytdl_options: %s", str(ytdl_options)) + download_logger.debug("ytdl_options: %s", str(ytdl_options_overrides)) with Logger.handle_external_logs(name="yt-dlp"): - with ytdl.YoutubeDL(ytdl_options) as ytdl_downloader: + with ytdl.YoutubeDL(ytdl_options_overrides) as ytdl_downloader: yield ytdl_downloader @property @@ -176,7 +193,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): ------- True if dry-run is enabled. False otherwise. """ - return self.ytdl_options.get("skip_download", False) + return self.download_ytdl_options.get("skip_download", False) @property def is_entry_thumbnails_enabled(self) -> bool: @@ -185,9 +202,9 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): ------- True if entry thumbnails should be downloaded. False otherwise. """ - return self.ytdl_options.get("writethumbnail", False) + return self.download_ytdl_options.get("writethumbnail", False) - def extract_info(self, ytdl_options_overrides: Optional[Dict] = None, **kwargs) -> Dict: + def extract_info(self, ytdl_options_overrides: Dict, **kwargs) -> Dict: """ Wrapper around yt_dlp.YoutubeDL.YoutubeDL.extract_info All kwargs will passed to the extract_info function. @@ -204,9 +221,9 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): def extract_info_with_retry( self, + ytdl_options_overrides: Dict, is_downloaded_fn: Optional[Callable[[], bool]] = None, is_thumbnail_downloaded_fn: Optional[Callable[[], bool]] = None, - ytdl_options_overrides: Optional[Dict] = None, **kwargs, ) -> Dict: """ @@ -218,12 +235,12 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): Parameters ---------- + ytdl_options_overrides + Dict containing ytdl args to override other predefined ytdl args is_downloaded_fn Optional. Function to check if the entry is downloaded is_thumbnail_downloaded_fn Optional. Function to check if the entry thumbnail is downloaded - ytdl_options_overrides - Optional. Dict containing ytdl args to override other predefined ytdl args **kwargs arguments passed directory to YoutubeDL extract_info @@ -318,8 +335,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): def extract_info_via_info_json( self, - ytdl_options_overrides: Optional[Dict] = None, - only_info_json: bool = False, + ytdl_options_overrides: Dict, log_prefix_on_info_json_dl: Optional[str] = None, **kwargs, ) -> List[Dict]: @@ -334,33 +350,16 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): Parameters ---------- ytdl_options_overrides - Optional. Dict containing ytdl args to override other predefined ytdl args - only_info_json - Default false. Skip download and thumbnail download if True. + Dict containing ytdl args to override other predefined ytdl args log_prefix_on_info_json_dl Optional. Spin a new thread to listen for new info.json files. Log f'{log_prefix_on_info_json_dl} {title}' when a new one appears **kwargs arguments passed directory to YoutubeDL extract_info """ - ytdl_options_builder = self._ytdl_options_builder.clone() - if ytdl_options_overrides is None: - ytdl_options_overrides = {} - - ytdl_options_builder.add({"writeinfojson": True}, ytdl_options_overrides) - if only_info_json: - ytdl_options_builder.add( - { - "skip_download": True, - "writethumbnail": False, - } - ) - try: with self._listen_and_log_downloaded_info_json(log_prefix=log_prefix_on_info_json_dl): - _ = self.extract_info( - ytdl_options_overrides=ytdl_options_builder.to_dict(), **kwargs - ) + _ = self.extract_info(ytdl_options_overrides=ytdl_options_overrides, **kwargs) except RejectedVideoReached: download_logger.debug("RejectedVideoReached, stopping additional downloads") except ExistingVideoReached: @@ -395,7 +394,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): clear_info_json_files Whether to delete info.json files after yield """ - archive_path = self.ytdl_options.get("download_archive", "") + archive_path = self.download_ytdl_options.get("download_archive", "") backup_archive_path = f"{archive_path}.backup" # If archive path exists, maintain download archive is enable @@ -435,7 +434,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): if (self.is_dry_run or not self.is_entry_thumbnails_enabled) else entry.is_thumbnail_downloaded, url=entry.webpage_url, - ytdl_options_overrides={"writeinfojson": False, "skip_download": self.is_dry_run}, + ytdl_options_overrides=self.download_ytdl_options, ) return Entry(download_entry_dict, working_directory=self.working_directory) @@ -450,8 +449,10 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): entry.add_kwargs( { - # Workaround for the yt-dlp issue that does not include subs in playlist downloads + # Subtitles are not downloaded in metadata run, only here, so move over REQUESTED_SUBTITLES: download_entry.kwargs_get(REQUESTED_SUBTITLES), + # Same with sponsorblock chapters + SPONSORBLOCK_CHAPTERS: download_entry.kwargs_get(SPONSORBLOCK_CHAPTERS), # Tracks number of entries downloaded DOWNLOAD_INDEX: download_idx, # Tracks number of entries with the same upload date to make them unique @@ -502,7 +503,7 @@ class Downloader(DownloadArchiver, Generic[DownloaderOptionsT], ABC): with self._separate_download_archives(): entry_dicts = self.extract_info_via_info_json( - only_info_json=True, + ytdl_options_overrides=self.metadata_ytdl_options, url=url, log_prefix_on_info_json_dl="Downloading metadata for", ) diff --git a/src/ytdl_sub/downloaders/youtube/merge_playlist.py b/src/ytdl_sub/downloaders/youtube/merge_playlist.py index fdfffdaf..48d1eae9 100644 --- a/src/ytdl_sub/downloaders/youtube/merge_playlist.py +++ b/src/ytdl_sub/downloaders/youtube/merge_playlist.py @@ -146,7 +146,7 @@ class YoutubeMergePlaylistDownloader(Downloader[YoutubeMergePlaylistDownloaderOp """Download a single Youtube video, then split it into multiple videos""" url = self.overrides.apply_formatter(self.collection.urls.list[0].url) - entry_dict = self.extract_info(url=url) + entry_dict = self.extract_info(url=url, ytdl_options_overrides=self.download_ytdl_options) merged_video = self._to_merged_video(entry_dict=entry_dict) merged_video_metadata = self._get_chapters( diff --git a/src/ytdl_sub/entries/variables/kwargs.py b/src/ytdl_sub/entries/variables/kwargs.py index 1b9100e6..b9c74161 100644 --- a/src/ytdl_sub/entries/variables/kwargs.py +++ b/src/ytdl_sub/entries/variables/kwargs.py @@ -43,6 +43,7 @@ PLAYLIST_UPLOADER_URL = _("playlist_uploader_url") DOWNLOAD_INDEX = _("download_index", backend=True) UPLOAD_DATE_INDEX = _("upload_date_index", backend=True) REQUESTED_SUBTITLES = _("requested_subtitles", backend=True) +SPONSORBLOCK_CHAPTERS = _("sponsorblock_chapters", backend=True) UID = _("id") EXTRACTOR = _("extractor") EPOCH = _("epoch") diff --git a/src/ytdl_sub/subscriptions/subscription_download.py b/src/ytdl_sub/subscriptions/subscription_download.py index 6d407496..d321db38 100644 --- a/src/ytdl_sub/subscriptions/subscription_download.py +++ b/src/ytdl_sub/subscriptions/subscription_download.py @@ -237,19 +237,20 @@ class SubscriptionDownload(BaseSubscription, ABC): self._enhanced_download_archive.reinitialize(dry_run=dry_run) plugins = self._initialize_plugins() - ytdl_options_builder = SubscriptionYTDLOptions( + subscription_ytdl_options = SubscriptionYTDLOptions( preset=self._preset_options, plugins=plugins, enhanced_download_archive=self._enhanced_download_archive, working_directory=self.working_directory, dry_run=dry_run, - ).builder() + ) with self._subscription_download_context_managers(): downloader = self.downloader_class( download_options=self.downloader_options, enhanced_download_archive=self._enhanced_download_archive, - ytdl_options_builder=ytdl_options_builder, + download_ytdl_options=subscription_ytdl_options.download_builder(), + metadata_ytdl_options=subscription_ytdl_options.metadata_builder(), overrides=self.overrides, ) diff --git a/src/ytdl_sub/subscriptions/subscription_ytdl_options.py b/src/ytdl_sub/subscriptions/subscription_ytdl_options.py index 7096ecd9..045845b0 100644 --- a/src/ytdl_sub/subscriptions/subscription_ytdl_options.py +++ b/src/ytdl_sub/subscriptions/subscription_ytdl_options.py @@ -74,6 +74,14 @@ class SubscriptionYTDLOptions: # TODO: find a way to not write subtitles; using `simulate: True` breaks tests } + @property + def _info_json_only_options(self) -> Dict: + return { + "skip_download": True, + "writethumbnail": False, + "writeinfojson": True, + } + @property def _output_options(self) -> Dict: ytdl_options = {} @@ -94,12 +102,26 @@ class SubscriptionYTDLOptions: def _user_ytdl_options(self) -> Dict: return self._preset.ytdl_options.dict - def builder(self) -> YTDLOptionsBuilder: + def metadata_builder(self) -> YTDLOptionsBuilder: """ Returns ------- YTDLOptionsBuilder - Builder with values set based on the subscription + Builder with values set for fetching metadata (.info.json) only + """ + return YTDLOptionsBuilder().add( + self._global_options, + self._plugin_ytdl_options(DateRangePlugin), + self._user_ytdl_options, # user ytdl options... + self._info_json_only_options, # then info_json_only options + ) + + def download_builder(self) -> YTDLOptionsBuilder: + """ + Returns + ------- + YTDLOptionsBuilder + Builder with values set based on the subscription for actual downloading """ ytdl_options_builder = YTDLOptionsBuilder().add(self._global_options) if self._dry_run: diff --git a/tests/resources/expected_downloads_summaries/plugins/test_audio_extract_playlist.json b/tests/resources/expected_downloads_summaries/plugins/test_audio_extract_playlist.json index 22b7eebd..e026e3c7 100644 --- a/tests/resources/expected_downloads_summaries/plugins/test_audio_extract_playlist.json +++ b/tests/resources/expected_downloads_summaries/plugins/test_audio_extract_playlist.json @@ -1,5 +1,5 @@ { - "Jesse's Minecraft Server [Trailer - Feb.1].ogg": "afb2264e271e3c464d0fe9ba0db37f88", - "Jesse's Minecraft Server [Trailer - Feb.27].ogg": "6f322d842a1e43b4c9981b42423d9b4c", - "Jesse's Minecraft Server [Trailer - Mar.21].ogg": "fe1dc3361a102661c27020d5b95a2a81" + "Jesse's Minecraft Server [Trailer - Feb.1].ogg": "0d4509c21883fbc46584d25f6ec4fd80", + "Jesse's Minecraft Server [Trailer - Feb.27].ogg": "7ab2863ee0e1c215102c4512085f877d", + "Jesse's Minecraft Server [Trailer - Mar.21].ogg": "ed6a54d5b3ec6303ea6a175e02c69511" } \ No newline at end of file