<div dir="ltr">Always inline the patches so they can be reviewed and commented on as text.  I'm pasting bits of it here so I can give feedback.<div><br></div><div style>First, this function table does not have to be exposed in primitives.h, it can be a private function table to intrapred.inc.  So remove all the changes to primitives.h</div>
<div style><br></div><div style><div>+void xPredIntraAng4x4_m_32(pixel* pDst, int dstStride, int width, int dirMode, pixel *refLeft, pixel *refAbove)</div><div><br></div><div>SJB:  we don't need to copy the HM's strange x prefix for function names in our primitives..</div>
<div><br></div><div> {</div><div style><br></div><div>SJB:  All the code from here..<br></div><div style><br></div><div>     int blkSize        = width;</div><div><br></div><div>@@ -1692,6 +1693,86 @@</div><div>     intraPredAngle     = signAng * absAng;</div>
<div><br></div><div>     // Do angular predictions</div><div>+    pixel* refMain;</div><div>+    pixel* refSide;</div><div>+</div><div>+    // Initialise the Main and Left reference array.</div><div>+    if (intraPredAngle < 0)</div>
<div>+    {</div><div>+        refMain = (modeVer ? refAbove : refLeft);     // + (blkSize - 1);</div><div>+        refSide = (modeVer ? refLeft : refAbove);     // + (blkSize - 1);</div><div>+</div><div>+        // Extend the Main reference to the left.</div>
<div>+        int invAngleSum    = 128;     // rounding for (shift by 8)</div><div>+        for (int k = -1; k > blkSize * intraPredAngle >> 5; k--)</div><div>+        {</div><div>+            invAngleSum += invAngle;</div>
<div>+            refMain[k] = refSide[invAngleSum >> 8];</div><div>+        }</div><div>+    }</div><div>+    else</div><div>+    {</div><div>+        refMain = modeVer ? refAbove : refLeft;</div><div>+        refSide = modeVer ? refLeft  : refAbove;</div>
<div>+    }</div><div><br></div><div style>SJB: .. to here should be handled by the caller.  The function arguments should only</div><div style>           include what these few lines of code below need.</div><div><br></div>
<div>+    Vec8s row11, row12, row21, row22, row31, row32, row41, row42;</div><div>+    Vec16uc tmp16_1, tmp16_2;</div><div>+    Vec2uq tmp2uq;</div><div>+    Vec8s v_deltaFract, v_deltaPos(0), thirty2(32), thirty1(31), v_ipAngle(0);</div>
<div>+</div><div>+    tmp16_1 = (Vec16uc)load_partial(const_int(8), refMain);    //-1,0,1,2</div><div>+    store_partial(const_int(4), pDst, tmp16_1);</div><div>+    tmp16_2 = (Vec16uc)load_partial(const_int(8), refMain - 1); //-2,-1,0,1</div>
<div>+    store_partial(const_int(4), pDst + dstStride, tmp16_2);</div><div>+    tmp16_2 = (Vec16uc)load_partial(const_int(8), refMain - 2);</div><div>+    store_partial(const_int(4), pDst + 2 * dstStride, tmp16_2);</div>
<div>+    tmp16_2 = (Vec16uc)load_partial(const_int(8), refMain - 3);</div><div>+    store_partial(const_int(4), pDst + 3 * dstStride, tmp16_2);</div><div>+    return;</div><div><br></div><div>SJB: .. drop this return statement</div>
<div><br></div><div>+}</div><div>+</div><div><div>+int get_xPredIntraAng_func(int dirMode)</div><div>+{</div><div>+    bool modeHor       = (dirMode < 18);</div><div>+    bool modeVer       = !modeHor;</div><div>+    int intraPredAngle = modeVer ? (int)dirMode - VER_IDX : modeHor ? -((int)dirMode - HOR_IDX) : 0;</div>
<div>+    int func_num  = abs(intraPredAngle);</div><div>+    if(intraPredAngle < 0)</div><div>+    {</div><div>+        func_num  += 8;</div><div>+    }</div><div>+    return func_num;</div><div>+}</div></div><div><br>
</div><div style>SJB: Why bother with this at all?  Why not just index into the table with dirMode?</div><div style><br></div><div style><div>+void xPredIntraAng4x4(int /*bitDepth*/, pixel* pDst, int dstStride, int width, int dirMode, pixel *refLeft, pixel *refAbove, bool bFilter = true)</div>
<div>+{</div><div>+    int blkSize        = width;</div><div>+</div><div>+    // Map the mode index to main prediction direction and angle</div><div>+    assert(dirMode > 1); //no planar and dc</div><div>+    bool modeHor       = (dirMode < 18);</div>
<div>+    bool modeVer       = !modeHor;</div><div>+    int intraPredAngle = modeVer ? (int)dirMode - VER_IDX : modeHor ? -((int)dirMode - HOR_IDX) : 0;</div><div>+    int absAng         = abs(intraPredAngle);</div><div>+    int signAng        = intraPredAngle < 0 ? -1 : 1;</div>
<div>+</div><div>+    // Set bitshifts and scale the angle parameter to block size</div><div>+    int angTable[9]    = { 0,    2,    5,   9,  13,  17,  21,  26,  32 };</div><div>+    int invAngTable[9] = { 0, 4096, 1638, 910, 630, 482, 390, 315, 256 }; // (256 * 32) / Angle</div>
<div>+    int invAngle       = invAngTable[absAng];</div><div>+    absAng             = angTable[absAng];</div><div>+    intraPredAngle     = signAng * absAng;</div><div>+</div><div>+    if(dirMode==18)</div><div><br></div>
<div style>SJB: mind your white-space</div><div style><br></div><div>+    {</div><div>+        x265::EncoderPrimitives p;</div><div>+        p.xPredIntraAng4x4_table[X_PRED_INTRA_ANG_M_32] = xPredIntraAng4x4_m_32;</div><div>
+        p.xPredIntraAng4x4_table[get_xPredIntraAng_func(dirMode)](pDst, dstStride, width, dirMode, refLeft, refAbove);</div><div>+        return;</div><div><br></div><div style>SJB: this is all kinds of ugly.  You can't instantiate an encoder primitive struct here.  You should define a function table as a file static and setup that table in the setup function at the bottom of the file, and (repeating above) the table should be indexed by dirMode and the arguments should only be the bare minimum needed to generate the prediction.</div>
<div><br></div><div>+    }</div><div>+    // Do angular predictions</div><div><br></div></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 24, 2013 at 7:11 AM,  <span dir="ltr"><<a href="mailto:mandar@multicorewareinc.com" target="_blank">mandar@multicorewareinc.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Patch subject is complete summary.<br>
<br>
<br>
<br>_______________________________________________<br>
x265-devel mailing list<br>
<a href="mailto:x265-devel@videolan.org">x265-devel@videolan.org</a><br>
<a href="http://mailman.videolan.org/listinfo/x265-devel" target="_blank">http://mailman.videolan.org/listinfo/x265-devel</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div>-- <br>Steve Borho
</div>