> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf > Of Hendrik Leppkes > Sent: Tuesday, August 6, 2019 16:27 > To: FFmpeg development discussions and patches <ffmpeg- > de...@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] lavc/vaapi_decode: recreate > hw_frames_ctx for vp9 without destroy va_context > > 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
There is a redundant whitespace here, so I removed it within this patch. > > @@ -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. Didn't quite get your point. IMHO, avctx->internal->hwaccel_priv_data_keeping won't be set in other cases other than vaapi_vp9, so this patch won't break the default pipeline, and hwaccel_uninit(avctx) will always be called. > > 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 Same, another redundant whitespace. A separate patch will be more acceptable? - linjie _______________________________________________ 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".