Fix VideoToolbox ignoring the Quality (CRF) setting on macOS - #19
Open
adamisbk wants to merge 1 commit into
Open
Fix VideoToolbox ignoring the Quality (CRF) setting on macOS#19adamisbk wants to merge 1 commit into
adamisbk wants to merge 1 commit into
Conversation
mux_audio() sent -qp to every encoder that wasn't libx264 or h264_nvenc. ffmpeg accepts -qp for h264_videotoolbox but does not apply it, so on macOS the Quality slider had no effect at all — every export came out at VideoToolbox's own default quality. Verified: -qp 12 and -qp 32 produce byte-identical output (121791 bytes on a fixed source). VideoToolbox takes -q:v instead, on a 1-100 scale where higher is better — the inverse of CRF. Mapped linearly over H.264's 0-51 quantiser range and clamped to 1-100. Measured against libx264 on the same source (1280x720, 4s): crf 12 -> -q:v 76 -> 6,324,274 B (libx264: 6,327,650 B, 0.05% diff) crf 18 -> -q:v 65 -> 3,878,249 B (libx264: 3,829,364 B, 1.3% diff) crf 26 -> -q:v 49 -> 1,896,310 B crf 32 -> -q:v 37 -> 974,704 B Close to libx264 over the upper half of the UI's 12-32 slider; below ~crf 22 the mapping errs toward larger files, i.e. more quality than requested rather than less. Only VideoToolbox is special-cased. libx265, h264_amf, h264_qsv and hevc_nvenc all genuinely honour -qp (confirmed: libx265 -qp 12 -> 9,112,212 B vs -qp 32 -> 1,230,731 B), so their behaviour is unchanged. Extracted as quality_args() so the mapping is unit-testable on any platform, without VideoToolbox hardware or ffmpeg present. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
The bug
mux_audio()picks the constant-quality flag per encoder:ffmpeg accepts
-qpforh264_videotoolboxand then ignores it. It's not an error, so nothing surfaces — but the Quality (CRF) slider in Settings has no effect whatsoever on macOS. Every export comes out at VideoToolbox's own default quality.The fix
VideoToolbox uses
-q:von a 1–100 scale where higher is better — the inverse of CRF. Mapped linearly across H.264's 0–51 quantiser range and clamped to 1–100.I picked the mapping by measuring rather than guessing. Same source (1280x720, 4s),
-q:vswept against libx264 at each CRF:-q:v 76-q:v 65-q:v 49-q:v 37It tracks libx264 very closely over the upper half of the UI's 12–32 slider (including the default of 18). Below roughly CRF 22 the curves diverge and this errs toward larger files — more quality than asked for rather than less, which seemed like the right direction to fail in.
I deliberately did not fit a curve to the measured points. That's one clip on one Apple Silicon machine, and VideoToolbox's behaviour varies across silicon generations and macOS versions; a fitted curve would be false precision. The linear map is one line, monotonic, and exact where it matters most. Happy to revisit if you'd rather have the low end tuned.
Scope
Only VideoToolbox is special-cased. I checked that the other encoders falling through to
-qpgenuinely honour it, so none of their behaviour changes:The logic is extracted into
quality_args()so it's unit-testable on any platform — the 8 new tests need neither VideoToolbox hardware nor ffmpeg, so they run on the Windows CI runner too.Testing
pytest tests/ -q→ 578 passed, 26 skipped on macOS (was 570 passed; +8 new)quality_args()output confirms the slider now moves: 6.3 MB → 3.9 MB → 1.9 MB → 974 KB across CRF 12/18/26/32, where previously all four were byte-identicalNotes
Environment: macOS on Apple Silicon, Python 3.13, ffmpeg 8.1.2,
pip install -e ".[dev]".While verifying this I also confirmed the rest of the macOS story is in good shape — the full suite passes and
python main.pylaunches and loads cleanly. Two other things I noticed but left out to keep this PR focused, happy to open issues or follow-up PRs if useful:OpenLap.speccan't be parsed on macOS at all — it raisesKeyError: 'LOCALAPPDATA'in_find_chromium_build()before PyInstaller starts, so there's no path to a macOS build today.frontend/tests/data.test.jsfail fromlocalStoragebeing undefined under vitest's jsdom (data.js:1123uses it as a bare global). That one is platform-independent — it should fail on Windows too.