Re: [x265] [PATCH 2 of 2 V2] framefilter: comment cleanups, use pixel data type

2013-09-16 Thread Steve Borho
On Sun, Sep 15, 2013 at 11:06 PM, Deepthi Nandakumar 
deep...@multicorewareinc.com wrote:




 On Fri, Sep 13, 2013 at 11:11 PM, Steve Borho st...@borho.org wrote:

 # HG changeset patch
 # User Steve Borho st...@borho.org
 # Date 1379053732 18000
 #  Fri Sep 13 01:28:52 2013 -0500
 # Node ID b8bb66cd21bcab6505b7fe321e95875861c84bda
 # Parent  2614338b90d3533c2760a94fa10ffb5dee57910c
 framefilter: comment cleanups, use pixel data type

 diff -r 2614338b90d3 -r b8bb66cd21bc source/encoder/frameencoder.cpp
 --- a/source/encoder/frameencoder.cpp   Fri Sep 13 10:55:03 2013 -0500
 +++ b/source/encoder/frameencoder.cpp   Fri Sep 13 01:28:52 2013 -0500
 @@ -937,14 +937,13 @@
  return;
  }
  }
 +// this row of CTUs has been encoded

 -// Active Loopfilter
 +// Run row-wise loop filters
  if (row = m_filterRowDelay)
  {
  m_frameFilter.processRow(row - m_filterRowDelay);
  }
 -
 -// this row of CTUs has been encoded
  if (row == m_numRows - 1)
  {
  for(int i = m_numRows - m_filterRowDelay; i  m_numRows; i++)
 diff -r 2614338b90d3 -r b8bb66cd21bc source/encoder/framefilter.cpp
 --- a/source/encoder/framefilter.cppFri Sep 13 10:55:03 2013 -0500
 +++ b/source/encoder/framefilter.cppFri Sep 13 01:28:52 2013 -0500
 @@ -170,7 +170,7 @@
  m_sao.processSaoUnitRow(saoParam-saoLcuParam[2], row -
 1, 2);
  }

 -// TODO: this code is NOT VERIFY because TransformSkip and
 PCM mode have some bugs, they always not active!
 +// TODO: this code is NOT VERIFIED because TransformSkip and
 PCM modes have some bugs, they are never enabled
  bool  bPCMFilter = (m_pic-getSlice()-getSPS()-getUsePCM()
  m_pic-getSlice()-getSPS()-getPCMFilterDisableFlag()) ? true : false;
  if (bPCMFilter ||
 m_pic-getSlice()-getPPS()-getTransquantBypassEnableFlag())
  {
 @@ -187,8 +187,6 @@

  // this row of CTUs has been encoded

 -// TODO: extend margins for motion reference
 -
  if (row  0)
  {
  processRowPost(row - 1);
 @@ -209,7 +207,7 @@
  m_sao.processSaoUnitRow(saoParam-saoLcuParam[2], row, 2);
  }

 -// TODO: this code is NOT VERIFY because TransformSkip and PCM
 mode have some bugs, they always not active!
 +// TODO: this code is NOT VERIFIED because TransformSkip and PCM
 modes have some bugs, they are never enabled
  bool  bPCMFilter = (m_pic-getSlice()-getSPS()-getUsePCM() 
 m_pic-getSlice()-getSPS()-getPCMFilterDisableFlag()) ? true : false;
  if (bPCMFilter ||
 m_pic-getSlice()-getPPS()-getTransquantBypassEnableFlag())
  {
 @@ -234,10 +232,6 @@
  const int lastH = ((recon-getHeight() % g_maxCUHeight) ?
 (recon-getHeight() % g_maxCUHeight) : g_maxCUHeight);
  const int realH = (row != m_numRows - 1) ? g_maxCUHeight : lastH;

 -// TODO: Remove when we confirm below code is right
 -//recon-xExtendPicCompBorder(recon-getLumaAddr(),
 recon-getStride(), recon-getWidth(), recon-getHeight(),
 recon-m_lumaMarginX, recon-m_lumaMarginY);
 -//recon-xExtendPicCompBorder(recon-getCbAddr(),
 recon-getCStride(), recon-getWidth()  1, recon-getHeight()  1,
 recon-m_chromaMarginX, recon-m_chromaMarginY);
 -//recon-xExtendPicCompBorder(recon-getCrAddr(),
 recon-getCStride(), recon-getWidth()  1, recon-getHeight()  1,
 recon-m_chromaMarginX, recon-m_chromaMarginY);
  // Border extend Left and Right
  primitives.extendRowBorder(recon-getLumaAddr(lineStartCUAddr),
 recon-getStride(), recon-getWidth(), realH, recon-getLumaMarginX());
  primitives.extendRowBorder(recon-getCbAddr(lineStartCUAddr),
 recon-getCStride(), recon-getWidth()  1, realH  1,
 recon-getChromaMarginX());
 @@ -248,9 +242,9 @@
  {
  const intptr_t stride = recon-getStride();
  const intptr_t strideC = recon-getCStride();
 -Pel *pixY = recon-getLumaAddr(lineStartCUAddr) -
 recon-getLumaMarginX();
 -Pel *pixU = recon-getCbAddr(lineStartCUAddr) -
 recon-getChromaMarginX();
 -Pel *pixV = recon-getCrAddr(lineStartCUAddr) -
 recon-getChromaMarginX();
 +pixel *pixY = recon-getLumaAddr(lineStartCUAddr) -
 recon-getLumaMarginX();
 +pixel *pixU = recon-getCbAddr(lineStartCUAddr) -
 recon-getChromaMarginX();
 +pixel *pixV = recon-getCrAddr(lineStartCUAddr) -
 recon-getChromaMarginX();


 Not sure why Pel has been changed to pixel (dropping 16-bit support
 altogether ?) since getLuma/Cb/CrAddr still return Pel.


For quite a while now Pel and pixel are always the same data type, We've
been trying to use only pixel in the code that we add to the repo.  It
might be easier at this point just to search/replace all Pel use into pixel.


 Anyways, the sizeof operator in the following memcpy's should also be
 changed to pixel.


good catch.

--
Steve
___
x265-devel mailing list
x265-devel@videolan.org

[x265] [PATCH] X265: header guards format Changed to X265_FILENAME_H

2013-09-16 Thread Gopu Govindaswamy
# HG changeset patch
# User Gopu Govindaswamy g...@multicorewareinc.com
# Date 1379314396 -19800
# Node ID d09f36e4dc8b998a2ca42694fd1ce5cf04421eee
# Parent  6bab41a554b36133865fe3378964cb9e76c24ebd
X265: header guards format Changed to X265_FILENAME_H

Globally all the x265 header files header guards format changed into 
X265_FILENAME_H

diff -r 6bab41a554b3 -r d09f36e4dc8b source/Lib/TLibCommon/AccessUnit.h
--- a/source/Lib/TLibCommon/AccessUnit.hFri Sep 13 17:24:05 2013 +0530
+++ b/source/Lib/TLibCommon/AccessUnit.hMon Sep 16 12:23:16 2013 +0530
@@ -36,8 +36,8 @@
  \briefAccess Unit class (header)
  */
 
-#ifndef _ACCESS_UNIT_
-#define _ACCESS_UNIT_ 1
+#ifndef X265_ACCESSUNIT_H
+#define X265_ACCESSUNIT_H
 
 #include NAL.h
 #include list
@@ -74,4 +74,4 @@
 
 //! \}
 
-#endif // ifndef _ACCESS_UNIT_
+#endif // ifndef X265_ACCESSUNIT_H
diff -r 6bab41a554b3 -r d09f36e4dc8b source/Lib/TLibCommon/CommonDef.h
--- a/source/Lib/TLibCommon/CommonDef.h Fri Sep 13 17:24:05 2013 +0530
+++ b/source/Lib/TLibCommon/CommonDef.h Mon Sep 16 12:23:16 2013 +0530
@@ -35,8 +35,8 @@
 \briefDefines constants, macros and tool parameters
 */
 
-#ifndef __COMMONDEF__
-#define __COMMONDEF__
+#ifndef X265_COMMONDEF_H
+#define X265_COMMONDEF_H
 
 #include algorithm
 #include cstdlib
@@ -172,4 +172,4 @@
 
 //! \}
 
-#endif // end of #ifndef  __COMMONDEF__
+#endif // ifndef X265_COMMONDEF_H
diff -r 6bab41a554b3 -r d09f36e4dc8b source/Lib/TLibCommon/ContextModel.h
--- a/source/Lib/TLibCommon/ContextModel.h  Fri Sep 13 17:24:05 2013 +0530
+++ b/source/Lib/TLibCommon/ContextModel.h  Mon Sep 16 12:23:16 2013 +0530
@@ -35,8 +35,8 @@
 \briefcontext model class (header)
 */
 
-#ifndef __CONTEXT_MODEL__
-#define __CONTEXT_MODEL__
+#ifndef X265_CONTEXTMODEL_H
+#define X265_CONTEXTMODEL_H
 
 #include CommonDef.h
 
@@ -104,4 +104,4 @@
 }
 //! \}
 
-#endif // ifndef __CONTEXT_MODEL__
+#endif // ifndef X265_CONTEXTMODEL_H
diff -r 6bab41a554b3 -r d09f36e4dc8b 
source/Lib/TLibCommon/ContextModel3DBuffer.h
--- a/source/Lib/TLibCommon/ContextModel3DBuffer.h  Fri Sep 13 17:24:05 
2013 +0530
+++ b/source/Lib/TLibCommon/ContextModel3DBuffer.h  Mon Sep 16 12:23:16 
2013 +0530
@@ -35,8 +35,8 @@
 \briefcontext model 3D buffer class (header)
 */
 
-#ifndef _HM_CONTEXT_MODEL_3DBUFFER_H_
-#define _HM_CONTEXT_MODEL_3DBUFFER_H_
+#ifndef X265_CONTEXTMODEL3DBUFFER_H
+#define X265_CONTEXTMODEL3DBUFFER_H
 
 #include stdio.h
 #include assert.h
@@ -106,4 +106,4 @@
 }
 //! \}
 
-#endif // _HM_CONTEXT_MODEL_3DBUFFER_H_
+#endif // ifndef X265_CONTEXTMODEL3DBUFFER_H
diff -r 6bab41a554b3 -r d09f36e4dc8b source/Lib/TLibCommon/ContextTables.h
--- a/source/Lib/TLibCommon/ContextTables.h Fri Sep 13 17:24:05 2013 +0530
+++ b/source/Lib/TLibCommon/ContextTables.h Mon Sep 16 12:23:16 2013 +0530
@@ -36,8 +36,8 @@
 \todo number of context models is not matched to actual use, should be 
