Skip to content

decoder: return a flushed picture's buffer when decoding single-threaded - #4037

Open
ehfd wants to merge 1 commit into
cisco:masterfrom
ehfd:flush-frame-release
Open

ehfd wants to merge 1 commit into
cisco:masterfrom
ehfd:flush-frame-release

Conversation

@ehfd

@ehfd ehfd commented Sep 29, 2026

Copy link
Copy Markdown

FlushFrame() hands the pictures it releases to ReleaseBufferedReadyPictureReorder() and ReleaseBufferedReadyPictureNoReorder() with a NULL context. Both then drop the picture's reference through m_pPicBuff, which only threaded decoding sets (in ParseAccessUnit()), so a single-threaded decoder never gets the picture's buffer back. A flush at the end of a stream hides this, but a low-latency caller that flushes after each access unit that returns no picture runs the pool dry after a few pictures, and decoding stops with PrefetchPic ERROR (dsOutOfMemory).

It bites a High profile stream without B-frames whose picture order count steps by two, which some hardware encoders write (we met it with AMD VCE output under Mesa 24.0): the decoder holds every such picture for reordering, and a flush after each access unit is the only way to take each one without a frame of delay. With DecodeFrameNoDelay() and a FlushFrame() after each access unit that returned no picture:

Stream (30 frames) v2.6.0 master master with this change
High, POC step 2, no B-frames, 32x32 (the new test's) 4 pictures, then none 4 pictures, then none 30 pictures
x264 Main, two B-frames, 320x240 7 pictures, then none 10 pictures, then none 30 pictures
x264 High without B-frames, and x264 Baseline, 320x240 30 pictures 30 pictures 30 pictures

Both functions now take the single-threaded context when given none, before choosing the picture buffer; ReleaseBufferedReadyPictureReorder() already took it, but only after that choice. Threaded decoding is unchanged and keeps using m_pPicBuff.

The new DecoderFlushFrame.FlushAfterEachAccessUnitKeepsDecoding test decodes res/High_POC_step2_32x32_30frames.264 (1.8 KB: an I_PCM IDR, then P pictures of skipped macroblocks) that way and expects a picture from every access unit. On Linux x86_64 with meson (release): test_api 209 of 209 and test_decoder 61 of 61 with the change; without it, test_api fails only the new test.

Copilot AI balanced review requested due to automatic review settings September 29, 2026 12:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

ehfd added a commit to selkies-project/pixelflux that referenced this pull request Sep 29, 2026
…ch served never comes from Selkies' clients

This reverts 362f11e, which vendored openh264-sys2 0.9.8 (344 files of
OpenH264's source) to carry a three-line fix to its single-threaded
FlushFrame, together with 4b725e3's release note and 93a3b08's
twelve-frame VA-API device check, which passed on Mesa 24.0.5 only
through that fix. The flush stalls only on H.264 above the baseline
profile, and every H.264 stream Selkies' clients send is Constrained
Baseline: the WebSockets webcam encodes avc1.42E01F, and the WebRTC
codec list carries Baseline, Constrained Baseline, and High 4:4:4, which
no browser encodes. OpenH264 never holds a Baseline picture, so the
flush never runs on one. The fix is proposed upstream as
cisco/openh264#4037, against master, which still has the bug.

Reproduced upstream with DecodeFrameNoDelay and a FlushFrame after each
access unit that returned no picture: a High-profile stream without
B-frames whose picture order count steps by two gives 4 of 30 pictures
and then none on v2.6.0 and on master, an x264 Main stream with B-frames
7 and 10; with the fix, 30 of 30 each, and an x264 Baseline stream 30 of
30 on every version. The scalable-extension test now decodes a
Constrained Baseline stream, and still fails with the extension units
let through; the flush's comment names the limitation.

cargo test --lib passes in both configurations (403 and 396), and
scripts/check-licenses.py passes.
@ehfd

ehfd commented Oct 20, 2026

Copy link
Copy Markdown
Author

The linux-cross-loongarch64 failure is DecodeMbAuxTest.WelsIDctT4Rec_lasx, which doesn't depend on this change. The test feeds the LASX IDCT random coefficients, and codec_unittest seeds rand() from time(NULL) (test/api/simple_test.cpp). This run's seed was 1791528518. Master at 1bca623 without this PR, built and run the same way as the job (gcc-14 cross, qemu-loongarch64-static ./codec_unittest --seed=1791528518), fails the same test with that seed and passes everything else. A rerun of the job should pass.

FlushFrame() releases the pictures it hands over through
ReleaseBufferedReadyPictureReorder() and ReleaseBufferedReadyPictureNoReorder()
with a NULL context. Both then drop the picture's reference through
m_pPicBuff, which only threaded decoding sets (in ParseAccessUnit()), so a
single-threaded decoder never gets the picture's buffer back. A flush at the
end of a stream hides this, but a low-latency caller that flushes after each
access unit that returns no picture runs the pool dry after a few pictures,
and the decode stops with

    Error:DecodeCurrentAccessUnit()::::::PrefetchPic ERROR, pSps->iNumRefFrames:2.

A High profile stream without B-frames whose picture order count steps by
two has every picture held for reordering: flushing after each access unit
took 4 of its 30 pictures, then none, on v2.6.0 and on master. An x264 Main
profile stream with B-frames flushed the same way stopped after 10 pictures.

Both functions now take the single-threaded context when given none, before
choosing the picture buffer; ReleaseBufferedReadyPictureReorder() already
took it, but only after that choice. Threaded decoding keeps using
m_pPicBuff. The new DecoderFlushFrame test decodes such a stream that way
and expects a picture from every access unit.
@ehfd
ehfd force-pushed the flush-frame-release branch from 6b49ebf to c167a4a Compare October 20, 2026 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants