[x265] [PATCH] Use mixed bitmap between FrameEncoder and FrameFilter to Fix crash and hash mistake in WPP mode

Min Chen chenm003 at 163.com
Mon Sep 16 13:34:09 CEST 2013


# HG changeset patch
# User Min Chen <chenm003 at 163.com>
# Date 1379324742 -28800
# Node ID 3cae8b54f7e7139ec0502d44fef8088280826a25
# Parent  5a00b5a430d138ed4f77d008d36f037aa8536a6d
Use mixed bitmap between FrameEncoder and FrameFilter to Fix crash and hash mistake in WPP mode

I change task schedult bitmap to mixed FrameEncoder and FrameFilter
because there catch two bugs, and I want to reduce latency of Frame
Parallelism.
The new bitmap mapping 2N+0 to FrameEncoder and 2N+1 to FrameFilter.

Side effect:
1. We can remove the lock from FrameFilter.
2. Mixed bitmap let us do Filter early, so reduce latency of Frame
   Parallelism

Solved bugs:
1. CRASH: the reason is sometime two of threads finish in same time,
   so they will enter Filter in wrong order and sent Finished Event
   early.
   when main thread dequeue JobProvider and execute FrameFilter, we
   will catch a crash!

2. HASH MISTAKE: the reason is same as below, but last row is right
   order, we will got worng reconst image.

diff -r 5a00b5a430d1 -r 3cae8b54f7e7 source/encoder/frameencoder.cpp
--- a/source/encoder/frameencoder.cpp	Mon Sep 16 17:41:02 2013 +0800
+++ b/source/encoder/frameencoder.cpp	Mon Sep 16 17:45:42 2013 +0800
@@ -98,7 +98,8 @@
         m_rows[i].create(top);
     }
 
-    if (!WaveFront::init(m_numRows))
+    // NOTE: 2 times of numRows because both Encoder and Filter in same queue
+    if (!WaveFront::init(m_numRows * 2))
     {
         assert(!"Unable to initialize job queue.");
         m_pool = NULL;
@@ -870,9 +871,9 @@
                 }
             }
 
-            WaveFront::enableRow(row);
+            enableRowEncoder(row);
             if (row == 0)
-                WaveFront::enqueueRow(row);
+                enqueueRowEncoder(0);
             else
                 m_pool->pokeIdleThread();
         }
@@ -883,31 +884,43 @@
     }
     else
     {
-        for (int i = 0; i < this->m_numRows; i++)
+        for (int i = 0; i < this->m_numRows + m_filterRowDelay; i++)
         {
-            // block until all reference frames have reconstructed the rows we need
-            for (int l = 0; l < numPredDir; l++)
+            // Encoder
+            if (i < m_numRows)
             {
-                RefPicList list = (l ? REF_PIC_LIST_1 : REF_PIC_LIST_0);
-                for (int ref = 0; ref < slice->getNumRefIdx(list); ref++)
+                // block until all reference frames have reconstructed the rows we need
+                for (int l = 0; l < numPredDir; l++)
                 {
-                    TComPic *refpic = slice->getRefPic(list, ref);
-                    while ((refpic->m_reconRowCount != (UInt)m_numRows) && (refpic->m_reconRowCount < i + refLagRows))
+                    RefPicList list = (l ? REF_PIC_LIST_1 : REF_PIC_LIST_0);
+                    for (int ref = 0; ref < slice->getNumRefIdx(list); ref++)
                     {
-                        refpic->m_reconRowWait.wait();
+                        TComPic *refpic = slice->getRefPic(list, ref);
+                        while ((refpic->m_reconRowCount != (UInt)m_numRows) && (refpic->m_reconRowCount < i + refLagRows))
+                        {
+                            refpic->m_reconRowWait.wait();
+                        }
                     }
                 }
+
+                processRow(i * 2 + 0);
             }
 
-            processRow(i);
+            // Filter
+            if (i >= m_filterRowDelay)
+            {
+                processRow((i - m_filterRowDelay) * 2 + 1);
+            }
         }
     }
 }
 
