Commit 3738dd5c for openh264
commit 3738dd5c7dd4f459c1018f5ee37238ba1ab9085f
Author: BenzhengZhang <140143892+BenzhengZhang@users.noreply.github.com>
Date: Tue Aug 25 11:23:51 2026 +0800
decoder: prevent threaded decoder deadlock on lost references (#3983)
* decoder: prevent threaded decoder deadlock on lost references
* test: add threaded decoder hang regression
Decode three frames of Static.264 with two threads (first frame truncated) to ensure DecodeFrameNoDelay cannot deadlock on lost references.
---------
Co-authored-by: benzzhan <benzzhan@cisco.com>
diff --git a/codec/decoder/core/inc/wels_decoder_thread.h b/codec/decoder/core/inc/wels_decoder_thread.h
index ebb8015c..c133b59c 100644
--- a/codec/decoder/core/inc/wels_decoder_thread.h
+++ b/codec/decoder/core/inc/wels_decoder_thread.h
@@ -103,6 +103,9 @@ typedef struct tagWelsDecThread {
#endif//_WIN32
+// Bound inter-thread waits so a lost reference cannot hang the decoder.
+#define WELS_DEC_THREAD_WAIT_TIMEOUT_MS 3000
+
#define WelsDecThreadReturn WELS_THREAD_ROUTINE_RETURN(0);
int32_t GetCPUCount();
diff --git a/codec/decoder/core/src/decoder_core.cpp b/codec/decoder/core/src/decoder_core.cpp
index dd5d5888..2be06c47 100644
--- a/codec/decoder/core/src/decoder_core.cpp
+++ b/codec/decoder/core/src/decoder_core.cpp
@@ -2817,7 +2817,11 @@ int32_t DecodeCurrentAccessUnit (PWelsDecoderContext pCtx, uint8_t** ppDst, SBuf
for (int32_t i = 0; i < iThreadCount; ++i) {
if (i == id || pThreadCtx[i - id].pCtx->uiDecodingTimeStamp == 0) continue;
if (pThreadCtx[i - id].pCtx->uiDecodingTimeStamp < pCtx->uiDecodingTimeStamp) {
- WAIT_EVENT (&pThreadCtx[i - id].sSliceDecodeFinish, WELS_DEC_THREAD_WAIT_INFINITE);
+ if (WAIT_EVENT (&pThreadCtx[i - id].sSliceDecodeFinish, WELS_DEC_THREAD_WAIT_TIMEOUT_MS)
+ != WELS_DEC_THREAD_WAIT_SIGNALED) {
+ pCtx->iErrorCode |= dsRefLost;
+ return GENERATE_ERROR_NO (ERR_LEVEL_SLICE_DATA, ERR_INFO_REFERENCE_PIC_LOST);
+ }
}
}
pCtx->pLastDecPicInfo->uiDecodingTimeStamp = pCtx->uiDecodingTimeStamp;
diff --git a/codec/decoder/core/src/rec_mb.cpp b/codec/decoder/core/src/rec_mb.cpp
index 9034cc4d..2fbf7877 100644
--- a/codec/decoder/core/src/rec_mb.cpp
+++ b/codec/decoder/core/src/rec_mb.cpp
@@ -267,7 +267,10 @@ void BaseMC (PWelsDecoderContext pCtx, sMCRefMember* pMCRefMem, const int32_t& l
if (offset > pCtx->lastReadyHeightOffset[listIdx][iRefIdx]) {
const int32_t down_line = WELS_MIN (offset >> 4, int32_t (pCtx->sMb.iMbHeight) - 1);
if (pRefPic->pReadyEvent[down_line].isSignaled != 1) {
- WAIT_EVENT (&pRefPic->pReadyEvent[down_line], WELS_DEC_THREAD_WAIT_INFINITE);
+ if (WAIT_EVENT (&pRefPic->pReadyEvent[down_line], WELS_DEC_THREAD_WAIT_TIMEOUT_MS) != WELS_DEC_THREAD_WAIT_SIGNALED) {
+ pCtx->iErrorCode |= dsRefLost;
+ return;
+ }
}
pCtx->lastReadyHeightOffset[listIdx][iRefIdx] = offset;
}
diff --git a/codec/decoder/plus/src/welsDecoderExt.cpp b/codec/decoder/plus/src/welsDecoderExt.cpp
index 06d11217..818dd3cc 100644
--- a/codec/decoder/plus/src/welsDecoderExt.cpp
+++ b/codec/decoder/plus/src/welsDecoderExt.cpp
@@ -101,7 +101,10 @@ static DECODING_STATE ConstructAccessUnit (CWelsDecoder* pWelsDecoder, PWelsDec
//WelsMutexLock (&pWelsDecoder->m_csDecoder);
if (pThrCtx->pCtx->pLastThreadCtx != NULL) {
PWelsDecoderThreadCTX pLastThreadCtx = (PWelsDecoderThreadCTX) (pThrCtx->pCtx->pLastThreadCtx);
- WAIT_EVENT (&pLastThreadCtx->sSliceDecodeStart, WELS_DEC_THREAD_WAIT_INFINITE);
+ if (WAIT_EVENT (&pLastThreadCtx->sSliceDecodeStart, WELS_DEC_THREAD_WAIT_TIMEOUT_MS) != WELS_DEC_THREAD_WAIT_SIGNALED) {
+ pThrCtx->pCtx->iErrorCode |= dsRefLost;
+ return (DECODING_STATE)GENERATE_ERROR_NO (ERR_LEVEL_SLICE_DATA, ERR_INFO_REFERENCE_PIC_LOST);
+ }
RESET_EVENT (&pLastThreadCtx->sSliceDecodeStart);
}
pThrCtx->pDec = NULL;
@@ -1339,7 +1342,7 @@ DECODING_STATE CWelsDecoder::ParseAccessUnit (SWelsDecoderThreadCTX& sThreadCtx)
}
m_bParamSetsLostFlag = sThreadCtx.pCtx->bNewSeqBegin ? false : sThreadCtx.pCtx->bParamSetsLostFlag;
m_bFreezeOutput = sThreadCtx.pCtx->bNewSeqBegin ? false : sThreadCtx.pCtx->bFreezeOutput;
- return (DECODING_STATE)iErr;
+ return (DECODING_STATE) (iRet | iErr);
}
/*
* Run decoding picture in separate thread.
@@ -1372,7 +1375,6 @@ int CWelsDecoder::ThreadDecodeFrameInternal (const unsigned char* kpSrc, const i
}
}
- m_pDecThrCtxActive[m_DecCtxActiveCount++] = &m_pDecThrCtx[signal];
if (m_pLastDecThrCtx != NULL) {
m_pDecThrCtx[signal].pCtx->pLastThreadCtx = m_pLastDecThrCtx;
}
@@ -1381,7 +1383,18 @@ int CWelsDecoder::ThreadDecodeFrameInternal (const unsigned char* kpSrc, const i
m_pDecThrCtx[signal].ppDst = ppDst;
memcpy (&m_pDecThrCtx[signal].sDstInfo, pDstInfo, sizeof (SBufferInfo));
- ParseAccessUnit (m_pDecThrCtx[signal]);
+ state = ParseAccessUnit (m_pDecThrCtx[signal]);
+ if (state != dsErrorFree) {
+ RELEASE_SEMAPHORE (&m_pDecThrCtx[signal].sThreadInfo.sIsIdle);
+ return state;
+ }
+
+ if (m_iThreadCount > 1 && m_pDecThrCtx[signal].pCtx->pAccessUnitList->uiAvailUnitsNum == 0) {
+ RELEASE_SEMAPHORE (&m_pDecThrCtx[signal].sThreadInfo.sIsIdle);
+ return state;
+ }
+
+ m_pDecThrCtxActive[m_DecCtxActiveCount++] = &m_pDecThrCtx[signal];
if (m_iThreadCount > 1) {
m_pLastDecThrCtx = &m_pDecThrCtx[signal];
}
diff --git a/test/api/thread_decoder_test.cpp b/test/api/thread_decoder_test.cpp
index 1c303f3d..2db4c20f 100644
--- a/test/api/thread_decoder_test.cpp
+++ b/test/api/thread_decoder_test.cpp
@@ -1,7 +1,11 @@
#include <gtest/gtest.h>
#include "utils/HashFunctions.h"
#include "BaseThreadDecoderTest.h"
+#include <climits>
+#include <cstring>
+#include <fstream>
#include <string>
+#include <vector>
static void UpdateHashFromPlane (SHA1Context* ctx, const uint8_t* plane,
int width, int height, int stride) {
@@ -11,6 +15,178 @@ static void UpdateHashFromPlane (SHA1Context* ctx, const uint8_t* plane,
}
}
+static int32_t ReadBitForHangRegression (uint8_t* pBufPtr, int32_t& curBit) {
+ int nIndex = curBit / 8;
+ int nOffset = curBit % 8 + 1;
+
+ curBit++;
+ return (pBufPtr[nIndex] >> (8 - nOffset)) & 0x01;
+}
+
+static int32_t ReadBitsForHangRegression (uint8_t* pBufPtr, int32_t& n, int32_t& curBit) {
+ int r = 0;
+ for (int i = 0; i < n; ++i) {
+ r |= (ReadBitForHangRegression (pBufPtr, curBit) << (n - i - 1));
+ }
+ return r;
+}
+
+static int32_t BsGetUeForHangRegression (uint8_t* pBufPtr, int32_t& curBit) {
+ int r = 0;
+ int i = 0;
+ while ((ReadBitForHangRegression (pBufPtr, curBit) == 0) && (i < 32)) {
+ ++i;
+ }
+ r = ReadBitsForHangRegression (pBufPtr, i, curBit);
+ r += (1 << i) - 1;
+ return r;
+}
+
+static int32_t ReadFirstMbInSliceForHangRegression (uint8_t* pSliceNalPtr) {
+ int32_t curBit = 0;
+ return BsGetUeForHangRegression (pSliceNalPtr + 1, curBit);
+}
+
+static int32_t ReadFrameForHangRegression (uint8_t* pBuf, const int32_t& iFileSize, const int32_t& bufPos) {
+ int32_t bytesAvailable = iFileSize - bufPos;
+ if (bytesAvailable < 4) {
+ return bytesAvailable;
+ }
+
+ uint8_t* ptr = pBuf + bufPos;
+ int32_t readBytes = 0;
+ int32_t spsCount = 0;
+ int32_t ppsCount = 0;
+ int32_t nonIdrPictCount = 0;
+ int32_t idrPictCount = 0;
+ int32_t nalDelimiterCount = 0;
+
+ while (readBytes < bytesAvailable - 4) {
+ bool has4ByteStartCode = ptr[0] == 0 && ptr[1] == 0 && ptr[2] == 0 && ptr[3] == 1;
+ bool has3ByteStartCode = false;
+ if (!has4ByteStartCode) {
+ has3ByteStartCode = ptr[0] == 0 && ptr[1] == 0 && ptr[2] == 1;
+ }
+
+ if (has4ByteStartCode || has3ByteStartCode) {
+ int32_t byteOffset = has4ByteStartCode ? 4 : 3;
+ uint8_t nalUnitType = has4ByteStartCode ? (ptr[4] & 0x1F) : (ptr[3] & 0x1F);
+
+ if (nalUnitType == 1) {
+ int32_t firstMbInSlice = ReadFirstMbInSliceForHangRegression (ptr + byteOffset);
+ if (++nonIdrPictCount >= 1 && idrPictCount >= 1 && firstMbInSlice == 0) {
+ return readBytes;
+ }
+ if (nonIdrPictCount >= 2 && firstMbInSlice == 0) {
+ return readBytes;
+ }
+ } else if (nalUnitType == 5) {
+ int32_t firstMbInSlice = ReadFirstMbInSliceForHangRegression (ptr + byteOffset);
+ if (++idrPictCount >= 1 && nonIdrPictCount >= 1 && firstMbInSlice == 0) {
+ return readBytes;
+ }
+ if (idrPictCount >= 2 && firstMbInSlice == 0) {
+ return readBytes;
+ }
+ } else if (nalUnitType == 7) {
+ if ((++spsCount >= 1) && (nonIdrPictCount >= 1 || idrPictCount >= 1)) {
+ return readBytes;
+ }
+ if (spsCount == 2) {
+ return readBytes;
+ }
+ } else if (nalUnitType == 8) {
+ if (++ppsCount >= 1 && (nonIdrPictCount >= 1 || idrPictCount >= 1)) {
+ return readBytes;
+ }
+ } else if (nalUnitType == 9) {
+ if (++nalDelimiterCount == 2) {
+ return readBytes;
+ }
+ }
+
+ if (readBytes >= bytesAvailable - 4) {
+ return bytesAvailable;
+ }
+ readBytes += 4;
+ ptr += 4;
+ } else {
+ ++ptr;
+ ++readBytes;
+ }
+ }
+
+ return bytesAvailable;
+}
+
+class ThreadDecoderHangRegressionTest : public ::testing::Test {
+};
+
+TEST_F (ThreadDecoderHangRegressionTest, Static264ThreeDecodeCallsDoNotDeadlock) {
+ std::ifstream file ("res/Static.264", std::ios::in | std::ios::binary);
+ ASSERT_TRUE (file.is_open());
+ std::vector<uint8_t> bitstream ((std::istreambuf_iterator<char> (file)), std::istreambuf_iterator<char> ());
+ ASSERT_FALSE (bitstream.empty());
+
+ int32_t fileSize = static_cast<int32_t> (bitstream.size());
+ int32_t pos = 0;
+ int32_t frame1 = ReadFrameForHangRegression (bitstream.data(), fileSize, pos);
+ pos += frame1;
+ int32_t frame2 = ReadFrameForHangRegression (bitstream.data(), fileSize, pos);
+ pos += frame2;
+ int32_t frame3 = ReadFrameForHangRegression (bitstream.data(), fileSize, pos);
+
+ ASSERT_GT (frame1, 1);
+ ASSERT_GT (frame2, 0);
+ ASSERT_GT (frame3, 0);
+
+ ISVCDecoder* decoder = NULL;
+ ASSERT_EQ (0, WelsCreateDecoder (&decoder));
+ ASSERT_TRUE (decoder != NULL);
+
+ int threadCount = 2;
+ decoder->SetOption (DECODER_OPTION_NUM_OF_THREADS, &threadCount);
+
+ SDecodingParam decodingParam;
+ std::memset (&decodingParam, 0, sizeof (SDecodingParam));
+ decodingParam.uiTargetDqLayer = UCHAR_MAX;
+ decodingParam.eEcActiveIdc = ERROR_CON_SLICE_COPY;
+ decodingParam.sVideoProperty.eVideoBsType = VIDEO_BITSTREAM_DEFAULT;
+ ASSERT_EQ (0, decoder->Initialize (&decodingParam));
+
+ uint8_t* dst[3] = {NULL, NULL, NULL};
+ SBufferInfo info;
+
+ std::memset (&info, 0, sizeof (info));
+ info.uiInBsTimeStamp = 1;
+ DECODING_STATE state = decoder->DecodeFrameNoDelay (bitstream.data(), frame1 - 1, dst, &info);
+ EXPECT_EQ (dsErrorFree, state);
+
+ std::memset (&info, 0, sizeof (info));
+ info.uiInBsTimeStamp = 2;
+ state = decoder->DecodeFrameNoDelay (bitstream.data() + frame1, frame2, dst, &info);
+ EXPECT_EQ (dsErrorFree, state);
+
+ std::memset (&info, 0, sizeof (info));
+ info.uiInBsTimeStamp = 3;
+ state = decoder->DecodeFrameNoDelay (bitstream.data() + frame1 + frame2, frame3, dst, &info);
+ EXPECT_EQ (dsErrorFree, state);
+
+ // Drain pipelined in-flight frames before teardown so Uninitialize() does not
+ // free decoder state while a worker thread is still reconstructing.
+ int32_t endOfStream = 1;
+ decoder->SetOption (DECODER_OPTION_END_OF_STREAM, &endOfStream);
+ int32_t remaining = 0;
+ decoder->GetOption (DECODER_OPTION_NUM_OF_FRAMES_REMAINING_IN_BUFFER, &remaining);
+ for (int32_t i = 0; i < remaining; ++i) {
+ std::memset (&info, 0, sizeof (info));
+ decoder->FlushFrame (dst, &info);
+ }
+
+ decoder->Uninitialize();
+ WelsDestroyDecoder (decoder);
+}
+
class ThreadDecoderCapabilityTest : public ::testing::Test {
public:
virtual void SetUp() {}