From e6aaab1016e27327490bae8db8389b30e71182bb Mon Sep 17 00:00:00 2001 From: Roland Date: Fri, 20 Mar 2026 13:02:46 -0600 Subject: [PATCH] Creating video duration cache to avoid requesting video duration between restarts. --- modules/videos/downloader.py | 44 +++++++++++++++++++-- modules/videos/test/test_downloader.py | 44 +++++++++++++++++++++ wrolpi/conftest.py | 10 ++++- wrolpi/downloader.py | 55 ++++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 4 deletions(-) diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py index a69a4c32..4d28dd1c 100755 --- a/modules/videos/downloader.py +++ b/modules/videos/downloader.py @@ -25,7 +25,7 @@ from wrolpi.common import logger, get_media_directory, escape_file_name, resolve from wrolpi.dates import now from wrolpi.db import get_db_session from wrolpi.downloader import Downloader, Download, DownloadResult, \ - clear_download_progress, _parse_size, make_progress_callback + clear_download_progress, _parse_size, make_progress_callback, get_download_cache_config from wrolpi.errors import UnrecoverableDownloadError, BotBlockedDownloadError from wrolpi.files.lib import glob_shared_stem, split_path_stem_and_suffix from wrolpi.files.models import FileGroup @@ -75,6 +75,21 @@ def parse_ytdlp_progress(line: str) -> dict | None: ) +def _log_ytdlp_versions(): + """Log yt-dlp and yt-dlp-ejs versions for debugging download issues.""" + try: + import yt_dlp.version + ytdlp_version = yt_dlp.version.__version__ + except Exception: + ytdlp_version = 'unknown' + try: + from importlib.metadata import version + ejs_version = version('yt-dlp-ejs') + except Exception: + ejs_version = 'not installed' + logger.info(f'yt-dlp={ytdlp_version} yt-dlp-ejs={ejs_version}') + + VIDEO_RESOLUTION_MAP = { '360p': [ '134+bestaudio', '230+bestaudio', 'mp4-360p', 'res:360', @@ -312,21 +327,41 @@ def preview_filename(filename_format: str) -> str: raise RuntimeError(f'Invalid variable: {e}') +VIDEO_DURATION_CACHE_MAX_SIZE = 1_000 + + +def _cache_video_duration(url: str, duration: int): + """Persist a video duration to the download cache config. Evicts oldest entries when over the max size.""" + cache_config = get_download_cache_config() + entries = [e for e in cache_config.video_durations if e[0] != url] + entries.append([url, duration]) + if len(entries) > VIDEO_DURATION_CACHE_MAX_SIZE: + entries = entries[-VIDEO_DURATION_CACHE_MAX_SIZE:] + cache_config.video_durations = entries + + @cached_multiprocessing_result async def fetch_video_duration(url: str) -> int: - """Get video duration in seconds. Attempts to get the duration from a Video that has already been downloaded, - otherwise uses YouTube API for YouTube URLs, falling back to yt-dlp.""" + """Get video duration in seconds. Checks persistent cache, then DB, then YouTube API, then yt-dlp.""" + # Check persistent cache first (survives restarts). + cached_duration = dict(get_download_cache_config().video_durations).get(url) + if cached_duration is not None: + logger.debug(f'Using cached duration {cached_duration} for {url}') + return cached_duration + with get_db_session() as session: # Join with Video to get duration; a URL may have other files (such as an Archive). video = Video.get_by_url(session, url) if video and (duration := video.file_group.length): logger.debug(f'Using already-known duration {duration} for {url} from FileGroup') + _cache_video_duration(url, duration) return duration # Try YouTube API first (faster than yt-dlp) duration = await get_youtube_duration(url) if duration is not None: logger.info(f'Fetched video duration of {duration} from {url} using YouTube API') + _cache_video_duration(url, duration) return duration # Fall back to yt-dlp for non-YouTube URLs or API failures @@ -334,6 +369,7 @@ async def fetch_video_duration(url: str) -> int: info = extract_info(url, process=False) duration = info['duration'] logger.info(f'Fetched video duration of {duration} from {url}') + _cache_video_duration(url, duration) return duration @@ -353,6 +389,7 @@ class ChannelDownloader(Downloader, ABC): async def do_download(self, download: Download) -> DownloadResult: """Update a Channel's catalog, then schedule downloads of every missing video.""" + _log_ytdlp_versions() try: info = extract_info(download.url, process=False) except Exception as e: @@ -537,6 +574,7 @@ class VideoDownloader(Downloader, ABC): return file_groups async def do_download(self, download: Download) -> DownloadResult: + _log_ytdlp_versions() with get_db_session() as session: if download.attempts >= 10: raise UnrecoverableDownloadError('Max download attempts reached') diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py index 62cee2de..258d8752 100644 --- a/modules/videos/test/test_downloader.py +++ b/modules/videos/test/test_downloader.py @@ -963,3 +963,47 @@ def test_parse_ytdlp_progress_not_progress_line(): assert parse_ytdlp_progress('[info] Downloading video 1 of 1') is None assert parse_ytdlp_progress('') is None assert parse_ytdlp_progress('[download] Destination: video.mp4') is None + + +@pytest.mark.asyncio +async def test_fetch_video_duration_persistent_cache(test_download_cache_config): + """Video durations are persisted to the download cache config and reused on subsequent calls.""" + from modules.videos.downloader import _cache_video_duration + from wrolpi.downloader import get_download_cache_config + + url = 'https://www.youtube.com/watch?v=test123' + + # Verify cache is initially empty. + assert get_download_cache_config().video_durations == [] + + # Persist a duration directly. + _cache_video_duration(url, 120) + assert get_download_cache_config().video_durations == [[url, 120]] + + # Re-caching the same URL updates the entry (no duplicate). + _cache_video_duration(url, 200) + assert get_download_cache_config().video_durations == [[url, 200]] + + +@pytest.mark.asyncio +async def test_fetch_video_duration_cache_eviction(test_download_cache_config): + """Video duration cache evicts oldest entries when over the max size.""" + from modules.videos.downloader import _cache_video_duration, VIDEO_DURATION_CACHE_MAX_SIZE + from wrolpi.downloader import get_download_cache_config + + # Fill to max size. + for i in range(VIDEO_DURATION_CACHE_MAX_SIZE): + _cache_video_duration(f'https://example.com/video{i}', i) + assert len(get_download_cache_config().video_durations) == VIDEO_DURATION_CACHE_MAX_SIZE + + # Adding one more should evict the oldest (video0), keeping it at max size. + _cache_video_duration('https://example.com/new_video', 999) + durations = get_download_cache_config().video_durations + assert len(durations) == VIDEO_DURATION_CACHE_MAX_SIZE + # Newest entry is at the end. + assert durations[-1] == ['https://example.com/new_video', 999] + # Oldest entry (video0) was evicted. + durations_dict = dict(durations) + assert 'https://example.com/video0' not in durations_dict + # Second oldest (video1) is still present. + assert durations_dict['https://example.com/video1'] == 1 diff --git a/wrolpi/conftest.py b/wrolpi/conftest.py index f205ec5c..5726cd1b 100644 --- a/wrolpi/conftest.py +++ b/wrolpi/conftest.py @@ -44,7 +44,7 @@ from wrolpi.contexts import attach_shared_contexts, initialize_configs_contexts from wrolpi.dates import set_test_now from wrolpi.db import postgres_engine, get_db_args from wrolpi.downloader import DownloadManager, DownloadResult, Download, Downloader, \ - downloads_manager_config_context + downloads_manager_config_context, download_cache_config_context from wrolpi.errors import UnrecoverableDownloadError from wrolpi.files.models import Directory, FileGroup from wrolpi.switches import await_switches as await_switches_ @@ -279,6 +279,14 @@ def test_download_manager_config(async_client, test_directory) -> Generator[Path yield config_path +@pytest.fixture +def test_download_cache_config(async_client, test_directory) -> Generator[Path, Any, None]: + with download_cache_config_context(): + (test_directory / 'config').mkdir(exist_ok=True) + config_path = test_directory / 'config/download_cache.yaml' + yield config_path + + @pytest.fixture async def await_switches(async_client): """Returns the `switches.await_switches` function, but primes the Sanic App to actually handle signals.""" diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index 3a5d30fa..9d086509 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -1784,6 +1784,61 @@ DOWNLOAD_MANAGER_CONFIG: DownloadManagerConfig = DownloadManagerConfig() TEST_DOWNLOAD_MANAGER_CONFIG: DownloadManagerConfig = None +@dataclass +class DownloadCacheConfigValidator: + version: int = None + video_durations: list = field(default_factory=list) + + +class DownloadCacheConfig(ConfigFile): + file_name = 'download_cache.yaml' + default_config = dict( + video_durations=list(), + version=0, + ) + validator = DownloadCacheConfigValidator + + @property + def video_durations(self) -> List[list]: + return self._config['video_durations'] or list() + + @video_durations.setter + def video_durations(self, value: List[list]): + self.update({'video_durations': value}) + + +DOWNLOAD_CACHE_CONFIG: DownloadCacheConfig = DownloadCacheConfig() +TEST_DOWNLOAD_CACHE_CONFIG: DownloadCacheConfig = None + + +@contextlib.contextmanager +def download_cache_config_context() -> Generator[DownloadCacheConfig, Any, None]: + """Used to create a test config.""" + global TEST_DOWNLOAD_CACHE_CONFIG + TEST_DOWNLOAD_CACHE_CONFIG = DownloadCacheConfig() + yield TEST_DOWNLOAD_CACHE_CONFIG + TEST_DOWNLOAD_CACHE_CONFIG = None + + +def get_download_cache_config() -> DownloadCacheConfig: + global TEST_DOWNLOAD_CACHE_CONFIG + if isinstance(TEST_DOWNLOAD_CACHE_CONFIG, ConfigFile): + return TEST_DOWNLOAD_CACHE_CONFIG + + global DOWNLOAD_CACHE_CONFIG + return DOWNLOAD_CACHE_CONFIG + + +@register_switch_handler('save_download_cache_config') +def save_download_cache_config(): + """Save the download cache config to disk.""" + get_download_cache_config().dump_config() + logger.info('save_download_cache_config completed') + + +save_download_cache_config: ActivateSwitchMethod + + @contextlib.contextmanager def downloads_manager_config_context() -> Generator[DownloadManagerConfig, Any, None]: """Used to create a test config."""