Commit Graph
16 Commits
Author SHA1 Message Date
peter b1832afd18 style: apply ruff format across backend + scripts (line-length 100)
Mechanical, whole-tree `ruff format` pass — no behaviour change. Recorded in
.git-blame-ignore-revs (next commit) so git blame skips it.
2026-08-23 07:43:06 +02:00
peter 8e4dee1bb6 refactor(downloads): introduce SourceAdapter — a thin per-source Strategy over yt-dlp
Consolidates source-specific policy (youtube / facebook / reddit / generic) that was scattered
across worker.py, backfill.py, service.py, formats.py and storage.py with no shared source
identity. app/downloads/adapters.py holds the base (= generic yt-dlp behaviour) + per-source
overrides; resolve_adapter(source_kind, source_ref, info) picks one by source_kind, the yt-dlp
extractor_key, or the URL host, falling back to generic. yt-dlp stays the engine — the adapter is
policy only.

Moved into adapters (behaviour-equivalent):
- uploader_url derivation (YouTube channel_url / Facebook uploader_id / reddit None) — removes the
  worker/backfill duplication (build_uploader_url + _should_refetch gone).
- the player-client fallback sets + retriable markers + YouTube extractor_args/POT (build_ydl_opts
  now takes source-agnostic opts plus an `extra` dict from the adapter).
- source_url reconstruction (service.source_url delegates to the adapter).

Two intentional (approved) behaviour changes:
- the player-client fallback is now YouTube-only; non-YouTube sources make a single extraction
  attempt instead of a pointless double attempt with youtube-only client args.
- the .nfo <uniqueid type=...> follows the source (adapter.nfo_uniqueid_type) instead of the
  hardcoded "youtube" that mislabelled every url-source download.

Deliberately left generic (documented non-moves): title cleaning (normalize_title is harmless
across sources), the classify marker table, thumbnail_to_jpg, the format selectors, and og.py's
YouTube-hqdefault og:image preference (a self-contained thumbnail heuristic on the share path).

Verified end-to-end on localdev across all four adapters: downloads succeed and uploader_url + the
.nfo uniqueid type are correct per source.
2026-08-13 00:34:42 +02:00
peter fea6f55d4f refactor(downloads): address /code-review on the download hotfix
- edit.py: extract _run_ffmpeg_best_effort (one run+timeout+swallow policy shared by
  thumbnail_to_jpg / ensure_poster / ensure_storyboard) + a build_thumbnail_jpg_cmd
  builder, matching the module's other ffmpeg commands.
- thumbnail_to_jpg: copy an already-JPEG thumbnail (Facebook, and Reddit's jpeg-under-
  a-.png-name) instead of re-encoding it; only webp/png-proper are decoded via ffmpeg.
- errors.py: drop the redundant "account authentication" classify marker
  ("authentication is required" already covers the known Reddit message).
- formats.py: document that the opt-in EmbedThumbnail is unaffected by dropping the
  convertor (it converts unsupported thumbs to PNG, not the crashing mjpeg2jpeg path).
- test_titles.py: guard the ReDoS regression with a thread + timeout (deterministic
  "must not hang") instead of a wall-clock assertion.
2026-08-12 18:47:01 +02:00
peter 37cca723fc fix(downloads): stop a ReDoS title and a fatal thumbnail convert from wedging/killing downloads
Three prod-observed download bugs, root-caused from a frozen prod queue with the
worker pinned at 100% CPU:

- ReDoS in normalize_title: _TRAILING_HASHTAGS had two overlapping unbounded
  quantifiers around the required letter, so a Facebook title with a long hashtag
  cluster that is NOT at the end ("... #a #b #c | Page") backtracked catastrophically.
  It runs inside the yt-dlp progress hook holding the GIL, so one such download froze
  the whole worker (and could stall feed enrichment too - sync/videos.py calls it).
  Rewrite with a zero-width "has a letter" lookahead + a possessive tag body: one
  unambiguous split, nothing to backtrack. 40-tag stress case: ~0.1ms.

