diff --git a/.env.example b/.env.example index bde007a..40b8d40 100644 --- a/.env.example +++ b/.env.example @@ -2,7 +2,8 @@ # --- Host data path (Syncthing share root) ------------------------------ # Prod default (compose.yml): /home/frank/sync.schwenk.online/data/livef12 -# Layout under that path: incoming/ variants/ intermediates/ meta/ +# Layout under that path: incoming/ variants/ intermediates/ meta/ webcache/ +# (webcache = resized WebP for srcset; safe to wipe / add to phone .stignore) # Local override when the absolute path does not exist: # DATA_HOST_DIR=./data # DATA_HOST_DIR=/home/frank/sync.schwenk.online/data/livef12 diff --git a/.gitignore b/.gitignore index f5aae13..5e4e33b 100644 --- a/.gitignore +++ b/.gitignore @@ -4,13 +4,6 @@ # Runtime data (bind-mounted / Syncthing share — never in git) /data/ -# Local rembg/withoutbg comparison scratch -/compare-bg/inputs/ -/compare-bg/outputs/ -/compare-bg/model-cache/ -/compare-bg/compare.log -.venv-compare/ - # Python __pycache__/ *.pyc diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d2e24d4..e4b5f87 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -50,18 +50,18 @@ The gmic filter/blend chain is ported from an external reference script `make_random.py` (not in this repo). `config.POST_FILTERS` and asset lists under `assets/` mirror that script's behaviour. -## Background removal choice +## Background removal -`compare-bg/` is a **standalone benchmark** (rembg models vs withoutbg). -It is not part of the runtime stack. Production uses `u2net` + alpha -matting via `app/rembg_cli.py` (see `config.REMBG_MODEL`). +Production uses `u2net` + alpha matting via `app/rembg_cli.py` +(see `config.REMBG_MODEL`). ## Known limitations (intentional) - **No web auth** — anyone with the link sees all jobs (event tool). - **Sequential worker** — one photo at a time; burst uploads queue. -- **Meta JSON** — worker and web both read-modify-write `meta/*.json` - without file locking; concurrent remix during active processing can race. +- **Meta JSON** — worker and web share `meta/*.json` via locked + read-modify-write (`fcntl.flock`); variants are merged by id so + concurrent remix during processing does not clobber entries. - **Incoming is append-only** — worker never deletes from `incoming/`. ## Dev / test diff --git a/CLEANUP_PLAN.md b/CLEANUP_PLAN.md index 5eeceec..7545350 100644 --- a/CLEANUP_PLAN.md +++ b/CLEANUP_PLAN.md @@ -21,7 +21,7 @@ Phase 0 analysis (2026-07-18). Working branch: `cleanup/code-quality`. 4. **Dead code**: `blend_layers()` unused 5. **Duplication**: atomic JSON write ×3, `basicConfig` ×2, bool-env parsing ×2, compose env block ×2 6. **Double meta read** on index page (`read_status` + `read_manifest`) -7. **`compare-bg/`** — separate experiment, leave as-is (gitignored cache) +7. ~~`compare-bg/`~~ — removed (was a standalone experiment) 8. **`make_random.py`** — external reference only, document in ARCHITECTURE ## Risk matrix @@ -63,6 +63,6 @@ Phase 0 analysis (2026-07-18). Working branch: `cleanup/code-quality`. - No `pipeline.py` module split in this pass - No meta file locking / concurrency fix - No dependency major version bumps -- No changes to `compare-bg/` experiment +- ~~No changes to `compare-bg/` experiment~~ (later removed entirely) - No deploy CI (manual `docker compose` on boka stays) - No behavior changes to gmic/rembg processing logic diff --git a/CLEANUP_REPORT.md b/CLEANUP_REPORT.md index a9ddefe..dcb59ea 100644 --- a/CLEANUP_REPORT.md +++ b/CLEANUP_REPORT.md @@ -31,19 +31,21 @@ behavior changes** to the photo pipeline, worker loop, or web UX. | Item | Reason | |------|--------| | `pipeline.py` module split | Medium risk without broader integration tests; deferred | -| Meta file locking | Behavior change; documented in `ARCHITECTURE.md` | | Dependency major bumps | Per plan — list only, no auto-bump | -| `compare-bg/` | Separate experiment; left untouched | | Deploy CI | Manual deploy on boka stays; STANDARDS says ask first | -| FastAPI `TemplateResponse` API migration | Deprecation warning only; no functional change | -| `_validate_job_id` always sanitizes to valid ID | Pre-existing; documented in tests | -## Bugs noticed (not fixed) +## Follow-ups (later branch) -1. **Meta JSON race** — worker and web concurrent RMW without locking. -2. **`_filter_timeout_override` global** — thread-unsafe under concurrent remix (uvicorn workers). -3. **`sanitize_stem("a/b")` → `"b"`** — `Path.stem` drops path prefix before slash replacement. -4. **`_validate_job_id`** — sanitization makes rejection path effectively unreachable. +Meta locking, timeout thread-local, TemplateResponse migration, stem/validate +hardening, and removal of `compare-bg/` were done on a subsequent branch +(`fix/concurrency-and-cleanup`). + +## Bugs noticed (historical — fixed later) + +1. **Meta JSON race** — fixed with `fcntl.flock` + variant merge-by-id. +2. **`_filter_timeout_override` global** — fixed with `threading.local`. +3. **`sanitize_stem("a/b")` → `"b"`** — fixed (separators replaced before `Path.stem`). +4. **`_validate_job_id`** — rejection path restored for path-like raw IDs. ## Dependency notes (suggestions only) @@ -84,6 +86,5 @@ docs: update README and add ARCHITECTURE ## External references -- `compare-bg/` — local venv `.venv-compare/` and 1.5GB model cache (gitignored) - `make_random.py` — external reference script, not vendored - Submodule/symlink: none diff --git a/app/config.py b/app/config.py index 1a0a0ad..c816677 100644 --- a/app/config.py +++ b/app/config.py @@ -34,6 +34,8 @@ INTERMEDIATES_DIR = DATA_DIR / "intermediates" # Web bookkeeping (status + manifest). Not needed on the phone — ignore via # Syncthing .stignore if desired. META_DIR = DATA_DIR / "meta" +# Resized WebP cache for srcset (web container only; safe to wipe / .stignore). +WEB_CACHE_DIR = DATA_DIR / "webcache" ASSETS_DIR = DATA_DIR / "assets" PROCESSED_FILE = DATA_DIR / "processed.json" diff --git a/app/images.py b/app/images.py new file mode 100644 index 0000000..ca9ba4f --- /dev/null +++ b/app/images.py @@ -0,0 +1,89 @@ +"""On-demand web image resizing with disk cache (srcset helpers).""" + +from __future__ import annotations + +import logging +import tempfile +from pathlib import Path +from urllib.parse import quote + +from PIL import Image + +from . import config + +logger = logging.getLogger("livef12.images") + +# Allowed ?w= values for /jobs/.../files/... — keep in sync with templates. +SRCSET_WIDTHS: tuple[int, ...] = (320, 640, 960, 1280) + +# Default sizes for the 1/2/4-column job + variant grids. +GRID_SIZES = "(min-width: 960px) 25vw, (min-width: 640px) 50vw, 100vw" +# Original/rembg pair is always two columns. +PAIR_SIZES = "(min-width: 640px) 50vw, 50vw" + + +def file_url(job_id: str, filename: str, width: int | None = None) -> str: + base = f"/jobs/{quote(job_id, safe='')}/files/{quote(filename, safe='')}" + if width is None: + return base + return f"{base}?w={width}" + + +def srcset_for(job_id: str, filename: str) -> str: + return ", ".join(f"{file_url(job_id, filename, w)} {w}w" for w in SRCSET_WIDTHS) + + +def img_attrs(job_id: str, filename: str, *, sizes: str = GRID_SIZES) -> dict[str, str]: + """Attrs for responsive : src, srcset, sizes, full_src (lightbox).""" + return { + "src": file_url(job_id, filename, 640), + "srcset": srcset_for(job_id, filename), + "sizes": sizes, + # Lightbox: large webp, not the multi-MB master PNG/JPEG. + "full_src": file_url(job_id, filename, SRCSET_WIDTHS[-1]), + } + + +def get_or_create_resized(source: Path, width: int) -> Path: + """Return a cached WebP whose longest edge is at most `width` (no upscale).""" + if width not in SRCSET_WIDTHS: + raise ValueError(f"unsupported width: {width}") + if not source.is_file(): + raise FileNotFoundError(source) + + config.WEB_CACHE_DIR.mkdir(parents=True, exist_ok=True) + cache_path = config.WEB_CACHE_DIR / f"{source.name}.w{width}.webp" + try: + if cache_path.is_file() and cache_path.stat().st_mtime >= source.stat().st_mtime: + return cache_path + except OSError: + pass + + _write_resized(source, cache_path, width) + return cache_path + + +def _write_resized(source: Path, dest: Path, width: int) -> None: + with Image.open(source) as img: + img.load() + if img.mode == "P": + img = img.convert("RGBA" if "transparency" in img.info else "RGB") + elif img.mode not in ("RGB", "RGBA"): + img = img.convert("RGB") + + if img.width > width: + new_h = max(1, round(img.height * (width / img.width))) + img = img.resize((width, new_h), Image.Resampling.LANCZOS) + + dest.parent.mkdir(parents=True, exist_ok=True) + with tempfile.NamedTemporaryFile( + dir=dest.parent, suffix=".webp", delete=False + ) as tmp: + tmp_path = Path(tmp.name) + try: + img.save(tmp_path, format="WEBP", quality=78, method=4) + tmp_path.replace(dest) + except Exception: + tmp_path.unlink(missing_ok=True) + raise + logger.debug("cached %s -> %s (%dx%d)", source.name, dest.name, img.width, img.height) diff --git a/app/io_utils.py b/app/io_utils.py index 79e0c23..07bebb8 100644 --- a/app/io_utils.py +++ b/app/io_utils.py @@ -2,7 +2,11 @@ from __future__ import annotations +import fcntl import json +import os +from collections.abc import Iterator +from contextlib import contextmanager from pathlib import Path from typing import Any @@ -23,3 +27,16 @@ def write_json_atomic(path: Path, data: Any) -> None: tmp = path.with_suffix(path.suffix + ".tmp") tmp.write_text(json.dumps(data, indent=2, ensure_ascii=False), encoding="utf-8") tmp.replace(path) + + +@contextmanager +def file_lock(lock_path: Path) -> Iterator[None]: + """Exclusive advisory lock via ``fcntl.flock`` (cross-process).""" + lock_path.parent.mkdir(parents=True, exist_ok=True) + fd = os.open(lock_path, os.O_CREAT | os.O_RDWR, 0o644) + try: + fcntl.flock(fd, fcntl.LOCK_EX) + yield + finally: + fcntl.flock(fd, fcntl.LOCK_UN) + os.close(fd) diff --git a/app/main.py b/app/main.py index c20d1c0..f35e356 100644 --- a/app/main.py +++ b/app/main.py @@ -13,7 +13,7 @@ from fastapi.responses import FileResponse, HTMLResponse, RedirectResponse from fastapi.staticfiles import StaticFiles from fastapi.templating import Jinja2Templates -from . import config, pipeline, remix +from . import config, images, pipeline, remix from .logging_config import configure_logging configure_logging() @@ -23,6 +23,9 @@ app = FastAPI(title=config.SITE_TITLE) BASE_DIR = Path(__file__).resolve().parent templates = Jinja2Templates(directory=str(BASE_DIR / "templates")) +templates.env.globals["img_attrs"] = images.img_attrs +templates.env.globals["PAIR_SIZES"] = images.PAIR_SIZES +templates.env.globals["GRID_SIZES"] = images.GRID_SIZES app.mount("/static", StaticFiles(directory=str(BASE_DIR / "static")), name="static") # Job id = sanitized source stem (may include dots). @@ -30,7 +33,10 @@ _JOB_ID_RE = re.compile(r"^[A-Za-z0-9._-]+$") def _validate_job_id(job_id: str) -> str: - stem = pipeline.sanitize_stem(job_id) + raw = job_id or "" + if not raw.strip() or "/" in raw or "\\" in raw or ".." in raw: + raise HTTPException(status_code=400, detail="Ungueltige Job-ID") + stem = pipeline.sanitize_stem(raw) if not _JOB_ID_RE.match(stem): raise HTTPException(status_code=400, detail="Ungueltige Job-ID") return stem @@ -82,8 +88,9 @@ def list_jobs() -> list[dict[str, Any]]: @app.get("/", response_class=HTMLResponse) def index(request: Request) -> HTMLResponse: return templates.TemplateResponse( + request, "index.html", - {"request": request, "jobs": list_jobs(), "site_title": config.SITE_TITLE}, + {"jobs": list_jobs(), "site_title": config.SITE_TITLE}, ) @@ -100,9 +107,9 @@ def job_detail(request: Request, job_id: str) -> HTMLResponse: ) return templates.TemplateResponse( + request, "job.html", { - "request": request, "site_title": config.SITE_TITLE, "job_id": stem, "status": status, @@ -116,9 +123,25 @@ def job_detail(request: Request, job_id: str) -> HTMLResponse: @app.get("/jobs/{job_id}/files/{filename:path}") -def job_file(job_id: str, filename: str) -> FileResponse: +def job_file( + job_id: str, + filename: str, + w: int | None = Query(None, description="Longest-edge width for srcset WebP"), +) -> FileResponse: path = _safe_job_file(job_id, filename) - return FileResponse(path) + if w is None: + return FileResponse(path) + if w not in images.SRCSET_WIDTHS: + raise HTTPException( + status_code=400, + detail=f"Ungueltige Breite (erlaubt: {', '.join(map(str, images.SRCSET_WIDTHS))})", + ) + try: + cached = images.get_or_create_resized(path, w) + except OSError as exc: + logger.warning("resize failed for %s w=%s: %s", path.name, w, exc) + raise HTTPException(status_code=500, detail="Bild konnte nicht skaliert werden") from exc + return FileResponse(cached, media_type="image/webp", filename=cached.name) @app.get("/jobs/{job_id}/remix", response_class=HTMLResponse) @@ -166,9 +189,9 @@ def remix_form( } return templates.TemplateResponse( + request, "remix.html", { - "request": request, "site_title": config.SITE_TITLE, "job_id": stem, "options": options, diff --git a/app/pipeline.py b/app/pipeline.py index 8bcc736..576706a 100644 --- a/app/pipeline.py +++ b/app/pipeline.py @@ -15,6 +15,7 @@ import re import shutil import subprocess import sys +import threading import time import uuid from collections.abc import Iterator @@ -25,14 +26,14 @@ from pathlib import Path from typing import Any from . import config -from .io_utils import read_json, write_json_atomic +from .io_utils import file_lock, read_json, write_json_atomic logger = logging.getLogger("livef12.pipeline") _ANSI_RE = re.compile(r"\x1b\[[0-9;]*m") -# Optional override for run_gmic timeout (used by the long-retry pass). -_filter_timeout_override: int | None = None +# Per-thread override for run_gmic timeout (used by the long-retry pass). +_filter_timeout_state = threading.local() class PipelineError(RuntimeError): @@ -69,19 +70,19 @@ def _nice(cmd: list[str]) -> list[str]: def _active_filter_timeout() -> int: - return _filter_timeout_override if _filter_timeout_override is not None else config.FILTER_TIMEOUT + override = getattr(_filter_timeout_state, "override", None) + return override if override is not None else config.FILTER_TIMEOUT @contextmanager def filter_timeout(seconds: int) -> Iterator[None]: """Temporarily override FILTER_TIMEOUT for run_gmic (long-retry pass).""" - global _filter_timeout_override - previous = _filter_timeout_override - _filter_timeout_override = seconds + previous = getattr(_filter_timeout_state, "override", None) + _filter_timeout_state.override = seconds try: yield finally: - _filter_timeout_override = previous + _filter_timeout_state.override = previous def _magick_bin() -> str: @@ -306,8 +307,10 @@ _UNSAFE_STEM_RE = re.compile(r"[^\w.\-]+", re.UNICODE) def sanitize_stem(name: str) -> str: """Safe basename stem for flat files (no path separators / junk).""" - stem = Path(name).stem if name else "" - stem = stem.replace("/", "_").replace("\\", "_").strip().strip(".") + # Replace separators before Path.stem so "a/b" → "a_b", not "b". + stem = (name or "").replace("/", "_").replace("\\", "_") + stem = Path(stem).stem + stem = stem.strip().strip(".") stem = _UNSAFE_STEM_RE.sub("_", stem).strip("._") return stem or "photo" @@ -354,11 +357,39 @@ def read_meta(stem: str) -> dict[str, Any] | None: return data if isinstance(data, dict) else None -def _write_meta(paths: JobPaths, data: dict[str, Any]) -> None: +def _meta_lock_path(stem: str) -> Path: + return config.META_DIR / f"{sanitize_stem(stem)}.lock" + + +@contextmanager +def meta_lock(stem: str) -> Iterator[None]: + """Exclusive lock for meta/{stem}.json read-modify-write.""" + with file_lock(_meta_lock_path(stem)): + yield + + +def _write_meta_unlocked(paths: JobPaths, data: dict[str, Any]) -> None: data["updated_at"] = now_iso() write_json_atomic(paths.meta, data) +def _merge_variants(existing: list[Any], incoming: list[Any]) -> list[dict[str, Any]]: + """Union variants by id; incoming wins on conflict; preserve discovery order.""" + by_id: dict[str, dict[str, Any]] = {} + order: list[str] = [] + for group in (existing, incoming): + for item in group: + if not isinstance(item, dict): + continue + vid = item.get("id") + if not isinstance(vid, str) or not vid: + continue + if vid not in by_id: + order.append(vid) + by_id[vid] = item + return [by_id[vid] for vid in order] + + def read_status(job_id: str) -> dict[str, Any] | None: data = read_meta(job_id) if not data: @@ -375,13 +406,14 @@ def read_status(job_id: str) -> dict[str, Any] | None: def write_status(paths: JobPaths, status: str, **extra: Any) -> None: - data = read_meta(paths.stem) or {} - data["job_id"] = paths.stem - data["source_stem"] = paths.stem - data["status"] = status - data.setdefault("created_at", now_iso()) - data.update(extra) - _write_meta(paths, data) + with meta_lock(paths.stem): + data = read_meta(paths.stem) or {} + data["job_id"] = paths.stem + data["source_stem"] = paths.stem + data["status"] = status + data.setdefault("created_at", now_iso()) + data.update(extra) + _write_meta_unlocked(paths, data) def read_manifest(job_id: str) -> dict[str, Any] | None: @@ -400,16 +432,52 @@ def read_manifest(job_id: str) -> dict[str, Any] | None: def write_manifest(paths: JobPaths, manifest: dict[str, Any]) -> None: - data = read_meta(paths.stem) or {} - data["job_id"] = paths.stem - data["source_stem"] = manifest.get("source_stem") or paths.stem - data["original_file"] = manifest.get("original_file") - data["rembg_file"] = manifest.get("rembg_file") - data["variants"] = manifest.get("variants") or [] - if "created_at" in manifest: - data.setdefault("created_at", manifest["created_at"]) - data.setdefault("status", data.get("status", "processing")) - _write_meta(paths, data) + """Upsert manifest fields; merge variants by id so concurrent writers do not clobber.""" + with meta_lock(paths.stem): + data = read_meta(paths.stem) or {} + data["job_id"] = paths.stem + if manifest.get("source_stem"): + data["source_stem"] = manifest["source_stem"] + else: + data.setdefault("source_stem", paths.stem) + if "original_file" in manifest and manifest["original_file"] is not None: + data["original_file"] = manifest["original_file"] + if "rembg_file" in manifest and manifest["rembg_file"] is not None: + data["rembg_file"] = manifest["rembg_file"] + data["variants"] = _merge_variants(data.get("variants") or [], manifest.get("variants") or []) + if "created_at" in manifest: + data.setdefault("created_at", manifest["created_at"]) + data.setdefault("status", data.get("status", "processing")) + _write_meta_unlocked(paths, data) + + +def append_manifest_variant( + paths: JobPaths, + entry: dict[str, Any], + *, + original_file: str | None = None, + rembg_file: str | None = None, + source_stem: str | None = None, + created_at: str | None = None, +) -> None: + """Append one variant under meta lock (re-reads disk so concurrent updates survive).""" + with meta_lock(paths.stem): + data = read_meta(paths.stem) or {} + data["job_id"] = paths.stem + if source_stem: + data["source_stem"] = source_stem + else: + data.setdefault("source_stem", paths.stem) + if original_file: + data["original_file"] = original_file + if rembg_file: + data["rembg_file"] = rembg_file + if created_at: + data.setdefault("created_at", created_at) + data.setdefault("created_at", now_iso()) + data.setdefault("status", data.get("status", "done")) + data["variants"] = _merge_variants(data.get("variants") or [], [entry]) + _write_meta_unlocked(paths, data) def list_job_ids() -> list[str]: diff --git a/app/remix.py b/app/remix.py index a55e177..6d52533 100644 --- a/app/remix.py +++ b/app/remix.py @@ -3,6 +3,7 @@ cached original + rembg output and user-chosen filters/blends/opacity.""" from __future__ import annotations +import uuid from dataclasses import dataclass from . import pipeline @@ -52,8 +53,8 @@ def create_remix_variant(job_id: str, choice: RemixChoice) -> dict: "variants": [], } variant_stem = pipeline.resolve_source_stem(manifest, paths) - manifest.setdefault("source_stem", variant_stem) - variant_id = pipeline.next_variant_id(manifest) + # Unique id avoids collisions when two remixes run in parallel. + variant_id = f"v{uuid.uuid4().hex[:8]}" entry = pipeline.compose_variant( paths, @@ -67,6 +68,12 @@ def create_remix_variant(job_id: str, choice: RemixChoice) -> dict: fg_mode=choice.fg_blend, opacity=choice.opacity, ) - manifest["variants"].append(entry) - pipeline.write_manifest(paths, manifest) + pipeline.append_manifest_variant( + paths, + entry, + original_file=paths.original.name, + rembg_file=paths.rembg.name, + source_stem=variant_stem, + created_at=manifest.get("created_at") if isinstance(manifest.get("created_at"), str) else None, + ) return entry diff --git a/app/templates/base.html b/app/templates/base.html index 0154cab..b1864c3 100644 --- a/app/templates/base.html +++ b/app/templates/base.html @@ -44,7 +44,8 @@ if (!target.classList.contains("lightboxable")) return; e.preventDefault(); e.stopPropagation(); - openLightbox(target.currentSrc || target.src, target.alt); + var full = target.getAttribute("data-full-src"); + openLightbox(full || target.currentSrc || target.src, target.alt); }); box.addEventListener("click", function (e) { diff --git a/app/templates/index.html b/app/templates/index.html index 2b791a5..5bd9d50 100644 --- a/app/templates/index.html +++ b/app/templates/index.html @@ -9,7 +9,15 @@
  • {% if job.thumbnail %} - Variante von Job {{ job.job_id }} + {% set img = img_attrs(job.job_id, job.thumbnail) %} + Variante von Job {{ job.job_id }} {% else %} {{ job.status }} {% endif %} diff --git a/app/templates/job.html b/app/templates/job.html index 5bcb780..4fa8aa7 100644 --- a/app/templates/job.html +++ b/app/templates/job.html @@ -13,8 +13,16 @@

    Original & Rembg

    {% if original_name %} + {% set img = img_attrs(job_id, original_name, sizes=PAIR_SIZES) %}
    - Original + Original
    Original · Download @@ -22,8 +30,16 @@
    {% endif %} {% if has_rembg and rembg_name %} + {% set img = img_attrs(job_id, rembg_name, sizes=PAIR_SIZES) %}
    - Freigestellt (rembg) + Freigestellt (rembg)
    Rembg · Download @@ -45,8 +61,16 @@ {% else %}
      {% for v in manifest.variants %} + {% set img = img_attrs(job_id, v.file) %}
    • - Variante {{ v.id }} + Variante {{ v.id }}
      {{ v.id }} {{ v.source }}
      @@ -72,8 +96,16 @@

      Zwischenschritte

        {% for name in intermediates %} + {% set img = img_attrs(job_id, name) %}
      • - {{ name }} + {{ name }} {{ name }}
      • {% endfor %} diff --git a/compare-bg/README.md b/compare-bg/README.md deleted file mode 100644 index d0027eb..0000000 --- a/compare-bg/README.md +++ /dev/null @@ -1,30 +0,0 @@ -# Background-removal comparison - -Inbox originals from `live.f12.rocks` vs. backends. - -## Layout - -- `inputs/` — originals from boka inbox -- `outputs/` — results, naming: - -``` -__rembg-u2net__no-alpha.png -__rembg-u2net__alpha.png -__rembg-default__no-alpha.png -__rembg-default__alpha.png -__rembg-birefnet-general__no-alpha.png -__rembg-birefnet-general__alpha.png -__withoutbg-open-weights.png -``` - -- `rembg-default` = `rembg.remove()` without session (library default = u2net) -- `rembg-u2net` = explicit `new_session("u2net")` — expect identical to default -- `withoutbg` open-weights includes matting in-graph (~2GB RAM claimed) - -## Re-run - -```bash -./run.sh -``` - -Skips existing non-empty outputs. Models cached in `model-cache/`. diff --git a/compare-bg/run.sh b/compare-bg/run.sh deleted file mode 100755 index 86b6818..0000000 --- a/compare-bg/run.sh +++ /dev/null @@ -1,28 +0,0 @@ -#!/usr/bin/env bash -# Run rembg/withoutbg comparison in Python 3.11 (host is 3.14-only). -set -euo pipefail -ROOT="$(cd "$(dirname "$0")" && pwd)" -IMG="livef12-bg-compare:py311" -LOG="$ROOT/compare.log" - -docker build -t "$IMG" - <<'EOF' -FROM python:3.11-slim-bookworm -RUN apt-get update && apt-get install -y --no-install-recommends libgl1 libglib2.0-0 \ - && rm -rf /var/lib/apt/lists/* -RUN pip install --no-cache-dir 'rembg>=2.0.59,<2.1' 'pillow>=10.4,<11' \ - 'onnxruntime>=1.19,<1.20' withoutbg -WORKDIR /work -EOF - -echo "Starting comparison; log: $LOG" -docker run --rm \ - --name livef12-bg-compare \ - -e TQDM_DISABLE=1 \ - -e COMPARE_INPUT=/data/inputs \ - -e COMPARE_OUTPUT=/data/outputs \ - -e HOME=/cache \ - -v "$ROOT/inputs:/data/inputs:ro" \ - -v "$ROOT/outputs:/data/outputs" \ - -v "$ROOT/run_compare.py:/work/run_compare.py:ro" \ - -v "$ROOT/model-cache:/cache" \ - "$IMG" python -u /work/run_compare.py 2>&1 | tee "$LOG" diff --git a/compare-bg/run_compare.py b/compare-bg/run_compare.py deleted file mode 100644 index 96f4e32..0000000 --- a/compare-bg/run_compare.py +++ /dev/null @@ -1,115 +0,0 @@ -#!/usr/bin/env python3 -"""Compare background-removal backends on the live.f12.rocks inbox uploads. - -Outputs land next to each other with explicit names so Fränky can pick a winner: - - __rembg-u2net__no-alpha.png - __rembg-u2net__alpha.png - __rembg-default__no-alpha.png - __rembg-default__alpha.png - __rembg-birefnet-general__no-alpha.png - __rembg-birefnet-general__alpha.png - __withoutbg-open-weights.png - -`rembg-default` = rembg.remove() with no session (library default model). -`rembg-u2net` = rembg.new_session("u2net") explicitly. -withoutbg embeds matting in its open-weights graph — one output only. -""" - -from __future__ import annotations - -import os -import sys -import time -import traceback -from pathlib import Path - -os.environ.setdefault("TQDM_DISABLE", "1") - -INPUT_DIR = Path(os.environ.get("COMPARE_INPUT", "/data/inputs")) -OUTPUT_DIR = Path(os.environ.get("COMPARE_OUTPUT", "/data/outputs")) -EXTS = {".jpg", ".jpeg", ".png", ".webp"} - - -def log(msg: str) -> None: - print(msg, flush=True) - - -def run_rembg_named(data: bytes, label: str, model: str | None, alpha: bool, out: Path) -> None: - from rembg import new_session, remove - - t0 = time.monotonic() - kwargs: dict = {"alpha_matting": alpha} - if model is None: - result = remove(data, **kwargs) - model_note = "default" - else: - session = new_session(model) - result = remove(data, session=session, **kwargs) - model_note = model - out.write_bytes(result) - log(f" OK {label} model={model_note} alpha={alpha} {time.monotonic()-t0:.1f}s -> {out.name} ({out.stat().st_size} bytes)") - - -def run_withoutbg(path: Path, out: Path) -> None: - from withoutbg import WithoutBG - - t0 = time.monotonic() - model = WithoutBG.open_weights() - result = model.remove_background(str(path)) - result.save(out) - log(f" OK withoutbg-open-weights {time.monotonic()-t0:.1f}s -> {out.name} ({out.stat().st_size} bytes)") - - -def process_one(path: Path) -> None: - stem = path.stem - log(f"=== {path.name} ===") - data = path.read_bytes() - jobs = [ - ("rembg-u2net", "u2net", False), - ("rembg-u2net", "u2net", True), - ("rembg-default", None, False), - ("rembg-default", None, True), - ("rembg-birefnet-general", "birefnet-general", False), - ("rembg-birefnet-general", "birefnet-general", True), - ] - for label, model, alpha in jobs: - tag = "alpha" if alpha else "no-alpha" - out = OUTPUT_DIR / f"{stem}__{label}__{tag}.png" - if out.exists() and out.stat().st_size > 0: - log(f" SKIP {out.name} (exists)") - continue - try: - run_rembg_named(data, label, model, alpha, out) - except Exception as exc: # noqa: BLE001 - err = OUTPUT_DIR / f"{stem}__{label}__{tag}.ERROR.txt" - err.write_text(f"{type(exc).__name__}: {exc}\n\n{traceback.format_exc()}", encoding="utf-8") - log(f" FAIL {label} alpha={alpha}: {type(exc).__name__}: {exc}") - - out_w = OUTPUT_DIR / f"{stem}__withoutbg-open-weights.png" - if out_w.exists() and out_w.stat().st_size > 0: - log(f" SKIP {out_w.name} (exists)") - else: - try: - run_withoutbg(path, out_w) - except Exception as exc: # noqa: BLE001 - err = OUTPUT_DIR / f"{stem}__withoutbg-open-weights.ERROR.txt" - err.write_text(f"{type(exc).__name__}: {exc}\n\n{traceback.format_exc()}", encoding="utf-8") - log(f" FAIL withoutbg: {type(exc).__name__}: {exc}") - - -def main() -> int: - OUTPUT_DIR.mkdir(parents=True, exist_ok=True) - inputs = sorted(p for p in INPUT_DIR.iterdir() if p.is_file() and p.suffix.lower() in EXTS) - if not inputs: - log(f"No images in {INPUT_DIR}") - return 1 - log(f"Comparing {len(inputs)} image(s); outputs -> {OUTPUT_DIR}") - for path in inputs: - process_one(path) - log("DONE") - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/tests/conftest.py b/tests/conftest.py index 5ccea05..96b5fc3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,7 +14,8 @@ def data_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: intermediates = tmp_path / "intermediates" meta = tmp_path / "meta" inbox = tmp_path / "incoming" - for directory in (variants, intermediates, meta, inbox): + webcache = tmp_path / "webcache" + for directory in (variants, intermediates, meta, inbox, webcache): directory.mkdir() monkeypatch.setattr("app.config.DATA_DIR", tmp_path) @@ -22,5 +23,6 @@ def data_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: monkeypatch.setattr("app.config.INTERMEDIATES_DIR", intermediates) monkeypatch.setattr("app.config.META_DIR", meta) monkeypatch.setattr("app.config.INBOX_DIR", inbox) + monkeypatch.setattr("app.config.WEB_CACHE_DIR", webcache) monkeypatch.setattr("app.config.PROCESSED_FILE", tmp_path / "processed.json") return tmp_path diff --git a/tests/test_io_utils.py b/tests/test_io_utils.py index e99c58d..faecc7a 100644 --- a/tests/test_io_utils.py +++ b/tests/test_io_utils.py @@ -24,3 +24,21 @@ def test_write_json_atomic_roundtrip(tmp_path: Path) -> None: write_json_atomic(path, payload) assert json.loads(path.read_text(encoding="utf-8")) == payload assert not path.with_suffix(path.suffix + ".tmp").exists() + + +def test_file_lock_exclusive(tmp_path: Path) -> None: + from concurrent.futures import ThreadPoolExecutor + + from app.io_utils import file_lock + + lock_path = tmp_path / "job.lock" + counter = {"n": 0} + + def bump() -> None: + with file_lock(lock_path): + current = counter["n"] + counter["n"] = current + 1 + + with ThreadPoolExecutor(max_workers=8) as pool: + list(pool.map(lambda _: bump(), range(40))) + assert counter["n"] == 40 diff --git a/tests/test_main.py b/tests/test_main.py index 5f43f11..c583173 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -5,10 +5,11 @@ from __future__ import annotations from pathlib import Path import pytest -from app import config, pipeline +from app import config, images, pipeline from app.main import _safe_job_file, _validate_job_id, app from fastapi import HTTPException from fastapi.testclient import TestClient +from PIL import Image @pytest.fixture @@ -16,11 +17,26 @@ def client(data_dir: Path) -> TestClient: return TestClient(app) +def _write_test_png(path: Path, size: tuple[int, int] = (1200, 800)) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + Image.new("RGB", size, color=(40, 80, 120)).save(path, format="PNG") + + def test_validate_job_id_sanitizes_input() -> None: assert _validate_job_id("foo@bar") == "foo_bar" assert _validate_job_id("photo01") == "photo01" +def test_validate_job_id_rejects_path_like() -> None: + with pytest.raises(HTTPException) as exc: + _validate_job_id("a/b") + assert exc.value.status_code == 400 + with pytest.raises(HTTPException): + _validate_job_id("../evil") + with pytest.raises(HTTPException): + _validate_job_id("") + + def test_safe_job_file_rejects_wrong_prefix(data_dir: Path) -> None: stem = "photo01" paths = pipeline.job_paths(stem) @@ -40,3 +56,48 @@ def test_index_empty(client: TestClient) -> None: def test_job_not_found(client: TestClient) -> None: response = client.get("/jobs/does-not-exist") assert response.status_code == 404 + + +def test_job_file_srcset_resize(client: TestClient, data_dir: Path) -> None: + stem = "photo01" + paths = pipeline.job_paths(stem) + png = paths.variants / f"{stem}_v1.png" + _write_test_png(png) + pipeline.write_status(paths, "done", created_at="2026-01-01T00:00:00") + pipeline.write_manifest( + paths, + { + "variants": [{"id": "v1", "file": png.name}], + "created_at": "2026-01-01T00:00:00", + }, + ) + + full = client.get(f"/jobs/{stem}/files/{png.name}") + assert full.status_code == 200 + assert full.headers["content-type"].startswith("image/") + + bad = client.get(f"/jobs/{stem}/files/{png.name}?w=999") + assert bad.status_code == 400 + + resized = client.get(f"/jobs/{stem}/files/{png.name}?w=320") + assert resized.status_code == 200 + assert resized.headers["content-type"] == "image/webp" + assert len(resized.content) < len(full.content) + + cached = config.WEB_CACHE_DIR / f"{png.name}.w320.webp" + assert cached.is_file() + with Image.open(cached) as img: + assert img.width == 320 + + index = client.get("/") + assert index.status_code == 200 + assert f"/jobs/{stem}/files/{png.name}?w=640" in index.text + assert "srcset=" in index.text + assert "data-full-src=" in index.text + + +def test_img_attrs_builds_srcset() -> None: + attrs = images.img_attrs("job1", "job1_v1.png") + assert attrs["src"].endswith("?w=640") + assert "320w" in attrs["srcset"] + assert attrs["full_src"].endswith("?w=1280") diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index aff36c7..ad3be95 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -13,6 +13,7 @@ from app import pipeline [ ("photo.jpg", "photo"), ("../evil", "evil"), + ("a/b", "a_b"), ("", "photo"), (" spaced ", "spaced"), ("foo@bar", "foo_bar"), @@ -78,3 +79,56 @@ def test_list_job_ids(data_dir: Path) -> None: paths = pipeline.job_paths("b") pipeline.write_status(paths, "processing") assert sorted(pipeline.list_job_ids()) == ["a", "b"] + + +def test_parallel_manifest_merge_keeps_all_variants(data_dir: Path) -> None: + """Concurrent write_manifest callers must not clobber each other's variants.""" + from concurrent.futures import ThreadPoolExecutor + + paths = pipeline.job_paths("race") + pipeline.write_status(paths, "processing") + pipeline.write_manifest( + paths, + { + "source_stem": "race", + "original_file": "race_original.jpg", + "rembg_file": "race_rembg.png", + "created_at": pipeline.now_iso(), + "variants": [], + }, + ) + + def write_one(i: int) -> None: + pipeline.write_manifest( + paths, + { + "variants": [{"id": f"v{i}", "file": f"race_v{i}.png"}], + }, + ) + + with ThreadPoolExecutor(max_workers=8) as pool: + list(pool.map(write_one, range(20))) + + meta = pipeline.read_meta("race") + assert meta is not None + ids = {v["id"] for v in meta["variants"]} + assert ids == {f"v{i}" for i in range(20)} + assert meta["original_file"] == "race_original.jpg" + + +def test_filter_timeout_is_thread_local() -> None: + import threading + + results: dict[str, int] = {} + + def worker(name: str, value: int) -> None: + with pipeline.filter_timeout(value): + results[name] = pipeline._active_filter_timeout() + + t1 = threading.Thread(target=worker, args=("a", 11)) + t2 = threading.Thread(target=worker, args=("b", 22)) + t1.start() + t2.start() + t1.join() + t2.join() + assert results == {"a": 11, "b": 22}