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

[Steve Borho]

On Sun, Sep 15, 2013 at 11:06 PM, Deepthi Nandakumar <
deepthi at multicorewareinc.com> wrote:

>
>
>
> On Fri, Sep 13, 2013 at 11:11 PM, Steve Borho <steve at borho.org> wrote:
>
>> # HG changeset patch
>> # User Steve Borho <steve at 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.cpp    Fri Sep 13 10:55:03 2013 -0500
>> +++ b/source/encoder/framefilter.cpp    Fri 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20130916/15510830/attachment-0001.html>