[x265] [PATCH] cutree: when cutree enabled P_SLICE CU QP always calculated from qpoffset,

[Steve Borho]

On Wed, Apr 2, 2014 at 12:52 AM,  <gopu at multicorewareinc.com> wrote:
> # HG changeset patch
> # User Gopu Govindaswamy
> # Date 1396417894 -19800
> #      Wed Apr 02 11:21:34 2014 +0530
> # Node ID 931b4cd4da91dc667efd16d26b4e9ea834007623
> # Parent  03bad90e94adce6fb89c6d5edb86919a1e783402
> cutree: when cutree enabled P_SLICE CU QP always calculated from qpoffset,
>
> diff -r 03bad90e94ad -r 931b4cd4da91 source/encoder/frameencoder.cpp
> --- a/source/encoder/frameencoder.cpp   Wed Apr 02 06:51:35 2014 +0530
> +++ b/source/encoder/frameencoder.cpp   Wed Apr 02 11:21:34 2014 +0530
> @@ -1228,7 +1228,17 @@
>      int block_y = (cuAddr / m_pic->getPicSym()->getFrameWidthInCU()) * noOfBlocks;
>      int block_x = (cuAddr * noOfBlocks) - block_y * m_pic->getPicSym()->getFrameWidthInCU();
>
> -    double *qpoffs = (m_pic->getSlice()->isReferenced() && m_cfg->param->rc.cuTree) ? m_pic->m_lowres.qpOffset : m_pic->m_lowres.qpAqOffset;
> +    double *qpoffs;
> +    if (m_cfg->param->rc.cuTree)
> +    {
> +       if(m_pic->getSlice()->isReferenced() || m_pic->getSlice()->getSliceType() == P_SLICE)
> +           qpoffs = m_pic->m_lowres.qpOffset;
> +       else
> +           qpoffs = m_pic->m_lowres.qpAqOffset;
> +    }
> +    else
> +       qpoffs = m_pic->m_lowres.qpAqOffset;

Ok, I've realized qpOffset is the buffer holding the cuTree adjusted
offsets while qpAqOffset has just the AQ adjustments (these variable
names suck, but that is out-of-scope for this patch discussion)

So the net effect of this patch is to treat all P slices as referenced
and force them to use cuTree modified AQ offsets because apparently
the reference flag is not deterministic.

Your commit message was unclear, it should have looked more like this:

"""
cutree: force P slices to use cuTree offsets, even if unreferenced

We find that the referenced state of P frames is non-deterministic,
leading to non-deterministic encodes.  This patch makes the encodes
deterministic.
"""

Then we could make a better evaluation on the merits of the patch, and
the first thing that pops into my head is:  "Why is the reference
state of P frames non-deterministic?" That is the root cause of the
problem and it needs to be tackled next.  Once that is fixed this
workaround can be backed out, since I see an equivalent patch was
already applied.

-- 
Steve Borho