Skip to content
This repository has been archived by the owner on Jul 29, 2024. It is now read-only.

Commit

Permalink
fix EB_ENC_PD_ERROR4 EB_ENC_PD_ERROR8 in race conditions
Browse files Browse the repository at this point in the history
race condition 1 in PA happens in this way:

EbGetEmptyObject(queue, &p);
EbObjectIncLiveCount(&p, 1);
EbReleaseObject(&p)
EbObjectIncLiveCount(&p, 1); //bug, increase live count after release
EbReleaseObject(&p) //bug, double release

fix in this way:

EbGetEmptyObject(queue, &p);
EbObjectIncLiveCount(&p, 2); //increase by 2 directly
EbReleaseObject(&p)
EbReleaseObject(&p)

race condition 2 in PD and ENC:

ENC call EbReleaseObject() before PD call EbObjectIncLiveCount(1)
in other words:
dependentPicturesCount-- before dependentPicturesCount++, so as that
dependentPicturesCount become negative e.g. -1

another race condition: ++/-- is NOT thread safe, dependentPicturesCount
++/-- happen at same time so that this counter cannot go back to zero,
leads to the queue entry will never be released, queue full, 0x2103 error

fix in this way:

first remove the useless pair of EbReleaseObject and EbObjectIncLiveCount
and then workaround: force remove the entries that stay in the queue for
unreasonable long time even dependentPicturesCount is not reduced to zero

Signed-off-by: xiaoxime <xiao.xi.meng@intel.com>
  • Loading branch information
inteltiger committed Apr 30, 2021
1 parent 9e5e7ac commit b09984c
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 32 deletions.
4 changes: 0 additions & 4 deletions Source/Lib/Codec/EbEncDecProcess.c
Original file line number Diff line number Diff line change
Expand Up @@ -3231,15 +3231,11 @@ void* EncDecKernel(void *inputPtr)
if (((pictureControlSetPtr->sliceType == EB_P_PICTURE) || (pictureControlSetPtr->sliceType == EB_B_PICTURE)) &&
pictureControlSetPtr->ParentPcsPtr->refPaPcsArray[REF_LIST_0]) {
((EbPaReferenceObject_t *)pictureControlSetPtr->ParentPcsPtr->refPaPcsArray[REF_LIST_0]->paReferencePictureWrapperPtr->objectPtr)->dependentPicturesCount--;
EbReleaseObject(pictureControlSetPtr->ParentPcsPtr->refPaPcsArray[REF_LIST_0]->paReferencePictureWrapperPtr);
EbReleaseObject(pictureControlSetPtr->ParentPcsPtr->refPaPcsArray[REF_LIST_0]->pPcsWrapperPtr);
}

if ((pictureControlSetPtr->sliceType == EB_B_PICTURE) &&
pictureControlSetPtr->ParentPcsPtr->refPaPcsArray[REF_LIST_1]) {
((EbPaReferenceObject_t *)pictureControlSetPtr->ParentPcsPtr->refPaPcsArray[REF_LIST_1]->paReferencePictureWrapperPtr->objectPtr)->dependentPicturesCount--;
EbReleaseObject(pictureControlSetPtr->ParentPcsPtr->refPaPcsArray[REF_LIST_1]->paReferencePictureWrapperPtr);
EbReleaseObject(pictureControlSetPtr->ParentPcsPtr->refPaPcsArray[REF_LIST_1]->pPcsWrapperPtr);
}

// Note: release the PPCS and its PA reference picture in both EncDec and RateControl
Expand Down
28 changes: 10 additions & 18 deletions Source/Lib/Codec/EbPictureDecisionProcess.c
Original file line number Diff line number Diff line change
Expand Up @@ -1234,17 +1234,8 @@ void* PictureDecisionKernel(void *inputPtr)
pictureControlSetPtr->refPicPocArray[REF_LIST_0] = refPoc;
pictureControlSetPtr->refPaPcsArray[REF_LIST_0] = paReferenceEntryPtr->pPcsPtr;

// Increment the PA Reference's liveCount by the number of tiles in the input picture
EbObjectIncLiveCount(
paReferenceEntryPtr->inputObjectPtr,
1);

((EbPaReferenceObject_t*)pictureControlSetPtr->refPaPicPtrArray[REF_LIST_0]->objectPtr)->pPcsPtr = paReferenceEntryPtr->pPcsPtr;

EbObjectIncLiveCount(
paReferenceEntryPtr->pPcsPtr->pPcsWrapperPtr,
1);

--paReferenceEntryPtr->dependentCount;
((EbPaReferenceObject_t *)paReferenceEntryPtr->pPcsPtr->paReferencePictureWrapperPtr->objectPtr)->dependentPicturesCount++;
}
Expand All @@ -1270,17 +1261,8 @@ void* PictureDecisionKernel(void *inputPtr)
pictureControlSetPtr->refPaPicPtrArray[REF_LIST_1] = paReferenceEntryPtr->inputObjectPtr;
pictureControlSetPtr->refPicPocArray[REF_LIST_1] = refPoc;

// Increment the PA Reference's liveCount by the number of tiles in the input picture
EbObjectIncLiveCount(
paReferenceEntryPtr->inputObjectPtr,
1);

((EbPaReferenceObject_t*)pictureControlSetPtr->refPaPicPtrArray[REF_LIST_1]->objectPtr)->pPcsPtr = paReferenceEntryPtr->pPcsPtr;

EbObjectIncLiveCount(
paReferenceEntryPtr->pPcsPtr->pPcsWrapperPtr,
1);

