diff --git a/VERSION b/VERSION index e2050de8..7c6425a5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.62.1 +0.62.2 \ No newline at end of file diff --git a/backend/app/downloads/edit.py b/backend/app/downloads/edit.py index 99235aea..ed671a47 100644 --- a/backend/app/downloads/edit.py +++ b/backend/app/downloads/edit.py @@ -25,6 +25,7 @@ import json import logging import math import queue +import shutil import subprocess import threading import time @@ -489,6 +490,61 @@ def build_poster_cmd(src: Path, dest: Path, at_s: float) -> list[str]: ] +def build_thumbnail_jpg_cmd(src: Path, dest: Path) -> list[str]: + """Decode a downloaded thumbnail (webp/png/…) to a single baseline JPEG frame.""" + return [ + "ffmpeg", "-y", "-hide_banner", "-loglevel", "error", "-nostdin", + "-i", str(src), "-frames:v", "1", "-update", "1", str(dest), + ] + + +def _run_ffmpeg_best_effort(cmd: list[str], timeout_s: int) -> Exception | None: + """Run a one-shot ffmpeg command, bounded by `timeout_s`. Returns None on success, or the + exception on a timeout/non-zero-exit/OS error — never raises. Shared by the best-effort image + steps (poster / thumbnail / storyboard) so the run+timeout+swallow policy lives in ONE place; + each caller logs its own context from the returned exception. (The download/transcode path uses + the richer `run_ffmpeg` instead, which streams progress and honours cooperative cancel.)""" + try: + subprocess.run(cmd, capture_output=True, timeout=timeout_s, check=True) + return None + except (subprocess.TimeoutExpired, subprocess.CalledProcessError, OSError) as exc: + return exc + + +def _is_jpeg(path: Path) -> bool: + """True if the file starts with the JPEG SOI marker (FF D8 FF).""" + try: + with path.open("rb") as f: + return f.read(3) == b"\xff\xd8\xff" + except OSError: + return False + + +def thumbnail_to_jpg(src: Path | None, dest: Path, timeout_s: int = 30) -> Path | None: + """Turn a downloaded thumbnail (webp / png / jpeg — whatever the source shipped) into a real + baseline JPEG for the `.jpg` poster + og:image share sidecar. + + Deliberately NOT yt-dlp's `FFmpegThumbnailsConvertor` (whose `-bsf:v mjpeg2jpeg` path crashes on + some already-JPEG thumbnails, e.g. Reddit's, and used to fail the entire download). A thumbnail + that is already JPEG is copied as-is; anything else is decoded via ffmpeg. Best-effort: returns + None on any failure, so the caller falls back to a cut video frame (`ensure_poster`) and the + download still succeeds.""" + if src is None or not src.exists(): + return None + if _is_jpeg(src): # already JPEG (e.g. Facebook) — no decode/re-encode needed + try: + shutil.copyfile(src, dest) + return dest + except OSError as exc: + log.info("thumbnail copy failed (%s); poster falls back to a cut frame", str(exc)[:120]) + return None + err = _run_ffmpeg_best_effort(build_thumbnail_jpg_cmd(src, dest), timeout_s) + if err is not None: + log.info("thumbnail->jpg conversion failed (%s); poster falls back to a cut frame", str(err)[:120]) + return None + return dest if dest.exists() else None + + def ensure_poster(asset, download_root: str, timeout_s: int = 30) -> str | None: """Generate a poster frame for a ready asset that has no thumbnail (e.g. a direct media URL like a reddit HLS manifest, which yt-dlp can't get a thumbnail for). Written as the media's @@ -506,10 +562,9 @@ def ensure_poster(asset, download_root: str, timeout_s: int = 30) -> str | None: if dest.exists(): return rel at = min(3.0, (asset.duration_s or 10) * 0.1) - try: - subprocess.run(build_poster_cmd(src, dest, at), capture_output=True, timeout=timeout_s, check=True) - except (subprocess.TimeoutExpired, subprocess.CalledProcessError, OSError) as exc: - log.info("poster gen skipped for asset %s: %s", asset.id, str(exc)[:120]) + err = _run_ffmpeg_best_effort(build_poster_cmd(src, dest, at), timeout_s) + if err is not None: + log.info("poster gen skipped for asset %s: %s", asset.id, str(err)[:120]) return None return rel if dest.exists() else None @@ -538,10 +593,9 @@ def ensure_storyboard(asset, download_root: str, timeout_s: int = 90) -> dict | return None dest.parent.mkdir(parents=True, exist_ok=True) cmd = build_storyboard_cmd(src, dest, asset.duration_s, geom) - try: - subprocess.run(cmd, capture_output=True, timeout=timeout_s, check=True) - except (subprocess.TimeoutExpired, subprocess.CalledProcessError, OSError) as exc: - log.info("storyboard gen skipped for asset %s: %s", asset.id, str(exc)[:120]) + err = _run_ffmpeg_best_effort(cmd, timeout_s) + if err is not None: + log.info("storyboard gen skipped for asset %s: %s", asset.id, str(err)[:120]) return None if not dest.exists(): return None diff --git a/backend/app/downloads/errors.py b/backend/app/downloads/errors.py index ae57b56b..68947a8a 100644 --- a/backend/app/downloads/errors.py +++ b/backend/app/downloads/errors.py @@ -21,6 +21,9 @@ _RULES: tuple[tuple[str, str], ...] = ( ("available to this channel's members", "login_required"), ("join this channel", "login_required"), ("this video is only available to", "login_required"), + # Some extractors (e.g. Reddit) gate content behind a logged-in account: "Account + # authentication is required." — an auth wall we deliberately don't cross, not a transient error. + ("authentication is required", "login_required"), # Geo-restricted. ("in your country", "geo_blocked"), ("available in your location", "geo_blocked"), diff --git a/backend/app/downloads/formats.py b/backend/app/downloads/formats.py index 5170b39e..662c85d9 100644 --- a/backend/app/downloads/formats.py +++ b/backend/app/downloads/formats.py @@ -211,9 +211,16 @@ def build_ydl_opts( opts["postprocessors"].append( {"key": "FFmpegMetadata", "add_chapters": True, "add_metadata": True} ) - # Normalize the written thumbnail to jpg so we always have a `.jpg` poster sidecar. - # `already_have_thumbnail` keeps that file on disk after embedding (default deletes it). - opts["postprocessors"].append({"key": "FFmpegThumbnailsConvertor", "format": "jpg"}) + # NOTE: we deliberately do NOT add yt-dlp's `FFmpegThumbnailsConvertor` here. Its jpg conversion + # runs `ffmpeg -bsf:v mjpeg2jpeg` when the written thumbnail is already JPEG bytes (Reddit serves + # a JPEG under a `.png` name), and that bitstream filter *fails* on some such images — and since a + # postprocessor error is fatal, it used to kill the WHOLE download even though the media was fine. + # The `.jpg` poster sidecar is instead produced robustly by our own pipeline (worker + # `_download` → `editmod.thumbnail_to_jpg`, with `ensure_poster` cutting a video frame as a + # fallback), so the og:image share preview (Facebook Messenger / MS Teams) keeps working. + # The opt-in EmbedThumbnail below is unaffected: it does its OWN format handling — it embeds a + # jpg/png thumbnail directly and converts anything else to PNG (not the crashing mjpeg2jpeg-to-jpg + # path), so it never needed the convertor and does not reintroduce the crash. if s["embed_thumbnail"]: opts["postprocessors"].append( {"key": "EmbedThumbnail", "already_have_thumbnail": True} diff --git a/backend/app/titles.py b/backend/app/titles.py index 83746636..1ebd2a42 100644 --- a/backend/app/titles.py +++ b/backend/app/titles.py @@ -34,7 +34,15 @@ _ENGAGEMENT_PREFIX = re.compile( re.IGNORECASE, ) _LETTER_RUN = re.compile(r"[^\W\d_]+", re.UNICODE) -_TRAILING_HASHTAGS = re.compile(r"(?:\s+#[^\s#]*[^\W\d\s_][^\s#]*)+\s*$", re.UNICODE) +# Trailing SEO hashtag cluster: a run of whitespace-separated `#tags` at the very end, where each +# tag contains at least one letter (a bare numeric "#3" episode marker is NOT a hashtag and +# survives). The "has a letter" test is a zero-width lookahead and the tag body is matched +# possessively (`*+`), so there is exactly ONE way to split each tag and nothing to backtrack over. +# The previous form `[^\s#]*[letter][^\s#]*` had TWO overlapping unbounded quantifiers around the +# required letter: on a long hashtag run that ends with trailing non-tag text (a Facebook +# "… #a #b #c | Page" title), the `$` anchor fails and the engine backtracks catastrophically +# (ReDoS) — it pinned a worker thread at 100% CPU forever inside the download progress hook. +_TRAILING_HASHTAGS = re.compile(r"(?:\s+#(?=[^\s#]*[^\W\d\s_])[^\s#]*+)+\s*$", re.UNICODE) _REPEAT_PUNCT = re.compile(r"([!?.,\-])\1{1,}") # Short function words → lowercased when the title is de-shouted (proper title-case style). diff --git a/backend/app/worker.py b/backend/app/worker.py index c2949104..0364703e 100644 --- a/backend/app/worker.py +++ b/backend/app/worker.py @@ -617,8 +617,12 @@ def _download(job_id: int, asset_id: int, source_kind: str, source_ref: str, spe layout, retention_days = _download_settings() rel = storage.rel_path(meta, ext, layout, asset_id=asset_id) thumb = _find_thumbnail(staging, produced) + # Convert the raw thumbnail (webp/png/jpeg) to a real JPEG ourselves — robustly — for the + # poster + og:image sidecar, now that yt-dlp's fatal thumbnail convertor is gone. On failure + # this is None and write_sidecars writes no poster, so ensure_poster cuts a frame instead. + poster_jpg = editmod.thumbnail_to_jpg(thumb, staging / "poster.jpg") storage.place_file(produced, settings.download_root, rel) - nfo_ok = storage.write_sidecars(settings.download_root, rel, meta, thumb) + nfo_ok = storage.write_sidecars(settings.download_root, rel, meta, poster_jpg) final = storage.abs_path(settings.download_root, rel) size = final.stat().st_size if final.exists() else None diff --git a/backend/tests/test_download_errors.py b/backend/tests/test_download_errors.py index 85013f5c..20e0d6c2 100644 --- a/backend/tests/test_download_errors.py +++ b/backend/tests/test_download_errors.py @@ -12,6 +12,7 @@ def test_real_ytdlp_messages_map_to_categories(): "ERROR: [youtube] abc: Sign in to confirm your age. This video may be inappropriate": "login_required", "ERROR: [youtube] abc: Sign in to confirm you're not a bot": "login_required", "ERROR: [youtube] abc: Join this channel to get access to members-only content": "login_required", + "ERROR: [Reddit] 1vi1x56: Account authentication is required.": "login_required", "ERROR: [youtube] abc: The uploader has not made this video available in your country": "geo_blocked", "ERROR: [youtube] abc: Requested format is not available": "format_unavailable", "ERROR: [youtube] abc: Only images are available for download": "format_unavailable", diff --git a/backend/tests/test_thumbnail_poster.py b/backend/tests/test_thumbnail_poster.py new file mode 100644 index 00000000..679aca35 --- /dev/null +++ b/backend/tests/test_thumbnail_poster.py @@ -0,0 +1,46 @@ +"""`thumbnail_to_jpg`: robustly turn a downloaded thumbnail into a real JPEG poster/og:image. + +This replaced yt-dlp's `FFmpegThumbnailsConvertor`, whose `-bsf:v mjpeg2jpeg` path crashed on some +already-JPEG thumbnails (Reddit serves a JPEG under a `.png` name) — and because a postprocessor +error is fatal, it used to fail the WHOLE download. The key regression property here is that a bad +thumbnail returns None (poster falls back to a cut frame) instead of raising. +""" +import shutil +import subprocess +from pathlib import Path + +import pytest + +from app.downloads import edit + +_ffmpeg_missing = shutil.which("ffmpeg") is None +needs_ffmpeg = pytest.mark.skipif(_ffmpeg_missing, reason="ffmpeg not available") + + +def _is_jpeg(p: Path) -> bool: + return p.exists() and p.read_bytes()[:3] == b"\xff\xd8\xff" + + +@needs_ffmpeg +def test_converts_a_png_to_a_real_jpeg(tmp_path): + src = tmp_path / "thumb.png" + subprocess.run( + ["ffmpeg", "-y", "-loglevel", "error", "-f", "lavfi", + "-i", "color=c=red:s=64x64", "-frames:v", "1", str(src)], + check=True, + ) + out = edit.thumbnail_to_jpg(src, tmp_path / "poster.jpg") + assert out is not None and _is_jpeg(out) + + +def test_returns_none_for_a_missing_source(tmp_path): + assert edit.thumbnail_to_jpg(None, tmp_path / "p.jpg") is None + assert edit.thumbnail_to_jpg(tmp_path / "nope.png", tmp_path / "p.jpg") is None + + +@needs_ffmpeg +def test_returns_none_on_garbage_input_instead_of_raising(tmp_path): + # The whole point: a thumbnail ffmpeg can't decode must NOT crash the download. + bad = tmp_path / "bad.png" + bad.write_bytes(b"this is definitely not an image") + assert edit.thumbnail_to_jpg(bad, tmp_path / "p.jpg") is None diff --git a/backend/tests/test_titles.py b/backend/tests/test_titles.py index 3c082830..5d80bd48 100644 --- a/backend/tests/test_titles.py +++ b/backend/tests/test_titles.py @@ -1,4 +1,9 @@ """normalize_title: de-shout / de-clutter video titles without mangling non-English text.""" +from concurrent.futures import ThreadPoolExecutor +from concurrent.futures import TimeoutError as FuturesTimeout + +import pytest + from app.titles import normalize_title @@ -24,6 +29,35 @@ def test_trailing_hashtags_are_stripped(): assert normalize_title("Great cooking video #food #yummy") == "Great cooking video" +def test_numeric_episode_marker_is_not_treated_as_a_hashtag(): + # A bare "#3" is an episode marker, not an SEO hashtag (no letters) → it must survive. + assert normalize_title("My show #3") == "My show #3" + + +def test_midstring_hashtags_dont_hang_and_are_kept_redos_regression(): + # Facebook prepends an engagement prefix and appends a page name, leaving a long hashtag cluster + # in the MIDDLE. That is not a trailing SEO cluster, so the tags stay — and, critically, this + # exact shape used to trigger catastrophic backtracking (ReDoS) in `_TRAILING_HASHTAGS` that + # pinned a download-worker thread at 100% CPU forever inside the yt-dlp progress hook. + raw = ( + "398K views · 6.1K reactions | The money should follow 🙄 #reel #reels #meme #memes " + "#fun #funny #work #worklife #office #viral #trending #fyp #foryou #foryoupage | Memebonanza4U" + ) + # Run in a worker thread with a generous ceiling: the fixed regex returns in microseconds, so + # only a genuine catastrophic-backtracking regression fails to complete — a hang is caught + # deterministically rather than by asserting on wall-clock slack in the happy path. + ex = ThreadPoolExecutor(max_workers=1) + fut = ex.submit(normalize_title, raw) + try: + out = fut.result(timeout=5) or "" + except FuturesTimeout: + pytest.fail("normalize_title did not return within 5s — catastrophic backtracking regressed") + finally: + ex.shutdown(wait=False) # never join a (hypothetically) wedged thread + assert "#foryoupage" in out and "#reel" in out # mid-string hashtags preserved (not trailing) + assert "views" not in out # the engagement-count prefix was still stripped + + def test_emoji_and_symbols_are_dropped(): assert "🔥" not in (normalize_title("Fire recipe 🔥🔥") or "") diff --git a/frontend/src/lib/releaseNotes.ts b/frontend/src/lib/releaseNotes.ts index 51cad597..4578aacc 100644 --- a/frontend/src/lib/releaseNotes.ts +++ b/frontend/src/lib/releaseNotes.ts @@ -14,6 +14,14 @@ export interface ReleaseEntry { } export const RELEASE_NOTES: ReleaseEntry[] = [ + { + version: "0.62.2", + date: "2026-08-12", + fixes: [ + "Fixed a download that could get stuck and freeze the whole download queue — a video whose title had a long run of hashtags (common on Facebook reels) would hang and stop everything else from downloading. The queue keeps moving now.", + "Reddit links that failed right at the end while preparing the thumbnail now download correctly, and link-share previews (the thumbnail shown in Messenger / Teams) are generated more reliably.", + ], + }, { version: "0.62.1", date: "2026-08-12",