[x265] [PATCH] fix deadlock and output change on new ParallelFilter framework. (Issue #225)
Deepthi Nandakumar
deepthi at multicorewareinc.com
Thu Jan 14 09:57:59 CET 2016
Yes, need to handle the processing functions from FrameFilter (instead of
ParallelFilter)
On Thu, Jan 14, 2016 at 2:22 PM, Ashok Kumar Mishra <
ashok at multicorewareinc.com> wrote:
> I think FrameFilter should keep the instance of ParallelFilter, which has
> already there in code.
>
> ParallelFilter* m_parallelFilter;
>
> So in that case we don't need nested class.
>
> Thanks
> Ashok.
>
> On Thu, Jan 14, 2016 at 1:31 PM, Deepthi Nandakumar <
> deepthi at multicorewareinc.com> wrote:
>
>> Min,
>>
>> Sure - what I mean is, can we have a class that looks like:
>>
>> class FrameFilter
>> {
>> public:
>>
>> x265_param* m_param;
>> Frame* m_frame;
>> FrameEncoder* m_frameEncoder;
>> FrameData* m_encData;
>> int m_hChromaShift;
>> int m_vChromaShift;
>> int m_pad[2];
>>
>> uint32_t m_numRows;
>> uint32_t m_numCols;
>> uint32_t m_lastWidth;
>> .................
>> ..................
>> }
>>
>> class ParallelFilter
>> {
>> public:
>> uint32_t m_rowHeight;
>> uint32_t m_row;
>> uint32_t m_rowAddr;
>>
>> ParallelFilter* m_prevRow;
>> SAO m_sao;
>> ThreadSafeInteger m_lastCol; /* The column that next to
>> process */
>> ThreadSafeInteger m_allowedCol; /* The column that processed
>> from Encode pipeline */
>> ThreadSafeInteger m_lastDeblocked; /* The column that finished
>> all of Deblock stages */
>>
>> }
>>
>> The common variables are in FrameFilter, and ParallelFilter contains only
>> the row specific data.
>>
>> On Wed, Jan 13, 2016 at 12:15 AM, chen <chenm003 at 163.com> wrote:
>>
>>> I fixed the crash and modify/remove FrameFilter::processPostCu, I
>>> spending ~64 bytes every row.
>>>
>>> In my design, FrameFilter just one instance every frame and
>>> ParallelFilter instance for every row.
>>>
>>>
>>> At 2016-01-12 13:37:36,"Deepthi Nandakumar" <
>>> deepthi at multicorewareinc.com> wrote:
>>>
>>> Min, thanks. Queuing this.
>>>
>>> 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).
>>>
>>>
>>> On Tue, Jan 12, 2016 at 2:36 AM, Min Chen <chenm003 at 163.com> wrote:
>>>
>>>> # HG changeset patch
>>>> # User Min Chen <chenm003 at 163.com>
>>>> # Date 1452545402 21600
>>>> # Node ID 1c273c83a4478ed39b0e6734eec1ba1cfccd07d6
>>>> # Parent 19cfada7162147f293e37302e4c7f9c1760928a0
>>>> fix deadlock and output change on new ParallelFilter framework. (Issue
>>>> #225)
>>>> The bug from two part:
>>>> 1. we old sync system allow latest column execute parallelism
>>>> 2. new ParallelFilter logic will distribute threads and waitting the
>>>> status change, there have a race conditions
>>>> ---
>>>> source/encoder/frameencoder.cpp | 108
>>>> +++++++++++++++++++--------------------
>>>> 1 files changed, 53 insertions(+), 55 deletions(-)
>>>>
>>>> diff -r 19cfada71621 -r 1c273c83a447 source/encoder/frameencoder.cpp
>>>> --- a/source/encoder/frameencoder.cpp Mon Jan 11 16:28:39 2016 +0530
>>>> +++ b/source/encoder/frameencoder.cpp Mon Jan 11 14:50:02 2016 -0600
>>>> @@ -962,6 +962,58 @@
>>>> // Save CABAC state for next row
>>>> curRow.bufferedEntropy.loadContexts(rowCoder);
>>>>
>>>> + /* SAO parameter estimation using non-deblocked pixels for CTU
>>>> bottom and right boundary areas */
>>>> + if (m_param->bEnableSAO && m_param->bSaoNonDeblocked)
>>>> +
>>>> m_frameFilter.m_parallelFilter[row].m_sao.calcSaoStatsCu_BeforeDblk(m_frame,
>>>> col, row);
>>>> +
>>>> + /* Deblock with idle threading */
>>>> + if (m_param->bEnableLoopFilter | m_param->bEnableSAO)
>>>> + {
>>>> + // TODO: Multiple Threading
>>>> + // Delay ONE row to avoid Intra Prediction Conflict
>>>> + if (m_pool && (row >= 1))
>>>> + {
>>>> + // Waitting last threading finish
>>>> + m_frameFilter.m_parallelFilter[row - 1].waitForExit();
>>>> +
>>>> + // Processing new group
>>>> + int allowCol = col;
>>>> +
>>>> + // avoid race condition on last column
>>>> + if (row >= 2)
>>>> + {
>>>> + allowCol = X265_MIN(((col == numCols - 1) ?
>>>> m_frameFilter.m_parallelFilter[row - 2].m_lastDeblocked.get()
>>>> + :
>>>> m_frameFilter.m_parallelFilter[row - 2].m_lastCol.get()), (int)col);
>>>> + }
>>>> + m_frameFilter.m_parallelFilter[row -
>>>> 1].m_allowedCol.set(allowCol);
>>>> + m_frameFilter.m_parallelFilter[row -
>>>> 1].tryBondPeers(*this, 1);
>>>> + }
>>>> +
>>>> + // Last Row may start early
>>>> + if (m_pool && (row == m_numRows - 1))
>>>> + {
>>>> + // Waiting for the last thread to finish
>>>> + m_frameFilter.m_parallelFilter[row].waitForExit();
>>>> +
>>>> + // Deblocking last row
>>>> + int allowCol = col;
>>>> +
>>>> + // avoid race condition on last column
>>>> + if (row >= 2)
>>>> + {
>>>> + allowCol = X265_MIN(((col == numCols - 1) ?
>>>> m_frameFilter.m_parallelFilter[row - 1].m_lastDeblocked.get()
>>>> + :
>>>> m_frameFilter.m_parallelFilter[row - 1].m_lastCol.get()), (int)col);
>>>> + }
>>>> +
>>>> m_frameFilter.m_parallelFilter[row].m_allowedCol.set(allowCol);
>>>> +
>>>> m_frameFilter.m_parallelFilter[row].tryBondPeers(*this, 1);
>>>> + }
>>>> + }
>>>> + // Both Loopfilter and SAO Disabled
>>>> + else
>>>> + {
>>>> + m_frameFilter.processPostCu(row, col);
>>>> + }
>>>> +
>>>> // Completed CU processing
>>>> curRow.completed++;
>>>>
>>>> @@ -1091,60 +1143,6 @@
>>>> }
>>>> }
>>>>
>>>> - // TODO: move Deblock and SAO to before VBV check
>>>> -
>>>> - /* SAO parameter estimation using non-deblocked pixels for CTU
>>>> bottom and right boundary areas */
>>>> - if (m_param->bEnableSAO && m_param->bSaoNonDeblocked)
>>>> -
>>>> m_frameFilter.m_parallelFilter[row].m_sao.calcSaoStatsCu_BeforeDblk(m_frame,
>>>> col, row);
>>>> -
>>>> - /* Deblock with idle threading */
>>>> - if (m_param->bEnableLoopFilter | m_param->bEnableSAO)
>>>> - {
>>>> - // TODO: Multiple Threading
>>>> - // Delay ONE row to avoid Intra Prediction Conflict
>>>> - if (m_pool && (row >= 1))
>>>> - {
>>>> - // Waitting last threading finish
>>>> - m_frameFilter.m_parallelFilter[row - 1].waitForExit();
>>>> -
>>>> - // Processing new group
>>>> - int allowCol = col;
>>>> -
>>>> - // avoid race condition on last column
>>>> - if (row >= 2)
>>>> - {
>>>> - allowCol = X265_MIN(((col == numCols - 1) ?
>>>> m_frameFilter.m_parallelFilter[row - 2].m_lastDeblocked.get()
>>>> - :
>>>> m_frameFilter.m_parallelFilter[row - 2].m_lastCol.get()), (int)col);
>>>> - }
>>>> - m_frameFilter.m_parallelFilter[row -
>>>> 1].m_allowedCol.set(allowCol);
>>>> - m_frameFilter.m_parallelFilter[row -
>>>> 1].tryBondPeers(*this, 1);
>>>> - }
>>>> -
>>>> - // Last Row may start early
>>>> - if (m_pool && (row == m_numRows - 1))
>>>> - {
>>>> - // Waiting for the last thread to finish
>>>> - m_frameFilter.m_parallelFilter[row].waitForExit();
>>>> -
>>>> - // Deblocking last row
>>>> - int allowCol = col;
>>>> -
>>>> - // avoid race condition on last column
>>>> - if (row >= 2)
>>>> - {
>>>> - allowCol = X265_MIN(((col == numCols - 1) ?
>>>> m_frameFilter.m_parallelFilter[row - 1].m_lastDeblocked.get()
>>>> - :
>>>> m_frameFilter.m_parallelFilter[row - 1].m_lastCol.get()), (int)col);
>>>> - }
>>>> -
>>>> m_frameFilter.m_parallelFilter[row].m_allowedCol.set(allowCol);
>>>> -
>>>> m_frameFilter.m_parallelFilter[row].tryBondPeers(*this, 1);
>>>> - }
>>>> - }
>>>> - // Both Loopfilter and SAO Disabled
>>>> - else
>>>> - {
>>>> - m_frameFilter.processPostCu(row, col);
>>>> - }
>>>> -
>>>> if (m_param->bEnableWavefront && curRow.completed >= 2 && row
>>>> < m_numRows - 1 &&
>>>> (!m_bAllRowsStop || intRow + 1 < m_vbvResetTriggerRow))
>>>> {
>>>> @@ -1161,7 +1159,7 @@
>>>>
>>>> ScopedLock self(curRow.lock);
>>>> if ((m_bAllRowsStop && intRow > m_vbvResetTriggerRow) ||
>>>> - (row > 0 && curRow.completed < numCols - 1 && m_rows[row -
>>>> 1].completed < m_rows[row].completed + 2))
>>>> + (row > 0 && ((curRow.completed < numCols - 1) ||
>>>> (m_rows[row - 1].completed < numCols)) && m_rows[row - 1].completed <
>>>> m_rows[row].completed + 2))
>>>> {
>>>> curRow.active = false;
>>>> curRow.busy = false;
>>>>
>>>> _______________________________________________
>>>> x265-devel mailing list
>>>> x265-devel at videolan.org
>>>> https://mailman.videolan.org/listinfo/x265-devel
>>>>
>>>
>>>
>>>
>>> --
>>> Deepthi Nandakumar
>>> Engineering Manager, x265
>>> Multicoreware, Inc
>>>
>>>
>>> _______________________________________________
>>> x265-devel mailing list
>>> x265-devel at videolan.org
>>> https://mailman.videolan.org/listinfo/x265-devel
>>>
>>>
>>
>>
>> --
>> Deepthi Nandakumar
>> Engineering Manager, x265
>> Multicoreware, Inc
>>
>> _______________________________________________
>> x265-devel mailing list
>> x265-devel at videolan.org
>> https://mailman.videolan.org/listinfo/x265-devel
>>
>>
>
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>
>
--
Deepthi Nandakumar
Engineering Manager, x265
Multicoreware, Inc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20160114/973a3a32/attachment-0001.html>
More information about the x265-devel
mailing list