<div dir="ltr"><div class="gmail_extra">Agreed, and the averaging should be weighted by CU block size. <br><br></div><div class="gmail_extra"><div class="gmail_quote">On Sun, Apr 26, 2015 at 11:17 PM, 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 Sun, Apr 26, 2015 at 12:21 PM, Steve Borho <<a href="mailto:steve@borho.org">steve@borho.org</a>> wrote:<br>
> # HG changeset patch<br>
> # User Steve Borho <<a href="mailto:steve@borho.org">steve@borho.org</a>><br>
> # Date 1429943995 18000<br>
> # Sat Apr 25 01:39:55 2015 -0500<br>
> # Node ID 68a13226d586b335c02cade9311e093f0149c42a<br>
> # Parent 6a0a37c01cff03cadd44691a0fe447d17ec0b14f<br>
> analysis: always configure quant QP directly after setting RD lambda<br>
><br>
> Basically, everywhere we adjust or assign QP we set quant QP immediately. This<br>
> removes a great many ad-hoc calls to setQPforQuant() and hopefully makes it<br>
> impossible to miss quant being configured properly.<br>
><br>
> This patch fixes a layering violation where the frame encoder was setting the<br>
> RDO lambdas directly, but only when delta-QP was not enabled.<br>
><br>
> diff -r 6a0a37c01cff -r 68a13226d586 source/common/quant.cpp<br>
> --- a/source/common/quant.cpp Sat Apr 25 00:39:48 2015 -0500<br>
> +++ b/source/common/quant.cpp Sat Apr 25 01:39:55 2015 -0500<br>
> @@ -225,16 +225,15 @@<br>
> X265_FREE(m_fencShortBuf);<br>
> }<br>
><br>
> -void Quant::setQPforQuant(const CUData& cu)<br>
> +void Quant::setQPforQuant(const CUData& ctu, int qp)<br>
> {<br>
> - m_tqBypass = !!cu.m_tqBypass[0];<br>
> + m_tqBypass = !!ctu.m_tqBypass[0];<br>
> if (m_tqBypass)<br>
> return;<br>
> - m_nr = m_frameNr ? &m_frameNr[cu.m_encData->m_frameEncoderID] : NULL;<br>
> - int qpy = cu.m_qp[0];<br>
> - m_qpParam[TEXT_LUMA].setQpParam(qpy + QP_BD_OFFSET);<br>
> - setChromaQP(qpy + cu.m_slice->m_pps->chromaQpOffset[0], TEXT_CHROMA_U, cu.m_chromaFormat);<br>
> - setChromaQP(qpy + cu.m_slice->m_pps->chromaQpOffset[1], TEXT_CHROMA_V, cu.m_chromaFormat);<br>
> + m_nr = m_frameNr ? &m_frameNr[ctu.m_encData->m_frameEncoderID] : NULL;<br>
> + m_qpParam[TEXT_LUMA].setQpParam(qp + QP_BD_OFFSET);<br>
> + setChromaQP(qp + ctu.m_slice->m_pps->chromaQpOffset[0], TEXT_CHROMA_U, ctu.m_chromaFormat);<br>
> + setChromaQP(qp + ctu.m_slice->m_pps->chromaQpOffset[1], TEXT_CHROMA_V, ctu.m_chromaFormat);<br>
> }<br>
><br>
> void Quant::setChromaQP(int qpin, TextType ttype, int chFmt)<br>
> diff -r 6a0a37c01cff -r 68a13226d586 source/common/quant.h<br>
> --- a/source/common/quant.h Sat Apr 25 00:39:48 2015 -0500<br>
> +++ b/source/common/quant.h Sat Apr 25 01:39:55 2015 -0500<br>
> @@ -103,7 +103,7 @@<br>
> bool allocNoiseReduction(const x265_param& param);<br>
><br>
> /* CU setup */<br>
> - void setQPforQuant(const CUData& cu);<br>
> + void setQPforQuant(const CUData& ctu, int qp);<br>
><br>
> uint32_t transformNxN(const CUData& cu, const pixel* fenc, uint32_t fencStride, const int16_t* residual, uint32_t resiStride, coeff_t* coeff,<br>
> uint32_t log2TrSize, TextType ttype, uint32_t absPartIdx, bool useTransformSkip);<br>
> diff -r 6a0a37c01cff -r 68a13226d586 source/encoder/analysis.cpp<br>
> --- a/source/encoder/analysis.cpp Sat Apr 25 00:39:48 2015 -0500<br>
> +++ b/source/encoder/analysis.cpp Sat Apr 25 01:39:55 2015 -0500<br>
> @@ -146,18 +146,16 @@<br>
><br>
> if (m_slice->m_pps->bUseDQP)<br>
> {<br>
> - m_aqQP[0] = calculateQpforCuSize(ctu, cuGeom);<br>
> - setLambdaFromQP(*m_slice, m_aqQP[0]);<br>
> - m_aqQP[0] = x265_clip3(QP_MIN, QP_MAX_SPEC, m_aqQP[0]);<br>
> - ctu.setQPSubParts((int8_t)m_aqQP[0], 0, 0);<br>
> + m_aqQP[0] = setLambdaFromQP(ctu, calculateQpforCuSize(ctu, cuGeom));<br>
><br>
> if (m_slice->m_pps->maxCuDQPDepth)<br>
> initAqQPs(1, ctu, &cuGeom + 1);<br>
> }<br>
> else<br>
> - m_aqQP[0] = m_slice->m_sliceQp;<br>
> + /* adaptive quant disabled, CTU QP is always slice QP, and within spec range */<br>
> + m_aqQP[0] = setLambdaFromQP(ctu, m_slice->m_sliceQp);<br>
><br>
> - m_quant.setQPforQuant(ctu);<br>
> + ctu.setQPSubParts((int8_t)m_aqQP[0], 0, 0);<br>
> m_rqt[0].cur.load(initialContext);<br>
> m_modeDepth[0].fencYuv.copyFromPicYuv(*m_frame->m_fencPic, ctu.m_cuAddr, 0);<br>
><br>
> @@ -231,20 +229,24 @@<br>
> return;<br>
> else if (md.bestMode->cu.isIntra(0))<br>
> {<br>
> + m_quant.m_tqBypass = true;<br>
> md.pred[PRED_LOSSLESS].initCosts();<br>
> md.pred[PRED_LOSSLESS].cu.initLosslessCU(md.bestMode->cu, cuGeom);<br>
> PartSize size = (PartSize)md.pred[PRED_LOSSLESS].cu.m_partSize[0];<br>
> uint8_t* modes = md.pred[PRED_LOSSLESS].cu.m_lumaIntraDir;<br>
> checkIntra(md.pred[PRED_LOSSLESS], cuGeom, size, modes, NULL);<br>
> checkBestMode(md.pred[PRED_LOSSLESS], cuGeom.depth);<br>
> + m_quant.m_tqBypass = false;<br>
> }<br>
> else<br>
> {<br>
> + m_quant.m_tqBypass = true;<br>
> md.pred[PRED_LOSSLESS].initCosts();<br>
> md.pred[PRED_LOSSLESS].cu.initLosslessCU(md.bestMode->cu, cuGeom);<br>
> md.pred[PRED_LOSSLESS].predYuv.copyFromYuv(md.bestMode->predYuv);<br>
> encodeResAndCalcRdInterCU(md.pred[PRED_LOSSLESS], cuGeom);<br>
> checkBestMode(md.pred[PRED_LOSSLESS], cuGeom.depth);<br>
> + m_quant.m_tqBypass = false;<br>
> }<br>
> }<br>
><br>
> @@ -269,7 +271,6 @@<br>
> PartSize size = (PartSize)reusePartSizes[zOrder];<br>
> Mode& mode = size == SIZE_2Nx2N ? md.pred[PRED_INTRA] : md.pred[PRED_INTRA_NxN];<br>
> mode.cu.initSubCU(parentCTU, cuGeom, qp);<br>
> - m_quant.setQPforQuant(<a href="http://mode.cu" target="_blank">mode.cu</a>);<br>
> checkIntra(mode, cuGeom, size, &reuseModes[zOrder], &reuseChromaModes[zOrder]);<br>
> checkBestMode(mode, depth);<br>
><br>
> @@ -287,7 +288,6 @@<br>
> else if (mightNotSplit)<br>
> {<br>
> md.pred[PRED_INTRA].cu.initSubCU(parentCTU, cuGeom, qp);<br>
> - m_quant.setQPforQuant(md.pred[PRED_INTRA].cu);<br>
> checkIntra(md.pred[PRED_INTRA], cuGeom, SIZE_2Nx2N, NULL, NULL);<br>
> checkBestMode(md.pred[PRED_INTRA], depth);<br>
><br>
> @@ -327,11 +327,7 @@<br>
> m_rqt[nextDepth].cur.load(*nextContext);<br>
><br>
> if (m_slice->m_pps->bUseDQP && nextDepth <= m_slice->m_pps->maxCuDQPDepth)<br>
> - {<br>
> - nextQP = m_aqQP[childGeom.index];<br>
> - setLambdaFromQP(*m_slice, nextQP);<br>
> - nextQP = x265_clip3(QP_MIN, QP_MAX_SPEC, nextQP);<br>
> - }<br>
> + nextQP = setLambdaFromQP(parentCTU, m_aqQP[childGeom.index]);<br>
><br>
> compressIntraCU(parentCTU, childGeom, zOrder, nextQP);<br>
><br>
> @@ -401,14 +397,9 @@<br>
> {<br>
> slave.m_slice = m_slice;<br>
> slave.m_frame = m_frame;<br>
> - slave.setLambdaFromQP(*m_slice, m_rdCost.m_qp);<br>
> + slave.setLambdaFromQP(md.pred[PRED_2Nx2N].cu, m_rdCost.m_qp);<br>
> slave.invalidateContexts(0);<br>
> -<br>
> - if (m_param->rdLevel >= 5)<br>
> - {<br>
> - slave.m_rqt[pmode.cuGeom.depth].cur.load(m_rqt[pmode.cuGeom.depth].cur);<br>
> - slave.m_quant.setQPforQuant(md.pred[PRED_2Nx2N].cu);<br>
> - }<br>
> + slave.m_rqt[pmode.cuGeom.depth].cur.load(m_rqt[pmode.cuGeom.depth].cur);<br>
> }<br>
><br>
> /* perform Mode task, repeat until no more work is available */<br>
> @@ -419,11 +410,6 @@<br>
> switch (pmode.modes[task])<br>
> {<br>
> case PRED_INTRA:<br>
> - if (&slave != this)<br>
> - {<br>
> - slave.m_rqt[pmode.cuGeom.depth].cur.load(m_rqt[pmode.cuGeom.depth].cur);<br>
> - slave.m_quant.setQPforQuant(md.pred[PRED_INTRA].cu);<br>
> - }<br>
> slave.checkIntraInInter(md.pred[PRED_INTRA], pmode.cuGeom);<br>
> if (m_param->rdLevel > 2)<br>
> slave.encodeIntraInInter(md.pred[PRED_INTRA], pmode.cuGeom);<br>
> @@ -739,11 +725,7 @@<br>
> m_rqt[nextDepth].cur.load(*nextContext);<br>
><br>
> if (m_slice->m_pps->bUseDQP && nextDepth <= m_slice->m_pps->maxCuDQPDepth)<br>
> - {<br>
> - nextQP = m_aqQP[childGeom.index];<br>
> - setLambdaFromQP(*m_slice, nextQP);<br>
> - nextQP = x265_clip3(QP_MIN, QP_MAX_SPEC, nextQP);<br>
> - }<br>
> + nextQP = setLambdaFromQP(parentCTU, m_aqQP[childGeom.index]);<br>
><br>
> compressInterCU_dist(parentCTU, childGeom, nextQP);<br>
><br>
> @@ -944,7 +926,6 @@<br>
> {<br>
> /* generate recon pixels with no rate distortion considerations */<br>
> CUData& cu = md.bestMode->cu;<br>
> - m_quant.setQPforQuant(cu);<br>
><br>
> uint32_t tuDepthRange[2];<br>
> cu.getInterTUQtDepthRange(tuDepthRange, 0);<br>
> @@ -969,7 +950,6 @@<br>
> {<br>
> /* generate recon pixels with no rate distortion considerations */<br>
> CUData& cu = md.bestMode->cu;<br>
> - m_quant.setQPforQuant(cu);<br>
><br>
> uint32_t tuDepthRange[2];<br>
> cu.getIntraTUQtDepthRange(tuDepthRange, 0);<br>
> @@ -1020,11 +1000,7 @@<br>
> m_rqt[nextDepth].cur.load(*nextContext);<br>
><br>
> if (m_slice->m_pps->bUseDQP && nextDepth <= m_slice->m_pps->maxCuDQPDepth)<br>
> - {<br>
> - nextQP = m_aqQP[childGeom.index];<br>
> - setLambdaFromQP(*m_slice, nextQP);<br>
> - nextQP = x265_clip3(QP_MIN, QP_MAX_SPEC, nextQP);<br>
> - }<br>
> + nextQP = setLambdaFromQP(parentCTU, m_aqQP[childGeom.index]);<br>
><br>
> compressInterCU_rd0_4(parentCTU, childGeom, nextQP);<br>
><br>
> @@ -1228,11 +1204,7 @@<br>
> m_rqt[nextDepth].cur.load(*nextContext);<br>
><br>
> if (m_slice->m_pps->bUseDQP && nextDepth <= m_slice->m_pps->maxCuDQPDepth)<br>
> - {<br>
> - nextQP = m_aqQP[childGeom.index];<br>
> - setLambdaFromQP(*m_slice, nextQP);<br>
> - nextQP = x265_clip3(QP_MIN, QP_MAX_SPEC, nextQP);<br>
> - }<br>
> + nextQP = setLambdaFromQP(parentCTU, m_aqQP[childGeom.index]);<br>
><br>
> compressInterCU_rd5_6(parentCTU, childGeom, zOrder, nextQP);<br>
><br>
> @@ -1758,7 +1730,6 @@<br>
> CUData& cu = bestMode->cu;<br>
><br>
> cu.copyFromPic(ctu, cuGeom);<br>
> - m_quant.setQPforQuant(cu);<br>
><br>
> Yuv& fencYuv = m_modeDepth[cuGeom.depth].fencYuv;<br>
> if (cuGeom.depth)<br>
> diff -r 6a0a37c01cff -r 68a13226d586 source/encoder/frameencoder.cpp<br>
> --- a/source/encoder/frameencoder.cpp Sat Apr 25 00:39:48 2015 -0500<br>
> +++ b/source/encoder/frameencoder.cpp Sat Apr 25 01:39:55 2015 -0500<br>
> @@ -851,12 +851,11 @@<br>
><br>
> if (m_param->rc.aqMode || bIsVbv)<br>
> {<br>
> + X265_CHECK(slice->m_pps->bUseDQP, "adaptive quant in use without DQP\n");<br>
> int qp = calcQpForCu(cuAddr, curEncData.m_cuStat[cuAddr].baseQp);<br>
> qp = x265_clip3(QP_MIN, QP_MAX_SPEC, qp);<br>
> curEncData.m_rowStat[row].sumQpAq += qp;<br>
> }<br>
> - else<br>
> - tld.analysis.setLambdaFromQP(*slice, slice->m_sliceQp);<br>
<br>
</div></div>Note that this change raises the question of whether calcQpForCu() is<br>
necessary any more in the frame encoder. The returned QP is only used<br>
to update the row sum QP. compressCTU() is free to use an entirely<br>
different QP (or QPs per CU).<br>
<br>
My guess is that rate control would work better if sumQpAq were<br>
incremented *after* the CTU was compressed, based on the average of<br>
the QPs actually used to code the block. And this would also be more<br>
work efficient.<br>
<div class="HOEnZb"><div class="h5"><br>
> if (m_param->bEnableWavefront && !col && row)<br>
> {<br>
> diff -r 6a0a37c01cff -r 68a13226d586 source/encoder/rdcost.h<br>
> --- a/source/encoder/rdcost.h Sat Apr 25 00:39:48 2015 -0500<br>
> +++ b/source/encoder/rdcost.h Sat Apr 25 01:39:55 2015 -0500<br>
> @@ -40,12 +40,13 @@<br>
> uint32_t m_chromaDistWeight[2];<br>
> uint32_t m_psyRdBase;<br>
> uint32_t m_psyRd;<br>
> - int m_qp;<br>
> + int m_qp; /* QP used to configure lambda, may be higher than QP_MAX_SPEC but <= QP_MAX_MAX */<br>
><br>
> void setPsyRdScale(double scale) { m_psyRdBase = (uint32_t)floor(65536.0 * scale * 0.33); }<br>
><br>
> void setQP(const Slice& slice, int qp)<br>
> {<br>
> + x265_emms(); /* TODO: if the lambda tables were ints, this would not be necessary */<br>
> m_qp = qp;<br>
><br>
> /* Scale PSY RD factor by a slice type factor */<br>
> diff -r 6a0a37c01cff -r 68a13226d586 source/encoder/search.cpp<br>
> --- a/source/encoder/search.cpp Sat Apr 25 00:39:48 2015 -0500<br>
> +++ b/source/encoder/search.cpp Sat Apr 25 01:39:55 2015 -0500<br>
> @@ -163,11 +163,16 @@<br>
> X265_FREE(m_tsRecon);<br>
> }<br>
><br>
> -void Search::setLambdaFromQP(const Slice& slice, int qp)<br>
> +int Search::setLambdaFromQP(const CUData& ctu, int qp)<br>
> {<br>
> - x265_emms(); /* TODO: if the lambda tables were ints, this would not be necessary */<br>
> + X265_CHECK(qp >= QP_MIN && qp <= QP_MAX_MAX, "QP used for lambda is out of range\n");<br>
> +<br>
> m_me.setQP(qp);<br>
> - m_rdCost.setQP(slice, qp);<br>
> + m_rdCost.setQP(*m_slice, qp);<br>
> +<br>
> + int quantQP = x265_clip3(QP_MIN, QP_MAX_SPEC, qp);<br>
> + m_quant.setQPforQuant(ctu, quantQP);<br>
> + return quantQP;<br>
> }<br>
><br>
> #if CHECKED_BUILD || _DEBUG<br>
> @@ -1364,8 +1369,6 @@<br>
> X265_CHECK(cu.m_partSize[0] == SIZE_2Nx2N, "encodeIntraInInter does not expect NxN intra\n");<br>
> X265_CHECK(!m_slice->isIntra(), "encodeIntraInInter does not expect to be used in I slices\n");<br>
><br>
> - m_quant.setQPforQuant(cu);<br>
> -<br>
> uint32_t tuDepthRange[2];<br>
> cu.getIntraTUQtDepthRange(tuDepthRange, 0);<br>
><br>
> @@ -1888,10 +1891,9 @@<br>
> /* Setup slave Search instance for ME for master's CU */<br>
> if (&slave != this)<br>
> {<br>
> - slave.setLambdaFromQP(*m_slice, m_rdCost.m_qp);<br>
> slave.m_slice = m_slice;<br>
> slave.m_frame = m_frame;<br>
> -<br>
> + slave.setLambdaFromQP(<a href="http://pme.mode.cu" target="_blank">pme.mode.cu</a>, m_rdCost.m_qp);<br>
> slave.m_me.setSourcePU(*pme.mode.fencYuv, pme.pu.ctuAddr, pme.pu.cuAbsPartIdx, pme.pu.puAbsPartIdx, pme.pu.width, pme.pu.height);<br>
> }<br>
><br>
> @@ -2523,9 +2525,6 @@<br>
> uint32_t log2CUSize = cuGeom.log2CUSize;<br>
> int sizeIdx = log2CUSize - 2;<br>
><br>
> - uint32_t tqBypass = cu.m_tqBypass[0];<br>
> - m_quant.setQPforQuant(interMode.cu);<br>
> -<br>
> resiYuv->subtract(*fencYuv, *predYuv, log2CUSize);<br>
><br>
> uint32_t tuDepthRange[2];<br>
> @@ -2536,6 +2535,7 @@<br>
> Cost costs;<br>
> estimateResidualQT(interMode, cuGeom, 0, 0, *resiYuv, costs, tuDepthRange);<br>
><br>
> + uint32_t tqBypass = cu.m_tqBypass[0];<br>
> if (!tqBypass)<br>
> {<br>
> uint32_t cbf0Dist = <a href="http://primitives.cu" target="_blank">primitives.cu</a>[sizeIdx].sse_pp(fencYuv->m_buf[0], fencYuv->m_size, predYuv->m_buf[0], predYuv->m_size);<br>
> diff -r 6a0a37c01cff -r 68a13226d586 source/encoder/search.h<br>
> --- a/source/encoder/search.h Sat Apr 25 00:39:48 2015 -0500<br>
> +++ b/source/encoder/search.h Sat Apr 25 01:39:55 2015 -0500<br>
> @@ -287,7 +287,7 @@<br>
> ~Search();<br>
><br>
> bool initSearch(const x265_param& param, ScalingList& scalingList);<br>
> - void setLambdaFromQP(const Slice& slice, int qp);<br>
> + int setLambdaFromQP(const CUData& ctu, int qp); /* returns real quant QP in valid spec range */<br>
><br>
> // mark temp RD entropy contexts as uninitialized; useful for finding loads without stores<br>
> void invalidateContexts(int fromDepth);<br>
<br>
<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Steve Borho<br>
</font></span><div class="HOEnZb"><div class="h5">_______________________________________________<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>
</div></div></blockquote></div><br></div></div>