[x265] [PATCH] stats: fix use of uninitialized pixels in maximum and average luma level calculation
Deepthi Nandakumar
deepthi at multicorewareinc.com
Fri Aug 14 05:44:29 CEST 2015
I did a double take on the if (padx) part as well, then figured it works
since padx is guaranteed to be non-zero (because of padx++).
Min, can you do a quick profile on whether it is better to do this outside
the padx loop, for the whole frame, or inside the padx loop, one row at a
time?
On Thu, Aug 13, 2015 at 6:19 PM, Steve Borho <steve at borho.org> wrote:
> On 08/13, kavitha at multicorewareinc.com wrote:
> > # HG changeset patch
> > # User Kavitha Sampath <kavitha at multicorewareinc.com>
> > # Date 1439458769 -19800
> > # Thu Aug 13 15:09:29 2015 +0530
> > # Node ID 27226b1c447f4df9cde5b7688f34fb8b7a86e193
> > # Parent bc5a7c2ac38b06d2a232b983f10bc0394d252ad7
> > stats: fix use of uninitialized pixels in maximum and average luma level
> calculation
> >
> > To avoid complexity in determining pixels of border CUs that lie outside
> the frame,
> > luma level calculations are performed at the frame level, immediately
> after
> > copying the picture into PicYuv
> >
> > diff -r bc5a7c2ac38b -r 27226b1c447f source/common/framedata.h
> > --- a/source/common/framedata.h Wed Aug 12 15:13:51 2015 +0530
> > +++ b/source/common/framedata.h Thu Aug 13 15:09:29 2015 +0530
> > @@ -55,8 +55,6 @@
> > double avgLumaDistortion;
> > double avgChromaDistortion;
> > double avgPsyEnergy;
> > - double avgLumaLevel;
> > - double lumaLevel;
> > double percentIntraNxN;
> > double percentSkipCu[NUM_CU_DEPTH];
> > double percentMergeCu[NUM_CU_DEPTH];
> > @@ -75,7 +73,6 @@
> > uint64_t cntIntra[NUM_CU_DEPTH];
> > uint64_t cuInterDistribution[NUM_CU_DEPTH][INTER_MODES];
> > uint64_t cuIntraDistribution[NUM_CU_DEPTH][INTRA_MODES];
> > - uint16_t maxLumaLevel;
> >
> > FrameStats()
> > {
> > diff -r bc5a7c2ac38b -r 27226b1c447f source/common/picyuv.cpp
> > --- a/source/common/picyuv.cpp Wed Aug 12 15:13:51 2015 +0530
> > +++ b/source/common/picyuv.cpp Thu Aug 13 15:09:29 2015 +0530
> > @@ -42,6 +42,9 @@
> > m_cuOffsetC = NULL;
> > m_buOffsetY = NULL;
> > m_buOffsetC = NULL;
> > +
> > + m_maxLumaLevel = 0;
> > + m_avgLumaLevel = 0;
> > }
> >
> > bool PicYuv::create(uint32_t picWidth, uint32_t picHeight, uint32_t
> picCsp)
> > @@ -235,17 +238,25 @@
> > pixel *U = m_picOrg[1];
> > pixel *V = m_picOrg[2];
>
>
> > + uint64_t sumLuma = 0;
> > for (int r = 0; r < height; r++)
> > {
> > - /* Clip luma of source picture to max and min values before
> extending edges of picYuv */
> > - for (int c = 0; c < m_stride; c++)
> > + for (int c = 0; c < width; c++)
> > + {
> > + /* Clip luma of source picture to max and min values
> before extending edges of picYuv */
> > Y[c] = x265_clip3((pixel)param.minLuma,
> (pixel)param.maxLuma, Y[c]);
> >
> > + /* Determine maximum and average luma level in a
> picture */
> > + m_maxLumaLevel = X265_MAX(Y[c], m_maxLumaLevel);
> > + sumLuma += Y[c];
> > + }
>
> this whole nested for-loop needs to be moved out of the if (padx)
> conditional expression
>
> > +
> > for (int x = 0; x < padx; x++)
> > Y[width + x] = Y[width - 1];
> >
> > Y += m_stride;
> > }
> > + m_avgLumaLevel = (double)(sumLuma) / (m_picHeight * m_picWidth);
> >
> > for (int r = 0; r < height >> m_vChromaShift; r++)
> > {
> > diff -r bc5a7c2ac38b -r 27226b1c447f source/common/picyuv.h
> > --- a/source/common/picyuv.h Wed Aug 12 15:13:51 2015 +0530
> > +++ b/source/common/picyuv.h Thu Aug 13 15:09:29 2015 +0530
> > @@ -60,6 +60,9 @@
> > uint32_t m_chromaMarginX;
> > uint32_t m_chromaMarginY;
> >
> > + uint16_t m_maxLumaLevel;
> > + double m_avgLumaLevel;
> > +
> > PicYuv();
> >
> > bool create(uint32_t picWidth, uint32_t picHeight, uint32_t csp);
> > diff -r bc5a7c2ac38b -r 27226b1c447f source/encoder/encoder.cpp
> > --- a/source/encoder/encoder.cpp Wed Aug 12 15:13:51 2015 +0530
> > +++ b/source/encoder/encoder.cpp Thu Aug 13 15:09:29 2015 +0530
> > @@ -1163,8 +1163,8 @@
> > frameStats->avgChromaDistortion =
> curFrame->m_encData->m_frameStats.avgChromaDistortion;
> > frameStats->avgLumaDistortion =
> curFrame->m_encData->m_frameStats.avgLumaDistortion;
> > frameStats->avgPsyEnergy =
> curFrame->m_encData->m_frameStats.avgPsyEnergy;
> > - frameStats->avgLumaLevel =
> curFrame->m_encData->m_frameStats.avgLumaLevel;
> > - frameStats->maxLumaLevel =
> curFrame->m_encData->m_frameStats.maxLumaLevel;
> > + frameStats->avgLumaLevel =
> curFrame->m_fencPic->m_avgLumaLevel;
> > + frameStats->maxLumaLevel =
> curFrame->m_fencPic->m_maxLumaLevel;
> > for (uint32_t depth = 0; depth <= g_maxCUDepth; depth++)
> > {
> > frameStats->cuStats.percentSkipCu[depth] =
> curFrame->m_encData->m_frameStats.percentSkipCu[depth];
> > diff -r bc5a7c2ac38b -r 27226b1c447f source/encoder/frameencoder.cpp
> > --- a/source/encoder/frameencoder.cpp Wed Aug 12 15:13:51 2015 +0530
> > +++ b/source/encoder/frameencoder.cpp Thu Aug 13 15:09:29 2015 +0530
> > @@ -592,10 +592,7 @@
> > m_frame->m_encData->m_frameStats.lumaDistortion +=
> m_rows[i].rowStats.lumaDistortion;
> > m_frame->m_encData->m_frameStats.chromaDistortion +=
> m_rows[i].rowStats.chromaDistortion;
> > m_frame->m_encData->m_frameStats.psyEnergy +=
> m_rows[i].rowStats.psyEnergy;
> > - m_frame->m_encData->m_frameStats.lumaLevel +=
> m_rows[i].rowStats.lumaLevel;
> >
> > - if (m_rows[i].rowStats.maxLumaLevel >
> m_frame->m_encData->m_frameStats.maxLumaLevel)
> > - m_frame->m_encData->m_frameStats.maxLumaLevel =
> m_rows[i].rowStats.maxLumaLevel;
> > for (uint32_t depth = 0; depth <= g_maxCUDepth; depth++)
> > {
> > m_frame->m_encData->m_frameStats.cntSkipCu[depth] +=
> m_rows[i].rowStats.cntSkipCu[depth];
> > @@ -609,7 +606,6 @@
> > m_frame->m_encData->m_frameStats.avgLumaDistortion =
> (double)(m_frame->m_encData->m_frameStats.lumaDistortion /
> m_frame->m_encData->m_frameStats.totalCtu);
> > m_frame->m_encData->m_frameStats.avgChromaDistortion =
> (double)(m_frame->m_encData->m_frameStats.chromaDistortion /
> m_frame->m_encData->m_frameStats.totalCtu);
> > m_frame->m_encData->m_frameStats.avgPsyEnergy =
> (double)(m_frame->m_encData->m_frameStats.psyEnergy /
> m_frame->m_encData->m_frameStats.totalCtu);
> > - m_frame->m_encData->m_frameStats.avgLumaLevel =
> (double)(m_frame->m_encData->m_frameStats.lumaLevel /
> m_frame->m_encData->m_frameStats.totalCtu);
> > m_frame->m_encData->m_frameStats.percentIntraNxN =
> (double)(m_frame->m_encData->m_frameStats.cntIntraNxN * 100) /
> m_frame->m_encData->m_frameStats.totalCu;
> > for (uint32_t depth = 0; depth <= g_maxCUDepth; depth++)
> > {
> > @@ -990,17 +986,6 @@
> > curRow.rowStats.cuIntraDistribution[depth][n] +=
> frameLog.cuIntraDistribution[depth][n];
> > }
> >
> > - /* calculate maximum and average luma levels */
> > - uint32_t ctuLumaLevel = 0;
> > - uint32_t ctuNoOfPixels = best.fencYuv->m_size *
> best.fencYuv->m_size;
> > - for (uint32_t i = 0; i < ctuNoOfPixels; i++)
> > - {
> > - pixel p = best.fencYuv->m_buf[0][i];
> > - ctuLumaLevel += p;
> > - curRow.rowStats.maxLumaLevel = X265_MAX(p,
> curRow.rowStats.maxLumaLevel);
> > - }
> > - curRow.rowStats.lumaLevel += (double)(ctuLumaLevel) /
> ctuNoOfPixels;
> > -
> > curEncData.m_cuStat[cuAddr].totalBits = best.totalBits;
> > x265_emms();
> >
> > _______________________________________________
> > x265-devel mailing list
> > x265-devel at videolan.org
> > https://mailman.videolan.org/listinfo/x265-devel
>
> --
> Steve Borho
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20150814/ecf45c64/attachment.html>
More information about the x265-devel
mailing list