Repository navigation
Add native MSVC build support for Windows - #1477
Conversation
cd41041 to
156b094
Compare
a60ccfd to
a861995
Compare
9b01286 to
a493150
Compare
|
Hi everyone, First of all, apologies for the noise — there were quite a few force‑pushes while I was trying to understand what looked like a regression in the macOS FFmpeg+libvmaf build. I genuinely thought I had introduced the issue myself, and it took a while to figure out what was actually happening. Good news: the PR is now fully green across all platforms, including MSVC, which produces exactly the same VMAF score as the original workflow. A small warning regarding macOS: while investigating, I temporarily removed After dropping all my changes to double‑check, I confirmed that the original workflow also prints this mismatch — it simply goes unnoticed because The difference is very small, but I wanted to highlight it explicitly so reviewers are aware. With everything now stable and the CI behaving consistently, the PR is ready for review. P.S. Ironically, I don’t even use Windows except for gaming — but at least now FFmpeg+libvmaf builds cleanly on MSVC. |
|
Hi @kylophone, just a gentle ping on this PR. I noticed the task list still shows some unfinished items (mainly due to dependencies on other PRs), but the work is actually complete and ready for review. Would really appreciate it if you could take a look whenever you have a moment. Happy to address any feedback! Thanks a lot for maintaining VMAF 🙏 |
I am working my way through the PR backlog. I will be taking a look at this before our next release. |
a493150 to
d6df1bd
Compare
Once this is ready, please let me know. I'm not on Windows, so I cannot test. |
From my POV it is ready unless @lusoris or @JensAc want/find a missing hunk from the other PR. As a reminder, this covers CI with ffmpeg+vmaf+score test on Windows so that's part of the testsuite. Question: What do we do with #1410 #1428 #1475 and #1476? Should I wait you to merge them first or should I reword their commit messages so they are merged along this PR? |
|
I compared #1551 (
#1410 is now on master ( On the FFmpeg score check that StormBytePP already flagged as stale in #1597: the expected 93.663925 predates two motion changes on master. With the |
I've seen it. However your last touch broke mingw compile (as I stated in the PR comment). Could you fix it before I rebase and drop the vendored commits? |
Fixed with 8e7a1ac, thanks. |
- Integrate GerHobbelt/pthread-win32 as bundled implementation - Add -Dbundled_winpthreads option (default: true) - Support detecting external pthreads libraries (pthreadVC3, etc.) Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Signed-off-by: David C. Manuelda <StormByte@gmail.com>
Building libvmaf with CUDA enabled under MSVC failed because several targets did not receive the winpthread / stdatomic compatibility pieces required on that toolchain: - nvcc fatbin custom targets: missing bundled pthread-win32 include path - cuda_static_lib: missing thread_lib and stdatomic_dependency - CUDA unit tests: same missing deps when compiling picture_cuda.c and ring_buffer.c Additionally: - pass -D_USE_MATH_DEFINES to nvcc so M_PI is defined with the MSVC CRT - replace C99 designated initializers in integer_adm.h with positional ones so the header is valid as C++ host code under nvcc+MSVC Signed-off-by: David C. Manuelda <StormByte@gmail.com>
When default_library=static the archive is vmaf.lib so FFmpeg/cl can link without a CI rename. When both, the static target is vmaf-static.lib to avoid clashing with the import library.
__lzcnt emits LZCNT; without BMI1/ABM that encoding is BSR and breaks VIF/ADM scalar paths. Algorithm from Netflix#1551 (0887b39), applied only to the MSVC compat header. Signed-off-by: Jens Schneider <jens.schneider.ac@posteo.de> Signed-off-by: David C. Manuelda <StormByte@gmail.com>
27ef765 to
e991a60
Compare
I've dropped the vendored PR that were merged and rebased from master. CI green, PR is ready to merge :) |
|
Merged, thanks for all your help on this @StormBytePP. |
…AF alone and with NEG, VMAF v1; RTX 5090, Intel iGPU, CPU), and every claim checked against the code, git and the runs Problem The landing page of 14a1322 had wrong or unsupported statements: - Its speed table came from older runs: libvmaf's CUDA code only from host memory (46 fps at 4K), no route from GPU memory, VMAF v1 on another film (3840x1608), and app-level numbers from VideoMetricsLab. - "Moving a 4K frame pair through system memory costs the CPU more than the GPU half of VMAF v1 saves": not so (the hybrid from system memory is 1.7x the CPU's speed at 4K with fewer cores busy). - "It is also faster than the CUDA code it ports": only for VMAF and NEG together, or from system memory. For VMAF alone from GPU memory, libvmaf's CUDA code is faster (465 against 382 fps at 4K). - "integer arithmetic only": the shaders use float for estimates the result does not depend on (ADM's angle pre-check, VIF's division), and double with the optional NATIVE_F64. - The AMD fault was described as a read "at a negative index"; it is the table's entries for values below -1, read at their usual place. - The self-test was called the engine's; it is the Python bindings' probe(). - VMAF v1's 71-case matrix was said to be identical "on all three GPUs"; the Radeon 780M ran 48 frames of film, not the matrix. - "libvmaf runs the two feature sets separately": it runs motion once and VIF and ADM twice (fex_ctx_vector.c merges extractors with equal options). - libvmaf's CUDA code "within about 0.001" of the CPU: that was before Netflix#1644; now only motion differs, by at most 0.000029 on the benchmark video. - Also: "fully on the GPU" (libvmaf predicts on the CPU); the five-frame window and moving average belong to the HFR models only; Netflix#1612's leak and Netflix#1652's hang were missing; the GPU half of VMAF v1 "alone at 655 fps" and "2.9 cores busy instead of 7.7" could not be reproduced here (dropped); the whole-film check ran before the AMD fix to one shader. - Of the ten shaders, only common.slang carried the Netflix and NVIDIA copyright notice the page says the engine keeps. - Source comments repeated "no float or double on the GPU" and pointed to fast/README.md for the pull requests, which the root README lists. Change - fast/tests/bench_readme.py (new): the page's tables. Frames decoded into memory first, then each route scores them in a loop: libvmaf on the CPU at 4-24 threads; libvmaf's CUDA code from pinned host pictures and from device pictures filled on the GPU; Vulkan from system memory and from GPU memory (its buffers imported into CUDA); VMAF v1 with each GPU. Speed and the process's CPU time per route; scores compared within each kind (GPU routes of VMAF v0.6.1 against libvmaf's CUDA code). --vmaf-only for VMAF without NEG. - README.md rewritten from the measurements and the checks: - Speed: one table for VMAF and VMAF + NEG at 4K and 1080p (CPU, CUDA and Vulkan from system and GPU memory, Intel iGPU), one for VMAF v1 with cores busy, and what they show, including where CUDA is faster. - How it was done: what is reproduced exactly and how (per-pixel float and double, tables, rounding of partial sums), where float is still used, the Intel and AMD driver faults as found, the self-tests as the bindings' probe(), VMAF v1's work split measured on the benchmark video (ADM3 + motion3 68% at 4K, 63% at 1080p), the route from GPU memory. - The pull request table with complete fixes; Netflix#1477 merged upstream on 2026-10-02; which seven changed since; Netflix#1562's second cause. - How it is checked: the two matrices' actual cases, real video, the whole film, and exactly what was run on the Radeon 780M. - fast/vulkan/shaders/*.slang: the libvmaf notice (Netflix 2016-2023, NVIDIA 2021, BSD+Patent) on the nine that lacked it; motion_v1 and vif_filter name the CPU files they come from. common.slang's note on arithmetic says where float and double are used. - fast/vulkan/vmaf_vulkan.cpp, common.slang, build_libvmaf_cuda.ps1: comments point to README.md for the pull requests. - fast/python/vmaf_fast: vulkan.py and v1.py docstrings no longer say "integer arithmetic only", "several times faster" or native/vmaf_vulkan; v1.py drops app numbers from VideoMetricsLab. - fast/README.md: bench_readme.py under Testing. Effect No change to any library: vmaf_vulkan.dll rebuilt from this tree is byte for byte the release's (SHA-256 47b95eeb...), so v3.2.0-fast.1 stays current. The landing page states only what was measured or read here. Verification (RTX 5090, Core Ultra 9 285K and its Intel GPU) - bench_readme.py, HoneyBee 3840x2160 10-bit against its x265 encode, 48 frames in memory: 480 pairs at 4K, 960 at 1920x1080, with and without --vmaf-only. Repeat runs within about 1%. libvmaf's CUDA code from pinned host pictures with 0, 2, 4 and 8 threads: 58-65 fps at 4K. - compare_vmaf_vulkan.py --matrix (45) and compare_vmaf_v1.py --matrix (71), each with --device 0 and 1: ALL IDENTICAL. diagnose_vmaf_vulkan.py: every sum and buffer is the reference's, on both. - Real video, 48 frames of HoneyBee at 4K 10-bit and 1080p 8-bit, both GPUs: VMAF, NEG and VMAF v1 IDENTICAL. - libvmaf CUDA against CPU, feature by feature on the benchmark video: VIF and ADM (VMAF and NEG) bit-identical; motion2 at most 2.87e-5 (4K) and 4.53e-6 (1080p). - VMAF v1 per feature, libvmaf on one thread, 96 frames: ADM3 55.2 ms, motion3 6.0, CAMBI 12.4, SpEED 16.0 at 4K. - The whole Beekeeper film again (VideoMetricsLab's scripts/verify_vmaf_vulkan_movie.py, the release's DLLs, RTX 5090): 151,919 of 151,919 frames, all 11 features bit-identical to CUDA, VMAF and NEG identical (means 94.927743 and 90.299552). The Intel run of 2026-10-04 (earlier build) is cited as such. - The pull requests' states, authors and head commits read with gh on 2026-10-05; upstream's tree has no GPU code but CUDA. Limits - One PC. The Radeon 780M results are the other PC's reports, not re-run. - Cores busy are compared at different speeds; libvmaf's CPU time per frame grows with its thread count, so they are not a per-frame cost. - libvmaf's CUDA code from device pictures crashed in this benchmark with libvmaf's thread pool on (threads > 0); its table row is with threads = 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ADM, vmaf_picture_convert fast was upstream cea2b4d (2026-10-01), the commit VideoMetricsLab's GPU VMAF was tested at, and 20 commits behind. What comes in - Upstream's native MSVC build (15 commits, merged there on 2026-10-02): the work of pull request Netflix#1477, which this fork carried as a merge. The build files that result differ from upstream's only by this fork's CUDA changes, so Netflix#1477 is no longer a carried pull request (11 remain). - ADM on ARM NEON (3 commits): no effect on x86-64. - vmaf_picture_convert (0497a0f): a new public API, with zimg optional and off. fast/scripts/build_libvmaf_cuda.ps1 exports its three functions from libvmaf.dll, as it does the rest of the public API. - M_PI taken from math.h (4e15006). libvmaf's ABI changes with it: 0497a0f puts a VmafColor (16 bytes) into VmafPicture, before ref and priv. A program built or bound against the old structure gives libvmaf pictures 16 bytes too short, and libvmaf writes behind them. fast/python/vmaf_fast/libvmaf.py's _Picture has the new field; with the old one, compare_vmaf_v1.py --matrix ended in a segmentation fault on its first case (the Vulkan matrix happened to pass). Whoever loads this libvmaf.dll through bindings of their own (VideoMetricsLab's vmaf_cuda.py) must change them with it. Conflicts: three, each one hunk where upstream's enable_zimg / zimg_dependency met this fork's lines in libvmaf/meson_options.txt, libvmaf/src/meson.build and libvmaf/test/meson.build (the CUDA tests of the carried pull requests). Both sides kept. README: the base commit, and Netflix#1477 out of the table of carried pull requests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add native MSVC support
This pull request introduces first-class, native MSVC support for
libvmafon Windows.The goal is to enable building, testing, and integrating
libvmafusing the Microsoft Visual C++ toolchain without relying on MinGW/MSYS2, greatly improving compatibility with Windows-native projects and CI environments.Motivation
Until now,
libvmafcould only be built on Windows through MinGW/MSYS2. While functional, this approach had several limitations:This PR adds full native MSVC support, including:
Dependencies / Related PRs
This PR depends on the following PRs being merged first.
Merging them first is especially important to ensure proper credit and authorship for all contributors — particularly for the two PRs that were not authored by me (#1410 and #1428).
Once those PRs are merged upstream, the temporary copies of their commits (included here only for CI to pass) will be removed. This way the original authors receive full and fair attribution in the git history.
The required PRs are:
This ensures:
What’s included
pthread-win32as a git submodule for MSVC buildsmeson,ninja,xxd,cmake)Windows.mddocumentation to include MSVC instructionsCI Improvements
The new GitHub Actions workflow builds both MSYS2 and MSVC in parallel, runs the full test suite under each environment, and uploads artifacts for both toolchains.
Importantly, the inclusion of the Windows MSVC tests (both with FFmpeg and the standalone library) in CI guarantees that Windows support will not be accidentally broken in the future. This provides ongoing assurance and prevents regressions as the project continues to evolve.
Issue reference
Notes
git submodule update --initafter checkout to pull thepthread-win32submodule.About tox tests on Windows
This PR intentionally does not enable the Python
toxtest suite on Windows.While the MSVC build and the C test suite are fully supported, the Python tests rely on several Unix-specific behaviors (e.g.,
fork()-based multiprocessing, POSIX path assumptions, and.exesuffix handling).These issues require non-trivial work to make the tox suite fully compatible with Windows and would significantly increase the scope of this PR.
Windows
toxsupport will be evaluated and implemented in a separate PR once native MSVC support is merged.About the FFmpeg build used in CI
The MSVC CI job also builds FFmpeg to validate that
libvmafintegrates correctly with a Windows-native FFmpeg toolchain. For this purpose, the workflow uses the Meson-based FFmpeg port maintained by the GStreamer project.This port provides a clean Meson build of FFmpeg that supports MSVC out of the box, making it ideal for verifying that
libvmafbuilds and runs correctly in a fully native Windows environment. The Meson-based FFmpeg port is used exclusively for the MSVC job; the Linux and macOS CI jobs continue to use the official FFmpeg repository exactly as before.Status