libvmaf: release both pictures on every return of vmaf_read_pictures()

[lusoris]

libvmaf.h says VmafContext takes ownership of both pictures passed to vmaf_read_pictures(). When the call returns an error it does not: it returns with both references still held for the caller. A caller that follows the header leaks the pair, and a caller that took the pair from vmaf_preallocate_pictures() hangs in vmaf_close(). This makes the call release both pictures on every return, so the code does what the header says.

What breaks today

Master 8e7a1ac4e, a small C program over the public API (CPU, no feature other than psnr): a first 32x32 pair is accepted, then a 64x48 pair is submitted. validate_pic_params() rejects it with -22. The caller does not unref, as the header says.

case	master	this branch
pair from vmaf_picture_alloc(), ASan + UBSan (LeakSanitizer)	9272 bytes leaked in 6 allocations (both pictures, their VmafRef and private data)	no leak
pair from vmaf_preallocate_pictures() (pool of 2), then vmaf_close()	never returns; the 10 s alarm() ends it (exit 142). vmaf_close() waits in vmaf_picture_pool_close() until every pool picture is back	returns

The error returns of vmaf_read_pictures() that leave the pair with the caller on master: validate_pic_params(), check_picture_pool(), check_ring_buffer() and the reference translate_picture() (CUDA builds), an extractor failure in the dispatch loop, and a failed enqueue in threaded_read_pictures_batch() (it drops the extra references it took itself, not the caller's), and the call after the context was flushed.

Fix

Once the call has a context and both pictures, each of those returns drops both references through the caller's pointers (release_picture_pair()). vmaf_picture_unref() clears the struct it is given (picture.c, the memset at its end), so a caller that unrefs after the error gets -EINVAL and nothing is released twice. A call without a context, with only one of the two pictures, and the flush call (both NULL) take nothing, as before. In vmaf_read_pictures() the flushed test moves below the flush branch: a flush after a flush still returns -EINVAL; a pair after a flush is now released as well.

On the threaded path a failed vmaf_thread_pool_enqueue() unrefs the three references threaded_read_pictures_batch() took (pic_a, pic_b, the previous pictures) and returns the error without touching the caller's pair; the caller's references are now released once, in vmaf_read_pictures().

libvmaf.h gets two sentences: the ownership holds on an error return too, and which calls take nothing.

Callers

FFmpeg's libavfilter/vf_libvmaf.c (master at 663a37f7f9d3, the latest commit touching the file; read on 2026-10-01): lines 163-166 and 790-793 log and return AVERROR(EINVAL) after a failed vmaf_read_pictures() and do not unref pic_ref / pic_dist (the unref at line 159 is for the earlier copy_picture_data() failure). It follows the header, so it leaked the pair on an error and is correct with this change. In this tree tools/vmaf.c does not touch the pictures after an error either, and vmaf_close() is not reached on that path (it returns after the flush), so the CLI is unaffected. A caller that unrefs after an error today (the tests added by #1639 and #1621 do) still works: the struct is cleared, the unref returns -EINVAL and releases nothing.

Test

libvmaf/test/test_read_pictures_release.c (new, registered with a 20 s timeout):

• a rejected pair: with an extra reference taken by the test, the count is back to 1 after the call (2 on master);
• the caller's structs are cleared and a second unref returns -EINVAL;
• one picture only, no context, and the flush call take nothing, and a second flush is rejected;
• a rejected pair from the pool: the two pool pictures can be fetched again, and vmaf_close() returns.

The pool case hangs on master. The bound is meson's timeout : 20 on the test() entry, so a regression fails the test instead of hanging the suite; run directly, the binary would block. On master the test fails at the first case ("rejected ref still referenced by the context"); with only the pool case it ends in TIMEOUT after 20 s; with the change it passes.

Validation

x86-64 Linux, GCC 16.2.1, RTX 4090 with CUDA 13.4.

Rebased on master 9e48141b (2026-10-02). The release and sanitizer builds, the new test and the Netflix pair comparison were re-run on it. The new test's failure on unpatched master (with libvmaf.c and libvmaf.h reverted, LeakSanitizer) was measured on cea2b4d8 and not repeated: neither file changed since. The CUDA build line below was measured on 8e7a1ac4e and not repeated.

• Release meson test with -Denable_float=true -Denable_checkasm=true: 25/25 on master, 26/26 here (the new test), re-run on 9e48141b. Default options (22/22 and 23/23) were measured on cea2b4d8 and not repeated.
• -Db_sanitize=address,undefined -Db_lto=false, same options: master 22 pass and 3 fail of 25; here 23 pass and 3 fail of 26. The failures are the same three on both: test_predict and test_pic_preallocation (SIGABRT, LeakSanitizer) and checkasm (heap-buffer-overflow in adm_dwt2_16, integer_adm.c:2603); this change touches none of them.
• The three Netflix pairs, .engagement/golden_compare.py master binary vs this branch: all identical (default dispatch and cpumask=-1), e.g. src01 vmaf_mean 76.667831 on both.
• CUDA build (-Denable_cuda=true, libvmaf/build-cuda), under the device lock: it compiles; test_read_pictures_release and test_pic_preallocation pass; test_cuda_pic_preallocation fails with SIGSEGV here and on unpatched master. vmaf --gpumask 0 on src01 gives vmaf_mean 76.668905 on both.
• Applying this commit on top of the branches of libvmaf: reject picture indices that do not increase in vmaf_read_pictures() #1639 and libvmaf: reject pictures whose bit depth does not match #1621 (the former needs the index check to release the pair too, see below): both build and pass with the same two sanitizer failures.

Not covered

• CUDA error paths that cannot be reached from the API without fault injection, and so were not run: translate_picture_device() leaves the freshly allocated host picture allocated when vmaf_cuda_picture_download_async() fails; the return value of the dist translate_picture() call is never checked; and on an error in the dispatch loop with device input, the host copy made for the CPU extractors is not released. cuda: complete device-picture cleanup after threaded dispatch #1613 does not touch those error paths (it changes the download mask to 0x7, adds a stream synchronize that returns -EIO without freeing the host picture, and moves the device cleanup after the threaded dispatch). They stay as they are.
• Callers outside this tree that unref after an error and rely on the struct being intact afterwards.

Relation to other changes

• cuda: complete device-picture cleanup after threaded dispatch #1613 edits the tail of vmaf_read_pictures() (the thread-pool branch); the two conflict textually in libvmaf.c. The resolution is mechanical: its if (vmaf->thread_pool) restructure plus release_picture_pair() on an error.
• libvmaf: reject picture indices that do not increase in vmaf_read_pictures() #1639 (index order) adds an -EINVAL return before the pair is touched; with this change that return needs the same release_picture_pair() call. libvmaf: reject pictures whose bit depth does not match #1621 (&& to || in validate_pic_params()) needs nothing: its error comes through the path released here. Both PRs' tests unref after a rejection and ignore the result, so they pass with or without this change.
• cuda: return an error from vmaf_cuda_buffer_alloc() when the device is out of memory #1646 changes vmaf_cuda_buffer_alloc() from assert(0) to an error (issue Crash when 2 files are analyzed simultaneously .../src/cuda/common.c:166: vmaf_cuda_buffer_alloc: Assertion `0' failed. #1420); an out-of-memory then reaches vmaf_read_pictures() as an error return, which this change handles. Other CHECK_CUDA sites, for example in vmaf_cuda_picture_alloc() at 400 MiB of free VRAM, still abort.

[lusoris]

Rebased onto acdd937; the new head is dd51642. The conflict was in libvmaf.c at the top of vmaf_read_pictures(), where convert_pictures() now runs. It is placed after the flushed check, and a failure of it releases both pictures like every other return. Both pictures are released through the caller's pointers, so this holds after a successful conversion too (the converted pictures have replaced the originals in the caller's structs). The new conversion-failure return is not covered by a test of its own; test_read_pictures_release covers the other returns (validation, pool, flushed, one-sided calls) and the pool-pair hang in vmaf_close().

Release build with checkasm: all 28 meson tests pass, and test_read_pictures_release runs 4 tests, all passing. ASan/UBSan build: 24 pass and 4 fail (test_predict, checkasm, test_read_pictures_convert, test_pic_preallocation); the same 4 fail on unpatched master acdd937, and test_read_pictures_release is not among them.