The test suite needs an ffmpeg new enough for -fps_mode. Nothing says so, and nothing checks.
CONTRIBUTING.md:22 asks only for:
- ffmpeg and ffprobe on
PATH for the test suite
-fps_mode is used at src/hflow/video.py:204 and src/hflow/episode.py:703. It replaced -vsync in a later ffmpeg than the one Ubuntu 22.04 ships (4.4.2), so a contributor on the distro default gets three test failures that look like their branch broke something.
Reported by @stgomoyaa on #490, who had to work out for themselves that the failures were environmental and not theirs. They were right, and it cost them time we can save the next person.
What to do
Say the minimum in CONTRIBUTING. One clause on line 22. Establish the actual minimum first rather than copying the number from this issue: check which ffmpeg release introduced -fps_mode, and confirm the suite passes on it. Say in the PR how you established it.
Then make the failure legible. Pick one:
- A session-scoped autouse fixture, or a
conftest.py check, that skips or fails the ffmpeg-dependent tests with a message naming the required version and the version found. A clear skip beats three obscure failures.
hflow doctor already reports environment problems. If it does not check the ffmpeg version, that is arguably the better home, since it helps users and not just contributors. Check what it does today before adding anything.
Do not add a version check to the encode path itself. It runs per episode and the answer cannot change mid-run.
Definition of done
- CONTRIBUTING names the minimum ffmpeg version, with the reason (
-fps_mode) so the number can be re-derived later.
- Running the suite on an older ffmpeg produces one clear message naming the required and found versions, not three failures inside encode assertions.
- On a supported ffmpeg, nothing changes: no new skips, same pass count.
- If you add a version probe, it runs once, not per episode.
Validation
uv sync --locked --all-extras
uv run ruff check
uv run ruff format --check
uv run ty check
uv run pytest -q
If you can, say which ffmpeg version you tested on. That is the number this issue is short of.
The test suite needs an ffmpeg new enough for
-fps_mode. Nothing says so, and nothing checks.CONTRIBUTING.md:22 asks only for:
-fps_modeis used atsrc/hflow/video.py:204andsrc/hflow/episode.py:703. It replaced-vsyncin a later ffmpeg than the one Ubuntu 22.04 ships (4.4.2), so a contributor on the distro default gets three test failures that look like their branch broke something.Reported by @stgomoyaa on #490, who had to work out for themselves that the failures were environmental and not theirs. They were right, and it cost them time we can save the next person.
What to do
Say the minimum in CONTRIBUTING. One clause on line 22. Establish the actual minimum first rather than copying the number from this issue: check which ffmpeg release introduced
-fps_mode, and confirm the suite passes on it. Say in the PR how you established it.Then make the failure legible. Pick one:
conftest.pycheck, that skips or fails the ffmpeg-dependent tests with a message naming the required version and the version found. A clear skip beats three obscure failures.hflow doctoralready reports environment problems. If it does not check the ffmpeg version, that is arguably the better home, since it helps users and not just contributors. Check what it does today before adding anything.Do not add a version check to the encode path itself. It runs per episode and the answer cannot change mid-run.
Definition of done
-fps_mode) so the number can be re-derived later.Validation
If you can, say which ffmpeg version you tested on. That is the number this issue is short of.