On 07/07/2019 17:38, Linjie Fu wrote:
> VP9 allows resolution changes per frame. Currently in VAAPI, resolution
> changes leads to va context destroy and reinit.
Which is correct - it needs to remake the context because the old one is for
the wrong resolution.
> This will cause
> reference frame surface lost and produce garbage.
This isn't right - the reference frame surfaces aren't lost. See
VP9Context.s.refs[], which holds references to the frames (including their
hardware frame contexts) until the stream indicates that they can be discarded
by overwriting their reference frame slots.
> As libva allows re-create surface separately without changing the
> context, this issue could be handled by only recreating hw_frames_ctx.
>
> Could be verified by:
> ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -i
> ./resolutions.ivf
> -pix_fmt p010le -f rawvideo -vframes 20 -y vaapi.yuv
>
> Signed-off-by: Linjie Fu <linjie...@intel.com>
> ---
> libavcodec/decode.c | 8 ++++----
> libavcodec/vaapi_decode.c | 40 ++++++++++++++++++++++------------------
> 2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 0863b82..a81b857 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1254,7 +1254,6 @@ int ff_decode_get_hw_frames_ctx(AVCodecContext *avctx,
>
> frames_ctx = (AVHWFramesContext*)avctx->hw_frames_ctx->data;
>
> -
> if (frames_ctx->initial_pool_size) {
> // We guarantee 4 base work surfaces. The function above guarantees 1
> // (the absolute minimum), so add the missing count.
> @@ -1333,7 +1332,7 @@ static int hwaccel_init(AVCodecContext *avctx,
> return AVERROR_PATCHWELCOME;
> }
>
> - if (hwaccel->priv_data_size) {
> + if (hwaccel->priv_data_size && !avctx->internal->hwaccel_priv_data) {
> avctx->internal->hwaccel_priv_data =
> av_mallocz(hwaccel->priv_data_size);
> if (!avctx->internal->hwaccel_priv_data)
> @@ -1397,8 +1396,9 @@ int ff_get_format(AVCodecContext *avctx, const enum
> AVPixelFormat *fmt)
>
> for (;;) {
> // Remove the previous hwaccel, if there was one.
> - hwaccel_uninit(avctx);
> -
> + // VAAPI allows reinit hw_frames_ctx only
> + if (choices[0] != AV_PIX_FMT_VAAPI_VLD)
Including codec-specific conditions in the generic code suggests that something
very shady is going on.
> + hwaccel_uninit(avctx);> user_choice =
> avctx->get_format(avctx, choices);
> if (user_choice == AV_PIX_FMT_NONE) {
> // Explicitly chose nothing, give up.
> diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
> index 69512e1..13f0cf0 100644
> --- a/libavcodec/vaapi_decode.c
> +++ b/libavcodec/vaapi_decode.c
> @@ -613,8 +613,10 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
> VAStatus vas;
> int err;
>
> - ctx->va_config = VA_INVALID_ID;
> - ctx->va_context = VA_INVALID_ID;
> + if (!ctx->va_config && !ctx->va_context){
> + ctx->va_config = VA_INVALID_ID;
> + ctx->va_context = VA_INVALID_ID;
> + }
>
> #if FF_API_STRUCT_VAAPI_CONTEXT
> if (avctx->hwaccel_context) {
> @@ -642,7 +644,6 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
> // present, so set it here to avoid the behaviour changing.
> ctx->hwctx->driver_quirks =
> AV_VAAPI_DRIVER_QUIRK_RENDER_PARAM_BUFFERS;
> -
> }
> #endif
>
> @@ -655,7 +656,8 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
> "context: %#x/%#x.\n", ctx->va_config, ctx->va_context);
> } else {
> #endif
> -
> + // Get a new hw_frames_ctx even if there is already one
> + // recreate surface without reconstuct va_context
> err = ff_decode_get_hw_frames_ctx(avctx, AV_HWDEVICE_TYPE_VAAPI);
> if (err < 0)
> goto fail;
> @@ -670,21 +672,23 @@ int ff_vaapi_decode_init(AVCodecContext *avctx)
> if (err)
> goto fail;
>
> - vas = vaCreateContext(ctx->hwctx->display, ctx->va_config,
> - avctx->coded_width, avctx->coded_height,
> - VA_PROGRESSIVE,
> - ctx->hwfc->surface_ids,
> - ctx->hwfc->nb_surfaces,
> - &ctx->va_context);
> - if (vas != VA_STATUS_SUCCESS) {
> - av_log(avctx, AV_LOG_ERROR, "Failed to create decode "
> - "context: %d (%s).\n", vas, vaErrorStr(vas));
> - err = AVERROR(EIO);
> - goto fail;
> - }
> + if (ctx->va_context == VA_INVALID_ID) {
> + vas = vaCreateContext(ctx->hwctx->display, ctx->va_config,
> + avctx->coded_width, avctx->coded_height,
> + VA_PROGRESSIVE,
> + ctx->hwfc->surface_ids,
> + ctx->hwfc->nb_surfaces,
> + &ctx->va_context);
> + if (vas != VA_STATUS_SUCCESS) {
> + av_log(avctx, AV_LOG_ERROR, "Failed to create decode "
> + "context: %d (%s).\n", vas, vaErrorStr(vas));
> + err = AVERROR(EIO);
> + goto fail;
> + }
>
> - av_log(avctx, AV_LOG_DEBUG, "Decode context initialised: "
> - "%#x/%#x.\n", ctx->va_config, ctx->va_context);
> + av_log(avctx, AV_LOG_DEBUG, "Decode context initialised: "
> + "%#x/%#x.\n", ctx->va_config, ctx->va_context);
> + }
If I'm reading this correctly, this changes to only creating one context ever
even when the resolution changes.
How can that work if the resolution increases? For example you start decoding
at 1280x720 and create a context for that, then the resolution changes to
3840x2160 and it tries to decode using the 1280x720 context.
> #if FF_API_STRUCT_VAAPI_CONTEXT
> }
> #endif
>
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
