Warn once, not per video, when ffmpeg is missing - #412
Merged
Merged
Conversation
ffmpeg_exe() deliberately re-probes after a failure, so with no ffmpeg at all every video in an index run logged the same warning. That is now the default in the Docker images (#410). Warn only on the transition into missing, log repeats at debug, and re-arm once a binary is found. The check-and-flip runs under the existing probe lock, so concurrent indexing workers still produce a single warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
extract_video_frame fell through to its generic per-video warning when ffmpeg was unavailable, so the spam survived the probe dedup. Also drop the re-arm claim: a found binary is memoized for the process lifetime. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When ffmpeg is missing, an index run logged the same warning for every video. With the Docker images shipping without ffmpeg (#410), that is now the default for every Docker user with videos. There were two sources:
ffmpeg_exe()deliberately looks for ffmpeg again after every failure, so a temporary glitch doesn't switch off video for the whole process. It also logged a WARNING on every failed attempt.extract_video_frame()then fell through to its genericCould not extract a frame from …; skipping it.warning, so each video produced a second line.Fix
_report_missing()helper logs a WARNING only the first time ffmpeg is found missing, and later failures at DEBUG. It runs under the existing_ffmpeg_exe_lock, so parallel indexing workers still produce a single warning. Whatffmpeg_exe()returns, and the look-again-every-time behaviour, are unchanged.extract_video_frame()now returnsNonequietly when ffmpeg is unavailable, instead of falling through to the per-video warning.Users still get told why videos were skipped: the end-of-indexing message ("…skipped: PhotoMapAI could not find a working ffmpeg") and its WARNING still fire on every run. As a side effect, video-conversion polls no longer log a warning on each poll; they still return
state="unavailable"with the explanation.Tests
New tests in
tests/backend/test_video_probe.py:Each test fails without its matching fix.
pytest tests/backend -k "video or index or umap or media or embed": 598 passed.ruff checkis clean.An adversarial fresh-context review found the frame-extraction warning, which is fixed in the second commit. It also pointed out that a "re-arm after recovery" test described something that can't happen, since a found binary is remembered for the life of the process. That test and the matching docstring claim were removed.
🤖 Generated with Claude Code