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

[Hendrik Leppkes]

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!

- Hendrik