Commit Graph
14 Commits
Author SHA1 Message Date
peter 657224c1ad refactor(downloads): address /code-review on the download supervisor
- worker.py: wrap the watchdog loop so a failure in it (e.g. a transient DB error in
  _job_liveness) still hard-kills the child + fails the job, instead of leaving the child
  running unsupervised — the exact wedge the feature prevents (#1).
- worker.py: _touch_job heartbeat bumps updated_at at each extraction attempt, so a slow /
  multi-client-set fallback extraction (which writes no progress, and re-writes identical
  values so ORM onupdate does not fire) is not false-killed as "stalled" (#2).
- edit.py: extract escalate_kill (terminate->wait->kill escalation); _stop and worker's
  _stop_child (process-group variant) now share it (#3).
- worker.py: document the edit path's second, tighter run_ffmpeg watchdog and that the
  supervisor is POSIX/Linux-container-only (#4, #6).
- tests: a long download stays alive while progress is fresh, and a watchdog-loop error
  still stops the child (#1, #5).
2026-08-12 23:04:48 +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 c4942eb590 fix(r5): address the fifth /code-review high round — 6 findings
- run_ffmpeg stops the process from ONE exit path (`except BaseException`), so
  an exception out of on_progress (a DB write) can no longer orphan a live
  ffmpeg after a 10s join on its pipes; _stop is now a no-op on a dead process
- both run_ffmpeg callbacks are throttled to ~1s (they were a DB round-trip per
  output line: ~10^5 queries on a long re-encode), mirroring the download hook
- volume arithmetic extracted to lib/playerVolume with unit tests; a corrupt
  level now falls back to the LAST GOOD one instead of full blast, and the
  sanitising happens where the stored value enters
- RSS is back to two except clauses (the shared tail already sits after the try)
- new tests: callback-exception cleanup, the two throttles; the fake Popen grew
  poll() and its wait count is now asserted

Mutation-checked: each new group goes red when its fix is reverted.
2026-07-24 01:50:50 +02:00
peter 2a7f49c981 fix(r5): address the /code-review high round — 10 findings
- HLS sweep only deletes directories matching the session-name shape; the root
  is admin-configurable and may be a shared path
- playlist unwrap picks the first entry that actually downloaded, not entries[0]
- concat quoting keeps backslashes literal (ffmpeg treats them so inside '')
- ArrowUp/Down defer while the focused element can still scroll that way
- the staging fallback skips yt-dlp working files (.part-Frag/.ytdl/.temp/.fNNN)
- the boot HLS sweep runs off the event loop
- volume persistence is debounced (was a setItem per wheel notch) and a spin no
  longer loses notches to the iframe's lagging getVolume
- _Breaker.run owns the whole open/call/record protocol; four call sites shrink
- should_prune extracted and unit-tested, plus RSS error-vs-empty tests
- the worker exits non-zero when the schema never lands
2026-07-23 23:25:06 +02:00
peter 63a36db116 fix(downloads): probe codecs from ffprobe JSON, not its flat output
ffprobe prints codec_name BEFORE codec_type, so the line parser attributed every
name to the previous stream: a plain H.264+AAC mp4 probed as (aac, None) and the
browser-playability check re-encoded a perfectly good file on EVERY download.
Verified on localdev — the same source now stores without a re-encode pass.
2026-07-23 22:56:42 +02:00
peter 2f46947dcf fix(downloads): R5 S1 — downloads robustness
- pick the produced file safely: Path("") is Path("."), which passes .exists(),
  so a missing yt-dlp filepath skipped the staging fallback and crashed in
  with_name ("PosixPath('.') has an empty name") on non-YouTube sources
- unwrap a playlist/multi_video extraction to the entry that carries the media
- run_ffmpeg: drain stderr while it runs (a full 64KB pipe blocked ffmpeg
  forever and hung the worker thread), 1s cancel polling, stall watchdog and a
  bounded wait on exit
- escape apostrophes/backslashes in the ffmpeg concat list (pre-0.51.0 names)
- sweep orphaned HLS segment dirs at startup (nothing else ever reaped them)
2026-07-23 22:32:25 +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 e46f215ada feat(downloads): ASCII-slug names, universal browser playability, force re-download
Mini-epic "Downloads correctness & UX" (S1–S3), on top of the iOS-AV1 + 720p fixes.

S1 — naming & titles:
- ASCII-fold on-disk dir/file names AND the served (Content-Disposition) filename: no
  spaces (underscore-joined), no accents (á→a, ő→o, ß→ss), pure ASCII — portable and free
  of the `?` mojibake accented names show in non-UTF-8 tools. (Forward-only; existing files
  clean up via re-download.)
- Strip a leading social engagement prefix ("1.6M views · 66K reactions | …") from titles.

S2 — universal browser playability (generalizes the iOS re-encode):
- ensure_browser_playable guarantees the stored file is H.264 + AAC/MP3 in mp4 — the only
  combo every browser decodes. Re-encode just the offending stream(s) (video for AV1/VP9/HEVC,
  audio for Opus/etc), or remux when only the container is wrong. Gated on the mp4 compat
  profile (an explicit vcodec / non-mp4 custom profile opts out).

S3 — force re-download (preserves share links):
- POST /downloads/{job_id}/redownload keeps the job row so its public DownloadLink survives
  and resolves to the freshly-downloaded file; deletes the old file and re-fetches under the
  current rules. A failed re-download leaves the job in error (the intended broken-link case).
- Library "Re-download" action (icon + confirm), api method, hu/en strings.

Tests: naming/title-strip (test_naming), browser_transcode_flags matrix; existing
download_filename expectation updated for the underscore policy.
2026-07-22 02:04:27 +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 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 8667161589 feat(downloads): clickable channels + poster fallback for thumbnail-less sources
Two visual gaps for non-catalog downloads:
- Channel link: YouTube already exposes channel_url; Facebook exposes none but a
  numeric uploader_id that resolves at facebook.com/<id>. `_uploader_url` derives
  it so the auto-detected channel renders as a real clickable link.
- Poster: a source with no thumbnail (e.g. a direct reddit HLS URL) showed a
  blank image box. The worker now cuts a representative frame with ffmpeg
  (`ensure_poster`) into the `<base>.jpg` sidecar and records `poster_path`
  (migration 0050). The card, the public watch page (<video poster> + og:image),
  and link previews fall back to it via new authed + public poster endpoints.

Adds `app.downloads.backfill` (one-off, re-run-safe) to fill uploader_url
(re-extract YouTube/Facebook metadata) and posters for pre-existing downloads.
2026-07-07 22:28:49 +02:00
peter a166770b7a feat(downloads): editor concat — multi-segment join (cut-list)
Editor v2 backend: an edit_spec can carry a 'segments' cut-list that concatenates the kept
ranges into ONE file (accurate=filter_complex trim+concat re-encode; fast=concat-demuxer
stream-copy with per-segment inpoint/outpoint). normalize/edit_sig/clip_duration/needs_reencode
handle segments; worker writes the concat list + runs build_concat_plan. Single-trim path (used by
'separate files' export) unchanged.
2026-07-04 03:15:45 +02:00
peter 470231516f feat(downloads): editor backend — trim/crop derivatives (phase 2)
Per-user trim/crop clips derived from a finished download via ffmpeg, riding the same
MediaAsset/DownloadJob/worker/quota/GC machinery (asset source_kind='edit', per-user cache key):
- migration 0041: download_jobs job_kind/source_job_id/source_asset_id/edit_spec
- app/downloads/edit.py: spec normalize+sig, ffmpeg trim/crop cmd (fast stream-copy vs frame-
  accurate re-encode, per-edit 'accurate' toggle), progress+cooperative-cancel runner, filmstrip
- service.enqueue_edit; worker _process_edit branch; storage.edit_rel_path (.edits/{user} tree)
- routes: POST /edit, GET /{id}/storyboard(.jpg)
2026-07-04 01:01:46 +02:00