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

Ronald S. Bultje rsbultje at gmail.com
Mon Oct 26 01:49:45 CET 2015


Hi,

On Sun, Oct 25, 2015 at 5:55 PM, Hendrik Leppkes <h.leppkes at gmail.com>
wrote:

> On Fri, Oct 23, 2015 at 1:54 PM, Hendrik Leppkes <h.leppkes at gmail.com>
> wrote:
> > HWAccels with frame threads are fundamentally flawed in avcodecs current
> > design, and there are several known problems ranging from image
> corruption
> > to driver crashes.
> >
> > These problems come down to two design problems in the interaction of
> > threads and HWAccel decoding:
> >
> > (1)
> > While avcodec prevents parallel decoding and as such simultaneous access
> > to the hardware accelerator from the decoding threads, it cannot account
> > for the user code and its access to the hardware surfaces and the
> hardware
> > itself.
> >
> > This can result in image corruption or even driver crashes if the
> > user code locks image surfaces while they are being used by the decoder
> > threads as reference frames.
> >
> > The current HWAccel API does not offer any way to ensure exclusive access
> > to the hardware or the surfaces if frame threading is used.
> >
> > (2)
> > Initialization of the HWAccel with frame threads is non-trivial, and many
> > decoders had and still have issues that cause excess calls to the
> > get_format callback.
> >
> > This will potentially cause duplicate HWAccel initialization, which in
> > extreme cases can even lead to driver crashes if the HWAccel is
> > re-initialized while the user code is actively accessing the hardware
> > surfaces associated with it, or lead to image corruption due to lost
> > reference frames.
> >
> > While both of these issues are solvable, fixing (1) would at least
> require
> > a huge API redesign which would move a lot of complexity into the user
> > code.
> >
> > The only reason the combination of frame threads and HWAccel was
> > considered useful is to allow a seamless fallback to multi-threaded
> > software decoding if the HWAccel is not available, however the issues
> > outlined above far outweight this.
> >
> > The proper solution for a fallback is to re-open the AVCodecContext with
> > threading enabled if the HWAccel failed, which is a practice commonly
> used
> > by various user applications using avcodec today already.
> >
> > Reviewed-by: Gwenole Beauchesne <gb.devel at gmail.com>
> > Reviewed-by: wm4 <nfxjfg at googlemail.com>
> > Signed-off-by: Hendrik Leppkes <h.leppkes at gmail.com>
> > ---
> >  libavcodec/utils.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> > index 83a2078..6a58056 100644
> > --- a/libavcodec/utils.c
> > +++ b/libavcodec/utils.c
> > @@ -1007,6 +1007,12 @@ static int setup_hwaccel(AVCodecContext *avctx,
> >      AVHWAccel *hwa = find_hwaccel(avctx->codec_id, fmt);
> >      int ret        = 0;
> >
> > +    if (avctx->active_thread_type & FF_THREAD_FRAME) {
> > +        av_log(avctx, AV_LOG_ERROR,
> > +               "Hardware accelerated decoding with frame threading is
> not supported.\n");
> > +        return AVERROR(EINVAL);
> > +    }
> > +
> >      if (!hwa) {
> >          av_log(avctx, AV_LOG_ERROR,
> >                 "Could not find an AVHWAccel for the pixel format: %s",
> > --
> > 2.6.2.windows.1
> >
>
> Last call for tangible feedback, otherwise its going in tomorrow!


This is certainly not irreversible in case something improves, so I'd just
commit it already. lgtm.

Ronald


More information about the ffmpeg-devel mailing list