[x265] [PATCH 2 of 2] psy-rd: ported few more fucntions from x264 and included psyRd variable in x265
Steve Borho
steve at borho.org
Fri Mar 21 02:59:31 CET 2014
On Mon, Mar 17, 2014 at 1:31 AM, <sumalatha at multicorewareinc.com> wrote:
> # HG changeset patch
> # User Sumalatha Polureddy
> # Date 1395037805 -19800
> # Node ID 5c6f04999bc0fc9ee2f6cc2e19f51b022fe3b89a
> # Parent 6f38933d38248001389f1c5f90f97dfce3a4ae2c
> psy-rd: ported few more fucntions from x264 and included psyRd variable in x265
I have lots of coding style and detail nits below, but the biggest
question I have is whether this feature works.
>
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/Lib/TLibCommon/TComDataCU.cpp
> --- a/source/Lib/TLibCommon/TComDataCU.cpp Mon Mar 17 11:29:07 2014 +0530
> +++ b/source/Lib/TLibCommon/TComDataCU.cpp Mon Mar 17 12:00:05 2014 +0530
> @@ -237,6 +237,7 @@
> m_avgCost[i] = 0;
> m_count[i] = 0;
> }
> + m_psyRdLambda = x265_lambda2_non_I[qp];
>
> // CHECK_ME: why partStartIdx always negative
> int numElements = m_numPartitions;
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/Lib/TLibCommon/TComDataCU.h
> --- a/source/Lib/TLibCommon/TComDataCU.h Mon Mar 17 11:29:07 2014 +0530
> +++ b/source/Lib/TLibCommon/TComDataCU.h Mon Mar 17 12:00:05 2014 +0530
> @@ -181,6 +181,12 @@
> uint32_t m_count[4];
> uint64_t m_sa8dCost;
> double m_baseQp; //Qp of Cu set from RateControl/Vbv.
> +
> + /* Psy RD SATD/SA8D scores cache */
> + uint64_t fencHadamardCache[9];
> + uint32_t fencSatdCache[32];
> + double m_psyRdLambda;
> +
> // -------------------------------------------------------------------------------------------------------------------
> // create / destroy / initialize / copy
> // -------------------------------------------------------------------------------------------------------------------
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/Lib/TLibCommon/TComPic.cpp
> --- a/source/Lib/TLibCommon/TComPic.cpp Mon Mar 17 11:29:07 2014 +0530
> +++ b/source/Lib/TLibCommon/TComPic.cpp Mon Mar 17 12:00:05 2014 +0530
> @@ -109,6 +109,8 @@
> ok &= m_lowres.create(m_origPicYuv, cfg->param->bframes, !!cfg->param->rc.aqMode);
>
> bool isVbv = cfg->param->rc.vbvBufferSize > 0 && cfg->param->rc.vbvMaxBitrate > 0;
> + psyRd = cfg->param->subpelRefine >= 6 ? FIX8(cfg->param->psyRd) : 0;
do any of our presets use subpelRefine > 6?
> +
> if (ok && (isVbv || cfg->param->rc.aqMode))
> {
> int numRows = m_picSym->getFrameHeightInCU();
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/Lib/TLibCommon/TComPic.h
> --- a/source/Lib/TLibCommon/TComPic.h Mon Mar 17 11:29:07 2014 +0530
> +++ b/source/Lib/TLibCommon/TComPic.h Mon Mar 17 12:00:05 2014 +0530
> @@ -117,6 +117,8 @@
> double m_avgQpRc; //avg QP as decided by ratecontrol
> double m_avgQpAq; //avg QP as decided by AQ in addition to ratecontrol
>
> + int psyRd; /* Psy RD strength */
class member variables start with m_
> +
> TComPic();
> virtual ~TComPic();
>
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/common/common.h
> --- a/source/common/common.h Mon Mar 17 11:29:07 2014 +0530
> +++ b/source/common/common.h Mon Mar 17 12:00:05 2014 +0530
> @@ -119,6 +119,8 @@
> } \
> }
>
> +#define FIX8(f) ((int)(f * (1 << 8) + .5))
> +
> #if defined(_MSC_VER)
> #define X265_LOG2F(x) (logf((float)(x)) * 1.44269504088896405f)
> #define X265_LOG2(x) (log((double)(x)) * 1.4426950408889640513713538072172)
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/common/param.cpp
> --- a/source/common/param.cpp Mon Mar 17 11:29:07 2014 +0530
> +++ b/source/common/param.cpp Mon Mar 17 12:00:05 2014 +0530
> @@ -205,6 +205,7 @@
> param->vui.bEnableNalHrdParametersPresentFlag = 0;
> param->vui.bEnableBitstreamRestrictionFlag = 0;
> param->vui.bEnableSubPicHrdParamsPresentFlag = 0;
> + param->psyRd = 0.0;
> }
>
> extern "C"
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/encoder/CMakeLists.txt
> --- a/source/encoder/CMakeLists.txt Mon Mar 17 11:29:07 2014 +0530
> +++ b/source/encoder/CMakeLists.txt Mon Mar 17 12:00:05 2014 +0530
> @@ -58,4 +58,5 @@
> reference.cpp reference.h
> encoder.cpp encoder.h
> api.cpp
> - weightPrediction.cpp)
> + weightPrediction.cpp
> + rdo.cpp rdo.h)
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/encoder/rdo.cpp
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/source/encoder/rdo.cpp Mon Mar 17 12:00:05 2014 +0530
> @@ -0,0 +1,129 @@
> +/*****************************************************************************
> + * Copyright (C) 2013 x265 project
> + *
> + * Authors: Sumalatha PoluReddy <sumalatha at 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 licensing at multicorewareinc.com.
> + *****************************************************************************/
> +
> +#include "rdo.h"
> +
> +using namespace x265;
> +
> +static uint64_t cachedHadamard(TComDataCU* cu, int size, int x, int y, pixel *fenc, intptr_t fencStride)
> +{
> + static const uint8_t hadamardShiftX[4] = {4, 4, 3, 3};
> + static const uint8_t hadamardShiftY[4] = {4-0, 3-0, 4-1, 3-1};
> + static const uint8_t hadamardOffset[4] = {0, 1, 3, 5};
> + int cacheIndex = (x >> hadamardShiftX[size]) + (y >> hadamardShiftY[size])
> + + hadamardOffset[size];
> + uint64_t res = cu->fencHadamardCache[cacheIndex];
> + if (res)
> + return res - 1;
> + else
> + {
> + //pixel *fenc = cu->getPic()->getori.p_fenc[0] + x + y*FENC_STRIDE;
this commented line needs to go
> + res = primitives.pixelHadamardAc[size](fenc, fencStride);
> + cu->fencHadamardCache[cacheIndex] = res + 1;
> + return res;
> + }
> +}
> +
> +static int cachedSatd(TComDataCU* cu, int size, int x, int y, pixel *fenc, intptr_t fencStride)
> +{
> + static const uint8_t satdShiftX[3] = {3, 2, 2};
> + static const uint8_t satdShiftY[3] = {2-1, 3-2, 2-2};
> + static const uint8_t satdOffset[3] = {0, 8, 16};
> + pixel zero[16] = {0};
> + int cacheIndex = (x >> satdShiftX[size - 0/*PIXEL_8x4*/]) + (y >> satdShiftY[size - 0/*PIXEL_8x4*/])
> + + satdOffset[size - 0/*PIXEL_8x4*/]; //TODO: need to check for PIXEL_8x4
> + int res = cu->fencSatdCache[cacheIndex];
> + if (res)
> + return res - 1;
> + else
> + {
> + //pixel *fenc = h->mb.pic.p_fenc[0] + x + y*FENC_STRIDE;
> + int dc = primitives.sad[size](fenc, fencStride, zero, 0) >> 1;
> + res = primitives.satd[size](fenc, fencStride, zero, 0) - dc;
> + cu->fencSatdCache[cacheIndex] = res + 1;
> + return res;
> + }
> +}
these functions have too much knowledge of other encoder data
structures. It should just be passed pixel pointers, strides and
psy-rd constants
> +int ssdPlane(TComDataCU* cu, int size, int p, int x, int y, TComYuv* fencYuv, TComYuv* fdecYuv)
> +{
> + static pixel zero[16] = {0};
> + int satd = 0;
> + pixel *fenc = NULL, *fdec = NULL;
> + intptr_t fencStride = 0, fdecStride = 0;
> + if (p == 0)
> + {
> + fdec = fdecYuv->getLumaAddr();
> + fenc = fencYuv->getLumaAddr();
> + fencStride = fencYuv->getStride();
> + fdecStride = fdecYuv->getStride();
> + }
> + else if (p == 1)
> + {
> + fdec = fdecYuv->getCbAddr();
> + fenc = fencYuv->getCbAddr();
> + fencStride = fencYuv->getCStride();
> + fdecStride = fdecYuv->getCStride();
> + }
> + else if (p == 2)
> + {
> + fdec = fdecYuv->getCrAddr();
> + fenc = fencYuv->getCrAddr();
> + fencStride = fencYuv->getCStride();
> + fdecStride = fdecYuv->getCStride();
> + }
> +
> + if (p == 0 && cu->getPic()->psyRd)
> + {
> + /* If the plane is smaller than 8x8, we can't do an SA8D; this probably isn't a big problem. */
we should enforce minimum picture sizes, if we have any
> + if (size <= LUMA_8x8 )
> + {
> + uint64_t fdec_acs = primitives.pixelHadamardAc[size](fdec, fdecYuv->getStride());
> + uint64_t fenc_acs = cachedHadamard(cu, size, x, y, fenc, fencYuv->getStride());
> + satd = abs((int32_t)fdec_acs - (int32_t)fenc_acs)
> + + abs((int32_t)(fdec_acs>>32) - (int32_t)(fenc_acs>>32));
> + satd >>= 1;
> + }
> + else
> + {
> + int dc = primitives.sad[size](fdec, fdecYuv->getStride(), zero, 0) >> 1;
> + satd = abs(primitives.satd[size](fdec, fdecYuv->getStride(), zero, 0) - dc - cachedSatd(cu, size, x, y, fenc, fencYuv->getStride()));
> + }
> + satd = (satd * (int)(cu->getPic()->psyRd * cu->m_psyRdLambda) + 128) >> 8;
> + }
> + return primitives.sse_pp[size](fenc, fencStride, fdec, fdecStride) + satd;
> +}
> +
> +
> +/* Reset fenc satd scores cache for psy RD */
> +void cuInitFencCache(TComDataCU* cu, int b_satd)
our coding style would make that bool bSatd
> +{
> + //if( h->param.analyse.i_trellis == 2 && h->mb.i_psy_trellis )
> + //x264_psy_trellis_init( h, h->param.analyse.b_transform_8x8 );
more commented lines, must be removed
> + if (cu->getPic()->psyRd)
> + return;
is this check necessary and correct?
> + /* Writes beyond the end of the array, but not a problem since fenc_satd_cache is right after. */
ehh.. is this comment really true?
> + memset(cu->fencHadamardCache, 0, sizeof(cu->fencHadamardCache));
> + if (b_satd)
> + memset(cu->fencSatdCache, 0, sizeof(cu->fencSatdCache));
> +}
> \ No newline at end of file
needs a newline
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/encoder/rdo.h
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/source/encoder/rdo.h Mon Mar 17 12:00:05 2014 +0530
> @@ -0,0 +1,35 @@
> +
> +/*****************************************************************************
> + * Copyright (C) 2013 x265 project
> + *
> + * Authors: Sumalatha PoluReddy <sumalatha at 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 licensing at multicorewareinc.com.
> + *****************************************************************************/
add a blank line here
> +#ifndef X265_RDO_H
> +#define X265_RDO_H
> +#include "primitives.h"
> +#include "TLibCommon/TComYuv.h"
> +#include "TLibCommon/TComDataCU.h"
> +#include "TLibCommon\TComPic.h"
backslashes in paths are a great big no-no
And these functions should not need to know the internals of all those
data structures
> +
> +using namespace x265;
> +
> +int ssdPlane(TComDataCU* cu, int size, int p, int x, int y, TComYuv* fencYuv, TComYuv* fdecYuv);
> +void cuInitFencCache(TComDataCU* cu, int b_satd);
> +#endif // ifndef X265_RDO_H
> \ No newline at end of file
and a newline here
> diff -r 6f38933d3824 -r 5c6f04999bc0 source/x265.h
> --- a/source/x265.h Mon Mar 17 11:29:07 2014 +0530
> +++ b/source/x265.h Mon Mar 17 12:00:05 2014 +0530
> @@ -835,6 +835,10 @@
> int bEnableSubPicHrdParamsPresentFlag;
> } vui;
>
> + float psyRd; /* Psy RD strength */
> +
> +
> +
these blank lines violate our coding style. This field belongs close
to the other analysis / RD fields, and it really deserves a better
comment.
> } x265_param;
This is an API change so X265_BUILD must be bumped and this needs a CLI option.
More information about the x265-devel
mailing list