Skip to content

fix(tune): omit -crf in libx264 two-pass encoding and update state.md - #1265

Merged
lusoris merged 1 commit into
masterfrom
fix/vmaftune-state-bugs
Sep 4, 2026
Merged

lusoris merged 1 commit into
masterfrom
fix/vmaftune-state-bugs

Conversation

@lusoris

@lusoris lusoris commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes open bug T-VMAFTUNE-TWOPASS-CRF-INVALID-2026-08-30 from docs/state.md where libx264 multi-pass encoding emitted conflicting rate control flags (-crf and -pass), causing FFmpeg to reject pass 2 (CRF/CQP is incompatible with 2pass.). In multi-pass mode (pass_number != 0), -crf is omitted across both Go (pkg/codecadapter, pkg/ffencode, pkg/corpus) and Python (vmaftune.codec_adapters.x264, vmaftune.encode). Also records verification of T-SPEED-GPU-REGISTRY-ORPHAN-2026-06-19, which was already resolved on master via PR #1004 (commit a0bf83c214).

Type

  • fix — bug fix

Checklist

  • Commits follow Conventional Commits (the commit-msg hook enforces this).
  • make format && make lint is green locally.
  • Unit tests pass: meson test -C build.
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is <= 2.
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below.
  • If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header (see CONTRIBUTING.md).
  • If this is a breaking change, the commit message uses ! or BREAKING CHANGE: and the migration path is documented below.
  • If this PR adds an ADR, the ADR row lives in docs/adr/_index_fragments/<NNNN-slug>.md and the slug is appended to docs/adr/_index_fragments/_order.txt — do not edit docs/adr/README.md directly (regenerated by scripts/docs/concat-adr-index.sh; see ADR-0221).

Bug-status hygiene (ADR-0165)

  • docs/state.md updated in this PR with rows in Recently closed for T-VMAFTUNE-TWOPASS-CRF-INVALID-2026-08-30 and T-SPEED-GPU-REGISTRY-ORPHAN-2026-06-19.

Netflix golden-data gate (ADR-0024)

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: bug fix for conflicting FFmpeg rate-control flags.
  • Decision matrix — no alternatives: only-one-way fix.
  • AGENTS.md invariant note — no rebase-sensitive invariants.
  • Reproducer / smoke-test command — pasted below under "Reproducer".
  • CHANGELOG fragment — changelog.d/fixed/vmaftune-twopass-crf-conflict.md added.
  • Rebase note — docs/rebase-notes.md updated with entry for fix/vmaftune-state-bugs.

Reproducer

go test -v ./pkg/ffencode/... ./pkg/codecadapter/... ./pkg/corpus/...
pytest tools/vmaf-tune/tests/test_codec_adapter_x265_two_pass.py -v

@lusoris lusoris added this to the 1.0.0 — First release milestone Sep 3, 2026
@lusoris
lusoris force-pushed the fix/vmaftune-state-bugs branch from 270511e to 87c7aa2 Compare September 4, 2026 18:50
@lusoris
lusoris marked this pull request as ready for review September 4, 2026 19:01
@lusoris
lusoris force-pushed the fix/vmaftune-state-bugs branch from 87c7aa2 to f689203 Compare September 4, 2026 19:13
In two-pass encoding, FFmpeg libx264 rate control rejects -crf with
"CRF/CQP is incompatible with 2pass" (exit status 187). Omit -crf in
multi-pass mode across Go and Python codec adapters so two-pass
invocations emit valid FFmpeg commands, and update state.md closing
T-VMAFTUNE-TWOPASS-CRF-INVALID-2026-08-30 and recording verification of
T-SPEED-GPU-REGISTRY-ORPHAN-2026-06-19.
@lusoris
lusoris force-pushed the fix/vmaftune-state-bugs branch from f689203 to 9f8e129 Compare September 4, 2026 19:59
@lusoris
lusoris merged commit 0a8727c into master Sep 4, 2026
72 checks passed
@lusoris
lusoris deleted the fix/vmaftune-state-bugs branch September 4, 2026 20:30
@lusoris lusoris added the type:bug Something isn't working label Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant