[FFmpeg-devel] [PATCH][RFC] avcodec: disallow hwaccel with frame threads

[wm4]

On Sat, 17 Oct 2015 21:28:50 +0200
Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Sat, Oct 17, 2015 at 9:27 PM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> > ---
> >  libavcodec/utils.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >  
> 
> Above patch is submitted as an RFC, however I strongly believe its the
> only way to keep hwaccels sane in the future.
> There are several known problems when a hwaccel is used with frame
> threading, at least with DXVA2, ranging from simple image corruption
> to crashes in the GPU drivers.
> 
> All the problems essentially come down to two factors:
> 
> (1) while avcodec tries to prevent simultaneous access from different
> threads, it cannot control user code.
> As a API user you have no chance to avoid simultaneous access to the
> hardware surfaces with frame threading, because as soon as avcodec
> hands you a decoded surface,
> it'll already have started working on the next one. This is a common
> source for image corruption, as the decoder may fail to get a
> reference frame if its currently
> locked by the user code.
> 
> This issue is not really fixable, other than introducing a mutex
> around every call that the user code would have to lock as well.
> API break and making hwaccel even more complex to use.
> 
> (2) decoders often have had (or still have) trouble initializing the
> hwaccel properly with multiple threads, which can result in multiple
> threads re-creating the hwaccel
> and disrupting decoding, or even crashing in the driver.
> 
> While this is in theory fixable inside the decoders in question, it
> can lead to quite a bit of complexity, which should be avoided.
> 
> We should not be offering a mode that is known to be broken and even
> crash (even if the crash is not in our code).
> On top of that, frame threading with hwaccel does not actually improve
> the speed at all, if anything it might be slightly slower, and will
> definitely use more GPU memory.
> 
> The only reason this combination even exists is because API users
> insisted on it, and it was then added without properly taking the
> consequences into account.
> 
> There is an easy solution to keep using frame threads with software
> decoding while using a hwaccel otherwise - its used by myself, by mpv
> and by Kodi - simply re-open the decoder with the new threading flags.
> Its really not a lot of work, and keeps the code sane and the decoding reliable.

I agree with all points. Even if you'd consider this worth fixing (and
only so that the software fallback actually works?), there are many
practical problems with making it work. Let's just disallow it.

An alternative might be transparently disabling threading while a
hwaccel is active (and using threading normally when doing sw
decoding). Maybe someone comes up with a simply patch to achieve
this. But until then, this patch should be applied, since multithreaded
hw accel simply doesn't work and can crash.