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

[Steve Borho]

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