fixed
 */
 
-#ifndef __CONTEXTTABLES__
-#define __CONTEXTTABLES__
+#ifndef X265_CONTEXTTABLES_H
+#define X265_CONTEXTTABLES_H
 
 //! \ingroup TLibCommon
 //! \{
@@ -320,4 +320,4 @@
 }
 //! \}
 
-#endif // ifndef __CONTEXTTABLES__
+#endif // ifndef X265_CONTEXTTABLES_H
diff -r 6bab41a554b3 -r d09f36e4dc8b source/Lib/TLibCommon/NAL.h
--- a/source/Lib/TLibCommon/NAL.h   Fri Sep 13 17:24:05 2013 +0530
+++ b/source/Lib/TLibCommon/NAL.h   Mon Sep 16 12:23:16 2013 +0530
@@ -31,8 +31,8 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef _NAL_
-#define _NAL_ 1
+#ifndef X265_NAL_H
+#define X265_NAL_H
 
 #include CommonDef.h
 #include x265.h
@@ -118,4 +118,4 @@
 }
 //! \}
 
-#endif // ifndef _NAL_
+#endif // ifndef X265_NAL_H
diff -r 6bab41a554b3 -r d09f36e4dc8b source/Lib/TLibCommon/SEI.h
--- a/source/Lib/TLibCommon/SEI.h   Fri Sep 13 17:24:05 2013 +0530
+++ b/source/Lib/TLibCommon/SEI.h   Mon Sep 16 12:23:16 2013 +0530
@@ -31,8 +31,8 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef _SEI_
-#define _SEI_ 1
+#ifndef X265_SEI_H
+#define X265_SEI_H
 
 #include list
 #include vector
@@ -309,4 +309,4 @@
 }
 //! \}
 
-#endif // ifndef _SEI_
+#endif // ifndef X265_SEI_H
diff -r 6bab41a554b3 -r d09f36e4dc8b source/Lib/TLibCommon/TComBitCounter.h
--- a/source/Lib/TLibCommon/TComBitCounter.hFri Sep 13 17:24:05 2013 +0530
+++ b/source/Lib/TLibCommon/TComBitCounter.hMon Sep 16 12:23:16 2013 +0530
@@ -35,8 +35,8 @@
 \briefClass for counting bits (header)
 */
 
-#ifndef __COMBITCOUNTER__
-#define __COMBITCOUNTER__
+#ifndef X265_COMBITCOUNTER_H
+#define X265_COMBITCOUNTER_H
 
 #include TComBitStream.h
 
@@ -72,4 +72,4 @@
 }
 //! \}
 
-#endif // ifndef __COMBITCOUNTER__
+#endif // ifndef X265_COMBITCOUNTER_H
diff -r 6bab41a554b3 -r d09f36e4dc8b source/Lib/TLibCommon/TComBitStream.h
--- a/source/Lib/TLibCommon/TComBitStream.h Fri Sep 13 17:24:05 2013 +0530
+++ b/source/Lib/TLibCommon/TComBitStream.h Mon Sep 16 12:23:16 2013 +0530
@@ -35,8 +35,8 @@
 \briefclass for handling bitstream (header)
 */
 

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

2013-09-16 Thread Min Chen
# HG changeset patch
# User Min Chen chenm...@163.com
# Date 1379321243 -28800
# Node ID 5ff15bbb2bda4fedca72c9093374de6dd8c262b3
# Parent  6bab41a554b36133865fe3378964cb9e76c24ebd
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 6bab41a554b3 -r 5ff15bbb2bda source/encoder/frameencoder.cpp
--- a/source/encoder/frameencoder.cpp   Fri Sep 13 17:24:05 2013 +0530
+++ b/source/encoder/frameencoder.cpp   Mon Sep 16 16:47:23 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,7 +884,7 @@
 }
 else
 {
-for (int i = 0; i  this-m_numRows; i++)
+for (int i = 0; i  this-m_numRows * 2; i++)
 {
 // block until all reference frames have reconstructed the rows we 
need
 for (int l = 0; l  numPredDir; l++)
@@ -904,10 +905,12 @@
 }
 }
 
