YouTube import diagnostics, an opt-in cookies file, and 0.75x playback - #439
Merged
Conversation
added 6 commits
August 25, 2026 08:52
Below roughly 0.7x the time-stretch artefacts dominate and the part gets harder to follow, which is the opposite of what a practice speed is for. The preset list, the button id list in state.js and the markup all have to agree, so all three move together. applySpeed already snaps to the nearest preset, so a stored or stale 0.5 lands on 0.75 rather than being rejected. No i18n keys change: the labels are numeric.
Every yt-dlp failure arrived as the bare word "unknown", so diagnosing one meant reading data/logs/. Two independent causes, either enough on its own: - _CAUSE_PATTERNS had no download category at all, so bot checks, HTTP 429 and "Requested format is not available" all fell through to the honest default. - _quarantine_failed_job only appended a message when the exception carried a stderr tail, and only SeparationError does. A yt-dlp DownloadError never has one, so even a correct cause arrived with no detail. Adds source-blocked and source-unavailable, ordered after the resource-level causes so a disk-full during a download is still a disk-full, and falls back to the exception message when there is no tail. The message goes through redact() first: yt-dlp embeds the source URL in its errors and error_detail is served to the client and pasted into public reports.
download.py built its YoutubeDL options in four separate places: playlist expansion, the metadata probe, the audio fetch and the MP4 video fetch. They had already drifted (ffmpeg_location in one of four, noplaylist in three), and the drift is invisible: _download_video_track swallows every exception and falls back to audio-only, so an option missing there costs the user their MP4 export with nothing surfaced anywhere. _base_ydl_opts() now carries what all four must share. allowed_extractors stays a required argument because it is the SSRF boundary from #173 and the two valid values are genuinely different. On top of that, a cookies.txt path in Settings. YouTube's bot check has no other remedy in-tree, since yt-dlp ships no PO token generator, so an IP YouTube has flagged cannot import anything without credentials. A file path rather than cookiesfrombrowser: reading a live browser profile means touching the user's logged-in session and only works while that browser is closed. Empty by default, and that matters. Supplying cookies makes yt-dlp skip every client that does not support them, which removes the unauthenticated fallback clients that work for most people today. Turning this on when you do not need it makes imports worse. The field does not route through the shared post() helper, which drops a non-ok response silently. A path the user typed has to say when it is wrong. config.bundled_js_runtime() is the hook the challenge solver needs; it returns None until a build actually ships a runtime, so nothing changes yet. No dependency change, so uv.lock and the desktop runtimeId are untouched and existing installs can still take this as an in-app update.
Sending cookies on every request would have made imports worse for most people. yt-dlp skips every client that does not support cookies, and those skipped clients are exactly the ones resolving formats today without any JS challenge solver. The setting would have removed a working path in order to fix a broken one for the smaller group whose IP YouTube has flagged. So the first attempt never carries cookies. They are used only after YouTube has actually turned us away with a bot check or a 429. By then the path they displace has already failed, which is what makes the setting incapable of making anything worse. Same shape as separate()'s GPU to CPU retry: the fallback runs only once the primary path is known to be dead. The probe is where the bot check lands, so that is where the decision is made. Whether cookies were needed is returned rather than re-derived, and passed to the audio fetch and the video fetch so one job never spends a second round trip proving the same thing twice. Second half: when cookies clear the bot check and the job then dies for want of a solver, say so. Without it the user sees 'Requested format is not available' with no way to connect it to the setting they just turned on. The rewrite only happens when no JS runtime can be found, so a genuine format failure keeps its real message, and the original error stays chained.
Both video paths are best-effort by design, and they should stay that way: a track that separated fine must not fail because a video stream could not be had. But collapsing every outcome into has_video = False meant a user who imported a track specifically to export a karaoke video could not tell 'this never had video' from 'the video fetch broke', and nothing surfaced either. The job just completed, with the MP4 option quietly absent. video_status now records which it was: ok, unavailable when the source genuinely offers no video stream, failed when the fetch or extract errored. None stays the default for SoundCloud and non-mp4 uploads, which never try. Only 'failed' is shown. An absent MP4 button on an audio-only source is normal and not worth a message; a broken one nobody mentioned is the bug. The local .mp4 path gained the same distinction, and a try/except it did not have: a missing or timed-out ffmpeg used to surface as 'source has no video stream', which sends the user looking at their file instead of their install.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five commits, four issues, one shared root:
download.pybuilt its yt-dlp options in four separate places and every failure came back as the wordunknown.Closes #433
Closes #434
Closes #435
Closes #436
Partially addresses #432. The JS challenge solver is held for investigation and tracked in #438.
What changed
Playback speed (#433). Presets are 0.75x and 1x. Below roughly 0.7x the time-stretch artefacts dominate and the part gets harder to follow, which is the opposite of what a practice speed is for. The preset list, the button id list and the markup all have to agree, so all three move together. No i18n keys change, the labels are numeric.
Failure diagnostics (#434). Every yt-dlp failure arrived as the bare word
unknown, so diagnosing one meant readingdata/logs/. Two independent causes, either enough on its own:_CAUSE_PATTERNShad no download category, and_quarantine_failed_jobonly appended a message when the exception carried a stderr tail, which onlySeparationErrordoes. Addssource-blockedandsource-unavailable, ordered after the resource-level causes so a disk-full during a download is still a disk-full, and falls back to the exception message. The message goes throughredact()first, since yt-dlp embeds the source URL in its errors anderror_detailis served to the client.One option builder (#435). Playlist expansion, the metadata probe, the audio fetch and the MP4 video fetch each built their own options and had already drifted.
allowed_extractorsstays a required argument because it is the SSRF boundary from #173 and the two valid values are genuinely different.MP4 export failures are visible (#436). Both video paths stay best-effort, but
video_statusnow records which outcome it was:ok,unavailablewhen the source has no video stream,failedwhen the fetch or extract errored. Onlyfailedis shown. An absent MP4 button on an audio-only source is normal; a broken one nobody mentioned is the bug. The local.mp4path also gained atry/exceptit never had, so a missing or timed-out ffmpeg no longer reports as "source has no video stream".Opt-in cookies file (#432, partial). A cookies.txt path in Settings, empty by default. YouTube's bot check has no other remedy in-tree, since yt-dlp ships no PO token generator.
Cookies are a fallback, never the default path. Supplying them makes yt-dlp skip every client without
SUPPORTS_COOKIES, and those are exactly the clients resolving formats today without any JS challenge solver. Applying them to every request would have broken imports that currently work. So the first attempt never carries cookies and they are used only after YouTube has actually turned us away. Same shape asseparate()'s GPU to CPU retry: the fallback runs once the primary path is known to be dead.Verification
Beyond the unit tests, the download path was exercised against real YouTube:
download(): 3.4 MB fetched, title, duration, tags and video track all correctuse_cookieswasFalseon all three yt-dlp calls and the result was byte-identical, so the new setting cannot break an import that was going to succeedvideo_statusreachesto_state()correctly on a real job650 tests pass. The 14 failures on this branch also fail on
mainin this environment (ffmpeg not on PATH for beatgrid, the Linux executable bit, a CRLF assertion in the logs zip test).ruff checkandruff format --checkclean. i18n parity clean across all eight tables.Notes for review
uv.lockand the desktopruntimeIdare untouched and existing installs can take this as an in-app updatebundled_js_runtime()is wired into all four call sites but returnsNoneuntil a build ships a runtime, so it changes nothing yet. It is the hook Ship YouTube's JS challenge solver (yt-dlp-ejs + a bundled JS runtime) #438 needspost()helper, which drops a non-ok response silently