Name ffmpeg as the cause when videos are skipped at index time

photomap/backend/embeddings.py

@@ -50,7 +50,11 @@
 from .thumbnail_cache import keep_hashes_for, thumbnail_dir
 from .thumbnail_cache import prune as prune_thumbnails
 from .util import atomic_savez
-from .video import VIDEO_METADATA_KEY, extract_video_frame
+from .video import (
+    VIDEO_METADATA_KEY,
+    extract_video_frame,
+    ffmpeg_known_unavailable,
+)
 from .video_cache import VideoFrameCache
 from .video_transcode import TranscodeCache
 
@@ -1349,10 +1353,43 @@ def _register_unreadable_files_warning(
             return
         count = len(result.bad_files)
         noun, verb = ("file", "was") if count == 1 else ("files", "were")
-        progress_tracker.add_completion_warning(
-            album_key,
-            f"{count} {noun} could not be read and {verb} skipped.",
-        )
+        message = f"{count} {noun} could not be read and {verb} skipped."
+
+        # On a platform with no ffmpeg binary *every* video fails, and the
+        # generic notice above sends the user hunting for corrupt files that
+        # are in fact fine. Naming the cause is the difference between "my
+        # videos are broken" and "this machine needs ffmpeg".
+        #
+        # ``ffmpeg_known_unavailable`` rather than ``ffmpeg_exe()``: this runs
+        # on the asyncio event loop (both callers are ``async def``), and
+        # ffmpeg_exe can stat a hung network mount or spawn an untimed
+        # ``ffmpeg -version``. It only reports the probe every video failure
+        # already performed on a worker thread.
+        #
+        # Worded as what this process observed, not as a claim about the
+        # machine: imageio memoizes its own negative, so a binary that exists
+        # but could not be executed once (an AV scanner holding a freshly
+        # unpacked ffmpeg.exe) stays unavailable for the process lifetime, and
+        # "there is no ffmpeg here" would be a lie to someone who can see it.
+        videos = sum(1 for path in result.bad_files if is_video(path))
+        if videos and ffmpeg_known_unavailable():
+            video_noun = "video" if videos == 1 else "videos"
+            if videos == count:
+                message = (
+                    f"{count} {video_noun} could not be indexed and {verb} "
+                    "skipped: PhotoMapAI could not find a working ffmpeg."
+                )
+            else:
+                # Plural here is independent of ``verb``: the mixed branch
+                # always has count >= 2, but ``videos`` can still be 1.
+                needs = "needs" if videos == 1 else "need"
+                message = (
+                    f"{count} {noun} could not be read and {verb} skipped, "
+                    f"including {videos} {video_noun} that {needs} ffmpeg, "
+                    "which PhotoMapAI could not find."
+                )
+
+        progress_tracker.add_completion_warning(album_key, message)
         logger.warning(
             f"Skipped {count} unreadable {noun} in album '{album_key}': "
             + ", ".join(p.name for p in result.bad_files[:5])

photomap/backend/video.py

@@ -247,9 +247,28 @@ def _has_decodable_video_stream(report: str) -> bool:
 
 _ffmpeg_exe_cache: str | None = None
 _ffmpeg_exe_probed = False
+_ffmpeg_missing = False
 _ffmpeg_exe_lock = threading.Lock()
 
 
+def ffmpeg_known_unavailable() -> bool:
+    """Did the most recent probe in this process fail to find a binary?
+
+    Reports what already happened instead of asking again. Callers on the
+    asyncio event loop must not trigger :func:`ffmpeg_exe`'s work: it stats
+    whatever ``$IMAGEIO_FFMPEG_EXE`` names, which is accepted unchecked by
+    imageio and may be a hung network mount, and the underlying probe spawns
+    ``ffmpeg -version`` with no timeout. Every video failure has already
+    driven a real probe on a worker thread (``_load_video`` ->
+    ``extract_video_frame`` -> :func:`_run_ffmpeg`), so by the time a run
+    reports its skipped files this is an answer rather than a guess.
+
+    ``False`` until something has probed: absence of evidence has to read as
+    "do not blame ffmpeg", or a caller would invent a cause it never observed.
+    """
+    return _ffmpeg_missing
+
+
 def ffmpeg_exe() -> str | None:
     """A runnable ffmpeg, or ``None`` if this platform has none.
 
@@ -270,7 +289,7 @@ def ffmpeg_exe() -> str | None:
       name ``"ffmpeg"`` for a PATH lookup, so a non-``None`` answer is not on
       its own evidence that anything is executable.
     """
-    global _ffmpeg_exe_cache, _ffmpeg_exe_probed
+    global _ffmpeg_exe_cache, _ffmpeg_exe_probed, _ffmpeg_missing
 
     if _ffmpeg_exe_probed:
         return _ffmpeg_exe_cache
@@ -286,6 +305,7 @@ def ffmpeg_exe() -> str | None:
             logger.warning(
                 f"No usable ffmpeg binary found ({e}); video files will be skipped."
             )
+            _ffmpeg_missing = True
             return None  # not memoized — the next call retries
 
         resolved = candidate if os.path.isabs(candidate) else shutil.which(candidate)
@@ -294,19 +314,22 @@ def ffmpeg_exe() -> str | None:
                 f"ffmpeg reported as {candidate!r} but is not executable; "
                 "video files will be skipped."
             )
+            _ffmpeg_missing = True
             return None
 
         _ffmpeg_exe_cache = resolved
         _ffmpeg_exe_probed = True
+        _ffmpeg_missing = False
         return _ffmpeg_exe_cache
 
 
 def _reset_ffmpeg_exe_cache() -> None:
     """Test seam: forget the probed binary so the next call re-resolves."""
-    global _ffmpeg_exe_cache, _ffmpeg_exe_probed
+    global _ffmpeg_exe_cache, _ffmpeg_exe_probed, _ffmpeg_missing
     with _ffmpeg_exe_lock:
         _ffmpeg_exe_cache = None
         _ffmpeg_exe_probed = False
+        _ffmpeg_missing = False
 
 
 def _parse_ffmpeg_banner(stderr: str) -> dict[str, object]:

photomap/frontend/static/css/album-manager.css

@@ -256,6 +256,12 @@
 .index-status.completed {
   color: green !important;
 }
+/* Completed, but something was skipped. Needs both the !important and the
+   extra class: .index-status.completed is also !important, so only higher
+   specificity can beat it — an inline style cannot. */
+.index-status.completed.with-warning {
+  color: #ff9800 !important;
+}
 .index-status.error {
   color: #b00020 !important;
 }

photomap/frontend/static/javascript/album-manager.js

@@ -1952,14 +1952,21 @@ export class AlbumManager {
 
   updateProgressStatus(status, progress, estimatedTime) {
     if (progress.status === "completed") {
-      status.className = AlbumManager.STATUS_CLASSES.COMPLETED;
-      if (progress.warning_message) {
-        status.textContent = `Indexing completed — ${progress.warning_message}`;
-        status.style.color = "#ff9800"; // Orange: completed, but with a caveat
-      } else {
-        status.textContent = "Indexing completed successfully";
-        status.style.color = "green";
-      }
+      // The colour has to come from the stylesheet, not from style.color:
+      // `.index-status.completed` carries an author !important, which outranks
+      // a *normal* inline declaration, so the orange set here was silently
+      // discarded and every warning rendered in success-green — the one state
+      // where the colour is carrying the message.
+      const hasWarning = Boolean(progress.warning_message);
+      status.className = hasWarning
+        ? `${AlbumManager.STATUS_CLASSES.COMPLETED} with-warning`
+        : AlbumManager.STATUS_CLASSES.COMPLETED;
+      status.textContent = hasWarning
+        ? `Indexing completed — ${progress.warning_message}`
+        : "Indexing completed successfully";
+      // Clear any inline colour a previous poll tick left behind (the branches
+      // below still set one), so the rule that applies is this state's.
+      status.style.color = "";
       estimatedTime.textContent = "";
     } else if (progress.status === "error") {
       status.className = AlbumManager.STATUS_CLASSES.ERROR;

tests/backend/test_video_guards.py

@@ -20,8 +20,8 @@
 from fixtures import media_fixture_path
 from PIL import Image
 
-from photomap.backend.embeddings import Embeddings, _open_npz_file
-from photomap.backend.progress import ProgressTracker
+from photomap.backend.embeddings import Embeddings, IndexResult, _open_npz_file
+from photomap.backend.progress import ProgressTracker, progress_tracker
 from photomap.backend.util import atomic_savez
 
 ENCODER_SPEC = "openai-clip:ViT-B/32"
@@ -440,6 +440,130 @@ def test_adding_an_empty_warning_is_a_noop():
     assert tracker.get_progress("album").warning_message == "kept"
 
 
+# --------------------------------------------------------------------------
+# Naming ffmpeg as the cause of a skip
+# --------------------------------------------------------------------------
+
+
+def _skip_result(*names: str) -> IndexResult:
+    """An IndexResult whose only content is a list of files that failed."""
+    return IndexResult(
+        embeddings=np.zeros((0, EMBEDDING_DIM), dtype=np.float32),
+        filenames=np.array([], dtype=object),
+        modification_times=np.array([], dtype=float),
+        metadata=np.array([], dtype=object),
+        bad_files=[Path(name) for name in names],
+    )
+
+
+def _registered_warning(monkeypatch, result: IndexResult, missing: bool) -> str:
+    """Run the notice builder with ffmpeg observed present/absent."""
+    monkeypatch.setattr(
+        "photomap.backend.embeddings.ffmpeg_known_unavailable", lambda: missing
+    )
+    captured: list[str] = []
+    monkeypatch.setattr(
+        progress_tracker,
+        "add_completion_warning",
+        lambda album_key, message: captured.append(message),
+    )
+    Embeddings._register_unreadable_files_warning("warning-test", result)
+    assert len(captured) == 1
+    return captured[0]
+
+
+def test_missing_ffmpeg_is_named_when_only_videos_failed(monkeypatch):
+    """Otherwise a platform with no ffmpeg wheel (win_arm64, musl, armv7)
+    reports every video as unreadable and sends the user hunting for corrupt
+    files that are perfectly fine."""
+    warning = _registered_warning(
+        monkeypatch, _skip_result("a.mp4", "b.mov", "c.mkv"), missing=True
+    )
+
+    assert warning == (
+        "3 videos could not be indexed and were skipped: PhotoMapAI could "
+        "not find a working ffmpeg."
+    )
+
+
+def test_missing_ffmpeg_notice_agrees_for_a_single_video(monkeypatch):
+    warning = _registered_warning(monkeypatch, _skip_result("only.mp4"), missing=True)
+
+    assert warning == (
+        "1 video could not be indexed and was skipped: PhotoMapAI could not "
+        "find a working ffmpeg."
+    )
+
+
+def test_a_mixed_failure_keeps_the_generic_count_and_adds_the_cause(monkeypatch):
+    """The images failed for their own reasons, so the notice must not claim
+    ffmpeg explains all four."""
+    warning = _registered_warning(
+        monkeypatch,
+        _skip_result("a.mp4", "b.mov", "broken.jpg", "truncated.png"),
+        missing=True,
+    )
+
+    assert warning == (
+        "4 files could not be read and were skipped, including 2 videos that "
+        "need ffmpeg, which PhotoMapAI could not find."
+    )
+
+
+def test_a_mixed_failure_with_one_video_still_agrees(monkeypatch):
+    """The mixed branch always has count >= 2, so ``verb`` is always plural
+    there — but ``videos`` can be 1, which needs its own verb. One video plus
+    some unreadable photos is the likeliest mixed shape in the wild, and
+    pinning only the videos == 2 case let "1 video that need ffmpeg" through.
+    """
+    warning = _registered_warning(
+        monkeypatch, _skip_result("a.mp4", "broken.jpg"), missing=True
+    )
+
+    assert warning == (
+        "2 files could not be read and were skipped, including 1 video that "
+        "needs ffmpeg, which PhotoMapAI could not find."
+    )
+
+
+def test_wording_is_unchanged_when_ffmpeg_is_present(monkeypatch):
+    """A truncated clip on a working install is not an ffmpeg problem, and
+    saying so would be actively misleading."""
+    warning = _registered_warning(
+        monkeypatch, _skip_result("broken.mp4"), missing=False
+    )
+
+    assert warning == "1 file could not be read and was skipped."
+
+
+def test_image_only_failures_never_mention_ffmpeg(monkeypatch):
+    """ffmpeg must not be consulted at all when no video failed.
+
+    The observation is process-wide, so a machine with no ffmpeg would
+    otherwise let an all-photos album inherit "could not find ffmpeg" as the
+    explanation for two corrupt JPEGs.
+    """
+
+    def _fail():
+        raise AssertionError("ffmpeg must not be consulted for an image-only skip")
+
+    monkeypatch.setattr(
+        "photomap.backend.embeddings.ffmpeg_known_unavailable", _fail
+    )
+    captured: list[str] = []
+    monkeypatch.setattr(
+        progress_tracker,
+        "add_completion_warning",
+        lambda album_key, message: captured.append(message),
+    )
+
+    Embeddings._register_unreadable_files_warning(
+        "image-only", _skip_result("broken.jpg", "truncated.png")
+    )
+
+    assert captured == ["2 files could not be read and were skipped."]
+
+
 def test_set_completion_warning_still_replaces():
     tracker = ProgressTracker()
     tracker.set_completion_warning("album", "first")

tests/backend/test_video_probe.py

@@ -401,6 +401,82 @@ def test_ffmpeg_exe_points_at_a_real_binary():
     assert Path(ffmpeg_exe()).exists()
 
 
+def test_known_unavailable_tracks_the_last_probe(monkeypatch, tmp_path):
+    """The indexing completion notice reads this instead of calling
+    ``ffmpeg_exe`` — it runs on the asyncio event loop, where a stat of a hung
+    ``$IMAGEIO_FFMPEG_EXE`` mount or an untimed ``ffmpeg -version`` spawn would
+    freeze every request. So it has to follow the probe in both directions,
+    including back to False once a binary is found.
+    """
+    video_module._reset_ffmpeg_exe_cache()
+    try:
+        # Nothing probed yet: no evidence must not read as "ffmpeg is missing".
+        assert video_module.ffmpeg_known_unavailable() is False
+
+        stand_in = tmp_path / "ffmpeg"
+        stand_in.write_text("")
+        calls = []
+
+        def flaky():
+            calls.append(1)
+            if len(calls) == 1:
+                raise OSError("Cannot allocate memory")
+            return str(stand_in)
+
+        import imageio_ffmpeg
+
+        monkeypatch.setattr(imageio_ffmpeg, "get_ffmpeg_exe", flaky)
+
+        assert video_module.ffmpeg_exe() is None
+        assert video_module.ffmpeg_known_unavailable() is True
+
+        assert video_module.ffmpeg_exe() == str(stand_in)
+        assert video_module.ffmpeg_known_unavailable() is False
+    finally:
+        video_module._reset_ffmpeg_exe_cache()
+
+
+def test_known_unavailable_is_set_when_the_candidate_is_not_executable(
+    monkeypatch, tmp_path
+):
+    """The other ``return None`` path: imageio hands back a name that does not
+    resolve to anything runnable."""
+    video_module._reset_ffmpeg_exe_cache()
+    try:
+        import imageio_ffmpeg
+
+        monkeypatch.setattr(
+            imageio_ffmpeg,
+            "get_ffmpeg_exe",
+            lambda: str(tmp_path / "definitely-not-here"),
+        )
+
+        assert video_module.ffmpeg_exe() is None
+        assert video_module.ffmpeg_known_unavailable() is True
+    finally:
+        video_module._reset_ffmpeg_exe_cache()
+
+
+def test_resetting_the_cache_also_clears_the_unavailable_flag(monkeypatch):
+    """Otherwise the flag outlives the probe it describes, and the next test
+    (or a re-probe after the condition clears) inherits a stale verdict."""
+    video_module._reset_ffmpeg_exe_cache()
+    try:
+        import imageio_ffmpeg
+
+        monkeypatch.setattr(
+            imageio_ffmpeg,
+            "get_ffmpeg_exe",
+            lambda: (_ for _ in ()).throw(OSError("boom")),
+        )
+        assert video_module.ffmpeg_exe() is None
+        assert video_module.ffmpeg_known_unavailable() is True
+    finally:
+        video_module._reset_ffmpeg_exe_cache()
+
+    assert video_module.ffmpeg_known_unavailable() is False
+
+
 # --------------------------------------------------------------------------
 # Banner parsing against hostile input
 # --------------------------------------------------------------------------