<div dir="ltr">Yes, need to handle the processing functions from FrameFilter (instead of ParallelFilter)</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 14, 2016 at 2:22 PM, Ashok Kumar Mishra <span dir="ltr"><<a href="mailto:ashok@multicorewareinc.com" target="_blank">ashok@multicorewareinc.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I think FrameFilter should keep the instance of ParallelFilter, which has already there in code.<div><br></div><div>ParallelFilter*     m_parallelFilter;<br></div><div><br></div><div>So in that case we don't need nested class.</div><div><br></div><div>Thanks</div><span class="HOEnZb"><font color="#888888"><div>Ashok.</div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 14, 2016 at 1:31 PM, Deepthi Nandakumar <span dir="ltr"><<a href="mailto:deepthi@multicorewareinc.com" target="_blank">deepthi@multicorewareinc.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Min,<div><br></div><div>Sure - what I mean is, can we have a class that looks like: </div><div><br></div><div><div>class FrameFilter</div><div>{</div><div>public:</div><div><br></div><div>    x265_param*   m_param;</div><div>    Frame*        m_frame;</div><div>    FrameEncoder* m_frameEncoder;</div><div>    FrameData*    m_encData;</div><div>    int           m_hChromaShift;</div><div>    int           m_vChromaShift;</div><div>    int           m_pad[2];</div><div><br></div><div>    uint32_t      m_numRows;</div><div>    uint32_t      m_numCols;</div><div>    uint32_t      m_lastWidth;</div><div>   .................</div><div>..................</div><div>}</div><div><br></div><div>class ParallelFilter</div><div>{</div><div>public:</div><div>    uint32_t            m_rowHeight;</div><div>    uint32_t            m_row;</div><div>    uint32_t            m_rowAddr;</div><div><br></div><div>    ParallelFilter*     m_prevRow;</div><div>    SAO                 m_sao;</div><div>    ThreadSafeInteger   m_lastCol;          /* The column that next to process */</div><div>    ThreadSafeInteger   m_allowedCol;       /* The column that processed from Encode pipeline */</div><div>    ThreadSafeInteger   m_lastDeblocked;   /* The column that finished all of Deblock stages  */</div><div><br></div><div>}</div></div><div><br></div><div>The common variables are in FrameFilter, and ParallelFilter contains only the row specific data. </div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 13, 2016 at 12:15 AM, chen <span dir="ltr"><<a href="mailto:chenm003@163.com" target="_blank">chenm003@163.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="line-height:1.7;color:#000000;font-size:14px;font-family:arial"><div>I fixed the crash and modify/remove FrameFilter::processPostCu, I spending ~64 bytes every row.</div><div><br></div><div>In my design, FrameFilter just one instance every frame and ParallelFilter instance for every row.</div><div><div><div><br></div><div></div><div></div><div><br></div>At 2016-01-12 13:37:36,"Deepthi Nandakumar" <<a href="mailto:deepthi@multicorewareinc.com" target="_blank">deepthi@multicorewareinc.com</a>> wrote:<br> <blockquote style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"><div dir="ltr"><div>Min, thanks. Queuing this.<br><br></div>I still think the ParallelFilter class needs a refactor - not nested, but inherited from FrameFilter. And FrameFilter contains all the constants for a single frame - numCols, numRows etc. This will also clean up FrameFilter::processPostCu (which is causing the crash in no-deblock, no-sao, since now you'll only refer to FrameFilter class members and not ParallelFilter (which is null if sao and deblock are disabled). <br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 12, 2016 at 2:36 AM, Min Chen <span dir="ltr"><<a href="mailto:chenm003@163.com" target="_blank">chenm003@163.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"># HG changeset patch<br>
# User Min Chen <<a href="mailto:chenm003@163.com" target="_blank">chenm003@163.com</a>><br>
# Date 1452545402 21600<br>
# Node ID 1c273c83a4478ed39b0e6734eec1ba1cfccd07d6<br>
# Parent  19cfada7162147f293e37302e4c7f9c1760928a0<br>
fix deadlock and output change on new ParallelFilter framework. (Issue #225)<br>
The bug from two part:<br>
1. we old sync system allow latest column execute parallelism<br>
2. new ParallelFilter logic will distribute threads and waitting the status change, there have a race conditions<br>
---<br>
 source/encoder/frameencoder.cpp |  108 +++++++++++++++++++--------------------<br>
 1 files changed, 53 insertions(+), 55 deletions(-)<br>
<br>
diff -r 19cfada71621 -r 1c273c83a447 source/encoder/frameencoder.cpp<br>
--- a/source/encoder/frameencoder.cpp   Mon Jan 11 16:28:39 2016 +0530<br>
+++ b/source/encoder/frameencoder.cpp   Mon Jan 11 14:50:02 2016 -0600<br>
@@ -962,6 +962,58 @@<br>
             // Save CABAC state for next row<br>
             curRow.bufferedEntropy.loadContexts(rowCoder);<br>
<br>
+        /* SAO parameter estimation using non-deblocked pixels for CTU bottom and right boundary areas */<br>
+        if (m_param->bEnableSAO && m_param->bSaoNonDeblocked)<br>
+            m_frameFilter.m_parallelFilter[row].m_sao.calcSaoStatsCu_BeforeDblk(m_frame, col, row);<br>
+<br>
+        /* Deblock with idle threading */<br>
+        if (m_param->bEnableLoopFilter | m_param->bEnableSAO)<br>
+        {<br>
+            // TODO: Multiple Threading<br>
+            // Delay ONE row to avoid Intra Prediction Conflict<br>
+            if (m_pool && (row >= 1))<br>
+            {<br>
+                // Waitting last threading finish<br>
+                m_frameFilter.m_parallelFilter[row - 1].waitForExit();<br>
+<br>
+                // Processing new group<br>
+                int allowCol = col;<br>
+<br>
+                // avoid race condition on last column<br>
+                if (row >= 2)<br>
+                {<br>
+                    allowCol = X265_MIN(((col == numCols - 1) ? m_frameFilter.m_parallelFilter[row - 2].m_lastDeblocked.get()<br>
+                                                              : m_frameFilter.m_parallelFilter[row - 2].m_lastCol.get()), (int)col);<br>
+                }<br>
+                m_frameFilter.m_parallelFilter[row - 1].m_allowedCol.set(allowCol);<br>
+                m_frameFilter.m_parallelFilter[row - 1].tryBondPeers(*this, 1);<br>
+            }<br>
+<br>
+            // Last Row may start early<br>
+            if (m_pool && (row == m_numRows - 1))<br>
+            {<br>
+                // Waiting for the last thread to finish<br>
+                m_frameFilter.m_parallelFilter[row].waitForExit();<br>
+<br>
+                // Deblocking last row<br>
+                int allowCol = col;<br>
+<br>
+                // avoid race condition on last column<br>
+                if (row >= 2)<br>
+                {<br>
+                    allowCol = X265_MIN(((col == numCols - 1) ? m_frameFilter.m_parallelFilter[row - 1].m_lastDeblocked.get()<br>
+                                                              : m_frameFilter.m_parallelFilter[row - 1].m_lastCol.get()), (int)col);<br>
+                }<br>
+                m_frameFilter.m_parallelFilter[row].m_allowedCol.set(allowCol);<br>
+                m_frameFilter.m_parallelFilter[row].tryBondPeers(*this, 1);<br>
+            }<br>
+        }<br>
+        // Both Loopfilter and SAO Disabled<br>
+        else<br>
+        {<br>
+            m_frameFilter.processPostCu(row, col);<br>
+        }<br>
+<br>
         // Completed CU processing<br>
         curRow.completed++;<br>
<br>
@@ -1091,60 +1143,6 @@<br>
             }<br>
         }<br>