-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 +944,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 +960,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 6bab41a554b3 -r 5ff15bbb2bda source/encoder/frameencoder.h
--- a/source/encoder/frameencoder.h Fri Sep 13 17:24:05 2013 +0530
+++ b/source/encoder/frameencoder.h Mon Sep 16 16:47:23 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 6bab41a554b3 -r 5ff15bbb2bda source/encoder/framefilter.cpp
--- a/source/encoder/framefilter.cppFri Sep 13 17:24:05 2013 +0530
+++ b/source/encoder/framefilter.cppMon Sep 16 16:47:23 2013 +0800
@@ -102,8 +102,6 @@
 {
 

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

2013-09-16 Thread chen
Excuse me, this is wrong patch, I will send a new one soon.

At 2013-09-16 16:51:58,Min Chen chenm...@163.com wrote:
# HG changeset patch
# User Min Chen chenm...@163.com
# Date 1379321243 -28800
# Node ID 5ff15bbb2bda4fedca72c9093374de6dd8c262b3
# Parent  6bab41a554b36133865fe3378964cb9e76c24ebd
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 6bab41a554b3 -r 5ff15bbb2bda source/encoder/frameencoder.cpp
--- a/source/encoder/frameencoder.cpp Fri Sep 13 17:24:05 2013 +0530
+++ b/source/encoder/frameencoder.cpp Mon Sep 16 16:47:23 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,7 +884,7 @@
 }
 else
 {
-for (int i = 0; i  this-m_numRows; i++)
+for (int i = 0; i  this-m_numRows * 2; i++)
 {
 // block until all reference frames have reconstructed the rows 
 we need
 for (int l = 0; l  numPredDir; l++)
@@ -904,10 +905,12 @@
 }
 }
 
-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 +944,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 +960,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 6bab41a554b3 -r 5ff15bbb2bda source/encoder/frameencoder.h
--- a/source/encoder/frameencoder.h Fri Sep 13 17:24:05 2013 +0530
+++ b/source/encoder/frameencoder.h Mon Sep 16 16:47:23 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 6bab41a554b3 -r 5ff15bbb2bda source/encoder/framefilter.cpp
--- a/source/encoder/framefilter.cpp Fri Sep 13 17:24:05 2013 +0530
+++ 

Re: [x265] [PATCH]RDLevel: Disable RDOQTS when RDO and/or TS are disabled.

2013-09-16 Thread Derek Buitenhuis
On Mon, Sep 16, 2013 at 7:11 AM, Deepthi Nandakumar
deep...@multicorewareinc.com wrote:
 # HG changeset patch
 # User Deepthi Nandakumar deep...@multicorewareinc.com
 # Date 1379311811 -19800
 # Node ID c1ec5b8a6752529368753c110d626f1c53d90f3e
 # Parent  22c4e67c7246cd77968a86e967377e4a18b47b31
 RDLevel: Disable RDOQTS when RDO and/or TS are disabled.

[...]

 +else
 +{
 +_param-bEnableRDOQTS = 0;
 +}

Is the param structure not zero-allocated with something like calloc
or malloc+memset?

- Derek
___
x265-devel mailing list
x265-devel@videolan.org
https://mailman.videolan.org/listinfo/x265-devel


Re: [x265] [PATCH]RDLevel: Disable RDOQTS when RDO and/or TS are disabled.

2013-09-16 Thread Derek Buitenhuis
On Mon, Sep 16, 2013 at 10:47 AM, Deepthi Nandakumar
deep...@multicorewareinc.com wrote:

 Yes, this particular param flag is initialised to 1 (highest quality
 setting) in x265_param_default. I'm setting it to zero for a certain set of
 user defined parameters.

