Fix EB_ENC_PD_ERROR4/8 in race conditions (#580)

- 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 <[email protected]>

Author: inteltiger

Source/Lib/Codec/EbEncDecProcess.c

@@ -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

Source/Lib/Codec/EbPictureDecisionProcess.c

@@ -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++;
                             }
@@ -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++;
                             }
@@ -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

Source/Lib/Codec/EbReferenceObject.h

@@ -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;
 

Source/Lib/Codec/EbResourceCoordinationProcess.c

@@ -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;
 
@@ -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

Source/Lib/Codec/EbSystemResourceManager.c

@@ -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
@@ -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);
@@ -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;