- Reddit download killed by a cosmetic thumbnail convert: yt-dlp's
  FFmpegThumbnailsConvertor runs "-bsf:v mjpeg2jpeg" on a JPEG-served-under-a-.png-name
  (Reddit) and fails; being a postprocessor, it failed the WHOLE download even though
  the media was fine. Drop it and produce the <base>.jpg poster/og:image ourselves via
  editmod.thumbnail_to_jpg (plain ffmpeg decode->mjpeg, best-effort -> ensure_poster
  cuts a frame on failure). The FB Messenger / MS Teams share preview keeps working
  (posters verified as real JPEGs).

- classify: map "authentication is required" (Reddit's auth wall) to login_required
  instead of unknown, so the UI explains it rather than offering a pointless retry.

Tests: ReDoS-timing + numeric-marker regression, the Reddit classify case, and
thumbnail_to_jpg (real JPEG out / None on bad input, never raises).
2026-08-12 18:27:35 +02:00
peter ebfddf330a fix(downloads): second-round review fixes (fork on re-download, title fallbacks)
- redownload now ALWAYS rebuilds into a fresh private asset instead of ever reusing a shared
  cached file: the cache sig is salted with job_id + current asset_id, so the fork is distinct
  from both the dedup pool and the job's own current asset. This resolves the co-held silent
  no-op (finding #1) — a shared asset is left intact for its other holders while THIS job
  re-fetches its own copy. Resolving the fork BEFORE releasing the old hold (and the unique
  salted key) also removes the IntegrityError-rollback-after-file-delete window (finding #2).
- format_sig gains an optional `salt` for that fork identity.
- titles: don't consume the space after `|`, so a "stats | #hashtags" remainder keeps the
  leading whitespace _TRAILING_HASHTAGS needs — and fall back to the prefix+hashtag-stripped
  text, so neither the stats nor the SEO hashtags are resurrected when a title empties (#3).
- worker: early-fill asset.title falls back to the video id so a title that cleaned to empty
  doesn't show a blank card while downloading (#5).
- Tests for the salt fork and the hashtag-remainder case.

(Finding #4 — quota on re-download — intentionally not changed: a re-download reuses the
existing job and re-fetches ~the same bytes, so calling check_enqueue would wrongly reject on
the job-count cap while footprint is unchanged.)
2026-07-22 02:48:16 +02:00
peter cae5702fbb fix(downloads): address code-review findings on the R2.5 mini-epic
- redownload: never clobber a co-held (deduped) asset. Drop the force-reset-to-pending;
  _release_asset already deletes+recreates a solo asset (genuine re-fetch), while a shared
  ready asset is left intact and the re-queued job re-points to it via _finish_from_cache —
  so one user's re-download can't delete another's file mid-serve (finding #1).
- redownload: reset queue_pos to the tail so a re-download doesn't front-run newer queued
  jobs on the worker's `ORDER BY queue_pos` claim (finding #3). Made service.next_queue_pos public.
- formats: video-only mp4 selector prefers every video-only option before any progressive
  `best`, so a "Video only" download stays audio-free whenever a video-only stream exists (#2).
- titles: a title that is only the engagement prefix no longer resurrects the stripped stats
  (return the stripped text / empty, not the raw) (#4).
- worker: probe the file's duration when yt-dlp gave none, so a long re-encode shows real
  progress instead of a frozen 0% (#5); tighten the ensure_browser_playable docstring to its
  actual mp4-profile scope (#6).
2026-07-22 02:21:05 +02:00
peter 8fd0049dce fix(downloads): make shared files iOS-playable + default to 720p
Shared /watch links showed a "broken play button" on iPad (any iOS
browser — all forced onto WebKit) while playing fine on desktop Chrome.
Root cause: the stored mp4's video codec was AV1 (confirmed via ffprobe),
which WebKit can't decode. The existing H.264 guard was only a
format_sort *preference*, so a non-YouTube source (Facebook) that ships
AV1 as its best video-only stream slipped through.

- B1: for the mp4 compat profile, prefer H.264 at the SELECTION level
  with a progressive fallback (bestvideo[vcodec^=avc1]+bestaudio /
  best[vcodec^=avc1] / anything) so an avc1 progressive is chosen over
  an AV1-only video-only stream.
- B2: re-encode the rare AV1/VP9-only result to H.264+AAC (+faststart)
  in the worker before storing, gated on the compat profile; corrects
  the asset codec metadata to match.
- Bump _SIG_VERSION 2->3 so cached AV1 assets re-derive.
- Default download preset is now 720p (migration 0058 re-seeds builtins
  720p-first; ProfileEditor BLANK 1080->720).
- Tests for the selector, compat opt-outs, and the transcode predicate.
2026-07-22 01:19:31 +02:00
peter 4b22a7fa23 Merge branch 'dev' into chore/code-hygiene 2026-07-11 16:22:39 +02:00
peter 006f38382b fix(downloads): repair 5 confirmed backend bugs (B1-B5)
- B1 (sticky errored asset): get_or_create_asset now resets a reused status=='error'
  asset back to 'pending' (+clears the error), so a fresh enqueue/edit of a once-failed
  (source,format) pair actually re-downloads instead of the worker short-circuiting the
  new job with the stale error. Errored assets carry no expires_at, so without this the
  pair was permanently poisoned for all users until someone hit resume.
- B2 (ref_count leak): _release_asset counts 'error' as a holding state, so deleting an
  errored job decrements the ref_count that enqueue always incremented (the worker never
  decrements on failure). Errored rows are deliberately NOT deleted here — a concurrent
  B1 reuse could otherwise be lost-updated + FK-nulled; the row is fileless and harmless.
- B3: admin storage dashboard reads sysconfig.get_int(db,'download_total_max_bytes')
  (the admin-editable DB value GC enforces) instead of the raw env default.
- B4: the single-trim branch of normalize_edit_spec guards its float() coercion like the
  crop/segments branches — a malformed trim now yields a 400, not an unhandled 500.
- B5: formats.normalize guards int(max_height) → falls back to "best" instead of 500.

Reviewed (race in an earlier B2 draft caught + fixed); localdev boots, B4/B5 unit-verified.
2026-07-11 16:15:12 +02:00
peter efcac631de chore(downloads): C1+C2 — drop dead target_ext; centralize path-traversal guard
- C1: remove downloads/formats.py target_ext() — defined but never called (the
  worker derives the real extension from the produced file's suffix).
- C2: the download-root containment+existence guard was copy-pasted 6× across the
  file-serving endpoints (routes/downloads.py ×3, routes/public.py ×3). Extract
  storage.safe_abs_path(root, rel) -> Path|None so this security-sensitive check
  lives in one place; behavior identical (same containment test + messages). The
  extraction also made `pathlib.Path` unused in both route modules (removed).
2026-07-11 05:41:43 +02:00
peter ddfe1854e9 fix(downloads): prefer H.264+AAC for mp4 so shared files play on iOS/Safari
mp4 downloads used yt-dlp's default best video+audio, which on YouTube is
VP9+Opus at 1080p. Chromium plays that, but iOS/Safari WebKit (which every
iOS browser is forced to use) decodes only H.264/H.265+AAC inside mp4 — so a
shared /watch link showed a broken player on iPhone/iPad while the download
still worked. Rank vcodec:h264 + acodec:aac above resolution for mp4 output
(unless a custom profile pins a vcodec), keeping VP9/AV1 as a graceful
fallback. Bump the format signature (v2) so an identical spec gets a fresh
cache identity instead of hitting a stale VP9 asset.
2026-07-04 17:29:05 +02:00
peter b5a8b7d740 feat(downloads): automatic PO-token provider (bgutil sidecar) to beat YouTube bot-detection
YouTube increasingly demands a Proof-of-Origin token (or shows 'confirm you're not a bot').
Instead of manual cookies, run the bgutil POT provider as a sidecar that mints tokens on demand:
- compose: bgutil-pot service (brainicism/bgutil-ytdlp-pot-provider:1.3.1, port 4416, init)
- requirements: bgutil-ytdlp-pot-provider==1.3.1 plugin (version pinned to the image)
- config: DOWNLOAD_POT_BASE_URL (http://bgutil-pot:4416) + DOWNLOAD_PLAYER_CLIENTS
  (web_safari,web,tv — POT-capable; the default android_vr client never requests a token)
- formats.build_ydl_opts wires extractor_args youtube:player_client + youtubepot-bgutilhttp:base_url

Verified: the sidecar mints a valid PO token (BotGuard challenge solved) and yt-dlp fetches it
from the provider. NOTE: end-to-end download couldn't be confirmed here because the test IP got
hard-flagged by YouTube from heavy testing (~16 GB + dozens of extractions) — a hard abuse block
needs cooldown/cookies, which POT doesn't lift; POT bypasses the normal soft bot-check.
2026-07-03 04:40:10 +02:00
peter 89ccdbbaee improvement(downloads): no-embed builtin presets + 1080p default
Embedding (thumbnail/chapters) rewrote the whole file — a 10 GB video spent minutes making a
10 GB temp copy — while the Plex .nfo + poster sidecars already carry that metadata. So:
- builtin presets no longer embed (migration 0040 re-seeds them; formats._DEFAULTS embed off);
  only the tiny FFmpegThumbnailsConvertor (for poster.jpg) remains, so downloads go
  video -> audio -> merge -> done with no full-file rewrite
- default preset is now 1080p (Best stays available, second in the list) so a long video
  doesn't silently pull ~10 GB and blow the quota
Users who want self-contained files can enable embedding in a custom profile.
2026-07-03 04:08:53 +02:00
peter d6e3b1c436 fix(downloads): force IPv4 (fixes container 'Errno -5' DNS failures) + retry resets errored asset
Root cause of the failing downloads: containers usually have no working IPv6 route, but
googlevideo CDN hosts advertise AAAA records, so the downloader tried IPv6 and failed with
'[Errno -5] No address associated with hostname'. Small clips happened to use IPv4; long videos
(more CDN requests / the ffmpeg path) hit IPv6 and died.
- yt-dlp source_address=0.0.0.0 forces IPv4 for every connection (== --force-ipv4). Verified: the
  full video+audio download completes cleanly.
- Also: retry (resume) now resets the shared asset from 'error' to 'pending', so it actually
  re-downloads — previously the requeued job instantly re-inherited the asset's stale error
  because the worker short-circuits a job whose asset already errored.
- Dropped the localdev DNS override (wrong hypothesis; the issue was IPv6, not the resolver).
- Kept yt-dlp retries/fragment_retries/socket_timeout for transient blips.

Verified end-to-end in the worker: two previously-failing long videos now download to done.
2026-07-03 03:47:13 +02:00
peter 598c097289 feat(downloads): clear multi-phase download progress + transient-error retries
The progress bar looked broken (95% -> 5% -> recount) because yt-dlp downloads the video and
audio streams separately (each 0->100%) then merges — with no indication of which step is
running. Now:
- worker labels the phase from the stream codecs (video / audio) and via a postprocessor hook
  (merging / processing), so the user sees what's happening
- DownloadCenter shows the phase as the status and renders an indeterminate pulse (no bogus %)
  during merge/processing; phase i18n en/hu/de
- yt-dlp retries (3) + fragment_retries (5) so transient network blips self-heal instead of
  failing the job (seen once as a DNS 'No address associated with hostname' error)

Verified: phase transitions queued -> video -> processing -> done.
2026-07-03 02:42:36 +02:00
peter 81941cb8e8 fix(downloads): add the downloads package (was wrongly gitignored) + robust sanitizer
The .gitignore 'downloads/' pattern for the DOWNLOAD_ROOT bind mount also matched the
backend/app/downloads source package, so formats.py/storage.py/service.py never got
committed with M2. Anchor the ignore to '/downloads/' (repo root only) and add the package.

storage.sanitize() now handles arbitrarily messy titles (emoji, ZWJ, fullwidth, clickbait
punctuation): NFKC-normalize, drop emoji/symbol/control unicode categories, collapse
repeated punctuation, underscore-join words -> space-free paths. Accents (HU/DE) preserved,
no ASCII folding. Plex layout uses _-_ separators + Season_{year}.
2026-07-03 00:19:22 +02:00