diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3d5ce85..1492ab5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -30,6 +30,10 @@ env: PYTHON_VERSION: '3.12' APP_NAME: 'YoutubeWeekly' BINARIES_TAG: 'v0.0.0-binaries' + # yt-dlp runs YouTube's own JavaScript to solve the signature / "n" + # challenge. Without an engine, every format above 360p is dropped on the + # videos that demand it. quickjs-ng ships static single-file builds ~2MB. + QUICKJS_TAG: 'v0.16.2' FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true jobs: @@ -216,6 +220,13 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Download bundled QuickJS + run: | + New-Item -ItemType Directory -Force -Path app/tools/quickjs_win64 | Out-Null + curl.exe -L --fail --retry 5 --retry-delay 10 --retry-all-errors --connect-timeout 30 --max-time 300 ` + "https://github.com/quickjs-ng/quickjs/releases/download/${{ env.QUICKJS_TAG }}/qjs-windows-x86_64.exe" ` + -o app/tools/quickjs_win64/qjs.exe + - name: Inject version run: | $version = "${{ needs.create-release.outputs.version }}" @@ -284,6 +295,14 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Download bundled QuickJS + run: | + mkdir -p app/tools/quickjs_macOS/arm64 + curl -L --fail --retry 5 --retry-delay 10 --retry-all-errors --connect-timeout 30 --max-time 300 \ + https://github.com/quickjs-ng/quickjs/releases/download/${{ env.QUICKJS_TAG }}/qjs-darwin-arm64 \ + -o app/tools/quickjs_macOS/arm64/qjs + chmod +x app/tools/quickjs_macOS/arm64/qjs + - name: Inject version run: | sed -i '' "s/__version__ = \".*\"/__version__ = \"${{ needs.create-release.outputs.version }}\"/" app/backend/config.py @@ -353,6 +372,14 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Download bundled QuickJS + run: | + mkdir -p app/tools/quickjs_macOS/intel + curl -L --fail --retry 5 --retry-delay 10 --retry-all-errors --connect-timeout 30 --max-time 300 \ + https://github.com/quickjs-ng/quickjs/releases/download/${{ env.QUICKJS_TAG }}/qjs-darwin-x86_64 \ + -o app/tools/quickjs_macOS/intel/qjs + chmod +x app/tools/quickjs_macOS/intel/qjs + - name: Inject version run: | sed -i '' "s/__version__ = \".*\"/__version__ = \"${{ needs.create-release.outputs.version }}\"/" app/backend/config.py @@ -426,6 +453,14 @@ jobs: tar -xf ffmpeg-linux.tar.xz --strip-components=1 -C app/tools/ffmpeg_linux/ffmpeg-7.0.2-amd64-static chmod +x app/tools/ffmpeg_linux/ffmpeg-7.0.2-amd64-static/ffmpeg + - name: Download bundled QuickJS + run: | + mkdir -p app/tools/quickjs_linux + curl -L --fail --retry 5 --retry-delay 10 --retry-all-errors --connect-timeout 30 --max-time 300 \ + https://github.com/quickjs-ng/quickjs/releases/download/${{ env.QUICKJS_TAG }}/qjs-linux-x86_64 \ + -o app/tools/quickjs_linux/qjs + chmod +x app/tools/quickjs_linux/qjs + - name: Inject version run: | sed -i "s/__version__ = \".*\"/__version__ = \"${{ needs.create-release.outputs.version }}\"/" app/backend/config.py diff --git a/CHANGELOG.md b/CHANGELOG.md index dc6221a..ff1ab9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased +- **Children's Videos Download Again, in Full Quality**: Videos marked "Made for Kids" on YouTube — children's songs, Bible stories and the like — all failed with "This video is not available", even though they play perfectly in a browser. YouTube holds those back unless the app can run a piece of its code, so the app now carries the small engine needed. They download at full 1080p again, and any link that still refuses is retried a second way before the app gives up - **Sharper App Icon**: The icon shipped at a single small size, so Windows stretched it for shortcuts, folder views and notifications — it looked fuzzy nearly everywhere. It now includes every size Windows asks for, up to 256px ## v1.6.0 diff --git a/CHANGELOG_ro.md b/CHANGELOG_ro.md index 04d4dc3..5cb4fb2 100644 --- a/CHANGELOG_ro.md +++ b/CHANGELOG_ro.md @@ -1,6 +1,7 @@ # Istoric versiuni ## Unreleased +- **Videoclipurile pentru copii se descarcă din nou, la calitate maximă**: Videoclipurile marcate „Made for Kids” pe YouTube — cântece pentru copii, istorioare biblice și altele — eșuau toate cu „Acest videoclip nu este disponibil”, deși se redau perfect în browser. YouTube le reține dacă aplicația nu poate rula o bucată din codul lui, așa că aplicația include acum micul motor necesar. Se descarcă din nou la 1080p complet, iar un link care tot refuză este reîncercat pe a doua cale înainte ca aplicația să renunțe - **Pictogramă mai clară**: Pictograma era livrată într-o singură dimensiune mică, așa că Windows o întindea pentru scurtături, foldere și notificări — arăta neclară aproape peste tot. Acum include toate dimensiunile cerute de Windows, până la 256px ## v1.6.0 diff --git a/CLAUDE.md b/CLAUDE.md index 17adbc8..ba0feb8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -58,7 +58,7 @@ The build bundles config defaults from `config/`, platform binaries for mpv/ffmp - **Backend (`app/backend/`)**: Core business logic - `config.py`: Configuration management, platform-specific executable paths, `__version__` constant, app data dir (`CONFIG_DIR`); copies default configs from `config/` into the OS-specific app data dir on first run. `load_settings()` returns `(settings, warnings)`. - - `downloader.py`: yt-dlp-based downloads (find videos, quality/format selection, delete old videos, protected videos, recent Sabbath dates). Includes fuzzy date matching — handles off-by-one-day titles and formatting variants, and surfaces a confirmation prompt to the user. + - `downloader.py`: yt-dlp-based downloads (find videos, quality/format selection, delete old videos, protected videos, recent Sabbath dates). Includes fuzzy date matching — handles off-by-one-day titles and formatting variants, and surfaces a confirmation prompt to the user. Hands yt-dlp the bundled JS engine, and retries a failed download once across `FALLBACK_PLAYER_CLIENTS` — see "JavaScript engine (QuickJS)" and "Player-client fallback" below. - `auto_downloader.py`: Automatic download scheduling for upcoming Sabbath (Fri/Sat). - `updater.py`: Talks to the GitHub releases API (`ThorSPB/YoutubeWeekly`). Provides current-release check, paginated listing of available versions for rollback (`MIN_ROLLBACK_VERSION = 1.1.0`), and asset downloader with progress callback. Per-platform asset selection (`win64`, `macos-arm64`, `macos-intel`, `linux-x64`). - `update_bootstrap.py`: External process that swaps a downloaded ZIP over the running install. Uses `WaitForSingleObject` on Windows for reliable exit detection; renames the running bootstrap before extraction so it can update itself. @@ -146,6 +146,77 @@ Videos with "diaspora" in the title are excluded. The fuzzy matcher (since v1.1. - Date selector: "automat" (next Saturday) or a specific past Sabbath date (last 30 Saturdays). - "Others" section: paste any YouTube URL to download into the `other/` folder. Tracked separately in telemetry (since v1.2.0) so quality selection there is metered independently. +### JavaScript engine (QuickJS) + +**Downloads need a JavaScript engine.** yt-dlp solves YouTube's signature / +`n` challenge by *running YouTube's own JavaScript*, and some videos release no +adaptive-format URL until that is solved. Adaptive formats are the only route +above 360p (the sole surviving muxed format is 360p), so with no engine those +videos fail outright — with YouTube's flatly wrong "This video is not +available", because yt-dlp then falls through its clients until `visionos` +refuses. + +**The discriminator is YouTube's "Made for Kids" flag.** Measured 2026-09-07 +across 22 videos, no engine vs `quickjs`: **8/8 kids videos failed, 0/10 +non-kids failed**; every failure returned 1080p with an engine. The kids set +spanned five unrelated channels plus Cocomelon and Baby Shark, 2021 and 2026 +uploads, all `People & Blogs`, `isFamilySafe: true` in *both* groups. That is +why nothing else had ever failed: an ordinary video is served by `visionos` +with clean URLs and never needs JavaScript at all. + +- **`app/tools/quickjs_/qjs[.exe]`**, fetched per platform in + `release.yml` (pinned by `QUICKJS_TAG`) from **quickjs-ng**, which publishes + static single-file builds of ~2 MB. `app/tools` is bundled wholesale by the + spec, so no spec change is needed for a new binary under it. +- `config.py::get_default_executable_paths()` resolves it into + **`js_runtime_path`**, recomputed on every `load_settings()` like + `mpv_path`/`ffmpeg_path` — a saved path breaks when the app is moved or + updated. +- A missing engine adds **no user-facing warning**, unlike mpv and ffmpeg: it + is not exposed in Settings so there is nothing to configure, a source + checkout legitimately has none, and an empty path just means "let yt-dlp + auto-detect". `download_video` logs which engine it used instead, so a build + that shipped without the binary — or lost its executable bit in packaging — + is visible in the log rather than silently back to 360p-or-nothing. +- ⚠ **`js_runtimes` must be a dict** of `{runtime: {config}}`. A list raises a + bare `ValueError` from `YoutubeDL.__init__` that names no option. +- ⚠ **yt-dlp auto-enables only `deno`.** Node can sit on `PATH` completely + unused, which is what makes "no JS runtime" so easy to misdiagnose. + +Dead ends, all measured — don't re-walk them: +- **PO tokens are irrelevant here.** `bgutil-ytdlp-pot-provider` minted a real + GVS token and nothing changed. +- **Forcing `player_client=web` is SABR-only regardless**, so every web-client + probe looks like a dead end and sends you chasing SABR (yt-dlp #12482). +- Upgrading yt-dlp changes nothing (stable 2026.08.19 == nightly 2026.08.30). +- The `android` client alone "works" but serves only muxed **360p** — a trap + that looks like a fix. + +### Player-client fallback + +A safety net *behind* the JavaScript engine above, not a substitute for it: it +rescues a challenge-blocked video at 360p, where the engine gets 1080p. It +earns its place for the case where the bundled binary is missing or broken, and +for whatever YouTube does next. + +`download_video` treats a failure as inconclusive: it retries **once** with +`extractor_args={"youtube": {"player_client": FALLBACK_PLAYER_CLIENTS}}`. +Notes on that list and the retry: +- `default` stays **first** in it. Without it the retry would trade a perfectly + good 1080p stream for whatever the older clients happen to offer. +- The retry is deliberately **not** gated on the error text. YouTube's messages + churn, and a fix that pattern-matches them is a fix that quietly stops + working. The cost of an unnecessary retry is a few HTTP requests on a path + that was already failing. +- The error handed back to the caller is the **first** attempt's, not the + retry's: the retry ran against clients the user never chose, and its message + would only mislead. +- A retry restarts from zero bytes, so it emits `PROGRESS_RESET_STATUS` first. + `DownloadProgress` only ever moves forward, so without that the bar would sit + at the abandoned attempt's high-water mark. +- Every download in the app (channels, "Others", link overrides) goes through + `download_video`, so all of them get this. + ### Video Overrides A safety valve for the case the date matcher structurally cannot handle: the uploader typed the wrong date. Real example (2026-08-15): ScoalaDeSabat titled diff --git a/app/backend/config.py b/app/backend/config.py index 78469fd..50f8ff3 100644 --- a/app/backend/config.py +++ b/app/backend/config.py @@ -65,24 +65,29 @@ def get_default_executable_paths(): base_path = os.path.abspath(os.path.join(os.path.dirname(__file__), '..')) # This should be YoutubeWeekly/app mpv_path = "" ffmpeg_path = "" + js_runtime_path = "" warnings = [] system = platform.system() if system == "Windows": mpv_candidate = os.path.join(base_path, "player", "win64", "mpv-x86_64-20250715-git-fdbea0f", "mpv.exe") ffmpeg_candidate = os.path.join(base_path, "tools", "ffmpeg_win64", "ffmpeg-7.1.1-essentials_build", "bin", "ffmpeg.exe") + qjs_candidate = os.path.join(base_path, "tools", "quickjs_win64", "qjs.exe") elif system == "Darwin": # macOS if platform.machine() == "arm64": mpv_candidate = os.path.join(base_path, "player", "macOS", "arm64", "mpv-arm64-0.40.0", "mpv.app", "Contents", "MacOS", "mpv") ffmpeg_candidate = os.path.join(base_path, "tools", "ffmpeg_macOS", "ffmpeg711arm", "ffmpeg") + qjs_candidate = os.path.join(base_path, "tools", "quickjs_macOS", "arm64", "qjs") else: # Intel mpv_candidate = os.path.join(base_path, "player", "macOS", "intel", "mpv-0.39.0", "mpv.app", "Contents", "MacOS", "mpv") ffmpeg_candidate = os.path.join(base_path, "tools", "ffmpeg_macOS", "ffmpeg71intel", "ffmpeg") + qjs_candidate = os.path.join(base_path, "tools", "quickjs_macOS", "intel", "qjs") elif system == "Linux": # Assuming a 64-bit Linux for now, adjust if 32-bit is needed mpv_candidate = "/usr/bin/mpv" # Placeholder, as you didn't provide a bundled Linux mpv ffmpeg_candidate = os.path.join(base_path, "tools", "ffmpeg_linux", "ffmpeg-7.0.2-amd64-static", "ffmpeg") - + qjs_candidate = os.path.join(base_path, "tools", "quickjs_linux", "qjs") + # Validate mpv path if os.path.exists(mpv_candidate) and os.access(mpv_candidate, os.X_OK): mpv_path = mpv_candidate @@ -95,7 +100,19 @@ def get_default_executable_paths(): else: warnings.append(f"Warning: Default FFmpeg executable not found or not executable at '{ffmpeg_candidate}'. Please configure FFmpeg path in settings.") - return {"mpv_path": mpv_path, "ffmpeg_path": ffmpeg_path}, warnings + # Validate the bundled JavaScript engine - see "JavaScript engine (QuickJS)" + # in CLAUDE.md for why downloads need one at all. + # + # Deliberately adds NO warning when it is missing, unlike mpv and ffmpeg: + # there is nothing for the user to configure (it is not exposed in + # Settings), a source checkout legitimately has no bundled binary, and + # yt-dlp falls back to auto-detecting a system runtime. An empty path here + # means "let yt-dlp decide", which is exactly the old behaviour. + if qjs_candidate and os.path.exists(qjs_candidate) and os.access(qjs_candidate, os.X_OK): + js_runtime_path = qjs_candidate + + return {"mpv_path": mpv_path, "ffmpeg_path": ffmpeg_path, + "js_runtime_path": js_runtime_path}, warnings def _merge_missing_defaults(settings): """Add keys present in bundled defaults but missing from the user's settings. @@ -110,7 +127,7 @@ def _merge_missing_defaults(settings): # Window geometries and bundled exe paths are runtime-only; defaults file # may carry stale values for them, so skip those keys during migration. - skip_keys = {"mpv_path", "ffmpeg_path"} + skip_keys = {"mpv_path", "ffmpeg_path", "js_runtime_path"} added = False for key, default_value in defaults.items(): @@ -142,6 +159,7 @@ def load_settings(): # Always use the bundled executables settings["mpv_path"] = default_paths["mpv_path"] settings["ffmpeg_path"] = default_paths["ffmpeg_path"] + settings["js_runtime_path"] = default_paths["js_runtime_path"] # Ensure video_folder is an absolute path relative to the executable video_folder = settings.get("video_folder", "data/videos") diff --git a/app/backend/downloader.py b/app/backend/downloader.py index c764f95..20b0152 100644 --- a/app/backend/downloader.py +++ b/app/backend/downloader.py @@ -5,7 +5,7 @@ import logging from datetime import datetime, timedelta from app.backend.config import load_settings, SETTINGS_FILE, settings_lock -from app.backend.progress import PROGRESS_PLAN_STATUS +from app.backend.progress import PROGRESS_PLAN_STATUS, PROGRESS_RESET_STATUS from tkinter import messagebox @@ -16,6 +16,16 @@ # "is it already downloaded?" check. PARTIAL_SUFFIXES = (".part", ".ytdl", ".temp") +# YouTube does not serve every video to every InnerTube client, and yt-dlp only +# asks the handful it defaults to. Some channels answer UNPLAYABLE - which +# surfaces as the flatly wrong "This video is not available" - on exactly those +# clients while an older one still hands over a real stream. Measured +# 2026-09-07 against the Mount Moriah Sabbath School channel: `visionos` and +# `android_vr` refuse every video on it, `android` serves them. So a failure is +# not proof the video is gone; it is worth asking again through a wider set of +# clients before telling the user it cannot be had. +FALLBACK_PLAYER_CLIENTS = ("default", "android", "ios", "web_safari", "tv") + def is_partial_download(filename): """True for a yt-dlp scratch file from a download that never finished.""" @@ -319,6 +329,7 @@ def download_video(video_url, video_folder, quality_pref="1080p", protect=False, settings, _ = load_settings() ffmpeg_path = settings.get("ffmpeg_path") + js_runtime_path = settings.get("js_runtime_path") ydl_opts = { 'outtmpl': os.path.join(video_folder, '%(title)s.%(ext)s'), @@ -331,6 +342,22 @@ def download_video(video_url, video_folder, quality_pref="1080p", protect=False, 'progress_hooks': [progress_hook] if progress_hook else [] } + if js_runtime_path: + # Hand yt-dlp the bundled QuickJS so it can run YouTube's own + # JavaScript and solve the signature / "n" challenge. Some videos - + # every "Made for Kids" one - release no adaptive-format URL until that + # is solved, and without an engine every format above 360p is dropped. + # + # Must be a DICT of {runtime: {config}}. A list raises a bare + # ValueError from YoutubeDL.__init__ that names no option. + ydl_opts['js_runtimes'] = {'quickjs': {'path': js_runtime_path}} + logging.info(f"JS engine: {js_runtime_path}") + else: + # Logged rather than warned so a build that shipped without the binary + # (or lost its executable bit in packaging) is visible in the log + # instead of silently reverting to 360p-or-nothing on those videos. + logging.info("JS engine: none bundled; leaving yt-dlp to auto-detect") + if quality_pref == "mp3": ydl_opts['postprocessors'].append({ 'key': 'FFmpegExtractAudio', @@ -338,45 +365,74 @@ def download_video(video_url, video_folder, quality_pref="1080p", protect=False, 'preferredquality': '192', }) - with yt_dlp.YoutubeDL(ydl_opts) as ydl: + def attempt(extra_opts=None): + """One download run. Returns an error string, or None on success.""" + opts = dict(ydl_opts) + if extra_opts: + opts.update(extra_opts) + with yt_dlp.YoutubeDL(opts) as ydl: + try: + logging.info(f"Downloading: {video_url} with quality {quality_pref}") + if progress_hook: + # Tell the UI how much is coming before any bytes move, so + # its bar is weighted by real sizes rather than stream count. + plan = build_download_plan(ydl, video_url) or {} + try: + progress_hook({ + "status": PROGRESS_PLAN_STATUS, + "streams": plan.get("streams"), + "total_bytes": plan.get("total_bytes"), + }) + except Exception: + pass + ydl.download([video_url]) + logging.info("Download complete.") + if protect: + # Get video info to accurately identify the downloaded file + info = ydl.extract_info(video_url, download=False) + if info: + # Construct the expected filename based on yt-dlp's output template + # This assumes the default outtmpl: '%(title)s.%(ext)s' + video_filename = f"{info.get('title')}.{info.get('ext')}" + add_protected_video(os.path.basename(video_folder), video_filename) + except yt_dlp.utils.DownloadError as e: + error_message = str(e) + logging.error(f"Download failed: {error_message}") + # A partly-transferred video-only stream is worse than nothing: + # it is offered as playable (picture, no sound) and its name + # blocks the retry. Take it with us. + purge_partial_downloads(video_folder) + return error_message + except Exception as e: + error_message = str(e) + logging.error(f"An unexpected error occurred during download: {error_message}") + purge_partial_downloads(video_folder) + return error_message + return None + + error = attempt() + if error is None: + return None + + # Ask again through the wider client list before giving up - see + # FALLBACK_PLAYER_CLIENTS. Nothing has been transferred that survived the + # purge above, so the bar has to start over too. + clients = ",".join(FALLBACK_PLAYER_CLIENTS) + logging.info(f"Retrying with player clients: {clients}") + if progress_hook: try: - logging.info(f"Downloading: {video_url} with quality {quality_pref}") - if progress_hook: - # Tell the UI how much is coming before any bytes move, so its - # bar is weighted by real sizes rather than stream count. - plan = build_download_plan(ydl, video_url) or {} - try: - progress_hook({ - "status": PROGRESS_PLAN_STATUS, - "streams": plan.get("streams"), - "total_bytes": plan.get("total_bytes"), - }) - except Exception: - pass - ydl.download([video_url]) - logging.info("Download complete.") - if protect: - # Get video info to accurately identify the downloaded file - info = ydl.extract_info(video_url, download=False) - if info: - # Construct the expected filename based on yt-dlp's output template - # This assumes the default outtmpl: '%(title)s.%(ext)s' - video_filename = f"{info.get('title')}.{info.get('ext')}" - add_protected_video(os.path.basename(video_folder), video_filename) - except yt_dlp.utils.DownloadError as e: - error_message = str(e) - logging.error(f"Download failed: {error_message}") - # A partly-transferred video-only stream is worse than nothing: it - # is offered as playable (picture, no sound) and its name blocks - # the retry. Take it with us. - purge_partial_downloads(video_folder) - return error_message - except Exception as e: - error_message = str(e) - logging.error(f"An unexpected error occurred during download: {error_message}") - purge_partial_downloads(video_folder) - return error_message - return None # Return None on successful download + progress_hook({"status": PROGRESS_RESET_STATUS}) + except Exception: + pass + retry_error = attempt({ + "extractor_args": {"youtube": {"player_client": list(FALLBACK_PLAYER_CLIENTS)}}, + }) + if retry_error is None: + logging.info(f"Succeeded on retry with player clients: {clients}") + return None + # Report the first failure: the retry's message is about the fallback + # clients and would only mislead about what the user actually asked for. + return error def get_recent_sabbaths(n=30, date_format="%d.%m.%Y"): """Return the last `n` Sabbath (Saturday) dates formatted.""" diff --git a/app/backend/progress.py b/app/backend/progress.py index e479f9d..0e8e058 100644 --- a/app/backend/progress.py +++ b/app/backend/progress.py @@ -22,6 +22,12 @@ # status itself. PROGRESS_PLAN_STATUS = "ytw_plan" +# Synthetic event for "throw away what you have and start over" - emitted when a +# download is retried from scratch. The model's percentage only ever moves +# forward, so without this a retry would sit at the failed attempt's high-water +# mark instead of tracking the transfer actually in flight. +PROGRESS_RESET_STATUS = "ytw_reset" + def streams_implied_by(info): """How many streams a download will fetch, judged from its first one. diff --git a/app/frontend/gui.py b/app/frontend/gui.py index 1e9ca6d..811efa6 100644 --- a/app/frontend/gui.py +++ b/app/frontend/gui.py @@ -20,7 +20,9 @@ list_playable_files, folder_snapshot, newly_downloaded_file, purge_partial_downloads, ) -from app.backend.progress import DownloadProgress, PROGRESS_PLAN_STATUS +from app.backend.progress import ( + DownloadProgress, PROGRESS_PLAN_STATUS, PROGRESS_RESET_STATUS, +) from app.backend.changelog import load_changelog, notes_since, all_notes from datetime import datetime from app.frontend.settings_window import SettingsWindow @@ -1092,6 +1094,12 @@ def progress_hook(self, d): status = d.get("status") + if status == PROGRESS_RESET_STATUS: + # A retry is starting from zero bytes; so must the bar. + self._progress.reset() + self._render_progress(0.0) + return + if status == PROGRESS_PLAN_STATUS: self._progress.plan(streams=d.get("streams"), total_bytes=d.get("total_bytes")) diff --git a/tests/test_config_loading.py b/tests/test_config_loading.py index 2cd00fd..de7e774 100644 --- a/tests/test_config_loading.py +++ b/tests/test_config_loading.py @@ -160,3 +160,77 @@ def test_load_settings_no_write_when_nothing_to_merge(tmp_path, monkeypatch): assert mtime_after == mtime_before, "load_settings rewrote the file when no migration was needed" assert settings["default_quality"] == "720p" + + +# --- bundled JavaScript engine ------------------------------------------- +# +# yt-dlp runs YouTube's own JavaScript to solve the signature / "n" challenge. +# Videos that demand it - every "Made for Kids" one - release no +# adaptive-format URL until it is solved, so without an engine every format +# above 360p is dropped and the download fails outright. + +def _fake_bundle(tmp_path, monkeypatch, *, with_qjs, system="Linux", machine="x86_64"): + """Point config at a fake app/ tree, optionally carrying a qjs binary.""" + import os + app_dir = tmp_path / "app" + (app_dir / "tools").mkdir(parents=True) + if with_qjs: + rel = {"Linux": ("quickjs_linux", "qjs"), + "Windows": ("quickjs_win64", "qjs.exe"), + "Darwin": (os.path.join("quickjs_macOS", machine_dir(machine)), "qjs")}[system] + d = app_dir / "tools" / rel[0] + d.mkdir(parents=True, exist_ok=True) + exe = d / rel[1] + exe.write_text("#!/bin/sh\nexit 0\n") + exe.chmod(0o755) + monkeypatch.setattr("app.backend.config.platform.system", lambda: system) + monkeypatch.setattr("app.backend.config.platform.machine", lambda: machine) + monkeypatch.setattr("app.backend.config.os.path.dirname", + lambda _p: str(app_dir / "backend")) + return app_dir + + +def machine_dir(machine): + return "arm64" if machine == "arm64" else "intel" + + +@pytest.mark.parametrize("system, machine, expected_tail", [ + ("Linux", "x86_64", ("quickjs_linux", "qjs")), + ("Windows", "AMD64", ("quickjs_win64", "qjs.exe")), + ("Darwin", "arm64", ("arm64", "qjs")), + ("Darwin", "x86_64", ("intel", "qjs")), +]) +def test_bundled_js_engine_is_found_on_every_platform( + tmp_path, monkeypatch, system, machine, expected_tail): + import os + from app.backend.config import get_default_executable_paths + _fake_bundle(tmp_path, monkeypatch, with_qjs=True, system=system, machine=machine) + paths, _ = get_default_executable_paths() + found = paths["js_runtime_path"] + assert found, f"no js_runtime_path resolved for {system}/{machine}" + assert os.path.basename(found) == expected_tail[1] + assert expected_tail[0] in found + + +def test_missing_js_engine_is_silent_not_a_user_warning(tmp_path, monkeypatch): + """A source checkout has no bundled binary, and there is nothing for the + user to configure - it is not exposed in Settings. An empty path means + "let yt-dlp auto-detect", which is the old behaviour, so nagging about it + would be noise in the same list that reports a genuinely broken mpv.""" + from app.backend.config import get_default_executable_paths + _fake_bundle(tmp_path, monkeypatch, with_qjs=False) + paths, warnings = get_default_executable_paths() + assert paths["js_runtime_path"] == "" + assert not [w for w in warnings if "quickjs" in w.lower() or "javascript" in w.lower()] + + +def test_load_settings_always_resolves_the_js_engine(mock_settings_file, monkeypatch): + """Like mpv_path and ffmpeg_path, it is recomputed on every load rather + than trusted from the settings file - a saved path breaks the moment the + app is updated or moved.""" + monkeypatch.setattr("app.backend.config.SETTINGS_FILE", str(mock_settings_file)) + monkeypatch.setattr("app.backend.config.get_default_executable_paths", + lambda: ({"mpv_path": "", "ffmpeg_path": "", + "js_runtime_path": "/bundled/qjs"}, [])) + settings, _ = load_settings() + assert settings["js_runtime_path"] == "/bundled/qjs" diff --git a/tests/test_downloader.py b/tests/test_downloader.py index 80dbb24..28368ae 100644 --- a/tests/test_downloader.py +++ b/tests/test_downloader.py @@ -1,9 +1,12 @@ import pytest import os import json +import yt_dlp from unittest.mock import patch, MagicMock from datetime import datetime, timedelta +from app.backend.progress import PROGRESS_PLAN_STATUS, PROGRESS_RESET_STATUS from app.backend.downloader import ( + FALLBACK_PLAYER_CLIENTS, load_protected_videos, add_protected_video, get_next_saturday, @@ -250,10 +253,114 @@ def test_download_video_protect(mock_download_dependencies): def test_download_video_download_failure(mock_download_dependencies): mock_download_dependencies["mock_ydl_instance"].download.side_effect = Exception("Download error") + error = download_video("http://example.com/video", "/tmp/videos") + # Two runs: the plain one, then the wider-player-client retry. + assert mock_download_dependencies["mock_ydl_instance"].download.call_count == 2 + # The message the user sees describes the download they asked for, not the + # fallback clients they never chose. + assert error == "Download error" + + +def _opts_of(call): + args, _ = call + return args[0] + + +def test_download_video_hands_yt_dlp_the_bundled_js_engine(mock_download_dependencies, monkeypatch): + """Without a JS engine yt-dlp cannot solve YouTube's signature / "n" + challenge, and every format above 360p is dropped on the videos that + demand it (all "Made for Kids" ones).""" + monkeypatch.setattr("app.backend.downloader.load_settings", + lambda: ({"ffmpeg_path": "/usr/bin/ffmpeg", + "js_runtime_path": "/bundled/qjs"}, [])) + download_video("http://example.com/video", "/tmp/videos") + opts = _opts_of(mock_download_dependencies["mock_ydl"].call_args_list[0]) + # A DICT of {runtime: {config}}. Passing a list raises a bare ValueError + # from YoutubeDL.__init__ that names no option, so the shape is the test. + assert opts["js_runtimes"] == {"quickjs": {"path": "/bundled/qjs"}} + + +def test_download_video_omits_js_runtimes_when_none_is_bundled(mock_download_dependencies, monkeypatch): + """An absent engine must leave yt-dlp to auto-detect a system one rather + than being pinned to a path that does not exist.""" + monkeypatch.setattr("app.backend.downloader.load_settings", + lambda: ({"ffmpeg_path": "/usr/bin/ffmpeg", + "js_runtime_path": ""}, [])) + download_video("http://example.com/video", "/tmp/videos") + opts = _opts_of(mock_download_dependencies["mock_ydl"].call_args_list[0]) + assert "js_runtimes" not in opts + + +def test_download_video_js_engine_survives_the_player_client_retry(mock_download_dependencies, monkeypatch): + """The retry must not drop it - a fallback that cannot solve the challenge + is exactly the 360p-or-nothing case this engine exists to avoid.""" + monkeypatch.setattr("app.backend.downloader.load_settings", + lambda: ({"ffmpeg_path": "/usr/bin/ffmpeg", + "js_runtime_path": "/bundled/qjs"}, [])) + instance = mock_download_dependencies["mock_ydl_instance"] + instance.download.side_effect = [Exception("boom"), None] download_video("http://example.com/video", "/tmp/videos") - mock_download_dependencies["mock_ydl_instance"].download.assert_called_once() - # Assert that logging.error was called, but mocking logging is more complex. - # For now, just ensure no other unexpected calls or crashes. + calls = mock_download_dependencies["mock_ydl"].call_args_list + assert len(calls) == 2 + for call in calls: + assert _opts_of(call)["js_runtimes"] == {"quickjs": {"path": "/bundled/qjs"}} + + +def _player_clients_of(call): + """The player_client list a YoutubeDL(...) call was given, if any.""" + args, _ = call + return (args[0].get("extractor_args") or {}).get("youtube", {}).get("player_client") + + +def test_download_video_no_retry_when_the_first_attempt_works(mock_download_dependencies): + assert download_video("http://example.com/video", "/tmp/videos") is None + calls = mock_download_dependencies["mock_ydl"].call_args_list + assert len(calls) == 1 + # A working download must not pay for extra InnerTube round-trips. + assert _player_clients_of(calls[0]) is None + + +def test_download_video_retries_across_more_player_clients(mock_download_dependencies): + """A channel YouTube hides from yt-dlp's default clients still downloads. + + Reproduces the 2026-09-07 report: `visionos` answers UNPLAYABLE for the + whole channel, so the first attempt fails with YouTube's misleading "This + video is not available" - and `android`, in the fallback list, serves it. + """ + instance = mock_download_dependencies["mock_ydl_instance"] + instance.download.side_effect = [ + yt_dlp.utils.DownloadError("ERROR: [youtube] X: This video is not available"), + None, + ] + + assert download_video("http://example.com/video", "/tmp/videos") is None + + calls = mock_download_dependencies["mock_ydl"].call_args_list + assert len(calls) == 2 + assert _player_clients_of(calls[0]) is None + assert _player_clients_of(calls[1]) == list(FALLBACK_PLAYER_CLIENTS) + # The default has to stay in the list, or the retry would trade a 1080p + # stream for whatever the older clients happen to offer. + assert "default" in FALLBACK_PLAYER_CLIENTS + + +def test_download_video_retry_restarts_the_progress_bar(mock_download_dependencies): + """The failed attempt's high-water mark must not stick. + + DownloadProgress only ever moves forward, so a retry that starts from zero + bytes needs an explicit reset or the bar reports the abandoned attempt. + """ + instance = mock_download_dependencies["mock_ydl_instance"] + instance.download.side_effect = [Exception("boom"), None] + statuses = [] + + download_video("http://example.com/video", "/tmp/videos", + progress_hook=lambda d: statuses.append(d.get("status"))) + + assert PROGRESS_RESET_STATUS in statuses + # It has to land between the two attempts' plans, not after them. + assert statuses.index(PROGRESS_RESET_STATUS) < len(statuses) - 1 + assert statuses[statuses.index(PROGRESS_RESET_STATUS) + 1] == PROGRESS_PLAN_STATUS # Test for get_recent_sabbaths @pytest.mark.parametrize("n, expected_sabbaths", [