Conversation
lusoris
force-pushed
the
feat/vmaf-tune-encode-multi-codec
branch
from
e451408 to
01eb63b
Compare
…s 17 adapters) Refactors `tools/vmaf-tune/src/vmaftune/encode.py` away from the Phase A hard-coded `libx264` `-c:v / -preset / -crf` argv. `run_encode` now looks up the codec adapter via `codec_adapters.get_adapter(req.encoder)` and asks it for the FFmpeg argv slice via `adapter.ffmpeg_codec_args(preset, quality)` plus an optional `adapter.extra_params()`. Adapters that don't yet expose `ffmpeg_codec_args` fall back silently to the legacy x264-CRF shape so partial in-flight adapter PRs stay drivable end-to-end. `parse_versions(stderr, encoder=...)` selects a per-codec version probe (libx264, libx265, libsvtav1, libvpx-vp9, libaom-av1, libvvenc, NVENC, QSV, AMF, VideoToolbox); unknown encoders return "unknown" rather than raising. The `EncodeRequest.crf` field is preserved unchanged for the SCHEMA_VERSION=1 row contract; a `quality` property mirrors it for adapter-side codec-agnostic vocabulary. Existing 13-test x264 suite still green; new 19-test multi-codec suite covers 9 representative codec shapes plus the unknown-codec / missing-method fallback paths. Unblocks 17 in-flight codec adapter PRs (#360 libaom, #362 libx265, #364 NVENC, #366 AMF, #367 QSV, #368 libvvenc, #370 libsvtav1, #373 VideoToolbox, plus follow-on waves) which can now drive end-to-end encodes without copying or mutating the harness. Ships ADR-0294 + research digest 0054, vmaf-tune.md "Codec adapter contract" section, rebase-notes #228 invariant, CHANGELOG entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lusoris
force-pushed
the
feat/vmaf-tune-encode-multi-codec
branch
from
01eb63b to
607c242
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors vmaf-tune’s encode.py into a codec-agnostic FFmpeg command dispatcher driven by codec adapters, expanding version parsing to be per-encoder and adding a multi-codec test suite to lock in the new contract.
Changes:
- Turn
encode.pyinto a dispatcher that delegates codec argv construction toadapter.ffmpeg_codec_args(...)with a legacy x264-shaped fallback. - Extend
parse_versions(stderr, encoder=...)with a probe table covering multiple encoder families. - Add multi-codec dispatcher tests plus documentation/ADR/research/changelog updates for the new contract.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/vmaf-tune/src/vmaftune/encode.py | Implements adapter-driven argv composition, legacy fallback, and per-encoder version probing. |
| tools/vmaf-tune/src/vmaftune/codec_adapters/init.py | Updates the adapter Protocol to include dispatcher-consumed methods. |
| tools/vmaf-tune/src/vmaftune/codec_adapters/x264.py | Implements ffmpeg_codec_args/extra_params for x264 to preserve bit-identical argv. |
| tools/vmaf-tune/tests/test_encode_multi_codec.py | Adds a multi-codec test suite validating dispatcher behavior and fallbacks. |
| docs/usage/vmaf-tune.md | Documents the codec adapter contract and dispatcher command shape. |
| docs/research/0070-vmaf-tune-encode-multi-codec.md | Adds a research digest for the dispatcher design. |
| docs/rebase-notes.md | Adds rebase note entry pinning the “no codec branching in harness” invariant. |
| docs/adr/README.md | Registers the new ADR entry in the ADR index. |
| docs/adr/0297-vmaf-tune-encode-multi-codec.md | Adds the ADR describing the dispatcher decision and consequences. |
| CHANGELOG.md | Adds an Unreleased changelog entry for the dispatcher + tests + docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+6
| # ADR-0294: `vmaf-tune` — codec-agnostic encode dispatcher | ||
|
|
||
| - **Status**: Accepted | ||
| - **Date**: 2026-05-03 | ||
| - **Deciders**: Lusoris | ||
| - **Tags**: tooling, ffmpeg, codec, automation, fork-local |
| @@ -0,0 +1,127 @@ | |||
| # Research-0054: `vmaf-tune` codec-agnostic encode dispatcher | |||
Comment on lines
46
to
+73
| @@ -56,6 +60,18 @@ class CodecAdapter(Protocol): | |||
| quality_default: int | |||
| invert_quality: bool | |||
|
|
|||
| def ffmpeg_codec_args(self, preset: str, quality: int) -> list[str]: | |||
| """FFmpeg ``-c:v ...`` argv slice for one encode.""" | |||
| ... | |||
|
|
|||
| def extra_params(self) -> tuple[str, ...]: | |||
| """Additional non-codec argv (e.g. tile flags). May be empty.""" | |||
| ... | |||
|
|
|||
| def validate(self, preset: str, quality: int) -> None: | |||
| """Raise ``ValueError`` if ``(preset, quality)`` is unsupported.""" | |||
| ... | |||
Comment on lines
+162
to
+164
| _SVTAV1_VERSION_RE = re.compile(r"SVT-AV1.*?\sv?(\d+\.\d+(?:\.\d+)?)") | ||
| _LIBVPX_VERSION_RE = re.compile(r"libvpx-vp9.*?(\d+\.\d+\.\d+)|vpxenc\s+v(\d+\.\d+\.\d+)") | ||
| _LIBAOM_VERSION_RE = re.compile(r"libaom.*?(\d+\.\d+\.\d+)|aomenc.*?(\d+\.\d+\.\d+)") |
Comment on lines
+26
to
+30
| _HERE = Path(__file__).resolve().parent | ||
| sys.path.insert(0, str(_HERE.parent / "src")) | ||
|
|
||
| from vmaftune import codec_adapters as ca # noqa: E402 | ||
| from vmaftune.encode import ( # noqa: E402 |
| ### 0228 — `y4m_convert_411_422jpeg` 1-byte heap-buffer-overflow fix | ||
| ### 0228 — `vmaf-tune` resolution-aware model selection (ADR-0289) | ||
| ### 0282 — `vmaf-tune` AMD AMF codec adapters (ADR-0282) | ||
| ### 0228 — `tools/vmaf-tune/` codec-agnostic encode dispatcher (ADR-0294) |
This was referenced May 6, 2026
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
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.
Summary
tools/vmaf-tune/src/vmaftune/encode.pyinto a thincodec-agnostic dispatcher that delegates FFmpeg argv composition to
the registered codec adapter (
adapter.ffmpeg_codec_args(preset, quality)+ optionaladapter.extra_params()). Adapters withoutffmpeg_codec_argsfall back silently to the legacy x264-CRF shapeso partial in-flight adapters stay drivable.
parse_versions(stderr, encoder=...)gains a per-codec versionprobe table (libx264, libx265, libsvtav1, libvpx-vp9, libaom-av1,
libvvenc, NVENC, QSV, AMF, VideoToolbox).
EncodeRequest.crfstaysfor SCHEMA_VERSION=1 row compatibility;
EncodeRequest.qualityis arequest-side property mirror.
(libx265), feat(tools): vmaf-tune — NVENC adapters (h264/hevc/av1) #364 (NVENC), feat(tools): vmaf-tune — AMF adapters (h264/hevc/av1) #366 (AMF), feat(tools): vmaf-tune — QSV adapters (h264/hevc/av1) #367 (QSV), feat(tools): vmaf-tune — VVenC + NNVC adapter (AI-augmented H.266) #368 (libvvenc),
feat(tools): vmaf-tune — SVT-AV1 codec adapter #370 (libsvtav1), feat(tools+ai): VideoToolbox adapters + 16-slot codec schema expansion #373 (VideoToolbox), plus follow-on waves — which
can now drive end-to-end encodes without copying or mutating
encode.py.Closes the dispatcher prerequisite called out in every adapter PR
description. Existing 13-test x264 suite stays bit-identical; new
19-test multi-codec suite (
test_encode_multi_codec.py) pins thedispatcher contract per codec.
ADR-0108 deliverables
docs/research/0054-vmaf-tune-encode-multi-codec.md.codec-agnostic-harness invariant (ADR-0237 follow-up).
pytest tools/vmaf-tune/tests/ -q(32 passed).docs/rebase-notes.mdentry 0228.
Test plan
pytest tools/vmaf-tune/tests/ -q— 32 passed (13 existingx264 + 19 new multi-codec).
ruff check tools/vmaf-tune/— clean.black --check tools/vmaf-tune/— clean.isort --check tools/vmaf-tune/— clean.pre-commit run --files <changed>— green (markdown / black /isort / ruff / semgrep / heredoc / conventional-commit).
(libx264, medium, crf=23)matches Phase A verbatim.-c:v <enc> -preset <p> -crf <q>instead of raising.ADR-0237's intent ("harness never branches on codec identity").
Hard-rules compliance
bit-identically; existing 13-test suite still green.
ffmpeg_codec_args— fallback path explicitly retained andpinned by
test_dispatcher_legacy_adapter_without_ffmpeg_codec_argstest_dispatcher_unknown_codec_falls_back_to_x264_shape.🤖 Generated with Claude Code