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

wm4 nfxjfg at googlemail.com
Fri Oct 23 14:03:57 CEST 2015

On Fri, 23 Oct 2015 13:54:25 +0200
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",

Still looks good to me. Maybe it would be worth mentioning that new
hwaccels (like your VP9 patch) also have non-trivial issues with this.

More information about the ffmpeg-devel mailing list