What's the point of the first check
(http://mailman.videolan.org/pipermail/x265-devel/2013-September/000783.html)
which sets it to 1 then?

- Derek
___
x265-devel mailing list
x265-devel@videolan.org
https://mailman.videolan.org/listinfo/x265-devel


[x265] [PATCH] List: std::list Implementation

2013-09-16 Thread Gopu Govindaswamy
# HG changeset patch
# User Gopu Govindaswamy g...@multicorewareinc.com
# Date 1379327475 -19800
# Node ID e499466c7c6591345af2a625da12185c7735347b
# Parent  6bab41a554b36133865fe3378964cb9e76c24ebd
List: std::list Implementation

To remove the std::list dependency from X265,  and this class can be enhanced 
based on the types of std::list API's used in current x265

diff -r 6bab41a554b3 -r e499466c7c65 source/common/CMakeLists.txt
--- a/source/common/CMakeLists.txt  Fri Sep 13 17:24:05 2013 +0530
+++ b/source/common/CMakeLists.txt  Mon Sep 16 16:01:15 2013 +0530
@@ -16,8 +16,10 @@
 
 file(GLOB LIBCOMMON_HDR ../Lib/TLibCommon/*.h)
 file(GLOB LIBCOMMON_SRC ../Lib/TLibCommon/*.cpp)
+file(GLOB LIBUTIL ../util/*.cpp ../util/*.h)
 source_group(TLibCommon FILES ${LIBCOMMON_SRC})
 source_group(TLibCommonH FILES ${LIBCOMMON_HDR})
+source_group(Util FILES ${LIBUTIL})
 if(GCC)
 set_source_files_properties(${LIBCOMMON_SRC} PROPERTIES COMPILE_FLAGS 
 -Wno-sign-compare)
@@ -38,7 +40,7 @@
 endif(MSVC)
 
 add_library(common STATIC ../../COPYING
-${LIBCOMMON_SRC} ${LIBCOMMON_HDR}
+${LIBCOMMON_SRC} ${LIBCOMMON_HDR} ${LIBUTIL}
 primitives.cpp primitives.h
 pixel.cpp dct.cpp ipfilter.cpp intrapred.cpp
 ../VectorClass/instrset_detect.cpp
diff -r 6bab41a554b3 -r e499466c7c65 source/util/list.h
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ b/source/util/list.hMon Sep 16 16:01:15 2013 +0530
@@ -0,0 +1,158 @@
+/*
+ * Copyright (C) 2013 x265 project
+ *
+ * Authors: Gopu Govindaswamy g...@multicorewareinc.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02111, USA.
+ *
+ * This program is also available under a commercial proprietary license.
+ * For more information, contact us at licens...@multicorewareinc.com.
+ */
+
+#ifndef X265_LIST_H
+#define X265_LIST_H
+
+#include common.h
+
+// Short Notes: 
+// Under development
+// this class is used to remove the std::list dependency from x265, 
+// Providing Minimum std::list API implementation
+// this class can be enhanced based on types of std::list API's used in 
current x265
+
+templateclass T
+struct List
+{
+private:
+
+struct Node
+{
+T object;
+Node* next;
+};
+
+public:
+
+Node* head;
+Node* tail;
+int size;
+
+List() { head = 0; size = 0; }
+
+inline T* begin() { return head; }
+
+inline T* end() { return tail; }
+
+inline bool isEmpty()
+{
+if (head == NULL)
+return true;
+else
+return false;
+}
+
+inline void push_back(T value)
+{
+if (head == NULL)
+{
+head = (Node*)X265_MALLOC(Node, sizeof(Node));
+head-object = value;
+head-next = NULL;
+tail = head;
+}
+else
+{
+Node *nodePtr = tail;
+nodePtr-next = (Node*)X265_MALLOC(Node, sizeof(Node));
+nodePtr-next-object = value;
+nodePtr-next-next = NULL;
+tail = nodePtr-next;
+}
+size += 1;
+}
+
+inline void push_front(T value)
+{
+if (head == NULL)
+{
+head = (Node*)X265_MALLOC(Node, sizeof(Node));
+head-object = value;
+head-next = NULL;
+tail = head;
+}
+else
+{
+Node *front;
+front = (Node*)X265_MALLOC(Node, sizeof(Node));
+front-object = value;
+front-next = head;
+head = front;
+}
+size += 1;
+}
+
+inline void pop_front()
+{
+if (head != NULL)
+{
+if (head-next == NULL)
+X265_FREE(head);
+else
+{
+Node *temp = head-next;
+X265_FREE(head);
+head = temp;
+}
+size -= 1;
+}
+}
+
+inline void pop_back()
+{
+if (head != NULL)
+{
+if (head-next == NULL)
+X265_FREE(head);
+else
+{
+Node *ptr = head, *ptr1 = NULL;
+while (ptr-next != NULL)
+

Re: [x265] [PATCH]RDLevel: Disable RDOQTS when RDO and/or TS are disabled.

2013-09-16 Thread Deepthi Nandakumar
Agreed. Resending the patch - alongwith a few cleanups to make it easier to
track all elements in the param struct.


On Mon, Sep 16, 2013 at 3:27 PM, Derek Buitenhuis 
derek.buitenh...@gmail.com wrote:

 On Mon, Sep 16, 2013 at 10:47 AM, Deepthi Nandakumar
 deep...@multicorewareinc.com wrote:

  Yes, this particular param flag is initialised to 1 (highest quality
  setting) in x265_param_default. I'm setting it to zero for a certain set
 of
  user defined parameters.

 What's the point of the first check
 (
 http://mailman.videolan.org/pipermail/x265-devel/2013-September/000783.html
 )
 which sets it to 1 then?

 - Derek
 ___
 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] [PATCH] Use mixed bitmap between FrameEncoder and FrameFilter to Fix crash and hash mistake in WPP mode

2013-09-16 Thread Min Chen
# HG changeset patch
# User Min Chen chenm...@163.com
# Date 1379336600 -28800
# Node ID 7669e48568743a927c5f1ee14c8adf76d5e3a406
# Parent  62964b3d57d9e47cf68aad57808bab568b105d61
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 62964b3d57d9 -r 7669e4856874 source/encoder/frameencoder.cpp
--- a/source/encoder/frameencoder.cpp   Mon Sep 16 21:01:47 2013 +0800
+++ b/source/encoder/frameencoder.cpp   Mon Sep 16 21:03:20 2013 +0800
@@ -919,8 +919,6 @@
 {
 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];
@@ -954,7 +952,7 @@
 m_rows[row + 1].m_completed + 2 = m_rows[row].m_completed)
 {
 m_rows[row + 1].m_active = true;
-enqueueRowEncoder(row + 1);
+WaveFront::enqueueRow(row + 1);
 }
 }
 
@@ -970,16 +968,13 @@
 // Run row-wise loop filters
 if (row = m_filterRowDelay)
 {
-enqueueRowFilter(row - m_filterRowDelay);
-
-// NOTE: Active Filter to first row (row 0)
-if (row == m_filterRowDelay)
-enableRowFilter(0);
+m_frameFilter.processRow(row - m_filterRowDelay);
 }
 if (row == m_numRows - 1)
 {
 for(int i = m_numRows - m_filterRowDelay; i  m_numRows; i++)
-enqueueRowFilter(i);
+m_frameFilter.processRow(i);
+m_completionEvent.trigger();
 }
 }
 

___
x265-devel mailing list
x265-devel@videolan.org
https://mailman.videolan.org/listinfo/x265-devel


Re: [x265] [PATCH v2]: CLI: remove CLI option rdoqts; cleanup

2013-09-16 Thread Deepthi Nandakumar
Thanks for pointing that out, that was indeed unintentional. Pushed the
fix.


On Mon, Sep 16, 2013 at 9:11 PM, Derek Buitenhuis 
derek.buitenh...@gmail.com wrote:

 On Mon, Sep 16, 2013 at 1:30 PM, Deepthi Nandakumar
 deep...@multicorewareinc.com wrote:
  # HG changeset patch
  # User Deepthi Nandakumar deep...@multicorewareinc.com
  # Date 1379334518 -19800
  # Node ID 46b065f7d676e7ff26c46a40f1790bdae290d7fa
  # Parent  881444f5910b2b0e0f286a6ca47fcc743515cbb2
  CLI options: Eliminate rdoqts option; cleanup
 
  1. Eliminate rdoqts CLI option: enabled when rdoq and ts are both
 enabled.
  2. Rearrange default initialisations in x265_param_ t structure
 
  diff -r 881444f5910b -r 46b065f7d676 source/common/common.cpp
  --- a/source/common/common.cppMon Sep 16 09:41:34 2013 +0530
  +++ b/source/common/common.cppMon Sep 16 17:58:38 2013 +0530
  @@ -115,45 +115,58 @@
   va_end(arg);
   }
 
  -extern C
   void x265_param_default(x265_param_t *param)

 This looks incorrect. The function needs to be exported and to be able
 to be called
 from C.


  diff -r 881444f5910b -r 46b065f7d676 source/encoder/encoder.cpp
  --- a/source/encoder/encoder.cppMon Sep 16 09:41:34 2013 +0530
  +++ b/source/encoder/encoder.cppMon Sep 16 17:58:38 2013 +0530
  @@ -219,6 +219,11 @@
   _param-rc.rateControlMode = X265_RC_ABR;
   }
 
  +if(!(_param-bEnableRDOQ  _param-bEnableTransformSkip))
  +{
  +_param-bEnableRDOQTS = 0;
  +}

 Please add a note in the commit message about this.

 Rest is OK.

 - Derek
 ___
 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] List: std::list Implementation

