On Thu, Jul 11, 2019 at 10:20 AM Linjie Fu <linjie...@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...@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ 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".