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

[Deepthi Nandakumar]

Should m_startEndOrder be a member of Encoder, rather than ratecontrol,
since it serializes RcStarts, slice context inits as well as   some CTU row
encodes?
On Jun 29, 2015 9:21 PM, "Steve Borho" <steve at borho.org> wrote:

> On 06/29, aarthi at multicorewareinc.com wrote:
> > # HG changeset patch
> > # User Aarthi Thirumalai
> > # Date 1435312791 -19800
> > #      Fri Jun 26 15:29:51 2015 +0530
> > # Node ID 2dac97717742d6187df7798bdfbd8a9f04e6f446
> > # Parent  f7bbb04e1992a238aee739a0e21777932c1dfa57
> > rc: fixes inconsistent output in linux because of RC Lock in CQP/CRF
> >
> > the inconsistency is due to a race hazrd in slice context
> initializations when 2 Frame Encoders complete
> > RateControlStart in correct order but slice context initializations in
> the wrong order in CRF/CQP.
>
> Ok.
>
> So for CRF and CQP instead of auto-incrementing m_startEndOrder just
> before exiting the rateControlStart() function, the 'end' signal is
> deffered until just before FE enters the CTU compression loop, thus all
> slice setup work becomes serialized between frame encoders.
>
> makes sense. but my older comment stands in that this is somewhat of a
> layering violation. but I don't have any good ideas right now about how
> to improve it.
>
> I hope m_startEndOrder has a decent comment somewhere which describes
> that it is serializing certain portions of the frame encoder main loop
> to protect rate control and/or slice initialization from race hazards.
> And how, for ABR, it even serializes a certain number of CTU rows (up to
> rateControlUpdateStats) in order to improve ABR efficiency.
>
> > diff -r f7bbb04e1992 -r 2dac97717742 source/encoder/frameencoder.cpp
> > --- a/source/encoder/frameencoder.cpp Fri Jun 26 10:16:29 2015 +0530
> > +++ b/source/encoder/frameencoder.cpp Fri Jun 26 15:29:51 2015 +0530
> > @@ -474,6 +474,19 @@
> >          m_nalList.serialize(NAL_UNIT_PREFIX_SEI, m_bs);
> >      }
> >
> > +    /* 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, after the slicecontexts are initialized.
> For the rest - ABR
> > +     * and VBV, unlock only after rateControlUpdateStats of this frame
> is called */
> > +    if (m_param->rc.rateControlMode != X265_RC_ABR &&
> !m_top->m_rateControl->m_isVbv)
> > +    {
> > +        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
> > +    }
> > +
> >      /* Analyze CTU rows, most of the hard work is done here.  Frame is
> >       * compressed in a wave-front pattern if WPP is enabled. Row based
> loop
> >       * filters runs behind the CTU compression and reconstruction */
> > diff -r f7bbb04e1992 -r 2dac97717742 source/encoder/ratecontrol.cpp
> > --- a/source/encoder/ratecontrol.cpp  Fri Jun 26 10:16:29 2015 +0530
> > +++ b/source/encoder/ratecontrol.cpp  Fri Jun 26 15:29:51 2015 +0530
> > @@ -1087,18 +1087,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;
> >  }
> >
> > _______________________________________________
> > 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/20150629/aa661e12/attachment.html>