<div dir="ltr">Sent a new patch.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Mar 8, 2014 at 12:50 AM, Steve Borho <span dir="ltr"><<a href="mailto:steve@borho.org" target="_blank">steve@borho.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Mar 7, 2014 at 5:45 AM,  <<a href="mailto:murugan@multicorewareinc.com">murugan@multicorewareinc.com</a>> wrote:<br>

> # HG changeset patch<br>
> # User Murugan Vairavel <<a href="mailto:murugan@multicorewareinc.com">murugan@multicorewareinc.com</a>><br>
> # Date 1394192688 -19800<br>
> #      Fri Mar 07 17:14:48 2014 +0530<br>
> # Node ID 5c626bf1e275596b45808c14952bd5aea8aaeb3e<br>
> # Parent  2bf727dca27d6f69e96d4412850661cbe036cbef<br>
> shortyuv: integrated asm primitives for blockcopy<br>
><br>
> diff -r 2bf727dca27d -r 5c626bf1e275 source/common/shortyuv.cpp<br>
> --- a/source/common/shortyuv.cpp        Fri Mar 07 15:11:13 2014 +0530<br>
> +++ b/source/common/shortyuv.cpp        Fri Mar 07 17:14:48 2014 +0530<br>
> @@ -129,32 +129,27 @@<br>
><br>
>  void ShortYuv::copyPartToPartLuma(ShortYuv* dstPicYuv, uint32_t partIdx, uint32_t width, uint32_t height)<br>
>  {<br>
> +    int part = partitionFromSizes(width, height);<br>
>      int16_t* src = getLumaAddr(partIdx);<br>
>      int16_t* dst = dstPicYuv->getLumaAddr(partIdx);<br>
><br>
>      uint32_t srcStride = m_width;<br>
>      uint32_t dstStride = dstPicYuv->m_width;<br>
> -#if HIGH_BIT_DEPTH<br>
> -    primitives.blockcpy_pp(width, height, (pixel*)dst, dstStride, (pixel*)src, srcStride);<br>
> -#else<br>
> -    for (uint32_t y = height; y != 0; y--)<br>
> -    {<br>
> -        ::memcpy(dst, src, width * sizeof(int16_t));<br>
> -        src += srcStride;<br>
> -        dst += dstStride;<br>
> -    }<br>
> -#endif<br>
> +<br>
> +    primitives.luma_copy_ss[part](dst, dstStride, src, srcStride);<br>
> +<br>
>  }<br>
><br>
>  void ShortYuv::copyPartToPartLuma(TComYuv* dstPicYuv, uint32_t partIdx, uint32_t width, uint32_t height)<br>
>  {<br>
> +    int part = partitionFromSizes(width, height);<br>
>      int16_t* src = getLumaAddr(partIdx);<br>
>      pixel* dst = dstPicYuv->getLumaAddr(partIdx);<br>
><br>
>      uint32_t srcStride = m_width;<br>
>      uint32_t dstStride = dstPicYuv->getStride();<br>
><br>
> -    primitives.blockcpy_ps(width, height, dst, dstStride, src, srcStride);<br>
> +    primitives.luma_copy_sp[part](dst, dstStride, src, srcStride);<br>
>  }<br>
><br>
>  void ShortYuv::copyPartToPartChroma(ShortYuv* dstPicYuv, uint32_t partIdx, uint32_t width, uint32_t height)<br>
> @@ -163,23 +158,15 @@<br>
>      int16_t* srcV = getCrAddr(partIdx);<br>
>      int16_t* dstU = dstPicYuv->getCbAddr(partIdx);<br>
>      int16_t* dstV = dstPicYuv->getCrAddr(partIdx);<br>
> +    width  = width  << m_hChromaShift;<br>
> +    height = height << m_vChromaShift;<br>
> +    int part = partitionFromSizes(width, height);<br>
><br>
>      uint32_t srcStride = m_cwidth;<br>
>      uint32_t dstStride = dstPicYuv->m_cwidth;<br>
> -#if HIGH_BIT_DEPTH<br>
> -    primitives.blockcpy_pp(width, height, (pixel*)dstU, dstStride, (pixel*)srcU, srcStride);<br>
> -    primitives.blockcpy_pp(width, height, (pixel*)dstV, dstStride, (pixel*)srcV, srcStride);<br>
> -#else<br>
> -    for (uint32_t y = height; y != 0; y--)<br>
> -    {<br>
> -        ::memcpy(dstU, srcU, width * sizeof(int16_t));<br>
> -        ::memcpy(dstV, srcV, width * sizeof(int16_t));<br>
> -        srcU += srcStride;<br>
> -        srcV += srcStride;<br>
> -        dstU += dstStride;<br>
> -        dstV += dstStride;<br>
> -    }<br>
> -#endif<br>
> +<br>
> +    primitives.chroma[m_csp].copy_ss[part](dstU, dstStride, srcU, srcStride);<br>
> +    primitives.chroma[m_csp].copy_ss[part](dstV, dstStride, srcV, srcStride);<br>
<br>
<br>
</div></div>I'm pretty sure this is wrong.  The chroma primitives are indexed by<br>
color space and luma partition size, so the function pointer itself<br>
scales the luma dimensions down to the correct chroma dimensions for<br>
that color space.<br>
<br>
So essentially this function cannot use the chroma block copy<br>
functions without changing the function arguments and callers to pass<br>
in luma dimensions or the luma partition enum.<br>
<div class=""><br>
>  }<br>
><br>
>  void ShortYuv::copyPartToPartChroma(TComYuv* dstPicYuv, uint32_t partIdx, uint32_t width, uint32_t height)<br>
> @@ -188,28 +175,29 @@<br>
>      int16_t* srcV = getCrAddr(partIdx);<br>
>      pixel* dstU = dstPicYuv->getCbAddr(partIdx);<br>
>      pixel* dstV = dstPicYuv->getCrAddr(partIdx);<br>
> +    width  = width  << m_hChromaShift;<br>
> +    height = height << m_vChromaShift;<br>
> +    int part = partitionFromSizes(width, height);<br>
><br>
>      uint32_t srcStride = m_cwidth;<br>
>      uint32_t dstStride = dstPicYuv->getCStride();<br>
><br>
> -    primitives.blockcpy_ps(width, height, dstU, dstStride, srcU, srcStride);<br>
> -    primitives.blockcpy_ps(width, height, dstV, dstStride, srcV, srcStride);<br>
> +    primitives.chroma[m_csp].copy_sp[part](dstU, dstStride, srcU, srcStride);<br>
> +    primitives.chroma[m_csp].copy_sp[part](dstV, dstStride, srcV, srcStride);<br>
<br>
</div>ditto<br>
<div><div class="h5"><br>
>  }<br>
><br>
>  void ShortYuv::copyPartToPartChroma(ShortYuv* dstPicYuv, uint32_t partIdx, uint32_t width, uint32_t height, uint32_t chromaId)<br>
>  {<br>
> +    width  = width  << m_hChromaShift;<br>
> +    height = height << m_vChromaShift;<br>
> +    int part = partitionFromSizes(width, height);<br>
>      if (chromaId == 0)<br>
>      {<br>
>          int16_t* srcU = getCbAddr(partIdx);<br>
>          int16_t* dstU = dstPicYuv->getCbAddr(partIdx);<br>
>          uint32_t srcStride = m_cwidth;<br>
>          uint32_t dstStride = dstPicYuv->m_cwidth;<br>
> -        for (uint32_t y = height; y != 0; y--)<br>
> -        {<br>
> -            ::memcpy(dstU, srcU, width * sizeof(int16_t));<br>
> -            srcU += srcStride;<br>
> -            dstU += dstStride;<br>
> -        }<br>
> +        primitives.chroma[m_csp].copy_ss[part](dstU, dstStride, srcU, srcStride);<br>
>      }<br>
>      else if (chromaId == 1)<br>
>      {<br>
> @@ -217,12 +205,7 @@<br>
>          int16_t* dstV = dstPicYuv->getCrAddr(partIdx);<br>
>          uint32_t srcStride = m_cwidth;<br>
>          uint32_t dstStride = dstPicYuv->m_cwidth;<br>
> -        for (uint32_t y = height; y != 0; y--)<br>
> -        {<br>
> -            ::memcpy(dstV, srcV, width * sizeof(int16_t));<br>
> -            srcV += srcStride;<br>
> -            dstV += dstStride;<br>
> -        }<br>
> +        primitives.chroma[m_csp].copy_ss[part](dstV, dstStride, srcV, srcStride);<br>
>      }<br>
>      else<br>
>      {<br>
> @@ -232,27 +215,23 @@<br>
>          int16_t* dstV = dstPicYuv->getCrAddr(partIdx);<br>
>          uint32_t srcStride = m_cwidth;<br>
>          uint32_t dstStride = dstPicYuv->m_cwidth;<br>
> -        for (uint32_t y = height; y != 0; y--)<br>
> -        {<br>
> -            ::memcpy(dstU, srcU, width * sizeof(int16_t));<br>
> -            ::memcpy(dstV, srcV, width * sizeof(int16_t));<br>
> -            srcU += srcStride;<br>
> -            srcV += srcStride;<br>
> -            dstU += dstStride;<br>
> -            dstV += dstStride;<br>
> -        }<br>
> +        primitives.chroma[m_csp].copy_ss[part](dstU, dstStride, srcU, srcStride);<br>
> +        primitives.chroma[m_csp].copy_ss[part](dstV, dstStride, srcV, srcStride);<br>
<br>
</div></div>and... ditto<br>
<div><div class="h5"><br>
>      }<br>
>  }<br>
><br>
>  void ShortYuv::copyPartToPartChroma(TComYuv* dstPicYuv, uint32_t partIdx, uint32_t width, uint32_t height, uint32_t chromaId)<br>
>  {<br>
> +    width  = width  << m_hChromaShift;<br>
> +    height = height << m_vChromaShift;<br>
> +    int part = partitionFromSizes(width, height);<br>
>      if (chromaId == 0)<br>
>      {<br>
>          int16_t* srcU = getCbAddr(partIdx);<br>
>          pixel* dstU = dstPicYuv->getCbAddr(partIdx);<br>
>          uint32_t srcStride = m_cwidth;<br>
>          uint32_t dstStride = dstPicYuv->getCStride();<br>
> -        primitives.blockcpy_ps(width, height, dstU, dstStride, srcU, srcStride);<br>
> +        primitives.chroma[m_csp].copy_sp[part](dstU, dstStride, srcU, srcStride);<br>
>      }<br>
>      else if (chromaId == 1)<br>
>      {<br>
> @@ -260,7 +239,7 @@<br>
>          pixel* dstV = dstPicYuv->getCrAddr(partIdx);<br>
>          uint32_t srcStride = m_cwidth;<br>
>          uint32_t dstStride = dstPicYuv->getCStride();<br>
> -        primitives.blockcpy_ps(width, height, dstV, dstStride, srcV, srcStride);<br>
> +        primitives.chroma[m_csp].copy_sp[part](dstV, dstStride, srcV, srcStride);<br>
>      }<br>
>      else<br>
>      {<br>
> @@ -271,7 +250,7 @@<br>
><br>
>          uint32_t srcStride = m_cwidth;<br>
>          uint32_t dstStride = dstPicYuv->getCStride();<br>
> -        primitives.blockcpy_ps(width, height, dstU, dstStride, srcU, srcStride);<br>
> -        primitives.blockcpy_ps(width, height, dstV, dstStride, srcV, srcStride);<br>
> +        primitives.chroma[m_csp].copy_sp[part](dstU, dstStride, srcU, srcStride);<br>
> +        primitives.chroma[m_csp].copy_sp[part](dstV, dstStride, srcV, srcStride);<br>
<br>
</div></div>still busted.<br>
<br>
Have a look at how the TComYuv methods were modified to handle this.<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Steve Borho<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" target="_blank">https://mailman.videolan.org/listinfo/x265-devel</a><br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">With Regards,<div><br></div><div>Murugan. V</div><div>+919659287478</div></div>
</div>