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

[wm4]

On Wed, 21 Oct 2015 11:53:46 +0200
Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Wed, Oct 21, 2015 at 11:44 AM, Gwenole Beauchesne <gb.devel at gmail.com> wrote:
> > 2015-10-21 11:27 GMT+02:00 Hendrik Leppkes <h.leppkes at gmail.com>:  
> >> On Sat, Oct 17, 2015 at 9:28 PM, 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.
> >>>  
> >>
> >> No further comments? No thoughts? No-one that would object if I were
> >> to push the proposed patch?  
> >
> > Fine with me.
> >
> > Probably add a link to your ffmpeg-devel message since it provides all
> > the necessary rationale behind this?
> >  
> 
> I might condense it down a bit and include it in the commit message,
> links have a habit of dying.

Yeah, there's plenty of free space in the commit message field.