-void FrameEncoder::processRow(int row)
+void FrameEncoder::processRowEncoder(int row)
 {
     PPAScopeEvent(Thread_ProcessRow);
 
+    //printf("Encoder(%2d)\n", row);
+
     // Called by worker threads
     CTURow& curRow  = m_rows[row];
     CTURow& codeRow = m_rows[m_cfg->param.bEnableWavefront ? row : 0];
@@ -941,7 +954,7 @@
                 m_rows[row + 1].m_completed + 2 <= m_rows[row].m_completed)
             {
                 m_rows[row + 1].m_active = true;
-                WaveFront::enqueueRow(row + 1);
+                enqueueRowEncoder(row + 1);
             }
         }
 
@@ -957,13 +970,16 @@
     // Run row-wise loop filters
     if (row >= m_filterRowDelay)
     {
-        m_frameFilter.processRow(row - m_filterRowDelay);
+        enqueueRowFilter(row - m_filterRowDelay);
+
+        // NOTE: Active Filter to first row (row 0)
+        if (row == m_filterRowDelay)
+            enableRowFilter(0);
     }
     if (row == m_numRows - 1)
     {
         for(int i = m_numRows - m_filterRowDelay; i < m_numRows; i++)
-            m_frameFilter.processRow(i);
-        m_completionEvent.trigger();
+            enqueueRowFilter(i);
     }
 }
 
diff -r 5a00b5a430d1 -r 3cae8b54f7e7 source/encoder/frameencoder.h
--- a/source/encoder/frameencoder.h	Mon Sep 16 17:41:02 2013 +0800
+++ b/source/encoder/frameencoder.h	Mon Sep 16 17:45:42 2013 +0800
@@ -64,7 +64,54 @@
 
     void destroy();
 
-    void processRow(int row);
+    void processRowEncoder(int row);
+
+    void processRowFilter(int row)
+    {
+        m_frameFilter.processRow(row);
+    }
+
+    void enqueueRowEncoder(int row)
+    {
+        WaveFront::enqueueRow(row * 2 + 0);
+    }
+
+    void enqueueRowFilter(int row)
+    {
+        WaveFront::enqueueRow(row * 2 + 1);
+    }
+
+    void enableRowEncoder(int row)
+    {
+        WaveFront::enableRow(row * 2 + 0);
+    }
+
+    void enableRowFilter(int row)
+    {
+        WaveFront::enableRow(row * 2 + 1);
+    }
+
+    void processRow(int row)
+    {
+        const int realRow = row >> 1;
+        const int typeNum = row & 1;
+
+        // TODO: use switch when more type
+        if (typeNum == 0)
+        {
+            processRowEncoder(realRow);
+        }
+        else
+        {
+            processRowFilter(realRow);
+
+            // NOTE: Active next row
+            if (realRow != m_numRows - 1)
+                enableRowFilter(realRow + 1);
+            else
+                m_completionEvent.trigger();
+        }
+    }
 
     TEncEntropy* getEntropyCoder(int row)      { return &this->m_rows[row].m_entropyCoder; }
 
diff -r 5a00b5a430d1 -r 3cae8b54f7e7 source/encoder/framefilter.cpp
--- a/source/encoder/framefilter.cpp	Mon Sep 16 17:41:02 2013 +0800
+++ b/source/encoder/framefilter.cpp	Mon Sep 16 17:45:42 2013 +0800
@@ -102,8 +102,6 @@
 {
     PPAScopeEvent(Thread_filterCU);
 
-    ScopedLock s(m_filterLock); // Only allow one row to be filtered at a time
-
     if (!m_cfg->param.bEnableLoopFilter)
     {
         processRowPost(row);
diff -r 5a00b5a430d1 -r 3cae8b54f7e7 source/encoder/framefilter.h
--- a/source/encoder/framefilter.h	Mon Sep 16 17:41:02 2013 +0800
+++ b/source/encoder/framefilter.h	Mon Sep 16 17:45:42 2013 +0800
@@ -70,8 +70,6 @@
     TEncBinCABACCounter         m_rdGoOnBinCodersCABAC;
     TComBitCounter              m_bitCounter;
     TEncSbac*                   m_rdGoOnSbacCoderRow0;  // for bitstream exact only, depends on HM's bug
-
-    Lock                        m_filterLock;
 };
 }
 



More information about the x265-devel mailing list