<br>
-        // TODO: move Deblock and SAO to before VBV check<br>
-<br>
-        /* SAO parameter estimation using non-deblocked pixels for CTU bottom and right boundary areas */<br>
-        if (m_param->bEnableSAO && m_param->bSaoNonDeblocked)<br>
-            m_frameFilter.m_parallelFilter[row].m_sao.calcSaoStatsCu_BeforeDblk(m_frame, col, row);<br>
-<br>
-        /* Deblock with idle threading */<br>
-        if (m_param->bEnableLoopFilter | m_param->bEnableSAO)<br>
-        {<br>
-            // TODO: Multiple Threading<br>
-            // Delay ONE row to avoid Intra Prediction Conflict<br>
-            if (m_pool && (row >= 1))<br>
-            {<br>
-                // Waitting last threading finish<br>
-                m_frameFilter.m_parallelFilter[row - 1].waitForExit();<br>
-<br>
-                // Processing new group<br>
-                int allowCol = col;<br>
-<br>
-                // avoid race condition on last column<br>
-                if (row >= 2)<br>
-                {<br>
-                    allowCol = X265_MIN(((col == numCols - 1) ? m_frameFilter.m_parallelFilter[row - 2].m_lastDeblocked.get()<br>
-                                                              : m_frameFilter.m_parallelFilter[row - 2].m_lastCol.get()), (int)col);<br>
-                }<br>
-                m_frameFilter.m_parallelFilter[row - 1].m_allowedCol.set(allowCol);<br>
-                m_frameFilter.m_parallelFilter[row - 1].tryBondPeers(*this, 1);<br>
-            }<br>
-<br>
-            // Last Row may start early<br>
-            if (m_pool && (row == m_numRows - 1))<br>
-            {<br>
-                // Waiting for the last thread to finish<br>
-                m_frameFilter.m_parallelFilter[row].waitForExit();<br>
-<br>
-                // Deblocking last row<br>
-                int allowCol = col;<br>
-<br>
-                // avoid race condition on last column<br>
-                if (row >= 2)<br>
-                {<br>
-                    allowCol = X265_MIN(((col == numCols - 1) ? m_frameFilter.m_parallelFilter[row - 1].m_lastDeblocked.get()<br>
-                                                              : m_frameFilter.m_parallelFilter[row - 1].m_lastCol.get()), (int)col);<br>
-                }<br>
-                m_frameFilter.m_parallelFilter[row].m_allowedCol.set(allowCol);<br>
-                m_frameFilter.m_parallelFilter[row].tryBondPeers(*this, 1);<br>
-            }<br>
-        }<br>
-        // Both Loopfilter and SAO Disabled<br>
-        else<br>
-        {<br>
-            m_frameFilter.processPostCu(row, col);<br>
-        }<br>
-<br>
         if (m_param->bEnableWavefront && curRow.completed >= 2 && row < m_numRows - 1 &&<br>
             (!m_bAllRowsStop || intRow + 1 < m_vbvResetTriggerRow))<br>
         {<br>
@@ -1161,7 +1159,7 @@<br>
<br>
         ScopedLock self(curRow.lock);<br>
         if ((m_bAllRowsStop && intRow > m_vbvResetTriggerRow) ||<br>
-            (row > 0 && curRow.completed < numCols - 1 && m_rows[row - 1].completed < m_rows[row].completed + 2))<br>
+            (row > 0 && ((curRow.completed < numCols - 1) || (m_rows[row - 1].completed < numCols)) && m_rows[row - 1].completed < m_rows[row].completed + 2))<br>
         {<br>
             curRow.active = false;<br>
             curRow.busy = false;<br>
<br>
_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org" target="_blank">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
</blockquote></div><br><br clear="all"><br>-- <br><div><div dir="ltr"><div><div>Deepthi Nandakumar<br></div>Engineering Manager, x265<br></div>Multicoreware, Inc<br></div></div>
</div>
</blockquote></div></div></div><br>_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org" target="_blank">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><div dir="ltr"><div><div>Deepthi Nandakumar<br></div>Engineering Manager, x265<br></div>Multicoreware, Inc<br></div></div>
</div>
</div></div><br>_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org" target="_blank">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
<br></blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="https://mailman.videolan.org/listinfo/x265-devel" rel="noreferrer" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr"><div><div>Deepthi Nandakumar<br></div>Engineering Manager, x265<br></div>Multicoreware, Inc<br></div></div>
</div>