2013-09-16 Thread Steve Borho
On Mon, Sep 16, 2013 at 5:31 AM, Gopu Govindaswamy 
g...@multicorewareinc.com wrote:

 # HG changeset patch
 # User Gopu Govindaswamy g...@multicorewareinc.com
 # Date 1379327475 -19800
 # Node ID e499466c7c6591345af2a625da12185c7735347b
 # Parent  6bab41a554b36133865fe3378964cb9e76c24ebd
 List: std::list Implementation

 To remove the std::list dependency from X265,  and this class can be
 enhanced based on the types of std::list API's used in current x265

 diff -r 6bab41a554b3 -r e499466c7c65 source/common/CMakeLists.txt
 --- a/source/common/CMakeLists.txt  Fri Sep 13 17:24:05 2013 +0530
 +++ b/source/common/CMakeLists.txt  Mon Sep 16 16:01:15 2013 +0530
 @@ -16,8 +16,10 @@

  file(GLOB LIBCOMMON_HDR ../Lib/TLibCommon/*.h)
  file(GLOB LIBCOMMON_SRC ../Lib/TLibCommon/*.cpp)


the CMakeLists.txt file in the source/ folder will need a line like this:

include_directories(util)


 +file(GLOB LIBUTIL ../util/*.cpp ../util/*.h)


the LIB prefix to LIBUTIL is unnecessary, it doesn't live in the Lib/ folder


  source_group(TLibCommon FILES ${LIBCOMMON_SRC})
  source_group(TLibCommonH FILES ${LIBCOMMON_HDR})
 +source_group(Util FILES ${LIBUTIL})
  if(GCC)
  set_source_files_properties(${LIBCOMMON_SRC} PROPERTIES COMPILE_FLAGS
  -Wno-sign-compare)
 @@ -38,7 +40,7 @@
  endif(MSVC)

  add_library(common STATIC ../../COPYING
 -${LIBCOMMON_SRC} ${LIBCOMMON_HDR}
 +${LIBCOMMON_SRC} ${LIBCOMMON_HDR} ${LIBUTIL}
  primitives.cpp primitives.h
  pixel.cpp dct.cpp ipfilter.cpp intrapred.cpp
  ../VectorClass/instrset_detect.cpp
 diff -r 6bab41a554b3 -r e499466c7c65 source/util/list.h
 --- /dev/null   Thu Jan 01 00:00:00 1970 +
 +++ b/source/util/list.hMon Sep 16 16:01:15 2013 +0530
 @@ -0,0 +1,158 @@

 +/*
 + * Copyright (C) 2013 x265 project
 + *
 + * Authors: Gopu Govindaswamy g...@multicorewareinc.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02111,
 USA.
 + *
 + * This program is also available under a commercial proprietary license.
 + * For more information, contact us at licens...@multicorewareinc.com.
 +
 */
 +
 +#ifndef X265_LIST_H
 +#define X265_LIST_H
 +
 +#include common.h
 +
 +// Short Notes:
 +// Under development
 +// this class is used to remove the std::list dependency from x265,
 +// Providing Minimum std::list API implementation


either list a negative list (of unsupported APIs) or a positive list (of
supported APIs).  I'm not sure I follow the capitalization policy of that
sentence.


 +// this class can be enhanced based on types of std::list API's used in
 current x265


this comment is pretty redundant, can be removed.  You could say we only
support APIs used by x265, which is at least informative.

Also add a comment stating this class does not pretend to be thread-safe


 +
 +templateclass T
 +struct List
 +{
 +private:
 +
 +struct Node
 +{
 +T object;
 +Node* next;
 +};
 +
 +public:
 +
 +Node* head;
 +Node* tail;
 +int size;
 +
 +List() { head = 0; size = 0; }


tail is uninitialized


 +
 +inline T* begin() { return head; }
 +
 +inline T* end() { return tail; }
 +
 +inline bool isEmpty()
 +{


would be helpful to assert head == NULL and size == 0 here


 +if (head == NULL)
 +return true;
 +else
 +return false;
 +}
 +
 +inline void push_back(T value)
 +{
 +if (head == NULL)
 +{
 +head = (Node*)X265_MALLOC(Node, sizeof(Node));


these mallocs are wrong.  the arguments to X265_MALLOC are type, count


 +head-object = value;
 +head-next = NULL;
 +tail = head;
 +}
 +else
 +{
 +Node *nodePtr = tail;
 +nodePtr-next = (Node*)X265_MALLOC(Node, sizeof(Node));
 +nodePtr-next-object = value;
 +nodePtr-next-next = NULL;
 +tail = nodePtr-next;
 +}
 +size += 1;
 +}
 +
 +inline void push_front(T value)
 +{
 +if (head == NULL)
 +{
 +head = (Node*)X265_MALLOC(Node, sizeof(Node));
 +head-object = value;
 +   

[x265] [PATCH] ABR - Removed unused states, cleaned up the comments

2013-09-16 Thread aarthi
# HG changeset patch
# User Aarthi Thirumalai
# Date 1379356988 -19800
#  Tue Sep 17 00:13:08 2013 +0530
# Node ID baeddbdc9ec349002f574bfc1763927f487cc88d
# Parent  9a727efab9fa4048eed7330abe28cf7e088590fb
ABR - Removed unused states, cleaned up the comments.

Removed states that are not used in ABR or CQP rate contnrol modes.

diff -r 9a727efab9fa -r baeddbdc9ec3 source/encoder/ratecontrol.cpp
--- a/source/encoder/ratecontrol.cppMon Sep 16 22:57:07 2013 +0530
+++ b/source/encoder/ratecontrol.cppTue Sep 17 00:13:08 2013 +0530
@@ -61,7 +61,7 @@
 ncu = (int)((param-sourceHeight * param-sourceWidth) / 
pow((int)param-maxCUSize, 2.0));
 lastNonBPictType = -1;
 baseQp = param-rc.qp;
-qpm = qp = baseQp;
+qp = baseQp;
 
 // heuristics- encoder specific
 qCompress = param-rc.qCompress; // tweak and test for x265.
@@ -101,7 +101,7 @@
 
 //qstep - value set as encoder specific.
 lstep = pow(2, param-rc.qpStep / 6.0);
-cbrDecay = 1.0;
+
 }
 
 void RateControl::rateControlStart(TComPic* pic, Lookahead *l, 
RateControlEntry* rce)
@@ -116,8 +116,7 @@
 lastSatd = l-getEstimatedPictureCost(pic);
 double q = qScale2qp(rateEstimateQscale(rce));
 qp = Clip3(MIN_QP, MAX_QP, (int)(q + 0.5));
-rce-qpaRc = qpm = q;
-rce-newQp = qp;
+rce-qpaRc  = q;
 accumPQpUpdate();
 break;
 }
@@ -146,15 +145,14 @@
 accumPNorm *= .95;
 accumPNorm += 1;
 if (frameType == I_SLICE)
-accumPQp += qpm + ipOffset;
+accumPQp += qp + ipOffset;
 else
-accumPQp += qpm;
+accumPQp += qp;
 }
 
 double RateControl::rateEstimateQscale(RateControlEntry *rce)
 {
 double q;
-// ratecontrol_entry_t rce = UNINIT(rce);
 int pictType = frameType;
 
 if (pictType == B_SLICE)
@@ -207,8 +205,6 @@
  * tolerances, the bit distribution approaches that of 2pass. */
 
 double wantedBits, overflow = 1;
-rce-pCount = ncu;
-
 shortTermCplxSum *= 0.5;
 shortTermCplxCount *= 0.5;
 shortTermCplxSum += lastSatd / (CLIP_DURATION(frameDuration) / 
BASE_FRAME_DURATION);
@@ -217,7 +213,6 @@
 rce-blurredComplexity = shortTermCplxSum / shortTermCplxCount;
 rce-mvBits = 0;
 rce-pictType = pictType;
-//need to checked where it is initialized
 q = getQScale(rce, wantedBitsWindow / cplxrSum);
 
 /* ABR code can potentially be counterproductive in CBR, so just don't 
bother.
@@ -236,7 +231,6 @@
 }
 
 if (pictType == I_SLICE  keyFrameInterval  1
-/* should test _next_ pict type, but that isn't decided yet */
  lastNonBPictType != I_SLICE)
 {
 q = qp2qScale(accumPQp / accumPNorm);
@@ -270,12 +264,10 @@
 q = Clip3(lqmin, lqmax, q);
 }
 
-//FIXME use get_diff_limited_q() ?
 double lmin1 = lmin[pictType];
 double lmax1 = lmax[pictType];
 q = Clip3(lmin1, lmax1, q);
-lastQScaleFor[pictType] =
-lastQScale = q;
+lastQScaleFor[pictType] = q;
 
 if (curFrame-getPOC() == 0)
 lastQScaleFor[P_SLICE] = q * fabs(ipFactor);
@@ -300,12 +292,11 @@
 {
 rce-lastRceq = q;
 q /= rateFactor;
-lastQScale = q;
 }
 return q;
 }
 
