diff --git a/CHANGELOG.md b/CHANGELOG.md index ddbf2c08..47a6e808 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ edit the release notes and re-run the generator. Upgrading, pinning a version and rolling back: [docs/self-hosting.md](docs/self-hosting.md#upgrading). +## 0.101.0 — 2026-10-06 + +### New + +- A shared link plays on any phone, iPhone and iPad included: downloads, clips and uploads are kept in a format every phone plays, and an upload in another format is converted after it arrives. +- A shared link opens much faster: its page no longer loads the whole app. + +### Fixed + +- The YouTube player keeps its size on a short window and when maximised — before, it could shrink to a thin strip. Plus smaller fixes around uploads and clips. + ## 0.100.0 — 2026-10-05 ### New diff --git a/CLAUDE.md b/CLAUDE.md index 87ef68f4..b5d30889 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,7 +65,8 @@ how it gets built. Concretely: `downloads/`, `youtube/`, `issues/` (shared by routes + scheduler + worker) → flat core (`auth`, `quota`, `sysconfig`, `state`, `audit`, `notifications`, `userscope`, `ratelimit`, `bodylimit`, `progress`, `collation`, `security`, `logs`, `logsink`, `diagnostics`, `debugsession`) → `models/db/config`. **Dependency direction is one-way: core never imports routes.** -Frontend: `main.tsx` provider tree → `App.tsx` shell → `moduleRegistry` route-chunks → module +Frontend: `main.tsx` provider tree → `App.tsx` shell (a shared `/watch` link boots from its own +entry, `watch.html` → `watchMain.tsx`, held app-free by a contract test) → `moduleRegistry` route-chunks → module components → the **platform layer `components/ui/`** (domain-free primitives: the popover engine and its menus, `Modal`/`Overlay`, `Button`/`form`, `DataTable`/`VirtualGrid`, `QueryState`, …). That boundary is one-way and **lint-enforced**, not just described: a platform component may not import a diff --git a/VERSION b/VERSION index 897e2158..0f00ab7c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.100.0 +0.101.0 diff --git a/backend/app/downloads/edit.py b/backend/app/downloads/edit.py index 24498872..21b82983 100644 --- a/backend/app/downloads/edit.py +++ b/backend/app/downloads/edit.py @@ -188,6 +188,51 @@ def clip_duration(spec: dict, source_duration: int | None) -> int | None: # --- ffmpeg: trim / crop -------------------------------------------------------------------- +# Every libx264 / aac re-encode in this package uses these, so a re-encoded file is always the +# phone-safe shape (`playable`): `-pix_fmt yuv420p` pins 8-bit 4:2:0 — without it libx264 keeps a +# 10-bit or 4:4:4 source's pixel format and writes H.264 High 10 / High 4:4:4, which browsers decode +# only patchily (the one safe H.264 is 8-bit 4:2:0). +X264_ARGS = ["-c:v", "libx264", "-preset", "veryfast", "-crf", "20", "-pix_fmt", "yuv420p"] +AAC_ARGS = ["-c:a", "aac", "-b:a", "192k"] +# The last step of every video filter chain a libx264 re-encode runs: 4:2:0 halves the chroma, so +# libx264 refuses an odd width or height ("width not divisible by 2") — measured on an odd source +# that is not 4:2:0 already (a 4:4:4 or RGB screen capture, 1365x767). A crop of a 4:2:0 source is +# rounded to its chroma grid by ffmpeg itself; this holds every other path to the encoder to the same +# even frame. Rounding down to even costs at most one pixel row/column. +EVEN_FRAME = "scale=trunc(iw/2)*2:trunc(ih/2)*2" + +# The demuxers ffmpeg/ffprobe may use to open a library file — single-file containers only. Without +# it ffmpeg picks the demuxer from the file's CONTENT and NAME, and an upload's name is the user's: +# a playlist-type demuxer (HLS, DASH, concat) opens whatever local paths its entries name, so a +# crafted upload could copy another user's clip or a Plex file into its own converted asset. A file +# no demuxer here can open fails the probe, and an unprobeable upload is a download-only 'file'. +MEDIA_DEMUXERS = ( + "mov", # mp4, m4a, mov, 3gp + "matroska", # mkv, webm + "mpegts", + "mpeg", + "avi", + "asf", + "flv", + "ogg", + "mp3", + "aac", + "ac3", + "eac3", + "flac", + "wav", + "w64", + "aiff", + "caf", + "amr", +) +INPUT_GUARD = ["-format_whitelist", ",".join(MEDIA_DEMUXERS)] +# A downloaded thumbnail is the remote site's bytes under the suffix yt-dlp gave it (`.png`, `.webp`, +# `.jpg` — `pipeline._THUMB_SUFFIXES`): only the single-image demuxers those open, so a "thumbnail" +# that is really a playlist cannot pull a local file into the poster. Anything else fails, and the +# poster falls back to a cut frame (`ensure_poster`). +THUMB_GUARD = ["-format_whitelist", "png_pipe,jpeg_pipe,webp_pipe"] + def build_edit_cmd(src: Path, dest: Path, spec: dict, out_ext: str) -> list[str]: """ffmpeg command for a trim/crop edit. `-progress pipe:1` streams machine-readable progress. @@ -214,7 +259,7 @@ def build_edit_cmd(src: Path, dest: Path, spec: dict, out_ext: str) -> list[str] ] if start: cmd += ["-ss", f"{start:.3f}"] - cmd += ["-i", str(src)] + cmd += [*INPUT_GUARD, "-i", str(src)] if end is not None: dur = end - start if dur > 0: @@ -222,21 +267,9 @@ def build_edit_cmd(src: Path, dest: Path, spec: dict, out_ext: str) -> list[str] if reencode: # Input -ss + re-encode is frame-accurate (decodes from the prior keyframe, discards up to - # the exact cut). A crop adds the filter; an accurate trim re-encodes with no filter. - if crop: - cmd += ["-vf", f"crop={crop['w']}:{crop['h']}:{crop['x']}:{crop['y']}"] - cmd += [ - "-c:v", - "libx264", - "-preset", - "veryfast", - "-crf", - "20", - "-c:a", - "aac", - "-b:a", - "192k", - ] + # the exact cut). A crop adds the filter; an accurate trim only evens the frame. + cropf = f"crop={crop['w']}:{crop['h']}:{crop['x']}:{crop['y']}," if crop else "" + cmd += ["-vf", cropf + EVEN_FRAME, *X264_ARGS, *AAC_ARGS] else: cmd += ["-c", "copy", "-avoid_negative_ts", "make_zero"] @@ -284,11 +317,14 @@ def build_concat_plan(src: Path, dest: Path, spec: dict, out_ext: str, staging: parts, labels = [], [] for i, s in enumerate(segs): st, en = s["start_s"], s["end_s"] - parts.append(f"[0:v]trim=start={st}:end={en},setpts=PTS-STARTPTS{cropf}[v{i}]") + parts.append( + f"[0:v]trim=start={st}:end={en},setpts=PTS-STARTPTS{cropf},{EVEN_FRAME}[v{i}]" + ) parts.append(f"[0:a]atrim=start={st}:end={en},asetpts=PTS-STARTPTS[a{i}]") labels.append(f"[v{i}][a{i}]") parts.append(f"{''.join(labels)}concat=n={len(segs)}:v=1:a=1[v][a]") cmd = head + [ + *INPUT_GUARD, "-i", str(src), "-filter_complex", @@ -297,26 +333,23 @@ def build_concat_plan(src: Path, dest: Path, spec: dict, out_ext: str, staging: "[v]", "-map", "[a]", - "-c:v", - "libx264", - "-preset", - "veryfast", - "-crf", - "20", - "-c:a", - "aac", - "-b:a", - "192k", + *X264_ARGS, + *AAC_ARGS, ] else: listp = staging / "concat.txt" - lines = [] + lines = ["ffconcat version 1.0"] for s in segs: lines.append(f"file '{_concat_quote(src)}'") + # ffmpeg hands the list's whitelist (which must name `concat` to read the list) down to + # the file it names, so a library file whose bytes are a concat list would be read as one + # and open the paths inside it. Each entry's own whitelist replaces the inherited one. + lines.append(f"option format_whitelist {INPUT_GUARD[1]}") lines.append(f"inpoint {s['start_s']}") lines.append(f"outpoint {s['end_s']}") prep[listp] = "\n".join(lines) + "\n" - cmd = head + ["-f", "concat", "-safe", "0", "-i", str(listp), "-c", "copy"] + guard = ["-format_whitelist", "concat," + INPUT_GUARD[1]] + cmd = head + ["-f", "concat", "-safe", "0", *guard, "-i", str(listp), "-c", "copy"] if out_ext in ("mp4", "m4a", "mov"): cmd += ["-movflags", "+faststart"] @@ -474,14 +507,7 @@ def run_ffmpeg( raise StatedFailure(f"ffmpeg failed ({ret}): {''.join(err_tail)[-300:]}") -# --- ffmpeg: universal browser playability -------------------------------------------------- -# A downloaded file must play in EVERY browser, whatever the source shipped. Universal = H.264 -# video + AAC/MP3 audio in an mp4 container: H.264 is the only video codec all browsers decode -# (VP9/AV1 fail on Safari/iOS; HEVC fails on Chrome/Firefox), and mp4/AAC is the safe pairing. -# Anything else is fixed post-download: re-encode the offending stream(s), or just remux when the -# codecs are already fine but the container isn't. -_BROWSER_SAFE_VCODEC = ("h264",) -_BROWSER_SAFE_ACODEC = ("aac", "mp3") +# --- ffprobe --------------------------------------------------------------------------------- def run_ffprobe_json( @@ -493,10 +519,10 @@ def run_ffprobe_json( ) -> dict | None: """One ffprobe call → its parsed JSON dict, or None on any failure (timeout, non-zero exit, missing binary, unparseable output). The SINGLE ffprobe-JSON-invocation point in the downloads - package: the browser-playability codec probe here and the shared media probe (`probe.probe_media`, - used by both the download worker's finalize and the upload ingest) both go through it, so the + package: the shared media probe (`probe.probe_json`: the stored facts and the phone-safe rule) goes + through it, so the subprocess flags/timeout/error handling live in one place instead of drifting copies.""" - args = ["ffprobe", "-v", "error"] + args = ["ffprobe", "-v", "error", *INPUT_GUARD] if show_streams: args.append("-show_streams") if show_format: @@ -517,35 +543,6 @@ def run_ffprobe_json( return None -def _codecs_from_streams(streams: list) -> tuple[str | None, str | None]: - """(video_codec, audio_codec) lowercased from an ffprobe JSON stream list — the first of each kind. - - Parsed from the JSON `streams` list, NOT ffprobe's flat `default=nw=1` output: there the fields - arrive in ffprobe's own struct order, which puts `codec_name` BEFORE `codec_type`, so a line-by-line - "remember the last codec_type" parser attributes every name to the PREVIOUS stream. Measured on real - downloads: an ordinary H.264+AAC mp4 probed as `v=aac a=None` — the browser-playability check then - believed the video stream was AAC and re-encoded a perfectly fine file on every download. Reading each - stream dict by its own `codec_type` is immune to that ordering.""" - vcodec = acodec = None - for s in streams: - name = (s.get("codec_name") or "").lower() or None - ctype = (s.get("codec_type") or "").lower() - if ctype == "video" and vcodec is None: - vcodec = name - elif ctype == "audio" and acodec is None: - acodec = name - return vcodec, acodec - - -def probe_codecs(path: Path) -> tuple[str | None, str | None]: - """Return (video_codec, audio_codec) lowercased via one ffprobe call; None for a missing - stream or if ffprobe fails (a failed probe is treated as 'leave it alone' upstream).""" - data = run_ffprobe_json(path, entries="stream=codec_type,codec_name") - if data is None: - return None, None - return _codecs_from_streams(data.get("streams") or []) - - def probe_duration(path: Path) -> int: """Media duration in whole seconds via ffprobe, or 0 if unknown — a progress denominator for a re-encode when the yt-dlp info dict carried no `duration` (some URL/live extractions).""" @@ -555,6 +552,7 @@ def probe_duration(path: Path) -> int: "ffprobe", "-v", "error", + *INPUT_GUARD, "-show_entries", "format=duration", "-of", @@ -577,49 +575,6 @@ def probe_duration(path: Path) -> int: return 0 -def browser_transcode_flags( - vcodec: str | None, acodec: str | None, is_mp4: bool -) -> tuple[bool, bool, bool]: - """Decide what (if anything) makes a file universally browser-playable. Returns - (needs_change, reencode_video, reencode_audio). needs_change=False => already H.264 + AAC/MP3 - in mp4. A None codec (no such stream, or a failed probe) is never re-encoded.""" - rv = bool(vcodec) and vcodec not in _BROWSER_SAFE_VCODEC - ra = bool(acodec) and acodec not in _BROWSER_SAFE_ACODEC - return (rv or ra or not is_mp4), rv, ra - - -def build_browser_safe_cmd( - src: Path, dest: Path, reencode_video: bool, reencode_audio: bool, threads: int | None = None -) -> list[str]: - """ffmpeg command that lands `src` as a browser-safe mp4 (faststart): re-encode only the - stream(s) that need it (libx264 / aac), stream-copy the rest, always into mp4. `-progress - pipe:1` streams machine-readable progress for run_ffmpeg. `threads` caps the encoder so a - re-encode can't claim every core on the shared box (see the worker-child de-prioritization).""" - cmd = [ - "ffmpeg", - "-y", - "-hide_banner", - "-loglevel", - "error", - "-nostdin", - "-progress", - "pipe:1", - "-nostats", - "-i", - str(src), - ] - cmd += ( - ["-c:v", "libx264", "-preset", "veryfast", "-crf", "20"] - if reencode_video - else ["-c:v", "copy"] - ) - cmd += ["-c:a", "aac", "-b:a", "192k"] if reencode_audio else ["-c:a", "copy"] - if threads and threads > 0: - cmd += ["-threads", str(threads)] - cmd += ["-movflags", "+faststart", str(dest)] - return cmd - - # --- filmstrip (editor scrub track) --------------------------------------------------------- _SB_COLS = 12 @@ -655,6 +610,7 @@ def build_storyboard_cmd(src: Path, dest: Path, duration_s: int | None, geom: di "-loglevel", "error", "-nostdin", + *INPUT_GUARD, "-i", str(src), "-frames:v", @@ -678,6 +634,7 @@ def build_poster_cmd(src: Path, dest: Path, at_s: float) -> list[str]: "-nostdin", "-ss", f"{max(0.0, at_s):.3f}", + *INPUT_GUARD, "-i", str(src), "-frames:v", @@ -699,6 +656,7 @@ def build_thumbnail_jpg_cmd(src: Path, dest: Path) -> list[str]: "-loglevel", "error", "-nostdin", + *THUMB_GUARD, "-i", str(src), "-frames:v", @@ -734,7 +692,7 @@ def _step_failure(err: BaseException) -> str: def _is_jpeg(path: Path) -> bool: - """True if the file starts with the JPEG SOI marker (FF D8 FF).""" + """True if the file starts with the JPEG SOI marker (0xFF 0xD8 0xFF).""" try: with path.open("rb") as f: return f.read(3) == b"\xff\xd8\xff" diff --git a/backend/app/downloads/errors.py b/backend/app/downloads/errors.py index 3475d5f2..e390d355 100644 --- a/backend/app/downloads/errors.py +++ b/backend/app/downloads/errors.py @@ -53,6 +53,7 @@ from app.downloads.credentials import host_key_for_url # constants, imported above; each has a rule below that gives its category back, and a catalog entry # that gives the site's sentence. QUOTA_FULL_MESSAGE = "Your download storage is full — free up space, then try again." +UPLOAD_MISSING_MESSAGE = "The uploaded file is missing." NETWORK_REDACTED_MESSAGE = "The server could not reach that address." # (marker, category) — first match wins, so order most-specific → most-general. Matching is @@ -98,6 +99,8 @@ _RULES: tuple[tuple[str, str], ...] = ( # An edit's source download went away — our own wording, and MORE specific than the generic # "no longer available" below, so it must be matched first. ("source download is no longer available", "source_missing"), + # An upload's original went away before its conversion ran — our wording (UPLOAD_MISSING_MESSAGE). + (UPLOAD_MISSING_MESSAGE.lower().rstrip("."), "upload_missing"), ("source file is missing", "source_missing"), # Private / deleted / removed / terminated. ("private video", "unavailable"), @@ -263,6 +266,15 @@ def classify(msg: str | None) -> str: return "unknown" +def classify_conversion(msg: str | None) -> str: + """The category of an upload's failed conversion. The generic ffmpeg categories speak of a + download ("the video downloaded but…"), which an upload never was: a failed encode or an + unrecognised failure of one is `convert_failed`; a category of its own (a full quota, a missing + original) stays as it is.""" + code = classify(msg) + return "convert_failed" if code in ("postprocess", "unknown") else code + + def diagnose( msg: str | None, *, url: str | None = None, cookie_attached: bool = False ) -> Diagnosis: diff --git a/backend/app/downloads/formats.py b/backend/app/downloads/formats.py index a8deef1e..edf5dc8a 100644 --- a/backend/app/downloads/formats.py +++ b/backend/app/downloads/formats.py @@ -182,7 +182,9 @@ _DEFAULTS = { # v4: EVERY download is clamped to the mp4/H.264/AAC compat profile (no more mkv/webm/VP9/AV1/Opus # opt-out), so any asset cached under an old non-compat spec — or a stale non-H.264 mp4 that slipped # the v3 net — must re-derive through the current browser-safe guarantee. -_SIG_VERSION = 4 +# v5: the phone-safe rule (`playable`) also judges the pixel format, so a cached 10-bit / 4:4:4 H.264 +# asset (which v4 let through as "H.264") re-derives as 8-bit 4:2:0 instead of being reused. +_SIG_VERSION = 5 # Audio-only downloads version on their own, so a change to the audio bytes doesn't re-download every # cached video. v5: an extracted .mp3/.m4a is stored as itself instead of remuxed into an .mp4 that # iPad's WebKit refuses to play, so a cached remuxed audio asset must re-derive. @@ -219,12 +221,13 @@ def normalize(spec: dict) -> dict: s["embed_subs"] = False # Browser-playability guarantee: the Download module exists to download → share → PLAY # in-browser, so every output must play in EVERY browser, iOS/Safari/WebKit included. The only - # universally-safe combo is mp4 + H.264 + AAC/MP3, so we CLAMP the spec to it HERE — the single - # point every path (picker, stored profile, raw API) flows through — instead of only hiding - # options in the UI. Non-mp4 containers (mkv doesn't play at all; webm/VP9/AV1 fail iOS) and any - # explicit vcodec are coerced to the compat profile; Opus audio (Safari-flaky as a bare file) - # falls back to AAC/m4a. A source that exposes no H.264 is still caught by the worker's - # browser-safe re-encode (`_ensure_browser_playable`), so playback is guaranteed end-to-end. + # universally-safe combo is mp4 + H.264 + AAC (an .mp3/.m4a for audio), so we CLAMP the spec to + # it HERE — the single point every path (picker, stored profile, raw API) flows through — instead + # of only hiding options in the UI. Non-mp4 containers (mkv plays nowhere on iOS; AV1 fails there, + # and webm/VP9 plays only on recent devices) and any explicit vcodec are coerced to the compat + # profile; Opus audio falls back to AAC/m4a. A source that exposes no H.264 (or a 10-bit one) is + # still caught by the worker's phone-safe step (`downloads.playable`), so playback is guaranteed + # end-to-end. s["container"] = "mp4" s["vcodec"] = None if s["audio_format"] not in ("m4a", "mp3"): @@ -351,7 +354,7 @@ def build_ydl_opts( # inside mp4 — a VP9/AV1-in-mp4 file shows a broken player there — so H.264 is selected at the # SELECTION level, not merely preferred: a sort key can't conjure an H.264 stream a source # doesn't expose as video-only. A source that ships only non-H.264 is caught after download by - # the worker's browser-safe re-encode (`_ensure_browser_playable`). + # the worker's phone-safe step (`downloads.playable`). if s["mode"] == "v": # Video-only: keep it audio-free. Prefer an H.264 video-only stream, then any video-only, # and only fall to a progressive (audio-bearing) `best` as a last resort. diff --git a/backend/app/downloads/gc.py b/backend/app/downloads/gc.py index 60267f98..7a9c2afd 100644 --- a/backend/app/downloads/gc.py +++ b/backend/app/downloads/gc.py @@ -6,12 +6,13 @@ Five passes each run: 2. Expiry deletion — delete assets past their TTL (files + row). The FK SET NULL nulls the holders' jobs' asset_id, so those jobs surface as "expired" (re-downloadable) in the UI. 3. LRU eviction — if a total cache cap is set and exceeded, delete least-recently-accessed - ready assets until back under it. + ready assets (never an upload) until back under it. 4. Orphan reap — delete errored, fileless, unreferenced asset rows (they carry no TTL, so passes 1-3 skip them; benign row-count hygiene). 5. Orphan FILE sweep — delete on-disk files/dirs no surviving asset owns (cruft left when a - re-download renames an asset, e.g. old accented channel dirs), skipping system trees and - anything newer than an hour so an in-flight download is never touched. + re-download renames an asset, e.g. old accented channel dirs, or an upload's stranded file). + It skips the dot-prefixed system trees (all but `.uploads`) and anything newer than an hour, + so an in-flight download is never touched. Runs in the API process (which mounts DOWNLOAD_ROOT). Pure disk/DB work — no YouTube quota. """ @@ -20,19 +21,19 @@ import time from datetime import datetime, timedelta, timezone from pathlib import Path -from sqlalchemy import func, select +from sqlalchemy import func, or_, select from sqlalchemy.orm import Session from app import logs, sysconfig from app.config import settings -from app.downloads import storage +from app.downloads import quota, storage from app.models import DownloadJob, MediaAsset from app.notifications import create_notification log = logs.get_logger(__name__) -# Dot-prefixed top-level trees that are NOT part of the Plex/flat asset layout — the orphan sweep -# never touches them (user clips, Plex HLS/img caches, in-progress staging, editor storyboards). -_SYSTEM_TREE = {".edits", ".plex-hls", ".plex-img-cache", ".staging", ".storyboards"} +# The one dot-prefixed top-level tree the orphan sweep enters. The others are not part of the asset +# layout (user clips, Plex HLS/img caches, in-progress staging, editor storyboards) and are skipped. +_UPLOAD_TREE = ".uploads" _ORPHAN_MIN_AGE_S = 3600 # only sweep files older than this, so an in-flight download is never hit @@ -67,16 +68,20 @@ def _delete_asset(db: Session, asset: MediaAsset, ntype: str) -> None: def _sweep_orphan_files(db: Session, download_root: str) -> int: """Delete media/sidecar files (and now-empty dirs) under DOWNLOAD_ROOT that no `ready` asset owns — cruft left when a re-download changes an asset's on-disk name (e.g. an old accented - channel dir, or a poster from a superseded naming scheme). Skips the dot-prefixed system trees - and only removes files older than `_ORPHAN_MIN_AGE_S`, so a just-completed or in-flight download - is never deleted, and only files NOT owned by a surviving asset.""" + channel dir, or a poster from a superseded naming scheme) — or, under `.uploads`, that no upload + row names at all. Skips the other dot-prefixed system trees and only removes files older than + `_ORPHAN_MIN_AGE_S`, so a just-completed or in-flight download is never deleted, and only files + NOT owned by a surviving asset.""" root = Path(download_root) if not root.is_dir(): return 0 keep: set[Path] = set() + # An upload's file is on disk from ingest on, whatever its state (a queued conversion's original, + # a failed one kept for Retry), so every upload row keeps its file — not only a ready one. for rel in db.execute( select(MediaAsset.rel_path).where( - MediaAsset.status == "ready", MediaAsset.rel_path.is_not(None) + or_(MediaAsset.status == "ready", MediaAsset.source_kind == "upload"), + MediaAsset.rel_path.is_not(None), ) ).scalars(): media = (root / rel).resolve() @@ -99,7 +104,10 @@ def _sweep_orphan_files(db: Session, download_root: str) -> int: unreclaimed = 0 for entry in root.iterdir(): - if entry.name.startswith("."): # .edits / .plex-* / .staging / .storyboards + any dotfile + # .edits / .plex-* / .staging / .storyboards + any dotfile — but `.uploads` is swept: a + # conversion stopped between placing the new file and the commit that points at it (or the + # unlink of the original after it) leaves a file no row names, and nothing else finds it. + if entry.name.startswith(".") and entry.name != _UPLOAD_TREE: continue files = [entry] if entry.is_file() else [p for p in entry.rglob("*") if p.is_file()] for f in files: @@ -188,17 +196,17 @@ def run_download_gc(db: Session) -> dict: if cap > 0: total = ( db.execute( - select(func.coalesce(func.sum(MediaAsset.size_bytes), 0)).where( - MediaAsset.status == "ready" - ) + select(func.coalesce(func.sum(MediaAsset.size_bytes), 0)).where(quota.ON_DISK) ).scalar() or 0 ) if total > cap: + # An upload counts toward the total (it is disk like any other) but is never evicted: + # it has no source to fetch again, and it lives until its owner deletes it. candidates = ( db.execute( select(MediaAsset) - .where(MediaAsset.status == "ready") + .where(MediaAsset.status == "ready", MediaAsset.source_kind != "upload") .order_by(MediaAsset.last_access_at.asc().nulls_first()) ) .scalars() @@ -232,6 +240,29 @@ def run_download_gc(db: Session) -> dict: if orphans: db.commit() + # 4b. Reap unheld uploads. An upload carries no expires_at, is never evicted and never re-enqueued, + # so the passes above never free one that lost its last holder outside `release_asset` — a + # conversion that finished after its job was cancelled, or one cancelled mid-run (the asset + # was `downloading` then). Its file, ready or still the unconverted original, is at `rel_path`. + unheld = ( + db.execute( + select(MediaAsset).where( + MediaAsset.source_kind == "upload", + MediaAsset.ref_count == 0, + MediaAsset.status != "downloading", + ) + ) + .scalars() + .all() + ) + for asset in unheld: + if asset.rel_path: # no holder is left to tell, so no notice — just the file and the row + storage.delete_asset_files(settings.download_root, asset.rel_path) + db.delete(asset) + reaped += 1 + if unheld: + db.commit() + # 5. Orphan FILE sweep — remove on-disk cruft no surviving asset owns (old accented dirs, stale # posters). Runs last so the keep-set reflects the assets that survived passes 2-4. swept = _sweep_orphan_files(db, settings.download_root) diff --git a/backend/app/downloads/og.py b/backend/app/downloads/og.py index 29d1e9aa..2bc7bf47 100644 --- a/backend/app/downloads/og.py +++ b/backend/app/downloads/og.py @@ -147,13 +147,14 @@ def _watch_tags(token: str, db: Session) -> str | None: return "\n ".join(tags) -def render_watch_html(token: str, index_html: Path, db: Session) -> str | None: - """Return index.html with the OG block replaced for this watch token, or None to serve the - generic page unchanged (invalid/expired/not-ready link, or a template without the markers).""" +def render_watch_html(token: str, page_html: Path, db: Session) -> str | None: + """Return the watch page (watch.html) with the OG block replaced for this token, or None to + serve the generic page unchanged (invalid/expired/not-ready link, or a template without the + markers).""" tags = _watch_tags(token, db) if tags is None: return None - text = index_html.read_text(encoding="utf-8") + text = page_html.read_text(encoding="utf-8") if "" not in text: return None # A function replacement avoids re.sub interpreting backslashes/group refs in `tags`. diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index e2554fbe..b8ac2324 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -13,7 +13,8 @@ from app.config import settings from app.db import SessionLocal from app.downloads import edit as editmod from app.downloads import errors as dlerrors -from app.downloads import adapters, credentials, formats, mediakind, probe, quota, service, storage +from app.downloads import adapters, credentials, formats, mediakind, playable, probe, quota +from app.downloads import service, storage from app.downloads import urlguard from app.downloads.jobdb import ( _edit_staging_dir, @@ -58,6 +59,10 @@ _DB_WRITE_INTERVAL_S = 1.0 _CANCEL_POLL_INTERVAL_S = 1.0 _THUMB_SUFFIXES = (".jpg", ".jpeg", ".png", ".webp") +# The job phase while a file is brought into the phone-safe shape (`_ensure_phone_safe`) for a +# download or an upload. Its own word, not yt-dlp's "processing": it reports ffmpeg's real progress, +# so the queue row shows a % for it, where yt-dlp's post-steps only get an indeterminate pulse. +CONVERTING = "converting" # yt-dlp's working files in the staging dir. None of them is ever the finished media, but several # CAN be the biggest file there (a video-only per-format stream, an abandoned fragment run), so the # staging fallback must exclude them by name — picking one would store e.g. a silent video-only @@ -75,7 +80,7 @@ def _notify_download_done(db, user_id: int | None, title: str | None, job_id, *, """Post a best-effort 'ready' notice to the owner's inbox — downloads/edits used to finish silently while scheduler jobs and playlist-push already notified (feedback symmetry). A notification failure must never mask the completed job, so it is isolated + rolled back (mirrors - scheduler._notify_done). `kind` ("download"/"edit") lets the inbox pick accurate copy; the + scheduler._notify_done). `kind` ("download"/"edit"/"upload") lets the inbox pick accurate copy; the English title here is only a fallback for an unknown type. `user_id`/`title` are captured by the caller BEFORE its commit, so reading them here needs no post-commit refresh of an expired row.""" if user_id is None: @@ -416,48 +421,50 @@ def _download_settings() -> tuple[str, int]: ) -def _ensure_browser_playable(job_id: int, produced: Path, spec: dict, info: dict) -> Path: - """Make an mp4-compat-profile download play in every browser: H.264 + AAC/MP3 in mp4. If the - download (whatever the source shipped) isn't already that, re-encode the offending stream(s) — - or just remux when the codecs are fine but the container isn't. **Scope:** only the mp4 compat - profile is normalized; an explicit `vcodec` or a non-mp4 custom profile is the user opting out - of the guarantee and is left as-is (so a `webm` profile can still ship VP9/Opus). Returns the - file to store (converted, or the original) and corrects `info`'s codec fields to match disk. +def _ensure_phone_safe( + job_id: int, + produced: Path, + out_dir: Path, + phase: str, + total_s: int | None = None, + *, + keep_source: bool = False, +) -> Path: + """Bring a file that is about to become a ready library asset into the phone-safe shape + (`playable`), writing any converted copy into `out_dir`. Returns the file to store — `produced` + itself when it already is that shape, or isn't media at all — and removes `produced` once a + converted copy exists, unless `keep_source` (an upload's original, which must outlive the + conversion until the database points at the new file). The job shows `phase` with real ffmpeg + progress while it converts. - An audio download (mode "a") is never touched: yt-dlp already extracted it to .mp3 / .m4a, which - every browser plays as audio/mpeg / audio/mp4. Remuxing it into .mp4 served an mp3 as video/mp4, - which iPad's WebKit refuses to play (prod job 268).""" - if spec.get("mode") == "a" or spec.get("container") != "mp4" or spec.get("vcodec"): + Every ready path calls this — a download, an upload's worker conversion and an editor clip — so + a share link can only ever serve the one shape every phone and browser plays.""" + p = playable.plan(probe.probe_json(produced), produced.suffix) + if p is None or not p.needs_change: return produced - vcodec, acodec = editmod.probe_codecs(produced) - is_mp4 = produced.suffix.lower() == ".mp4" - needs, reenc_v, reenc_a = editmod.browser_transcode_flags(vcodec, acodec, is_mp4) - if not needs: - return produced - - dest = produced.with_name(produced.stem + ".browser.mp4") - action = "re-encode" if (reenc_v or reenc_a) else "remux" + dest = out_dir / f"{produced.stem}.safe.{p.ext}" + action = "re-encode" if (p.reencode_video or p.reencode_audio) else "remux" log.info( - "job %s: %s (v=%s a=%s mp4=%s) -> browser-safe mp4", job_id, action, vcodec, acodec, is_mp4 + "job %s: %s (video=%s audio=%s) -> phone-safe .%s", + job_id, + action, + p.reencode_video, + p.reencode_audio, + p.ext, + extra=logs.event("download.phone_safe", job_id=job_id, action=action, ext=p.ext), ) - _set_job(job_id, phase="processing", progress=0, speed_bps=None, eta_s=None) - # Prefer the info duration; probe the file when the extractor didn't provide one, so a long - # re-encode still shows real progress instead of a frozen 0%. - total_s = int(info["duration"]) if info.get("duration") else editmod.probe_duration(produced) + _set_job(job_id, phase=phase, progress=0, speed_bps=None, eta_s=None) + # A re-encode with no known length would hold a frozen 0%; probe the file for a denominator. editmod.run_ffmpeg( - editmod.build_browser_safe_cmd( - produced, dest, reenc_v, reenc_a, threads=editmod.encode_thread_cap() - ), - total_s=total_s, - on_progress=_ffmpeg_progress_writer(job_id, "processing"), + playable.build_cmd(produced, dest, p, threads=editmod.encode_thread_cap()), + total_s=total_s or editmod.probe_duration(produced), + on_progress=_ffmpeg_progress_writer(job_id, phase), should_cancel=_cancel_poll(job_id), ) - produced.unlink(missing_ok=True) - # Keep the stored asset metadata truthful about what's now on disk. - if reenc_v: - info["vcodec"] = "h264" - if reenc_a: - info["acodec"] = "aac" + if not dest.exists(): + raise StatedFailure("ffmpeg produced no output file") + if not keep_source: + produced.unlink(missing_ok=True) return dest @@ -522,7 +529,13 @@ def _download( adapter = adapters.resolve_adapter(source_kind, source_ref, info) produced = _produced_file(info, staging) - produced = _ensure_browser_playable(job_id, produced, spec, info) + produced = _ensure_phone_safe( + job_id, + produced, + staging, + CONVERTING, + int(info["duration"]) if info.get("duration") else None, + ) ext = produced.suffix.lstrip(".").lower() # Codecs/dims/duration come from a PROBE of the produced file — the ground truth — NOT yt-dlp's # `info` dict, whose fields are absent for some sources (Facebook among them): deriving @@ -722,9 +735,12 @@ def _log_download_failed(job_id: int, msg: str, code: str | None, exc: BaseExcep ) -def _fail_edit(job_id: int, asset_id: int, msg: str, exc: BaseException | None = None) -> None: - """Fail an edit job, and say so ONCE — every way an edit fails comes through here, so this is - where its line and its event code live. `exc` is the exception when there was one. +def _fail_edit( + job_id: int, asset_id: int, msg: str, exc: BaseException | None = None, *, what: str = "edit" +) -> None: + """Fail a local ffmpeg job — an edit, or an upload's conversion (`what="upload"`) — and say so + ONCE: every way one fails comes through here, so this is where its line and its event code + (`.job.failed`) live. `exc` is the exception when there was one. Its traceback goes with the line unless the message already tells it all (`_failure_stack`). Logged after the write, as on the download error path.""" @@ -739,19 +755,22 @@ def _fail_edit(job_id: int, asset_id: int, msg: str, exc: BaseException | None = job_id, status=JobStatus.ERROR, error=msg, - error_code=dlerrors.classify(msg), + error_code=dlerrors.classify_conversion(msg) + if what == "upload" + else dlerrors.classify(msg), phase=None, ) db.commit() # `msg` is the user's copy; ffmpeg's stderr tail in it can name the source file, whose path is # built from the channel and the title. log.warning( - "edit job %s failed: %s", + "%s job %s failed: %s", + what, job_id, formats.log_safe(msg), extra={ **logs.event( - "edit.job.failed", + f"{what}.job.failed", job_id=job_id, asset_id=asset_id, **({"error_type": type(exc).__name__} if exc is not None else {}), @@ -811,11 +830,17 @@ def _process_edit(job_id: int, asset_id: int) -> None: ) if not dest_staging.exists(): raise StatedFailure("ffmpeg produced no output file") + # A fast trim stream-copies the source's codecs and container, so the clip is only as + # playable as its source: hold it to the same rule as every other ready file. + dest_staging = _ensure_phone_safe(job_id, dest_staging, staging, "editing", total_s) + out_ext = dest_staging.suffix.lstrip(".").lower() or out_ext rel = storage.edit_rel_path(user_id, asset_id, out_ext) storage.place_file(dest_staging, settings.download_root, rel) final = storage.abs_path(settings.download_root, rel) size = final.stat().st_size if final.exists() else None + # The clip's real frame: a re-encode evens an odd crop or source (`editmod.EVEN_FRAME`). + _, _, out_w, out_h, _ = probe.probe_media(final) _, retention_days = _download_settings() expires = datetime.now(timezone.utc) + timedelta(days=retention_days) @@ -827,7 +852,9 @@ def _process_edit(job_id: int, asset_id: int) -> None: asset.size_bytes = size asset.container = out_ext asset.duration_s = total_s or asset.duration_s - if crop: + if out_w and out_h: + asset.width, asset.height = out_w, out_h + elif crop: asset.width = edit_spec["crop"]["w"] asset.height = edit_spec["crop"]["h"] else: @@ -867,3 +894,163 @@ def _process_edit(job_id: int, asset_id: int) -> None: _fail_edit(job_id, asset_id, str(exc)[:500], exc) finally: shutil.rmtree(staging, ignore_errors=True) + + +def _check_converted_fits(owner_id: int | None, old_size: int, new_size: int) -> None: + """Refuse a converted upload that would push its owner over the storage cap: the upload was + charged at its original size, and a re-encode can come out larger. The original stays, counted, + for the owner to delete or keep.""" + if owner_id is None: + return + with SessionLocal() as db: + if not quota.fits_after_conversion(db, owner_id, old_size, new_size): + raise StatedFailure(dlerrors.QUOTA_FULL_MESSAGE) + + +def _unplace(placed: Path | None) -> None: + """Remove a converted upload that was placed at its new rel but never committed: the row still + names the original, so nothing else would ever find this file.""" + if placed is not None: + placed.unlink(missing_ok=True) + + +def _conversion_landed(asset_id: int, new_rel: str) -> bool | None: + """After the commit that makes a converted upload ready RAISED: did it land anyway? A commit can + reach the server and lose its answer (the connection drops after COMMIT). True/False by what the + row now says; None when the row cannot be read either.""" + try: + with SessionLocal() as db: + asset = db.get(MediaAsset, asset_id) + return asset is not None and asset.status == "ready" and asset.rel_path == new_rel + except Exception: # noqa: BLE001 — "cannot tell" is the answer, the caller acts on it + # silent-ok: the caller re-raises the commit's own error, which `_fail_edit` reports. + return None + + +def _drop_original(job_id: int, src: Path) -> None: + """Remove an upload's original once the converted file is committed in its place. Best effort: + the job is done and its file is ready, so a failure here must not fail it — the original is + named by no row now, and the GC's `.uploads` sweep takes it.""" + try: + src.unlink(missing_ok=True) + except OSError as exc: + log.warning( + "upload job %s: original not removed, left to the GC sweep: %s", + job_id, + exc, + extra=logs.event("upload.original.unlink_failed", job_id=job_id), + ) + + +def _free_unheld_upload(asset_id: int) -> bool: + """Free an upload no job holds any more — cancelled or deleted while its conversion ran, when + `release_asset` had to leave it because the conversion still read its file. Once that has + stopped, free it as the release would have; left alone, its file would sit on disk counted by no + owner's footprint until the GC's upload pass. False when a job still holds it (paused).""" + with SessionLocal() as db: + asset = db.get(MediaAsset, asset_id) + if asset is None or asset.ref_count != 0: + return False + if asset.rel_path: + storage.delete_asset_files(settings.download_root, asset.rel_path) + db.delete(asset) + db.commit() + return True + + +def _convert_upload(job_id: int, asset_id: int) -> None: + """Convert a queued upload into the phone-safe shape (`playable`) and make it ready. + + The original sits at the asset's `rel_path` (see `upload`); the converted file replaces it there + under the new extension, and only then is the original removed — so a failed or stopped + conversion leaves the original in place for Retry, and a delete still finds it by `rel_path`. + A converted file placed but never committed is removed again on the way out. When the commit's + own outcome is unknown neither file is removed: the row names one of them, and the GC's `.uploads` + sweep takes the one it does not — as it does an original whose unlink after the commit failed.""" + with SessionLocal() as db: + asset = db.get(MediaAsset, asset_id) + old_rel = asset.rel_path if asset else None + old_ext = asset.container if asset else None + old_size = (asset.size_bytes or 0) if asset else 0 + duration = asset.duration_s if asset else None + job = db.get(DownloadJob, job_id) + owner_id = job.user_id if job else None + src = storage.safe_abs_path(settings.download_root, old_rel) if old_rel else None + if old_rel is None or src is None or not src.exists(): + _fail_edit(job_id, asset_id, dlerrors.UPLOAD_MISSING_MESSAGE, what="upload") + return + staging = _staging_dir(asset_id) + placed: Path | None = None # the converted file while it is at its new rel, uncommitted + try: + shutil.rmtree(staging, ignore_errors=True) + staging.mkdir(parents=True, exist_ok=True) + safe = _ensure_phone_safe(job_id, src, staging, CONVERTING, duration, keep_source=True) + ext = safe.suffix.lstrip(".").lower() + new_rel = storage.with_ext(old_rel, old_ext, ext) + if safe != src: + _check_converted_fits(owner_id, old_size, safe.stat().st_size) + storage.place_file(safe, settings.download_root, new_rel) + if new_rel != old_rel: + placed = storage.abs_path(settings.download_root, new_rel) + final = storage.abs_path(settings.download_root, new_rel) + vcodec, acodec, width, height, dur = probe.probe_media(final) + with SessionLocal() as db: + asset = db.get(MediaAsset, asset_id) + if asset: + asset.status = "ready" + asset.rel_path = new_rel + asset.container = ext + asset.size_bytes = final.stat().st_size + asset.vcodec, asset.acodec = vcodec, acodec + asset.width, asset.height = width or asset.width, height or asset.height + asset.duration_s = dur or asset.duration_s + asset.error = None + asset.last_access_at = datetime.now(timezone.utc) + title = asset.title if asset else None + finalized = service.finalize_if_running( + db, job_id, status=JobStatus.DONE, progress=100, phase=None + ) + try: + db.commit() + except Exception: + landed = _conversion_landed(asset_id, new_rel) + if landed is False: + raise # rolled back: the original is still the asset's file — fail as any step + # Landed, or cannot tell: the converted file may be the asset's file now, so it is + # never removed blind (the sweep takes whichever file no row names). + placed = None + if landed is None: + raise + db.rollback() + placed = None + if finalized: + _notify_download_done(db, owner_id, title, job_id, kind="upload") + if new_rel != old_rel: + _drop_original(job_id, src) + if not finalized: + _free_unheld_upload(asset_id) # finished after its job was cancelled or deleted + if finalized: + log.info( + "upload job %s converted: asset %s", + job_id, + asset_id, + extra=logs.event("upload.job.done", job_id=job_id, asset_id=asset_id, ext=ext), + ) + except editmod.EditAborted: + # Paused/canceled mid-encode: the original is untouched. A paused job keeps it for its + # resume; one cancelled or deleted holds it no more, so it is freed now. + _unplace(placed) + if not _free_unheld_upload(asset_id): + _reset_asset_pending(asset_id) + log.info( + "upload job %s conversion aborted (pause/cancel)", + job_id, + extra=logs.event("upload.job.aborted", job_id=job_id), + ) + except Exception as exc: # noqa: BLE001 — surface the failure, keep the worker alive + # silent-ok: not silent — `_fail_edit` is the report, and it takes `exc` for its stack. + _unplace(placed) + _fail_edit(job_id, asset_id, str(exc)[:500], exc, what="upload") + + finally: + shutil.rmtree(staging, ignore_errors=True) diff --git a/backend/app/downloads/playable.py b/backend/app/downloads/playable.py new file mode 100644 index 00000000..ec0381c4 --- /dev/null +++ b/backend/app/downloads/playable.py @@ -0,0 +1,103 @@ +"""The phone-safe rule: what a library file must be so it plays wherever a share link lands. + +A shared `/watch` link is opened by whoever got it, on whatever they hold: an iPhone's WebKit, an +Android Chrome, a desktop Firefox. The one shape every one of them plays: + + * video → H.264, 8-bit 4:2:0 (`yuv420p`, or its full-range twin `yuvj420p`), AAC audio, in .mp4 + * audio only → MP3 in .mp3, or AAC in .m4a + +Measured on the owner's iPhone + iPad (2026-10-05): AV1, Matroska, HEVC tagged `hev1` and MP3 +inside an .mp4 all fail there. VP9/WebM, HEVC `hvc1` and 10-bit H.264 played on those devices, but +Chrome and Firefox do not decode HEVC and browsers decode 10-bit / 4:4:4 H.264 only patchily, so the +rule is the shape every browser plays, not the shape one device plays. + +Every file that becomes a ready library asset passes through here — a download, an upload (the +worker converts it, see `upload`), and an editor clip — so a link can only ever serve this shape. +`plan` says what (if anything) must change; `build_cmd` is the one ffmpeg command that changes it: +re-encode only the stream that fails, copy the rest, or just remux when only the container is wrong. +""" + +from dataclasses import dataclass +from pathlib import Path + +from app.downloads import edit as editmod +from app.downloads import probe + +_SAFE_VCODEC = "h264" +_SAFE_PIX_FMTS = frozenset({"yuv420p", "yuvj420p"}) +_VIDEO_EXT = "mp4" +# An audio-only file: the codec, and the extension it plays in. Anything else becomes AAC in .m4a. +_AUDIO_HOME = {"mp3": "mp3", "aac": "m4a"} +_AUDIO_FALLBACK_EXT = "m4a" + + +@dataclass(frozen=True) +class Plan: + """What makes one file phone-safe. `ext` is the extension the safe file has (no dot).""" + + ext: str + has_video: bool + reencode_video: bool + reencode_audio: bool + needs_change: bool + + +def plan(data: dict | None, ext: str) -> Plan | None: + """The phone-safe plan for a file whose `probe.probe_json` is `data` and whose extension is + `ext`; None when it is not media at all (an unreadable probe, a still image, no audio or video + stream) — such a file is a download-only 'file' and never plays inline, so there is nothing to + make safe.""" + if data is None: + return None + video, audio = probe.primary_streams(data) + ext = ext.lower().lstrip(".") + if video is not None: + rv = (video.get("codec_name") or "").lower() != _SAFE_VCODEC or ( + video.get("pix_fmt") or "" + ).lower() not in _SAFE_PIX_FMTS + ra = audio is not None and (audio.get("codec_name") or "").lower() != "aac" + return Plan(_VIDEO_EXT, True, rv, ra, rv or ra or ext != _VIDEO_EXT) + if audio is not None: + home = _AUDIO_HOME.get((audio.get("codec_name") or "").lower()) + target = home or _AUDIO_FALLBACK_EXT + return Plan(target, False, False, home is None, home is None or ext != target) + return None + + +def build_cmd(src: Path, dest: Path, p: Plan, threads: int | None = None) -> list[str]: + """The ffmpeg command that writes `src` to `dest` (whose suffix is `p.ext`) in the phone-safe + shape: the failing stream(s) re-encoded with the package's shared encoder settings, the rest + stream-copied. An audio-only result drops any cover-art stream. `-progress pipe:1` + streams machine-readable progress; `threads` caps the encoder so a re-encode cannot claim every + core on the shared box (see the worker-child de-prioritization).""" + cmd = [ + "ffmpeg", + "-y", + "-hide_banner", + "-loglevel", + "error", + "-nostdin", + "-progress", + "pipe:1", + "-nostats", + *editmod.INPUT_GUARD, + "-i", + str(src), + ] + # Exactly the two streams `plan` judged: `V` is a video stream that is not cover art. Nothing + # else rides along — a subtitle or data track the .mp4 muxer cannot take would fail the run. + if p.has_video: + cmd += ["-map", "0:V:0", "-map", "0:a:0?"] + if p.reencode_video: + cmd += ["-vf", editmod.EVEN_FRAME, *editmod.X264_ARGS] + else: + cmd += ["-c:v", "copy"] + else: + cmd += ["-map", "0:a:0"] + cmd += editmod.AAC_ARGS if p.reencode_audio else ["-c:a", "copy"] + if threads and threads > 0: + cmd += ["-threads", str(threads)] + if p.ext in ("mp4", "m4a"): + cmd += ["-movflags", "+faststart"] + cmd += [str(dest)] + return cmd diff --git a/backend/app/downloads/probe.py b/backend/app/downloads/probe.py index 04dd70d3..eaf2a650 100644 --- a/backend/app/downloads/probe.py +++ b/backend/app/downloads/probe.py @@ -34,6 +34,28 @@ _IMAGE_FORMATS = { } +def primary_streams(data: dict) -> tuple[dict | None, dict | None]: + """(video stream, audio stream) — the first of each kind — from a NON-None ffprobe JSON dict, by + the same reading the stored facts use: a still image / image-sequence container has neither (it + is a generic 'file'), and a video stream flagged `attached_pic` (an mp3/m4a's cover art) is not a + video track. The phone-safe rule (`playable`) judges exactly these two streams.""" + fmt_names = { + n.strip() for n in ((data.get("format") or {}).get("format_name") or "").split(",") + } + if fmt_names & _IMAGE_FORMATS: + return None, None + video = audio = None + for stream in data.get("streams", []): + kind = stream.get("codec_type") + if kind == "video" and video is None: + if (stream.get("disposition") or {}).get("attached_pic"): + continue # cover art, not a video track + video = stream + elif kind == "audio" and audio is None: + audio = stream + return video, audio + + def _facts_from_probe( data: dict, ) -> tuple[str | None, str | None, int | None, int | None, int | None]: @@ -45,19 +67,11 @@ def _facts_from_probe( fmt_names = {n.strip() for n in (fmt.get("format_name") or "").split(",")} if fmt_names & _IMAGE_FORMATS: return None, None, None, None, None - - vcodec = acodec = None - width = height = None - for stream in data.get("streams", []): - kind = stream.get("codec_type") - if kind == "video" and vcodec is None: - if (stream.get("disposition") or {}).get("attached_pic"): - continue # cover art, not a video track - vcodec = stream.get("codec_name") - width = stream.get("width") - height = stream.get("height") - elif kind == "audio" and acodec is None: - acodec = stream.get("codec_name") + video, audio = primary_streams(data) + vcodec = video.get("codec_name") if video else None + width = video.get("width") if video else None + height = video.get("height") if video else None + acodec = audio.get("codec_name") if audio else None duration = None try: @@ -71,16 +85,29 @@ def _facts_from_probe( return vcodec, acodec, width, height, duration -def probe_media(path: Path) -> tuple[str | None, str | None, int | None, int | None, int | None]: - """(vcodec, acodec, width, height, duration_s) from one ffprobe call; all None if ffprobe fails - or the file isn't playable media (an unprobeable file — or a still image — is treated as a generic - non-media file, so it becomes a download-only 'file' rather than a broken 'video').""" - data = editmod.run_ffprobe_json(path, show_streams=True, show_format=True) +def probe_json(path: Path) -> dict | None: + """The full ffprobe JSON (streams + format) of `path`, or None when ffprobe could not read it — + the one call both the stored facts (`facts`) and the phone-safe rule read from.""" + return editmod.run_ffprobe_json(path, show_streams=True, show_format=True) + + +def facts( + data: dict | None, +) -> tuple[str | None, str | None, int | None, int | None, int | None]: + """(vcodec, acodec, width, height, duration_s) of a `probe_json` result; all None when it is None + (an unprobeable file is treated as a generic non-media file, so it becomes a download-only 'file' + rather than a broken 'video').""" if data is None: return None, None, None, None, None return _facts_from_probe(data) +def probe_media(path: Path) -> tuple[str | None, str | None, int | None, int | None, int | None]: + """(vcodec, acodec, width, height, duration_s) from one ffprobe call; all None if ffprobe fails + or the file isn't playable media (a still image is a generic 'file', see `primary_streams`).""" + return facts(probe_json(path)) + + def resolve_media_facts( path: Path, info: dict ) -> tuple[str | None, str | None, int | None, int | None, int | None]: @@ -94,18 +121,18 @@ def resolve_media_facts( is authoritative — INCLUDING a deliberate None (cover-art-only audio, a still image). A per-field `probe or info` fallback would let `info`'s stale vcodec resurrect a video track the file doesn't have (yt-dlp reports a progressive `vcodec` for an audio-mode download whose video it then strips), - mislabelling an audio download as a broken in-app 'video'. Only when ffprobe could NOT read the - file at all do we fall back to `info` (the pre-probe behaviour, a safety net); the cosmetic - dimensions and the duration still gap-fill from `info` in either case.""" + mislabelling an audio download as a broken in-app 'video'. When ffprobe could NOT read the file + at all, no codec is reported: the probe runs with the media demuxer whitelist, so a file it + cannot open is one the player and the editor cannot open either — and taking `info`'s codecs + for it would make it an editable 'video' whose bytes are whatever the remote site served. The + cosmetic dimensions and the duration still gap-fill from `info` in either case.""" data = editmod.run_ffprobe_json(path, show_streams=True, show_format=True) - info_v = info.get("vcodec") if info.get("vcodec") not in (None, "none") else None - info_a = info.get("acodec") if info.get("acodec") not in (None, "none") else None info_w = int(info["width"]) if info.get("width") else None info_h = int(info["height"]) if info.get("height") else None info_d = int(info["duration"]) if info.get("duration") else None if data is None: - # ffprobe could not read the file — fall back to the extractor's info entirely. - return info_v, info_a, info_w, info_h, info_d + # Unreadable to the guarded probe: a download-only 'file' (no codecs → media_kind FILE). + return None, None, info_w, info_h, info_d pv, pa, pw, ph, pd = _facts_from_probe(data) # Probe ran → its codecs win outright (a None means "no such track", not "unknown"); info only # gap-fills the width/height/duration the probe couldn't read. diff --git a/backend/app/downloads/quota.py b/backend/app/downloads/quota.py index 2fcd174e..66c92ef8 100644 --- a/backend/app/downloads/quota.py +++ b/backend/app/downloads/quota.py @@ -1,7 +1,9 @@ """Per-user download limits + footprint accounting. A user's *footprint* is the total size of the ready assets their library references (cache hits -count — it's "how much disk you're responsible for", not "how much you personally downloaded"). +count — it's "how much disk you're responsible for", not "how much you personally downloaded"). An +upload counts from the moment it lands, ready or not: its original is on disk while its conversion +waits, and stays there when the conversion fails (for Retry) — bytes a ready-only sum never saw. Limits come from a per-user DownloadQuota row if the admin set one, else the sysconfig defaults. `unlimited` (e.g. the admin's own account) bypasses the byte cap. @@ -18,16 +20,24 @@ is unknown until it finishes. Edit jobs do not consult it at all (tracked as SEC from dataclasses import dataclass -from sqlalchemy import func, select +from sqlalchemy import and_, func, or_, select from sqlalchemy.orm import Session from app import sysconfig +from app.downloads.status import JobStatus from app.models import DownloadJob, DownloadQuota, MediaAsset # Jobs a user is considered to be "holding" (occupy a job slot / footprint). Terminal failures # (error, canceled) don't count. -_HOLDING = ("queued", "running", "paused", "done") -_RUNNING = ("queued", "running") +_HOLDING = (JobStatus.QUEUED, JobStatus.RUNNING, JobStatus.PAUSED, JobStatus.DONE) +_RUNNING = (JobStatus.QUEUED, JobStatus.RUNNING) +# An asset whose file is on disk: a ready one, or an upload in any state (its original lands at +# ingest, see the module docstring). Every byte sum — a user's footprint, the GC's total-cap pass, +# the admin's storage view — reads it, so they all count the same disk. +ON_DISK = or_( + MediaAsset.status == "ready", + and_(MediaAsset.source_kind == "upload", MediaAsset.rel_path.is_not(None)), +) @dataclass @@ -67,12 +77,17 @@ def resolve(db: Session, user_id: int) -> QuotaLimits: def footprint(db: Session, user_id: int) -> int: - """Total bytes of the distinct ready assets this user's active jobs reference.""" + """Total bytes of the distinct assets this user's jobs hold on disk: the ready ones their active + jobs reference, and every upload of theirs — including one whose conversion failed, which keeps + its original (see the module docstring).""" held_assets = ( select(DownloadJob.asset_id) .where( DownloadJob.user_id == user_id, - DownloadJob.status.in_(_HOLDING), + or_( + DownloadJob.status.in_(_HOLDING), + and_(DownloadJob.job_kind == "upload", DownloadJob.status == JobStatus.ERROR), + ), DownloadJob.asset_id.is_not(None), ) .distinct() @@ -80,7 +95,7 @@ def footprint(db: Session, user_id: int) -> int: return int( db.execute( select(func.coalesce(func.sum(MediaAsset.size_bytes), 0)).where( - MediaAsset.id.in_(held_assets), MediaAsset.status == "ready" + MediaAsset.id.in_(held_assets), ON_DISK ) ).scalar() or 0 @@ -111,7 +126,7 @@ def at_concurrency_limit(db: Session, user_id: int) -> bool: lim = resolve(db, user_id) if lim.unlimited: return False - running = _count(db, user_id, ("running",)) + running = _count(db, user_id, (JobStatus.RUNNING,)) return running >= lim.max_concurrent @@ -160,6 +175,18 @@ def check_upload(db: Session, user_id: int, add_bytes: int) -> None: raise QuotaExceeded("max_bytes", lim.max_bytes, projected) +def fits_after_conversion(db: Session, user_id: int, old_bytes: int, new_bytes: int) -> bool: + """Whether an upload converted from `old_bytes` (what it is charged at now) to `new_bytes` keeps + its owner within the cap. A re-encode can come out larger than the original, and nothing else + re-checks it once the upload was let in.""" + if new_bytes <= old_bytes: # a smaller file never breaks a cap the larger one was let in under + return True + lim = resolve(db, user_id) + if lim.unlimited: + return True + return footprint(db, user_id) - old_bytes + new_bytes <= lim.max_bytes + + def usage(db: Session, user_id: int) -> dict: """Compact usage snapshot for the UI (footprint + counts vs limits).""" lim = resolve(db, user_id) diff --git a/backend/app/downloads/runner.py b/backend/app/downloads/runner.py index b609e32e..654de3e8 100644 --- a/backend/app/downloads/runner.py +++ b/backend/app/downloads/runner.py @@ -95,12 +95,14 @@ def _run(job_id, asset_id, job_kind, source_kind, source_ref, spec, user_id) -> (`cause=child_exit`).""" # Imported here (not at module top) so the child pays the yt-dlp import cost only when it has # real work, and to keep the import graph obvious: runner is a thin entry over the pipeline. - from app.downloads.pipeline import _download, _process_edit + from app.downloads.pipeline import _convert_upload, _download, _process_edit with formats.hiding(source_ref): try: if job_kind == "edit": _process_edit(job_id, asset_id) + elif job_kind == "upload": + _convert_upload(job_id, asset_id) else: _download(job_id, asset_id, source_kind, source_ref, spec, user_id) except Exception as exc: # noqa: BLE001 — whatever it is, it is reported here or raw diff --git a/backend/app/downloads/service.py b/backend/app/downloads/service.py index f643d04d..22c2a019 100644 --- a/backend/app/downloads/service.py +++ b/backend/app/downloads/service.py @@ -318,14 +318,22 @@ def release_asset(db: Session, job: DownloadJob) -> None: re-enqueuing the same (source,format) — get_or_create_asset would reset that row to `pending` and +1 it, and deleting it under that would FK-null the new job's asset (a lost update on ref_count). A ready asset is still freed at ref==0 (its file is the point); an orphaned errored row is cheap - (no file) and gets reused+reset by the next enqueue, or reclaimed by a later GC pass.""" + (no file) and gets reused+reset by the next enqueue, or reclaimed by a later GC pass. + + An UPLOAD is the exception: it is never shared or re-enqueued (its ref is unique), and while it + waits for or failed its phone-safe conversion the original file already sits at `rel_path` — so + it is freed at ref==0 whatever its state, except mid-conversion (`downloading`: the worker child + still reads it; the GC upload pass takes it once the child has stopped).""" if not job.asset_id or job.status not in HOLDING: return asset = db.get(MediaAsset, job.asset_id) if asset is None: return new_count = decr_ref_count(db, asset.id) - if new_count == 0 and asset.status == "ready": + freeable = asset.status == "ready" or ( + asset.source_kind == "upload" and asset.status != "downloading" + ) + if new_count == 0 and freeable: if asset.rel_path: storage.delete_asset_files(settings.download_root, asset.rel_path) db.delete(asset) diff --git a/backend/app/downloads/storage.py b/backend/app/downloads/storage.py index 0084938e..4a3ea17e 100644 --- a/backend/app/downloads/storage.py +++ b/backend/app/downloads/storage.py @@ -168,13 +168,25 @@ def upload_rel_path(user_id: int, asset_id: int, filename: str) -> str: episode sidecars). The name is system-decided and bound to the asset id (`_a{id}` keeps two same-named uploads apart); the original filename is display metadata only. The extension is preserved (shell-safe, bounded) so the served file + its Content-Disposition carry a correct - suffix; an extensionless upload stays extensionless.""" - stem = sanitize(Path(filename).stem, 80) + suffix; an extensionless upload stays extensionless. + + The stem holds no dot, so the last dot of the name is always the extension's: every sibling + derived by swapping it (`with_suffix` — the poster, the converted file) keeps `_a{id}`. A dotted + stem with no extension ('Holiday.2024.ä') once lost it, and two such uploads shared one file.""" + stem = sanitize(Path(filename).stem, 80).replace(".", "_") ext = _clean_upload_ext(filename) suffix = f".{ext}" if ext else "" return f".uploads/{user_id}/{stem}_a{asset_id}{suffix}" +def with_ext(rel: str, old_ext: str | None, ext: str) -> str: + """`rel` with its extension — `old_ext`, the stored `container`, None for a bare name — swapped + for `ext`. Not `Path.with_suffix`: on a bare name whose stem holds a dot (an upload placed before + the stems lost their dots) that would take the stem's tail for the extension, and `_a{id}` with it.""" + base = rel[: -len(old_ext) - 1] if old_ext and rel.endswith(f".{old_ext}") else rel + return f"{base}.{ext}" + + def upload_ext(filename: str) -> str | None: """The stored `container` for an uploaded file: its shell-safe lowercased extension, or None when the upload has no extension (a bare name). Shares `_clean_upload_ext` with `upload_rel_path` so the diff --git a/backend/app/downloads/upload.py b/backend/app/downloads/upload.py index ea565c50..1629a05e 100644 --- a/backend/app/downloads/upload.py +++ b/backend/app/downloads/upload.py @@ -2,14 +2,23 @@ A user-uploaded file joins the SAME library as a downloaded one, so it inherits the whole function set already built (metadata chips, share links, and — for media — in-app play/edit). Unlike a -download it bypasses yt-dlp AND the worker entirely: the file is already on the host, so ingest -probes it, places it on disk, and creates an already-`ready` MediaAsset + `done` DownloadJob in one -step — the same "the file exists, mark it done now" shortcut `service.enqueue` takes on a cache hit. +download it bypasses yt-dlp: the file is already on the host, so ingest probes it and places it on +disk. A file that already is the phone-safe shape (`playable`) — or is not media at all — becomes an +already-`ready` MediaAsset + `done` DownloadJob in one step, the same "the file exists, mark it done +now" shortcut `service.enqueue` takes on a cache hit. + +Media that is NOT phone-safe (an iPhone's HEVC .mov, a VP9 .webm, an .mkv) is placed as it came and +queued: the asset stays `pending` and the job `queued`, and the download worker converts it +(`pipeline._convert_upload`) — so a share link never hands a friend a file their phone cannot play. +Only the converted file is kept (the owner's choice: one file per upload). A 10-bit HDR original +becomes 8-bit H.264 with its colour tags kept — still tagged HDR (HLG/PQ, BT.2020), not tone-mapped +to SDR (the owner's choice too). Until then the original sits at the asset's `rel_path`, which is +what a failed conversion's Retry starts from again, and what a delete removes +(`service.release_asset`). Kinds (the invariant lives in `mediakind`): a probe with a real video stream -> video, audio-only -> audio, neither -> a generic 'file' (pdf/zip/docx/…) that is download-only and never rendered -inline (see the force-download serving in the file/watch routes). This module is the ingest seam a -future sub-function (e.g. convert) plugs a new adapter into, mirroring how it sidesteps the worker. +inline (see the force-download serving in the file/watch routes). An upload does NOT get an `expires_at`: it's user-provided and may be irreplaceable (there's no source to re-download), so it doesn't age out on the retention clock the way a re-fetchable download @@ -24,7 +33,8 @@ from sqlalchemy.orm import Session from app.config import settings from app.downloads import edit as editmod -from app.downloads import mediakind, probe, quota, service, storage +from app.downloads import mediakind, playable, probe, quota, service, storage +from app.downloads.status import JobStatus from app.models import DownloadJob, MediaAsset, display_text @@ -44,15 +54,22 @@ def ingest_upload( size: int, display_name: str | None = None, ) -> DownloadJob: - """Ingest an already-uploaded file at `src_path` into `user_id`'s library and return the `done` - job. `src_path` is MOVED into place on success (the caller stages it to a temp file first). + """Ingest an already-uploaded file at `src_path` into `user_id`'s library and return its job — + `done`, or `queued` for the worker's phone-safe conversion (see the module docstring). `src_path` + is MOVED into place on success (the caller stages it to a temp file first). Raises quota.QuotaExceeded (holdings/footprint cap) — checked BEFORE any disk write, so a rejected upload leaves nothing behind and the caller just cleans up its temp file.""" quota.check_upload(db, user_id, size) # the file's size is known → charge it up front - vcodec, acodec, width, height, duration = probe.probe_media(src_path) + data = probe.probe_json(src_path) + vcodec, acodec, width, height, duration = probe.facts(data) kind = mediakind.derive(vcodec, acodec) + # Judged by the extension the file is PLACED under (and the worker re-judges), not the raw one: + # 'clip.mp4~' is stored as .mp4, and judging 'mp4~' queued a conversion with nothing to convert. + ext = storage.upload_ext(filename) or "" + safe = playable.plan(data, ext) if kind != mediakind.FILE else None + convert = safe is not None and safe.needs_change # An upload never dedups (each file is its own asset): a random ref makes the cache key unique, # so `format_sig` is a constant marker rather than a format hash. @@ -66,17 +83,18 @@ def ingest_upload( rel = storage.upload_rel_path(user_id, asset.id, filename) storage.place_file(src_path, settings.download_root, rel) # From here the file is on disk under `.uploads/`. If the transaction fails before it commits, the - # asset/job rows roll back but the placed file would be ORPHANED — nothing references it, the - # per-user delete path keys off asset.rel_path (which is gone), and the GC orphan sweep skips the - # dot-prefixed `.uploads/` tree. So roll back the on-disk side too on any failure. + # asset/job rows roll back and nothing names the placed file: the per-user delete path keys off + # asset.rel_path (which is gone), and the GC's `.uploads` sweep reaches it only an hour later. So + # roll back the on-disk side too — unless the commit itself raised after it landed (see below). + asset_id = asset.id try: final = storage.abs_path(settings.download_root, rel) actual_size = final.stat().st_size if final.exists() else size - asset.status = "ready" + asset.status = "pending" if convert else "ready" asset.rel_path = rel asset.size_bytes = actual_size - asset.container = storage.upload_ext(filename) + asset.container = ext or None asset.vcodec = vcodec asset.acodec = acodec asset.width = width @@ -100,8 +118,8 @@ def ingest_upload( source_ref=ref, profile_snapshot={}, display_name=display_text(display_name), - status="done", - progress=100, + status=JobStatus.QUEUED if convert else JobStatus.DONE, + progress=0 if convert else 100, queue_pos=service.next_queue_pos(db), ) db.add(job) @@ -109,6 +127,11 @@ def ingest_upload( db.commit() except Exception: db.rollback() + if _row_landed(db, asset_id) is not False: + # The commit reached the server and lost its answer, or the row cannot be read: the file + # may be the asset's now, so it is never removed blind — the sweep takes it if no row + # names it. + raise # The rows are gone; remove the now-unreferenced file so no bytes leak. Clean the poster by its # DETERMINISTIC path (rel with a .jpg suffix, the same path ensure_poster writes to) rather than # ensure_poster's return value: it writes the frame directly to that path and returns None on an @@ -120,3 +143,13 @@ def ingest_upload( raise db.refresh(job) return job + + +def _row_landed(db: Session, asset_id: int) -> bool | None: + """After the ingest transaction raised: is the asset row there anyway? A commit can reach the + server and lose its answer (the connection drops after COMMIT). None when it cannot be read.""" + try: + return db.get(MediaAsset, asset_id) is not None + except Exception: # noqa: BLE001 — "cannot tell" is the answer, the caller acts on it + # silent-ok: the caller re-raises the ingest's own error. + return None diff --git a/backend/app/main.py b/backend/app/main.py index 14a2dafa..e06746b9 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -339,6 +339,8 @@ STATIC_DIR = Path(__file__).parent / "static_spa" # stay cached but forces revalidation (cheap 304 via the FileResponse ETag) on every load. The # hashed bundles under /assets can cache forever (a new build changes their filename). INDEX_HTML = STATIC_DIR / "index.html" +# A public share link's own page: the player alone, without the app's bundle (frontend/watch.html). +WATCH_HTML = STATIC_DIR / "watch.html" INDEX_HEADERS = {"Cache-Control": "no-cache"} app.mount( "/assets", @@ -354,16 +356,16 @@ async def index() -> FileResponse: @app.get("/watch/{token}") async def watch_page(token: str): - """Serve the SPA for a public share link, but with per-video Open Graph tags injected so the + """Serve the public player page for a share link, with per-video Open Graph tags injected so the link unfurls richly (title/channel/thumbnail) in Messenger/social crawlers. A real browser - ignores the extra tags and hydrates WatchPage as usual; invalid/expired/password links fall - back to the generic card.""" + ignores the extra tags and boots WatchPage; invalid/expired/password links fall back to the + generic card.""" from app.downloads import og with SessionLocal() as db: - rendered = og.render_watch_html(token, INDEX_HTML, db) + rendered = og.render_watch_html(token, WATCH_HTML, db) if rendered is None: - return FileResponse(INDEX_HTML, headers=INDEX_HEADERS) + return FileResponse(WATCH_HTML, headers=INDEX_HEADERS) return HTMLResponse(rendered, headers=INDEX_HEADERS) @@ -381,4 +383,8 @@ async def spa_fallback(full_path: str) -> FileResponse: # Real SPA-root assets (welcome images, favicon, robots.txt) rarely change and have # stable names, so a moderate cache is safe and satisfies the cache-lifetime audit. return FileResponse(candidate, headers={"Cache-Control": "public, max-age=604800"}) + # A /watch/ path the route above does not match (a trailing segment, no token) is still the + # player's: WatchPage reads the token off the path and says when a link is gone. + if full_path.startswith("watch/"): + return FileResponse(WATCH_HTML, headers=INDEX_HEADERS) return FileResponse(INDEX_HTML, headers=INDEX_HEADERS) diff --git a/backend/app/models.py b/backend/app/models.py index 608bc46d..4c799a33 100644 --- a/backend/app/models.py +++ b/backend/app/models.py @@ -912,7 +912,8 @@ class DownloadJob(Base, TimestampMixin, UpdatedAtMixin): ) source_kind: Mapped[str] = mapped_column(String(16)) # "youtube" | "url" source_ref: Mapped[str] = mapped_column(String(512)) - # "download" (default) or "edit" (a per-user trim/crop derivative of another job). + # "download" (default), "edit" (a per-user trim/crop derivative of another job) or "upload" (a + # user's own file, converted by the worker when it is not phone-safe — see `downloads.upload`). job_kind: Mapped[str] = mapped_column(String(16), default="download", server_default="download") # For edit jobs: the parent download this clip is cut from (SET NULL if the parent is deleted). source_job_id: Mapped[int | None] = mapped_column( diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index cb9e3e23..a6508e4b 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -32,6 +32,7 @@ from app.downloads import links as linksmod from app.downloads import quota, service, storage from app.downloads import upload as uploadmod from app.downloads import urlguard +from app.downloads.status import JobStatus from app.models import ( DownloadJob, DownloadLink, @@ -131,8 +132,8 @@ def _serialize(job: DownloadJob, asset: MediaAsset | None) -> dict: "job_kind": job.job_kind, "source_job_id": job.source_job_id, "edit_spec": job.edit_spec, - "can_download": ready and job.status == "done", - "expired": job.asset_id is None and job.status == "done", + "can_download": ready and job.status == JobStatus.DONE, + "expired": job.asset_id is None and job.status == JobStatus.DONE, "asset": None if asset is None else { @@ -502,7 +503,8 @@ def upload_download( (video/audio) gets the full function set, a generic file is download-only. The body IS the file (see the header note above); `spool_upload` has already authenticated the caller and streamed it to a temp path under the download volume, so what is left here is the non-yt-dlp ingest which - probes + places it and returns an already-`done` job. + probes + places it and returns its job — `done`, or `queued` for the worker's phone-safe + conversion (`downloads.upload`). A sync (threadpool) handler on purpose: the ingest does blocking work — an ffprobe of up to 30s plus sync DB writes — so it must NOT run on the event loop. The streaming half is async because @@ -539,7 +541,7 @@ def enqueue_edit( clip is a NEW per-user derivative in the editor's own library (counts against their quota); the source file is only read, never modified, so editing a shared video is safe.""" source_job = _accessible_job(db, user, body.source_job_id) - if source_job.status != "done" or source_job.asset_id is None: + if source_job.status != JobStatus.DONE or source_job.asset_id is None: raise refused(409, "edit_unfinished") spec = body.edit_spec display_name = display_text(body.display_name) @@ -593,7 +595,12 @@ def my_usage(user: User = Depends(current_user), db: Session = Depends(get_db)) # Priority when a source has several jobs (e.g. a done one plus a fresh queued one). -_INDEX_RANK = {"done": 4, "running": 3, "paused": 2, "queued": 1} +_INDEX_RANK: dict[str, int] = { + JobStatus.DONE: 4, + JobStatus.RUNNING: 3, + JobStatus.PAUSED: 2, + JobStatus.QUEUED: 1, +} @router.get("/index") @@ -603,7 +610,7 @@ def my_download_index(user: User = Depends(current_user), db: Session = Depends( rows = db.execute( select(DownloadJob.source_ref, DownloadJob.status).where( DownloadJob.user_id == user.id, - DownloadJob.status.in_(("queued", "running", "paused", "done")), + DownloadJob.status.in_(tuple(_INDEX_RANK)), ) ).all() out: dict[str, str] = {} @@ -734,7 +741,7 @@ def redownload_download( # Neither has a re-fetchable source: an editor clip is derived locally, an upload IS the # source. Re-running yt-dlp against source_kind='upload'/'edit' would just fail. raise refused(409, "not_redownloadable") - if job.status in ("queued", "running", "paused"): + if job.status in (JobStatus.QUEUED, JobStatus.RUNNING, JobStatus.PAUSED): raise refused(409, "still_downloading") service.redownload(db, job) return _serialize_job(db, job) @@ -886,7 +893,7 @@ def share_download( db: Session = Depends(get_db), ) -> dict: job = owned_or_404(db, DownloadJob, job_id, user, what="download") - if job.status != "done" or job.asset_id is None: + if job.status != JobStatus.DONE or job.asset_id is None: raise refused(409, "share_unfinished") target = body.email.strip().lower() if not target: @@ -1033,7 +1040,7 @@ def list_links( class CreateLinkIn(BaseModel): password: str = "" - # D1: a newly shared download is downloadable by default — the common intent is "let the + # A newly shared download is downloadable by default — the common intent is "let the # recipient have the file", not stream-only. Forward-only: this only sets the default for links # minted from now on; existing links keep whatever `allow_download` they were created with (the # column default stays False, so no back-catalogue link silently flips). The owner can still @@ -1050,7 +1057,7 @@ def create_link( db: Session = Depends(get_db), ) -> dict: job = owned_or_404(db, DownloadJob, job_id, user, what="download") - if job.status != "done" or job.asset_id is None: + if job.status != JobStatus.DONE or job.asset_id is None: raise refused(409, "share_unfinished") pw = body.password.strip() link = DownloadLink( @@ -1137,10 +1144,10 @@ def admin_list_downloads( @admin_router.get("/storage") def admin_storage(admin: User = Depends(admin_user), db: Session = Depends(get_db)) -> dict: - ready = db.execute( - select(func.count(), func.coalesce(func.sum(MediaAsset.size_bytes), 0)).where( - MediaAsset.status == "ready" - ) + # Every file on disk (`quota.ON_DISK`): ready ones, and uploads waiting for or failed in their + # conversion — the same disk the footprint and the GC cap count. + on_disk = db.execute( + select(func.count(), func.coalesce(func.sum(MediaAsset.size_bytes), 0)).where(quota.ON_DISK) ).one() total_assets = db.execute(select(func.count()).select_from(MediaAsset)).scalar() per_user = db.execute( @@ -1153,8 +1160,8 @@ def admin_storage(admin: User = Depends(admin_user), db: Session = Depends(get_d for u in db.execute(select(User).where(User.id.in_([uid for uid, _ in per_user]))).scalars() } return { - "ready_files": ready[0], - "total_bytes": int(ready[1]), + "files_on_disk": on_disk[0], + "total_bytes": int(on_disk[1]), "total_assets": total_assets, "total_cap_bytes": sysconfig.get_int(db, "download_total_max_bytes"), "per_user": [ diff --git a/backend/app/routes/public.py b/backend/app/routes/public.py index 90bd0a4d..fcf0d575 100644 --- a/backend/app/routes/public.py +++ b/backend/app/routes/public.py @@ -16,6 +16,7 @@ from app.config import settings from app.db import get_db from app.downloads import links as linksmod from app.downloads import storage +from app.downloads.status import JobStatus from app.models import DownloadJob, DownloadLink, MediaAsset from app.ratelimit import RateLimiter, client_key from app.security import verify_password @@ -46,6 +47,17 @@ def _link_or_404(db: Session, token: str) -> DownloadLink: def _ready_target(db: Session, link: DownloadLink) -> tuple[DownloadJob, MediaAsset]: job = db.get(DownloadJob, link.job_id) asset = db.get(MediaAsset, job.asset_id) if job and job.asset_id else None + if ( + job is not None + and asset is not None + and asset.status in ("pending", "downloading") + and job.status in (JobStatus.QUEUED, JobStatus.RUNNING) + ): + # Being (re)made — a re-download, or a conversion — so the link works again once it is ready: + # its own reason, so the page can say "try again shortly" rather than "this link is gone". + # Only while the job is still going: a cancelled or paused rebuild leaves the asset pending + # with nothing working on it, and "try again in a few minutes" would never come true. + raise refused(404, "video_preparing") if job is None or asset is None or asset.status != "ready" or not asset.rel_path: raise refused(404, "video_gone") # Verify the file is actually on disk here (not just flagged ready), so the watch page never diff --git a/backend/app/sync/tagsuggest.py b/backend/app/sync/tagsuggest.py index 14d0f7e6..a4e2d99e 100644 --- a/backend/app/sync/tagsuggest.py +++ b/backend/app/sync/tagsuggest.py @@ -12,7 +12,7 @@ result (measured on real data: every untagged channel got a suggestion, most on alone). Topic overlap is what makes "you tagged similar channels this way" meaningful. Purely read-only and deterministic — the Channel Manager surfaces the result as one-tap-accept -"ghost" chips; nothing is written here (D4 = suggest-and-confirm, no silent auto-apply). The +"ghost" chips; nothing is written here (suggest-and-confirm, never a silent auto-apply). The thresholds are tuning constants rather than admin knobs on purpose: they mirror how ``autotag.py`` keeps its mappings inline, and sysconfig only stores int/str/bool (no float), so a knob would mean storing percentages — not worth the surface for values an operator has no reason to tune per-install. diff --git a/backend/tests/test_download_deprioritize.py b/backend/tests/test_download_deprioritize.py index 0264a4c5..1e16a25c 100644 --- a/backend/tests/test_download_deprioritize.py +++ b/backend/tests/test_download_deprioritize.py @@ -8,6 +8,7 @@ import pytest from app.config import settings from app.downloads import edit as editmod +from app.downloads import playable from app.downloads import runner @@ -32,11 +33,12 @@ class TestEncodeThreadCap: assert editmod.encode_thread_cap() == 1 # max(1, 2 - 1) -class TestBrowserSafeThreads: +_REENCODE_VIDEO = playable.Plan("mp4", True, True, False, True) + + +class TestPhoneSafeThreads: def test_threads_flag_added_when_capped(self): - cmd = editmod.build_browser_safe_cmd( - "/in.mkv", "/out.mp4", reencode_video=True, reencode_audio=False, threads=2 - ) + cmd = playable.build_cmd("/in.mkv", "/out.mp4", _REENCODE_VIDEO, threads=2) assert "-threads" in cmd assert cmd[cmd.index("-threads") + 1] == "2" # The cap precedes the output file, so it binds the encoder (not input decoding). @@ -44,9 +46,7 @@ class TestBrowserSafeThreads: @pytest.mark.parametrize("threads", [0, None]) def test_no_threads_flag_when_uncapped(self, threads): - cmd = editmod.build_browser_safe_cmd( - "/in.mkv", "/out.mp4", reencode_video=True, reencode_audio=False, threads=threads - ) + cmd = playable.build_cmd("/in.mkv", "/out.mp4", _REENCODE_VIDEO, threads=threads) assert "-threads" not in cmd diff --git a/backend/tests/test_download_output.py b/backend/tests/test_download_output.py index 9d888b75..4e286817 100644 --- a/backend/tests/test_download_output.py +++ b/backend/tests/test_download_output.py @@ -6,7 +6,7 @@ with "PosixPath('.') has an empty name" instead of downloading.""" import pytest -from app.downloads.edit import _codecs_from_streams, _concat_quote, run_ffprobe_json +from app.downloads.edit import _concat_quote, run_ffprobe_json from app.downloads.pipeline import _media_info, _produced_file from pathlib import Path, PurePosixPath @@ -105,46 +105,9 @@ class TestMediaInfo: assert _media_info(info)["id"] == "vid" -class TestCodecsFromStreams: - # ffprobe emits codec_name BEFORE codec_type, so the old line parser shifted every name onto - # the previous stream and an H.264+AAC mp4 came back as (aac, None) → a needless re-encode. - # _codecs_from_streams reads each stream dict by its own codec_type, immune to that ordering. - def test_reads_each_stream_by_its_own_type(self): - streams = [ - {"codec_name": "h264", "codec_type": "video"}, - {"codec_name": "aac", "codec_type": "audio"}, - ] - assert _codecs_from_streams(streams) == ("h264", "aac") - - def test_audio_first_ordering_is_handled(self): - streams = [ - {"codec_name": "AAC", "codec_type": "audio"}, - {"codec_name": "H264", "codec_type": "video"}, - ] - assert _codecs_from_streams(streams) == ("h264", "aac") - - def test_extra_streams_do_not_win_over_the_first_of_their_kind(self): - streams = [ - {"codec_name": "h264", "codec_type": "video"}, - {"codec_name": "vp9", "codec_type": "video"}, - {"codec_name": "mov_text", "codec_type": "subtitle"}, - {"codec_name": "opus", "codec_type": "audio"}, - ] - assert _codecs_from_streams(streams) == ("h264", "opus") - - def test_audio_only_and_video_only(self): - assert _codecs_from_streams([{"codec_name": "opus", "codec_type": "audio"}]) == ( - None, - "opus", - ) - assert _codecs_from_streams([{"codec_name": "av1", "codec_type": "video"}]) == ("av1", None) - - def test_empty_stream_list(self): - assert _codecs_from_streams([]) == (None, None) - - class TestRunFfprobeJson: - """The single ffprobe-JSON invocation point (shared by probe_codecs + probe.probe_media). A failed + """The single ffprobe-JSON invocation point (`probe.probe_json`, which the stored facts and the + phone-safe rule both read). A failed probe or unparseable output must degrade to None, not raise — the malformed-input safety the old parse_probe_streams string parser used to own now lives here.""" @@ -174,7 +137,7 @@ class TestRunFfprobeJson: monkeypatch.setattr(subprocess, "run", lambda *a, **k: _R()) data = run_ffprobe_json(Path("x"), entries="stream=codec_type,codec_name") - assert _codecs_from_streams(data["streams"]) == ("h264", None) + assert data["streams"] == [{"codec_name": "h264", "codec_type": "video"}] class TestConcatQuote: diff --git a/backend/tests/test_ffmpeg_input_guard.py b/backend/tests/test_ffmpeg_input_guard.py new file mode 100644 index 00000000..0d73299d --- /dev/null +++ b/backend/tests/test_ffmpeg_input_guard.py @@ -0,0 +1,198 @@ +"""Every ffmpeg/ffprobe the downloads package runs over a library file names the demuxers it may use +(`edit.INPUT_GUARD`, a downloaded thumbnail's `edit.THUMB_GUARD`). + +Without the list, ffmpeg picks a demuxer from the file's content and name, and both are the user's +for an upload (the remote site's for a thumbnail): a playlist-type demuxer opens whatever local paths +its entries name. These tests read the argv each builder hands to ffmpeg — the guard is a flag, so +the flag is what is held — and scan the package for an ffmpeg/ffprobe call written without one. + +Not in scope: `app/plex`, whose ffmpeg reads the admin's own Plex library, not user-supplied bytes. +""" + +import ast +import shutil +import subprocess +from pathlib import Path + +import pytest + +from app.downloads import edit, playable + +DOWNLOADS = Path(__file__).resolve().parents[1] / "app" / "downloads" +# The demuxers that read paths named inside a file. `concat` is allowed only where the list is ours. +PLAYLIST_DEMUXERS = {"hls", "dash", "concat", "image2", "ffconcat", "tee"} +MEDIA = set(edit.MEDIA_DEMUXERS) + + +def _whitelists(cmd: list[str]) -> list[set[str] | None]: + """For each `-i` in `cmd`, the demuxers the `-format_whitelist` given since the previous input + allows — None when that input has none.""" + out, current = [], None + for i, tok in enumerate(cmd): + if tok == "-format_whitelist": + current = set(cmd[i + 1].split(",")) + elif tok == "-i": + out.append(current) + current = None + return out + + +def _assert_media_guarded(cmd: list[str], extra: set[str] = frozenset()) -> None: + lists = _whitelists(cmd) + assert lists, cmd # a builder that opens no input is not what this test is about + for allowed in lists: + assert allowed is not None, f"an input with no -format_whitelist: {cmd}" + assert allowed <= MEDIA | extra, allowed - MEDIA - extra + assert not (allowed - extra) & PLAYLIST_DEMUXERS + + +def test_the_media_list_names_no_demuxer_that_reads_other_paths(): + assert not MEDIA & PLAYLIST_DEMUXERS + assert edit.INPUT_GUARD == ["-format_whitelist", ",".join(edit.MEDIA_DEMUXERS)] + + +def test_the_phone_safe_conversion_is_guarded(): + p = playable.Plan("mp4", True, True, True, True) + _assert_media_guarded(playable.build_cmd(Path("in.mov"), Path("out.mp4"), p)) + + +@pytest.mark.parametrize( + "spec", + [ + {"trim": {"start_s": 1, "end_s": 3}, "accurate": False}, + {"trim": {"start_s": 1, "end_s": 3}, "accurate": True}, + {"crop": {"x": 0, "y": 0, "w": 101, "h": 51}}, + ], + ids=["fast trim", "accurate trim", "crop"], +) +def test_a_clip_is_guarded(spec): + spec = edit.normalize_edit_spec(spec) + _assert_media_guarded(edit.build_edit_cmd(Path("in.mp4"), Path("out.mp4"), spec, "mp4")) + + +@pytest.mark.parametrize("accurate", [True, False], ids=["accurate join", "fast join"]) +def test_a_join_is_guarded(accurate, tmp_path): + segs = [{"start_s": 0, "end_s": 1}, {"start_s": 2, "end_s": 3}] + spec = edit.normalize_edit_spec({"segments": segs, "accurate": accurate}) + cmd, _ = edit.build_concat_plan(Path("in.mp4"), Path("out.mp4"), spec, "mp4", tmp_path) + # The fast join reads a concat list WE write, so the list itself may be opened as concat. + _assert_media_guarded(cmd, extra=set() if accurate else {"concat"}) + + +def test_the_fast_join_list_gives_each_entry_the_media_list_without_concat(tmp_path): + # ffmpeg hands the list's whitelist (with `concat`) down to the library file each entry names; + # the entry's own `option format_whitelist` must replace it, or that file may open as a list. + segs = [{"start_s": 0, "end_s": 1}, {"start_s": 2, "end_s": 3}] + spec = edit.normalize_edit_spec({"segments": segs, "accurate": False}) + _, prep = edit.build_concat_plan(Path("in.mp4"), Path("out.mp4"), spec, "mp4", tmp_path) + (text,) = prep.values() + lines = text.splitlines() + entries = [i for i, ln in enumerate(lines) if ln.startswith("file ")] + assert len(entries) == 2 + for i in entries: + key, _, allowed = lines[i + 1].partition(" format_whitelist ") + assert key == "option", lines[i + 1] + assert set(allowed.split(",")) == MEDIA + + +@pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available") +def test_the_fast_join_still_joins_a_real_file(tmp_path): + src = tmp_path / "in.mp4" + subprocess.run( + ["ffmpeg", "-y", "-loglevel", "error", "-f", "lavfi", "-i", "testsrc=size=64x48:rate=10", + "-f", "lavfi", "-i", "sine=frequency=440", "-t", "4", "-c:v", "libx264", + "-pix_fmt", "yuv420p", "-c:a", "aac", str(src)], + check=True, + ) # fmt: skip + segs = [{"start_s": 0, "end_s": 1}, {"start_s": 2, "end_s": 3}] + spec = edit.normalize_edit_spec({"segments": segs, "accurate": False}) + out = tmp_path / "out.mp4" + cmd, prep = edit.build_concat_plan(src, out, spec, "mp4", tmp_path) + for p, content in prep.items(): + p.write_text(content, encoding="utf-8") + subprocess.run(cmd, check=True, capture_output=True) + assert (edit.probe_duration(out) or 0) > 0 + + +def test_the_poster_and_the_filmstrip_are_guarded(): + _assert_media_guarded(edit.build_poster_cmd(Path("in.mp4"), Path("p.jpg"), 2.0)) + geom = edit.storyboard_geometry(60) + _assert_media_guarded(edit.build_storyboard_cmd(Path("in.mp4"), Path("s.jpg"), 60, geom)) + + +def test_a_downloaded_thumbnail_opens_as_a_single_image_only(): + (allowed,) = _whitelists(edit.build_thumbnail_jpg_cmd(Path("t.webp"), Path("t.jpg"))) + assert allowed == {"png_pipe", "jpeg_pipe", "webp_pipe"} + + +def test_every_ffprobe_is_guarded(monkeypatch): + seen = [] + + class _Done: + stdout = "{}" + + def _run(args, **_k): + seen.append(args) + return _Done() + + monkeypatch.setattr(edit.subprocess, "run", _run) + edit.run_ffprobe_json(Path("x.mp4"), show_streams=True) + edit.probe_duration(Path("x.mp4")) + assert len(seen) == 2 + for args in seen: + i = args.index("-format_whitelist") + assert set(args[i + 1].split(",")) == MEDIA + + +def _unguarded_calls(source: str) -> list[int]: + """Lines of a list literal that starts an ffmpeg/ffprobe command or hands one an input (`-i`) + and carries no `*…GUARD` / `*guard` splat and no "-format_whitelist" of its own. A builder that + adds its input in a later `+=` must guard that list — that is where the input is.""" + bad = [] + for node in ast.walk(ast.parse(source)): + if not isinstance(node, ast.List): + continue + consts = {e.value for e in node.elts if isinstance(e, ast.Constant)} + if not consts & {"-i", "ffprobe"}: + continue + guarded = "-format_whitelist" in consts or any( + isinstance(e, ast.Starred) + and isinstance(v := e.value, ast.Name | ast.Attribute) + and (v.id if isinstance(v, ast.Name) else v.attr).upper().endswith("GUARD") + for e in node.elts + ) + if not guarded: + bad.append(node.lineno) + return bad + + +def test_no_ffmpeg_call_in_the_package_opens_an_input_unguarded(): + found = { + f"{p.name}:{line}" + for p in sorted(DOWNLOADS.rglob("*.py")) + for line in _unguarded_calls(p.read_text(encoding="utf-8")) + } + assert found == set() + + +def test_the_scan_sees_an_unguarded_call(): + # The scan above is green over nothing if it cannot see one: both shapes it must catch. + assert _unguarded_calls('cmd = ["ffmpeg", "-y", "-i", src]') == [1] + assert _unguarded_calls('a = ["ffprobe", "-v", "error", str(p)]') == [1] + assert _unguarded_calls('cmd = [*INPUT_GUARD, "-i", src]') == [] + + +@pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available") +@pytest.mark.parametrize("ext", ["png", "webp", "jpg"]) +def test_the_thumbnail_guard_still_opens_every_thumbnail_a_download_keeps(ext, tmp_path): + # `pipeline._THUMB_SUFFIXES`: what the guard must still let through (a .jpg that is really a + # PNG goes the ffmpeg route too — `thumbnail_to_jpg` copies only true JPEG bytes). + src = tmp_path / f"t.{ext}" + fmt = ["-f", "image2", "-c:v", "png"] if ext == "jpg" else [] + subprocess.run( + ["ffmpeg", "-y", "-loglevel", "error", "-f", "lavfi", "-i", "testsrc2=size=64x48", + "-frames:v", "1", *fmt, str(src)], + check=True, + ) # fmt: skip + out = edit.thumbnail_to_jpg(src, tmp_path / "poster.jpg") + assert out is not None and edit._is_jpeg(out) diff --git a/backend/tests/test_formats.py b/backend/tests/test_formats.py index abf79b1c..ab461d42 100644 --- a/backend/tests/test_formats.py +++ b/backend/tests/test_formats.py @@ -94,23 +94,6 @@ def test_audio_only_unaffected(): assert _opts({"mode": "a", "audio_format": "m4a"})["format"] == "bestaudio/best" -def test_browser_transcode_flags_universal_playability(): - F = editmod.browser_transcode_flags - # Already universal: H.264 + AAC in mp4 → no change. - assert F("h264", "aac", True) == (False, False, False) - assert F("h264", "mp3", True) == (False, False, False) - # Non-H.264 video → full video re-encode (AV1/VP9 fail iOS; HEVC fails Chrome/FF). - assert F("av1", "aac", True) == (True, True, False) - assert F("vp9", "opus", True) == (True, True, True) - assert F("hevc", "aac", True) == (True, True, False) - # H.264 but bad audio → audio-only transcode (video copied). - assert F("h264", "opus", True) == (True, False, True) - # Codecs fine but wrong container → remux only (no re-encode). - assert F("h264", "aac", False) == (True, False, False) - # A failed probe / missing stream is never re-encoded. - assert F(None, None, True) == (False, False, False) - - # --- SEC-13 · one enqueue is one bounded download ------------------------------------------- diff --git a/backend/tests/test_links.py b/backend/tests/test_links.py index 78c18096..f3e8e47d 100644 --- a/backend/tests/test_links.py +++ b/backend/tests/test_links.py @@ -99,7 +99,7 @@ class TestPublicMetaAudioFlag: class TestCreateLinkDefault: - """D1: a newly minted share link is downloadable by default. The one-click Share relies on + """A newly minted share link is downloadable by default. The one-click Share relies on this — it POSTs no `allow_download`, so the API default decides. Forward-only lives here (the API default), NOT in a migration: the DownloadLink column default stays False, so no existing back- catalogue link silently flips to downloadable.""" diff --git a/backend/tests/test_odd_frame.py b/backend/tests/test_odd_frame.py new file mode 100644 index 00000000..13f13561 --- /dev/null +++ b/backend/tests/test_odd_frame.py @@ -0,0 +1,73 @@ +"""A libx264 re-encode of an odd-sized frame: 4:2:0 halves the chroma, so libx264 refuses an odd +width or height ("width not divisible by 2"). Measured: a source that is odd AND not 4:2:0 already +(a 4:4:4 or RGB screen capture, 1365x767) failed its conversion outright; a 4:2:0 source never +reaches the encoder odd (ffmpeg's crop rounds to its chroma grid), but the clip then stored the +size the user dragged, not the one the file has. Every re-encode now ends its filter chain on +`edit.EVEN_FRAME` (at most one pixel row or column), and a clip stores its probed frame. + +Real ffmpeg on both paths — the refusal is the encoder's, so only the encoder can show it is gone. +""" + +import shutil +import subprocess +from pathlib import Path + +import pytest + +from app.config import settings +from app.downloads import pipeline, playable, probe, service, storage +from app.downloads import upload as uploadmod +from app.models import DownloadJob, MediaAsset + +pytestmark = pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available") + + +def _ffmpeg(*args: str) -> None: + subprocess.run(["ffmpeg", "-y", "-loglevel", "error", *args], check=True) + + +def _frame(path: Path) -> tuple[int | None, int | None]: + _, _, w, h, _ = probe.probe_media(path) + return w, h + + +def test_an_odd_sized_4_4_4_video_converts_to_the_even_frame_below_it(tmp_path): + src = tmp_path / "in.mkv" + _ffmpeg( + "-f", "lavfi", "-i", "testsrc=size=161x121:rate=10:duration=1", + "-c:v", "libx264", "-pix_fmt", "yuv444p", str(src), + ) # fmt: skip + assert _frame(src) == (161, 121) # odd for real (`testsrc2` rounds its own size) + p = playable.plan(probe.probe_json(src), "mkv") + assert p is not None and p.reencode_video + dest = tmp_path / "out.mp4" + subprocess.run(playable.build_cmd(src, dest, p), check=True, capture_output=True) + assert _frame(dest) == (160, 120) + + +def test_an_odd_crop_makes_a_clip_and_stores_the_frame_it_has(db, make_user, tmp_path, monkeypatch): + monkeypatch.setattr(settings, "download_root", str(tmp_path)) + src = tmp_path / "in.mp4" + _ffmpeg( + "-f", "lavfi", "-i", "testsrc2=size=160x120:rate=10:duration=2", + "-f", "lavfi", "-i", "sine=duration=2", + "-c:v", "libx264", "-pix_fmt", "yuv420p", "-c:a", "aac", str(src), + ) # fmt: skip + u = make_user() + source = uploadmod.ingest_upload( + db, u.id, filename="clip.mp4", src_path=src, size=src.stat().st_size + ) + assert source.status == "done" # already phone-safe: the clip's source as it came + job = service.enqueue_edit(db, u.id, source, {"crop": {"x": 3, "y": 5, "w": 101, "h": 51}}) + asset = db.get(MediaAsset, job.asset_id) + job.status, asset.status = "running", "downloading" # the worker's claim + db.commit() + + pipeline._process_edit(job.id, asset.id) + + db.expire_all() + job, asset = db.get(DownloadJob, job.id), db.get(MediaAsset, job.asset_id) + assert (job.status, job.error, asset.status) == ("done", None, "ready") + final = storage.abs_path(str(tmp_path), asset.rel_path) + assert _frame(final) == (100, 50) + assert (asset.width, asset.height) == (100, 50) # what the file is, not what was asked diff --git a/backend/tests/test_phone_safe_contract.py b/backend/tests/test_phone_safe_contract.py new file mode 100644 index 00000000..50505f73 --- /dev/null +++ b/backend/tests/test_phone_safe_contract.py @@ -0,0 +1,163 @@ +"""The share-link contract: every media file a /watch link can serve passes the phone-safe rule +(`downloads.playable`). + +Two halves. The structural one reads the code: a link serves an asset once it is `ready`, so every +function that marks one ready must run the rule — a new path to `ready` that skips it is named here, +before a friend's phone meets the file. The end-to-end one sends the files measured broken on an +iPhone + iPad (and the ones browsers other than Safari cannot be trusted with) through upload, the +worker's conversion and the public file route, and judges the bytes the link actually hands out. +""" + +import ast +import shutil +import subprocess +from pathlib import Path + +import pytest + +from app.config import settings +from app.downloads import pipeline, playable, probe +from app.models import DownloadJob, MediaAsset + +APP = Path(__file__).resolve().parents[1] / "app" + +# The calls that ARE the rule: the worker's step (download, clip, upload conversion) and the upload's +# judgement of a file it keeps as it came. +_RULE_CALLS = {"_ensure_phone_safe", "plan"} + + +def _marks_ready(node: ast.AST) -> bool: + """`x.status = "ready"`, `status="ready"`, or `"…" if … else "ready"` handed to either.""" + + def is_ready(v: ast.AST | None) -> bool: + if isinstance(v, ast.Constant): + return v.value == "ready" + if isinstance(v, ast.IfExp): + return is_ready(v.body) or is_ready(v.orelse) + return False + + if isinstance(node, ast.Assign): + return is_ready(node.value) and any( + isinstance(t, ast.Attribute) and t.attr == "status" for t in node.targets + ) + return isinstance(node, ast.keyword) and node.arg == "status" and is_ready(node.value) + + +def _calls(fn: ast.AST) -> set[str]: + names = set() + for n in ast.walk(fn): + if isinstance(n, ast.Call): + f = n.func + names.add(f.attr if isinstance(f, ast.Attribute) else getattr(f, "id", "")) + return names + + +def ready_sites(source: str) -> list[tuple[str, int, bool]]: + """(function, line, runs the rule) for every place `source` marks something ready.""" + sites = [] + for fn in ast.walk(ast.parse(source)): + if not isinstance(fn, ast.FunctionDef | ast.AsyncFunctionDef): + continue + runs_rule = bool(_calls(fn) & _RULE_CALLS) + for n in ast.walk(fn): + if _marks_ready(n): + sites.append((fn.name, n.lineno, runs_rule)) + return sites + + +def test_every_path_to_ready_runs_the_rule(): + found, skipped = [], [] + for path in sorted(APP.rglob("*.py")): + for name, line, runs_rule in ready_sites(path.read_text(encoding="utf-8")): + where = f"{path.relative_to(APP.parent).as_posix()}:{line} {name}" + found.append(where) + if not runs_rule: + skipped.append(where) + # The paths known today: a download, an editor clip, an upload's conversion, an upload kept as is. + # Fewer means the scan went blind (a rename, a new status spelling) — not that a path is gone. + names = {w.rsplit(" ", 1)[1] for w in found} + assert {"_download", "_process_edit", "_convert_upload", "ingest_upload"} <= names, found + assert skipped == [], f"marks an asset ready without the phone-safe rule: {skipped}" + + +def test_the_scan_names_a_path_that_skips_the_rule(): + planted = 'def restore(asset):\n asset.status = "ready"\n' + guarded = ( + 'def restore(asset, p):\n _ensure_phone_safe(1, p, p, "x")\n asset.status = "ready"\n' + ) + assert ready_sites(planted) == [("restore", 2, False)] + assert ready_sites(guarded) == [("restore", 3, True)] + assert ( + ready_sites('def f(a, x):\n a.update(status="pending" if x else "ready")\n')[0][2] + is False + ) + + +# --- end to end: upload → worker → the bytes a link serves --------------------------------------- + +_V = ["-f", "lavfi", "-i", "testsrc2=size=160x120:rate=10:duration=1"] +_A = ["-f", "lavfi", "-i", "sine=duration=1"] + +# name -> (ffmpeg args after the inputs, filename the user uploads) +_FILES = { + "hevc tagged hev1 (iOS: broken)": ( + [*_V, *_A, "-c:v", "libx265", "-tag:v", "hev1", "-c:a", "aac"], + "phone.mov", + ), + "av1 (iOS: broken)": ([*_V, *_A, "-c:v", "libsvtav1", "-c:a", "aac"], "clip.mp4"), + "h264 in matroska (iOS: broken)": ([*_V, *_A, "-c:v", "libx264", "-c:a", "aac"], "clip.mkv"), + "mp3 inside an mp4 (iOS: broken)": ([*_A, "-c:a", "libmp3lame", "-f", "mp4"], "song.mp4"), + "vp9 + opus webm": ([*_V, *_A, "-c:v", "libvpx-vp9", "-c:a", "libopus"], "clip.webm"), + "10-bit h264": ( + [*_V, *_A, "-c:v", "libx264", "-pix_fmt", "yuv420p10le", "-c:a", "aac"], + "clip.mp4", + ), + "opus in ogg": ([*_A, "-c:a", "libopus"], "voice.ogg"), +} + + +@pytest.fixture +def media_root(tmp_path, monkeypatch): + root = tmp_path / "media" + root.mkdir() + monkeypatch.setattr(settings, "download_root", str(root)) + return root + + +@pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available") +@pytest.mark.parametrize("name", _FILES) +def test_what_a_link_serves_passes_the_rule( + name, db, as_user, make_user, client, media_root, tmp_path, upload_headers +): + args, filename = _FILES[name] + src = tmp_path / f"src{Path(filename).suffix}" + subprocess.run(["ffmpeg", "-y", "-loglevel", "error", *args, str(src)], check=True) + + owner = as_user(make_user()) + r = owner.post( + "/api/downloads/upload", content=src.read_bytes(), headers=upload_headers(filename) + ) + assert r.status_code == 200, r.text + job_id = r.json()["id"] + assert r.json()["status"] == "queued" # every one of these fails the rule as it came + job = db.get(DownloadJob, job_id) + asset = db.get(MediaAsset, job.asset_id) + job.status, asset.status = "running", "downloading" # what the worker's claim does + db.commit() + pipeline._convert_upload(job.id, asset.id) + db.expire_all() + assert db.get(DownloadJob, job_id).status == "done" + + link = owner.post(f"/api/downloads/{job_id}/links", json={}) + assert link.status_code == 200, link.text + served = client.get(f"/api/public/watch/{link.json()['token']}/file") + assert served.status_code == 200 + got = tmp_path / "served.bin" + got.write_bytes(served.content) + data = probe.probe_json(got) + ext = {"video/mp4": "mp4", "audio/mp4": "m4a", "audio/mpeg": "mp3"}.get( + served.headers["content-type"].split(";")[0] + ) + assert ext is not None, served.headers["content-type"] # served as a type every phone plays + verdict = playable.plan(data, ext) + assert verdict is not None and not verdict.needs_change, (name, data["streams"]) diff --git a/backend/tests/test_playable.py b/backend/tests/test_playable.py new file mode 100644 index 00000000..ddb58e35 --- /dev/null +++ b/backend/tests/test_playable.py @@ -0,0 +1,157 @@ +"""The phone-safe rule (`downloads.playable`): what a library file must be so a share link plays +on any phone or browser, and the one ffmpeg command that makes it so. + +The plan table is the measured device verdict, class by class: what failed on a real iPhone + iPad +(AV1, Matroska, HEVC, MP3 inside an .mp4) must change, and so must what browsers other than Safari +cannot be trusted with (10-bit / 4:4:4 H.264, VP9, Opus). The ffmpeg cases run the real encoder and +probe its output, because the rule is about the bytes, not the flags. +""" + +import shutil +import subprocess +from pathlib import Path + +import pytest + +from app.downloads import playable, probe + +# conftest turns a missing ffmpeg into a hard error in the gate (SIFTLODE_TEST_REQUIRE_DB), so this +# only ever skips on an explicit pure-logic run. +needs_ffmpeg = pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available") + + +def _data(video=None, audio=None, fmt="mov,mp4,m4a,3gp,3g2,mj2", cover=False): + streams = [] + if cover: + streams.append( + {"codec_type": "video", "codec_name": "mjpeg", "disposition": {"attached_pic": 1}} + ) + if video: + codec, pix = video + streams.append({"codec_type": "video", "codec_name": codec, "pix_fmt": pix}) + if audio: + streams.append({"codec_type": "audio", "codec_name": audio}) + return {"streams": streams, "format": {"format_name": fmt}} + + +# (probe, extension) -> (ext, reencode_video, reencode_audio, needs_change) +CASES = { + "h264 8-bit + aac in mp4": (_data(("h264", "yuv420p"), "aac"), "mp4", ("mp4", 0, 0, 0)), + "full-range h264 is still 8-bit 4:2:0": ( + _data(("h264", "yuvj420p"), "aac"), + "mp4", + ("mp4", 0, 0, 0), + ), + "10-bit h264": (_data(("h264", "yuv420p10le"), "aac"), "mp4", ("mp4", 1, 0, 1)), + "4:4:4 h264": (_data(("h264", "yuv444p"), "aac"), "mp4", ("mp4", 1, 0, 1)), + "hevc (an iPhone's .mov)": (_data(("hevc", "yuv420p10le"), "aac"), "mov", ("mp4", 1, 0, 1)), + "vp9 + opus webm": (_data(("vp9", "yuv420p"), "opus"), "webm", ("mp4", 1, 1, 1)), + "av1": (_data(("av1", "yuv420p"), "aac"), "mp4", ("mp4", 1, 0, 1)), + "h264 + aac in mkv: remux only": (_data(("h264", "yuv420p"), "aac"), "mkv", ("mp4", 0, 0, 1)), + "h264 + opus in mp4": (_data(("h264", "yuv420p"), "opus"), "mp4", ("mp4", 0, 1, 1)), + "mp3 beside video in mp4": (_data(("h264", "yuv420p"), "mp3"), "mp4", ("mp4", 0, 1, 1)), + "a silent h264 video": (_data(("h264", "yuv420p")), "mp4", ("mp4", 0, 0, 0)), + "mp3": (_data(audio="mp3", fmt="mp3"), "mp3", ("mp3", 0, 0, 0)), + "mp3 with cover art": (_data(audio="mp3", fmt="mp3", cover=True), "mp3", ("mp3", 0, 0, 0)), + "mp3 inside an mp4: remux to .mp3": (_data(audio="mp3"), "mp4", ("mp3", 0, 0, 1)), + "aac in m4a": (_data(audio="aac"), "m4a", ("m4a", 0, 0, 0)), + "aac audio-only in mp4: remux to .m4a": (_data(audio="aac"), "mp4", ("m4a", 0, 0, 1)), + "opus in ogg": (_data(audio="opus", fmt="ogg"), "ogg", ("m4a", 0, 1, 1)), +} + + +@pytest.mark.parametrize("name", CASES) +def test_the_plan_for_each_class(name): + data, ext, (want_ext, rv, ra, needs) = CASES[name] + p = playable.plan(data, ext) + assert p is not None + assert (p.ext, p.reencode_video, p.reencode_audio, p.needs_change) == ( + want_ext, + bool(rv), + bool(ra), + bool(needs), + ) + + +@pytest.mark.parametrize( + "data", + [ + None, # ffprobe could not read it + {"streams": [], "format": {"format_name": "pdf"}}, + _data(("png", "rgb24"), fmt="png_pipe"), # a still image is a download-only file + ], +) +def test_a_file_that_is_not_media_has_no_plan(data): + assert playable.plan(data, "bin") is None + + +def test_the_extension_is_read_with_or_without_its_dot(): + data = _data(("h264", "yuv420p"), "aac") + assert playable.plan(data, ".MP4") == playable.plan(data, "mp4") + assert not playable.plan(data, ".MP4").needs_change + + +def test_a_video_re_encode_pins_8_bit_420_and_maps_only_the_judged_streams(): + p = playable.Plan("mp4", True, True, False, True) + cmd = playable.build_cmd(Path("/in.mov"), Path("/out.mp4"), p) + assert cmd[cmd.index("-pix_fmt") + 1] == "yuv420p" + assert cmd[cmd.index("-c:v") + 1] == "libx264" + assert cmd[cmd.index("-c:a") + 1] == "copy" + maps = [cmd[i + 1] for i, a in enumerate(cmd) if a == "-map"] + assert maps == ["0:V:0", "0:a:0?"] # never cover art, never a subtitle the muxer refuses + assert cmd[-3:] == ["-movflags", "+faststart", "/out.mp4"] + + +def test_an_audio_remux_copies_the_audio_alone(): + p = playable.Plan("mp3", False, False, False, True) + cmd = playable.build_cmd(Path("/in.mp4"), Path("/out.mp3"), p) + assert [cmd[i + 1] for i, a in enumerate(cmd) if a == "-map"] == ["0:a:0"] + assert "-c:v" not in cmd and "-pix_fmt" not in cmd + assert cmd[cmd.index("-c:a") + 1] == "copy" + assert "-movflags" not in cmd # an .mp3 has no moov atom to move + + +# --- the real encoder ------------------------------------------------------------------------ + + +def _ffmpeg(*args: str) -> None: + subprocess.run(["ffmpeg", "-y", "-loglevel", "error", *args], check=True) + + +def _convert(src: Path, tmp_path: Path) -> Path: + p = playable.plan(probe.probe_json(src), src.suffix) + assert p is not None and p.needs_change + dest = tmp_path / f"out.{p.ext}" + subprocess.run(playable.build_cmd(src, dest, p), check=True, capture_output=True) + return dest + + +@needs_ffmpeg +def test_a_10_bit_matroska_with_opus_comes_out_phone_safe(tmp_path): + src = tmp_path / "in.mkv" + _ffmpeg( + "-f", "lavfi", "-i", "testsrc2=size=160x120:rate=10:duration=1", + "-f", "lavfi", "-i", "sine=duration=1", + "-c:v", "libx264", "-pix_fmt", "yuv420p10le", "-c:a", "libopus", + str(src), + ) # fmt: skip + out = _convert(src, tmp_path) + data = probe.probe_json(out) + video, audio = probe.primary_streams(data) + assert (video["codec_name"], video["pix_fmt"], video["profile"]) == ("h264", "yuv420p", "High") + assert audio["codec_name"] == "aac" + assert out.suffix == ".mp4" + assert not playable.plan(data, out.suffix).needs_change # the rule accepts its own output + + +@needs_ffmpeg +def test_mp3_inside_an_mp4_comes_out_as_an_mp3(tmp_path): + # The one unsafe file prod held: an audio download remuxed into .mp4, which iPhone and iPad + # both refused to play. + src = tmp_path / "in.mp4" + _ffmpeg("-f", "lavfi", "-i", "sine=duration=1", "-c:a", "libmp3lame", "-f", "mp4", str(src)) + out = _convert(src, tmp_path) + data = probe.probe_json(out) + assert out.suffix == ".mp3" + assert probe.primary_streams(data)[1]["codec_name"] == "mp3" + assert not playable.plan(data, out.suffix).needs_change diff --git a/backend/tests/test_public_unlock_limit.py b/backend/tests/test_public_unlock_limit.py index 816575a2..d4fc12d6 100644 --- a/backend/tests/test_public_unlock_limit.py +++ b/backend/tests/test_public_unlock_limit.py @@ -20,7 +20,10 @@ _PW = "open sesame" @pytest.fixture def unlock_url(as_user, make_user, tmp_path, monkeypatch, upload_headers): monkeypatch.setattr(settings, "download_root", str(tmp_path)) - monkeypatch.setattr(probemod, "probe_media", lambda _p: ("h264", "aac", 320, 240, 5)) + video = {"codec_type": "video", "codec_name": "h264", "pix_fmt": "yuv420p"} + audio = {"codec_type": "audio", "codec_name": "aac"} + data = {"streams": [video, audio], "format": {"format_name": "mov,mp4", "duration": "5"}} + monkeypatch.setattr(probemod, "probe_json", lambda _p: data) owner = as_user(make_user()) job = owner.post( "/api/downloads/upload", content=b"fake-video", headers=upload_headers("clip.mp4") diff --git a/backend/tests/test_spa_entries.py b/backend/tests/test_spa_entries.py new file mode 100644 index 00000000..7fab4d24 --- /dev/null +++ b/backend/tests/test_spa_entries.py @@ -0,0 +1,67 @@ +"""Which HTML page the server hands a path (main.py), DB-lane. + +The frontend builds two entries: the app (index.html) and a shared link's player alone (watch.html), so +the friend a /watch link was sent to never downloads the app. Every /watch/ path must get the player's +page — with the link's Open Graph tags when the token names a playable link — and nothing else may. +""" + +import pytest + +from app import main +from app.config import settings +from app.downloads import probe as probemod + +_PAGE = """ + +""" + + +@pytest.fixture +def pages(tmp_path, monkeypatch): + index, watch = tmp_path / "index.html", tmp_path / "watch.html" + index.write_text(_PAGE.format(entry="app"), encoding="utf-8") + watch.write_text(_PAGE.format(entry="watch"), encoding="utf-8") + monkeypatch.setattr(main, "INDEX_HTML", index) + monkeypatch.setattr(main, "WATCH_HTML", watch) + + +@pytest.fixture +def link_token(as_user, make_user, tmp_path, monkeypatch, upload_headers): + monkeypatch.setattr(settings, "download_root", str(tmp_path / "media")) + video = {"codec_type": "video", "codec_name": "h264", "pix_fmt": "yuv420p"} + audio = {"codec_type": "audio", "codec_name": "aac"} + data = {"streams": [video, audio], "format": {"format_name": "mov,mp4", "duration": "5"}} + monkeypatch.setattr(probemod, "probe_json", lambda _p: data) + owner = as_user(make_user()) + job = owner.post( + "/api/downloads/upload", + content=b"fake-video", + headers=upload_headers("clip.mp4", display_name="Holiday clip"), + ) + assert job.status_code == 200, job.text + link = owner.post(f"/api/downloads/{job.json()['id']}/links", json={}) + assert link.status_code == 200, link.text + return link.json()["token"] + + +@pytest.mark.parametrize("path", ["/", "/feed", "/downloads/42"]) +def test_an_app_path_gets_the_app(pages, client, path): + r = client.get(path) + assert r.status_code == 200 + assert "/assets/app.js" in r.text + + +@pytest.mark.parametrize("path", ["/watch/no-such-token", "/watch/no-such-token/extra"]) +def test_every_watch_path_gets_the_player_alone(pages, client, path): + r = client.get(path) + assert r.status_code == 200 + assert "/assets/watch.js" in r.text and "/assets/app.js" not in r.text + assert r.headers["cache-control"] == "no-cache" + + +def test_a_live_link_unfurls_from_the_players_page(pages, client, link_token): + r = client.get(f"/watch/{link_token}") + assert r.status_code == 200 + assert "/assets/watch.js" in r.text + assert 'content="Holiday clip"' in r.text + assert '' not in r.text # replaced, not appended diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index 39de2ca2..f7360911 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -7,15 +7,18 @@ guards that keep an upload out of paths that assume a re-fetchable source (edit DB-backed (real Postgres) — skipped when no database is reachable. `settings.download_root` is pointed at a per-test tmp dir so ingest's on-disk moves are isolated. The ingest/route tests -monkeypatch the shared `probe.probe_media` so they don't shell out to ffprobe; the classifier's OWN +monkeypatch the shared `probe.probe_json` so they don't shell out to ffprobe; the classifier's OWN parsing (image → file, cover-art skip, dims, duration) is covered directly by `TestProbeClassifier` -below, which stubs one level down (`run_ffprobe_json`) and exercises the real `probe_media`. +below, which stubs one level down (`run_ffprobe_json`) and exercises the real `probe_media`. The +phone-safe conversion of an upload runs the real ffmpeg (`TestPhoneSafeUpload`). """ import ast import asyncio import io import pathlib +import shutil +import subprocess from pathlib import Path import pytest @@ -25,9 +28,11 @@ from app.config import settings from app.routes._common import capped_stream_to from sqlalchemy import select -from app.downloads import mediakind, quota, storage +from app.downloads import gc as gcmod +from app.downloads import mediakind, pipeline, playable, quota, service, storage from app.downloads import probe as probemod from app.downloads import upload as uploadmod +from app.downloads.stated import StatedFailure from app.models import DISPLAY_TEXT_MAX, DownloadJob, DownloadQuota, MediaAsset @@ -51,9 +56,20 @@ def _root(tmp_path, monkeypatch): def _probe(monkeypatch, result): - # upload ingest now calls the shared `probe.probe_media` (same ground-truth ffprobe the download - # worker uses); patch it at that shared home. - monkeypatch.setattr(probemod, "probe_media", lambda _p: result) + """Stub the shared ffprobe call (`probe.probe_json`, the same ground truth the download worker + reads) with the JSON a file of these (vcodec, acodec, width, height, duration) would give — its + video 8-bit 4:2:0, so an H.264 + AAC .mp4 is already phone-safe and ingests straight to `done`. + All-None is a file ffprobe could not read.""" + v, a, w, h, d = result + streams = [] + if v: + streams.append( + {"codec_type": "video", "codec_name": v, "pix_fmt": "yuv420p", "width": w, "height": h} + ) + if a: + streams.append({"codec_type": "audio", "codec_name": a}) + data = {"streams": streams, "format": {"format_name": "mov,mp4", "duration": d}} + monkeypatch.setattr(probemod, "probe_json", lambda _p: data if streams else None) def test_ingest_generic_file_is_download_only(db, make_user, _root, monkeypatch): @@ -281,7 +297,7 @@ def test_upload_route_generic_forces_download( def test_upload_route_media_html_extension_not_served_as_html( - as_user, make_user, _root, monkeypatch, upload_headers + db, as_user, make_user, _root, monkeypatch, upload_headers ): """The XSS guard: a MEDIA-classified upload (real codecs) named with an executable extension must NOT be served with a guessed text/html type. The served Content-Type comes from the verified @@ -298,6 +314,14 @@ def test_upload_route_media_html_extension_not_served_as_html( assert r.status_code == 200, r.text job = r.json() assert job["asset"]["media_kind"] == "video" and job["asset"]["container"] == "html" + # Media in no phone-safe container is queued for conversion to .mp4, so today it is never served + # as .html at all. The serving guard below is the second line: a row from before that rule (or + # one whose conversion is skipped some day) must still not run as HTML. + assert job["status"] == "queued" + row = db.get(DownloadJob, job["id"]) + db.get(MediaAsset, row.asset_id).status = "ready" + row.status = "done" + db.commit() # Authenticated endpoint: never text/html. fr = c.get(f"/api/downloads/{job['id']}/file") @@ -779,10 +803,10 @@ def test_ingest_rejects_over_max_jobs(db, make_user, _root, monkeypatch): def test_ingest_cleans_up_placed_files_on_commit_failure(db, make_user, _root, monkeypatch): """A failure AFTER storage.place_file has moved the file onto disk must roll back AND unlink the - placed file + any (even partial) poster — otherwise the orphan is permanent (the per-user delete - keys off the rolled-back asset.rel_path, and the GC orphan sweep skips the dot-prefixed .uploads/ - tree). Exercises the rollback/cleanup branch that IS the leak remediation, incl. the poster case - where ensure_poster left a partial file but returned None.""" + placed file + any (even partial) poster — otherwise nothing names it (the per-user delete keys + off the rolled-back asset.rel_path) and it sits on disk until the GC's `.uploads` sweep. Exercises + the rollback/cleanup branch, incl. the poster case where ensure_poster left a partial file but + returned None.""" _probe(monkeypatch, ("h264", "aac", 320, 240, 5)) # video → the poster path runs u = make_user() @@ -813,6 +837,28 @@ def test_ingest_cleans_up_placed_files_on_commit_failure(db, make_user, _root, m assert db.query(MediaAsset).count() == 0 # the flushed asset rolled back — no dangling row +def test_ingest_keeps_the_file_when_its_commit_landed_but_raised(db, make_user, _root, monkeypatch): + # The commit reached the server and lost its answer: the row names the file now, so removing it + # as "rolled back" would leave a ready upload pointing at nothing. + _probe(monkeypatch, ("h264", "aac", 320, 240, 5)) + monkeypatch.setattr(uploadmod.editmod, "ensure_poster", lambda *_a: None) + u = make_user() + real_commit = db.commit + + def _landed_then_lost(): + real_commit() + raise RuntimeError("connection lost after COMMIT") + + monkeypatch.setattr(db, "commit", _landed_then_lost) + src = _root / "clip.mp4" + src.write_bytes(b"fake-video-bytes") + with pytest.raises(RuntimeError): + uploadmod.ingest_upload(db, u.id, filename="clip.mp4", src_path=src, size=16) + + (asset,) = db.query(MediaAsset).all() + assert storage.abs_path(str(_root), asset.rel_path).exists() + + # --- classifier: the REAL probe_media parsing (stubs ffprobe one level down) ------------------ # The ingest/route tests above stub probe_media wholesale; these exercise its OWN logic — image # detection, the cover-art skip, dimension/duration parsing — by faking only run_ffprobe_json, so a @@ -998,19 +1044,132 @@ class TestResolveMediaFacts: ) assert v is None and mediakind.derive(v, a) == mediakind.FILE - def test_info_fills_in_only_when_ffprobe_could_not_read_the_file(self, monkeypatch): - # ffprobe failed entirely (None) → fall back to info so we never regress the working path. + def test_a_file_the_guarded_probe_cannot_read_is_a_download_only_file(self, monkeypatch): + # ffprobe (media demuxers only) failed entirely: the player and the editor cannot open it + # either, so info's codecs must not make it an editable 'video'. Dimensions still gap-fill. self._ffprobe(monkeypatch, None) v, a, w, h, _d = probemod.resolve_media_facts( - Path("x"), {"vcodec": "vp9", "acodec": "opus", "width": 1920, "height": 1080} + Path("x"), + {"vcodec": "avc1.64001f", "acodec": "mp4a.40.2", "width": 1920, "height": 1080}, ) - assert (v, a, w, h) == ("vp9", "opus", 1920, 1080) + assert (v, a, w, h) == (None, None, 1920, 1080) + assert mediakind.derive(v, a) == mediakind.FILE - def test_info_none_codec_string_is_treated_as_absent(self, monkeypatch): - # yt-dlp writes the literal "none" for a missing stream — it must not become a codec name. - self._ffprobe(monkeypatch, None) # probe failed → info path, where the "none" scrub applies - v, a, w, h, _d = probemod.resolve_media_facts( - Path("x"), {"vcodec": "none", "acodec": "aac"} + +# --- the phone-safe conversion (an iPhone's HEVC, a webm, an mkv) ------------------------------ + + +def _queued_upload(db, make_user, root, monkeypatch, name="clip.mov"): + """An upload no phone could play as it is: probed as HEVC, so ingest queues it.""" + _probe(monkeypatch, ("hevc", "aac", 640, 360, 6)) + src = root / "src.bin" + src.write_bytes(b"hevc bytes") + job = uploadmod.ingest_upload(db, make_user().id, filename=name, src_path=src, size=10) + return job, db.get(MediaAsset, job.asset_id) + + +class TestPhoneSafeUpload: + def test_an_unsafe_video_is_queued_for_the_worker_not_done( + self, db, make_user, _root, monkeypatch + ): + job, asset = _queued_upload(db, make_user, _root, monkeypatch) + assert (job.status, job.progress, asset.status) == ("queued", 0, "pending") + assert asset.rel_path.endswith(".mov") # the original waits at rel_path for the worker + assert storage.abs_path(str(_root), asset.rel_path).exists() + + def test_a_safe_video_still_lands_done(self, db, make_user, _root, monkeypatch): + _probe(monkeypatch, ("h264", "aac", 640, 360, 6)) + src = _root / "src.bin" + src.write_bytes(b"h264 bytes") + job = uploadmod.ingest_upload( + db, make_user().id, filename="clip.mp4", src_path=src, size=10 ) - assert (v, a) == (None, "aac") - assert mediakind.derive(v, a) == mediakind.AUDIO + assert job.status == "done" and db.get(MediaAsset, job.asset_id).status == "ready" + + def test_deleting_a_queued_upload_removes_its_original(self, db, make_user, _root, monkeypatch): + job, asset = _queued_upload(db, make_user, _root, monkeypatch) + path, asset_id = storage.abs_path(str(_root), asset.rel_path), asset.id + service.delete(db, job) + assert not path.exists() + assert db.get(MediaAsset, asset_id) is None + + def test_one_cancelled_mid_conversion_is_reaped_once_the_worker_let_go( + self, db, make_user, _root, monkeypatch + ): + job, asset = _queued_upload(db, make_user, _root, monkeypatch) + path, asset_id = storage.abs_path(str(_root), asset.rel_path), asset.id + asset.status = "downloading" # the worker child is reading the original + job.status = "running" + db.commit() + service.cancel(db, job) + assert path.exists() # never pulled out from under a running ffmpeg + db.get(MediaAsset, asset_id).status = "pending" # the worker stopped the child and let go + db.commit() + gcmod.run_download_gc(db) + db.expire_all() + assert not path.exists() + assert db.get(MediaAsset, asset_id) is None + + def test_a_failed_conversion_keeps_the_original_for_retry( + self, db, make_user, _root, monkeypatch + ): + job, asset = _queued_upload(db, make_user, _root, monkeypatch) + path = storage.abs_path(str(_root), asset.rel_path) + job.status, asset.status = "running", "downloading" + db.commit() + + def _ffmpeg_fails(*_a, **_k): + raise StatedFailure("ffmpeg failed (1): unsupported") + + monkeypatch.setattr(pipeline.editmod, "run_ffmpeg", _ffmpeg_fails) + pipeline._convert_upload(job.id, asset.id) + db.expire_all() + assert (db.get(DownloadJob, job.id).status, db.get(MediaAsset, asset.id).status) == ( + "error", + "error", + ) + assert path.exists() + service.resume(db, db.get(DownloadJob, job.id)) + assert db.get(MediaAsset, asset.id).status == "pending" # Retry converts from it again + + @pytest.mark.skipif(shutil.which("ffmpeg") is None, reason="ffmpeg not available") + def test_the_worker_converts_it_and_keeps_only_the_safe_file( + self, db, make_user, _root, monkeypatch + ): + # Real bytes, real probe, real ffmpeg: a 10-bit H.264 + Opus Matroska, the kind of file + # that reached the share page as-is before this rule. + src = _root / "src.mkv" + subprocess.run( + [ + "ffmpeg", "-y", "-loglevel", "error", + "-f", "lavfi", "-i", "testsrc2=size=160x120:rate=10:duration=1", + "-f", "lavfi", "-i", "sine=duration=1", + "-c:v", "libx264", "-pix_fmt", "yuv420p10le", "-c:a", "libopus", str(src), + ], + check=True, + ) # fmt: skip + job = uploadmod.ingest_upload( + db, make_user().id, filename="holiday.mkv", src_path=src, size=src.stat().st_size + ) + asset = db.get(MediaAsset, job.asset_id) + original = storage.abs_path(str(_root), asset.rel_path) + assert job.status == "queued" + job.status, asset.status = "running", "downloading" # what the worker's claim does + db.commit() + + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + asset = db.get(MediaAsset, job.asset_id) + assert db.get(DownloadJob, job.id).status == "done" + assert (asset.status, asset.container, asset.vcodec, asset.acodec) == ( + "ready", + "mp4", + "h264", + "aac", + ) + final = storage.abs_path(str(_root), asset.rel_path) + assert final.suffix == ".mp4" and final.exists() + assert asset.size_bytes == final.stat().st_size + assert not original.exists() # one file per upload + assert not playable.plan(probemod.probe_json(final), final.suffix).needs_change diff --git a/backend/tests/test_upload_conversion.py b/backend/tests/test_upload_conversion.py new file mode 100644 index 00000000..2a33bea5 --- /dev/null +++ b/backend/tests/test_upload_conversion.py @@ -0,0 +1,592 @@ +"""An upload after ingest: what it costs its owner from the moment it lands, how the worker's +conversion names, checks and cleans up the file it makes, and what the owner and a share link see. + +DB-backed. The conversion's ffmpeg step (`pipeline._ensure_phone_safe`) is stubbed to write a file of +a chosen size — these tests are about what happens around the encode; `test_upload.py` runs the real +one, and `test_playable.py` judges its bytes. +""" + +import os +import time +from datetime import datetime, timedelta, timezone +from pathlib import Path + +import pytest + +from app import sysconfig +from app.config import settings +from app.downloads import gc as gcmod +from app.downloads import pipeline, quota, storage +from app.downloads import probe as probemod +from app.downloads import upload as uploadmod +from app.downloads.stated import StatedFailure +from app.models import DownloadJob, DownloadQuota, MediaAsset, Notification +from sqlalchemy import select, update +from sqlalchemy.exc import OperationalError +from sqlalchemy.orm import Session + + +@pytest.fixture +def _root(tmp_path, monkeypatch): + monkeypatch.setattr(settings, "download_root", str(tmp_path)) + return tmp_path + + +def _probe(monkeypatch, vcodec="hevc"): + """ffprobe's view of every file here: `vcodec` + AAC, 8-bit 4:2:0 — HEVC is queued for the + worker, H.264 lands as it came.""" + data = { + "streams": [ + {"codec_type": "video", "codec_name": vcodec, "pix_fmt": "yuv420p", + "width": 640, "height": 360}, + {"codec_type": "audio", "codec_name": "aac"}, + ], + "format": {"format_name": "mov,mp4", "duration": "6"}, + } # fmt: skip + monkeypatch.setattr(probemod, "probe_json", lambda _p: data) + + +def _ingest(db, root, user_id, name="clip.mov", size=10): + src = root / "incoming.bin" + src.write_bytes(b"o" * size) + job = uploadmod.ingest_upload(db, user_id, filename=name, src_path=src, size=size) + return job, db.get(MediaAsset, job.asset_id) + + +def _claim(db, job, asset): + """What the worker's claim does before it converts.""" + job.status, asset.status = "running", "downloading" + db.commit() + + +def _converts_to(monkeypatch, size, phases=None): + """The encode writes a `size`-byte .mp4 into its staging dir (and notes the phase it reports).""" + + def _encode(job_id, produced, out_dir, phase, total_s=None, **_k): + if phases is not None: + phases.append(phase) + out = Path(out_dir) / "converted.mp4" + out.write_bytes(b"c" * size) + return out + + monkeypatch.setattr(pipeline, "_ensure_phone_safe", _encode) + + +def _cap(db, user_id, max_bytes): + db.add( + DownloadQuota( + user_id=user_id, max_bytes=max_bytes, max_concurrent=2, max_jobs=50, unlimited=False + ) + ) + db.commit() + + +def _uploads_dir(root, user_id) -> Path: + return root / ".uploads" / str(user_id) + + +# --- what it costs --------------------------------------------------------------------------- + + +def test_a_queued_upload_counts_against_its_owner_from_the_moment_it_lands( + db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id, size=10) + assert (job.status, asset.status) == ("queued", "pending") + assert quota.footprint(db, u.id) == 10 # its original is on disk already + + +def test_a_failed_conversion_still_counts_its_kept_original(db, make_user, _root, monkeypatch): + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id, size=10) + job.status, asset.status = "error", "error" # kept for Retry + db.commit() + assert storage.abs_path(str(_root), asset.rel_path).exists() + assert quota.footprint(db, u.id) == 10 + + +def test_a_conversion_that_outgrows_the_cap_is_refused_and_keeps_the_original( + db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + u = make_user() + _cap(db, u.id, max_bytes=100) + job, asset = _ingest(db, _root, u.id, size=10) + original = storage.abs_path(str(_root), asset.rel_path) + _claim(db, job, asset) + _converts_to(monkeypatch, size=500) + + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + job = db.get(DownloadJob, job.id) + assert (job.status, job.error_code) == ("error", "quota_exceeded") + assert original.exists() + assert [p.name for p in _uploads_dir(_root, u.id).iterdir()] == [original.name] + + +def test_a_conversion_that_shrinks_passes_an_owner_at_the_cap(db, make_user, _root, monkeypatch): + _probe(monkeypatch) + u = make_user() + _cap(db, u.id, max_bytes=10) + job, asset = _ingest(db, _root, u.id, size=10) + _claim(db, job, asset) + _converts_to(monkeypatch, size=5) + + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + assert db.get(DownloadJob, job.id).status == "done" + assert db.get(MediaAsset, asset.id).size_bytes == 5 + + +# --- the converted file's name and its leftovers ----------------------------------------------- + + +def test_two_dotted_bare_names_keep_a_file_each(db, make_user, _root, monkeypatch): + # 'Holiday.2024.ä': its suffix is no extension (nothing shell-safe is left of it), so the stem's + # own dot once read as one — and the converted name lost `_a{id}`, the one thing keeping two apart. + _probe(monkeypatch) + u = make_user() + _converts_to(monkeypatch, size=5) + finals = [] + for _ in range(2): + job, asset = _ingest(db, _root, u.id, name="Holiday.2024.ä") + _claim(db, job, asset) + pipeline._convert_upload(job.id, asset.id) + db.expire_all() + asset = db.get(MediaAsset, asset.id) + assert asset.status == "ready" + assert asset.rel_path.endswith(f"_a{asset.id}.mp4"), asset.rel_path + finals.append(asset.rel_path) + assert finals[0] != finals[1] + assert all(storage.abs_path(str(_root), r).exists() for r in finals) + + +def test_a_bare_names_stem_holds_no_dot_so_a_swapped_suffix_keeps_its_id(): + # Each of the two name rules is held on its own: a new name's stem has no dot, so a sibling + # derived by `with_suffix` (the poster, the converted file) keeps `_a{id}`. + rel = storage.upload_rel_path(1, 5, "Holiday.2024.ä") + assert rel == ".uploads/1/Holiday_2024_a5" + assert Path(rel).with_suffix(".jpg").name == "Holiday_2024_a5.jpg" + + +def test_an_upload_placed_with_a_dotted_bare_name_keeps_its_id_when_converted( + db, make_user, _root, monkeypatch +): + # A row from before the stems lost their dots: its stem's tail is no extension. + _probe(monkeypatch) + u = make_user() + _converts_to(monkeypatch, size=5) + job, asset = _ingest(db, _root, u.id, name="Holiday.2024.ä") + legacy = f".uploads/{u.id}/Holiday.2024_a{asset.id}" + storage.abs_path(str(_root), asset.rel_path).rename(storage.abs_path(str(_root), legacy)) + asset.rel_path, asset.container = legacy, None + _claim(db, job, asset) + + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + assert db.get(MediaAsset, asset.id).rel_path == f"{legacy}.mp4" + + +def test_a_conversion_failing_after_it_placed_its_file_removes_it( + db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id) + original = storage.abs_path(str(_root), asset.rel_path) + _claim(db, job, asset) + _converts_to(monkeypatch, size=5) + + def _probe_fails(_path): + raise RuntimeError("probe crashed") + + monkeypatch.setattr(pipeline.probe, "probe_media", _probe_fails) + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + assert db.get(DownloadJob, job.id).status == "error" + assert original.exists() # Retry starts from it + # The row still names the original: a converted file left beside it is one nothing would find. + assert [p.name for p in _uploads_dir(_root, u.id).iterdir()] == [original.name] + + +def _finishing_commit_raises(monkeypatch, *, lands: bool) -> None: + """The commit that makes the converted upload ready raises — after it reached the server + (`lands`: the connection dropped before its answer) or before (it rolled back).""" + real_finalize, real_commit = pipeline.service.finalize_if_running, Session.commit + state = {"session": None, "fired": False} + + def _finalize(db, *a, **k): + if not state["fired"] and state["session"] is None: + state["session"] = db + return real_finalize(db, *a, **k) + + def _commit(self): + if self is state["session"] and not state["fired"]: + state["fired"] = True + if lands: + real_commit(self) + raise OperationalError("COMMIT", {}, Exception("connection lost")) + return real_commit(self) + + monkeypatch.setattr(pipeline.service, "finalize_if_running", _finalize) + monkeypatch.setattr(Session, "commit", _commit) + + +def test_a_finishing_commit_that_landed_but_raised_keeps_the_converted_file( + db, make_user, _root, monkeypatch +): + # The row names the converted file now: removing it as "never committed" would leave the asset + # pointing at nothing and its original named by no row, for the sweep to take — both copies gone. + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id) + _claim(db, job, asset) + _converts_to(monkeypatch, size=5) + _finishing_commit_raises(monkeypatch, lands=True) + + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + asset = db.get(MediaAsset, asset.id) + assert (db.get(DownloadJob, job.id).status, asset.status) == ("done", "ready") + assert storage.abs_path(str(_root), asset.rel_path).exists() + + +def test_a_finishing_commit_that_rolled_back_fails_and_keeps_the_original( + db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id) + original = storage.abs_path(str(_root), asset.rel_path) + _claim(db, job, asset) + _converts_to(monkeypatch, size=5) + _finishing_commit_raises(monkeypatch, lands=False) + + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + assert db.get(DownloadJob, job.id).status == "error" + assert [p.name for p in _uploads_dir(_root, u.id).iterdir()] == [original.name] + + +def test_an_original_that_cannot_be_removed_leaves_the_upload_ready( + db, make_user, _root, monkeypatch +): + # The converted file is committed and the job done: the original's unlink failing must not flip + # a working asset to an error no Retry is offered for. The sweep takes the original later. + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id) + original = storage.abs_path(str(_root), asset.rel_path) + _claim(db, job, asset) + _converts_to(monkeypatch, size=5) + real_unlink = Path.unlink + + def _unlink(self, missing_ok=False): + if self == original: + raise PermissionError("read-only") + return real_unlink(self, missing_ok=missing_ok) + + monkeypatch.setattr(Path, "unlink", _unlink) + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + asset = db.get(MediaAsset, asset.id) + assert (db.get(DownloadJob, job.id).status, asset.status) == ("done", "ready") + assert original.exists() + + +# --- what the owner is told ------------------------------------------------------------------ + + +def test_a_failed_encode_is_a_failed_conversion_not_a_download_step( + db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + job, asset = _ingest(db, _root, make_user().id) + _claim(db, job, asset) + + def _ffmpeg_fails(*_a, **_k): + raise StatedFailure("ffmpeg failed (1): unsupported") + + monkeypatch.setattr(pipeline.editmod, "run_ffmpeg", _ffmpeg_fails) + pipeline._convert_upload(job.id, asset.id) + db.expire_all() + assert db.get(DownloadJob, job.id).error_code == "convert_failed" + + +def test_a_missing_original_says_so(db, make_user, _root, monkeypatch): + _probe(monkeypatch) + job, asset = _ingest(db, _root, make_user().id) + _claim(db, job, asset) + storage.abs_path(str(_root), asset.rel_path).unlink() + + pipeline._convert_upload(job.id, asset.id) + db.expire_all() + assert db.get(DownloadJob, job.id).error_code == "upload_missing" + + +def test_the_conversion_reports_its_own_phase(db, make_user, _root, monkeypatch): + _probe(monkeypatch) + job, asset = _ingest(db, _root, make_user().id) + _claim(db, job, asset) + phases = [] + _converts_to(monkeypatch, size=5, phases=phases) + pipeline._convert_upload(job.id, asset.id) + assert phases == ["converting"] # not yt-dlp's "processing": the queue row shows ffmpeg's % + + +def test_the_owner_is_told_their_upload_is_ready(db, make_user, _root, monkeypatch): + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id) + _claim(db, job, asset) + _converts_to(monkeypatch, size=5) + pipeline._convert_upload(job.id, asset.id) + notes = db.execute(select(Notification).where(Notification.user_id == u.id)).scalars().all() + assert [n.data.get("kind") for n in notes] == ["upload"] + + +def test_an_extension_is_judged_as_it_is_stored(db, make_user, _root, monkeypatch): + # 'clip.mp4~' is stored as .mp4; judged by the raw 'mp4~' it queued a conversion of a file that + # was phone-safe already. + _probe(monkeypatch, vcodec="h264") + job, asset = _ingest(db, _root, make_user().id, name="clip.mp4~") + assert (job.status, asset.status, asset.container) == ("done", "ready", "mp4") + + +# --- the garbage collector --------------------------------------------------------------------- + + +def _aged(path: Path, hours: float = 5) -> None: + old = time.time() - hours * 3600 + os.utime(path, (old, old)) + + +def test_the_sweep_takes_a_stranded_upload_file_and_keeps_a_waiting_original( + db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + u = make_user() + _job, asset = _ingest(db, _root, u.id) + original = storage.abs_path(str(_root), asset.rel_path) + stranded = original.with_name("left_by_a_stopped_conversion_a999.mp4") + stranded.write_bytes(b"x") + _aged(original) + _aged(stranded) + + gcmod._sweep_orphan_files(db, str(_root)) + + assert original.exists() # queued for conversion: its row names it, ready or not + assert not stranded.exists() # no row names it + + +@pytest.mark.parametrize( + ("job_status", "asset_status"), + [ + ("queued", "pending"), # waiting for the worker + ("running", "downloading"), # being converted, for longer than the sweep's age floor + ("error", "error"), # failed, kept for Retry + ("done", "ready"), # converted + ], +) +def test_the_sweep_keeps_an_aged_upload_file_its_row_names_in_any_state( + job_status, asset_status, db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + job, asset = _ingest(db, _root, make_user().id) + path = storage.abs_path(str(_root), asset.rel_path) + job.status, asset.status = job_status, asset_status + db.commit() + _aged(path) + + gcmod._sweep_orphan_files(db, str(_root)) + + assert path.exists() + + +def _row(db, asset_id: int) -> MediaAsset | None: + """The asset as the database has it now (a `get` would refresh the session's own copy, and + raise for one deleted under it).""" + db.expire_all() + return db.execute(select(MediaAsset).where(MediaAsset.id == asset_id)).scalar_one_or_none() + + +def _route_moved(db, job, asset, job_status: str, ref_count: int) -> None: + """What the cancel/pause/delete route leaves behind, written as SQL: the session's copy of + `ref_count` is stale (the routes count it in SQL), so an ORM assignment of the same value would + write nothing.""" + db.execute(update(DownloadJob).where(DownloadJob.id == job.id).values(status=job_status)) + db.execute(update(MediaAsset).where(MediaAsset.id == asset.id).values(ref_count=ref_count)) + db.commit() + + +def _stopped_by(monkeypatch, db, job, asset, job_status: str, ref_count: int) -> None: + """The conversion is stopped mid-encode after the route moved the job to `job_status` and its + hold on the asset to `ref_count` (cancel and delete release it; pause keeps it).""" + + def _encode(*_a, **_k): + _route_moved(db, job, asset, job_status, ref_count) + raise pipeline.editmod.EditAborted() + + monkeypatch.setattr(pipeline, "_ensure_phone_safe", _encode) + + +def test_an_upload_cancelled_mid_conversion_is_freed_once_the_conversion_stops( + db, make_user, _root, monkeypatch +): + # `release_asset` leaves it while the conversion reads the file; once it stopped, nothing holds it + # — kept, it would sit on disk outside every footprint until the GC's upload pass. + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id, size=10) + original = storage.abs_path(str(_root), asset.rel_path) + _claim(db, job, asset) + _stopped_by(monkeypatch, db, job, asset, "canceled", 0) + + pipeline._convert_upload(job.id, asset.id) + + assert _row(db, asset.id) is None + assert not original.exists() + assert quota.footprint(db, u.id) == 0 + + +def test_an_upload_paused_mid_conversion_keeps_its_original_for_the_resume( + db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id, size=10) + original = storage.abs_path(str(_root), asset.rel_path) + _claim(db, job, asset) + _stopped_by(monkeypatch, db, job, asset, "paused", 1) + + pipeline._convert_upload(job.id, asset.id) + + db.expire_all() + assert db.get(MediaAsset, asset.id).status == "pending" + assert original.exists() + + +def test_an_upload_whose_conversion_finished_after_its_job_was_cancelled_is_freed( + db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + u = make_user() + job, asset = _ingest(db, _root, u.id, size=10) + _claim(db, job, asset) + _converts_to(monkeypatch, size=5) + real_encode = pipeline._ensure_phone_safe + + def _encode_then_cancelled(*a, **k): + out = real_encode(*a, **k) + _route_moved(db, job, asset, "canceled", 0) + return out + + monkeypatch.setattr(pipeline, "_ensure_phone_safe", _encode_then_cancelled) + pipeline._convert_upload(job.id, asset.id) + + assert _row(db, asset.id) is None + left = _uploads_dir(_root, u.id) + assert not left.exists() or list(left.iterdir()) == [] + + +def test_the_cache_cap_never_evicts_an_upload(db, make_user, _root, monkeypatch): + _probe(monkeypatch, vcodec="h264") + u = make_user() + _job, upload = _ingest(db, _root, u.id, name="mine.mp4", size=10) + db.add( + MediaAsset( + source_kind="youtube", source_ref="vid1", format_sig="best", status="ready", + rel_path="Chan/clip.mp4", size_bytes=10, title="downloaded", + last_access_at=datetime.now(timezone.utc), + ) + ) # fmt: skip + # The upload is the least recently used: an LRU pass that may take it takes it first. + upload.last_access_at = datetime.now(timezone.utc) - timedelta(days=30) + db.commit() + (_root / "Chan").mkdir() + (_root / "Chan" / "clip.mp4").write_bytes(b"d" * 10) + sysconfig.set_value(db, "download_total_max_bytes", 15) # 20 on disk + db.commit() + + gcmod.run_download_gc(db) + + db.expire_all() + assert db.get(MediaAsset, upload.id).status == "ready" + assert storage.abs_path(str(_root), upload.rel_path).exists() + + +# --- a share link ---------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("job_status", "asset_status", "reason"), + [ + ("queued", "pending", "video_preparing"), # a re-download waiting for the worker + ("running", "downloading", "video_preparing"), # being made now + ("canceled", "pending", "video_gone"), # a rebuild cancelled: nothing will finish it + ("paused", "pending", "video_gone"), # held until its owner resumes it, maybe never + ], +) +def test_a_link_says_preparing_only_while_its_video_is_being_made( + job_status, asset_status, reason, db, as_user, make_user, _root, monkeypatch, upload_headers +): + # A link is made on a finished job; its asset can be (re)made under it later — a shared cache row + # re-fetched, a conversion retried. While that runs the page says "try again shortly"; once + # nothing is making it, it says the video is gone rather than promising it forever. + _probe(monkeypatch, vcodec="h264") + c = as_user(make_user()) + job = c.post( + "/api/downloads/upload", content=b"h264 bytes", headers=upload_headers("clip.mp4") + ).json() + link = c.post(f"/api/downloads/{job['id']}/links", json={"allow_download": False}).json() + assert c.get(f"/api/public/watch/{link['token']}").status_code == 200 + db.get(MediaAsset, job["asset"]["id"]).status = asset_status + db.get(DownloadJob, job["id"]).status = job_status + db.commit() + r = c.get(f"/api/public/watch/{link['token']}") + assert r.status_code == 404 + assert r.json()["detail"]["reason"] == reason + + +def test_the_admin_storage_view_counts_a_waiting_upload(db, as_user, make_user, _root, monkeypatch): + # The same disk the owner's footprint and the GC's cap count (`quota.ON_DISK`). + _probe(monkeypatch) + u = make_user() + _ingest(db, _root, u.id, size=10) + r = as_user(make_user(role="admin")).get("/api/admin/downloads/storage").json() + assert (r["files_on_disk"], r["total_bytes"]) == (1, 10) + assert {p["user_id"]: p["footprint_bytes"] for p in r["per_user"]} == {u.id: 10} + + +@pytest.mark.parametrize( + ("job_status", "asset_status", "ref_count"), + [ + ("done", "ready", 1), # a finished upload its owner keeps + ("queued", "pending", 1), # one waiting for the worker + ("canceled", "downloading", 0), # cancelled mid-conversion: the worker still reads it + ], +) +def test_the_upload_reap_leaves_one_still_held_or_still_read( + job_status, asset_status, ref_count, db, make_user, _root, monkeypatch +): + _probe(monkeypatch) + job, asset = _ingest(db, _root, make_user().id) + path = storage.abs_path(str(_root), asset.rel_path) + asset.status = asset_status + db.commit() + _route_moved(db, job, asset, job_status, ref_count) + + gcmod.run_download_gc(db) + + assert _row(db, asset.id) is not None + assert path.exists() diff --git a/backend/tests/test_worker_dispatch.py b/backend/tests/test_worker_dispatch.py new file mode 100644 index 00000000..8dc74b96 --- /dev/null +++ b/backend/tests/test_worker_dispatch.py @@ -0,0 +1,74 @@ +"""The worker child's dispatch (`runner._run`) and the edit path's last step before it stores a clip. + +`runner._run` picks the pipeline entry by the job's kind; the tests elsewhere call those entries +directly, so a kind routed to the wrong one (an upload sent to yt-dlp) would pass all of them. +""" + +from pathlib import Path + +import pytest + +from app.config import settings +from app.downloads import pipeline, runner, service, storage +from app.downloads import probe as probemod +from app.downloads import upload as uploadmod +from app.models import DownloadJob, MediaAsset + + +@pytest.mark.parametrize( + ("kind", "entry"), + [("upload", "_convert_upload"), ("edit", "_process_edit"), ("download", "_download")], +) +def test_each_job_kind_reaches_its_own_pipeline_entry(kind, entry, monkeypatch): + called = [] + for name in ("_convert_upload", "_process_edit", "_download"): + monkeypatch.setattr(pipeline, name, lambda *a, _n=name: called.append(_n)) + runner._run(5, 7, kind, "url", "https://example.org/v", {}, 1) + assert called == [entry] + + +def test_a_clip_the_phone_safe_step_changed_is_stored_under_its_new_extension( + db, make_user, tmp_path, monkeypatch +): + # A fast trim stream-copies its source's container; when the phone-safe step remuxes it (an + # audio clip out of an .mp4 becomes an .m4a), the stored path and container follow the new file. + monkeypatch.setattr(settings, "download_root", str(tmp_path)) + data = { + "streams": [{"codec_type": "audio", "codec_name": "aac"}], + "format": {"format_name": "mov,mp4", "duration": "10"}, + } + monkeypatch.setattr(probemod, "probe_json", lambda _p: data) + src = tmp_path / "in.bin" + src.write_bytes(b"a" * 10) + u = make_user() + source = uploadmod.ingest_upload(db, u.id, filename="song.m4a", src_path=src, size=10) + # Stored as an .mp4, the way an audio file was kept before every ready file met the rule. + kept = db.get(MediaAsset, source.asset_id) + old = storage.abs_path(str(tmp_path), kept.rel_path) + kept.rel_path, kept.container = kept.rel_path[: -len(".m4a")] + ".mp4", "mp4" + old.rename(storage.abs_path(str(tmp_path), kept.rel_path)) + db.commit() + job = service.enqueue_edit( + db, u.id, source, {"trim": {"start_s": 1, "end_s": 3}, "accurate": False} + ) + asset = db.get(MediaAsset, job.asset_id) + job.status, asset.status = "running", "downloading" + db.commit() + + def _trim(cmd, *_a, **_k): + Path(cmd[-1]).write_bytes(b"t" * 4) # the stream-copied clip, at the path ffmpeg was given + + def _remux(job_id, produced, out_dir, phase, total_s=None, **_k): + out = Path(out_dir) / "remuxed.m4a" + out.write_bytes(b"r" * 3) + return out + + monkeypatch.setattr(pipeline.editmod, "run_ffmpeg", _trim) + monkeypatch.setattr(pipeline, "_ensure_phone_safe", _remux) + pipeline._process_edit(job.id, asset.id) + + db.expire_all() + asset = db.get(MediaAsset, asset.id) + assert (db.get(DownloadJob, job.id).status, asset.container) == ("done", "m4a") + assert asset.rel_path.endswith(".m4a") + assert storage.abs_path(str(tmp_path), asset.rel_path).read_bytes() == b"rrr" diff --git a/docs/gates.md b/docs/gates.md index fc88f61e..75537e98 100644 --- a/docs/gates.md +++ b/docs/gates.md @@ -112,6 +112,7 @@ project checks needs a row. | coverage | not in the gate; no threshold | Owner decision E32 D4: never enforced; "every regression test seen red" is the bar, and a percentage invites tests written to move it. Out of the lanes since E35: read once, by hand, it found 5 bugs; as a lane it was a number nobody acted on, and the slowest vitest run (the one that flaked). On demand: docs/development.md. | frontend 80% / backend 78% of lines (E32 S1) | | CVE gate | HIGH/CRITICAL with a fix available only | `--ignore-unfixed` + `--severity HIGH,CRITICAL`: a finding with no fix cannot be acted on by an upgrade; it shows in the scan output. A fixable one fails the publish, and the only way past it is `.trivyignore.yaml`. | 0 open (E32 S1) | | npm audit | `--omit=dev --audit-level=high` | What ships to the browser, at high or above. The dev toolchain's advisories were burned down by hand in E32 S1 (18 → 0). | 0 | +| knip | `entry` — one file added | Not a narrowing: knip's Vite plugin reads entries from index.html only, so the shared link's page (`watch.html` → `src/watchMain.tsx`) is named here, or the lane reports that whole entry unused. It adds files to the lane, never takes any out. | 1 entry | | React Compiler | the functions it skips, pinned | `components/compilerCoverage.contract.test.ts` — the list may only shrink. End state: only VirtualGrid (TanStack Virtual, an IncompatibleLibrary) remains. | 93 (E32 S1) → 29 (E32 S3) → 1, VirtualGrid (E32 S4) | ## Accepted findings diff --git a/frontend/e2e/player.spec.ts b/frontend/e2e/player.spec.ts index 0fdc3bfd..4e4fcfdc 100644 --- a/frontend/e2e/player.spec.ts +++ b/frontend/e2e/player.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from "@playwright/test"; +import { test, expect, type Page } from "@playwright/test"; import { openApp, openFirstVideo } from "./helpers"; // Locks the in-app player's open/close contract. The player is a portaled overlay whose @@ -26,3 +26,53 @@ test.describe("player modal", () => { await expect(page.getByTestId("player-modal")).toHaveCount(0); }); }); + +// The video's size, in the windows the owner meets: layout bugs no DOM test can see. Each viewport +// is a real one — 1920×911 is a 1080p screen at 100% (the side-panel layout), 1097×520 the same +// screen at 175% or a small laptop at 100% (the stacked layout, on a short window), 1400×380 a wide +// window made short by docked devtools (the side-panel layout, whose card does not scroll). +test.describe("player modal sizing", () => { + test.beforeEach(async ({ page }) => { + await page.route(/youtube\.com/, (route) => route.abort()); + }); + + const box = async (page: Page, selector: string) => + (await page.locator(selector).first().boundingBox())!; + + for (const [w, h] of [ + [1920, 911], + [1097, 520], + [1400, 380], + ] as const) { + test(`the video keeps a real size at ${w}×${h}, and maximised fills the window`, async ({ + page, + }) => { + await page.setViewportSize({ width: w, height: h }); + await openApp(page); + await openFirstVideo(page); + + // A short window narrows the card so the chrome under the video fits — but the chrome grows + // as the card narrows (its text wraps), and that loop once ran the card down to 2 px. + const stage = await box(page, ".player-stage"); + expect(stage.width).toBeGreaterThanOrEqual(Math.min(w * 0.5, 560)); + expect(stage.height).toBeGreaterThan(stage.width * 0.5); + // …and all of it is in sight: its bottom edge is the YouTube control bar, and the side-panel + // card clips what overflows it (it does not scroll), the stacked card scrolls to it. + const overflow = await page + .locator(".player-stage") + .first() + .evaluate((el) => { + const card = el.parentElement!; + return el.getBoundingClientRect().bottom - card.getBoundingClientRect().bottom; + }); + expect(overflow).toBeLessThanOrEqual(0.5); + + // Maximise (F): the stage is fixed to the viewport. In the side-panel layout its `self-start` + // once aligned the fixed box too, and it shrank to its content's height — 0 px, a thin line. + await page.getByTestId("player-modal").press("f"); + await expect(page.locator(".player-stage--max")).toHaveCount(1); + const max = await box(page, ".player-stage--max"); + expect([Math.round(max.width), Math.round(max.height)]).toEqual([w, h]); + }); + } +}); diff --git a/frontend/knip.jsonc b/frontend/knip.jsonc index e5bb1921..11a391cb 100644 --- a/frontend/knip.jsonc +++ b/frontend/knip.jsonc @@ -1,5 +1,7 @@ { "$schema": "https://unpkg.com/knip@6/schema.json", + // The shared link's page (watch.html): the Vite plugin reads only index.html for entries. + "entry": ["src/watchMain.tsx"], "project": ["**/*.{ts,tsx,js}"], "playwright": { "config": ["playwright.config.ts"], diff --git a/frontend/src/components/AdminUsers.dom.test.tsx b/frontend/src/components/AdminUsers.dom.test.tsx index ef024bcf..7dc77a69 100644 --- a/frontend/src/components/AdminUsers.dom.test.tsx +++ b/frontend/src/components/AdminUsers.dom.test.tsx @@ -91,7 +91,7 @@ describe("the last sign-in column", () => { , ); - expect(await screen.findByText(relativeTime(at, "en"))).toBeTruthy(); + expect(await screen.findByText(relativeTime(at, "en", i18n.t))).toBeTruthy(); expect(screen.getAllByText("—")).toHaveLength(1); }); }); diff --git a/frontend/src/components/AdminUsers.tsx b/frontend/src/components/AdminUsers.tsx index da8953c7..5f8d3689 100644 --- a/frontend/src/components/AdminUsers.tsx +++ b/frontend/src/components/AdminUsers.tsx @@ -316,7 +316,7 @@ function UsersRoles({ me }: { me: Me }) { } > - {u.last_login_at ? relativeTime(u.last_login_at, lang) : "—"} + {u.last_login_at ? relativeTime(u.last_login_at, lang, t) : "—"} ({ jobs: [] as unknown[], - confirm: vi.fn(async (_opts: { title: string; confirmLabel?: string }) => false), + confirm: vi.fn( + async (_opts: { title: string; message?: string; confirmLabel?: string }) => false, + ), })); vi.mock("react-i18next", async (importOriginal) => { @@ -124,4 +126,58 @@ describe("DownloadCenter confirm dialogs", () => { ["downloads.confirm.deleteTitle", "downloads.actions.delete"], ]); }); + + it("cancelling an upload says the uploaded file goes with it", async () => { + // Nothing to fetch again: the cancel deletes the original from the server. + mocks.jobs = [{ ...job(3, "queued"), job_kind: "upload" }]; + render( + + + , + ); + await pick("Clip 3", "downloads.actions.cancel"); + await waitFor(() => expect(mocks.confirm).toHaveBeenCalledTimes(1)); + const [opts] = mocks.confirm.mock.calls[0]!; + expect([opts.title, opts.confirmLabel]).toEqual([ + "downloads.confirm.cancelUploadTitle", + "downloads.confirm.cancelUploadAction", + ]); + }); + + it("cancelling a clip speaks of the clip", async () => { + mocks.jobs = [{ ...job(4, "running"), job_kind: "edit" }]; + render( + + + , + ); + await pick("Clip 4", "downloads.actions.cancel"); + await waitFor(() => expect(mocks.confirm).toHaveBeenCalledTimes(1)); + const [opts] = mocks.confirm.mock.calls[0]!; + expect([opts.title, opts.message, opts.confirmLabel]).toEqual([ + "downloads.confirm.cancelEditTitle", + "downloads.confirm.cancelEditBody", + "downloads.confirm.cancelEditAction", + ]); + }); + + it.each([ + ["upload", "downloads.confirm.deleteUploadTitle", "downloads.confirm.deleteUploadBody"], + ["edit", "downloads.confirm.deleteEditTitle", "downloads.confirm.deleteEditBody"], + ["download", "downloads.confirm.deleteTitle", "downloads.confirm.deleteBody"], + ] as const)("deleting a finished %s says what happens to its file", async (kind, title, body) => { + // Only a download's file may stay cached for other users; an upload's or a clip's is theirs alone. + mocks.jobs = [{ ...job(5, "done"), job_kind: kind }]; + render( + + + , + ); + fireEvent.click(screen.getByRole("tab", { name: /downloads\.tabs\.done/ })); + await screen.findByText("Clip 5"); + await pick("Clip 5", "downloads.actions.delete"); + await waitFor(() => expect(mocks.confirm).toHaveBeenCalledTimes(1)); + const [opts] = mocks.confirm.mock.calls[0]!; + expect([opts.title, opts.message]).toEqual([title, body]); + }); }); diff --git a/frontend/src/components/ErrorBoundary.tsx b/frontend/src/components/ErrorBoundary.tsx index ce94e769..ede7e369 100644 --- a/frontend/src/components/ErrorBoundary.tsx +++ b/frontend/src/components/ErrorBoundary.tsx @@ -1,61 +1,30 @@ -import { Component, type ErrorInfo, type ReactNode } from "react"; +import type { ReactNode } from "react"; import i18n from "../i18n"; import { notify } from "../lib/notifications"; -import { errorAttrs, log } from "../lib/log"; -import { reloadingForNewBuild } from "../lib/staleBuild"; -import { Button } from "./ui/Button"; +import { CrashBoundary, type CrashCopy } from "./ui/CrashBoundary"; -interface Props { - children: ReactNode; -} -interface State { - crashed: boolean; -} +const copy = (): CrashCopy => ({ + title: i18n.t("errors.boundary.title"), + subtitle: i18n.t("errors.boundary.subtitle"), + reload: i18n.t("errors.boundary.reload"), +}); -// Catches render-time crashes anywhere in the tree, logs a fatal notification (kept -// in the center across the reload) and offers a recovery action. -export default class ErrorBoundary extends Component { - state: State = { crashed: false }; +// A fatal notification, kept in the center across the reload. +const onCrash = () => + notify({ + level: "fatal", + title: i18n.t("errors.boundary.notifTitle"), + // Never the raw `error.message`: a JS engine's English ("Cannot read properties of undefined…") + // is no message for the reader — the `ui.crash` log carries it for diagnosis. + message: i18n.t("errors.boundary.notifMessage"), + requiresInteraction: true, + }); - static getDerivedStateFromError(): State { - return { crashed: true }; - } - - componentDidCatch(error: Error, info: ErrorInfo) { - // The tab is already reloading onto a newer build (lib/staleBuild): this is the chunk it could not - // load, not a crash — no fatal notice, which would outlive the reload. - if (reloadingForNewBuild()) return; - log.fatal("ui.crash", "the app crashed while rendering", { - ...errorAttrs(error), - ...(info.componentStack ? { component_stack: info.componentStack } : {}), - }); - notify({ - level: "fatal", - title: i18n.t("errors.boundary.notifTitle"), - // Never the raw `error.message`: a JS engine's English ("Cannot read properties of undefined…") - // is no message for the reader — the `ui.crash` log above carries it for diagnosis. - message: i18n.t("errors.boundary.notifMessage"), - requiresInteraction: true, - }); - } - - render() { - if (this.state.crashed) { - if (reloadingForNewBuild()) return null; - return ( -
-
-
- {i18n.t("errors.boundary.title")} -
-
{i18n.t("errors.boundary.subtitle")}
- -
-
- ); - } - return this.props.children; - } +// The app's crash boundary: `CrashBoundary` in the app's language, plus the notice in the center. +export default function ErrorBoundary({ children }: { children: ReactNode }) { + return ( + + {children} + + ); } diff --git a/frontend/src/components/Home.tsx b/frontend/src/components/Home.tsx index 3d68b850..cd73a7ba 100644 --- a/frontend/src/components/Home.tsx +++ b/frontend/src/components/Home.tsx @@ -546,7 +546,7 @@ function Hero({ ? // The channel is the subtitle already. [ video.view_count != null && `${formatCompact(video.view_count, lang)} ${t("card.views")}`, - relativeTime(video.published_at, lang), + relativeTime(video.published_at, lang, t), ] : []; const factLine = facts.filter(Boolean).join(" · "); diff --git a/frontend/src/components/NotificationsPanel.menu.dom.test.tsx b/frontend/src/components/NotificationsPanel.menu.dom.test.tsx index d450ec12..5cb4ef94 100644 --- a/frontend/src/components/NotificationsPanel.menu.dom.test.tsx +++ b/frontend/src/components/NotificationsPanel.menu.dom.test.tsx @@ -364,3 +364,19 @@ describe("a download-lifecycle notice", () => { expect(screen.queryByText("download")).toBeNull(); }); }); + +describe("NotificationsPanel · a finished job's title", () => { + it("names what finished: a download, a clip, an upload", () => { + h.server = [ + server(1), + server(2, { data: { title: "Ready 2", kind: "edit" } }), + server(3, { data: { title: "Ready 3", kind: "upload" } }), + ]; + h.client = []; + mount(); + const titleOf = (text: string) => rowOf(text).textContent ?? ""; + expect(titleOf("Ready 1")).toContain("inbox.downloadDone.title"); + expect(titleOf("Ready 2")).toContain("inbox.editDone.title"); + expect(titleOf("Ready 3")).toContain("inbox.uploadDone.title"); + }); +}); diff --git a/frontend/src/components/NotificationsPanel.tsx b/frontend/src/components/NotificationsPanel.tsx index 9ea0cab7..134a5cfb 100644 --- a/frontend/src/components/NotificationsPanel.tsx +++ b/frontend/src/components/NotificationsPanel.tsx @@ -446,6 +446,11 @@ const DOWNLOAD_LIFECYCLE: Record = { download_expired: "downloadExpired", download_evicted: "downloadEvicted", }; +// A `download_done` notice's title by the producer's `kind`; any other kind reads as a download. +const DONE_TITLE: Record = { + edit: "inbox.editDone.title", + upload: "inbox.uploadDone.title", +}; function serverKind(n: AppNotification): "scheduler" | "download" | null { if (n.type === "scheduler" && typeof n.data?.job_id === "string") return "scheduler"; @@ -552,10 +557,11 @@ function NotificationRow({ title = t(`inbox.${lifecycle}.title`, { from: d.from ?? "" }); body = d.title || null; } else if (isDownloadDone) { - // "Download ready" / "Edit ready" per the producer's kind, with the video's title as the body - // (shipped in data.title). Same notification type; the copy differs so an edit isn't mislabelled. + // "Download ready" / "Edit ready" / "Upload ready" per the producer's kind, with the video's title + // as the body (shipped in data.title). Same notification type; the copy differs so an edit or an + // upload isn't called a download. const d = n.data as { title?: string | null; kind?: string }; - title = t(d.kind === "edit" ? "inbox.editDone.title" : "inbox.downloadDone.title"); + title = t(DONE_TITLE[d.kind ?? ""] ?? "inbox.downloadDone.title"); body = d.title || n.body; } return ( diff --git a/frontend/src/components/PlayerModal.dom.test.tsx b/frontend/src/components/PlayerModal.dom.test.tsx index f856b5cd..774e903b 100644 --- a/frontend/src/components/PlayerModal.dom.test.tsx +++ b/frontend/src/components/PlayerModal.dom.test.tsx @@ -465,7 +465,9 @@ describe("PlayerModal — fits the window's height", () => { {}} onState={() => {}} />, ); const row = document.querySelector(".player-stage")!.parentElement!.parentElement!; - expect(row.style.maxWidth).toBe("min(63rem, calc((100dvh - 3rem - 160px) * 16 / 9 + 7rem))"); + expect(row.style.maxWidth).toBe( + "min(63rem, calc(max(36rem, (100dvh - 3rem - 160px) * 16 / 9) + 7rem))", + ); }); }); diff --git a/frontend/src/components/PlayerModal.tsx b/frontend/src/components/PlayerModal.tsx index 3736cbf1..b4e24f3c 100644 --- a/frontend/src/components/PlayerModal.tsx +++ b/frontend/src/components/PlayerModal.tsx @@ -104,6 +104,8 @@ function canScrollBy(el: HTMLElement | null, dir: -1 | 1): boolean { // a rem width, so it grows with the text size; the video takes what is left. const WIDE_PLAYER_PX = 1280; const SIDE_PANEL = "20rem"; +// The narrowest the stacked video gets to make room for the chrome under it (see the card's maxWidth). +const MIN_VIDEO = "36rem"; // The meta line's items each lead with a "·" that parts them from what stands before: the channel name // on its line. In the wide panel the meta has a line of its own, where the first "·" would lead nothing. @@ -611,6 +613,10 @@ export default function PlayerModal({ onLoadVideo: loadVideo, }; + // The card's width budget (its maxWidth, below): the video's, and the side strips' gutters. + const fitWidth = `(100dvh - 3rem - ${below}px) * 16 / 9`; + const gutters = hasQueue ? " + 7rem" : ""; + // Portaled to , like every other full-screen overlay here. A fixed, full-viewport modal // must not render inside the page scroller: once that scroller is a stacking context (its // edge-fade mask makes it one) this z-overlay is scoped inside it, and the nav rail (z-rail) and the @@ -635,13 +641,16 @@ export default function PlayerModal({ className={`relative flex w-full max-h-full ${hasQueue ? "sm:px-14" : ""}`} // The design width (63rem with the side strips, else 56rem), narrowed so the 16:9 video plus the // chrome under it fits the window's height less the backdrop's padding (`p-6` = 3rem). - // Wide: no design cap — the video is as wide as the window's height allows, plus the side panel. + // Wide: no design cap — the video is as wide as the window's height allows, plus the side panel, + // whose content sits beside the video (`below` is ~0), so the fit is exact and the card, which + // does not scroll there, always holds the whole video. + // Stacked: never narrower than MIN_VIDEO. The chrome under the video grows as the card narrows + // (its text wraps), so on a short window fitting it was a loop that ran the card down to 2 px + // (a laptop at 100%, a 1080p screen at 175%). Past the floor the stacked card scrolls instead. style={{ maxWidth: wide - ? `calc((100dvh - 3rem - ${below}px) * 16 / 9 + ${SIDE_PANEL}${hasQueue ? " + 7rem" : ""})` - : `min(${hasQueue ? "63rem" : "56rem"}, calc((100dvh - 3rem - ${below}px) * 16 / 9${ - hasQueue ? " + 7rem" : "" - }))`, + ? `calc(${fitWidth} + ${SIDE_PANEL}${gutters})` + : `min(${hasQueue ? "63rem" : "56rem"}, calc(max(${MIN_VIDEO}, ${fitWidth})${gutters}))`, }} > {hasQueue && ( @@ -977,7 +986,7 @@ export default function PlayerModal({ )} {metaSep} - {relativeTime(active.published_at, lang)} + {relativeTime(active.published_at, lang, t)} {active.published_at && ` · ${fullDate(active.published_at)}`} {active.duration_seconds != null && ( @@ -1009,7 +1018,7 @@ export default function PlayerModal({ {detail.data?.published_at && ( {metaSep} - {relativeTime(detail.data.published_at, lang)} ·{" "} + {relativeTime(detail.data.published_at, lang, t)} ·{" "} {fullDate(detail.data.published_at)} )} diff --git a/frontend/src/components/PlexBrowse.tsx b/frontend/src/components/PlexBrowse.tsx index c65a1cf7..d8f9f0a6 100644 --- a/frontend/src/components/PlexBrowse.tsx +++ b/frontend/src/components/PlexBrowse.tsx @@ -68,7 +68,7 @@ const PlexCollectionView = lazy(() => import("./PlexCollectionView")); const PAGE = 40; -// D3: the "last watched" sort auto-applied when you enter the in-progress watch-state view (most +// The "last watched" sort auto-applied when you enter the in-progress watch-state view (most // recently watched first). Frozen for the same reason as PLEX_SORT_FALLBACK (handed out by reference). const LASTWATCHED_SORT: NonNullable = Object.freeze({ key: "lastwatched", @@ -229,7 +229,7 @@ export default function PlexBrowse() { return () => window.removeEventListener("keydown", onKey); }, [sub, canStepBack]); - // D3: entering the in-progress watch-state view auto-applies the "last watched" sort (most recently + // Entering the in-progress watch-state view auto-applies the "last watched" sort (most recently // watched first); leaving restores the sort that was active before entering. Only the ENTER edge // forces the sort, so a manual sort change while in the in-progress view still wins — it's just not // carried back out (exit returns to the pre-entry preference). We stash the raw provider `sort`, not diff --git a/frontend/src/components/PlexDiscover.dom.test.tsx b/frontend/src/components/PlexDiscover.dom.test.tsx index 63c4ace6..4dd6537a 100644 --- a/frontend/src/components/PlexDiscover.dom.test.tsx +++ b/frontend/src/components/PlexDiscover.dom.test.tsx @@ -273,7 +273,7 @@ describe("PlexDiscover", () => { }); // Every rail and the genre grid are lists the keyboard walks (lib/listNav), so `j` goes on -// from one to the next in page order (owner decision D4). +// from one to the next in page order (the owner's call). describe("PlexDiscover · keyboard", () => { it("registers every rail and the genre grid, and j walks on from rail to rail", async () => { plexDiscover.mockResolvedValue(fullPayload); diff --git a/frontend/src/components/Scheduler.tsx b/frontend/src/components/Scheduler.tsx index 0d8a1967..29075f6b 100644 --- a/frontend/src/components/Scheduler.tsx +++ b/frontend/src/components/Scheduler.tsx @@ -254,7 +254,7 @@ function JobRow({ {job.running ? t("scheduler.runningNow") : job.last_finished - ? t("scheduler.lastRun", { time: relativeTime(job.last_finished, lang) }) + ? t("scheduler.lastRun", { time: relativeTime(job.last_finished, lang, t) }) : t("scheduler.neverRun")} {job.last_error ? ( diff --git a/frontend/src/components/ShareDialog.tsx b/frontend/src/components/ShareDialog.tsx index d9a9f76b..77453bdd 100644 --- a/frontend/src/components/ShareDialog.tsx +++ b/frontend/src/components/ShareDialog.tsx @@ -235,7 +235,7 @@ function LinkShare({ job }: { job: DownloadJob }) { queryFn: () => api.downloadLinks(job.id), }); const [creating, setCreating] = useState(false); - // D1: downloadable by default — matches the one-click Share (which relies on the backend + // Downloadable by default — matches the one-click Share (which relies on the backend // default) and the common "let them have the file" intent. The owner can uncheck it here. const [allowDownload, setAllowDownload] = useState(true); const [expiryDays, setExpiryDays] = useState(null); @@ -260,7 +260,7 @@ function LinkShare({ job }: { job: DownloadJob }) { }); // Open with a link already generated: sharing is almost always the goal, so mint a default - // (no password, no expiry, downloadable per D1) link the moment the dialog finds none — as if the + // (no password, no expiry, downloadable) link the moment the dialog finds none — as if the // user had pressed "New link" → "Create". All further controls (password, download toggle, expiry, // extra links, revoke) stay available on the created link and via the "New link" button. const autoTried = useRef(false); diff --git a/frontend/src/components/UpNextCard.tsx b/frontend/src/components/UpNextCard.tsx index 8b541533..72d15ece 100644 --- a/frontend/src/components/UpNextCard.tsx +++ b/frontend/src/components/UpNextCard.tsx @@ -10,7 +10,7 @@ import { Button } from "./ui/Button"; import { IconButton } from "./ui/IconButton"; // Up-next / end-card. Shown at episode end when a next item exists and the pref is on, INSTEAD -// of the old instant jump — so binge stays under the viewer's control (D6-A). Counts down ~10s then +// of the old instant jump — so binge stays under the viewer's control. Counts down ~10s then // advances; "Play now" advances immediately; Cancel/✕ dismisses and stays on the finished frame. It only // lives on the play-to-the-end path; credits-auto-skip advances earlier and never reaches this card. const COUNTDOWN_S = 10; diff --git a/frontend/src/components/UploadDialog.dom.test.tsx b/frontend/src/components/UploadDialog.dom.test.tsx index 4a3ca217..097d0561 100644 --- a/frontend/src/components/UploadDialog.dom.test.tsx +++ b/frontend/src/components/UploadDialog.dom.test.tsx @@ -10,7 +10,7 @@ import { render, screen, fireEvent, waitFor, cleanup } from "@testing-library/re import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; const h = vi.hoisted(() => ({ - uploadDownload: vi.fn(async (_f: File, _o: unknown) => ({ id: 42 })), + uploadDownload: vi.fn(async (_f: File, _o: unknown) => ({ id: 42, status: "done" })), // The dialog reads the per-file upload cap so it can refuse an over-cap file before spending the // user's bandwidth on it. Generous by default; the cap test below lowers it. downloadUsage: vi.fn(async () => ({ upload_max_bytes: 1_000_000 })), @@ -78,9 +78,25 @@ describe("UploadDialog", () => { expect(passedFile).toBe(f); expect((opts as { displayName?: string }).displayName).toBe("My notes"); await waitFor(() => expect(h.notify).toHaveBeenCalledTimes(1)); + expect(h.notify.mock.calls[0]![0]).toMatchObject({ message: "downloads.upload.added" }); await waitFor(() => expect(onClose).toHaveBeenCalled()); }); + it("says so when the server queued the upload for its phone-safe conversion", async () => { + // A video no phone could play as it is (an iPhone's HEVC .mov) comes back `queued`, not `done`: + // "File uploaded" alone would send the user to a Library row that is still converting. + h.uploadDownload.mockResolvedValueOnce({ id: 43, status: "queued" }); + wrap(); + fireEvent.change(fileInput(), { + target: { files: [new File(["x"], "clip.mov", { type: "video/quicktime" })] }, + }); + fireEvent.click(screen.getByText("downloads.upload.submit")); + await waitFor(() => expect(h.notify).toHaveBeenCalledTimes(1)); + expect(h.notify.mock.calls[0]![0]).toMatchObject({ + message: "downloads.upload.addedConverting", + }); + }); + it("keeps Upload disabled until a file is chosen", () => { wrap(); const btn = screen.getByText("downloads.upload.submit").closest("button")!; diff --git a/frontend/src/components/UploadDialog.tsx b/frontend/src/components/UploadDialog.tsx index d410e70e..414624e4 100644 --- a/frontend/src/components/UploadDialog.tsx +++ b/frontend/src/components/UploadDialog.tsx @@ -19,7 +19,14 @@ import { Input } from "./ui/Input"; // Local-file ingest. A general upload — ANY file type, no format list: media // (video/audio) joins the library with the full function set, a generic file is download-only. The // file is streamed to the server via XHR so this dialog can show real upload progress. On success it -// lands in the Library as an already-`done` job, so we invalidate + point the user there. +// lands in the Library as an already-`done` job — or, for media no phone could play as it is (an +// iPhone's HEVC, a webm, an mkv), as a `queued` job the worker converts first — so we invalidate, +// say which, and point the user there. +// Out here, not inline in the upload handler: a `?:` inside its `try` makes the React Compiler +// skip the whole dialog. +const uploadedKey = (status: string) => + status === "done" ? "downloads.upload.added" : "downloads.upload.addedConverting"; + export default function UploadDialog({ onClose }: { onClose: () => void }) { const { t, i18n } = useTranslation(); const lang = i18n.language; @@ -73,7 +80,7 @@ export default function UploadDialog({ onClose }: { onClose: () => void }) { setPct(0); const displayName = name.trim() || undefined; try { - await api.uploadDownload(file, { + const job = await api.uploadDownload(file, { displayName, onProgress: setPct, signal: ac.signal, @@ -81,7 +88,7 @@ export default function UploadDialog({ onClose }: { onClose: () => void }) { invalidateDownloads(qc); notify({ level: "success", - message: t("downloads.upload.added"), + message: t(uploadedKey(job.status)), action: { label: t("downloads.dialog.view"), onClick: () => navigateTo("downloads") }, }); onClose(); diff --git a/frontend/src/components/VideoCard.tsx b/frontend/src/components/VideoCard.tsx index d8f624e1..5aa4a335 100644 --- a/frontend/src/components/VideoCard.tsx +++ b/frontend/src/components/VideoCard.tsx @@ -420,7 +420,7 @@ function VideoCard({ ); const published = ( <> - {relativeTime(video.published_at, lang)} + {relativeTime(video.published_at, lang, t)} {video.published_at && ( <> {" · "} diff --git a/frontend/src/components/WatchPage.dom.test.tsx b/frontend/src/components/WatchPage.dom.test.tsx index 1395cab8..5ae96f85 100644 --- a/frontend/src/components/WatchPage.dom.test.tsx +++ b/frontend/src/components/WatchPage.dom.test.tsx @@ -211,6 +211,46 @@ describe("WatchPage honest failures", () => { expect(page.queryByRole("button", { name: "Try again" })).toBeNull(); }); + it("a 404 whose video is being made again says to come back, with a retry", async () => { + window.history.replaceState({}, "", "/watch/tok-preparing"); + vi.stubGlobal( + "fetch", + vi + .fn() + .mockResolvedValueOnce(res(404, { detail: { code: 404, reason: "video_preparing" } })) + .mockResolvedValue( + res(200, { needs_password: false, file_url: "/f.mp4", title: "A clip" }), + ), + ); + const page = render(); + const retry = await page.findByRole("button", { name: "Try again" }); + expect(page.getByRole("alert").textContent).toContain("This video is being prepared."); + expect(page.queryByText(GONE)).toBeNull(); + + fireEvent.click(retry); + await waitFor(() => expect(page.container.querySelector("video")).not.toBeNull()); + }); + + it("a Retry that is answered 'still preparing' says so and keeps the focus on Retry", async () => { + // The answer replaces the button the viewer was on: announced (an alert, inserted filled), and + // the new Retry takes the focus instead of leaving it on . + window.history.replaceState({}, "", "/watch/tok-preparing"); + const preparing = () => res(404, { detail: { code: 404, reason: "video_preparing" } }); + vi.stubGlobal( + "fetch", + vi.fn().mockImplementation(() => Promise.resolve(preparing())), + ); + const page = render(); + const first = await page.findByRole("button", { name: "Try again" }); + expect(document.activeElement).not.toBe(first); // the first load moves no focus + + fireEvent.click(first); + await waitFor(() => + expect(document.activeElement).toBe(page.getByRole("button", { name: "Try again" })), + ); + expect(page.getByRole("alert").textContent).toContain("This video is being prepared."); + }); + async function tryPassword(second: () => Promise) { vi.stubGlobal( "fetch", diff --git a/frontend/src/components/WatchPage.tsx b/frontend/src/components/WatchPage.tsx index 40da9bf0..a42e4f9e 100644 --- a/frontend/src/components/WatchPage.tsx +++ b/frontend/src/components/WatchPage.tsx @@ -9,10 +9,11 @@ import { send } from "../lib/http"; import { Input } from "./ui/Input"; import { SURFACE } from "./ui/surfaces"; import { LinkButton } from "./ui/LinkButton"; +import { T, lang } from "./watchStrings"; -// Standalone, login-free video page for a shared /watch/ link. Rendered OUTSIDE the app -// tree (no auth, no providers) — like the /privacy and /terms pages. Uses plain fetch and a tiny -// self-contained i18n dict (the public viewer has no app language preference). +// Standalone, login-free video page for a shared /watch/ link. Its own entry (watchMain.tsx, +// watch.html): no auth, no providers, none of the app's bundle. Uses plain fetch and a tiny +// self-contained i18n dict (`watchStrings`: the public viewer has no app language preference). // Advertise the playing video to the OS via the Media Session API: lock-screen / control-center // metadata + play/pause handlers. This is also what lets a browser's own "background playback" @@ -75,79 +76,20 @@ function displayUrl(url: string): string { } } -const STR = { - en: { - loading: "Loading…", - gone: "This link is no longer available.", - goneHint: "It may have expired or been revoked by the owner.", - passwordTitle: "This video is password-protected", - passwordPlaceholder: "Enter password", - watch: "Watch", - wrong: "Wrong password.", - download: "Download", - file: "File", - links: "Links", - brand: "Shared via Siftlode", - repeat: "Repeat", - failed: "Couldn't load this link.", - failedHint: "Check your connection and try again.", - retry: "Try again", - tooMany: "Too many attempts. Try again in a few minutes.", - unlockFailed: "Couldn't check the password. Try again.", - }, - hu: { - loading: "Betöltés…", - gone: "Ez a link már nem elérhető.", - goneHint: "Lehet, hogy lejárt, vagy a tulajdonos visszavonta.", - passwordTitle: "Ez a videó jelszóval védett", - passwordPlaceholder: "Add meg a jelszót", - watch: "Megnézem", - wrong: "Hibás jelszó.", - download: "Letöltés", - file: "Fájl", - links: "Linkek", - brand: "Megosztva a Siftlode-dal", - repeat: "Ismétlés", - failed: "Nem sikerült betölteni a linket.", - failedHint: "Ellenőrizd a kapcsolatot, és próbáld újra.", - retry: "Újrapróbálás", - tooMany: "Túl sok próbálkozás. Próbáld újra pár perc múlva.", - unlockFailed: "Nem sikerült ellenőrizni a jelszót. Próbáld újra.", - }, - de: { - loading: "Wird geladen…", - gone: "Dieser Link ist nicht mehr verfügbar.", - goneHint: "Er ist möglicherweise abgelaufen oder wurde widerrufen.", - passwordTitle: "Dieses Video ist passwortgeschützt", - passwordPlaceholder: "Passwort eingeben", - watch: "Ansehen", - wrong: "Falsches Passwort.", - download: "Herunterladen", - file: "Datei", - links: "Links", - brand: "Geteilt über Siftlode", - repeat: "Wiederholen", - failed: "Der Link konnte nicht geladen werden.", - failedHint: "Prüfe deine Verbindung und versuche es erneut.", - retry: "Erneut versuchen", - tooMany: "Zu viele Versuche. Versuche es in ein paar Minuten erneut.", - unlockFailed: "Das Passwort konnte nicht geprüft werden. Versuche es erneut.", - }, -} as const; - -const lang = - ((navigator.language || "en").slice(0, 2) as keyof typeof STR) in STR - ? ((navigator.language || "en").slice(0, 2) as keyof typeof STR) - : "en"; -const T = STR[lang]; +/** What a 404 from the link means: `video_preparing` is a link whose video is being made again (a + * re-download, a conversion) and will work shortly; any other is a link that is gone. */ +async function notFoundState(r: Response): Promise<"preparing" | "gone"> { + const body = (await r.json().catch(() => null)) as { detail?: { reason?: string } } | null; + return body?.detail?.reason === "video_preparing" ? "preparing" : "gone"; +} export default function WatchPage() { const token = window.location.pathname.split("/watch/")[1]?.split(/[/?#]/)[0] ?? ""; // Seeded from the URL rather than corrected by the effect below: with no token there is nothing // to load, and "loading" was committed and painted before the effect said so. - const [status, setStatus] = useState<"loading" | "ready" | "password" | "gone" | "error">( - token ? "loading" : "gone", - ); + const [status, setStatus] = useState< + "loading" | "ready" | "password" | "gone" | "preparing" | "error" + >(token ? "loading" : "gone"); const [meta, setMeta] = useState(null); const [password, setPassword] = useState(""); const [error, setError] = useState(null); @@ -172,19 +114,23 @@ export default function WatchPage() { // Bumped by Retry, which asks again. const [attempt, setAttempt] = useState(0); + // Set once the viewer acted (Retry, the password): the state that answers replaces the button they + // were on, so its own Retry takes the focus rather than leaving it on . Not on the first load, + // where nothing had the focus to lose. + const [acted, setActed] = useState(false); useEffect(() => { if (!token) return; // the seed above already says "gone" send(`/api/public/watch/${encodeURIComponent(token)}`) - .then((r) => { - // Only a 404 says the link is gone (unknown, expired, revoked, or its file removed). Anything - // else — a 5xx while the server restarts, a dropped connection — is a failure worth retrying, - // not a dead link. - if (r.status === 404) return null; + .then((r): Promise => { + // Only a 404 says the link is gone (unknown, expired, revoked, or its file removed) — or that + // its video is being made again (`notFoundState`). Anything else — a 5xx while the server + // restarts, a dropped connection — is a failure worth retrying, not a dead link. + if (r.status === 404) return notFoundState(r); if (!r.ok) throw new Error(`HTTP ${r.status}`); return r.json() as Promise; }) .then((m) => { - if (!m) setStatus("gone"); + if (typeof m === "string") setStatus(m); else if (m.needs_password) setStatus("password"); else { setMeta(m); @@ -194,6 +140,7 @@ export default function WatchPage() { .catch(() => setStatus("error")); }, [token, attempt]); const retry = () => { + setActed(true); setStatus("loading"); setAttempt((n) => n + 1); }; @@ -216,7 +163,7 @@ export default function WatchPage() { return; } if (r.status === 404) { - setStatus("gone"); + setStatus(await notFoundState(r)); return; } if (!r.ok) throw new Error(`HTTP ${r.status}`); @@ -225,6 +172,7 @@ export default function WatchPage() { }; const unlock = async () => { + setActed(true); setUnlocking(true); setError(null); try { @@ -259,13 +207,40 @@ export default function WatchPage() { )} + {status === "preparing" && ( +
+ {/* An alert, like the failure below: it is inserted already filled, which a polite status + region is not reliably announced for — and a Retry that answers "still preparing" + would otherwise say nothing at all. */} +
+
{T.preparing}
+
{T.preparingHint}
+ +
+
+ )} + {status === "error" && (
{/* Announced: it replaces the loading line, and this page has no live region of its own. */}
{T.failed}
{T.failedHint}
-
diff --git a/frontend/src/components/downloadAdmin.tsx b/frontend/src/components/downloadAdmin.tsx index b7fb5f59..dcc59e4e 100644 --- a/frontend/src/components/downloadAdmin.tsx +++ b/frontend/src/components/downloadAdmin.tsx @@ -186,7 +186,7 @@ export function AdminSystem() { {s && (
{[ - [t("downloads.admin.readyFiles"), String(s.ready_files)], + [t("downloads.admin.filesOnDisk"), String(s.files_on_disk)], [t("downloads.admin.totalSize"), formatBytes(s.total_bytes, lang)], [ t("downloads.admin.cap"), diff --git a/frontend/src/components/downloadModals.tsx b/frontend/src/components/downloadModals.tsx index 9de5d747..c7bb47ab 100644 --- a/frontend/src/components/downloadModals.tsx +++ b/frontend/src/components/downloadModals.tsx @@ -7,7 +7,7 @@ import Modal, { ModalFooter } from "./ui/Modal"; import { api, type DownloadJob } from "../lib/api"; import { Button } from "./ui/Button"; import { IconBtn } from "./downloadRow"; -import { failCopyKeys } from "../lib/downloadFailure"; +import { failCopyKeys, failTitleKey } from "../lib/downloadFailure"; import { Input } from "./ui/Input"; // Per-item user modals split out of DownloadCenter (self-contained, prop-driven): rename/metadata @@ -112,7 +112,7 @@ export function MetaEditModal({ job, onClose }: { job: DownloadJob; onClose: () // retry control regardless. `unknown` is deliberately NOT here: an unclassified failure is most often an // auth wall we couldn't parse (a login-walled Facebook post reads as a bare "no format"), so a // prominent Retry would mislead; force-retrying from the row stays available for genuine transients. -const RETRYABLE_ERRORS = ["postprocess", "network"]; +const RETRYABLE_ERRORS = ["postprocess", "network", "convert_failed"]; // A readable "why did this fail" window for a failed job: a category headline + plain explanation, // the raw yt-dlp/ffmpeg text behind a disclosure, and a Retry when the failure may be transient. @@ -130,7 +130,7 @@ export function ErrorModal({ const [showRaw, setShowRaw] = useState(false); const retryable = RETRYABLE_ERRORS.includes(code); return ( - +
{t(failCopyKeys(job, "short"))}

{t(failCopyKeys(job, "body"))}

diff --git a/frontend/src/components/downloadRow.dom.test.tsx b/frontend/src/components/downloadRow.dom.test.tsx index 7ef2ba2e..39df6c0c 100644 --- a/frontend/src/components/downloadRow.dom.test.tsx +++ b/frontend/src/components/downloadRow.dom.test.tsx @@ -292,3 +292,27 @@ describe("DownloadRow · no picture", () => { expect(container.querySelector("svg.lucide-film")).not.toBeNull(); }); }); + +// The phone-safe conversion reports ffmpeg's real progress, so it gets the % bar a download has; +// a yt-dlp post-step reports none and keeps the pulse. +describe("DownloadRow · a running conversion", () => { + const running = (phase: string): DownloadJob => ({ + ...baseJob, + status: "running", + progress: 40, + phase, + }); + + it("shows its phase and its %", () => { + const { container } = render(); + expect(screen.getByText("downloads.phase.converting")).toBeTruthy(); + expect(screen.getByText("40%")).toBeTruthy(); + expect(container.querySelector(".animate-pulse")).toBeNull(); + }); + + it("a post-step with no % keeps the pulse", () => { + const { container } = render(); + expect(screen.queryByText("40%")).toBeNull(); + expect(container.querySelector(".animate-pulse")).not.toBeNull(); + }); +}); diff --git a/frontend/src/components/downloadRow.tsx b/frontend/src/components/downloadRow.tsx index 45096d94..926e9c3d 100644 --- a/frontend/src/components/downloadRow.tsx +++ b/frontend/src/components/downloadRow.tsx @@ -14,7 +14,7 @@ import { relativeTime, } from "../lib/format"; import { navItemAttrs } from "../lib/listNav"; -import { failCopyKeys } from "../lib/downloadFailure"; +import { failCopyKeys, failTitleKey } from "../lib/downloadFailure"; import { displayUrl, hasOwnTitle, jobTitle } from "./downloadTitle"; import { IconButton } from "./ui/IconButton"; import { Pressable } from "./ui/Pressable"; @@ -34,12 +34,14 @@ const STATUS_CLS: Record = { }; // Byte-progress phases (show a % bar) vs ffmpeg post-steps (no %, indeterminate pulse). -// "editing" (the phase-2 editor) reports a real % from ffmpeg's out_time, so it gets a bar. -const DOWNLOAD_PHASES = ["video", "audio", "editing"]; +// "editing" (the phase-2 editor) and "converting" (the phone-safe conversion of a download or an +// upload) report a real % from ffmpeg's out_time, so they get a bar. +const DOWNLOAD_PHASES = ["video", "audio", "editing", "converting"]; const PHASE_KEYS = [ "video", "audio", "editing", + "converting", "merging", "audio_extract", "thumbnail", @@ -304,7 +306,7 @@ export function DownloadRow({ {" · "} {" "} - {relativeTime(job.created_at, lang)} + {relativeTime(job.created_at, lang, t)} ) : null} {job.status === "error" ? ( @@ -313,7 +315,7 @@ export function DownloadRow({ onShowError?.(job)} className="text-danger inline-block leading-6 min-h-6 touch-target pointer-coarse:leading-(--touch-target)" - title={t("downloads.errors.fail.title")} + title={t(failTitleKey(job))} > · {t(failCopyKeys(job, "short"))}{" "} diff --git a/frontend/src/components/listNav.wiring.contract.test.ts b/frontend/src/components/listNav.wiring.contract.test.ts index f4c002c1..c34899a0 100644 --- a/frontend/src/components/listNav.wiring.contract.test.ts +++ b/frontend/src/components/listNav.wiring.contract.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { readSources } from "../lib/rawSources.testkit"; // Every rail and every card of a card list is walkable by the keyboard (lib/listNav). Two // opt-in halves, both checked here, because either one missing silently leaves the keyboard out — @@ -79,7 +80,7 @@ export function inList(code: string, pos: number): boolean { const has = (tag: Tag, prop: string) => new RegExp(`\\s${prop}\\b`).test(tag.text); function offenders(): { hits: string[]; tags: number; exempted: Set } { - const files = import.meta.glob("./**/*.tsx", { query: "?raw", import: "default", eager: true }); + const files = readSources(__dirname, "./**/*.tsx"); const hits: string[] = []; const exempted = new Set(); let tags = 0; diff --git a/frontend/src/components/plexCards.dom.test.tsx b/frontend/src/components/plexCards.dom.test.tsx index af05785c..6f508f8c 100644 --- a/frontend/src/components/plexCards.dom.test.tsx +++ b/frontend/src/components/plexCards.dom.test.tsx @@ -258,7 +258,7 @@ describe("EpisodeCard a11y", () => { }); // In a keyboard-walkable grid or rail the WHOLE card is the item (the ring goes round the -// poster and its title, D2) and the poster — the card's own role=button — is what Enter opens. +// poster and its title) and the poster — the card's own role=button — is what Enter opens. describe("Plex cards in a keyboard-walkable list", () => { const cases = [ [ diff --git a/frontend/src/components/ui/CrashBoundary.tsx b/frontend/src/components/ui/CrashBoundary.tsx new file mode 100644 index 00000000..5576ea82 --- /dev/null +++ b/frontend/src/components/ui/CrashBoundary.tsx @@ -0,0 +1,62 @@ +import { Component, type ErrorInfo, type ReactNode } from "react"; +import { errorAttrs, log } from "../../lib/log"; +import { reloadingForNewBuild } from "../../lib/staleBuild"; +import { Button } from "./Button"; + +export interface CrashCopy { + title: string; + subtitle: string; + reload: string; +} + +interface Props { + children: ReactNode; + /** Read at crash time, so the screen speaks the language active then. */ + copy: () => CrashCopy; + /** What the entry does beyond the log line (the app keeps a fatal notice in its center). */ + onCrash?: () => void; +} +interface State { + crashed: boolean; +} + +// Catches render-time crashes anywhere in the tree, logs them and offers a reload. Free of the app's +// i18n and notification store, so the public /watch entry wraps its page in it without loading +// either; `ErrorBoundary` is the app's wrapper around it. +export class CrashBoundary extends Component { + state: State = { crashed: false }; + + static getDerivedStateFromError(): State { + return { crashed: true }; + } + + componentDidCatch(error: Error, info: ErrorInfo) { + // The tab is already reloading onto a newer build (lib/staleBuild): this is the chunk it could not + // load, not a crash — no fatal notice, which would outlive the reload. + if (reloadingForNewBuild()) return; + log.fatal("ui.crash", "the app crashed while rendering", { + ...errorAttrs(error), + ...(info.componentStack ? { component_stack: info.componentStack } : {}), + }); + this.props.onCrash?.(); + } + + render() { + if (this.state.crashed) { + if (reloadingForNewBuild()) return null; + const copy = this.props.copy(); + return ( +
+
+
{copy.title}
+
{copy.subtitle}
+ +
+
+ ); + } + return this.props.children; + } +} diff --git a/frontend/src/components/ui/README.md b/frontend/src/components/ui/README.md index 533ba48f..417a6166 100644 --- a/frontend/src/components/ui/README.md +++ b/frontend/src/components/ui/README.md @@ -41,7 +41,7 @@ Counter-examples, deliberately NOT here: `PosterTile` (knows about Plex montage | actions & inputs | `Button`, `IconButton`, `Pressable` (`selectable` when its text is a title or a name), `Chip` + `chipStyles`, `DragHandle` + `OrderButtons` / `useOrderItems` (the non-drag reorder, WCAG 2.5.7) (one per button family, docs/DESIGN.md; a raw `