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