--paReferenceEntryPtr->dependentCount;
((EbPaReferenceObject_t*)paReferenceEntryPtr->pPcsPtr->paReferencePictureWrapperPtr->objectPtr)->dependentPicturesCount++;
}
Expand Down Expand Up @@ -1353,6 +1335,16 @@ void* PictureDecisionKernel(void *inputPtr)
if (((EbPaReferenceObject_t *)inputEntryPtr->pPcsPtr->paReferencePictureWrapperPtr->objectPtr)->dependentPicturesCount == 0) {
inputEntryPtr->inputObjectPtr = (EbObjectWrapper_t *)EB_NULL;
}
else {
// TODO sometimes dependentPicturesCount never returns zero, due to ++/-- is NOT thread safe
// Force remove the entry, if it was BLOCKED (due to dependentPicturesCount != 0) in the queue for NOT reasonable long time
EB_S32 qlen = (EB_S32)encodeContextPtr->pictureDecisionPaReferenceQueueTailIndex - (EB_S32)encodeContextPtr->pictureDecisionPaReferenceQueueHeadIndex;
if (qlen < 0) qlen += PICTURE_DECISION_PA_REFERENCE_QUEUE_MAX_DEPTH;
if (qlen > PRE_ASSIGNMENT_MAX_DEPTH) {
// SVT_LOG("Warning: force remove a blocked item from pictureDecisionPaReferenceQueue \n");
inputEntryPtr->inputObjectPtr = (EbObjectWrapper_t *)EB_NULL;
}
}
}

// Increment the HeadIndex if the head is null
Expand Down
2 changes: 1 addition & 1 deletion Source/Lib/Codec/EbReferenceObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ typedef struct EbPaReferenceObject_s {
EB_U8 yMean[MAX_NUMBER_OF_TREEBLOCKS_PER_PICTURE];
EB_PICTURE sliceType;

EB_U32 dependentPicturesCount; //number of pic using this reference frame
EB_S32 dependentPicturesCount; //number of pic using this reference frame
PictureParentControlSet_t *pPcsPtr;
} EbPaReferenceObject_t;

Expand Down
9 changes: 2 additions & 7 deletions Source/Lib/Codec/EbResourceCoordinationProcess.c
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ void* ResourceCoordinationKernel(void *inputPtr)
// Parent PCS is released by the Rate Control after passing through MDC->MD->ENCDEC->Packetization
EbObjectIncLiveCount(
pictureControlSetWrapperPtr,
1);
2);

pictureControlSetPtr = (PictureParentControlSet_t*) pictureControlSetWrapperPtr->objectPtr;

Expand Down Expand Up @@ -605,16 +605,11 @@ void* ResourceCoordinationKernel(void *inputPtr)
pictureControlSetPtr->paReferencePictureWrapperPtr = referencePictureWrapperPtr;

// Note: the PPCS and its PA reference picture will be released in both EncDec and RateControl kernels.
// Give the new Reference a nominal liveCount of 2, meanwhile increase liveCount of PPCS with 1 as it's
// already 1 after dequeuing from the PPCS FIFO.
// Give the new Reference a nominal liveCount of 2
EbObjectIncLiveCount(
pictureControlSetPtr->paReferencePictureWrapperPtr,
2);

EbObjectIncLiveCount(
pictureControlSetWrapperPtr,
1);

// Get Empty Output Results Object
// Note: record the PCS object into output of the Resource Coordination process for EOS frame(s).
// Because EbH265GetPacket() can get the encoded bit stream only if Packetization process has
Expand Down
18 changes: 16 additions & 2 deletions Source/Lib/Codec/EbSystemResourceManager.c
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,20 @@ EB_ERRORTYPE EbObjectIncLiveCount(
EbObjectWrapper_t *wrapperPtr,
EB_U32 incrementNumber)
{
EB_ERRORTYPE return_error = EB_ErrorNone;

EbBlockOnMutex(wrapperPtr->systemResourcePtr->emptyQueue->lockoutMutex);

wrapperPtr->liveCount += incrementNumber;
if (wrapperPtr->liveCount == EB_ObjectWrapperReleasedValue) {
SVT_LOG("Warning: %p is already released. Ignored the EbObjectIncLiveCount(%d) call \n", wrapperPtr, incrementNumber);
return_error = EB_ErrorBadParameter;
} else {
wrapperPtr->liveCount += incrementNumber;
}

EbReleaseMutex(wrapperPtr->systemResourcePtr->emptyQueue->lockoutMutex);

return EB_ErrorNone;
return return_error;
}

//ugly hack
Expand Down Expand Up @@ -429,6 +436,7 @@ static EB_ERRORTYPE EBObjectWrapperCtor(EbObjectWrapper_t* wrapper,
wrapper->dctor = EBObjectWrapperDctor;
wrapper->releaseEnable = EB_TRUE;
wrapper->quitSignal = EB_FALSE;
wrapper->liveCount = 0;
wrapper->systemResourcePtr = resource;
wrapper->objectDestroyer = objectDestroyer;
ret = objectCreator(&wrapper->objectPtr, objectInitDataPtr);
Expand Down Expand Up @@ -597,6 +605,12 @@ EB_ERRORTYPE EbReleaseObject(

EbBlockOnMutex(objectPtr->systemResourcePtr->emptyQueue->lockoutMutex);

if (objectPtr->liveCount == EB_ObjectWrapperReleasedValue) {
SVT_LOG("Warning: %p is already released. Ignored the double EbReleaseObject() call \n", objectPtr);
EbReleaseMutex(objectPtr->systemResourcePtr->emptyQueue->lockoutMutex);
return EB_ErrorBadParameter;
}

// Decrement liveCount
objectPtr->liveCount = (objectPtr->liveCount == 0) ? objectPtr->liveCount : objectPtr->liveCount - 1;

Expand Down

0 comments on commit b09984c

Please sign in to comment.