[FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate hw_frames_ctx without destroy va_context

[Fu, Linjie]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Hendrik Leppkes
> Sent: Sunday, July 7, 2019 15:39
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] lavc/vaapi_decode: recreate
> hw_frames_ctx without destroy va_context
> 
> On Sun, Jul 7, 2019 at 6:41 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.
> >
> > 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.fu at 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)
> > +            hwaccel_uninit(avctx);
> >          user_choice = avctx->get_format(avctx, choices);
> 
> Having VAAPI specific code in this generic code path (or any hwaccel
> specific code) seems like a design flaw, and should perhaps be
> adressed by a generic improvement, instead of VAAPI specific hacks.
> 

Yes, a better solution(maybe a flag set by reinit in vaapi) should be applied.
Thanks for your response.