[x265] [PATCH] rc: fixes inconsistent output in linux because of RC Lock issue in CQP/CRF

[Deepthi Nandakumar]

We discussed this, and validated that the inconsistency was due to the
slice context initializations. 2 FEs do ratecontrolStart in the right order
but slice context initializations in the wrong order. So the right fix to
this is to increment the counter after the slice context inits.
On Jun 27, 2015 12:45 AM, "Steve Borho" <steve at borho.org> wrote:

> On 06/26, aarthi at multicorewareinc.com wrote:
> > # HG changeset patch
> > # User Aarthi Thirumalai
> > # Date 1434973816 -19800
> > #      Mon Jun 22 17:20:16 2015 +0530
> > # Node ID 1c6744174bfcfc031492c041243e1d9a9c02a5cd
> > # Parent  1e5c4d155ab85e8e8dd199bb3515801766ea9e88
> > rc: fixes inconsistent output in linux because of RC Lock issue in
> CQP/CRF
>
> It's not clear why this is a race-hazard. Is there something in rate
> control that must be protected, or is this just covering up for
> insufficient amounts of ref-lag? What is the race hazard?
>
> It's a bit of a layering violation to be mucking about
> with rate-control internal here, but that can be dealt with in a
> follow-up patch.
>
> > diff -r 1e5c4d155ab8 -r 1c6744174bfc source/encoder/frameencoder.cpp
> > --- a/source/encoder/frameencoder.cpp Thu Jun 25 13:42:29 2015 +0530
> > +++ b/source/encoder/frameencoder.cpp Mon Jun 22 17:20:16 2015 +0530
> > @@ -1056,18 +1056,26 @@
> >              rowCount = X265_MIN((m_numRows + 1) / 2, m_numRows - 1);
> >          else
> >              rowCount = X265_MIN(m_refLagRows, m_numRows - 1);
> > -        if (row == rowCount)
> > +    }
> > +    if (row == rowCount)
> > +    {
> > +        m_rce.rowTotalBits = 0;
> > +        if (m_param->rc.rateControlMode == X265_RC_ABR || bIsVbv)
> >          {
> > -            m_rce.rowTotalBits = 0;
> >              if (bIsVbv)
> >                  for (uint32_t i = 0; i < rowCount; i++)
> >                      m_rce.rowTotalBits +=
> curEncData.m_rowStat[i].encodedBits;
> >              else
> >                  for (uint32_t cuAddr = 0; cuAddr < rowCount * numCols;
> cuAddr++)
> >                      m_rce.rowTotalBits +=
> curEncData.m_cuStat[cuAddr].totalBits;
> > -
> >              m_top->m_rateControl->rateControlUpdateStats(&m_rce);
> >          }
> > +        /* do not allow the next frame to enter rateControlStart()
> until this
> > +         * frame has updated its mid-frame statistics */
> > +        m_top->m_rateControl->m_startEndOrder.incr();
> > +
> > +        if (m_rce.encodeOrder < m_param->frameNumThreads - 1)
> > +            m_top->m_rateControl->m_startEndOrder.incr(); // faked
> rateControlEnd calls for negative frames
> >      }
> >
> >      /* flush row bitstream (if WPP and no SAO) or flush frame if no WPP
> and no SAO */
> > diff -r 1e5c4d155ab8 -r 1c6744174bfc source/encoder/ratecontrol.cpp
> > --- a/source/encoder/ratecontrol.cpp  Thu Jun 25 13:42:29 2015 +0530
> > +++ b/source/encoder/ratecontrol.cpp  Mon Jun 22 17:20:16 2015 +0530
> > @@ -1086,19 +1086,6 @@
> >          }
> >      }
> >      m_framesDone++;
> > -
> > -    /* CQP and CRF (without capped VBV) doesn't use mid-frame
> statistics to
> > -     * tune RateControl parameters for other frames.
> > -     * Hence, for these modes, update m_startEndOrder and unlock RC for
> previous threads waiting in
> > -     * RateControlEnd here.those modes here. For the rest - ABR
> > -     * and VBV, unlock only after rateControlUpdateStats of this frame
> is called */
> > -    if (m_param->rc.rateControlMode != X265_RC_ABR && !m_isVbv)
> > -    {
> > -        m_startEndOrder.incr();
> > -
> > -        if (rce->encodeOrder < m_param->frameNumThreads - 1)
> > -            m_startEndOrder.incr(); // faked rateControlEnd calls for
> negative frames
> > -    }
> >      return m_qp;
> >  }
> >
> > @@ -1625,16 +1612,6 @@
> >
> >      m_cplxrSum += rce->rowCplxrSum;
> >      m_totalBits += rce->rowTotalBits;
> > -
> > -    /* do not allow the next frame to enter rateControlStart() until
> this
> > -     * frame has updated its mid-frame statistics */
> > -    if (m_param->rc.rateControlMode == X265_RC_ABR || m_isVbv)
> > -    {
> > -        m_startEndOrder.incr();
> > -
> > -        if (rce->encodeOrder < m_param->frameNumThreads - 1)
> > -            m_startEndOrder.incr(); // faked rateControlEnd calls for
> negative frames
> > -    }
> >  }
> >
> >  void RateControl::checkAndResetABR(RateControlEntry* rce, bool
> isFrameDone)
> > _______________________________________________
> > 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/20150627/a9c2ce20/attachment-0001.html>