[glamberson]

Fixes #3872.

BufferingReadyPicture() in welsDecoderExt.cpp pins the previous decoded picture for possible error concealment when decoding single-threaded (++pPrevPic->iRefCount, only when GetThreadCount(pCtx) <= 1). Nothing released that pin, so every decoded picture leaked one picture-queue slot until the next IDR reset state. This is not a queue size problem: Neither iNumRefFrames * 2 nor iNumRefFrames + 4, both discussed in the issue, closes it. They only delay the failure. It also explains why maintainers testing with ffplay or mpv could not reproduce it past 2.5.0: Those default to multi-threaded decoding, which never takes the pin.

The fix records the pinned picture in SWelsLastDecPicInfo, next to pPreviousDecodedPictureInDpb, at the place the pin is taken. The next time BufferingReadyPicture() pins a different picture it releases the previous pin first, and it does nothing when it runs again for the same picture. Releasing the pin from decoder_core.cpp instead is not safe, because whether a picture is ever pinned depends on the immediate-output fast path in ReorderPicturesInDisplay(), which that file cannot see. The decoder core clears the pin wherever it frees the picture pool or zeroes the reference counts (WelsRequestMem() and DecreasePicBuff()), the same places it already clears pPreviousDecodedPictureInDpb, so the pin never refers to a freed picture.

Tests, all in test/api/decoder_ec_test.cpp:

• SingleRefFrameLongGopDecodesCleanly decodes a captured stream (res/issue3872_repro.264: 45 pictures, max_num_ref_frames=1, two IDR cycles) and calls FlushFrame() after every call that produces no output, which is how the openh264 Rust binding's default mode drives the API. Without the fix it fails at picture 3 with dsOutOfMemory, the signature reported in the issue. A synthetic encoder-generated stream with the same parameters did not reproduce it.
• BSliceReorderingDoesNotCorruptRefCounts decodes a real B-frame stream with multiple references and B-pyramid, which covers the immediate-output path.
• ResolutionChangeDoesNotLeaveDanglingPin decodes clips of different sizes back to back on one decoder. It reports no decode error even when the pin dangles, so it is meant for the asan job, where it fails with a heap-use-after-free without the pool-rebuild clearing.

Verified on current master (1bca623): The first two tests fail without the fix and pass with it. The full unit test suite passes (667 tests), including when built with the same command as the asan job (LDFLAGS=-fsanitize=address CFLAGS=-fsanitize=address make), and the meson build with --werror compiles cleanly.

[Copilot]

Copilot review overview

🟡 Changes recommended

The reference-count handling and regression test setup require correction before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Double-decrement reorder picture reference count · New

What changed in this PR

Fixes the single-threaded decoder picture-queue leak and adds regression coverage using a captured bitstream.

Changes:

• Adjusts previous-picture reference handling.
• Adds direct decode/flush regression coverage.

File	Summary
test/​api/​decoder_ec_test.cpp	Adds regression coverage. Moderate findings (1 vote each): configure one thread before initialization and prevent output locals from going out of scope.
codec/​decoder/​core/​src/​decoder_core.cpp	Adds the reference-count decrement. Critical finding (3 votes): the decrement may double-release pictures already handled by the normal release path.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.