-/* After encoding one frame, save stats and update ratecontrol state */
+/* After encoding one frame,  update ratecontrol state */
 int RateControl::rateControlEnd(int64_t bits, RateControlEntry* rce)
 {
 if (rateControlMode == X265_RC_ABR)
@@ -320,9 +311,7 @@
  * Not perfectly accurate with B-refs, but good enough. */
 cplxrSum += bits * qp2qScale(rce-qpaRc) / (rce-lastRceq * 
fabs(pbFactor));
 }
-cplxrSum *= cbrDecay;
 wantedBitsWindow += frameDuration * bitrate;
-wantedBitsWindow *= cbrDecay;   
 rce = NULL;
 }
 totalBits += bits;
diff -r 9a727efab9fa -r baeddbdc9ec3 source/encoder/ratecontrol.h
--- a/source/encoder/ratecontrol.h  Mon Sep 16 22:57:07 2013 +0530
+++ b/source/encoder/ratecontrol.h  Tue Sep 17 00:13:08 2013 +0530
@@ -35,12 +35,10 @@
 struct RateControlEntry
 {
 int pictType;
-int pCount;
-int newQp;
 int texBits;
 int mvBits;
 double blurredComplexity;
-double qpaRc;/* average of macroblocks' qp before aq */
+double qpaRc;
 double lastRceq;
 };
 
@@ -54,9 +52,7 @@
 int keyFrameInterval;   /* TODO: need to initialize in init */
 int qp; /* updated qp for current frame */
 int baseQp; /* CQP base QP */
-double frameDuration;/* current frame duration in seconds */
-double qpm;  /* qp for current macroblock: precise double 
for AQ */
-
+double frameDuration;/* current frame duration in seconds */ 
 double bitrate;