[x265] [PATCH] cost: fix mode cost check
Steve Borho
steve at borho.org
Tue Sep 29 15:43:11 CEST 2015
On 09/29, Divya Manivannan wrote:
> # HG changeset patch
> # User Divya Manivannan <divya at multicorewareinc.com>
> # Date 1443524396 -19800
> # Tue Sep 29 16:29:56 2015 +0530
> # Node ID 65896c63c989065770781d7234d72c9f89a1de17
> # Parent f4c267f28487161fa78c43cabb30dc4f4f82570c
> cost: fix mode cost check
lol, I'm fine with removing the validation checks, but the commit
message is a bit misleading. You're not fixing anything, you're
removing debug sanity checks for variables being set and adding
unrelated safety checks of rdcost inputs.
again, the root problem here is that SSE distortion costs can overflow a
uint32_t in one SSE call in main12, or in multiple SSE calls in main10
(think Y + U + V). this is long before lambda2 * bits enters the
equation. This patch does nothing to address that.
> diff -r f4c267f28487 -r 65896c63c989 source/encoder/analysis.cpp
> --- a/source/encoder/analysis.cpp Mon Sep 28 13:38:33 2015 +0530
> +++ b/source/encoder/analysis.cpp Tue Sep 29 16:29:56 2015 +0530
> @@ -128,9 +128,6 @@
> m_frame = &frame;
>
> #if _DEBUG || CHECKED_BUILD
> - for (uint32_t i = 0; i <= g_maxCUDepth; i++)
> - for (uint32_t j = 0; j < MAX_PRED_TYPES; j++)
> - m_modeDepth[i].pred[j].invalidate();
> invalidateContexts(0);
> #endif
>
> @@ -806,7 +803,6 @@
> }
>
> /* Copy best data to encData CTU and recon */
> - X265_CHECK(md.bestMode->ok(), "best mode is not ok");
> md.bestMode->cu.copyToPic(depth);
> md.bestMode->reconYuv.copyToPicYuv(*m_frame->m_reconPic, cuAddr, cuGeom.absPartIdx);
>
> @@ -1169,7 +1165,6 @@
> }
>
> /* Copy best data to encData CTU and recon */
> - X265_CHECK(md.bestMode->ok(), "best mode is not ok");
> md.bestMode->cu.copyToPic(depth);
> if (m_param->rdLevel)
> md.bestMode->reconYuv.copyToPicYuv(reconPic, cuAddr, cuGeom.absPartIdx);
> @@ -1419,7 +1414,6 @@
> }
>
> /* Copy best data to encData CTU and recon */
> - X265_CHECK(md.bestMode->ok(), "best mode is not ok");
> md.bestMode->cu.copyToPic(depth);
> md.bestMode->reconYuv.copyToPicYuv(*m_frame->m_reconPic, parentCTU.m_cuAddr, cuGeom.absPartIdx);
>
> @@ -1532,7 +1526,6 @@
> md.bestMode->cu.setPURefIdx(0, (int8_t)candMvField[bestSadCand][0].refIdx, 0, 0);
> md.bestMode->cu.setPURefIdx(1, (int8_t)candMvField[bestSadCand][1].refIdx, 0, 0);
> checkDQP(*md.bestMode, cuGeom);
> - X265_CHECK(md.bestMode->ok(), "Merge mode not ok\n");
> }
>
> /* sets md.bestMode if a valid merge candidate is found, else leaves it NULL */
> @@ -1655,7 +1648,6 @@
> bestPred->cu.setPURefIdx(0, (int8_t)candMvField[bestCand][0].refIdx, 0, 0);
> bestPred->cu.setPURefIdx(1, (int8_t)candMvField[bestCand][1].refIdx, 0, 0);
> checkDQP(*bestPred, cuGeom);
> - X265_CHECK(bestPred->ok(), "merge mode is not ok");
> }
>
> if (m_param->analysisMode)
> diff -r f4c267f28487 -r 65896c63c989 source/encoder/analysis.h
> --- a/source/encoder/analysis.h Mon Sep 28 13:38:33 2015 +0530
> +++ b/source/encoder/analysis.h Tue Sep 29 16:29:56 2015 +0530
> @@ -147,8 +147,6 @@
> /* check whether current mode is the new best */
> inline void checkBestMode(Mode& mode, uint32_t depth)
> {
> - X265_CHECK(mode.ok(), "mode costs are uninitialized\n");
> -
> ModeDepth& md = m_modeDepth[depth];
> if (md.bestMode)
> {
> diff -r f4c267f28487 -r 65896c63c989 source/encoder/rdcost.h
> --- a/source/encoder/rdcost.h Mon Sep 28 13:38:33 2015 +0530
> +++ b/source/encoder/rdcost.h Tue Sep 29 16:29:56 2015 +0530
> @@ -118,6 +118,15 @@
> /* return the RD cost of this prediction, including the effect of psy-rd */
> inline uint64_t calcPsyRdCost(sse_ret_t distortion, uint32_t bits, uint32_t psycost) const
> {
> +#if X265_DEPTH <= 10
> + X265_CHECK((bits <= (UINT64_MAX / m_lambda2)) && (psycost <= UINT64_MAX / (m_lambda * m_psyRd)),
> + "calcPsyRdCost wrap detected dist: %u, bits: %u, lambda: " X265_LL ", lambda2: " X265_LL "\n",
> + distortion, bits, m_lambda, m_lambda2);
> +#else
> + X265_CHECK((bits <= (UINT64_MAX / m_lambda2)) && (psycost <= UINT64_MAX / (m_lambda * m_psyRd)),
> + "calcPsyRdCost wrap detected dist: " X265_LL ", bits: %u, lambda: " X265_LL ", lambda2: " X265_LL "\n",
> + distortion, bits, m_lambda, m_lambda2);
> +#endif
> return distortion + ((m_lambda * m_psyRd * psycost) >> 24) + ((bits * m_lambda2) >> 8);
> }
>
> @@ -144,6 +153,8 @@
>
> inline uint32_t getCost(uint32_t bits) const
> {
> + X265_CHECK(bits <= (UINT64_MAX - 128) / m_lambda,
> + "getCost wrap detected bits: %u, lambda: " X265_LL "\n", bits, m_lambda);
> return (uint32_t)((bits * m_lambda + 128) >> 8);
> }
> };
> diff -r f4c267f28487 -r 65896c63c989 source/encoder/search.cpp
> --- a/source/encoder/search.cpp Mon Sep 28 13:38:33 2015 +0530
> +++ b/source/encoder/search.cpp Tue Sep 29 16:29:56 2015 +0530
> @@ -1365,7 +1365,6 @@
> intraMode.distortion = bsad;
> intraMode.sa8dCost = bcost;
> intraMode.sa8dBits = bbits;
> - X265_CHECK(intraMode.ok(), "intra mode is not ok");
> }
>
> void Search::encodeIntraInInter(Mode& intraMode, const CUGeom& cuGeom)
> @@ -2381,7 +2380,6 @@
>
> motionCompensation(cu, pu, *predYuv, true, bChromaMC);
> }
> - X265_CHECK(interMode.ok(), "inter mode is not ok");
> interMode.sa8dBits += totalmebits;
> }
>
> diff -r f4c267f28487 -r 65896c63c989 source/encoder/search.h
> --- a/source/encoder/search.h Mon Sep 28 13:38:33 2015 +0530
> +++ b/source/encoder/search.h Tue Sep 29 16:29:56 2015 +0530
> @@ -133,62 +133,8 @@
> coeffBits = 0;
> }
>
> - void invalidate()
> - {
> - /* set costs to invalid data, catch uninitialized re-use */
> - rdCost = UINT64_MAX / 2;
> - sa8dCost = UINT64_MAX / 2;
> - sa8dBits = MAX_UINT / 2;
> - psyEnergy = MAX_UINT / 2;
> -#if X265_DEPTH <= 10
> - resEnergy = MAX_UINT / 2;
> - lumaDistortion = MAX_UINT / 2;
> - chromaDistortion = MAX_UINT / 2;
> - distortion = MAX_UINT / 2;
> -#else
> - resEnergy = UINT64_MAX / 2;
> - lumaDistortion = UINT64_MAX / 2;
> - chromaDistortion = UINT64_MAX / 2;
> - distortion = UINT64_MAX / 2;
> -#endif
> - totalBits = MAX_UINT / 2;
> - mvBits = MAX_UINT / 2;
> - coeffBits = MAX_UINT / 2;
> - }
> -
> - bool ok() const
> - {
> -#if X265_DEPTH <= 10
> - return !(rdCost >= UINT64_MAX / 2 ||
> - sa8dCost >= UINT64_MAX / 2 ||
> - sa8dBits >= MAX_UINT / 2 ||
> - psyEnergy >= MAX_UINT / 2 ||
> - resEnergy >= MAX_UINT / 2 ||
> - lumaDistortion >= MAX_UINT / 2 ||
> - chromaDistortion >= MAX_UINT / 2 ||
> - distortion >= MAX_UINT / 2 ||
> - totalBits >= MAX_UINT / 2 ||
> - mvBits >= MAX_UINT / 2 ||
> - coeffBits >= MAX_UINT / 2);
> -#else
> - return !(rdCost >= UINT64_MAX / 2 ||
> - sa8dCost >= UINT64_MAX / 2 ||
> - sa8dBits >= MAX_UINT / 2 ||
> - psyEnergy >= MAX_UINT / 2 ||
> - resEnergy >= UINT64_MAX / 2 ||
> - lumaDistortion >= UINT64_MAX / 2 ||
> - chromaDistortion >= UINT64_MAX / 2 ||
> - distortion >= UINT64_MAX / 2 ||
> - totalBits >= MAX_UINT / 2 ||
> - mvBits >= MAX_UINT / 2 ||
> - coeffBits >= MAX_UINT / 2);
> -#endif
> - }
> -
> void addSubCosts(const Mode& subMode)
> {
> - X265_CHECK(subMode.ok(), "sub-mode not initialized");
> -
> rdCost += subMode.rdCost;
> sa8dCost += subMode.sa8dCost;
> sa8dBits += subMode.sa8dBits;
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
--
Steve Borho
More information about the x265-devel
mailing list