Re: [x265] [PATCH] framefilter: remove heap corruption in tld
sry, Thg client didnt sync the changes properly for the previous commit. here is the right one # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID a5a439242bbf367f5d76356b841cfa1ee9e119e4 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r a5a439242bbf source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -171,7 +171,7 @@ uint32_t m_checksum[3]; double m_elapsedCompressTime; // elapsed time spent in worker threads double m_frameTime; // wall time from frame start to finish - +ThreadLocalData m_tld; volatile boolm_bAllRowsStop; volatile int m_vbvResetTriggerRow; @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; diff -r a18972fd05b1 -r a5a439242bbf source/encoder/framefilter.cpp --- a/source/encoder/framefilter.cpp Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/framefilter.cpp Wed Jul 02 14:06:12 2014 +0530 @@ -124,8 +124,7 @@ void FrameFilter::processRow(int row, const int threadId) { PPAScopeEvent(Thread_filterCU); -assert(threadId = 0); -ThreadLocalData tld = Encoder::m_threadLocalData[threadId]; +ThreadLocalData tld = threadId = 0 ? Encoder::m_threadLocalData[threadId] : m_frame-m_tld; if (!m_param-bEnableLoopFilter !m_param-bEnableSAO) { On Wed, Jul 2, 2014 at 2:15 PM, aar...@multicorewareinc.com wrote: # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID 69d9bd3eb5bd015d2e0c90d51eec0d7f8a4747d0 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r 69d9bd3eb5bd source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel
Re: [x265] [PATCH] framefilter: remove heap corruption in tld
The framefilter structure needs ThreadLocalData m_tld, that has to be initialised, and then used if wpp is not enabled. Not sure what you're trying to do here? On Wed, Jul 2, 2014 at 2:20 PM, Aarthi Priya Thirumalai aar...@multicorewareinc.com wrote: sry, Thg client didnt sync the changes properly for the previous commit. here is the right one # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID a5a439242bbf367f5d76356b841cfa1ee9e119e4 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r a5a439242bbf source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -171,7 +171,7 @@ uint32_t m_checksum[3]; double m_elapsedCompressTime; // elapsed time spent in worker threads double m_frameTime; // wall time from frame start to finish - +ThreadLocalData m_tld; volatile boolm_bAllRowsStop; volatile int m_vbvResetTriggerRow; @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; diff -r a18972fd05b1 -r a5a439242bbf source/encoder/framefilter.cpp --- a/source/encoder/framefilter.cpp Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/framefilter.cpp Wed Jul 02 14:06:12 2014 +0530 @@ -124,8 +124,7 @@ void FrameFilter::processRow(int row, const int threadId) { PPAScopeEvent(Thread_filterCU); -assert(threadId = 0); -ThreadLocalData tld = Encoder::m_threadLocalData[threadId]; +ThreadLocalData tld = threadId = 0 ? Encoder::m_threadLocalData[threadId] : m_frame-m_tld; if (!m_param-bEnableLoopFilter !m_param-bEnableSAO) { On Wed, Jul 2, 2014 at 2:15 PM, aar...@multicorewareinc.com wrote: # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID 69d9bd3eb5bd015d2e0c90d51eec0d7f8a4747d0 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r 69d9bd3eb5bd source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel
Re: [x265] [PATCH] framefilter: remove heap corruption in tld
Exactly that! make FrameEncoder::m_tld a public member for framefilter to access, use it in when Wpp is disabled. On Wed, Jul 2, 2014 at 2:25 PM, Deepthi Nandakumar deep...@multicorewareinc.com wrote: The framefilter structure needs ThreadLocalData m_tld, that has to be initialised, and then used if wpp is not enabled. Not sure what you're trying to do here? On Wed, Jul 2, 2014 at 2:20 PM, Aarthi Priya Thirumalai aar...@multicorewareinc.com wrote: sry, Thg client didnt sync the changes properly for the previous commit. here is the right one # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID a5a439242bbf367f5d76356b841cfa1ee9e119e4 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r a5a439242bbf source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -171,7 +171,7 @@ uint32_t m_checksum[3]; double m_elapsedCompressTime; // elapsed time spent in worker threads double m_frameTime; // wall time from frame start to finish - +ThreadLocalData m_tld; volatile boolm_bAllRowsStop; volatile int m_vbvResetTriggerRow; @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; diff -r a18972fd05b1 -r a5a439242bbf source/encoder/framefilter.cpp --- a/source/encoder/framefilter.cpp Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/framefilter.cpp Wed Jul 02 14:06:12 2014 +0530 @@ -124,8 +124,7 @@ void FrameFilter::processRow(int row, const int threadId) { PPAScopeEvent(Thread_filterCU); -assert(threadId = 0); -ThreadLocalData tld = Encoder::m_threadLocalData[threadId]; +ThreadLocalData tld = threadId = 0 ? Encoder::m_threadLocalData[threadId] : m_frame-m_tld; if (!m_param-bEnableLoopFilter !m_param-bEnableSAO) { On Wed, Jul 2, 2014 at 2:15 PM, aar...@multicorewareinc.com wrote: # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID 69d9bd3eb5bd015d2e0c90d51eec0d7f8a4747d0 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r 69d9bd3eb5bd source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel
Re: [x265] [PATCH] framefilter: remove heap corruption in tld
Hmm, I'm wondering doesnt m_tld rightfully belong to Encoder? On Wed, Jul 2, 2014 at 3:39 PM, Aarthi Priya Thirumalai aar...@multicorewareinc.com wrote: Exactly that! make FrameEncoder::m_tld a public member for framefilter to access, use it in when Wpp is disabled. On Wed, Jul 2, 2014 at 2:25 PM, Deepthi Nandakumar deep...@multicorewareinc.com wrote: The framefilter structure needs ThreadLocalData m_tld, that has to be initialised, and then used if wpp is not enabled. Not sure what you're trying to do here? On Wed, Jul 2, 2014 at 2:20 PM, Aarthi Priya Thirumalai aar...@multicorewareinc.com wrote: sry, Thg client didnt sync the changes properly for the previous commit. here is the right one # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID a5a439242bbf367f5d76356b841cfa1ee9e119e4 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r a5a439242bbf source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -171,7 +171,7 @@ uint32_t m_checksum[3]; double m_elapsedCompressTime; // elapsed time spent in worker threads double m_frameTime; // wall time from frame start to finish - +ThreadLocalData m_tld; volatile boolm_bAllRowsStop; volatile int m_vbvResetTriggerRow; @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; diff -r a18972fd05b1 -r a5a439242bbf source/encoder/framefilter.cpp --- a/source/encoder/framefilter.cpp Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/framefilter.cpp Wed Jul 02 14:06:12 2014 +0530 @@ -124,8 +124,7 @@ void FrameFilter::processRow(int row, const int threadId) { PPAScopeEvent(Thread_filterCU); -assert(threadId = 0); -ThreadLocalData tld = Encoder::m_threadLocalData[threadId]; +ThreadLocalData tld = threadId = 0 ? Encoder::m_threadLocalData[threadId] : m_frame-m_tld; if (!m_param-bEnableLoopFilter !m_param-bEnableSAO) { On Wed, Jul 2, 2014 at 2:15 PM, aar...@multicorewareinc.com wrote: # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID 69d9bd3eb5bd015d2e0c90d51eec0d7f8a4747d0 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r 69d9bd3eb5bd source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel
Re: [x265] [PATCH] framefilter: remove heap corruption in tld
it has to be in scope of each Frame Encoder since frame can be are encoded in parallel with no wpp. On Wed, Jul 2, 2014 at 3:55 PM, Deepthi Nandakumar deep...@multicorewareinc.com wrote: Hmm, I'm wondering doesnt m_tld rightfully belong to Encoder? On Wed, Jul 2, 2014 at 3:39 PM, Aarthi Priya Thirumalai aar...@multicorewareinc.com wrote: Exactly that! make FrameEncoder::m_tld a public member for framefilter to access, use it in when Wpp is disabled. On Wed, Jul 2, 2014 at 2:25 PM, Deepthi Nandakumar deep...@multicorewareinc.com wrote: The framefilter structure needs ThreadLocalData m_tld, that has to be initialised, and then used if wpp is not enabled. Not sure what you're trying to do here? On Wed, Jul 2, 2014 at 2:20 PM, Aarthi Priya Thirumalai aar...@multicorewareinc.com wrote: sry, Thg client didnt sync the changes properly for the previous commit. here is the right one # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID a5a439242bbf367f5d76356b841cfa1ee9e119e4 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r a5a439242bbf source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -171,7 +171,7 @@ uint32_t m_checksum[3]; double m_elapsedCompressTime; // elapsed time spent in worker threads double m_frameTime; // wall time from frame start to finish - +ThreadLocalData m_tld; volatile boolm_bAllRowsStop; volatile int m_vbvResetTriggerRow; @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; diff -r a18972fd05b1 -r a5a439242bbf source/encoder/framefilter.cpp --- a/source/encoder/framefilter.cpp Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/framefilter.cpp Wed Jul 02 14:06:12 2014 +0530 @@ -124,8 +124,7 @@ void FrameFilter::processRow(int row, const int threadId) { PPAScopeEvent(Thread_filterCU); -assert(threadId = 0); -ThreadLocalData tld = Encoder::m_threadLocalData[threadId]; +ThreadLocalData tld = threadId = 0 ? Encoder::m_threadLocalData[threadId] : m_frame-m_tld; if (!m_param-bEnableLoopFilter !m_param-bEnableSAO) { On Wed, Jul 2, 2014 at 2:15 PM, aar...@multicorewareinc.com wrote: # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID 69d9bd3eb5bd015d2e0c90d51eec0d7f8a4747d0 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r 69d9bd3eb5bd source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel
Re: [x265] [PATCH] framefilter: remove heap corruption in tld
We can reuse [0] everytime, we alloc at least 1 TLD in Encoder::create() At 2014-07-02 18:09:51,Aarthi Priya Thirumalai aar...@multicorewareinc.com wrote: Exactly that! make FrameEncoder::m_tld a public member for framefilter to access, use it in when Wpp is disabled. On Wed, Jul 2, 2014 at 2:25 PM, Deepthi Nandakumar deep...@multicorewareinc.com wrote: The framefilter structure needs ThreadLocalData m_tld, that has to be initialised, and then used if wpp is not enabled. Not sure what you're trying to do here? On Wed, Jul 2, 2014 at 2:20 PM, Aarthi Priya Thirumalai aar...@multicorewareinc.com wrote: sry, Thg client didnt sync the changes properly for the previous commit. here is the right one # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID a5a439242bbf367f5d76356b841cfa1ee9e119e4 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r a5a439242bbf source/encoder/frameencoder.h --- a/source/encoder/frameencoder.hTue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.hWed Jul 02 14:06:12 2014 +0530 @@ -171,7 +171,7 @@ uint32_t m_checksum[3]; double m_elapsedCompressTime; // elapsed time spent in worker threads double m_frameTime; // wall time from frame start to finish - +ThreadLocalData m_tld; volatile boolm_bAllRowsStop; volatile int m_vbvResetTriggerRow; @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; diff -r a18972fd05b1 -r a5a439242bbf source/encoder/framefilter.cpp --- a/source/encoder/framefilter.cppTue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/framefilter.cppWed Jul 02 14:06:12 2014 +0530 @@ -124,8 +124,7 @@ void FrameFilter::processRow(int row, const int threadId) { PPAScopeEvent(Thread_filterCU); -assert(threadId = 0); -ThreadLocalData tld = Encoder::m_threadLocalData[threadId]; +ThreadLocalData tld = threadId = 0 ? Encoder::m_threadLocalData[threadId] : m_frame-m_tld; if (!m_param-bEnableLoopFilter !m_param-bEnableSAO) { On Wed, Jul 2, 2014 at 2:15 PM, aar...@multicorewareinc.com wrote: # HG changeset patch # User Aarthi Thirumalai # Date 1404290172 -19800 # Wed Jul 02 14:06:12 2014 +0530 # Node ID 69d9bd3eb5bd015d2e0c90d51eec0d7f8a4747d0 # Parent a18972fd05b1d6242a881bef979b9e1ff17543d9 framefilter: remove heap corruption in tld diff -r a18972fd05b1 -r 69d9bd3eb5bd source/encoder/frameencoder.h --- a/source/encoder/frameencoder.h Tue Jul 01 14:58:35 2014 -0500 +++ b/source/encoder/frameencoder.h Wed Jul 02 14:06:12 2014 +0530 @@ -191,7 +191,6 @@ Bitstream* m_outStreams; NoiseReduction m_nr; NALList m_nalList; -ThreadLocalData m_tld; Frame* m_frame; ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel
Re: [x265] [PATCH] framefilter: remove heap corruption in tld
On Wed, Jul 2, 2014 at 10:35 AM, chen chenm...@163.com wrote: We can reuse [0] everytime, we alloc at least 1 TLD in Encoder::create() Not if another frame encoder or frame filter can be using it at the same time; and definitely not if the Encoder's TLD array is static. This is why the FrameEncoder has it's own m_tld instance, for --no-wpp operation. There is a 1::1 correlation between FrameEncoder and FrameFilter, so it would be appropriate for the FrameFilter to use the FrameEncoder's reserved m_tld if the FrameFilter functions are not using a worker thread. -- Steve Borho ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel
Re: [x265] [PATCH] framefilter: remove heap corruption in tld
At 2014-07-02 23:54:17,Steve Borho st...@borho.org wrote: On Wed, Jul 2, 2014 at 10:35 AM, chen chenm...@163.com wrote: We can reuse [0] everytime, we alloc at least 1 TLD in Encoder::create() Not if another frame encoder or frame filter can be using it at the same time; and definitely not if the Encoder's TLD array is static. This is why the FrameEncoder has it's own m_tld instance, for --no-wpp operation. There is a 1::1 correlation between FrameEncoder and FrameFilter, so it would be appropriate for the FrameFilter to use the FrameEncoder's reserved m_tld if the FrameFilter functions are not using a worker thread. -- Steve Borho ___ you are right, it is my fault. I see the code below, I assume we just one thread in pool, but I forgot FrameParallelism have another thread, please ignore my patch before. // Trim the thread pool if WPP is disabled if (!p-bEnableWavefront) p-poolNumThreads = 1; ___ x265-devel mailing list x265-devel@videolan.org https://mailman.videolan.org/listinfo/x265-devel