diff --git a/py/api/__init__.py b/py/api/__init__.py index 9ae8e085..b3772d3d 100644 --- a/py/api/__init__.py +++ b/py/api/__init__.py @@ -630,16 +630,20 @@ class PromptManagerAPI( f"{url_quote(rel_path.as_posix(), safe='/')}" ) - # Check for thumbnail - rel_no_ext = rel_path.with_suffix("") - thumb_rel = ( - f"thumbnails/{rel_no_ext.as_posix()}_thumb{rel_path.suffix}" - ) + # Any preview the file owns, the current format first (#317) + from . import thumbnail_settings from .prompts import thumbnail_url_for - thumbnail_url = thumbnail_url_for( - thumb_rel, output_path / thumb_rel + existing = thumbnail_settings.existing_thumbnail( + output_path, rel_path ) + thumbnail_url = None + if existing is not None: + thumb_rel = ( + thumbnail_settings.thumbnail_stem_rel(rel_path) + + existing.suffix + ) + thumbnail_url = thumbnail_url_for(thumb_rel, existing) if thumbnail_url: image["thumbnail_url"] = thumbnail_url break # Found matching root, stop searching diff --git a/py/api/admin.py b/py/api/admin.py index 5d4a6363..1e54e991 100644 --- a/py/api/admin.py +++ b/py/api/admin.py @@ -18,6 +18,7 @@ from pathlib import Path from aiohttp import web from .prompts import thumbnail_url_for +from . import thumbnail_settings from .jobs import ( DEFAULT_SCAN_SCOPE, SCAN_GROUP_NAMES, @@ -146,39 +147,48 @@ def _physical_file_key(path, stat): return ("path", os.path.normcase(os.path.realpath(str(path)))) -def _thumbnail_rel_path(rel_path, thumbnail_ext): - """Relative path of the thumbnail the gallery would generate for ``rel_path``.""" - return f"thumbnails/{rel_path.with_suffix('').as_posix()}_thumb{thumbnail_ext}" - - -def _media_thumbnail_rel_path(rel_path): - """The thumbnail of a media file: a video's preview is a ``.jpg``.""" - extension = rel_path.suffix.lower() - return _thumbnail_rel_path( - rel_path, ".jpg" if extension in VIDEO_EXTENSIONS else rel_path.suffix - ) +def _other_sources(output_path, rel_path): + """Whether another media file shares *rel_path*'s stem in its folder.""" + folder = Path(output_path) / Path(rel_path).parent + stem = Path(rel_path).stem + own = Path(rel_path).suffix.lower() + for extension in IMAGE_EXTENSIONS + VIDEO_EXTENSIONS: + if extension == own: + continue + for name in (stem + extension, stem + extension.upper()): + if (folder / name).is_file(): + return True + return False def _remove_thumbnail(output_path, rel_path, logger=None): - """Remove the thumbnail of ``rel_path`` under ``output_path``, if any. + """Remove the previews of ``rel_path`` under ``output_path``, if any. - Shared by the legacy duplicate delete and the reviewed removal job. A - thumbnail that is a link, or one that cannot be removed, is left alone - (a stale preview never fails a removal). Returns whether one was removed. + Shared by the legacy duplicate delete and the reviewed removal job. + Every preview the file owns goes, in any format (#317); a preview name + another same-stem file still shares (``x_thumb.webp`` of ``x.png`` and + ``x.mp4``, ruling R2) stays for it. A preview that is a link, or one that + cannot be removed, is left alone (a stale preview never fails a removal). + Returns whether one was removed. """ output_path = Path(output_path) - thumbnail_path = output_path / _media_thumbnail_rel_path(Path(rel_path)) - try: - if thumbnail_path.is_symlink() or not thumbnail_path.is_file(): - return False - os.remove(thumbnail_path) - except OSError as e: + rel_path = Path(rel_path) + shared = _other_sources(output_path, rel_path) + removed = False + for thumbnail_path in thumbnail_settings.owned_thumbnails(output_path, rel_path): + extension = thumbnail_path.suffix.lower() + if shared and extension in thumbnail_settings.WRITTEN_EXTENSIONS: + continue + try: + os.remove(thumbnail_path) + except OSError as e: + if logger is not None: + logger.warning(f"Could not delete thumbnail for {rel_path}: {e}") + continue + removed = True if logger is not None: - logger.warning(f"Could not delete thumbnail for {rel_path}: {e}") - return False - if logger is not None: - logger.debug(f"Deleted associated thumbnail: {thumbnail_path}") - return True + logger.debug(f"Deleted associated thumbnail: {thumbnail_path}") + return removed async def _read_json(request): @@ -845,8 +855,13 @@ class AdminRoutesMixin: extension = media_path.suffix.lower() is_video = extension in VIDEO_EXTENSIONS - thumb_rel = _thumbnail_rel_path(rel_path, ".jpg" if is_video else extension) - thumbnail_url = thumbnail_url_for(thumb_rel, output_path / thumb_rel) + existing = thumbnail_settings.existing_thumbnail(output_path, rel_path) + thumbnail_url = None + if existing is not None: + thumb_rel = ( + thumbnail_settings.thumbnail_stem_rel(rel_path) + existing.suffix + ) + thumbnail_url = thumbnail_url_for(thumb_rel, existing) return { "id": str(hash(str(media_path))), diff --git a/py/api/autotag_routes.py b/py/api/autotag_routes.py index f72db79c..6114bb6d 100644 --- a/py/api/autotag_routes.py +++ b/py/api/autotag_routes.py @@ -10,6 +10,7 @@ from urllib.parse import quote from .jobs import AUTOTAG_BATCH_JOB_KIND from .prompts import thumbnail_url_for +from . import thumbnail_settings from .. import model_gate from aiohttp import web @@ -792,13 +793,16 @@ class AutotagRoutesMixin: rel_path = image_path.relative_to(output_path) thumbnail_url = None if has_thumbnails: - thumb_rel = ( - f"thumbnails/{rel_path.with_suffix('').as_posix()}" - f"_thumb{image_path.suffix}" - ) - thumbnail_url = thumbnail_url_for( - thumb_rel, output_path / thumb_rel + # Any preview the file owns, the current format first (#317) + existing = thumbnail_settings.existing_thumbnail( + output_path, rel_path ) + if existing is not None: + thumb_rel = ( + thumbnail_settings.thumbnail_stem_rel(rel_path) + + existing.suffix + ) + thumbnail_url = thumbnail_url_for(thumb_rel, existing) images.append( { "filename": image_path.name, diff --git a/py/api/history_current.py b/py/api/history_current.py index dcd3e4c1..2765d2e6 100644 --- a/py/api/history_current.py +++ b/py/api/history_current.py @@ -15,7 +15,9 @@ from typing import BinaryIO from aiohttp import web +from . import thumbnail_settings from .images import _canonical, _public_path, SERVABLE_EXTENSIONS, VIDEO_EXTENSIONS +from .images import media_thumbnail_rel try: from ...utils.execution_outputs import ( @@ -338,15 +340,11 @@ def _prepare_current_media( if thumbnail: relative = target.relative_to(resolved.gallery_root) is_video = target.suffix.lower() in VIDEO_EXTENSIONS - extension = ".jpg" if is_video else target.suffix.lower() - preview = ( - resolved.gallery_root - / "thumbnails" - / f"{relative.with_suffix('').as_posix()}_thumb{extension}" - ) - try: - preview.stat() - except FileNotFoundError: + # #317: any preview the file owns (the current format first); a + # missing one is made under the current format's name. + preview = thumbnail_settings.existing_thumbnail(resolved.gallery_root, relative) + if preview is None: + preview = resolved.gallery_root / media_thumbnail_rel(relative) if ensure_preview is not None and _preview_writable( resolved.gallery_root, preview ): @@ -362,7 +360,9 @@ def _prepare_current_media( return _OpenedCurrentMedia( handle, os.fstat(handle.fileno()).st_size, - mimetypes.guess_type(target.name)[0] or "application/octet-stream", + thumbnail_settings.content_type(target) + or mimetypes.guess_type(target.name)[0] + or "application/octet-stream", ) except BaseException: handle.close() diff --git a/py/api/images.py b/py/api/images.py index c851d3dd..c16595ab 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -6,10 +6,11 @@ import binascii import functools import hashlib import hmac +import io import json import os import re -import threading +import subprocess # nosec B404 - ffmpeg frames through run_encoder, fixed argv import time as _time import urllib.parse from bisect import bisect_left, bisect_right @@ -21,6 +22,18 @@ from aiohttp import web from PIL import Image, UnidentifiedImageError from .jobs import MAX_OUTCOME_ROWS, THUMBNAILS_JOB_KIND +from . import thumbnail_settings +from .thumbnail_encode import ( + PREVIEW_HELD, + EncoderClosed, + PreviewWriteHeld, + replace_unless_held, + run_encoder, + temp_sibling, + thumbnail_frame, + writer_settings, + write_thumbnail, +) from .prompts import bad_request, parse_page_params, publish_image_paths from .prompts import thumbnail_url_for from .prompts import safe_error_message as _safe_error @@ -212,6 +225,13 @@ def _has_traversal(relative): return ".." in re.split(r"[\\/]+", relative) +def _is_thumbnail_cache_path(relative): + """True when *relative* names something in a root's ``thumbnails/`` + cache (any case, either separator): never inspected as a media file.""" + first = re.split(r"[\\/]+", relative.lstrip("\\/"), maxsplit=1)[0] + return first.casefold() == "thumbnails" + + def _forbidden(message="Access denied"): return web.json_response({"success": False, "error": message}, status=403) @@ -256,55 +276,10 @@ def _thumbnail_batch_size(workers): return max(4, workers * 2) -def _temp_sibling(dst): - """Hidden temp path next to *dst* that keeps its extension (for format sniffing).""" - dst = Path(dst) - token = f"{os.getpid()}-{threading.get_ident()}" - return dst.with_name(f".{dst.stem}.part-{token}{dst.suffix}") - - -class PreviewWriteHeld(Exception): - """A preview write stopped before its rename: ``guard()`` said no (#249).""" - - -# The result error of a write held before its rename (never remembered). -PREVIEW_HELD = "Preview writes are held while another job runs" - - -def _replace_unless_held(tmp, dst, guard=None): - """Rename *tmp* over *dst*, unless ``guard()`` is false (PreviewWriteHeld).""" - if guard is not None and not guard(): - raise PreviewWriteHeld(PREVIEW_HELD) - os.replace(tmp, dst) - - -def _atomic_save(img, dst, fmt=None, *, guard=None, **save_kwargs): - """Save a PIL image to *dst* without a partially written file ever being - visible at that path: write a temp sibling, then rename over it. - - ``guard()`` (automatic previews) is asked again just before the rename; - false leaves *dst* untouched and raises PreviewWriteHeld. - """ - tmp = _temp_sibling(dst) - try: - img.save(tmp, fmt, **save_kwargs) - _replace_unless_held(tmp, dst, guard) - finally: - if tmp.exists(): - tmp.unlink() - - -# Encoder settings per thumbnail quality (the System thumbnail dialog's -# "Medium · optimized", "High · original", "Low · fastest"). The legacy -# routes always encode as "medium", which is what they always wrote. -THUMBNAIL_ENCODINGS = { - "low": {"quality": 70, "optimize": False}, - "medium": {"quality": 85, "optimize": True}, - "high": {"quality": 95, "optimize": True}, -} -DEFAULT_THUMBNAIL_ENCODING = THUMBNAIL_ENCODINGS["medium"] -# ffmpeg's JPEG scale (-q:v, 2 best to 31 worst) for each encoding's quality. -_FFMPEG_QSCALE = {95: 2, 85: 4, 70: 8} +# Preview files are written by thumbnail_encode in the configured format +# (thumbnail_settings, #317); these names stay importable from here. +_temp_sibling = temp_sibling +_replace_unless_held = replace_unless_held # A preview that could not be replaced (#249 S5 M6). Windows refuses to rename # over a file another process has open (WinError 5 or 32), such as a preview @@ -321,26 +296,15 @@ def _preview_write_error(exc): return PREVIEW_NOT_WRITABLE -def _ffmpeg_qscale(encoding): - """ffmpeg's ``-q:v`` for a THUMBNAIL_ENCODINGS entry.""" - return str(_FFMPEG_QSCALE.get(encoding["quality"], 4)) +def _write_image_thumbnail(src, dst, settings, guard=None, track=None): + """Write *src*'s preview into *dst* with *settings* (blocking, atomic). - -def _write_image_thumbnail(src, dst, thumbnail_size, encoding=None, guard=None): - """Resize *src* into *dst* (blocking PIL work), atomically. - - ``encoding`` is one of THUMBNAIL_ENCODINGS (medium when None); a PNG - preview keeps only its ``optimize`` flag. ``guard`` see _atomic_save. + The first frame, fitted inside the edge, in the configured format; + returns the path written (see thumbnail_encode.write_thumbnail). """ - encoding = encoding or DEFAULT_THUMBNAIL_ENCODING with Image.open(src) as img: - if img.mode in ("RGBA", "LA", "P"): - img = img.convert("RGB") - img.thumbnail(thumbnail_size, Image.Resampling.LANCZOS) - save_kwargs = dict(encoding) - if dst.suffix.lower() == ".png": - save_kwargs = {"optimize": encoding["optimize"]} - _atomic_save(img, dst, guard=guard, **save_kwargs) + frame = thumbnail_frame(img, settings.edge) + return write_thumbnail(frame, dst, settings, guard=guard, track=track) def _video_placeholder_image(thumbnail_size): @@ -452,6 +416,58 @@ def _thumbnail_complete_payload(total, stats, start, remaining=0, processed=None } +# The per-file states a thumbnail plan counts (#317). +_PLAN_COUNTS = ("skipped", "remaining", "fresh", "outdated", "missing") + + +def _preview_names(folders, folder): + """{normcased name: mtime} of the regular files in *folder*, listed + once per plan (a missing folder is empty).""" + names = folders.get(folder) + if names is None: + names = {} + try: + with os.scandir(folder) as entries: + for entry in entries: + try: + if entry.is_file(follow_symlinks=False): + stat = entry.stat(follow_symlinks=False) + names[os.path.normcase(entry.name)] = stat.st_mtime + except OSError: + continue + except OSError: + pass + folders[folder] = names + return names + + +def _preview_state(src, folder, stem, settings, folders): + """fresh, outdated or missing for *src* (see _plan_thumbnails).""" + names = _preview_names(folders, folder) + try: + src_mtime = src.stat().st_mtime + except OSError: + return "missing" + current = names.get(os.path.normcase(stem + settings.extension)) + others = [ + names[key] + for key in ( + os.path.normcase(stem + extension) + for extension in thumbnail_settings.candidate_extensions( + src.suffix, settings.extension + ) + if extension != settings.extension + ) + if key in names + ] + current_fresh = current is not None and current >= src_mtime + if current_fresh and not others: + return "fresh" + if others and (current_fresh or max(others) >= src_mtime): + return "outdated" + return "missing" + + def _thumbnail_is_fresh(src, dst): """True when ``dst`` exists and is at least as new as ``src`` (blocking stat).""" try: @@ -774,39 +790,42 @@ def _iter_media_files(output_path, max_files): yield root_path / name -def media_thumbnail_rel(rel_path): - """``thumbnails/_thumb`` (POSIX) for the media file *rel_path*. - - The extension is lower case, and a video's preview is a ``.jpg``: the - name every thumbnail writer (the plan, the job, automatic previews) uses. - """ - rel_path = Path(rel_path) - extension = rel_path.suffix.lower() - return ( - f"thumbnails/{rel_path.with_suffix('').as_posix()}_thumb" - f"{'.jpg' if extension in VIDEO_EXTENSIONS else extension}" - ) +def media_thumbnail_rel(rel_path, settings=None): + """``thumbnails/_thumb`` (POSIX) for the media file *rel_path*: + the name the preview gets when it is written now (#317), images and + videos alike (``writer_settings``' extension).""" + settings = settings or writer_settings() + return thumbnail_settings.thumbnail_rel(rel_path, settings.extension) def media_thumbnail(output_path, rel_path, root_index, *, root_scope=None): - """``(thumbnail_url, ready)`` for *rel_path* under *output_path* (one - blocking stat). The Images listing, the Needs review queue cards and the + """``(thumbnail_url, ready)`` for *rel_path* under *output_path* (a few + blocking stats). The Images listing, the Needs review queue cards and the Library use the same URL (#249 F, M10). - An existing preview's URL carries its mtime as ``v=`` and ``ready`` is - True. A missing one is still listed (without ``v=``): the serve route - makes it on first view and falls back when it cannot - (py/api/thumbnail_auto.py), so no card loads the full file because its - preview was not made yet. Media inside a nested ``thumbnails`` folder is - never previewed: ``(None, False)``, and its card shows the original. + An existing preview the file owns (the configured format first, then an + older name such as ``_thumb.png``, #317) is listed with its mtime as + ``v=`` and ``ready`` True. A missing one is still listed under the name + it will get (without ``v=``): the serve route makes it on first view and + falls back when it cannot (py/api/thumbnail_auto.py), so no card loads the + full file because its preview was not made yet. Media inside a nested + ``thumbnails`` folder is never previewed: ``(None, False)``, and its card + shows the original. """ rel_path = Path(rel_path) if "thumbnails" in rel_path.parts[:-1]: return None, False - thumb_rel = media_thumbnail_rel(rel_path) - thumbnail_url = thumbnail_url_for(thumb_rel, Path(output_path) / thumb_rel) + settings = writer_settings() + existing = thumbnail_settings.existing_thumbnail( + output_path, rel_path, settings.extension + ) + thumbnail_url = None + if existing is not None: + thumb_rel = thumbnail_settings.thumbnail_stem_rel(rel_path) + existing.suffix + thumbnail_url = thumbnail_url_for(thumb_rel, existing) ready = thumbnail_url is not None if thumbnail_url is None: + thumb_rel = media_thumbnail_rel(rel_path, settings) quoted = urllib.parse.quote(thumb_rel, safe="/") thumbnail_url = f"/prompt_manager/images/serve/{quoted}?root={root_index}" else: @@ -1733,6 +1752,11 @@ class ImageRoutesMixin: def _file_response(path): response = web.FileResponse(path) response.headers["Cache-Control"] = "public, max-age=3600" + # #317: name the image types explicitly; the platform's mimetypes + # table (the Windows registry) may not know .webp or .jxl. + content_type = thumbnail_settings.content_type(path) + if content_type is not None: + response.headers["Content-Type"] = content_type return response @staticmethod @@ -1758,7 +1782,12 @@ class ImageRoutesMixin: filepath = request.match_info["filepath"] if _has_traversal(filepath): return _forbidden() - if Path(filepath).suffix.lower() not in SERVABLE_EXTENSIONS: + suffix = Path(filepath).suffix.lower() + if suffix not in SERVABLE_EXTENSIONS and not ( + # A JPEG XL preview is served from thumbnails/ only (#317). + suffix in thumbnail_settings.THUMBNAIL_EXTENSIONS + and _is_thumbnail_cache_path(filepath) + ): return _forbidden("Only media files can be served") output_dirs = self._select_root(roots, request.query.get("root")) @@ -1873,22 +1902,14 @@ class ImageRoutesMixin: {"success": False, "error": _safe_error(e)}, status=500 ) - # Fixed quality presets: clients choose a name, never a raw pixel size. - THUMBNAIL_SIZES = {"low": (150, 150), "medium": (300, 300), "high": (600, 600)} - - @classmethod - def _thumbnail_size(cls, quality): - """Map a quality preset name to a pixel size; anything else is medium.""" - if not isinstance(quality, str): - return cls.THUMBNAIL_SIZES["medium"] - return cls.THUMBNAIL_SIZES.get(quality, cls.THUMBNAIL_SIZES["medium"]) - async def generate_thumbnails(self, request): """Generate every thumbnail in one executor job (POST, blocking). Holds the registry as guarded work: refused while a record-changing job runs, and such jobs are refused while it runs. A body that cannot be read is refused before the hold, so it is never listed as a job. + The legacy ``quality`` preset is accepted and ignored: previews follow + the server's thumbnail settings (#317 R3). """ data, error = await _json_body_or_error(request) if error is not None: @@ -1902,8 +1923,6 @@ class ImageRoutesMixin: async def _generate_thumbnails(self, data): try: - thumbnail_size = self._thumbnail_size(data.get("quality", "medium")) - output_dirs = self._get_all_output_dirs() if not output_dirs: return web.json_response( @@ -1912,7 +1931,7 @@ class ImageRoutesMixin: ) result = await self._run_in_executor( - self._generate_gallery_thumbnails_sync, output_dirs, thumbnail_size + self._generate_gallery_thumbnails_sync, output_dirs ) self.invalidate_gallery_cache() return web.json_response(result) @@ -1923,30 +1942,30 @@ class ImageRoutesMixin: {"success": False, "error": _safe_error(e)}, status=500 ) - def _generate_thumbnails_sync(self, output_path, thumbnails_dir, thumbnail_size): + def _generate_thumbnails_sync(self, output_path, thumbnails_dir, settings=None): """Blocking thumbnail generation loop (run in executor).""" + settings = writer_settings(settings) start = _time.monotonic() - plan = self._plan_thumbnails(output_path, thumbnails_dir) - payload = self._execute_thumbnail_plan(plan, thumbnail_size, start) + plan = self._plan_thumbnails(output_path, thumbnails_dir, settings=settings) + payload = self._execute_thumbnail_plan(plan, settings, start) payload["thumbnails_path"] = _public_path(thumbnails_dir) return payload def _generate_gallery_thumbnails_sync( - self, output_dirs: list[Path], thumbnail_size: tuple[int, int] + self, output_dirs: list[Path], settings=None ) -> dict: + settings = writer_settings(settings) start = _time.monotonic() - plan = self._plan_gallery_thumbnails(output_dirs) - payload = self._execute_thumbnail_plan(plan, thumbnail_size, start) + plan = self._plan_gallery_thumbnails(output_dirs, settings=settings) + payload = self._execute_thumbnail_plan(plan, settings, start) payload["thumbnails_path"] = _public_path(output_dirs[0] / "thumbnails") return payload - def _execute_thumbnail_plan( - self, plan: dict, thumbnail_size: tuple[int, int], start: float - ) -> dict: + def _execute_thumbnail_plan(self, plan: dict, settings, start: float) -> dict: targets = plan["targets"] stats = _new_thumbnail_stats() stats["skipped"] = plan["skipped"] - for result in self._generate_batch(targets, thumbnail_size): + for result in self._generate_batch(targets, settings): _record_thumbnail_result(stats, result) payload = _thumbnail_complete_payload( len(targets) + plan["skipped"] + plan["remaining"], @@ -1960,7 +1979,11 @@ class ImageRoutesMixin: return payload def _plan_gallery_thumbnails( - self, output_dirs: list[Path], rebuild: bool = False, stop=None + self, + output_dirs: list[Path], + mode: str = "missing", + stop=None, + settings=None, ) -> dict: """Plan every gallery root with one shared per-run generation cap. @@ -1968,33 +1991,32 @@ class ImageRoutesMixin: MAX_FRESH_LISTED files across the roots. ``stop()`` true ends the walk at the next file (a cancelled job); the partial plan is returned. """ - combined = { - "targets": [], - "skipped": 0, - "remaining": 0, - "fresh": 0, - "fresh_files": [], - } + settings = writer_settings(settings) + combined = {key: 0 for key in _PLAN_COUNTS} + combined.update(targets=[], fresh_files=[]) for root in output_dirs: if stop is not None and stop(): break root = Path(root) plan = self._plan_thumbnails( - root, root / "thumbnails", rebuild=rebuild, stop=stop + root, root / "thumbnails", mode=mode, stop=stop, settings=settings ) available = max(0, MAX_THUMBNAIL_FILES - len(combined["targets"])) combined["targets"].extend(plan["targets"][:available]) - combined["skipped"] += plan["skipped"] - combined["fresh"] += plan["fresh"] + for key in _PLAN_COUNTS: + combined[key] += plan[key] room = max(0, MAX_FRESH_LISTED - len(combined["fresh_files"])) combined["fresh_files"].extend(plan["fresh_files"][:room]) - combined["remaining"] += plan["remaining"] + max( - 0, len(plan["targets"]) - available - ) + combined["remaining"] += max(0, len(plan["targets"]) - available) return combined async def get_thumbnail_status(self, request: web.Request) -> web.Response: - """Count pending thumbnails off-loop without generating files.""" + """Count previews off-loop without generating files. + + ``pending`` previews are missing, ``outdated`` ones exist in another + format than the one written now (Convert outdated, #317) and ``ready`` + counts every file with a preview, outdated or not. + """ try: output_dirs = self._get_all_output_dirs() plan = await self._run_in_executor( @@ -2008,6 +2030,8 @@ class ImageRoutesMixin: "total_images": pending + plan["skipped"], "pending": pending, "ready": plan["skipped"], + "outdated": plan["outdated"], + "format": writer_settings().format, # Files whose automatic preview failed this session (#249). "failed": self._auto_thumbnail_failures(), "collection_id": hashlib.sha256(collection.encode()).hexdigest()[ @@ -2022,76 +2046,81 @@ class ImageRoutesMixin: {"success": False, "error": _safe_error(exc)}, status=500 ) - def _plan_thumbnails(self, output_path, thumbnails_dir, rebuild=False, stop=None): - """Decide which media files need a thumbnail (blocking walk). + def _plan_thumbnails( + self, output_path, thumbnails_dir, mode="missing", stop=None, settings=None + ): + """Decide which media files need a preview (blocking walk). - Returns ``{"targets": [(src, dst, is_video)], "skipped": n, - "remaining": n, "fresh": n, "fresh_files": [src]}``. Files whose - thumbnail is already newer than the source are ``fresh``. Without - ``rebuild`` they are counted as skipped and never queued, so a large - library that is mostly done only pays for the files that changed; - ``fresh_files`` lists the first MAX_FRESH_LISTED of them. With - ``rebuild`` every file is a target (nothing is skipped). The walk is - bounded like the gallery listing (MAX_SCAN_DEPTH, MAX_GALLERY_FILES, - no symlinks) and at most MAX_THUMBNAIL_FILES targets are queued per - run; ``remaining`` counts the targets beyond that cap. ``stop()`` true - ends the walk before the next file. + Each file is ``fresh`` (its preview in the format written now is at + least as new as the file, and no other owned preview is left), + ``outdated`` (an owned preview in another format: a legacy + ``_thumb.png`` or a format switched away from, #317) or ``missing``. + ``mode`` picks the targets: ``missing`` the missing ones (fresh and + outdated are ``skipped`` and never queued, so a large library that is + mostly done only pays for the files that changed), ``rebuild`` every + file, ``convert`` the outdated ones (missing ones are left to the + missing run). Returns ``{"targets": [(src, dst, is_video)], "skipped", + "remaining", "fresh", "outdated", "missing", "fresh_files": [src]}``; + ``fresh_files`` lists the first MAX_FRESH_LISTED fresh files. The walk + is bounded like the gallery listing (MAX_SCAN_DEPTH, MAX_GALLERY_FILES, + no symlinks), each preview folder is listed once, and at most + MAX_THUMBNAIL_FILES targets are queued per run; ``remaining`` counts + the targets beyond that cap. ``stop()`` true ends the walk before the + next file. """ + settings = writer_settings(settings) output_path = Path(output_path) - targets = [] - skipped = 0 - remaining = 0 - fresh = 0 - fresh_files = [] + thumbnails_dir = Path(thumbnails_dir) + plan = {key: 0 for key in _PLAN_COUNTS} + plan.update(targets=[], fresh_files=[]) + folders = {} for src in _iter_media_files(output_path, MAX_GALLERY_FILES): if stop is not None and stop(): break - suffix = src.suffix.lower() - is_video = suffix in VIDEO_EXTENSIONS rel_no_ext = src.relative_to(output_path).with_suffix("") - thumb_suffix = ".jpg" if is_video else suffix - dst = thumbnails_dir / f"{rel_no_ext.as_posix()}_thumb{thumb_suffix}" + stem = f"{rel_no_ext.name}{thumbnail_settings.THUMB_SUFFIX}" + dst = thumbnails_dir / rel_no_ext.parent / (stem + settings.extension) if not _is_within(dst, thumbnails_dir): self.logger.warning(f"Skipping thumbnail outside safe dir: {src.name}") continue - if _thumbnail_is_fresh(src, dst): - fresh += 1 - if not rebuild: - skipped += 1 - if len(fresh_files) < MAX_FRESH_LISTED: - fresh_files.append(src) - continue - if len(targets) >= MAX_THUMBNAIL_FILES: - remaining += 1 + state = _preview_state(src, dst.parent, stem, settings, folders) + plan[state] += 1 + target = ( + mode == "rebuild" + or (mode == "missing" and state == "missing") + or (mode == "convert" and state == "outdated") + ) + if not target: + plan["skipped"] += 1 + if state == "fresh" and len(plan["fresh_files"]) < MAX_FRESH_LISTED: + plan["fresh_files"].append(src) continue - targets.append((src, dst, is_video)) - if remaining: + if len(plan["targets"]) >= MAX_THUMBNAIL_FILES: + plan["remaining"] += 1 + continue + is_video = src.suffix.lower() in VIDEO_EXTENSIONS + plan["targets"].append((src, dst, is_video)) + if plan["remaining"]: self.logger.warning( f"Thumbnail run capped at {MAX_THUMBNAIL_FILES} new files; " - f"{remaining} more will be generated on the next run" + f"{plan['remaining']} more will be generated on the next run" ) - return { - "targets": targets, - "skipped": skipped, - "remaining": remaining, - "fresh": fresh, - "fresh_files": fresh_files, - } + return plan def _thumbnail_targets(self, output_path, thumbnails_dir): """Pending (src, dst, is_video) triples; see _plan_thumbnails.""" return self._plan_thumbnails(output_path, thumbnails_dir)["targets"] - def _generate_batch(self, targets, thumbnail_size, **options): + def _generate_batch(self, targets, settings, **options): """Create thumbnails for ``targets`` on the configured worker threads. Blocking; returns one result dict per target, in order. ``options`` - go to ``_generate_one`` (``encoding``, ``rebuild``, ``stop``). + go to ``_generate_one`` (``rebuild``, ``stop``). """ def one(target): src, dst, is_video = target - return self._generate_one(src, dst, thumbnail_size, is_video, **options) + return self._generate_one(src, dst, settings, is_video, **options) return map_parallel(one, targets, _worker_threads()) @@ -2099,9 +2128,8 @@ class ImageRoutesMixin: self, src, dst, - thumbnail_size, + settings, is_video, - encoding=None, rebuild=False, stop=None, guard=None, @@ -2109,35 +2137,40 @@ class ImageRoutesMixin: ): """Create one thumbnail (blocking). Never raises; returns a result dict. - ``stop()`` true before the file starts gives ``action: "cancelled"`` - and touches nothing (a job's file boundary). ``rebuild`` writes a - fresh preview again and never replaces an existing video preview with - the placeholder. Every write is atomic, so a failure keeps the old - preview. ``guard()`` false just before the rename gives ``action: - "error"`` with ``held: True`` and writes nothing; ``track(process)`` - is handed the ffmpeg process (both for automatic previews, #249). An - error result carries ``decode: True`` when the file itself cannot be - decoded (an I/O error does not). + *settings* (``thumbnail_settings.ThumbnailSettings``) give the format, + quality and edge; *dst* takes the format's extension. ``stop()`` true + before the file starts gives ``action: "cancelled"`` and touches + nothing (a job's file boundary). ``rebuild`` writes a fresh preview + again and never replaces an existing video preview with the + placeholder. Every write is atomic and verified, so a failure keeps + the old preview. ``guard()`` false just before the rename gives + ``action: "error"`` with ``held: True`` and writes nothing; + ``track(process)`` is handed every encoder process (both for automatic + previews, #249). An error result carries ``decode: True`` when the + file itself cannot be decoded (an I/O error does not). A generated + result names the file written as ``path``. """ result = { "file": src.name, "dir": src.parent.name, "type": "video" if is_video else "image", } + dst = Path(dst) + if dst.suffix != settings.extension: + dst = dst.with_suffix(settings.extension) try: if stop is not None and stop(): return {**result, "action": "cancelled"} if not src.is_file(): return {**result, "action": "error", "error": "File no longer exists"} if not rebuild and _thumbnail_is_fresh(src, dst): - return {**result, "action": "skipped"} + return {**result, "action": "skipped", "path": str(dst)} dst.parent.mkdir(parents=True, exist_ok=True) if is_video: made = self._generate_video_thumbnail( src, dst, - thumbnail_size, - encoding, + settings, keep_existing=rebuild, guard=guard, track=track, @@ -2149,9 +2182,10 @@ class ImageRoutesMixin: "error": "Video decode failed", "decode": True, } + written = made if isinstance(made, Path) else dst else: - _write_image_thumbnail(src, dst, thumbnail_size, encoding, guard) - return {**result, "action": "generated"} + written = _write_image_thumbnail(src, dst, settings, guard, track) + return {**result, "action": "generated", "path": str(written)} except PreviewWriteHeld: return {**result, "action": "error", "error": PREVIEW_HELD, "held": True} except UnidentifiedImageError: @@ -2186,41 +2220,44 @@ class ImageRoutesMixin: self, video_path, thumbnail_path, - thumbnail_size, - encoding=None, + settings, keep_existing=False, *, guard=None, track=None, ): - """Generate thumbnail from video file. Returns True if successful. + """Write a video's preview; returns the path written, or False. - Tries OpenCV, then ffmpeg, then writes the placeholder frame: a - decoder that cannot open or read the clip (an AV1 file OpenCV was - built without) falls through to the next one (#249 I1). ``encoding`` - is one of THUMBNAIL_ENCODINGS (medium when None); it encodes the - OpenCV frame, ffmpeg's frame (as ``-q:v``) and the placeholder alike. - Both decoders fit the frame inside ``thumbnail_size`` keeping its - aspect ratio. With ``keep_existing`` an existing preview is kept, and - the run fails, when no frame could be decoded, rather than replaced - by the placeholder. A PermissionError while replacing the preview - propagates (``_generate_one`` names it), as does PreviewWriteHeld - when ``guard()`` refuses the rename. ``track(process)`` is handed the - ffmpeg process so a shutdown can kill it. + The frame comes from OpenCV, else ffmpeg, else the placeholder frame: + a decoder that cannot open or read the clip (an AV1 file OpenCV was + built without) falls through to the next one (#249 I1). Every frame + is fitted inside the edge and encoded in *settings*' format like an + image's preview (#317). With ``keep_existing`` an existing preview is + kept, and the run fails, when no frame could be decoded, rather than + replaced by the placeholder. A PermissionError while replacing the + preview propagates (``_generate_one`` names it), as do + PreviewWriteHeld when ``guard()`` refuses the rename and + EncoderClosed at shutdown. ``track(process)`` is handed the ffmpeg + process so a shutdown can kill it. """ - encoding = encoding or DEFAULT_THUMBNAIL_ENCODING try: - frame = self._opencv_frame(video_path, thumbnail_size) + frame = self._opencv_frame(video_path, settings.size) + source = "OpenCV" + if frame is None: + frame = self._ffmpeg_frame(video_path, settings.size, track) + source = "ffmpeg" if frame is not None: - _atomic_save(frame, thumbnail_path, "JPEG", guard=guard, **encoding) - self.logger.debug( - f"Generated video thumbnail using OpenCV: {thumbnail_path}" + written = write_thumbnail( + thumbnail_frame(frame, settings.edge), + thumbnail_path, + settings, + guard=guard, + track=track, ) - return True - if self._ffmpeg_frame( - video_path, thumbnail_path, thumbnail_size, encoding, guard, track - ): - return True + self.logger.debug( + f"Generated video thumbnail using {source}: {written}" + ) + return written if keep_existing and thumbnail_path.exists(): self.logger.warning( f"No frame decoded for {video_path.name}; keeping its preview" @@ -2228,20 +2265,23 @@ class ImageRoutesMixin: return False # Last resort: the placeholder frame. try: - img = _video_placeholder_image(thumbnail_size) - _atomic_save(img, thumbnail_path, "JPEG", guard=guard, **encoding) - self.logger.debug( - f"Generated placeholder video thumbnail: {thumbnail_path}" + written = write_thumbnail( + _video_placeholder_image(settings.size), + thumbnail_path, + settings, + guard=guard, + track=track, ) - return True - except (PermissionError, PreviewWriteHeld): + self.logger.debug(f"Generated placeholder video thumbnail: {written}") + return written + except (PermissionError, PreviewWriteHeld, EncoderClosed): raise except Exception as e: self.logger.warning( f"Failed to create placeholder thumbnail for {video_path}: {e}" ) return False - except (PermissionError, PreviewWriteHeld): + except (PermissionError, PreviewWriteHeld, EncoderClosed): raise except Exception as e: self.logger.error( @@ -2275,15 +2315,14 @@ class ImageRoutesMixin: self.logger.debug(f"OpenCV could not read {video_path}: {e}") return None - def _ffmpeg_frame( - self, video_path, thumbnail_path, thumbnail_size, encoding, guard, track - ): - """Write ffmpeg's frame at 1 s into *thumbnail_path*; True on success.""" - import subprocess # nosec B404 - fixed argv, no shell - - tmp_path = _temp_sibling(thumbnail_path) + def _ffmpeg_frame(self, video_path, thumbnail_size, track=None): + """ffmpeg's frame at 1 s, fitted to *thumbnail_size*, as a PIL image + (a PNG on stdout, never a temp file), or None. EncoderClosed (the + shutdown) propagates.""" cmd = [ "ffmpeg", + "-v", + "error", "-i", str(video_path), "-ss", @@ -2291,45 +2330,36 @@ class ImageRoutesMixin: "-vframes", "1", "-vf", - f"scale={thumbnail_size[0]}:{thumbnail_size[1]}" + # Fit inside the edge, never upscale a small clip. + f"scale='min(iw,{thumbnail_size[0]})':'min(ih,{thumbnail_size[1]})'" ":force_original_aspect_ratio=decrease", - "-q:v", - _ffmpeg_qscale(encoding), - "-y", - str(tmp_path), + "-f", + "image2pipe", + "-vcodec", + "png", + "-", ] try: - process = subprocess.Popen( # nosec B603 B607 - fixed argv - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - ) - if track is not None: - track(process) - try: - _out, stderr = process.communicate(timeout=30) - except subprocess.TimeoutExpired: - process.kill() - process.communicate() - return False - if process.returncode == 0 and tmp_path.exists(): - _replace_unless_held(tmp_path, thumbnail_path, guard) - self.logger.debug( - f"Generated video thumbnail using ffmpeg: {thumbnail_path}" - ) - return True - self.logger.warning(f"ffmpeg failed for {video_path}: {stderr}") - return False - except (FileNotFoundError, OSError) as e: - if isinstance(e, PermissionError) and tmp_path.exists(): - raise - return False - finally: - try: - tmp_path.unlink() - except OSError: - pass + done = run_encoder(cmd, track=track) + except EncoderClosed: + raise + except (OSError, subprocess.SubprocessError) as e: + self.logger.debug(f"ffmpeg could not read {video_path}: {e}") + return None + if done.returncode != 0 or not done.stdout: + detail = done.stderr or b"" + if isinstance(detail, bytes): + detail = detail.decode("utf-8", "replace") + detail = detail[-300:] + self.logger.warning(f"ffmpeg failed for {video_path}: {detail}") + return None + try: + frame = Image.open(io.BytesIO(done.stdout)) + frame.load() + return frame + except Exception as e: + self.logger.warning(f"ffmpeg gave no readable frame for {video_path}: {e}") + return None async def clear_thumbnails(self, request: web.Request) -> web.Response: """Clear generated thumbnails across gallery roots, preserving originals. @@ -2423,7 +2453,8 @@ class ImageRoutesMixin: path = Path(root) / name if ( "_thumb" not in name.lower() - or path.suffix.lower() not in GALLERY_IMAGE_EXTENSIONS + or path.suffix.lower() + not in thumbnail_settings.THUMBNAIL_EXTENSIONS or path.is_symlink() ): continue @@ -2746,6 +2777,9 @@ class ImageRoutesMixin: or re.match(r"^[a-zA-Z]:", relative) ): return _forbidden() + if _is_thumbnail_cache_path(relative): + # A preview is a placeholder: it carries no metadata (#317). + return _forbidden("Previews are not inspected; inspect the original") path = Path(roots[index]) / relative denied = _media_access_error(path, [roots[index]]) if denied is not None: diff --git a/py/api/thumbnail_auto.py b/py/api/thumbnail_auto.py index 65f393c3..6a7e64fd 100644 --- a/py/api/thumbnail_auto.py +++ b/py/api/thumbnail_auto.py @@ -33,9 +33,11 @@ previews behind those URLs are made here, in three ways: (missing only, visible and cancellable) instead of writing them silently, and fewer go through the worker. -Every preview is written by ``_generate_one`` with the thumbnail job's default -size and quality (``DEFAULT_THUMBNAIL_EDGE``, medium), into a temp sibling -renamed over the old one, so a reader never sees a partial file. +Every preview is written by ``_generate_one`` with the server's thumbnail +settings (format, quality, size limit; ``thumbnail_settings``, #317), into a +temp sibling verified and renamed over the old one, so a reader never sees a +partial file. A request for an older preview name (``_thumb.png``) is answered +with the preview in the current format. Holds: automatic previews only write files under ``thumbnails/`` and never touch the database, like the thumbnail job, so they run beside a scan, image @@ -63,11 +65,10 @@ from pathlib import Path from aiohttp import web +from . import thumbnail_settings from .images import ( - GALLERY_IMAGE_EXTENSIONS, MEDIA_EXTENSIONS, THUMBNAIL_CLEAR_TITLE, - THUMBNAIL_ENCODINGS, VIDEO_EXTENSIONS, _has_traversal, _is_within, @@ -83,16 +84,22 @@ from .jobs import ( JobBusy, ) from .settings_routes import SOURCE_CHANGE_JOB_KIND -from .thumbnail_jobs import DEFAULT_THUMBNAIL_EDGE, ThumbnailOptions +from .thumbnail_encode import shutdown_encoders, writer_settings +from .thumbnail_jobs import ThumbnailOptions try: from ...utils.parallel import map_parallel except ImportError: # tests import the package top-level from utils.parallel import map_parallel -AUTO_THUMBNAIL_EDGE = DEFAULT_THUMBNAIL_EDGE -AUTO_THUMBNAIL_SIZE = (AUTO_THUMBNAIL_EDGE, AUTO_THUMBNAIL_EDGE) -AUTO_THUMBNAIL_ENCODING = THUMBNAIL_ENCODINGS["medium"] +# The placeholder frame a video without a preview is answered with (JPEG +# bytes, never written to disk); written previews follow the settings. +PLACEHOLDER_SIZE = (thumbnail_settings.DEFAULT_EDGE, thumbnail_settings.DEFAULT_EDGE) +PLACEHOLDER_QUALITY = 85 +# Source extensions in the order a shared preview name looks for them (R2). +_SOURCE_ORDER = (".png", ".jpg", ".jpeg", ".webp", ".gif") + tuple( + sorted(VIDEO_EXTENSIONS) +) # Previews views may write at once (the size of the view pool). ON_DEMAND_LIMIT = 3 # View requests that may wait on the gate; one more gets its fallback at once. @@ -146,8 +153,8 @@ def _dst_key(dst): def video_placeholder_jpeg(): """The placeholder frame as JPEG bytes (a video whose preview is missing).""" buffer = io.BytesIO() - _video_placeholder_image(AUTO_THUMBNAIL_SIZE).save( - buffer, "JPEG", **AUTO_THUMBNAIL_ENCODING + _video_placeholder_image(PLACEHOLDER_SIZE).save( + buffer, "JPEG", quality=PLACEHOLDER_QUALITY, optimize=True ) return buffer.getvalue() @@ -171,7 +178,7 @@ def _is_preview_request(filepath): stem.endswith("_thumb") and len(stem) > len("_thumb") and extension == extension.lower() - and extension in GALLERY_IMAGE_EXTENSIONS + and extension in thumbnail_settings.THUMBNAIL_EXTENSIONS ) @@ -184,12 +191,27 @@ def _regular_file(path): def _candidates(parts): - """``(folder parts, base, extensions)`` of a checked preview request.""" + """``(folder parts, base, source extensions)`` of a checked preview + request: every media extension that owns the requested preview name + (``thumbnail_settings.owns``), images first (ruling R2).""" stem, extension = os.path.splitext(parts[-1]) - wanted = [extension] + (sorted(VIDEO_EXTENSIONS) if extension == ".jpg" else []) + wanted = [ + source + for source in _SOURCE_ORDER + if source in MEDIA_EXTENSIONS and thumbnail_settings.owns(extension, source) + ] return parts[1:-1], stem[: -len("_thumb")], wanted +def _current_preview(root, parts): + """The preview *parts* names, in the format written now (#317): a request + for an older name (``_thumb.png`` before a conversion) is answered with + the current file.""" + _folder, base, _wanted = _candidates(parts) + name = base + thumbnail_settings.THUMB_SUFFIX + writer_settings().extension + return Path(root).joinpath(*parts[:-1], name) + + def _quick_source(root, parts): """``(src, is_video)`` by exact name only (no folder listing), or None.""" root = Path(root) @@ -207,11 +229,12 @@ def _thumbnail_source(root, parts): """``(src, dst, is_video)`` for the preview *parts* under *root*, or None. *parts* passed ``_is_preview_request``. The source has the preview's name - without ``_thumb`` and an extension that previews as the requested one - (any case: exact names first, then one listing of the folder); a ``.jpg`` - may also be a video's. It must be a regular file inside *root*. ``dst`` - is None when the preview's folder resolves outside ``root/thumbnails`` (a - link): nothing is written there. + without ``_thumb`` and an extension that owns the requested one (any case: + exact names first, then one listing of the folder; ruling R4). It must be + a regular file inside *root*. ``dst`` is the preview in the format written + now (#317), whatever extension was requested; it is None when the + preview's folder resolves outside ``root/thumbnails`` (a link): nothing is + written there. """ root = Path(root) folder_parts, base, wanted = _candidates(parts) @@ -224,7 +247,7 @@ def _thumbnail_source(root, parts): return None rel = Path(*folder_parts, found.name) dst = root / media_thumbnail_rel(rel) - if dst != root.joinpath(*parts): + if dst != _current_preview(root, parts): return None if not _is_within(root / "thumbnails", root) or not _is_within( dst, root / "thumbnails" @@ -605,6 +628,8 @@ class AutoThumbnailMixin: thumbnailer.close() if pool is not None: pool.shutdown(wait=False, cancel_futures=True) + if final: + shutdown_encoders() # kill JPEG XL / ffmpeg encoders still running def _auto_thumbnail_failures(self): thumbnailer = self._auto_thumbnailer @@ -614,9 +639,8 @@ class AutoThumbnailMixin: return self._generate_one( src, dst, - AUTO_THUMBNAIL_SIZE, + writer_settings(), is_video, - encoding=AUTO_THUMBNAIL_ENCODING, stop=stop, guard=guard, track=track, @@ -730,7 +754,7 @@ class AutoThumbnailMixin: if quick is None: continue return self._preview_response(BUSY, quick[0], None, quick[1]) - dst = root.joinpath(*parts) + dst = _current_preview(root, parts) if outcome == READY: return self._file_response(dst) located = await gate.loop.run_in_executor( @@ -807,7 +831,7 @@ class AutoThumbnailMixin: plan = await self._run_in_executor(self._plan_gallery_thumbnails, output_dirs) missing = len(plan["targets"]) + plan["remaining"] if missing >= SCAN_THUMBNAIL_JOB_THRESHOLD: - options = ThumbnailOptions("missing", AUTO_THUMBNAIL_EDGE, "medium") + options = ThumbnailOptions("missing", writer_settings()) try: self._start_thumbnail_job(options) except JobBusy as busy: diff --git a/py/api/thumbnail_encode.py b/py/api/thumbnail_encode.py index 960f763e..6ecfa22c 100644 --- a/py/api/thumbnail_encode.py +++ b/py/api/thumbnail_encode.py @@ -34,10 +34,13 @@ import os import shutil import subprocess # nosec B404 - fixed argv lists, never a shell import threading +from dataclasses import replace from pathlib import Path from PIL import Image +from . import thumbnail_settings + try: from ...utils.logging_config import get_logger except ImportError: # tests import the package top-level @@ -383,6 +386,17 @@ def _jxl_fallback(): ) +def writer_settings(settings=None): + """The settings a preview is written with now: the configured ones, but + WebP while JPEG XL has no encoder (logged once). Its extension names the + file, so "outdated" means a preview this server would not write now.""" + settings = settings or thumbnail_settings.current() + if settings.format == "jxl" and detect_jxl_encoder() is None: + _jxl_fallback() + return replace(settings, format="webp") + return settings + + def write_thumbnail(frame, dst, settings, *, guard=None, track=None): """Encode *frame* into *dst* in ``settings``' format; returns the path written (``.webp`` when JPEG XL fell back). Atomic and verified; raises diff --git a/py/api/thumbnail_jobs.py b/py/api/thumbnail_jobs.py index 9c0d6b63..25c4f119 100644 --- a/py/api/thumbnail_jobs.py +++ b/py/api/thumbnail_jobs.py @@ -1,29 +1,31 @@ -"""Thumbnail generation as an Activity job (#249 System S5, Task 12). +"""Thumbnail generation as an Activity job (#249 System S5, Task 12; #317). -- ``GET /prompt_manager/images/thumbnails/plan?mode=&size=&quality=`` counts - what a run would do over every gallery root and writes nothing: - ``{success, targets, fresh, remaining, roots, mode, size, quality}``. -- ``POST /prompt_manager/jobs/thumbnails {mode, size, quality}`` starts the - ``thumbnails`` job (202 ``{job}``). Its admission class is ``guarded``: it - never runs beside a record-changing job, and one thumbnail job runs at a - time. It stops at a file boundary when cancelled, keeping the previews it - wrote. Its outcome groups are ``generated``, ``skipped`` (already fresh) - and ``errors``. +- ``GET /prompt_manager/images/thumbnails/plan?mode=`` counts what a run + would do over every gallery root and writes nothing: ``{success, targets, + fresh, outdated, remaining, roots, mode, format, quality, size}``. +- ``POST /prompt_manager/jobs/thumbnails {mode}`` starts the ``thumbnails`` + job (202 ``{job}``). Its admission class is ``guarded``: it never runs + beside a record-changing job, and one thumbnail job runs at a time. It + stops at a file boundary when cancelled, keeping the previews it wrote. + Its outcome groups are ``generated``, ``skipped`` (already fresh) and + ``errors``. ``mode`` is ``missing`` (previews that are absent or older than their file) -or ``rebuild`` (every preview again, fresh ones too). ``size`` is the -preview's longest edge, 128 to 2048 pixels. ``quality`` picks the encoder -settings (``THUMBNAIL_ENCODINGS``). Every preview is written next to the old -one and renamed over it, so nothing is cleared first and a failure keeps the -old preview; originals are only read. +or ``rebuild`` (every preview again, fresh ones too). Format, quality and +size limit are the server's thumbnail settings (``thumbnail_settings``, +#317), read once when the job starts and shown in its scope; a ``size`` or +``quality`` an older client sends is ignored. Every preview is written next +to the old one, verified and renamed over it, so nothing is cleared first and +a failure keeps the old preview; originals are only read. The legacy gallery stream ``POST /images/generate-thumbnails/progress`` -starts this job (its quality preset still sizes the previews) or attaches to -the running one. Its first frame, ``job {id, started}``, names that job and -whether this stream started it (the gallery cancels by that id, and only a -job it started); then come the job's legacy ``status``/``start``/ -``progress``/``file_error``/``complete`` frames. A client that goes away no -longer stops the work: the job outlives the request, as a scan does. +starts this job (missing previews, the server's settings; its ``quality`` +preset is ignored, #317 R3) or attaches to the running one. Its first frame, +``job {id, started}``, names that job and whether this stream started it (the +gallery cancels by that id, and only a job it started); then come the job's +legacy ``status``/``start``/``progress``/``file_error``/``complete`` frames. A +client that goes away no longer stops the work: the job outlives the request, +as a scan does. """ import asyncio @@ -36,7 +38,6 @@ from aiohttp import web from .admin import _plural, _scan_location, _scan_roots, _scan_status_line from .images import ( - THUMBNAIL_ENCODINGS, _BodyTooLarge, _new_thumbnail_stats, _read_json_body, @@ -48,24 +49,12 @@ from .images import ( ) from .job_routes import busy_response from .jobs import THUMBNAIL_GROUP_NAMES, THUMBNAILS_JOB_KIND, JobBusy +from .thumbnail_encode import writer_settings +from .thumbnail_settings import ThumbnailSettings THUMBNAIL_JOB_TITLE = "Thumbnail generation" THUMBNAIL_MODES = ("missing", "rebuild") -THUMBNAIL_QUALITIES = tuple(THUMBNAIL_ENCODINGS) -MIN_THUMBNAIL_EDGE = 128 -MAX_THUMBNAIL_EDGE = 2048 -# The concept's "Preview size limit" default. -DEFAULT_THUMBNAIL_EDGE = 512 -DEFAULT_THUMBNAIL_OPTIONS = { - "mode": "missing", - "size": DEFAULT_THUMBNAIL_EDGE, - "quality": "medium", -} -_OPTIONS_ERROR = ( - 'mode must be "missing" or "rebuild", quality "low", "medium" or "high", ' - f"and size a whole number of pixels from {MIN_THUMBNAIL_EDGE} " - f"to {MAX_THUMBNAIL_EDGE}" -) +_OPTIONS_ERROR = 'mode must be "missing" or "rebuild"' CANCELLED_ERROR = "Thumbnail generation was cancelled" CANCELLED_BEFORE_WRITING = "Cancelled before any preview was written" NOTHING_TO_PREVIEW = "No images or videos to preview" @@ -80,56 +69,35 @@ _LOG_PREFIXES = {"generated": "+", "errors": "!"} @dataclass(frozen=True) class ThumbnailOptions: - """One run's choices: the mode, the preview's longest edge and quality.""" + """One run's choices: the mode and the thumbnail settings it writes with.""" mode: str - edge: int - quality: str - - @property - def size(self): - return (self.edge, self.edge) - - @property - def encoding(self): - return THUMBNAIL_ENCODINGS[self.quality] + settings: ThumbnailSettings @property def rebuild(self): return self.mode == "rebuild" def scope(self): - return {"mode": self.mode, "size": self.edge, "quality": self.quality} - - -def _edge(value, *, from_query): - """The preview edge as an int, or None when it is not one in range.""" - if from_query and isinstance(value, str): - if not value.isascii() or not value.isdigit() or len(value) > 5: - return None - value = int(value) - if isinstance(value, bool) or not isinstance(value, int): - return None - return value if MIN_THUMBNAIL_EDGE <= value <= MAX_THUMBNAIL_EDGE else None + return { + "mode": self.mode, + "format": self.settings.format, + "quality": self.settings.quality, + "size": self.settings.edge, + } def thumbnail_options(raw, *, from_query=False): """``(ThumbnailOptions, None)`` from a body or query, or ``(None, message)``. - Absent fields take ``DEFAULT_THUMBNAIL_OPTIONS``. Query values are - strings, so a query ``size`` may be decimal digits; a JSON ``size`` must - be an int. + An absent mode is ``missing``; the settings are the server's now. """ if not isinstance(raw, dict) and not hasattr(raw, "get"): return None, _OPTIONS_ERROR - mode = raw.get("mode", DEFAULT_THUMBNAIL_OPTIONS["mode"]) - quality = raw.get("quality", DEFAULT_THUMBNAIL_OPTIONS["quality"]) - edge = _edge(raw.get("size", DEFAULT_THUMBNAIL_EDGE), from_query=from_query) - if mode not in THUMBNAIL_MODES or quality not in THUMBNAIL_QUALITIES: + mode = raw.get("mode", "missing") + if mode not in THUMBNAIL_MODES: return None, _OPTIONS_ERROR - if edge is None: - return None, _OPTIONS_ERROR - return ThumbnailOptions(mode, edge, quality), None + return ThumbnailOptions(mode, writer_settings()), None def _invalid(message): @@ -168,7 +136,10 @@ class ThumbnailJobsMixin: try: output_dirs = await self._run_in_executor(self._get_all_output_dirs) plan = await self._run_in_executor( - self._plan_gallery_thumbnails, output_dirs, rebuild=options.rebuild + self._plan_gallery_thumbnails, + output_dirs, + options.mode, + settings=options.settings, ) except Exception: self.logger.exception("Thumbnail plan failed") @@ -185,6 +156,7 @@ class ThumbnailJobsMixin: "success": True, "targets": len(plan["targets"]), "fresh": plan["fresh"], + "outdated": plan["outdated"], "remaining": plan["remaining"], "roots": [self._public_path(root) for root in output_dirs], **options.scope(), @@ -265,7 +237,7 @@ class ThumbnailJobsMixin: job.update(status_line=f"Looking for media files in {folders}") job.log_line( f"Thumbnails started · {folders} · {_MODE_LOG[options.mode]} · " - f"{options.edge} px · {options.quality}" + f"{options.settings.describe()}" ) job.publish( _frame( @@ -276,8 +248,9 @@ class ThumbnailJobsMixin: plan = await self._run_in_executor( self._plan_gallery_thumbnails, output_dirs, - rebuild=options.rebuild, + options.mode, stop=lambda: job.cancelled, + settings=options.settings, ) if job.cancelled: # M2: stopped during the walk, before any start frame return self._end_cancelled(job, 0, CANCELLED_BEFORE_WRITING) @@ -367,7 +340,6 @@ class ThumbnailJobsMixin: start = time.monotonic() batch_size = _thumbnail_batch_size(_worker_threads()) generate = { - "encoding": options.encoding, "rebuild": options.rebuild, "stop": lambda: job.cancelled, } @@ -378,7 +350,7 @@ class ThumbnailJobsMixin: break batch = targets[batch_start : batch_start + batch_size] results = await self._run_in_executor( - self._generate_batch, batch, options.size, **generate + self._generate_batch, batch, options.settings, **generate ) for (src, _dst, _video), result in zip(batch, results): if result["action"] == "cancelled": @@ -430,14 +402,13 @@ class ThumbnailJobsMixin: async def generate_thumbnails_with_progress(self, request): """The gallery's thumbnail stream (POST, server-sent events). - Starts the thumbnail job with the gallery's quality preset as the - preview size (missing previews, medium encoding, as before), or - attaches to the running thumbnail job, and streams its frames. A + Starts the thumbnail job (missing previews with the server's + thumbnail settings; the gallery's ``quality`` preset is ignored, R3), + or attaches to the running thumbnail job, and streams its frames. A legacy hold (the blocking generate or clear) or a record-changing job refuses it with the stream's error frame. The job goes on when the client goes away. """ - thumbnail_size = self._thumbnail_size(request.query.get("quality", "medium")) response = web.StreamResponse( status=200, headers={ @@ -449,7 +420,7 @@ class ThumbnailJobsMixin: await response.prepare(request) # M3: read the registry after the last await, so a job that started # while the headers went out is attached to, not refused. - job, started, busy = self._legacy_stream_job(thumbnail_size) + job, started, busy = self._legacy_stream_job() if busy is not None: message = f"{busy.title} already running" await self._emit_sse( @@ -476,12 +447,12 @@ class ThumbnailJobsMixin: await events.aclose() return response - def _legacy_stream_job(self, thumbnail_size): + def _legacy_stream_job(self): """``(job, started, busy)`` for the legacy stream, synchronously. - Attaches to the running cancellable thumbnail job, or starts one with - the gallery's preset as the size (missing previews, medium encoding, - as the gallery always wrote), or names the job in the way. + Attaches to the running cancellable thumbnail job, or starts one for + the missing previews with the server's settings, or names the job in + the way. """ running = self._jobs.active(THUMBNAILS_JOB_KIND) if running is not None and running.cancellable: @@ -490,5 +461,5 @@ class ThumbnailJobsMixin: if busy is not None: return None, False, busy # blocker() said free and nothing awaited since, so the start is admitted. - options = ThumbnailOptions("missing", thumbnail_size[0], "medium") + options = ThumbnailOptions("missing", writer_settings()) return self._start_thumbnail_job(options), True, None diff --git a/py/api/thumbnail_settings.py b/py/api/thumbnail_settings.py index 553bbe93..0af38c92 100644 --- a/py/api/thumbnail_settings.py +++ b/py/api/thumbnail_settings.py @@ -164,10 +164,26 @@ def _is_preview_file(path): def _owned(root, rel_path, preferred): rel_path = Path(rel_path) stem = Path(root) / thumbnail_stem_rel(rel_path) - for extension in candidate_extensions(rel_path.suffix, preferred): + extensions = list(candidate_extensions(rel_path.suffix, preferred)) + if rel_path.suffix != rel_path.suffix.lower() and owns( + rel_path.suffix, rel_path.suffix + ): + # An older release kept the source's own spelling (``b_thumb.JPG``). + extensions.append(rel_path.suffix) + seen = set() + for extension in extensions: path = stem.with_name(stem.name + extension) - if _is_preview_file(path): - yield path + if not _is_preview_file(path): + continue + try: + stat = path.stat() + identity = (stat.st_dev, stat.st_ino) if stat.st_ino else str(path) + except OSError: + continue + if identity in seen: + continue # the same file under another case (Windows, macOS) + seen.add(identity) + yield path def owned_thumbnails(root, rel_path, preferred=None): diff --git a/tests/browser/server.py b/tests/browser/server.py index a04c66e7..41f66238 100644 --- a/tests/browser/server.py +++ b/tests/browser/server.py @@ -355,7 +355,12 @@ def create_app(root: Path) -> web.Application: with Image.new("RGB", (64, 64), (100, 80, 120)) as image: image.save(root / "output" / name, pnginfo=info) os.utime(root / "output" / name, (time.time() - 100, time.time() - 100)) - api._generate_gallery_thumbnails_sync(api._get_all_output_dirs(), (150, 150)) + from py.api.thumbnail_settings import ThumbnailSettings + + # 150 px previews in the default format (#317 thumbnail settings). + api._generate_gallery_thumbnails_sync( + api._get_all_output_dirs(), ThumbnailSettings("webp", 80, 150) + ) if include_video: # Seed after thumbnail generation to exercise a real unthumbnailed video. video = root / "output" / "clip.webm" diff --git a/tests/browser/workspace-images.spec.mjs b/tests/browser/workspace-images.spec.mjs index a15a3ea6..7a01102f 100644 --- a/tests/browser/workspace-images.spec.mjs +++ b/tests/browser/workspace-images.spec.mjs @@ -399,7 +399,7 @@ test('real video media has a usable card and native playback cleans up on close await expect(await request.post('/__test__/reset', { data: { include_video: true } })).toBeOK(); const media = await (await request.get('/prompt_manager/images/output?limit=40')).json(); // #249: a video with no preview yet still lists its preview URL; the server makes the frame on first view. - expect(media.images.find(image => image.filename === 'clip.webm').thumbnail_url).toMatch(/\/thumbnails\/clip_thumb\.jpg\?root=0/); + expect(media.images.find(image => image.filename === 'clip.webm').thumbnail_url).toMatch(/\/thumbnails\/clip_thumb\.webp\?root=0/); await page.goto(workspace); await refine(page, { Media: 'video' }); await expect(cards(page)).toHaveCount(1); @@ -722,7 +722,7 @@ test('refresh releases the previous inspector cover source', async ({ page, requ await card.getByRole('button', { name: 'Inspect clip.webm', exact: true }).click(); // #249: a video's cover is its preview frame, not the video. const cover = details(page).locator('img.detail-cover'); - await expect(cover).toHaveAttribute('src', /\/thumbnails\/clip_thumb\.jpg\?/); + await expect(cover).toHaveAttribute('src', /\/thumbnails\/clip_thumb\.webp\?/); await expect(details(page).locator('video')).toHaveCount(0); await cover.evaluate(element => { window.__oldInspectorCover = element; }); await rescan(page); diff --git a/tests/browser/workspace-thumbnails.spec.mjs b/tests/browser/workspace-thumbnails.spec.mjs index 3f515441..64c1ef2b 100644 --- a/tests/browser/workspace-thumbnails.spec.mjs +++ b/tests/browser/workspace-thumbnails.spec.mjs @@ -44,7 +44,7 @@ test('a page of the Images grid requests previews, never originals, in every lay const listing = await (await request.get('/prompt_manager/images/output?limit=40')).json(); const fresh = listing.images.filter(image => image.path.startsWith('unthumbnailed/')); expect(fresh).toHaveLength(6); - for (const image of fresh) expect(image.thumbnail_url).toMatch(/\/thumbnails\/unthumbnailed\/plain_\d{3}_thumb\.png\?root=0/); + for (const image of fresh) expect(image.thumbnail_url).toMatch(/\/thumbnails\/unthumbnailed\/plain_\d{3}_thumb\.webp\?root=0/); const seen = recordMedia(page); await page.goto(images); await expect(cards(page)).toHaveCount(8); @@ -73,7 +73,7 @@ test('the inspector cover is the preview and only the media viewer loads the ori const card = cards(page).filter({ hasText: 'plain_001.png' }); await card.getByRole('button', { name: 'Inspect plain_001.png', exact: true }).click(); const cover = details(page).locator('img.detail-cover'); - await expect(cover).toHaveAttribute('src', /\/thumbnails\/unthumbnailed\/plain_001_thumb\.png\?/); + await expect(cover).toHaveAttribute('src', /\/thumbnails\/unthumbnailed\/plain_001_thumb\.webp\?/); await settled(page); await cover.evaluate(image => image.decode().catch(() => {})); expect(originals(seen)).toEqual([]); @@ -88,7 +88,7 @@ test('a video card shows its preview frame instead of loading the video', async const seen = recordMedia(page); await page.goto(images); const card = cards(page).filter({ hasText: 'clip.webm' }); - await expect(card.locator('.media-art > .image-open > img')).toHaveAttribute('src', /\/thumbnails\/clip_thumb\.jpg\?/); + await expect(card.locator('.media-art > .image-open > img')).toHaveAttribute('src', /\/thumbnails\/clip_thumb\.webp\?/); await expect(card.locator('video')).toHaveCount(0); await settled(page); expect(originals(seen)).toEqual([]); @@ -107,7 +107,7 @@ test('Library cards and the shared gallery request previews only', async ({ page test('a video whose preview cannot be loaded swaps back to its own element', async ({ page, request }) => { await expect(await request.post('/__test__/reset', { data: { include_video: true } })).toBeOK(); - await page.route(url => url.pathname.endsWith('/thumbnails/clip_thumb.jpg'), route => route.fulfill({ status: 404, body: '' })); + await page.route(url => url.pathname.endsWith('/thumbnails/clip_thumb.webp'), route => route.fulfill({ status: 404, body: '' })); await page.goto(images); const card = cards(page).filter({ hasText: 'clip.webm' }); await expect(card.locator('.media-art > .image-open > video')).toHaveCount(1); diff --git a/tests/browser/workspace_concept_fixture.py b/tests/browser/workspace_concept_fixture.py index 1cf01921..a5dd23e8 100644 --- a/tests/browser/workspace_concept_fixture.py +++ b/tests/browser/workspace_concept_fixture.py @@ -513,7 +513,12 @@ def seed_workspace_concept( tags = [row[0] for row in conn.execute("SELECT name FROM tags ORDER BY id")] if tuple(counts) != (len(CONCEPT_PROMPTS), CONCEPT_RUN_COUNT, CONCEPT_RUN_COUNT): raise RuntimeError(f"Concept fixture run counts are not real: {tuple(counts)}") - api._generate_gallery_thumbnails_sync(api._get_all_output_dirs(), (150, 150)) + from py.api.thumbnail_settings import ThumbnailSettings + + # 150 px previews in the default format (#317 thumbnail settings). + api._generate_gallery_thumbnails_sync( + api._get_all_output_dirs(), ThumbnailSettings("webp", 80, 150) + ) api.invalidate_gallery_cache() return { "epoch": epoch, diff --git a/tests/browser/workspace_tagging_fixture.py b/tests/browser/workspace_tagging_fixture.py index 9bf0214c..6fa32198 100644 --- a/tests/browser/workspace_tagging_fixture.py +++ b/tests/browser/workspace_tagging_fixture.py @@ -148,7 +148,12 @@ def seed_tagging(api: PromptManagerAPI, root: Path, extra_images: int = 0) -> di image_id = db.link_image_to_prompt(positive_id, str(output / relative)) if kind == "linked": db.link_image_to_pair(pair_id, image_id, {"source": "fixture"}) - api._generate_gallery_thumbnails_sync(api._get_all_output_dirs(), (150, 150)) + from py.api.thumbnail_settings import ThumbnailSettings + + # 150 px previews in the default format (#317 thumbnail settings). + api._generate_gallery_thumbnails_sync( + api._get_all_output_dirs(), ThumbnailSettings("webp", 80, 150) + ) api.invalidate_gallery_cache() return { "positive_id": positive_id, diff --git a/tests/browser/workspace_visual_fixture.py b/tests/browser/workspace_visual_fixture.py index 3a983425..edb8dc01 100644 --- a/tests/browser/workspace_visual_fixture.py +++ b/tests/browser/workspace_visual_fixture.py @@ -164,5 +164,10 @@ def seed_workspace_visual(api: PromptManagerAPI, root: Path) -> None: path = root / directory / category.lower() / filename _write_art(path, index, positive, negatives[offset % len(negatives)]) api.db.link_image_to_prompt(prompt_id, str(path)) - api._generate_gallery_thumbnails_sync(api._get_all_output_dirs(), (150, 150)) + from py.api.thumbnail_settings import ThumbnailSettings + + # 150 px previews in the default format (#317 thumbnail settings). + api._generate_gallery_thumbnails_sync( + api._get_all_output_dirs(), ThumbnailSettings("webp", 80, 150) + ) api.invalidate_gallery_cache() diff --git a/tests/js/workspace-inspection-state.test.js b/tests/js/workspace-inspection-state.test.js index 17b9ce3a..5bdd7d89 100644 --- a/tests/js/workspace-inspection-state.test.js +++ b/tests/js/workspace-inspection-state.test.js @@ -913,6 +913,26 @@ test('record carries the listing thumbnail as its preview, never another file (# assert.deepEqual(toRecord(data, video, extractGenerationFacts).preview, { url: video.thumbnail_url }); }); +test('the preview may be any thumbnail format the file owns (#317)', () => { + const toRecord = contract('toInspectionRecord'), entry = image(); + const base = '/prompt_manager/images/serve/thumbnails/folder/a%20%23%25_thumb'; + for (const extension of ['.webp', '.jxl', '.jpg', '.png']) { + const thumbnail_url = `${base}${extension}?v=3&root=0&root_scope=${scopeA}`; + assert.deepEqual(toRecord(envelope(entry), { ...entry, thumbnail_url }, extractGenerationFacts).preview, { url: thumbnail_url }, extension); + } + // Not a thumbnail format, upper case, no extension, or a legacy name another file owns: no preview. + for (const extension of ['.txt', '.WEBP', '', '.gif', '.jpeg', '.webp.png']) { + const thumbnail_url = `${base}${extension}?root=0&root_scope=${scopeA}`; + assert.equal(toRecord(envelope(entry), { ...entry, thumbnail_url }, extractGenerationFacts).preview, null, extension); + } + const video = { root_index: 0, path: 'clip.MP4', filename: 'clip.MP4', url: `/prompt_manager/images/serve/clip.MP4?root=0&root_scope=${scopeA}`, root_scope: scopeA }; + const data = envelope(video); data.file = { ...data.file, media_type: 'video', extension: '.mp4', format: 'MP4' }; + for (const [extension, owned] of [['.webp', true], ['.jxl', true], ['.jpg', true], ['.png', false]]) { + const thumbnail_url = `/prompt_manager/images/serve/thumbnails/clip_thumb${extension}?root=0&root_scope=${scopeA}`; + assert.deepEqual(toRecord(data, { ...video, thumbnail_url }, extractGenerationFacts).preview, owned ? { url: thumbnail_url } : null, extension); + } +}); + test("names the server quotes (parentheses, '!*') still match their original and preview (#249 M11)", () => { const toRecord = contract('toInspectionRecord'); const entry = { root_index: 0, path: "folder/a (1)!'*.png", filename: "a (1)!'*.png", root_scope: scopeA, diff --git a/tests/test_api_history_outputs.py b/tests/test_api_history_outputs.py index 27ed77a6..9a4644fe 100644 --- a/tests/test_api_history_outputs.py +++ b/tests/test_api_history_outputs.py @@ -431,10 +431,11 @@ class HistoryOutputAPITests(ImageAPITestCase): self.assertEqual(int(response.headers["Content-Length"]), len(payload)) self.assertEqual(await response.read(), b"") # A missing preview is made on first view (#249): an undecodable video - # gets the placeholder frame, never the whole file. - preview = self.output_dir / "thumbnails" / "clip_thumb.jpg" + # gets the placeholder frame, never the whole file, written in the + # thumbnail format (#317, WebP by default). + preview = self.output_dir / "thumbnails" / "clip_thumb.webp" response = await self.current(descriptor["thumbnail_url"]) - self.assertEqual(response.headers["Content-Type"], "image/jpeg") + self.assertEqual(response.headers["Content-Type"], "image/webp") self.assertEqual(await response.read(), preview.read_bytes()) self.assertLess(preview.stat().st_size, len(payload)) make_png(preview) diff --git a/tests/test_api_images.py b/tests/test_api_images.py index 163805a7..d24e13bb 100644 --- a/tests/test_api_images.py +++ b/tests/test_api_images.py @@ -33,7 +33,8 @@ from aiohttp.test_utils import AioHTTPTestCase # noqa: E402 from PIL import Image # noqa: E402 from database.operations import PromptDatabase # noqa: E402 -import py.api.images as images_module # noqa: E402 +import py.api.images as images_module +from py.api.thumbnail_settings import ThumbnailSettings # noqa: E402 from py.api import PromptManagerAPI # noqa: E402 from py.api.prompts import MAX_PAGE_LIMIT # noqa: E402 from py.config import GalleryConfig # noqa: E402 @@ -397,7 +398,7 @@ class ThumbnailTestCase(ImageAPITestCase): self.api._run_in_executor = spy return calls - def _thumb_path(self, name, suffix=".png"): + def _thumb_path(self, name, suffix=".webp"): return self.output_dir / "thumbnails" / f"{name}_thumb{suffix}" @@ -588,14 +589,15 @@ class TestThumbnailProgressStream(ThumbnailTestCase): events = parse_sse(await resp.text()) self.assertEqual(events[-1][0], "error") - async def test_unknown_quality_falls_back_to_medium(self): - make_png(self.output_dir / "one.png", size=(400, 400)) + async def test_any_quality_preset_uses_the_server_size_limit(self): + # #317 R3: the server's thumbnail settings size every preview. + make_png(self.output_dir / "one.png", size=(1000, 1000)) resp = await self.client.request("POST", f"{self.PROGRESS_URL}?quality=huge") await resp.text() with Image.open(self._thumb_path("one")) as thumb: - self.assertEqual(max(thumb.size), 300) + self.assertEqual(max(thumb.size), 512) class TestGenerateThumbnailsPost(ThumbnailTestCase): @@ -910,7 +912,8 @@ class TestThumbnailFilesAreNeverSeenHalfWritten(ThumbnailTestCase): make_png(self.output_dir / "a.png", size=(64, 64)) thumbs = self.output_dir / "thumbnails" thumbs.mkdir() - dst = thumbs / "a_thumb.png" + dst = thumbs / "a_thumb.webp" + settings = ThumbnailSettings("webp", 80, 32) def crash_mid_write(self_img, fp, *args, **kwargs): # Simulates an interrupted encoder: bytes on disk, then failure. @@ -920,14 +923,14 @@ class TestThumbnailFilesAreNeverSeenHalfWritten(ThumbnailTestCase): with patch.object(Image.Image, "save", crash_mid_write): with self.assertRaises(OSError): images_module._write_image_thumbnail( - self.output_dir / "a.png", dst, (32, 32) + self.output_dir / "a.png", dst, settings ) self.assertFalse(dst.exists(), "a half-written thumbnail was left in place") self.assertEqual(os.listdir(thumbs), []) - images_module._write_image_thumbnail(self.output_dir / "a.png", dst, (32, 32)) + images_module._write_image_thumbnail(self.output_dir / "a.png", dst, settings) self.assertTrue(dst.is_file()) - self.assertEqual(os.listdir(thumbs), ["a_thumb.png"]) + self.assertEqual(os.listdir(thumbs), ["a_thumb.webp"]) def test_thumbnail_url_is_versioned_by_its_mtime(self): import os @@ -935,7 +938,9 @@ class TestThumbnailFilesAreNeverSeenHalfWritten(ThumbnailTestCase): make_png(self.output_dir / "a.png", size=(64, 64)) self.api._generate_thumbnails_sync( - self.output_dir, self.output_dir / "thumbnails", (32, 32) + self.output_dir, + self.output_dir / "thumbnails", + ThumbnailSettings("webp", 80, 32), ) thumb = self._thumb_path("a") @@ -944,7 +949,7 @@ class TestThumbnailFilesAreNeverSeenHalfWritten(ThumbnailTestCase): ) self.assertRegex( entry["thumbnail_url"], - r"^/prompt_manager/images/serve/thumbnails/a_thumb\.png\?v=\d+&root=0$", + r"^/prompt_manager/images/serve/thumbnails/a_thumb\.webp\?v=\d+&root=0$", ) first = int(re.search(r"v=(\d+)", entry["thumbnail_url"]).group(1)) self.assertEqual(first, int(thumb.stat().st_mtime)) @@ -960,7 +965,9 @@ class TestThumbnailFilesAreNeverSeenHalfWritten(ThumbnailTestCase): async def test_versioned_thumbnail_url_is_served(self): make_png(self.output_dir / "a.png", size=(64, 64)) self.api._generate_thumbnails_sync( - self.output_dir, self.output_dir / "thumbnails", (32, 32) + self.output_dir, + self.output_dir / "thumbnails", + ThumbnailSettings("webp", 80, 32), ) entry = images_module._output_image_entry( self.output_dir / "a.png", self.output_dir, 0 @@ -969,7 +976,7 @@ class TestThumbnailFilesAreNeverSeenHalfWritten(ThumbnailTestCase): resp = await self.client.request("GET", entry["thumbnail_url"]) self.assertEqual(resp.status, 200) - self.assertEqual(resp.headers["Content-Type"], "image/png") + self.assertEqual(resp.headers["Content-Type"], "image/webp") async def test_missing_image_404_is_not_cacheable(self): resp = await self.client.request( @@ -1024,7 +1031,9 @@ class TestMediaScanCaps(ImageAPITestCase): for name in ("a", "b", "c"): make_png(self.output_dir / f"{name}.png") self.api._generate_thumbnails_sync( - self.output_dir, self.output_dir / "thumbnails", (64, 64) + self.output_dir, + self.output_dir / "thumbnails", + ThumbnailSettings("webp", 80, 64), ) for name in ("d", "e", "f"): make_png(self.output_dir / f"{name}.png") @@ -1083,11 +1092,6 @@ class TestMediaScanCaps(ImageAPITestCase): self.assertEqual(targets, []) - def test_thumbnail_size_ignores_non_string_quality(self): - self.assertEqual(self.api._thumbnail_size({"w": 99999}), (300, 300)) - self.assertEqual(self.api._thumbnail_size(None), (300, 300)) - self.assertEqual(self.api._thumbnail_size("low"), (150, 150)) - class TestJsonBodyCaps(ImageAPITestCase): """JSON bodies are read in chunks and rejected past MAX_JSON_BODY_BYTES.""" @@ -1458,7 +1462,7 @@ class TestGalleryListing(ImageRouteCoverageCase): class TestThumbnailUnits(ImageRouteCoverageCase): - SIZE = (64, 64) + SIZE = ThumbnailSettings("webp", 80, 64) def test_generate_one_reports_vanished_source(self): result = self.api._generate_one( @@ -1471,11 +1475,9 @@ class TestThumbnailUnits(ImageRouteCoverageCase): src = self.output_dir / "clip.mp4" src.write_bytes(b"\x00") with patch.object(self.api, "_generate_video_thumbnail", return_value=True): - ok = self.api._generate_one(src, self._dst("clip", ".jpg"), self.SIZE, True) + ok = self.api._generate_one(src, self._dst("clip"), self.SIZE, True) with patch.object(self.api, "_generate_video_thumbnail", return_value=False): - bad = self.api._generate_one( - src, self._dst("clip", ".jpg"), self.SIZE, True - ) + bad = self.api._generate_one(src, self._dst("clip"), self.SIZE, True) self.assertEqual((ok["action"], ok["type"]), ("generated", "video")) self.assertEqual(bad["action"], "error") @@ -1498,7 +1500,7 @@ class TestThumbnailUnits(ImageRouteCoverageCase): ) self.assertEqual(result["count"], 2) self.assertTrue(self._dst("alpha").is_file()) - self.assertTrue(self._dst("photo", ".jpg").is_file()) + self.assertTrue(self._dst("photo").is_file()) async def test_emit_sse_swallows_write_errors(self): response = MagicMock() @@ -1521,14 +1523,15 @@ class TestThumbnailUnits(ImageRouteCoverageCase): ) self.assertEqual(status, 500) - def _dst(self, name, suffix=".png"): + def _dst(self, name, suffix=".webp"): return self.output_dir / "thumbnails" / f"{name}_thumb{suffix}" class TestVideoThumbnailFallbacks(ImageRouteCoverageCase): """_generate_video_thumbnail degrades: cv2 -> ffmpeg -> placeholder.""" - SIZE = (48, 48) + # JPEG keeps these checks format-specific; #317 settings carry the edge. + SIZE = ThumbnailSettings("jpeg", 85, 48) @property def video(self): @@ -1581,7 +1584,7 @@ class TestVideoThumbnailFallbacks(ImageRouteCoverageCase): ) self.assertTrue(ok) with Image.open(self.thumb) as img: - self.assertEqual((img.format, img.size), ("JPEG", self.SIZE)) + self.assertEqual((img.format, img.size), ("JPEG", self.SIZE.size)) def test_cv2_crash_is_contained(self): cv2 = MagicMock() @@ -1614,18 +1617,26 @@ class TestVideoThumbnailFallbacks(ImageRouteCoverageCase): ok = self.api._generate_video_thumbnail(self.video, self.thumb, self.SIZE) self.assertTrue(ok) with Image.open(self.thumb) as img: - self.assertEqual(img.size, self.SIZE) + self.assertEqual(img.size, self.SIZE.size) @staticmethod - def _process(returncode, stderr=""): + def _process(returncode, stderr=b"", stdout=b""): process = MagicMock(returncode=returncode) - process.communicate.return_value = ("", stderr) + process.communicate.return_value = (stdout, stderr) return process + @staticmethod + def _png_frame(size=(96, 64)): + import io + + buffer = io.BytesIO() + Image.new("RGB", size, (200, 0, 0)).save(buffer, "PNG") + return buffer.getvalue() + def test_ffmpeg_failure_yields_placeholder(self): with ( patch.dict(sys.modules, {"cv2": None}), - patch("subprocess.Popen", return_value=self._process(1, "no stream")), + patch("subprocess.Popen", return_value=self._process(1, b"no stream")), ): ok = self.api._generate_video_thumbnail(self.video, self.thumb, self.SIZE) self.assertTrue(ok) @@ -1634,18 +1645,23 @@ class TestVideoThumbnailFallbacks(ImageRouteCoverageCase): def test_ffmpeg_success(self): with ( patch.dict(sys.modules, {"cv2": None}), - patch("subprocess.Popen", return_value=self._process(0)) as run, + patch( + "subprocess.Popen", + return_value=self._process(0, stdout=self._png_frame()), + ) as run, ): ok = self.api._generate_video_thumbnail(self.video, self.thumb, self.SIZE) self.assertTrue(ok) self.assertEqual(run.call_args.args[0][0], "ffmpeg") + with Image.open(self.thumb) as img: + # ffmpeg's frame (a PNG on stdout) fitted inside the edge (#317). + self.assertEqual((img.format, img.size), ("JPEG", (48, 32))) def test_ffmpeg_is_tracked_and_a_hold_before_the_rename_writes_nothing(self): tracked = [] def writing(cmd, **kwargs): - Path(cmd[-1]).write_bytes(b"\xff\xd8 frame") - return self._process(0) + return self._process(0, stdout=self._png_frame()) with ( patch.dict(sys.modules, {"cv2": None}), @@ -2061,7 +2077,7 @@ class TestThumbnailStartupStatus(ThumbnailTestCase): ): with self.subTest(route=route): for root in (self.output_dir, second): - for thumb in (root / "thumbnails").glob("*_thumb.png"): + for thumb in (root / "thumbnails").glob("*_thumb.webp"): thumb.unlink() response = await self.client.post(route, json={}) if route == self.PROGRESS_URL: @@ -2069,7 +2085,7 @@ class TestThumbnailStartupStatus(ThumbnailTestCase): else: data = await response.json() self.assertEqual(data["count"], 2) - self.assertTrue((second / "thumbnails" / "b_thumb.png").exists()) + self.assertTrue((second / "thumbnails" / "b_thumb.webp").exists()) status = await (await self.client.get(self.STATUS_URL)).json() self.assertEqual(status["pending"], 0) self.assertEqual(status["total_images"], 2) @@ -2121,7 +2137,7 @@ class TestGalleryRootIdentity(ThumbnailTestCase): root = roots[image["root_index"]] for key, path in ( ("url", root / "same.png"), - ("thumbnail_url", root / "thumbnails" / "same_thumb.png"), + ("thumbnail_url", root / "thumbnails" / "same_thumb.webp"), ): response = await self.client.get(image[key]) self.assertEqual(response.status, 200) diff --git a/tests/test_api_review.py b/tests/test_api_review.py index d3810437..6eeaa89d 100644 --- a/tests/test_api_review.py +++ b/tests/test_api_review.py @@ -408,9 +408,10 @@ class TestQueueReads(ReviewRoutesTestCase): scope = item["target"]["root_scope"] self.assertEqual( item["thumbnail_url"], - "/prompt_manager/images/serve/thumbnails/sub/f0_thumb.png" + "/prompt_manager/images/serve/thumbnails/sub/f0_thumb.webp" f"?root=0&root_scope={scope}", ) + # A legacy preview name is still listed until it is converted (#317). thumb = self.output_dir / "thumbnails" / "sub" / "f0_thumb.png" thumb.parent.mkdir(parents=True) write_png(thumb, prompt=ambiguous_graph("cat", "dog"), color=(1, 2, 3)) diff --git a/tests/test_thumbnail_auto.py b/tests/test_thumbnail_auto.py index 821b45e3..d21677e9 100644 --- a/tests/test_thumbnail_auto.py +++ b/tests/test_thumbnail_auto.py @@ -143,15 +143,15 @@ class TestListingsAlwaysCarryAThumbnailUrl(AutoThumbnailCase): entry = _entry(self.api, self.output_dir, "sub/new.png") self.assertEqual( entry["thumbnail_url"], - f"{SERVE}thumbnails/sub/new_thumb.png?root=0", + f"{SERVE}thumbnails/sub/new_thumb.webp?root=0", ) self.assertFalse((self.output_dir / "thumbnails").exists(), "no write") - def test_missing_video_preview_is_a_jpeg_url(self): + def test_missing_video_preview_url_is_in_the_configured_format(self): (self.output_dir / "clip.mp4").write_bytes(b"not really a video") entry = _entry(self.api, self.output_dir, "clip.mp4") self.assertEqual( - entry["thumbnail_url"], f"{SERVE}thumbnails/clip_thumb.jpg?root=0" + entry["thumbnail_url"], f"{SERVE}thumbnails/clip_thumb.webp?root=0" ) def test_listing_says_whether_the_preview_file_exists(self): @@ -160,7 +160,7 @@ class TestListingsAlwaysCarryAThumbnailUrl(AutoThumbnailCase): self.assertIs( _entry(self.api, self.output_dir, "a.png")["thumbnail_ready"], False ) - make_png(self.output_dir / "thumbnails" / "a_thumb.png") + make_png(self.output_dir / "thumbnails" / "a_thumb.webp") self.assertIs( _entry(self.api, self.output_dir, "a.png")["thumbnail_ready"], True ) @@ -174,11 +174,11 @@ class TestListingsAlwaysCarryAThumbnailUrl(AutoThumbnailCase): def test_existing_preview_keeps_its_mtime_version(self): make_png(self.output_dir / "a.png", size=(64, 64)) - make_png(self.output_dir / "thumbnails" / "a_thumb.png") + make_png(self.output_dir / "thumbnails" / "a_thumb.webp") entry = _entry(self.api, self.output_dir, "a.png") self.assertRegex( entry["thumbnail_url"], - r"^/prompt_manager/images/serve/thumbnails/a_thumb\.png\?v=\d+&root=0$", + r"^/prompt_manager/images/serve/thumbnails/a_thumb\.webp\?v=\d+&root=0$", ) async def test_workspace_listing_never_returns_a_null_thumbnail(self): @@ -190,7 +190,7 @@ class TestListingsAlwaysCarryAThumbnailUrl(AutoThumbnailCase): urls = [image["thumbnail_url"] for image in data["images"]] self.assertEqual(len(urls), 3) for url in urls: - self.assertIn("_thumb.png", url) + self.assertIn("_thumb.webp", url) class TestThumbnailOnFirstView(AutoThumbnailCase): @@ -201,31 +201,33 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): resp = await self.client.get(url) self.assertEqual(resp.status, 200) - self.assertEqual(resp.headers["Content-Type"], "image/png") + self.assertEqual(resp.headers["Content-Type"], "image/webp") body = await resp.read() - thumb = self.output_dir / "thumbnails" / "sub" / "big_thumb.png" + thumb = self.output_dir / "thumbnails" / "sub" / "big_thumb.webp" self.assertTrue(thumb.is_file()) self.assertEqual(body, thumb.read_bytes()) with Image.open(thumb) as img: - self.assertEqual(max(img.size), auto.AUTO_THUMBNAIL_EDGE) + self.assertEqual(max(img.size), 512) # No temp sibling is ever left next to the preview. - self.assertEqual(os.listdir(thumb.parent), ["big_thumb.png"]) + self.assertEqual(os.listdir(thumb.parent), ["big_thumb.webp"]) async def test_upper_case_extension_source_is_found(self): make_png(self.output_dir / "Shot.PNG", size=(64, 64)) url = _entry(self.api, self.output_dir, "Shot.PNG")["thumbnail_url"] - self.assertIn("thumbnails/Shot_thumb.png", url) + self.assertIn("thumbnails/Shot_thumb.webp", url) resp = await self.client.get(url) self.assertEqual(resp.status, 200) - self.assertTrue((self.output_dir / "thumbnails" / "Shot_thumb.png").is_file()) + self.assertTrue((self.output_dir / "thumbnails" / "Shot_thumb.webp").is_file()) - async def test_video_preview_is_generated_as_jpeg(self): + async def test_video_preview_is_generated_in_the_configured_format(self): + # #317: a video's preview is written like an image's (WebP default); + # an undecodable clip's preview is the placeholder frame. (self.output_dir / "clip.mp4").write_bytes(b"not really a video") url = _entry(self.api, self.output_dir, "clip.mp4")["thumbnail_url"] resp = await self.client.get(url) self.assertEqual(resp.status, 200) - self.assertEqual(resp.headers["Content-Type"], "image/jpeg") - self.assertTrue((self.output_dir / "thumbnails" / "clip_thumb.jpg").is_file()) + self.assertEqual(resp.headers["Content-Type"], "image/webp") + self.assertTrue((self.output_dir / "thumbnails" / "clip_thumb.webp").is_file()) async def test_concurrent_views_generate_once(self): make_png(self.output_dir / "a.png", size=(256, 256)) @@ -298,14 +300,15 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): for name in names: stem = name[:-4] self.assertTrue( - (self.output_dir / "thumbnails" / f"{stem}_thumb.png").is_file() + (self.output_dir / "thumbnails" / f"{stem}_thumb.webp").is_file() ) async def test_requests_beyond_the_cap_get_the_fallback_at_once(self): names = [f"c{i}.png" for i in range(auto.ON_DEMAND_LIMIT)] for name in names + ["late.png"]: make_png(self.output_dir / name, size=(64, 64)) - (self.output_dir / "late.mp4").write_bytes(b"not a video") + # Another stem: same-stem files share one preview name (#317 R2). + (self.output_dir / "latev.mp4").write_bytes(b"not a video") entered, release, state = self._gate_writes() listings = [] real_scandir = auto.os.scandir @@ -327,25 +330,25 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): for n in names ] await self._writers(state, auto.ON_DEMAND_LIMIT) - image = await self.client.get(f"{SERVE}thumbnails/late_thumb.png?root=0") + image = await self.client.get(f"{SERVE}thumbnails/late_thumb.webp?root=0") self.assertEqual(image.status, 200) self.assertEqual(image.headers["Cache-Control"], "no-store") self.assertEqual( await image.read(), (self.output_dir / "late.png").read_bytes() ) - video = await self.client.get(f"{SERVE}thumbnails/late_thumb.jpg?root=0") + video = await self.client.get(f"{SERVE}thumbnails/latev_thumb.webp?root=0") self.assertEqual(video.status, 200) self.assertEqual(video.headers["Content-Type"], "image/jpeg") self.assertEqual(video.headers["Cache-Control"], "no-store") await video.read() - missing = await self.client.get(f"{SERVE}thumbnails/Nope_thumb.png?root=0") + missing = await self.client.get(f"{SERVE}thumbnails/Nope_thumb.webp?root=0") self.assertEqual(missing.status, 404) # M2: no folder is listed for a request the cap turned away. self.assertEqual(listings, []) release.set() responses = [await task for task in tasks] self.assertEqual([r.status for r in responses], [200] * len(names)) - self.assertFalse((self.output_dir / "thumbnails" / "late_thumb.png").exists()) + self.assertFalse((self.output_dir / "thumbnails" / "late_thumb.webp").exists()) async def test_mixed_case_folder_listing_runs_once_inside_admission(self): make_png(self.output_dir / "Shot.Png", size=(64, 64)) @@ -357,13 +360,13 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): return real_scandir(path) with patch.object(auto.os, "scandir", counting): - resp = await self.client.get(f"{SERVE}thumbnails/Shot_thumb.png?root=0") + resp = await self.client.get(f"{SERVE}thumbnails/Shot_thumb.webp?root=0") self.assertEqual(resp.status, 200) # A case-insensitive filesystem (macOS, Windows) finds it by its exact # name; a case-sensitive one lists the folder once. found_by_name = (self.output_dir / "Shot.png").exists() self.assertEqual(len(listings), 0 if found_by_name else 1) - self.assertTrue((self.output_dir / "thumbnails" / "Shot_thumb.png").is_file()) + self.assertTrue((self.output_dir / "thumbnails" / "Shot_thumb.webp").is_file()) async def test_broken_file_falls_back_to_the_original_and_is_remembered(self): broken = self.output_dir / "broken.png" @@ -380,7 +383,9 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): self.assertEqual(resp.headers["Cache-Control"], "no-store") self.assertEqual(await resp.read(), broken.read_bytes()) self.assertEqual(calls, ["broken.png"], "a known failure is not retried") - self.assertFalse((self.output_dir / "thumbnails" / "broken_thumb.png").exists()) + self.assertFalse( + (self.output_dir / "thumbnails" / "broken_thumb.webp").exists() + ) status = await ( await self.client.get("/prompt_manager/images/thumbnails/status") @@ -394,7 +399,9 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): third = await self.client.get(url) self.assertEqual(third.status, 200) self.assertEqual(calls, ["broken.png", "broken.png"]) - self.assertTrue((self.output_dir / "thumbnails" / "broken_thumb.png").is_file()) + self.assertTrue( + (self.output_dir / "thumbnails" / "broken_thumb.webp").is_file() + ) async def test_locked_preview_is_not_remembered_as_a_failure(self): make_png(self.output_dir / "a.png", size=(64, 64)) @@ -411,10 +418,10 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): resp = await self.client.get(url) self.assertEqual(resp.status, 200) - self.assertTrue((self.output_dir / "thumbnails" / "a_thumb.png").is_file()) + self.assertTrue((self.output_dir / "thumbnails" / "a_thumb.webp").is_file()) async def test_no_source_is_a_plain_uncacheable_404(self): - resp = await self.client.get(f"{SERVE}thumbnails/nope_thumb.png?root=0") + resp = await self.client.get(f"{SERVE}thumbnails/nope_thumb.webp?root=0") self.assertEqual(resp.status, 404) self.assertEqual(resp.headers["Cache-Control"], "no-store") text = await resp.text() @@ -424,13 +431,13 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): def test_drive_unc_and_absolute_segments_are_refused_as_text(self): # M1: a drive or UNC segment would reset the anchor of a joined path. for path in ( - "thumbnails/D:/x_thumb.png", - "thumbnails/D:x_thumb.png", - "thumbnails/\\\\server\\share/x_thumb.png", - "thumbnails//server/share/x_thumb.png", - "/thumbnails/x_thumb.png", - "thumbnails/a/./x_thumb.png", - "thumbnails/C:\\Users\\x_thumb.png", + "thumbnails/D:/x_thumb.webp", + "thumbnails/D:x_thumb.webp", + "thumbnails/\\\\server\\share/x_thumb.webp", + "thumbnails//server/share/x_thumb.webp", + "/thumbnails/x_thumb.webp", + "thumbnails/a/./x_thumb.webp", + "thumbnails/C:\\Users\\x_thumb.webp", ): with self.subTest(path=path): self.assertFalse(auto._is_preview_request(path)) @@ -442,9 +449,9 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): patch.object(auto.os, "stat") as stat, ): for path in ( - "thumbnails/D:/x_thumb.png", - "thumbnails/\\\\server\\share/x_thumb.png", - "/thumbnails/x_thumb.png", + "thumbnails/D:/x_thumb.webp", + "thumbnails/\\\\server\\share/x_thumb.webp", + "/thumbnails/x_thumb.webp", ): answer = await self.api._serve_missing_thumbnail( [self.output_dir], path @@ -457,8 +464,8 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): async def test_traversal_is_refused_before_any_filesystem_access(self): with patch.object(auto, "_thumbnail_source") as source: for path in ( - "thumbnails/../../etc/passwd_thumb.png", - "thumbnails/..%2F..%2Fx_thumb.png", + "thumbnails/../../etc/passwd_thumb.webp", + "thumbnails/..%2F..%2Fx_thumb.webp", ): resp = await self.client.get(f"{SERVE}{path}?root=0") self.assertIn(resp.status, (403, 404)) @@ -468,7 +475,7 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): outside = Path(self._output_tmp.name) / "outside.png" make_png(outside, size=(64, 64)) self._make_symlink(outside, self.output_dir / "link.png") - resp = await self.client.get(f"{SERVE}thumbnails/link_thumb.png?root=0") + resp = await self.client.get(f"{SERVE}thumbnails/link_thumb.webp?root=0") self.assertEqual(resp.status, 404) self.assertFalse((self.output_dir / "thumbnails").exists()) @@ -477,7 +484,7 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): elsewhere = Path(self._output_tmp.name) / "elsewhere" elsewhere.mkdir() self._make_symlink(elsewhere, self.output_dir / "thumbnails") - resp = await self.client.get(f"{SERVE}thumbnails/a_thumb.png?root=0") + resp = await self.client.get(f"{SERVE}thumbnails/a_thumb.webp?root=0") self.assertEqual(resp.status, 200) self.assertEqual(resp.headers["Cache-Control"], "no-store") self.assertEqual(os.listdir(elsewhere), []) @@ -524,7 +531,7 @@ class TestThumbnailOnFirstView(AutoThumbnailCase): _entry(self.api, self.output_dir, name)["thumbnail_url"] ) self.assertEqual(resp.status, 200) - thumb = self.output_dir / "thumbnails" / f"a{index}_thumb.png" + thumb = self.output_dir / "thumbnails" / f"a{index}_thumb.webp" self.assertTrue(thumb.is_file()) job.finish("finished") @@ -565,8 +572,12 @@ class TestVideoPreviewsNeverFallBackToTheVideo(AutoThumbnailCase): cv2 = MagicMock() cv2.VideoCapture.return_value.isOpened.return_value = False with patch.dict(sys.modules, {"cv2": cv2}): - await self._assert_placeholder(f"{SERVE}thumbnails/clip_thumb.jpg?root=0") - self.assertTrue((self.output_dir / "thumbnails" / "clip_thumb.jpg").is_file()) + resp = await self.client.get(f"{SERVE}thumbnails/clip_thumb.webp?root=0") + # The placeholder frame became the preview file, in the format (#317). + self.assertEqual(resp.status, 200) + self.assertEqual(resp.headers["Content-Type"], "image/webp") + made = self.output_dir / "thumbnails" / "clip_thumb.webp" + self.assertEqual(await resp.read(), made.read_bytes()) async def test_a_held_video_preview_is_the_placeholder_and_writes_nothing(self): from py.api.jobs import RESTORE_JOB_KIND @@ -574,7 +585,7 @@ class TestVideoPreviewsNeverFallBackToTheVideo(AutoThumbnailCase): self._video() self._hold(RESTORE_JOB_KIND) resp = await self._assert_placeholder( - f"{SERVE}thumbnails/clip_thumb.jpg?root=0" + f"{SERVE}thumbnails/clip_thumb.webp?root=0" ) self.assertEqual(resp.headers["Cache-Control"], "no-store") self.assertFalse((self.output_dir / "thumbnails").exists()) @@ -585,7 +596,7 @@ class TestVideoPreviewsNeverFallBackToTheVideo(AutoThumbnailCase): elsewhere.mkdir() self._make_symlink(elsewhere, self.output_dir / "thumbnails") resp = await self._assert_placeholder( - f"{SERVE}thumbnails/clip_thumb.jpg?root=0" + f"{SERVE}thumbnails/clip_thumb.webp?root=0" ) self.assertEqual(resp.headers["Cache-Control"], "no-store") self.assertEqual(os.listdir(elsewhere), []) @@ -594,7 +605,7 @@ class TestVideoPreviewsNeverFallBackToTheVideo(AutoThumbnailCase): self._video() with patch.object(self.api, "_generate_video_thumbnail", return_value=False): resp = await self._assert_placeholder( - f"{SERVE}thumbnails/clip_thumb.jpg?root=0" + f"{SERVE}thumbnails/clip_thumb.webp?root=0" ) self.assertEqual(resp.headers["Cache-Control"], "no-store") @@ -653,6 +664,9 @@ class TestWriteSafety(AutoThumbnailCase): class TestShutdown(AutoThumbnailCase): async def test_no_thread_remains_after_cleanup_and_capture_cannot_restart(self): + from py.api import thumbnail_encode + + self.addCleanup(thumbnail_encode.reset_encoders) path = make_png(self.output_dir / "a.png", size=(64, 64)) self.assertEqual(self.api.queue_capture_thumbnail(str(path)), "queued") thumbnailer = self.api._auto_thumbnails() @@ -676,6 +690,9 @@ class TestShutdown(AutoThumbnailCase): self.assertEqual(self.api.queue_capture_thumbnail(str(other)), "skipped") restarted = self.api._auto_thumbnailer self.assertTrue(restarted is None or restarted._worker is None) + # #317: no encoder process (ffmpeg, cjxl) starts after the cleanup. + with self.assertRaises(thumbnail_encode.EncoderClosed): + thumbnail_encode.run_encoder([sys.executable, "-c", "pass"]) async def test_an_idle_worker_leaves(self): path = make_png(self.output_dir / "a.png", size=(64, 64)) @@ -689,7 +706,7 @@ class TestShutdown(AutoThumbnailCase): other = make_png(self.output_dir / "b.png", size=(64, 64)) self.assertEqual(self.api.queue_capture_thumbnail(str(other)), "queued") await self._idle() - self.assertTrue((self.output_dir / "thumbnails" / "b_thumb.png").is_file()) + self.assertTrue((self.output_dir / "thumbnails" / "b_thumb.webp").is_file()) async def test_a_reload_stops_the_old_worker(self): entered, release, _state = self._gate_writes() @@ -761,7 +778,7 @@ class TestHistorySharesTheAdmission(AutoThumbnailCase): self.assertEqual(answers["history"], auto.READY) resp = answers["view"] self.assertEqual(resp.status, 200) - self.assertEqual(resp.headers["Content-Type"], "image/png") + self.assertEqual(resp.headers["Content-Type"], "image/webp") self.assertEqual(await resp.read(), dst.read_bytes()) async def test_history_joining_a_view_flight_gets_the_outcome(self): @@ -773,9 +790,10 @@ class TestHistorySharesTheAdmission(AutoThumbnailCase): from py.api.jobs import RESTORE_JOB_KIND make_png(self.output_dir / "c.png", size=(64, 64)) - (self.output_dir / "c.mp4").write_bytes(b"AV1 stream no decoder here") + # Another stem: same-stem files share one preview name (#317 R2). + (self.output_dir / "cv.mp4").write_bytes(b"AV1 stream no decoder here") self._hold(RESTORE_JOB_KIND) - for name, is_video in (("c.png", False), ("c.mp4", True)): + for name, is_video in (("c.png", False), ("cv.mp4", True)): with self.subTest(name=name): src = self.output_dir / name url = _entry(self.api, self.output_dir, name)["thumbnail_url"] @@ -797,9 +815,9 @@ class TestHistorySharesTheAdmission(AutoThumbnailCase): def test_history_and_serve_route_share_one_flight_key(self): root = self.output_dir - preview = root / "thumbnails" / "sub" / "a_thumb.png" + preview = root / "thumbnails" / "sub" / "a_thumb.webp" self.assertEqual( - auto.flight_key(root, ("thumbnails", "sub", "a_thumb.png")), + auto.flight_key(root, ("thumbnails", "sub", "a_thumb.webp")), auto.flight_key( Path(os.path.normcase(os.path.realpath(root))), preview.relative_to(root).parts, @@ -813,7 +831,7 @@ class TestThumbnailOnCapture(AutoThumbnailCase): outcome = self.api.queue_capture_thumbnail(str(path)) self.assertEqual(outcome, "queued") await self._idle() - thumb = self.output_dir / "thumbnails" / "run" / "ComfyUI_0001_thumb.png" + thumb = self.output_dir / "thumbnails" / "run" / "ComfyUI_0001_thumb.webp" self.assertTrue(thumb.is_file()) self.assertIn( "v=", @@ -837,7 +855,7 @@ class TestThumbnailOnCapture(AutoThumbnailCase): async def test_files_outside_the_roots_and_previews_are_ignored(self): outside = make_png(Path(self._output_tmp.name) / "x.png") - preview = make_png(self.output_dir / "thumbnails" / "y_thumb.png") + preview = make_png(self.output_dir / "thumbnails" / "y_thumb.webp") text = self.output_dir / "notes.txt" text.write_text("x") for path in (outside, preview, text, self.output_dir / "missing.png"): @@ -873,7 +891,7 @@ class TestThumbnailOnCapture(AutoThumbnailCase): self.assertLessEqual(state["peak"], 2) for i in range(5): self.assertTrue( - (self.output_dir / "thumbnails" / f"w{i}_thumb.png").is_file() + (self.output_dir / "thumbnails" / f"w{i}_thumb.webp").is_file() ) async def test_worker_survives_a_failing_write(self): @@ -884,7 +902,7 @@ class TestThumbnailOnCapture(AutoThumbnailCase): await self._idle() self.api.queue_capture_thumbnail(str(good)) await self._idle() - self.assertTrue((self.output_dir / "thumbnails" / "good_thumb.png").is_file()) + self.assertTrue((self.output_dir / "thumbnails" / "good_thumb.webp").is_file()) self.assertEqual(self.api._auto_thumbnails().failure_count, 1) async def test_worker_writes_nothing_during_a_restore(self): @@ -919,7 +937,7 @@ class TestThumbnailOnCapture(AutoThumbnailCase): await self._idle() for name in ("first", "second"): self.assertTrue( - (self.output_dir / "thumbnails" / f"{name}_thumb.png").is_file() + (self.output_dir / "thumbnails" / f"{name}_thumb.webp").is_file() ) async def test_a_worker_that_cannot_start_is_started_by_the_next_enqueue(self): @@ -940,7 +958,7 @@ class TestThumbnailOnCapture(AutoThumbnailCase): await self._idle() for name in ("first", "second"): self.assertTrue( - (self.output_dir / "thumbnails" / f"{name}_thumb.png").is_file() + (self.output_dir / "thumbnails" / f"{name}_thumb.webp").is_file() ) def test_queue_is_bounded(self): @@ -1041,7 +1059,7 @@ class TestScanHandsOffThumbnails(AutoThumbnailCase): await self._idle() for i in range(2): self.assertTrue( - (self.output_dir / "thumbnails" / f"s{i}_thumb.png").is_file() + (self.output_dir / "thumbnails" / f"s{i}_thumb.webp").is_file() ) async def test_a_scan_that_added_nothing_does_nothing(self): @@ -1091,12 +1109,12 @@ class TestScanHandsOffThumbnails(AutoThumbnailCase): outcome = await self.api._thumbnails_after_scan(job, [self.output_dir, other]) self.assertEqual(outcome, "queued") await self._idle() - self.assertTrue((other / "thumbnails" / "o_thumb.png").is_file()) + self.assertTrue((other / "thumbnails" / "o_thumb.webp").is_file()) self.assertFalse((self.output_dir / "thumbnails").exists()) async def test_nothing_missing_is_none(self): make_png(self.output_dir / "s.png", size=(32, 32)) - make_png(self.output_dir / "thumbnails" / "s_thumb.png") + make_png(self.output_dir / "thumbnails" / "s_thumb.webp") self.assertEqual( await self.api._thumbnails_after_scan(self._scan_job(1)), "none" ) diff --git a/tests/test_thumbnail_format.py b/tests/test_thumbnail_format.py new file mode 100644 index 00000000..18fa0796 --- /dev/null +++ b/tests/test_thumbnail_format.py @@ -0,0 +1,400 @@ +"""Every preview writer and lookup follows the thumbnail settings (#317). + +Automatic previews (capture and first view), the Activity job, the legacy +gallery routes, the listings, the history route, duplicates, enrichment and +the clear all name previews ``_thumb.``, +write them with the configured quality and size limit, and still find older +previews (``.png`` and friends) until they are converted. + +A thumbnail is only a grid or cover placeholder: inspection always reads the +original, and never resolves a request to a file under ``thumbnails/``. + +Ordering is driven by events, never by timers. +""" + +import asyncio +import json +import os +import shutil +import subprocess +import sys +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +try: + from tests.test_api_images import ThumbnailTestCase, make_png, parse_sse +except ImportError: # discovered with tests/ as the top-level directory + from test_api_images import ThumbnailTestCase, make_png, parse_sse + +from PIL import Image, PngImagePlugin # noqa: E402 + +import py.api.images as images_module # noqa: E402 +import py.api.thumbnail_auto as auto # noqa: E402 +from py.api import thumbnail_encode # noqa: E402 +from py.api.admin import _remove_thumbnail # noqa: E402 +from py.config import PromptManagerConfig # noqa: E402 + +SERVE = "/prompt_manager/images/serve/" +STATUS = "/prompt_manager/images/thumbnails/status" +PLAN = "/prompt_manager/images/thumbnails/plan" +JOB = "/prompt_manager/jobs/thumbnails" +WAIT = 10 +HAVE_FFMPEG = bool(shutil.which("ffmpeg")) + + +def graph(positive="portrait", negative="blurry"): + return { + "1": {"class_type": "CLIPTextEncode", "inputs": {"text": positive}}, + "2": {"class_type": "CLIPTextEncode", "inputs": {"text": negative}}, + "3": { + "class_type": "KSampler", + "inputs": {"positive": ["1", 0], "negative": ["2", 0]}, + }, + } + + +def png_with_prompt(path, size=(64, 64)): + info = PngImagePlugin.PngInfo() + info.add_text("prompt", json.dumps(graph())) + info.add_text("workflow", json.dumps({"nodes": [], "links": []})) + path.parent.mkdir(parents=True, exist_ok=True) + with Image.new("RGB", size, (9, 90, 200)) as image: + image.save(path, pnginfo=info) + return path + + +class FormatCase(ThumbnailTestCase): + async def asyncSetUp(self): + await super().asyncSetUp() + self._orig = PromptManagerConfig.get_config() + self.addCleanup(PromptManagerConfig.update_config, self._orig) + self._settings("webp", 80, 512) + thumbnail_encode.reset_encoders() + self.addCleanup(thumbnail_encode.reset_encoders) + self.addCleanup(self.api.close_auto_thumbnails) + + def _settings(self, fmt, quality, edge): + PromptManagerConfig.update_config( + {"thumbnails": {"format": fmt, "quality": quality, "edge": edge}} + ) + + def _thumb(self, name, suffix=".webp"): + return self.output_dir / "thumbnails" / f"{name}_thumb{suffix}" + + async def _idle(self): + loop = asyncio.get_running_loop() + thumbnailer = self.api._auto_thumbnails() + self.assertTrue(await loop.run_in_executor(None, thumbnailer.wait_idle, WAIT)) + + async def _capture(self, path): + self.assertEqual(self.api.queue_capture_thumbnail(path), "queued") + await self._idle() + + async def _finished(self, job_id): + job = self.api._jobs.get(job_id) + await asyncio.wait_for(job.task, WAIT) + return job + + def _format_of(self, path): + with Image.open(path) as img: + return img.format, img.size + + +class TestCaptureAndViewFollowTheSettings(FormatCase): + async def test_capture_writes_webp_at_the_size_limit(self): + self._settings("webp", 80, 200) + src = make_png(self.output_dir / "a.png", size=(800, 400)) + await self._capture(src) + self.assertEqual(self._format_of(self._thumb("a")), ("WEBP", (200, 100))) + self.assertFalse(self._thumb("a", ".png").exists()) + + async def test_capture_writes_jpeg_when_configured(self): + self._settings("jpeg", 50, 300) + src = make_png(self.output_dir / "sub" / "b.png", size=(600, 600)) + await self._capture(src) + made = self.output_dir / "thumbnails" / "sub" / "b_thumb.jpg" + self.assertEqual(self._format_of(made), ("JPEG", (300, 300))) + + async def test_quality_reaches_the_encoder(self): + noisy = Image.effect_noise((400, 400), 80).convert("RGB") + for name, quality in (("low", 10), ("high", 95)): + noisy.save(self.output_dir / f"{name}.png") + self._settings("webp", quality, 512) + await self._capture(self.output_dir / f"{name}.png") + self.assertLess( + self._thumb("low").stat().st_size, self._thumb("high").stat().st_size + ) + + async def test_an_animated_source_previews_as_a_still(self): + path = self.output_dir / "anim.gif" + frames = [Image.new("RGB", (64, 64), c) for c in ((250, 0, 0), (0, 0, 250))] + frames[0].save(path, save_all=True, append_images=frames[1:], duration=40) + await self._capture(path) + with Image.open(self._thumb("anim")) as img: + self.assertEqual(getattr(img, "n_frames", 1), 1) + self.assertGreater(img.convert("RGB").getpixel((32, 32))[0], 200) + + async def test_a_video_preview_is_the_configured_format(self): + frame = Image.new("RGB", (640, 360), (0, 200, 0)) + clip = self.output_dir / "clip.mp4" + clip.write_bytes(b"not decoded") + with patch.object(self.api, "_opencv_frame", return_value=frame): + await self._capture(clip) + self.assertEqual(self._format_of(self._thumb("clip"))[0], "WEBP") + self.assertFalse(self._thumb("clip", ".jpg").exists()) + + @unittest.skipUnless(HAVE_FFMPEG, "no ffmpeg") + async def test_ffmpeg_video_frame_is_encoded_in_the_format(self): + clip = self.output_dir / "real.mp4" + subprocess.run( + ["ffmpeg", "-v", "quiet", "-f", "lavfi", "-i", + "color=c=red:s=320x240:d=2", "-y", str(clip)], + check=True, timeout=60, + ) # fmt: skip + with patch.object(self.api, "_opencv_frame", return_value=None): + await self._capture(clip) + fmt, size = self._format_of(self._thumb("real")) + self.assertEqual(fmt, "WEBP") + self.assertEqual(size, (320, 240)) + + async def test_view_makes_the_preview_and_sends_its_type(self): + make_png(self.output_dir / "v.png", size=(64, 64)) + resp = await self.client.get(SERVE + "thumbnails/v_thumb.webp") + self.assertEqual(resp.status, 200) + self.assertEqual(resp.headers["Content-Type"], "image/webp") + self.assertEqual(resp.headers["Cache-Control"], "public, max-age=3600") + self.assertTrue(self._thumb("v").is_file()) + + async def test_a_jxl_preview_is_served_as_image_jxl(self): + make_png(self.output_dir / "j.png") + thumb = self._thumb("j", ".jxl") + thumb.parent.mkdir(parents=True, exist_ok=True) + thumb.write_bytes(b"\xff\x0a" + b"\x00" * 30) + resp = await self.client.get(SERVE + "thumbnails/j_thumb.jxl") + self.assertEqual(resp.status, 200) + self.assertEqual(resp.headers["Content-Type"], "image/jxl") + self.assertEqual(resp.headers["Cache-Control"], "public, max-age=3600") + + async def test_a_converted_away_legacy_url_answers_the_current_preview(self): + make_png(self.output_dir / "c.png") + await self._capture(self.output_dir / "c.png") + resp = await self.client.get(SERVE + "thumbnails/c_thumb.png?v=1") + self.assertEqual(resp.status, 200) + self.assertEqual(resp.headers["Content-Type"], "image/webp") + self.assertFalse(self._thumb("c", ".png").exists()) + + async def test_preview_requests_keep_the_text_checks_for_new_extensions(self): + accepted = ("thumbnails/a_thumb.jxl", "thumbnails/s/a_thumb.webp") + for text in accepted: + self.assertTrue(auto._is_preview_request(text), text) + refused = ( + "thumbnails/a_thumb.JXL", + "thumbnails\\a_thumb.jxl", + "thumbnails/C:a_thumb.webp", + "thumbnails//a_thumb.webp", + "thumbnails/./a_thumb.jxl", + "thumbnails/../a_thumb.jxl", + "thumbnails/thumbnails/a_thumb.webp", + "thumbnails/a_thumb.txt", + "thumbnails/_thumb.webp", + ) + for text in refused: + self.assertFalse(auto._is_preview_request(text), text) + make_png(self.outside_png()) + resp = await self.client.get(SERVE + "thumbnails/..%2F..%2Fout_thumb.webp") + self.assertIn(resp.status, (403, 404)) + self.assertFalse((self.comfy_dir / "out_thumb.webp").exists()) + + def outside_png(self): + return self.comfy_dir / "out.png" + + async def test_jxl_without_an_encoder_writes_webp_once(self): + self._settings("jxl", 80, 512) + src = make_png(self.output_dir / "x.png", size=(64, 64)) + calls = [] + original = self.api._generate_one + + def counting(*args, **kwargs): + calls.append(1) + return original(*args, **kwargs) + + self.api._generate_one = counting + with patch.object(thumbnail_encode, "detect_jxl_encoder", return_value=None): + await self._capture(src) + self.assertTrue(self._thumb("x").is_file()) + self.assertFalse(self._thumb("x", ".jxl").exists()) + self.assertEqual(self.api.queue_capture_thumbnail(src), "fresh") + resp = await self.client.get(SERVE + "thumbnails/x_thumb.jxl") + self.assertEqual(resp.headers["Content-Type"], "image/webp") + self.assertEqual(len(calls), 1) + + +class TestListingsFindAnyOwnedPreview(FormatCase): + def _listed(self, rel): + return images_module.media_thumbnail(self.output_dir, Path(rel), 0) + + def test_missing_preview_lists_the_configured_name(self): + make_png(self.output_dir / "m.png") + url, ready = self._listed("m.png") + self.assertEqual(url, SERVE + "thumbnails/m_thumb.webp?root=0") + self.assertFalse(ready) + + def test_a_legacy_preview_is_listed_until_a_current_one_exists(self): + make_png(self.output_dir / "l.png") + make_png(self._thumb("l", ".png")) + url, ready = self._listed("l.png") + self.assertTrue(url.startswith(SERVE + "thumbnails/l_thumb.png?v=")) + self.assertTrue(ready) + make_png(self._thumb("l")) + url, _ready = self._listed("l.png") + self.assertTrue(url.startswith(SERVE + "thumbnails/l_thumb.webp?v=")) + + def test_a_video_lists_its_legacy_jpg(self): + (self.output_dir / "v.mp4").write_bytes(b"x") + make_png(self._thumb("v", ".jpg")) + url, ready = self._listed("v.mp4") + self.assertTrue(url.startswith(SERVE + "thumbnails/v_thumb.jpg?v=")) + self.assertTrue(ready) + + def test_duplicates_listing_and_enrichment_use_the_existing_preview(self): + src = make_png(self.output_dir / "d.png") + make_png(self._thumb("d")) + described = self.api._describe_media_file(src, self.output_dir, "hash") + self.assertTrue( + described["thumbnail_url"].startswith(SERVE + "thumbnails/d_thumb.webp") + ) + images = self.api._enrich_images([{"image_path": str(src)}]) + self.assertTrue( + images[0]["thumbnail_url"].startswith(SERVE + "thumbnails/d_thumb.webp") + ) + + +class TestPlanStatusAndJobs(FormatCase): + async def test_status_counts_missing_outdated_and_ready(self): + make_png(self.output_dir / "new.png") # missing + make_png(self.output_dir / "old.png") + make_png(self._thumb("old", ".png")) # outdated (legacy PNG) + make_png(self.output_dir / "cur.png") + make_png(self._thumb("cur")) # current + body = await (await self.client.get(STATUS)).json() + self.assertEqual( + (body["pending"], body["outdated"], body["ready"], body["total_images"]), + (1, 1, 2, 3), + ) + plan = await (await self.client.get(PLAN, params={"mode": "missing"})).json() + self.assertEqual((plan["targets"], plan["fresh"], plan["outdated"]), (1, 1, 1)) + self.assertEqual( + (plan["format"], plan["quality"], plan["size"]), ("webp", 80, 512) + ) + + async def test_missing_job_writes_the_format_and_leaves_outdated_alone(self): + self._settings("webp", 70, 160) + make_png(self.output_dir / "n.png", size=(320, 320)) + make_png(self.output_dir / "o.png") + legacy = make_png(self._thumb("o", ".png")) + before = legacy.read_bytes() + resp = await self.client.post(JOB, json={"mode": "missing"}) + self.assertEqual(resp.status, 202) + job = await self._finished((await resp.json())["job"]["id"]) + self.assertEqual(job.state, "finished") + self.assertEqual(job.scope["format"], "webp") + self.assertEqual(self._format_of(self._thumb("n")), ("WEBP", (160, 160))) + self.assertFalse(self._thumb("o").exists()) + self.assertEqual(legacy.read_bytes(), before) + + async def test_the_legacy_stream_uses_the_server_settings(self): + # R3: the gallery's quality preset no longer sizes the preview. + self._settings("webp", 80, 200) + make_png(self.output_dir / "g.png", size=(800, 800)) + resp = await self.client.post(self.PROGRESS_URL + "?quality=high") + events = dict(parse_sse(await resp.text())) + self.assertEqual(events["complete"]["count"], 1) + self.assertEqual(self._format_of(self._thumb("g")), ("WEBP", (200, 200))) + + async def test_the_blocking_legacy_generate_uses_the_server_settings(self): + self._settings("jpeg", 80, 140) + make_png(self.output_dir / "h.png", size=(700, 700)) + resp = await self.client.post( + "/prompt_manager/images/generate-thumbnails", json={"quality": "low"} + ) + self.assertEqual(resp.status, 200) + made = self.output_dir / "thumbnails" / "h_thumb.jpg" + self.assertEqual(self._format_of(made), ("JPEG", (140, 140))) + + async def test_clear_removes_every_preview_format(self): + make_png(self.output_dir / "k.png") + for suffix in (".webp", ".png", ".jpg"): + make_png(self._thumb("k", suffix)) + jxl = self._thumb("k", ".jxl") + jxl.write_bytes(b"\xff\x0a") + resp = await self.client.post("/prompt_manager/images/clear-thumbnails") + self.assertEqual(resp.status, 200) + self.assertEqual((await resp.json())["cleared_files"], 4) + self.assertTrue((self.output_dir / "k.png").exists()) + + +class TestRemoval(FormatCase): + def test_removing_a_source_removes_its_previews_in_every_format(self): + make_png(self.output_dir / "r.png") + for suffix in (".png", ".webp"): + make_png(self._thumb("r", suffix)) + _remove_thumbnail(self.output_dir, Path("r.png")) + self.assertFalse(self._thumb("r", ".png").exists()) + self.assertFalse(self._thumb("r").exists()) + + def test_a_shared_preview_stays_while_another_source_owns_it(self): + # R2: r.png and r.mp4 share r_thumb.webp; the PNG's legacy preview is its own. + make_png(self.output_dir / "r.png") + (self.output_dir / "r.mp4").write_bytes(b"x") + make_png(self._thumb("r", ".png")) + make_png(self._thumb("r")) + _remove_thumbnail(self.output_dir, Path("r.png")) + self.assertFalse(self._thumb("r", ".png").exists()) + self.assertTrue(self._thumb("r").exists()) + + +class TestThumbnailsAreOnlyPlaceholders(FormatCase): + """Maintainer constraint: metadata always comes from the original.""" + + async def test_inspection_reads_the_original_not_its_webp_preview(self): + png_with_prompt(self.output_dir / "image.png") + await self._capture(self.output_dir / "image.png") + self.assertTrue(self._thumb("image").is_file()) + opened = [] + original_open = Image.open + + def recording(fp, *args, **kwargs): + opened.append(str(fp)) + return original_open(fp, *args, **kwargs) + + with patch.object(Image, "open", recording): + resp = await self.client.get( + "/prompt_manager/images/inspect/image.png", params={"root": "0"} + ) + body = await resp.json() + self.assertEqual(resp.status, 200) + self.assertEqual(body["prompts"]["positive"]["text"], "portrait") + self.assertEqual(body["prompts"]["negative"]["text"], "blurry") + self.assertFalse([p for p in opened if "thumbnails" in p], opened) + + async def test_inspection_never_resolves_a_thumbnail_path(self): + png_with_prompt(self.output_dir / "image.png") + png_with_prompt(self._thumb("image", ".png")) + await self._capture(self.output_dir / "image.png") + for rel in ( + "thumbnails/image_thumb.webp", + "thumbnails/image_thumb.png", + "Thumbnails/image_thumb.webp", + ): + resp = await self.client.get( + f"/prompt_manager/images/inspect/{rel}", params={"root": "0"} + ) + self.assertIn(resp.status, (403, 404), rel) + self.assertNotIn("portrait", await resp.text()) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_thumbnail_jobs.py b/tests/test_thumbnail_jobs.py index 86da03ea..33c97cdc 100644 --- a/tests/test_thumbnail_jobs.py +++ b/tests/test_thumbnail_jobs.py @@ -117,6 +117,14 @@ class ThumbnailJobCase(ThumbnailTestCase): self.addCleanup(patcher.stop) return lines + def _use(self, fmt="webp", quality=80, edge=512): + """Set the server's thumbnail settings for this test (#317).""" + saved = PromptManagerConfig.get_config()["thumbnails"] + self.addCleanup(PromptManagerConfig.update_config, {"thumbnails": saved}) + PromptManagerConfig.update_config( + {"thumbnails": {"format": fmt, "quality": quality, "edge": edge}} + ) + def _one_worker(self): original = PromptManagerConfig.WORKER_THREADS PromptManagerConfig.WORKER_THREADS = 1 # batches of 4 @@ -149,13 +157,10 @@ class TestThumbnailPlan(ThumbnailJobCase): os.utime(fresh, (source_time + 10, source_time + 10)) before = _tree_state(self.comfy_dir) + self._use("webp", 80, 512) with patch.object(images_module, "MAX_THUMBNAIL_FILES", 1): - missing = await self.client.get( - PLAN_URL, params={"mode": "missing", "size": "512", "quality": "high"} - ) - rebuild = await self.client.get( - PLAN_URL, params={"mode": "rebuild", "size": "256", "quality": "low"} - ) + missing = await self.client.get(PLAN_URL, params={"mode": "missing"}) + rebuild = await self.client.get(PLAN_URL, params={"mode": "rebuild"}) self.assertEqual(missing.status, 200) self.assertEqual( @@ -164,40 +169,38 @@ class TestThumbnailPlan(ThumbnailJobCase): "success": True, "targets": 1, "fresh": 1, + "outdated": 0, "remaining": 1, "roots": ["output"], "mode": "missing", + "format": "webp", "size": 512, - "quality": "high", + "quality": 80, }, ) data = await rebuild.json() # Rebuild regenerates fresh previews too: all three are work, one fits the cap. self.assertEqual((data["targets"], data["fresh"], data["remaining"]), (1, 1, 2)) - self.assertEqual( - (data["mode"], data["size"], data["quality"]), ("rebuild", 256, "low") - ) + self.assertEqual(data["mode"], "rebuild") self.assertEqual(_tree_state(self.comfy_dir), before) self.assertEqual(self.api._jobs.list(), []) - async def test_plan_defaults_to_missing_previews_at_the_concept_size(self): + async def test_plan_defaults_to_missing_previews_with_the_settings(self): + self._use("webp", 80, 512) make_png(self.output_dir / "a.png") data = await (await self.client.get(PLAN_URL)).json() self.assertEqual( - (data["mode"], data["size"], data["quality"], data["targets"]), - ("missing", 512, "medium", 1), + (data["mode"], data["format"], data["size"], data["quality"]), + ("missing", "webp", 512, 80), ) + self.assertEqual(data["targets"], 1) - async def test_size_bounds_validated(self): + async def test_mode_validated_and_size_quality_ignored(self): + # #317: format, quality and size limit are the server's settings; an + # older client's size or quality is ignored, an unknown mode refused. + self._use("webp", 80, 512) make_png(self.output_dir / "a.png") - for params in ( - {"size": "127"}, - {"size": "2049"}, - {"size": "512.5"}, - {"size": "big"}, - {"mode": "all"}, - {"quality": "ultra"}, - ): + for params in ({"mode": "all"}, {"mode": ""}): with self.subTest(params=params): resp = await self.client.get(PLAN_URL, params=params) body = await resp.json() @@ -205,24 +208,14 @@ class TestThumbnailPlan(ThumbnailJobCase): self.assertEqual( (body["success"], body["code"]), (False, "invalid-request") ) - for body in ( - {"size": 127}, - {"size": 2049}, - {"size": "512"}, - {"size": 512.0}, - {"size": True}, - {"mode": "rebuild-all"}, - {"quality": None}, - [], - ): + for body in ({"mode": "rebuild-all"}, {"mode": None}, []): with self.subTest(body=body): resp = await self.client.post(JOB_URL, json=body) self.assertEqual(resp.status, 400) self.assertEqual((await resp.json())["code"], "invalid-request") - for edge in (128, 2048): - with self.subTest(edge=edge): - resp = await self.client.get(PLAN_URL, params={"size": str(edge)}) - self.assertEqual(resp.status, 200) + resp = await self.client.get(PLAN_URL, params={"size": "9999", "quality": "x"}) + data = await resp.json() + self.assertEqual((data["size"], data["quality"]), (512, 80)) self.assertEqual(self.api._jobs.list(), []) self.assertFalse(self._thumb_path("a").exists()) @@ -233,7 +226,8 @@ class TestThumbnailJob(ThumbnailJobCase): make_png(self.output_dir / "sub" / "b.png", size=(64, 64)) (self.output_dir / "bad.png").write_bytes(b"\x89PNG not really") - status, body = await self._start(mode="missing", size=200, quality="medium") + self._use("webp", 80, 200) + status, body = await self._start(mode="missing", size=999, quality="high") self.assertEqual(status, 202) summary = body["job"] @@ -244,7 +238,8 @@ class TestThumbnailJob(ThumbnailJobCase): self.assertTrue(summary["cancellable"]) self.assertEqual(set(summary["counters"]), {"generated", "skipped", "errors"}) self.assertEqual( - summary["scope"], {"mode": "missing", "size": 200, "quality": "medium"} + summary["scope"], + {"mode": "missing", "format": "webp", "size": 200, "quality": 80}, ) job = await self._finished(summary["id"]) self.assertEqual(job.state, "finished") @@ -279,7 +274,8 @@ class TestThumbnailJob(ThumbnailJobCase): async def test_rebuild_never_touches_originals(self): for name in ("a", "b"): make_png(self.output_dir / f"{name}.png", size=(400, 400)) - _, body = await self._start(size=128) + self._use("webp", 80, 128) + _, body = await self._start() await self._finished(body["job"]["id"]) originals = { name: hashlib.sha256( @@ -288,7 +284,8 @@ class TestThumbnailJob(ThumbnailJobCase): for name in ("a", "b") } - _, body = await self._start(mode="rebuild", size=256, quality="high") + self._use("webp", 95, 256) + _, body = await self._start(mode="rebuild") job = await self._finished(body["job"]["id"]) self.assertEqual(job.counters, {"generated": 2, "skipped": 0, "errors": 0}) @@ -315,12 +312,12 @@ class TestThumbnailJob(ThumbnailJobCase): self.assertEqual(job.counters["errors"], 1) self.assertEqual(self._thumb_path("a").read_bytes(), preview) leftovers = [p.name for p in self._thumb_path("a").parent.iterdir()] - self.assertEqual(leftovers, ["a_thumb.png"]) # no temp file left + self.assertEqual(leftovers, ["a_thumb.webp"]) # no temp file left async def test_rebuild_keeps_an_old_video_preview_instead_of_a_placeholder(self): clip = self.output_dir / "clip.mp4" clip.write_bytes(b"\x00") - preview = make_png(self.output_dir / "thumbnails" / "clip_thumb.jpg") + preview = make_png(self.output_dir / "thumbnails" / "clip_thumb.webp") old = preview.read_bytes() with ( patch.dict(sys.modules, {"cv2": None}), @@ -339,7 +336,8 @@ class TestThumbnailJob(ThumbnailJobCase): img.save(self.output_dir / "photo.jpg", "JPEG", quality=95) sizes = {} for quality in ("low", "high"): - _, body = await self._start(mode="rebuild", size=300, quality=quality) + self._use("jpeg", 40 if quality == "low" else 95, 300) + _, body = await self._start(mode="rebuild") await self._finished(body["job"]["id"]) thumb = self._thumb_path("photo", ".jpg") with Image.open(thumb) as image: @@ -380,7 +378,7 @@ class TestThumbnailJob(ThumbnailJobCase): self.assertEqual(len(written), 2) for dst in written: self.assertTrue(dst.is_file()) - made = sorted((self.output_dir / "thumbnails").glob("*_thumb.png")) + made = sorted((self.output_dir / "thumbnails").glob("*_thumb.webp")) self.assertEqual(made, sorted(written)) self.assertTrue(any("Thumbnails cancelled" in line for line in job.log)) @@ -511,16 +509,16 @@ class TestThumbnailJob(ThumbnailJobCase): entered, release = threading.Event(), threading.Event() self.addCleanup(release.set) checked = [] - real_fresh = images_module._thumbnail_is_fresh + real_state = images_module._preview_state - def gated_fresh(src, dst): + def gated_state(src, *args): checked.append(src.name) if len(checked) == 1: entered.set() release.wait(10) - return real_fresh(src, dst) + return real_state(src, *args) - with patch.object(images_module, "_thumbnail_is_fresh", gated_fresh): + with patch.object(images_module, "_preview_state", gated_state): stream = asyncio.ensure_future(self.client.post(self.PROGRESS_URL)) await self._wait(entered) job = self.api._jobs.active("thumbnails") @@ -567,7 +565,7 @@ class TestThumbnailJob(ThumbnailJobCase): with ( patch.dict(sys.modules, {"cv2": None}), patch("subprocess.Popen", side_effect=FileNotFoundError("no ffmpeg")), - patch.object(images_module, "_atomic_save", refuse), + patch.object(images_module, "write_thumbnail", refuse), ): _, body = await self._start() job = await self._finished(body["job"]["id"]) @@ -586,52 +584,64 @@ class TestThumbnailJob(ThumbnailJobCase): [row] = job.outcomes["errors"] self.assertEqual(row["detail"], images_module.PREVIEW_NOT_WRITABLE) - async def test_ffmpeg_fallback_keeps_quality_and_aspect_ratio(self): - """M1: without OpenCV, ffmpeg keeps the chosen quality and the aspect ratio.""" + async def test_ffmpeg_fallback_keeps_the_aspect_ratio_and_the_settings(self): + """M1: without OpenCV, ffmpeg's frame keeps its aspect ratio; #317: it is + read as a PNG on stdout and encoded with the server's settings.""" + import io + (self.output_dir / "clip.mp4").write_bytes(b"\x00") commands = [] + frame = io.BytesIO() + Image.new("RGB", (640, 360), (0, 99, 0)).save(frame, "PNG") def fake_ffmpeg(cmd, **_kwargs): commands.append(cmd) - Path(cmd[-1]).write_bytes(b"jpeg") done = type("Done", (), {"returncode": 0})() - done.communicate = lambda timeout=None: ("", "") + done.communicate = lambda input=None, timeout=None: (frame.getvalue(), b"") + done.poll = lambda: 0 + done.kill = lambda: None return done - for quality, qscale in (("high", "2"), ("medium", "4"), ("low", "8")): - with ( - patch.dict(sys.modules, {"cv2": None}), - patch("subprocess.Popen", side_effect=fake_ffmpeg), - ): - _, body = await self._start(mode="rebuild", size=640, quality=quality) - job = await self._finished(body["job"]["id"]) - self.assertEqual(job.counters["generated"], 1, quality) - cmd = commands[-1] - self.assertNotIn("-s", cmd) - self.assertEqual( - cmd[cmd.index("-vf") + 1], - "scale=640:640:force_original_aspect_ratio=decrease", - ) - self.assertEqual(cmd[cmd.index("-q:v") + 1], qscale, quality) + self._use("webp", 70, 320) + with ( + patch.dict(sys.modules, {"cv2": None}), + patch("subprocess.Popen", side_effect=fake_ffmpeg), + ): + _, body = await self._start(mode="rebuild") + job = await self._finished(body["job"]["id"]) + self.assertEqual(job.counters["generated"], 1) + cmd = commands[-1] + self.assertNotIn("-q:v", cmd) + self.assertEqual( + cmd[cmd.index("-vf") + 1], + "scale='min(iw,320)':'min(ih,320)':force_original_aspect_ratio=decrease", + ) + self.assertEqual(cmd[-5:], ["-f", "image2pipe", "-vcodec", "png", "-"]) + with Image.open(self._thumb_path("clip")) as thumb: + self.assertEqual((thumb.format, thumb.size), ("WEBP", (320, 180))) - async def test_the_placeholder_uses_the_chosen_quality(self): + async def test_the_placeholder_uses_the_settings(self): (self.output_dir / "clip.mp4").write_bytes(b"\x00") saved = [] - real_save = images_module._atomic_save + real_write = images_module.write_thumbnail - def recording(img, dst, fmt=None, **kwargs): - # The encoder settings (guard is the automatic previews' hold check). - saved.append({k: v for k, v in kwargs.items() if k != "guard"}) - return real_save(img, dst, fmt, **kwargs) + def recording(frame, dst, settings, **kwargs): + saved.append(settings) + return real_write(frame, dst, settings, **kwargs) + self._use("jpeg", 33, 200) with ( patch.dict(sys.modules, {"cv2": None}), patch("subprocess.Popen", side_effect=FileNotFoundError("no ffmpeg")), - patch.object(images_module, "_atomic_save", recording), + patch.object(images_module, "write_thumbnail", recording), ): - _, body = await self._start(quality="low") + _, body = await self._start() await self._finished(body["job"]["id"]) - self.assertEqual(saved, [images_module.THUMBNAIL_ENCODINGS["low"]]) + self.assertEqual( + [(x.format, x.quality, x.edge) for x in saved], [("jpeg", 33, 200)] + ) + with Image.open(self._thumb_path("clip", ".jpg")) as thumb: + self.assertEqual(thumb.size, (200, 200)) # The legacy stream's frames and their keys, as the gallery page reads them. @@ -680,6 +690,7 @@ class TestLegacyStreamOnTheJob(ThumbnailJobCase): make_png(self.output_dir / "a.png", size=(400, 400)) make_png(self.output_dir / "b.png") + self._use("webp", 80, 200) resp = await self.client.post(f"{self.PROGRESS_URL}?quality=low") events = parse_sse(await resp.text()) @@ -696,7 +707,8 @@ class TestLegacyStreamOnTheJob(ThumbnailJobCase): {"phase": "scanning", "message": "Scanning for images and videos..."}, ) with Image.open(self._thumb_path("a")) as thumb: - self.assertEqual(max(thumb.size), 150) # the legacy preset still sizes + # #317 R3: the server's size limit, not the legacy preset (low = 150). + self.assertEqual(max(thumb.size), 200) [job] = self.api._jobs.list() self.assertEqual( (job.kind, job.title, job.state, job.cancellable), @@ -766,7 +778,8 @@ class TestLegacyStreamOnTheJob(ThumbnailJobCase): async def test_legacy_stream_attaches_to_a_running_job(self): make_png(self.output_dir / "a.png") entered, release = self._gate_generation() - _, started = await self._start(mode="missing", size=256) + self._use("webp", 80, 256) + _, started = await self._start(mode="missing") await self._wait(entered) job = self.api._jobs.get(started["job"]["id"]) diff --git a/web/js/workspace/inspection-state.mjs b/web/js/workspace/inspection-state.mjs index f8265447..7aa4e61e 100644 --- a/web/js/workspace/inspection-state.mjs +++ b/web/js/workspace/inspection-state.mjs @@ -71,20 +71,28 @@ function downloadFor(image, required) { } } -// The server's video extensions (py/api/inspection_service.py VIDEO_EXTENSIONS): their preview is a JPEG. +// The server's video extensions (py/api/inspection_service.py VIDEO_EXTENSIONS). const VIDEO_EXTENSIONS = new Set(['.mp4', '.webm', '.avi', '.mov', '.mkv', '.m4v', '.wmv']); +// #317: the formats a preview is written in (WebP, JPEG XL, JPEG), for images and videos alike. +const WRITTEN_PREVIEW_EXTENSIONS = ['.webp', '.jxl', '.jpg']; -/** `thumbnails/_thumb` for a listed file, as the server names its preview (images.media_thumbnail_rel). */ -function previewPath(path) { +/** + * Every `thumbnails/_thumb` the server may list as this file's preview (py/api/thumbnail_settings.py + * `owns`): a written format, or the name an older release gave it (the source's own extension, `.jpg` for a video). + */ +function previewPaths(path) { const slash = path.lastIndexOf('/'), dot = path.lastIndexOf('.'); - const cut = dot > slash ? dot : path.length, extension = path.slice(cut).toLowerCase(); - return `thumbnails/${path.slice(0, cut)}_thumb${VIDEO_EXTENSIONS.has(extension) ? '.jpg' : extension}`; + const cut = dot > slash ? dot : path.length, spelled = path.slice(cut), extension = spelled.toLowerCase(); + const stem = `thumbnails/${path.slice(0, cut)}_thumb`; + const legacy = VIDEO_EXTENSIONS.has(extension) ? ['.jpg'] : [extension, spelled]; + return new Set([...WRITTEN_PREVIEW_EXTENSIONS, ...legacy].filter(Boolean).map(ext => stem + ext)); } /** * #249: the listing's thumbnail of `image` as `{url}` for the inspector's cover, or null. Only the same-origin - * serve URL of this file's own preview under the same root and source scope is used; anything else keeps the - * original (a missing preview is made by the server on first view). + * serve URL of this file's own preview (any format it owns, #317) under the same root and source scope is used; + * anything else keeps the original (a missing preview is made by the server on first view). The cover is only a + * placeholder: metadata, downloads and the viewer always use the original. */ function previewFor(image) { try { @@ -93,9 +101,10 @@ function previewFor(image) { const reference = new URL(location?.href || 'http://localhost/'); const resolveUrl = globalThis.window?.ApiClient?.resolveUrl || globalThis.ApiClient?.resolveUrl || (url => url); const value = resolveUrl(image.thumbnail_url), url = new URL(value, reference); - const expected = new URL(resolveUrl(`/prompt_manager/images/serve/${previewPath(image.path).split('/').map(quoteSegment).join('/')}`), reference); + const expected = new Set([...previewPaths(image.path)].map(preview => + new URL(resolveUrl(`/prompt_manager/images/serve/${preview.split('/').map(quoteSegment).join('/')}`), reference).pathname)); const scope = descriptorScope(image); - if (url.origin !== reference.origin || url.username || url.password || url.hash || url.pathname !== expected.pathname || + if (url.origin !== reference.origin || url.username || url.password || url.hash || !expected.has(url.pathname) || url.searchParams.getAll('root').length !== 1 || url.searchParams.get('root') !== String(image.root_index) || url.searchParams.getAll('root_scope').length > 1 || (scope !== null && url.searchParams.get('root_scope') !== scope)) return null; return { url: value };