From 14598ec320123f584e0be985b9ef57398d393428 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 10 Oct 2026 02:31:38 +0200 Subject: [PATCH] feat(downloads): ad-hoc clips cut straight from a YouTube video or a Plex item POST /api/downloads/clip takes a source (a YouTube id or a Plex rating key), a range of at most ten minutes and, for Plex, the audio track being heard. The clip becomes a per-user asset in the library like an editor clip, shareable as soon as it is ready. A YouTube clip fetches only its range (yt-dlp download_ranges: 3-7 s whatever the video's length); a Plex clip reads the library file in place. Both then go through one re-encode: frame-exact at the marks, H.264 at most 1080p with HDR tonemapped to SDR, stereo AAC, no chapter track, .mp4. Job kinds are a JobKind StrEnum now, and Re-download refuses every kind but a download. --- backend/app/downloads/clip.py | 169 +++++++ backend/app/downloads/pipeline.py | 170 ++++++- backend/app/downloads/quota.py | 4 +- backend/app/downloads/runner.py | 9 +- backend/app/downloads/service.py | 114 ++++- backend/app/downloads/status.py | 17 + backend/app/downloads/upload.py | 4 +- backend/app/routes/downloads.py | 40 +- backend/tests/test_clip.py | 423 ++++++++++++++++++ backend/tests/test_phone_safe_contract.py | 11 +- backend/tests/test_upload.py | 5 +- frontend/src/components/downloadRow.tsx | 2 +- .../src/components/useDownloadActions.tsx | 2 + frontend/src/lib/api/downloads.ts | 8 +- frontend/src/lib/downloadActions.test.ts | 1 + frontend/src/lib/downloadActions.ts | 2 +- frontend/src/lib/downloadFailure.test.ts | 10 + frontend/src/lib/downloadFailure.ts | 4 +- 18 files changed, 963 insertions(+), 32 deletions(-) create mode 100644 backend/app/downloads/clip.py create mode 100644 backend/tests/test_clip.py diff --git a/backend/app/downloads/clip.py b/backend/app/downloads/clip.py new file mode 100644 index 00000000..e0340059 --- /dev/null +++ b/backend/app/downloads/clip.py @@ -0,0 +1,169 @@ +"""Ad-hoc clips: a marked range of a YouTube video or a Plex item, cut into a new per-user asset. + +A clip never needs the whole source first. A YouTube clip fetches only its range (yt-dlp's +`download_ranges`; measured: 3-7 s whether the video runs four minutes or an hour, and the fetched +range starts on the marked frame), and a Plex clip reads the library file in place. Either way the +range then goes through ONE re-encode built here, so every clip has the same shape whatever its +source: frame-exact at both ends, H.264 at most 1080p (an HDR source tonemapped to SDR — kept as +10-bit PQ it played washed out), stereo AAC, in an .mp4 that plays on any phone. + +The spec is stored on the job (`DownloadJob.edit_spec`) and is the clip's whole identity: its +signature is the asset's cache key, so marking the same range again is an instant cache hit, and a +different range or audio track is a different clip. +""" + +import hashlib +import json +import math +import re +from pathlib import Path + +from app.downloads.edit import AAC_ARGS, EVEN_FRAME, INPUT_GUARD, X264_ARGS + +SOURCES = ("youtube", "plex") +# Owner's cap: a clip is a moment, not a way to export a whole film to a public link. +MAX_CLIP_S = 600 +MIN_CLIP_S = 1 +MAX_HEIGHT = 1080 +# What yt-dlp fetches the range in (formats.normalize fills the rest): the clip's own ceiling, so the +# re-encode never upscales and never throws away a better source than it keeps. +YOUTUBE_FETCH_SPEC = {"mode": "av", "max_height": MAX_HEIGHT, "container": "mp4"} +# Bump when the spec → output-bytes mapping changes, so a cached clip re-derives. +_SIG_VERSION = 1 + +_YOUTUBE_ID = re.compile(r"[A-Za-z0-9_-]{11}") +_RATING_KEY = re.compile(r"[0-9]{1,32}") +_MAX_AUDIO_TRACK = 63 +# The transfer characteristics of HDR video (PQ, HLG): an 8-bit SDR encode must tonemap them. +_HDR_TRANSFERS = frozenset({"smpte2084", "arib-std-b67"}) +# HDR → SDR as measured on the owner's 4K HDR files (2026-10-10): linearise, tonemap (hable), +# convert the primaries and transfer to BT.709 and back to 8-bit 4:2:0. +_TONEMAP = ( + "zscale=t=linear:npl=100,format=gbrpf32le,zscale=p=bt709,tonemap=hable:desat=0," + "zscale=t=bt709:m=bt709:r=tv,format=yuv420p" +) +_SCALE = f"scale=-2:'min({MAX_HEIGHT},ih)'" + + +class ClipError(Exception): + """A clip request that cannot be cut; `reason` is an i18n-mappable key.""" + + def __init__(self, reason: str): + self.reason = reason + super().__init__(reason) + + +def normalize( + source: str, ref: str, start_s: float, end_s: float, audio_track: int | None = None +) -> dict: + """The validated, canonical clip spec. Times are rounded to the millisecond so the same marks + always give the same cache key. Raises ClipError.""" + if source not in SOURCES: + raise ClipError("bad_source") + pattern = _YOUTUBE_ID if source == "youtube" else _RATING_KEY + if not isinstance(ref, str) or not pattern.fullmatch(ref): + raise ClipError("bad_source") + if not (math.isfinite(start_s) and math.isfinite(end_s)) or start_s < 0: + raise ClipError("bad_range") + start, end = round(start_s, 3), round(end_s, 3) + if end - start < MIN_CLIP_S: + raise ClipError("too_short") + if end - start > MAX_CLIP_S: + raise ClipError("too_long") + spec: dict = {"source": source, "ref": ref, "start_s": start, "end_s": end} + if source == "plex" and audio_track is not None: + if not 0 <= audio_track <= _MAX_AUDIO_TRACK: + raise ClipError("bad_audio") + spec["audio_track"] = audio_track + return spec + + +def duration(spec: dict) -> float: + return spec["end_s"] - spec["start_s"] + + +def sig(spec: dict) -> str: + """The clip's cache signature: the spec itself, versioned.""" + body = json.dumps({**spec, "_v": _SIG_VERSION}, sort_keys=True, separators=(",", ":")) + return hashlib.sha256(body.encode()).hexdigest()[:24] + + +def asset_ref(user_id: int, spec: dict) -> str: + """The asset's `source_ref`. The user id makes a clip private to whoever cut it, like an edit: + never dedup-shared, never cache-served to someone else.""" + return f"{user_id}:{spec['source']}:{spec['ref']}" + + +def reference_url(spec: dict) -> str | None: + """Where a YouTube clip came from: its video at the clip's start. A Plex clip has no public page.""" + if spec.get("source") != "youtube" or not spec.get("ref"): + return None + return f"https://www.youtube.com/watch?v={spec['ref']}&t={int(spec.get('start_s') or 0)}s" + + +def is_hdr(probe: dict) -> bool: + """Whether the first video stream of an ffprobe `-show_streams` JSON carries an HDR transfer.""" + for s in probe.get("streams") or []: + if s.get("codec_type") == "video": + return s.get("color_transfer") in _HDR_TRANSFERS + return False + + +def audio_track_count(probe: dict) -> int: + return sum(1 for s in probe.get("streams") or [] if s.get("codec_type") == "audio") + + +def build_cmd( + src: Path, + dest: Path, + start_s: float, + dur_s: float, + *, + audio_track: int | None, + hdr: bool, + threads: int, +) -> list[str]: + """ffmpeg for one clip: an input seek (decodes from the keyframe before `start_s` and drops the + frames up to it, so the clip starts on the marked frame), the first video stream, the chosen audio + track (the first when none was chosen; none at all is fine), no subtitles, data or chapters.""" + vf = ",".join([*([_TONEMAP] if hdr else []), _SCALE, EVEN_FRAME]) + audio = f"0:a:{audio_track}" if audio_track is not None else "0:a:0?" + return [ + "ffmpeg", + "-y", + "-hide_banner", + "-loglevel", + "error", + "-nostdin", + "-progress", + "pipe:1", + "-nostats", + "-ss", + f"{start_s:.3f}", + *INPUT_GUARD, + "-i", + str(src), + "-t", + f"{dur_s:.3f}", + "-map", + "0:v:0", + "-map", + audio, + "-sn", + "-dn", + # A source's chapters would ride into the .mp4 as a data track of their own (an mkv's + # chapter list came out as `bin_data`); a clip of a moment has no chapters to keep. + "-map_chapters", + "-1", + "-vf", + vf, + *X264_ARGS, + "-threads", + str(threads), + *AAC_ARGS, + "-ac", + "2", + "-movflags", + "+faststart", + str(dest), + ] diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index b8ac2324..f8c94d10 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -7,10 +7,13 @@ from datetime import date, datetime, timedelta, timezone from pathlib import Path import yt_dlp +from sqlalchemy import select +from yt_dlp.utils import download_range_func from app import logs, sysconfig from app.config import settings from app.db import SessionLocal +from app.downloads import clip as clipmod from app.downloads import edit as editmod from app.downloads import errors as dlerrors from app.downloads import adapters, credentials, formats, mediakind, playable, probe, quota @@ -26,7 +29,8 @@ from app.downloads.jobdb import ( ) from app.downloads.stated import StatedFailure from app.downloads.status import JobStatus -from app.models import DISPLAY_TEXT_MAX, DownloadJob, MediaAsset +from app.models import DISPLAY_TEXT_MAX, DownloadJob, MediaAsset, PlexItem +from app.plex import paths as plex_paths from app.notifications import create_notification from app.titles import normalize_title @@ -333,7 +337,7 @@ def _media_info(info: dict) -> dict: def _extract_with_fallback( - job_id, asset_id, url, spec, staging, adapter, cookiefile=None, max_bytes=None + job_id, asset_id, url, spec, staging, adapter, cookiefile=None, max_bytes=None, section=None ): """Download via yt-dlp using the source `adapter`'s client sets + options. For YouTube the adapter supplies the primary player-client set then a POT-backed fallback (retried on a bot/DRM/format @@ -349,7 +353,11 @@ def _extract_with_fallback( `cookiefile` is the acting user's per-host cookies.txt, already gated by `credentials.resolve_cookiefile`; passed to every attempt so the fallback stays authenticated. - `max_bytes` (SEC-13) is the owner's remaining storage budget, applied to every attempt.""" + `max_bytes` (SEC-13) is the owner's remaining storage budget, applied to every attempt. + + `section` = (start_s, end_s) fetches only that range (an ad-hoc clip, see downloads/clip): the + range is stream-copied, and its first frame is the marked one (measured), so the clip's own + re-encode starts at 0 instead of re-fetching keyframes at the cut.""" client_sets = adapter.client_sets(authed=cookiefile is not None) outtmpl = str(staging / "%(id)s.%(ext)s") # Resolving may read a page first — network time the stall watchdog must not see as a stall. @@ -375,6 +383,9 @@ def _extract_with_fallback( max_bytes=max_bytes, ) opts["postprocessor_hooks"] = [_make_pp_hook(job_id)] + if section is not None: + opts["download_ranges"] = download_range_func([], [section]) + opts["force_keyframes_at_cuts"] = False _touch_job(job_id) # stall-watchdog heartbeat: extraction writes no progress, so a slow / # multi-client-set fallback would otherwise cross the stall timeout with a stale updated_at. # (Per-ATTEMPT only; a single extract_info that itself runs past the stall timeout is still @@ -896,6 +907,159 @@ def _process_edit(job_id: int, asset_id: int) -> None: shutil.rmtree(staging, ignore_errors=True) +def _clip_source(job_id: int, asset_id: int, spec: dict, staging: Path) -> tuple[Path, float]: + """The file a clip is cut from and where in it the clip starts. A YouTube clip fetches ONLY its + range into `staging` (so it starts at 0 there); a Plex clip reads the library file in place.""" + if spec["source"] == "youtube": + with SessionLocal() as db: + job = db.get(DownloadJob, job_id) + remaining = quota.remaining_bytes(db, job.user_id) if job else None + if remaining == 0: # see `_download`: zero is a refusal, not "no limit" + raise _OwnerRefused(dlerrors.QUOTA_FULL_MESSAGE) + _set_job(job_id, phase="downloading", progress=0) + # Signed out, like a download without a cookie: the cookie gate is written for a download's + # salted asset identity (`credentials.resolve_cookiefile`), not for a clip's. + info = _extract_with_fallback( + job_id, + asset_id, + service.source_url("youtube", spec["ref"]), + formats.normalize(clipmod.YOUTUBE_FETCH_SPEC), + staging, + adapters.resolve_adapter("youtube", spec["ref"]), + max_bytes=remaining, + section=(spec["start_s"], spec["end_s"]), + ) + return _produced_file(_media_info(dict(info)), staging), 0.0 + with SessionLocal() as db: + item = db.execute( + select(PlexItem).where(PlexItem.rating_key == spec["ref"]) + ).scalar_one_or_none() + src = plex_paths.local_media_path(db, item.file_path) if item else None + if src is None: + raise StatedFailure("The library file is not reachable from here.") + return src, spec["start_s"] + + +def _process_clip(job_id: int, asset_id: int) -> None: + """Cut an ad-hoc clip (downloads/clip): fetch or open its source, ONE re-encode to the clip + shape, then into the user's library beside their edits.""" + with SessionLocal() as db: + job = db.get(DownloadJob, job_id) + if job is None: + return + spec = dict(job.edit_spec or {}) + user_id = job.user_id + staging = _edit_staging_dir(asset_id) + try: + if staging.exists(): + shutil.rmtree(staging, ignore_errors=True) + staging.mkdir(parents=True, exist_ok=True) + src, start = _clip_source(job_id, asset_id, spec, staging) + data = probe.probe_json(src) or {} + audio_track = spec.get("audio_track") + if audio_track is not None and audio_track >= clipmod.audio_track_count(data): + raise StatedFailure("The chosen audio track is not in the file.") + total_s = clipmod.duration(spec) + out = staging / "clip.mp4" + _set_job(job_id, phase="editing", progress=0, speed_bps=None, eta_s=None) + editmod.run_ffmpeg( + clipmod.build_cmd( + src, + out, + start, + total_s, + audio_track=audio_track, + hdr=clipmod.is_hdr(data), + threads=editmod.encode_thread_cap(), + ), + total_s, + on_progress=_ffmpeg_progress_writer(job_id, "editing"), + should_cancel=_cancel_poll(job_id), + ) + if not out.exists(): + raise StatedFailure("ffmpeg produced no output file") + # The phone-safe shape by construction; still held to the rule every ready file meets. + out = _ensure_phone_safe(job_id, out, staging, "editing", round(total_s)) + ext = out.suffix.lstrip(".").lower() or "mp4" + rel = storage.edit_rel_path(user_id, asset_id, ext) + storage.place_file(out, settings.download_root, rel) + final = storage.abs_path(settings.download_root, rel) + vcodec, acodec, width, height, _ = probe.probe_media(final) + _, retention_days = _download_settings() + + with SessionLocal() as db: + asset = db.get(MediaAsset, asset_id) + if asset: + asset.status = "ready" + asset.rel_path = rel + asset.size_bytes = final.stat().st_size if final.exists() else None + asset.container = ext + asset.vcodec, asset.acodec, asset.width, asset.height = ( + vcodec, + acodec, + width, + height, + ) + asset.duration_s = round(total_s) + asset.media_kind = mediakind.derive(vcodec, acodec) + # The clip's own frame as its poster: the link preview shows the moment, and a Plex + # item has no public thumbnail at all. + asset.poster_path = editmod.ensure_poster(asset, settings.download_root) + asset.error = None + asset.last_access_at = datetime.now(timezone.utc) + asset.expires_at = datetime.now(timezone.utc) + timedelta(days=retention_days) + title = asset.title if asset else None + finalized = service.finalize_if_running( + db, job_id, status=JobStatus.DONE, progress=100, phase=None + ) + db.commit() + if finalized: + _notify_download_done(db, user_id, title, job_id, kind="edit") + log.info( + "clip job %s done: asset %s", + job_id, + asset_id, + extra=logs.event( + "clip.job.done", job_id=job_id, asset_id=asset_id, source=spec["source"] + ), + ) + except (_Aborted, editmod.EditAborted): + # Paused/canceled mid-fetch or mid-encode: free the asset for a later retry. + _reset_asset_pending(asset_id) + log.info( + "clip job %s aborted (pause/cancel)", + job_id, + extra=logs.event("clip.job.aborted", job_id=job_id), + ) + except _OwnerRefused as exc: + # Quota full: the job fails, the asset goes back to pending for a Retry once there is room + # (the download's handling; see `_download`). + msg = str(exc)[:500] + with SessionLocal() as db: + service.finalize_if_running( + db, + job_id, + status=JobStatus.ERROR, + error=msg, + error_code=dlerrors.classify(msg), + error_detail=None, + phase=None, + ) + db.commit() + _reset_asset_pending(asset_id) + log.info( + "clip job %s refused for its owner: %s", + job_id, + formats.log_safe(msg), + extra=logs.event("clip.job.refused", 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. + _fail_edit(job_id, asset_id, str(exc)[:500], exc, what="clip") + 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, diff --git a/backend/app/downloads/quota.py b/backend/app/downloads/quota.py index 66c92ef8..afde02ae 100644 --- a/backend/app/downloads/quota.py +++ b/backend/app/downloads/quota.py @@ -24,7 +24,7 @@ from sqlalchemy import and_, func, or_, select from sqlalchemy.orm import Session from app import sysconfig -from app.downloads.status import JobStatus +from app.downloads.status import JobKind, JobStatus from app.models import DownloadJob, DownloadQuota, MediaAsset # Jobs a user is considered to be "holding" (occupy a job slot / footprint). Terminal failures @@ -86,7 +86,7 @@ def footprint(db: Session, user_id: int) -> int: DownloadJob.user_id == user_id, or_( DownloadJob.status.in_(_HOLDING), - and_(DownloadJob.job_kind == "upload", DownloadJob.status == JobStatus.ERROR), + and_(DownloadJob.job_kind == JobKind.UPLOAD, DownloadJob.status == JobStatus.ERROR), ), DownloadJob.asset_id.is_not(None), ) diff --git a/backend/app/downloads/runner.py b/backend/app/downloads/runner.py index 654de3e8..9af835ef 100644 --- a/backend/app/downloads/runner.py +++ b/backend/app/downloads/runner.py @@ -22,6 +22,7 @@ from app.config import settings from app.db import SessionLocal from app.downloads import edit as editmod from app.downloads import formats +from app.downloads.status import JobKind from app.models import DownloadJob log = logs.get_logger(__name__) @@ -95,14 +96,16 @@ 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 _convert_upload, _download, _process_edit + from app.downloads.pipeline import _convert_upload, _download, _process_clip, _process_edit with formats.hiding(source_ref): try: - if job_kind == "edit": + if job_kind == JobKind.EDIT: _process_edit(job_id, asset_id) - elif job_kind == "upload": + elif job_kind == JobKind.UPLOAD: _convert_upload(job_id, asset_id) + elif job_kind == JobKind.CLIP: + _process_clip(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 22c2a019..f2c4ef63 100644 --- a/backend/app/downloads/service.py +++ b/backend/app/downloads/service.py @@ -16,11 +16,21 @@ from sqlalchemy.orm import Session from app.config import settings from app.downloads import adapters +from app.downloads import clip as clipmod from app.downloads import credentials from app.downloads import edit as editmod from app.downloads import formats, mediakind, quota, storage -from app.downloads.status import HOLDING, JobStatus -from app.models import DISPLAY_TEXT_MAX, Channel, DownloadJob, MediaAsset, Video +from app.downloads.status import HOLDING, JobKind, JobStatus +from app.models import ( + DISPLAY_TEXT_MAX, + Channel, + DownloadJob, + MediaAsset, + PlexItem, + PlexShow, + Video, +) +from app.plex import paths as plex_paths class EditError(Exception): @@ -40,10 +50,13 @@ def source_url(source_kind: str, source_ref: str) -> str: def reference_url(job, asset) -> str | None: """The clean "downloaded from" URL for a job — shared by the private Downloads page and the public watch page so both surfaces always agree. None for edit derivatives (a clip's source is the user's - own earlier download, not a web page); else the canonical page URL yt-dlp recorded; else one derived - from source_kind+source_ref (covers queued/older rows before the worker fills it).""" - if job is None or job.job_kind == "edit": + own earlier download, not a web page) and for a Plex clip (the library is not a public page); a + YouTube clip links its video at the clip's start; else the canonical page URL yt-dlp recorded; else + one derived from source_kind+source_ref (covers queued/older rows before the worker fills it).""" + if job is None or job.job_kind == JobKind.EDIT: return None + if job.job_kind == JobKind.CLIP: + return clipmod.reference_url(job.edit_spec or {}) if asset is not None and asset.source_webpage_url: return asset.source_webpage_url if job.source_kind in ("youtube", "url"): @@ -251,7 +264,7 @@ def enqueue_edit( job = DownloadJob( user_id=user_id, asset_id=asset.id, - job_kind="edit", + job_kind=JobKind.EDIT, source_kind="edit", source_ref=ref, source_job_id=source_job.id, @@ -275,6 +288,95 @@ def enqueue_edit( return job +def _clip_source_facts(db: Session, spec: dict) -> dict: + """Title, uploader, thumbnail and length of what a clip is cut from, read from our own catalog. + Raises ClipError when a Plex item is unknown or its file cannot be read here — said at enqueue, + rather than as a job that fails in the queue a minute later.""" + if spec["source"] == "youtube": + video = db.get(Video, spec["ref"]) + if video is None: # not in the catalog: the worker fills the title from yt-dlp's answer + return {"title": None, "uploader": None, "thumbnail_url": None, "length_s": None} + channel = db.get(Channel, video.channel_id) + return { + "title": video.title, + "uploader": channel.title if channel else None, + "thumbnail_url": video.thumbnail_url, + "length_s": video.duration_seconds, + } + item = db.execute( + select(PlexItem).where(PlexItem.rating_key == spec["ref"]) + ).scalar_one_or_none() + if item is None: + raise clipmod.ClipError("not_found") + if plex_paths.local_media_path(db, item.file_path) is None: + raise clipmod.ClipError("media_missing") + title = item.title + show = db.get(PlexShow, item.show_id) if item.show_id else None + if show is not None: + episode = ( + f"S{item.season_number}·E{item.episode_number}" + if item.season_number is not None and item.episode_number is not None + else None + ) + title = " — ".join(p for p in (show.title, episode, item.title) if p) + return { + "title": title, + "uploader": None, + # Plex art is served through our own authenticated routes, never a public URL; the worker + # cuts the clip's poster from the clip itself. + "thumbnail_url": None, + "length_s": item.duration_s, + } + + +def enqueue_clip( + db: Session, user_id: int, spec: dict, display_name: str | None = None +) -> DownloadJob: + """Queue an ad-hoc clip (`clipmod.normalize`d spec) for `user_id`: a NEW per-user asset cut + straight from the source — a YouTube range or a Plex library file (see downloads/clip). + + Raises ClipError (unknown or unreachable source, a range past its end) or quota.QuotaExceeded.""" + facts = _clip_source_facts(db, spec) + # One second of grace: a mark set on the last frame may round past the catalog's whole seconds. + if facts["length_s"] and spec["end_s"] > facts["length_s"] + 1: + raise clipmod.ClipError("past_end") + + quota.check_enqueue(db, user_id) + + ref = clipmod.asset_ref(user_id, spec)[:512] + asset = get_or_create_asset(db, "clip", ref, clipmod.sig(spec)) + if not asset.title: + asset.title = _clip_title(facts["title"], display_name) + asset.uploader = facts["uploader"] + asset.thumbnail_url = facts["thumbnail_url"] + asset.duration_s = round(clipmod.duration(spec)) + asset.media_kind = mediakind.VIDEO + + job = DownloadJob( + user_id=user_id, + asset_id=asset.id, + job_kind=JobKind.CLIP, + source_kind="clip", + source_ref=ref, + edit_spec=spec, + profile_snapshot={}, + display_name=display_name or asset.title[:DISPLAY_TEXT_MAX], + status="queued", + queue_pos=next_queue_pos(db), + ) + db.add(job) + incr_ref_count(db, asset.id) + + if asset.status == "ready": # the same range was already cut for this user → instant + job.status = "done" + job.progress = 100 + asset.last_access_at = datetime.now(timezone.utc) + + db.commit() + db.refresh(job) + return job + + def finalize_if_running(db: Session, job_id: int, **fields) -> bool: """Write terminal `fields` to a job in ONE atomic ``UPDATE ... WHERE status='running'`` and report whether it fired. This is the single owner of the invariant "a finishing worker child must never diff --git a/backend/app/downloads/status.py b/backend/app/downloads/status.py index 7ee2a05f..7849930e 100644 --- a/backend/app/downloads/status.py +++ b/backend/app/downloads/status.py @@ -33,6 +33,23 @@ HOLDING = frozenset( {JobStatus.QUEUED, JobStatus.RUNNING, JobStatus.PAUSED, JobStatus.DONE, JobStatus.ERROR} ) + +class JobKind(StrEnum): + """What a job produces, which decides the worker path that runs it and what the library may do + with the result (only a DOWNLOAD has a web source to fetch again).""" + + DOWNLOAD = "download" # yt-dlp fetches a source into a shared, deduplicated asset + EDIT = "edit" # a per-user trim/crop of a file already in the library + UPLOAD = "upload" # a file the user sent; the worker only converts it + CLIP = ( + "clip" # a per-user range of a YouTube video or a Plex item, cut straight from the source + ) + + +# A job kind with no web source to fetch again: Re-download refuses these. +NOT_REFETCHABLE = frozenset({JobKind.EDIT, JobKind.UPLOAD, JobKind.CLIP}) + + # States a route sets to STOP a running job. The worker's supervisor must step in (kill the child, free # the asset, leave the route's status intact) when it observes one of these on a job it is running. STOPPED_BY_ROUTE = frozenset({JobStatus.PAUSED, JobStatus.CANCELED}) diff --git a/backend/app/downloads/upload.py b/backend/app/downloads/upload.py index 1629a05e..7612c4d3 100644 --- a/backend/app/downloads/upload.py +++ b/backend/app/downloads/upload.py @@ -34,7 +34,7 @@ from sqlalchemy.orm import Session from app.config import settings from app.downloads import edit as editmod from app.downloads import mediakind, playable, probe, quota, service, storage -from app.downloads.status import JobStatus +from app.downloads.status import JobKind, JobStatus from app.models import DownloadJob, MediaAsset, display_text @@ -113,7 +113,7 @@ def ingest_upload( job = DownloadJob( user_id=user_id, asset_id=asset.id, - job_kind="upload", + job_kind=JobKind.UPLOAD, source_kind="upload", source_ref=ref, profile_snapshot={}, diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index 6dcc0784..2f513ef8 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -8,6 +8,7 @@ state, serves finished files (range-aware, with the user's custom display name), import os import re import tempfile +from typing import Literal from collections.abc import AsyncIterator, Sequence from dataclasses import dataclass from datetime import datetime, timedelta, timezone @@ -16,7 +17,7 @@ from urllib.parse import parse_qs, unquote, urlparse from fastapi import APIRouter, Depends, HTTPException, Request from fastapi.responses import FileResponse -from pydantic import BaseModel +from pydantic import BaseModel, Field from sqlalchemy import delete, func, select from sqlalchemy.dialects.postgresql import insert as pg_insert from sqlalchemy.orm import Session @@ -27,13 +28,14 @@ from app import audit, sysconfig from app.auth import admin_user, current_user from app.config import settings from app.db import get_db, release_read_connection +from app.downloads import clip as clipmod from app.downloads import credentials as dlcreds from app.downloads import edit as editmod 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.downloads.status import NOT_REFETCHABLE, JobStatus from app.models import ( DownloadJob, DownloadLink, @@ -576,6 +578,34 @@ def enqueue_edit( return _serialize_job(db, job) +class EnqueueClipIn(BaseModel): + source: Literal["youtube", "plex"] + # The YouTube video id or the Plex rating key. + ref: str = Field(max_length=32) + start_s: float + end_s: float + # Plex only: which of the file's audio tracks (0-based, in file order) — the one being heard. + audio_track: int | None = None + display_name: str | None = None + + +@router.post("/clip") +def enqueue_clip( + body: EnqueueClipIn, user: User = Depends(current_user), db: Session = Depends(get_db) +) -> dict: + """Queue an ad-hoc clip: the marked range of a YouTube video or a Plex item, cut straight from + the source into a new asset in this user's library (downloads/clip).""" + try: + spec = clipmod.normalize(body.source, body.ref, body.start_s, body.end_s, body.audio_track) + job = service.enqueue_clip(db, user.id, spec, display_text(body.display_name)) + except clipmod.ClipError as e: + status = 404 if e.reason == "not_found" else 422 + raise HTTPException(status_code=status, detail={"code": "clip", "reason": e.reason}) from e + except quota.QuotaExceeded as e: + raise HTTPException(status_code=422, detail=_quota_detail(e)) from e + return _serialize_job(db, job) + + def _quota_detail(e: quota.QuotaExceeded) -> dict: """Structured error the trilingual client localizes (reason key + numbers), instead of a pre-baked English string with a dot-decimal GB value. See lib/api.ts localizeDetail().""" @@ -768,9 +798,9 @@ def redownload_download( never deletes a file another job/user still shares: the old asset is only removed if THIS job was its last holder.""" job = _own_job_for_update(db, user, job_id) - if job.job_kind in ("edit", "upload"): - # 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. + if job.job_kind in NOT_REFETCHABLE: + # None has a re-fetchable source: an editor clip is derived locally, an upload IS the + # source, an ad-hoc clip is a range of a source the clip itself does not re-fetch. raise refused(409, "not_redownloadable") if job.status in (JobStatus.QUEUED, JobStatus.RUNNING, JobStatus.PAUSED): raise refused(409, "still_downloading") diff --git a/backend/tests/test_clip.py b/backend/tests/test_clip.py new file mode 100644 index 00000000..1913bb89 --- /dev/null +++ b/backend/tests/test_clip.py @@ -0,0 +1,423 @@ +"""Ad-hoc clips (downloads/clip): the spec, the one ffmpeg command every clip goes through, the route, +and the worker path for both sources. + +The ffmpeg command is judged at argv level: where the seek sits, which streams are mapped, what the +video filter does to an HDR source. The worker runs with ffmpeg / yt-dlp replaced by stand-ins that +write a file where they were told to, so what is checked is what the pipeline asks of them and what +it stores afterwards. +""" + +from pathlib import Path + +import pytest + +from app.config import settings +from app.downloads import clip as clipmod +from app.downloads import pipeline, quota, runner, service +from app.downloads import probe as probemod +from app.downloads.edit import INPUT_GUARD +from app.models import Channel, DownloadJob, MediaAsset, PlexItem, PlexLibrary, PlexShow, Video +from app.plex import paths as plex_paths + +# --- the spec ----------------------------------------------------------------------------------- + + +def test_a_spec_is_canonical_so_the_same_marks_hit_the_same_clip(): + a = clipmod.normalize("youtube", "abcdefghijk", 10.00049, 40.0) + b = clipmod.normalize("youtube", "abcdefghijk", 10.0, 40.0004) + assert a == b == {"source": "youtube", "ref": "abcdefghijk", "start_s": 10.0, "end_s": 40.0} + assert clipmod.sig(a) == clipmod.sig(b) + assert clipmod.sig(a) != clipmod.sig({**a, "end_s": 41.0}) + + +@pytest.mark.parametrize( + ("args", "reason"), + [ + (("vimeo", "abcdefghijk", 0, 10), "bad_source"), + (("youtube", "short", 0, 10), "bad_source"), + (("youtube", "abcdefghij/", 0, 10), "bad_source"), + (("plex", "12a", 0, 10), "bad_source"), + (("youtube", "abcdefghijk", -1, 10), "bad_range"), + (("youtube", "abcdefghijk", 0, float("nan")), "bad_range"), + (("youtube", "abcdefghijk", 5, 5.5), "too_short"), + (("youtube", "abcdefghijk", 30, 10), "too_short"), + (("youtube", "abcdefghijk", 0, 600.001), "too_long"), + ], +) +def test_a_range_it_cannot_cut_is_refused_with_its_reason(args, reason): + with pytest.raises(clipmod.ClipError) as e: + clipmod.normalize(*args) + assert e.value.reason == reason + + +def test_the_ten_minute_cap_itself_is_allowed(): + assert clipmod.duration(clipmod.normalize("plex", "42", 0, 600)) == 600 + + +def test_an_audio_track_is_kept_for_plex_only_and_must_be_sane(): + assert clipmod.normalize("plex", "42", 0, 10, 2)["audio_track"] == 2 + assert "audio_track" not in clipmod.normalize("youtube", "abcdefghijk", 0, 10, 2) + with pytest.raises(clipmod.ClipError, match="bad_audio"): + clipmod.normalize("plex", "42", 0, 10, -1) + + +def test_a_youtube_clip_links_its_video_at_the_clip_start_and_a_plex_clip_links_nothing(): + yt = clipmod.normalize("youtube", "abcdefghijk", 75.9, 90) + assert clipmod.reference_url(yt) == "https://www.youtube.com/watch?v=abcdefghijk&t=75s" + assert clipmod.reference_url(clipmod.normalize("plex", "42", 0, 10)) is None + + +# --- the ffmpeg command ---------------------------------------------------------------------------- + + +def _cmd(**kw): + args = {"audio_track": None, "hdr": False, "threads": 3, **kw} + return clipmod.build_cmd(Path("/m/in.mkv"), Path("/s/out.mp4"), 61.5, 30.0, **args) + + +def test_the_seek_is_on_the_input_so_the_clip_starts_on_the_marked_frame(): + cmd = _cmd() + i = cmd.index("-i") + assert cmd[cmd.index("-ss") + 1] == "61.500" and cmd.index("-ss") < i + assert cmd[i + 1] == "/m/in.mkv" + assert cmd[cmd.index("-t") + 1] == "30.000" and cmd.index("-t") > i + # The single-file demuxer allowlist goes with every library read. + g = cmd.index(INPUT_GUARD[0]) + assert cmd[g : g + len(INPUT_GUARD)] == INPUT_GUARD and g < i + assert cmd[-1] == "/s/out.mp4" + + +def test_it_maps_the_first_video_and_the_heard_audio_track_and_nothing_else(): + cmd = _cmd(audio_track=2) + maps = [cmd[i + 1] for i, a in enumerate(cmd) if a == "-map"] + assert maps == ["0:v:0", "0:a:2"] + assert "-sn" in cmd and "-dn" in cmd + # An mkv's chapters would ride into the .mp4 as a data track of their own (measured on a Blu-ray + # rip: `bin_data`), which -dn does not drop. + assert cmd[cmd.index("-map_chapters") + 1] == "-1" + # No track chosen: the first one, and a source with no audio at all still cuts. + maps = [c for i, c in enumerate(_cmd()) if i and _cmd()[i - 1] == "-map"] + assert maps == ["0:v:0", "0:a:0?"] + + +def test_the_output_is_phone_safe_stereo_h264_at_most_1080p(): + cmd = _cmd() + assert cmd[cmd.index("-c:v") + 1] == "libx264" + assert cmd[cmd.index("-pix_fmt") + 1] == "yuv420p" + assert cmd[cmd.index("-c:a") + 1] == "aac" and cmd[cmd.index("-ac") + 1] == "2" + assert cmd[cmd.index("-threads") + 1] == "3" + vf = cmd[cmd.index("-vf") + 1] + assert "min(1080,ih)" in vf and "tonemap" not in vf + + +def test_an_hdr_source_is_tonemapped_to_sdr_before_it_is_scaled(): + vf = _cmd(hdr=True)[_cmd(hdr=True).index("-vf") + 1] + assert vf.index("tonemap") < vf.index("zscale=t=bt709") < vf.index("min(1080,ih)") + + +@pytest.mark.parametrize( + ("transfer", "hdr"), + [("smpte2084", True), ("arib-std-b67", True), ("bt709", False), (None, False)], +) +def test_hdr_is_read_from_the_first_video_stream(transfer, hdr): + data = { + "streams": [{"codec_type": "audio"}, {"codec_type": "video", "color_transfer": transfer}] + } + assert clipmod.is_hdr(data) is hdr + + +# --- the route --------------------------------------------------------------------------------------- + + +@pytest.fixture +def yt_video(db): + db.add(Channel(id="UC_clip", title="The Channel")) + db.flush() + db.add( + Video( + id="abcdefghijk", + channel_id="UC_clip", + title="A Funny Video", + duration_seconds=300, + thumbnail_url="https://i.ytimg.com/vi/abcdefghijk/hq.jpg", + ) + ) + db.commit() + return "abcdefghijk" + + +@pytest.fixture +def plex_episode(db, tmp_path, monkeypatch): + lib = PlexLibrary(plex_key="2", title="Shows", kind="show", enabled=True) + db.add(lib) + db.flush() + show = PlexShow(rating_key="s9", library_id=lib.id, title="The Show") + db.add(show) + db.flush() + db.add( + PlexItem( + rating_key="777", + library_id=lib.id, + kind="episode", + title="Pilot", + show_id=show.id, + season_number=1, + episode_number=2, + duration_s=1800, + file_path="/data/shows/pilot.mkv", + ) + ) + db.commit() + media = tmp_path / "pilot.mkv" + media.write_bytes(b"m" * 8) + monkeypatch.setattr( + plex_paths, + "local_media_path", + lambda _db, p: media if p == "/data/shows/pilot.mkv" else None, + ) + return media + + +def _clip(client, **body): + return client.post("/api/downloads/clip", json={"start_s": 10, "end_s": 40, **body}) + + +def test_a_youtube_clip_is_queued_as_its_own_job_titled_after_its_video( + db, make_user, as_user, yt_video +): + r = _clip(as_user(make_user()), source="youtube", ref=yt_video) + assert r.status_code == 200, r.text + job = r.json() + assert (job["job_kind"], job["status"], job["display_name"]) == ( + "clip", + "queued", + "A Funny Video (clip)", + ) + assert job["edit_spec"] == {"source": "youtube", "ref": yt_video, "start_s": 10, "end_s": 40} + assert job["source_url"] == f"https://www.youtube.com/watch?v={yt_video}&t=10s" + asset = db.get(MediaAsset, job["asset"]["id"]) + assert (asset.duration_s, asset.uploader, asset.media_kind) == (30, "The Channel", "video") + + +def test_the_same_range_again_is_the_same_clip_but_never_another_users( + db, make_user, as_user, yt_video +): + a, b = make_user(), make_user() + first = _clip(as_user(a), source="youtube", ref=yt_video).json()["asset"]["id"] + again = _clip(as_user(a), source="youtube", ref=yt_video).json()["asset"]["id"] + other = _clip(as_user(b), source="youtube", ref=yt_video).json()["asset"]["id"] + longer = _clip(as_user(a), source="youtube", ref=yt_video, end_s=41).json()["asset"]["id"] + assert first == again + assert len({first, other, longer}) == 3 + + +def test_a_ready_clip_asked_for_again_is_done_at_once(db, make_user, as_user, yt_video): + u = make_user() + first = _clip(as_user(u), source="youtube", ref=yt_video).json() + asset = db.get(MediaAsset, first["asset"]["id"]) + asset.status, asset.rel_path = "ready", ".edits/1/clip_1.mp4" + db.commit() + again = _clip(as_user(u), source="youtube", ref=yt_video).json() + assert (again["status"], again["progress"]) == ("done", 100) + + +@pytest.mark.parametrize( + ("body", "status", "reason"), + [ + ({"end_s": 700}, 422, "too_long"), + ({"start_s": 299.5, "end_s": 330}, 422, "past_end"), + ({"ref": "nope"}, 422, "bad_source"), + ], +) +def test_a_youtube_clip_it_cannot_cut_says_why( + db, make_user, as_user, yt_video, body, status, reason +): + r = _clip(as_user(make_user()), **{"source": "youtube", "ref": yt_video, **body}) + assert r.status_code == status + assert r.json()["detail"] == {"code": "clip", "reason": reason} + assert db.query(DownloadJob).count() == 0 + + +def test_a_plex_clip_is_titled_show_episode_title_and_keeps_its_audio_track( + db, make_user, as_user, plex_episode +): + r = _clip(as_user(make_user()), source="plex", ref="777", audio_track=1) + assert r.status_code == 200, r.text + job = r.json() + assert job["display_name"] == "The Show — S1·E2 — Pilot (clip)" + assert job["edit_spec"]["audio_track"] == 1 + assert job["source_url"] is None + + +def test_a_plex_clip_of_an_unknown_item_is_a_404_and_of_an_unreachable_file_a_422( + db, make_user, as_user, plex_episode +): + c = as_user(make_user()) + assert _clip(c, source="plex", ref="999").json()["detail"]["reason"] == "not_found" + assert _clip(c, source="plex", ref="999").status_code == 404 + item = db.query(PlexItem).one() + item.file_path = "/data/elsewhere.mkv" + db.commit() + r = _clip(c, source="plex", ref="777") + assert (r.status_code, r.json()["detail"]["reason"]) == (422, "media_missing") + + +def test_a_clip_over_quota_is_refused_like_a_download( + db, make_user, as_user, yt_video, monkeypatch +): + def full(_db, _uid): + raise quota.QuotaExceeded("max_jobs", limit=5, current=5) + + monkeypatch.setattr(quota, "check_enqueue", full) + r = _clip(as_user(make_user()), source="youtube", ref=yt_video) + assert r.status_code == 422 + assert db.query(DownloadJob).count() == 0 + + +def test_a_clip_is_not_redownloadable(db, make_user, as_user, yt_video): + c = as_user(make_user()) + job = _clip(c, source="youtube", ref=yt_video).json() + row = db.get(DownloadJob, job["id"]) + row.status = "done" + db.commit() + r = c.post(f"/api/downloads/{job['id']}/redownload") + assert (r.status_code, r.json()["detail"]["reason"]) == (409, "not_redownloadable") + + +# --- the worker ---------------------------------------------------------------------------------------- + + +def test_a_clip_job_reaches_the_clip_entry(monkeypatch): + called = [] + for name in ("_convert_upload", "_process_edit", "_download", "_process_clip"): + monkeypatch.setattr(pipeline, name, lambda *a, _n=name: called.append(_n)) + runner._run(5, 7, "clip", "clip", "1:youtube:abcdefghijk", {}, 1) + assert called == ["_process_clip"] + + +@pytest.fixture +def worker(db, tmp_path, monkeypatch): + """The worker path with its external programs replaced: ffmpeg writes the file it was given, the + probe answers what the test says, the phone-safe step and the poster pass through.""" + monkeypatch.setattr(settings, "download_root", str(tmp_path / "root")) + seen: dict = {} + + def ffmpeg(cmd, *_a, **_k): + seen["cmd"] = cmd + Path(cmd[-1]).write_bytes(b"clip") + + monkeypatch.setattr(pipeline.editmod, "run_ffmpeg", ffmpeg) + monkeypatch.setattr(pipeline, "_ensure_phone_safe", lambda _j, p, *_a, **_k: p) + monkeypatch.setattr(pipeline.editmod, "ensure_poster", lambda *_a, **_k: None) + monkeypatch.setattr(probemod, "probe_media", lambda _p: ("h264", "aac", 1920, 1080, 30)) + seen["probe"] = {"streams": [{"codec_type": "video"}, {"codec_type": "audio"}]} + monkeypatch.setattr(probemod, "probe_json", lambda _p: seen["probe"]) + + def run(job_json): + job = db.get(DownloadJob, job_json["id"]) + asset = db.get(MediaAsset, job.asset_id) + job.status, asset.status = "running", "downloading" + db.commit() + pipeline._process_clip(job.id, asset.id) + db.expire_all() + return db.get(DownloadJob, job.id), db.get(MediaAsset, asset.id) + + seen["run"] = run + return seen + + +def test_a_plex_clip_is_cut_from_the_library_file_with_its_audio_track( + db, make_user, as_user, plex_episode, worker +): + worker["probe"] = { + "streams": [ + {"codec_type": "video", "color_transfer": "smpte2084"}, + {"codec_type": "audio"}, + {"codec_type": "audio"}, + ] + } + job = _clip(as_user(make_user()), source="plex", ref="777", audio_track=1).json() + job, asset = worker["run"](job) + assert (job.status, asset.status, asset.container, asset.height) == ( + "done", + "ready", + "mp4", + 1080, + ) + cmd = worker["cmd"] + assert cmd[cmd.index("-i") + 1] == str(plex_episode) + assert cmd[cmd.index("-ss") + 1] == "10.000" + assert "0:a:1" in cmd and "tonemap" in cmd[cmd.index("-vf") + 1] + assert asset.rel_path.startswith(".edits/") and asset.rel_path.endswith(".mp4") + + +def test_a_plex_clip_of_a_track_the_file_does_not_have_fails_and_says_so( + db, make_user, as_user, plex_episode, worker +): + job = _clip(as_user(make_user()), source="plex", ref="777", audio_track=3).json() + job, asset = worker["run"](job) + assert (job.status, asset.status) == ("error", "error") + assert "audio track" in job.error + assert "cmd" not in worker + + +def test_a_youtube_clip_fetches_only_its_range_and_cuts_it_from_zero( + db, make_user, as_user, yt_video, worker, monkeypatch +): + asked: dict = {} + + def extract(job_id, asset_id, url, spec, staging, adapter, **kw): + asked.update(url=url, spec=spec, **kw) + f = Path(staging) / "abcdefghijk.mp4" + f.write_bytes(b"range") + return {"requested_downloads": [{"filepath": str(f)}]} + + monkeypatch.setattr(pipeline, "_extract_with_fallback", extract) + job = _clip(as_user(make_user()), source="youtube", ref=yt_video).json() + job, asset = worker["run"](job) + assert (job.status, asset.status) == ("done", "ready") + assert asked["section"] == (10, 40) + assert asked["spec"]["max_height"] == 1080 + assert asked["url"].endswith(yt_video) + cmd = worker["cmd"] + assert cmd[cmd.index("-ss") + 1] == "0.000" and cmd[cmd.index("-t") + 1] == "30.000" + assert cmd[cmd.index("-i") + 1].endswith("abcdefghijk.mp4") + + +def test_a_youtube_clip_hands_the_range_to_yt_dlp(monkeypatch, tmp_path): + seen = {} + + class FakeYDL: + def __init__(self, opts): + seen.update(opts) + + def __enter__(self): + return self + + def __exit__(self, *_a): + return False + + def extract_info(self, *_a, **_k): + return {} + + monkeypatch.setattr(pipeline.yt_dlp, "YoutubeDL", FakeYDL) + monkeypatch.setattr(pipeline, "_touch_job", lambda *_a: None) + adapter = pipeline.adapters.resolve_adapter("youtube", "abcdefghijk") + pipeline._extract_with_fallback( + 1, + 2, + "https://www.youtube.com/watch?v=abcdefghijk", + pipeline.formats.normalize({}), + tmp_path, + adapter, + section=(10.0, 40.0), + ) + ranges = list(seen["download_ranges"]({}, None)) + assert [(r["start_time"], r["end_time"]) for r in ranges] == [(10.0, 40.0)] + assert seen["force_keyframes_at_cuts"] is False + + +def test_service_reference_url_follows_the_clip_spec(db, make_user, as_user, yt_video): + job = _clip(as_user(make_user()), source="youtube", ref=yt_video, start_s=65, end_s=90).json() + row = db.get(DownloadJob, job["id"]) + assert service.reference_url(row, None) == f"https://www.youtube.com/watch?v={yt_video}&t=65s" diff --git a/backend/tests/test_phone_safe_contract.py b/backend/tests/test_phone_safe_contract.py index 50505f73..a15e1476 100644 --- a/backend/tests/test_phone_safe_contract.py +++ b/backend/tests/test_phone_safe_contract.py @@ -73,10 +73,17 @@ def test_every_path_to_ready_runs_the_rule(): 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. + # The paths known today: a download, an editor clip, an ad-hoc 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 { + "_download", + "_process_edit", + "_process_clip", + "_convert_upload", + "ingest_upload", + } <= names, found assert skipped == [], f"marks an asset ready without the phone-safe rule: {skipped}" diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index f7360911..088e5f96 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -558,8 +558,8 @@ def _display_write_sites() -> dict[tuple[str, str], set[str]]: # Bounded by `display_text` unless noted: # service.redownload CLEARS the field so the fresh metadata wins — `None` is the # one value that needs no bound, and a clear is still a write. -# service.enqueue / enqueue_edit `display_name` is already bounded by its caller; the fallback -# slices the YouTube title to the same width. +# service.enqueue / enqueue_edit / `display_name` is already bounded by its caller; the fallback +# enqueue_clip slices the source's title to the same width. # downloads.upload_download passes through `spool_upload`'s value, bounded there. # # Held as EXACT EQUALITY rather than as a list of offenders. The offender form could only fail @@ -573,6 +573,7 @@ DISPLAY_WRITE_SITES = { "if asset.status == 'ready' and asset.title else None)" }, ("downloads/service.py", "enqueue_edit"): {"display_name or asset.title[:DISPLAY_TEXT_MAX]"}, + ("downloads/service.py", "enqueue_clip"): {"display_name or asset.title[:DISPLAY_TEXT_MAX]"}, ("downloads/upload.py", "ingest_upload"): {"display_text(display_name)"}, ("routes/auth.py", "callback"): {"display_text(userinfo.get('name'))"}, ("routes/auth.py", "_complete_link"): {"display_text(userinfo.get('name'))"}, diff --git a/frontend/src/components/downloadRow.tsx b/frontend/src/components/downloadRow.tsx index 7cc4689b..a9d4f3c0 100644 --- a/frontend/src/components/downloadRow.tsx +++ b/frontend/src/components/downloadRow.tsx @@ -268,7 +268,7 @@ export function DownloadRow({
- {job.job_kind === "edit" && ( + {(job.job_kind === "edit" || job.job_kind === "clip") && ( {t("downloads.clipBadge")} diff --git a/frontend/src/components/useDownloadActions.tsx b/frontend/src/components/useDownloadActions.tsx index d0696acf..d035d12c 100644 --- a/frontend/src/components/useDownloadActions.tsx +++ b/frontend/src/components/useDownloadActions.tsx @@ -59,6 +59,7 @@ const CANCEL_COPY: Record = { download: CANCEL_DOWNLOAD, upload: CANCEL_UPLOAD, edit: CANCEL_EDIT, + clip: CANCEL_EDIT, // both are a clip being cut: the same words }; // Deleting a download may leave its shared cache file for other holders. An upload and a clip are // their owner's alone (a clip's asset is keyed by its user), so deleting one removes the file for good. @@ -69,6 +70,7 @@ const DELETE_COPY: Record = { body: "downloads.confirm.deleteUploadBody", }, edit: { title: "downloads.confirm.deleteEditTitle", body: "downloads.confirm.deleteEditBody" }, + clip: { title: "downloads.confirm.deleteEditTitle", body: "downloads.confirm.deleteEditBody" }, }; export function useDownloadActions({ diff --git a/frontend/src/lib/api/downloads.ts b/frontend/src/lib/api/downloads.ts index aad414c6..01dcd8e4 100644 --- a/frontend/src/lib/api/downloads.ts +++ b/frontend/src/lib/api/downloads.ts @@ -44,10 +44,10 @@ interface DownloadAsset { } export type DownloadStatus = "queued" | "running" | "paused" | "done" | "error" | "canceled"; -// "download" (default), "edit" (a per-user trim/crop derivative cut from another job), or "upload" -// (a locally-uploaded file — no re-fetchable source, so re-download is rejected backend-side). -// Local (not exported): consumers compare `job.job_kind` against literals, never name the type. -export type DownloadJobKind = "download" | "edit" | "upload"; +// "download" (default), "edit" (a per-user trim/crop derivative cut from another job), "upload" +// (a locally-uploaded file) or "clip" (a marked range of a YouTube video or a Plex item, cut straight +// from the source). Only a download has a re-fetchable source; re-download refuses the rest. +export type DownloadJobKind = "download" | "edit" | "upload" | "clip"; // Phase-2 editor recipe. A single `trim` (one output file) or a `segments` cut-list joined into // one file. `accurate` re-encodes for a frame-accurate cut (a crop always re-encodes). diff --git a/frontend/src/lib/downloadActions.test.ts b/frontend/src/lib/downloadActions.test.ts index f638f195..2abe0a60 100644 --- a/frontend/src/lib/downloadActions.test.ts +++ b/frontend/src/lib/downloadActions.test.ts @@ -52,6 +52,7 @@ describe("libraryRowActions", () => { it("an edit clip cannot be re-downloaded", () => { expect(libraryRowActions(job({ job_kind: "edit" }, {})).redownload).toBe(false); + expect(libraryRowActions(job({ job_kind: "clip" }, {})).redownload).toBe(false); }); it("edit is hidden when the media has no duration", () => { diff --git a/frontend/src/lib/downloadActions.ts b/frontend/src/lib/downloadActions.ts index 2c97b761..022b37eb 100644 --- a/frontend/src/lib/downloadActions.ts +++ b/frontend/src/lib/downloadActions.ts @@ -22,7 +22,7 @@ export function libraryRowActions(job: DownloadJob): LibraryRowActions { play: media, edit: media && job.can_download && hasDuration(job), editDetails: media, - redownload: job.job_kind !== "edit" && job.job_kind !== "upload", + redownload: !job.job_kind || job.job_kind === "download", }; } diff --git a/frontend/src/lib/downloadFailure.test.ts b/frontend/src/lib/downloadFailure.test.ts index 8eba0bc3..0adcff5f 100644 --- a/frontend/src/lib/downloadFailure.test.ts +++ b/frontend/src/lib/downloadFailure.test.ts @@ -53,6 +53,7 @@ describe("failTitleKey", () => { await i18n.changeLanguage("en"); expect(failTitleKey({ job_kind: "upload" })).toBe("downloads.errors.fail.titleUpload"); expect(failTitleKey({ job_kind: "edit" })).toBe("downloads.errors.fail.titleEdit"); + expect(failTitleKey({ job_kind: "clip" })).toBe("downloads.errors.fail.titleEdit"); expect(failTitleKey({ job_kind: "download" })).toBe("downloads.errors.fail.title"); expect(failTitleKey({})).toBe("downloads.errors.fail.title"); for (const lng of ["en", "hu"]) { @@ -106,6 +107,15 @@ describe("failTitleKey", () => { ]); }); + it("a clip from YouTube or Plex is worded as the editor's clip", () => { + for (const part of ["short", "body"] as const) + expect( + failCopyKeys({ error_code: "unknown", error_detail: null, job_kind: "clip" }, part), + ).toEqual( + failCopyKeys({ error_code: "unknown", error_detail: null, job_kind: "edit" }, part), + ); + }); + it("an upload's own failures read as an upload's", async () => { await i18n.changeLanguage("en"); for (const code of ["convert_failed", "upload_missing"]) { diff --git a/frontend/src/lib/downloadFailure.ts b/frontend/src/lib/downloadFailure.ts index 72f3a4d5..5ab6c1cc 100644 --- a/frontend/src/lib/downloadFailure.ts +++ b/frontend/src/lib/downloadFailure.ts @@ -13,7 +13,8 @@ export function failCopyKeys( job: Pick, part: "short" | "body", ): string[] { - const kind = job.job_kind ?? "download"; + // A clip from YouTube or Plex is worded as the editor's clip: both are "making the clip". + const kind = job.job_kind === "clip" ? "edit" : (job.job_kind ?? "download"); const category = (code: string) => [ ...(kind === "download" ? [] : [`downloads.errors.fail.${kind}.${code}.${part}`]), `downloads.errors.fail.cat.${code}.${part}`, @@ -30,6 +31,7 @@ const TITLE_KEYS: Record = { download: "downloads.errors.fail.title", upload: "downloads.errors.fail.titleUpload", // its failure is a conversion, not a download edit: "downloads.errors.fail.titleEdit", // a clip being cut + clip: "downloads.errors.fail.titleEdit", // a clip being cut, from YouTube or Plex }; /** The title over a failed job's explanation, worded for what the job was making. */