[FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate hw_frames_ctx for vp9 without destroy va_context
Hendrik Leppkes
h.leppkes at gmail.com
Tue Aug 6 11:27:02 EEST 2019
On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu <linjie.fu at intel.com> wrote:
>
> VP9 allows resolution changes per frame. Currently in VAAPI, resolution
> changes leads to va context destroy and reinit. This will cause
> reference frame surface lost and produce garbage.
>
> Though refs surface id could be passed to media driver and found in
> RTtbl, vp9RefList[] in hal layer has already been destroyed. Thus the
> new created VaContext could only got an empty RefList.
>
> As libva allows re-create surface separately without changing the
> context, this issue could be handled by only recreating hw_frames_ctx.
>
> Set hwaccel_priv_data_keeping flag for vp9 to only recreating
> hw_frame_ctx when dynamic resolution changing happens.
>
> 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.fu at intel.com>
> ---
> libavcodec/decode.c | 10 +++++-----
> libavcodec/internal.h | 1 +
> libavcodec/pthread_frame.c | 2 ++
> libavcodec/vaapi_decode.c | 40 ++++++++++++++++++++++------------------
> libavcodec/vaapi_vp9.c | 4 ++++
> 5 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index 0863b82..7b15fa5 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.
Unrelated whitespace change
> @@ -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)
> @@ -1396,9 +1395,10 @@ int ff_get_format(AVCodecContext *avctx, const enum AVPixelFormat *fmt)
> memcpy(choices, fmt, (n + 1) * sizeof(*choices));
>
> for (;;) {
> - // Remove the previous hwaccel, if there was one.
> - hwaccel_uninit(avctx);
> -
> + // Remove the previous hwaccel, if there was one,
> + // and no need for keeping.
> + if (!avctx->internal->hwaccel_priv_data_keeping)
> + hwaccel_uninit(avctx);
> user_choice = avctx->get_format(avctx, choices);
> if (user_choice == AV_PIX_FMT_NONE) {
> // Explicitly chose nothing, give up.
There could be a dozen special cases how stuff can go wrong here. What
if get_format actually returns a different format then the one
currently in use? Or a software format?
Just removing this alone is not safe.
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 5096ffa..7adef08 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -188,6 +188,7 @@ typedef struct AVCodecInternal {
> * hwaccel-specific private data
> */
> void *hwaccel_priv_data;
> + int hwaccel_priv_data_keeping;
>
> /**
> * checks API usage: after codec draining, flush is required to resume operation
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index 36ac0ac..6032818 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -283,6 +283,7 @@ static int update_context_from_thread(AVCodecContext *dst, AVCodecContext *src,
> dst->sample_fmt = src->sample_fmt;
> dst->channel_layout = src->channel_layout;
> dst->internal->hwaccel_priv_data = src->internal->hwaccel_priv_data;
> + dst->internal->hwaccel_priv_data_keeping = src->internal->hwaccel_priv_data_keeping;
>
> if (!!dst->hw_frames_ctx != !!src->hw_frames_ctx ||
> (dst->hw_frames_ctx && dst->hw_frames_ctx->data != src->hw_frames_ctx->data)) {
> @@ -340,6 +341,7 @@ static int update_context_from_user(AVCodecContext *dst, AVCodecContext *src)
> dst->frame_number = src->frame_number;
> dst->reordered_opaque = src->reordered_opaque;
> dst->thread_safe_callbacks = src->thread_safe_callbacks;
> + dst->internal->hwaccel_priv_data_keeping = src->internal->hwaccel_priv_data_keeping;
>
> if (src->slice_count && src->slice_offset) {
> if (dst->slice_count < src->slice_count) {
> 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
>
More unrelated whitespace
> @@ -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 FF_API_STRUCT_VAAPI_CONTEXT
> }
> #endif
> diff --git a/libavcodec/vaapi_vp9.c b/libavcodec/vaapi_vp9.c
> index f384ba7..b313b5c 100644
> --- a/libavcodec/vaapi_vp9.c
> +++ b/libavcodec/vaapi_vp9.c
> @@ -25,6 +25,7 @@
> #include "hwaccel.h"
> #include "vaapi_decode.h"
> #include "vp9shared.h"
> +#include "internal.h"
>
> static VASurfaceID vaapi_vp9_surface_id(const VP9Frame *vf)
> {
> @@ -44,6 +45,9 @@ static int vaapi_vp9_start_frame(AVCodecContext *avctx,
> const AVPixFmtDescriptor *pixdesc = av_pix_fmt_desc_get(avctx->sw_pix_fmt);
> int err, i;
>
> + // keep hardware context because of DRC support for VP9
> + avctx->internal->hwaccel_priv_data_keeping = 1;
> +
> pic->output_surface = vaapi_vp9_surface_id(&h->frames[CUR_FRAME]);
>
> pic_param = (VADecPictureParameterBufferVP9) {
> --
> 2.7.4
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list