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