[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