Commit 4a0b5547 for openh264
commit 4a0b5547f47eadafecffdaa05c33290c830a0163
Author: tyan0 <takashi.yano@nifty.ne.jp>
Date: Mon Sep 14 17:38:08 2026 +0900
decoder: clear stale hand-off signals when the new-sequence chain is cut (#4010)
The new-sequence path in ParseAccessUnit() waits for every other worker
to go idle before it replaces the shared DPB storage, and then clears
the state that would otherwise point into the old storage: the chain
pointer on its own context, and pDec and pLastThreadCtx on every sibling.
That barrier is what makes the change below safe to make, and it is what
let me find this at all.
One more piece of per-worker state is left behind by the same cut.
sSliceDecodeStart is a manual-reset event, one per worker, and the worker
that consumes it is the one that resets it: ConstructAccessUnit() waits
on the previous worker's event and calls RESET_EVENT() on it afterwards.
Cutting the chain means that consumer never runs, so a signal raised just
before the boundary stays raised. The next frame that waits on that
worker then passes the wait immediately -- before the frame it is about
to mark has been decoded -- and marks whatever picture and marking state
that context still holds.
res/BA_MW_D.264 carries an IDR every 30 frames, and every access unit at
which threaded output first diverged from single-threaded output was
congruent to an IDR's modulo the thread count: the effect follows the
worker that decoded the IDR rather than a fixed frame offset, which is
what pointed at per-worker state. Instrumenting the marking shows the
consumer arriving before the producer has recorded anything for the
frame:
thr=2 begins ts=33 and clears its marking state
thr=0 at ts=34 reads thr=2's state for the marking -- not recorded yet
thr=2 records it only afterwards
and the next frame's reference list has lost a picture:
threads=1 frame_num=3 L0 = [2 1 0]
threads=3 frame_num=3 L0 = [1 0]
Reset the events in the same place the rest of that state is cleared.
Past the barrier nothing is decoding and the chain is cut for every
context, so any hand-off signal still raised has lost its consumer.
Cygwin/x86_64, gcc 14, 8 cores. ThreadDecoderPreviousPicRaceTest, 60
attempts:
parent commit 58 pass / 2 fail
with this change 60 pass / 0 fail
The whole ThreadDecoder* set passes. res/BA_MW_D.264, res/MR2_MW_A.264,
res/MR2_TANDBERG_E.264 and the 1280x544 and 1280x720 temporal_direct
vectors each give a single digest over fifteen in-process passes at two
and at three threads, equal to their threads=1 digest.
Two failures in sixty is a thin margin, so the change was also measured
on a build carrying diagnostic instrumentation, which widens the window
and raises the rate on res/BA_MW_D.264 at three threads from roughly one
pass in seventy to better than one in two. Decoding it repeatedly in one
process -- the effect only shows up that way, one decode per process
looks clean -- on the same build with the resets compiled in or out:
without with
output differs from threads=1 54 of 99 0 of 100
reference list differs from threads=1 54 0
ThreadDecoderPreviousPicRaceTest 50 of 60 fail 0 of 60 fail
ThreadDecoderPreviousPicRaceTest is what made this findable: it fails
intermittently on the parent commit, and it is deterministic afterwards.
One caveat worth stating: I could not observe the stale signal being
consumed directly. The proportion of hand-off waits that find the event
already raised is around 26% even in runs whose output is correct, so
that count cannot separate a legitimate early completion from a leftover.
The mechanism above matches the instrumented log, but the causal claim
rests on the effect of the change rather than on a direct observation of
the stale wait.
diff --git a/codec/decoder/plus/src/welsDecoderExt.cpp b/codec/decoder/plus/src/welsDecoderExt.cpp
index 80a27913..8f760eb8 100644
--- a/codec/decoder/plus/src/welsDecoderExt.cpp
+++ b/codec/decoder/plus/src/welsDecoderExt.cpp
@@ -1386,6 +1386,14 @@ DECODING_STATE CWelsDecoder::ParseAccessUnit (SWelsDecoderThreadCTX& sThreadCtx)
RELEASE_SEMAPHORE (&m_pDecThrCtxActive[i]->sThreadInfo.sIsIdle);
}
}
+ for (int32_t i = 0; i < m_iCtxCount; ++i) {
+ //Past the barrier nothing is decoding and the chain is cut for every context, so a
+ //hand-off signal still raised has lost the consumer that would have reset it. Left
+ //raised, the next frame that waits on that worker passes the wait at once and marks
+ //a picture that has not been decoded.
+ if (m_pDecThrCtx[i].pCtx != NULL)
+ RESET_EVENT (&m_pDecThrCtx[i].sSliceDecodeStart);
+ }
sThreadCtx.pCtx->pLastThreadCtx = NULL;
}
iErr = AllocPicBuffOnNewSeqBegin (sThreadCtx.pCtx);