[#1381] Change cuMemFreeAsync to cuMemFree in vmaf_cuda_picture_free()#1382
Conversation
|
Tagging @gedoensmax |
|
When does this crash occur ? During processing or only after processing for a while ? When using CUDA the internally preallocated pictures should be used in FFmpeg so that this free should only be called after processing all the pictures when closing the context. |
|
Dear @gedoensmax
The VMAF score is output correctly, but an assertion occurs when ffmpeg exits. Reproducing the issue is very easy. The key point for reproducing the issue is setting the virtual memory limit using ulimit -v.
I followed your guide and set the environment variables as shown below, but the same issue continues to occur. |
|
Ok so when using the internal memory pool correctly it should not have any performance impact to move to the synchronous version of this API. In case this is not possible though and pictures are allocated and free'd dynamically it will introduce a CUDA synchronization. |
We were limiting the virtual memory before running the ffmpeg process due to an unresolved memory leak issue in ffmpeg.
We understand what you mean. |
|
As you are saying this is not an easy questionand i think if your change is fixing the problem you shall use it. Do you need the change to be in main branch though ? Or maybe you can expose the change to synchronous free through an option ? |
|
As you mentioned, we can apply this fix only in our branch and it does not need to be applied to the main branch. |
Replace cuMemFreeAsync(ptr, priv->cuda.str) with synchronous cuMemFree(ptr) at libvmaf/src/cuda/picture_cuda.c:247 to fix the assertion-0 crash in vmaf_cuda_picture_free() reported upstream as Netflix#1381 and addressed by the open upstream PR Netflix#1382. The fork already issues cuStreamSynchronize(priv->cuda.str) two lines earlier, so the async variant offered no overlap benefit - cuMemFree is both correct and non-regressive for performance. Fixes T0-1 in .workingdir2/BACKLOG.md. First commit of Batch-A upstream small-fix sweep. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…4/0135) (#72) * fix(cuda): port Netflix#1382 cuMemFreeAsync -> cuMemFree (ADR-0131) Replace cuMemFreeAsync(ptr, priv->cuda.str) with synchronous cuMemFree(ptr) at libvmaf/src/cuda/picture_cuda.c:247 to fix the assertion-0 crash in vmaf_cuda_picture_free() reported upstream as Netflix#1381 and addressed by the open upstream PR Netflix#1382. The fork already issues cuStreamSynchronize(priv->cuda.str) two lines earlier, so the async variant offered no overlap benefit - cuMemFree is both correct and non-regressive for performance. Fixes T0-1 in .workingdir2/BACKLOG.md. First commit of Batch-A upstream small-fix sweep. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(feature): port Netflix#1406 mount/unmount model-list bugfix (ADR-0132) Fix vmaf_feature_collector_mount_model list corruption on >=3 mounts (the previous **head traversal mutated the list instead of walking a local cursor) and align vmaf_feature_collector_unmount_model to return -ENOENT for not-found instead of -EINVAL. Test coverage extended to a 3-element mount/unmount sequence with insertion-order verification. Upstream duplicated the setup across both tests; refactored into a shared load_three_test_models / destroy_three_test_models helper to keep each test body under the JPL Power-of-10 rule-4 size threshold. T4-4 in .workingdir2/BACKLOG.md. Second commit of Batch-A. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * build(meson): port Netflix#1451 declare_dependency + override_dependency (ADR-0134) Appends declare_dependency(link_with: libvmaf, include_directories: [libvmaf_inc]) + meson.override_dependency('libvmaf', libvmaf_dep) to libvmaf/src/meson.build so consumers can use the fork as a meson subproject with the standard dependency('libvmaf') idiom. Fork uses trailing-comma style to match fork build-file conventions. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(api): port Netflix#1424 built-in model version iterator (ADR-0135) Add public vmaf_model_version_next(prev, &version) iterator. Ports upstream PR#1424's API surface verbatim while correcting three latent defects during port: - NULL-pointer arithmetic UB on first call (upstream missed an else between the two if-branches, so NULL+1 on the second). - Off-by-one returning the {0} sentinel at end of iteration — condition must be idx+1 < CNT, not idx < CNT. - const-qualifier mismatches in the test (upstream used char* / void* against a const-qualified API, not allowed in C11). Early-returns NULL when BUILT_IN_MODEL_CNT == 0 so zero-models build configurations link cleanly. Test asserts the iterator both hands out the stored version pointer and visits every model exactly once. Doxygen header-doc replaces upstream's one-line comment. docs/api/index.md gains a programmatic-discovery example per the CLAUDE §12 r10 per-surface doc rule. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(batch-a): ADR-0108 deep-dive deliverables for upstream port sweep Bundles the five in-tree deep-dive deliverables for the Batch-A PR (ADR-0108 §Per-surface minimum bars; sixth deliverable — the reproducer/smoke-test — lives in the PR description). - docs/research/0009-batch-a-upstream-port-strategy.md: research digest covering why port OPEN upstream PRs now, per-PR defect analysis (three latent bugs corrected in Netflix#1424, test refactor in Netflix#1406), port-from-PR-tip rationale, and batch-shape justification. - CHANGELOG.md: Added entries for T4-5 (meson declare_dependency) and T4-6 (vmaf_model_version_next iterator); Fixed entries for T0-1 (CUDA cuMemFree) and T4-4 (feature_collector list corruption). - docs/rebase-notes.md §0031: Touches / Invariant / Re-test for all four ports with explicit "keep fork version on upstream conflict" resolution policy and file-by-file conflict predictions. - libvmaf/src/feature/AGENTS.md: new rebase-sensitive invariant for feature_collector mount/unmount traversal + shared test helpers. - libvmaf/src/cuda/AGENTS.md: new Rebase-sensitive invariants section for picture_cuda.c synchronous cuMemFree contract. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * style(feature-collector): clang-format test helpers from Batch-A T4-4 port The `load_three_test_models` signature and one `mu_assert` split arguments across two lines where .clang-format prefers a single line — caught by pre-commit on the Batch-A deep-dive pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This PR is a fix to #1381
Description
We have identified that when using the libvmaf_cuda(vmaf-3.0.0) filter with ffmpeg in a constrained virtual memory environment, an assertion failure occurs in vmaf_cuda_picture_free() when ffmpeg terminates.
You can easily reproduce the issue by running the ffmpeg command with the libvmaf_cuda filter along with
ulimit -v 16777216. (16777216 represents 16GB, which is a sufficient size for virtual memory.)According to the API documentation, cuMemFreeAsync() does not return CUDA_ERROR_OUT_OF_MEMORY. However, in this abnormal situation, it is returning CUDA_ERROR_OUT_OF_MEMORY.
We have confirmed that using the synchronous memory free API, cuMemFree(), resolves the issue. We propose modifying the code to use cuMemFree() for freeing memory until the underlying cause of the issue with the CUDA asynchronous API is resolved.