Re: [FFmpeg-devel] [PATCH v5 2/2] lavc/vaapi_decode: add support for HWACCEL_CAP_RESET_WITHOUT_UNINIT

["Wang, Fei W" <fei.w.wang-at-intel.com@ffmpeg.org>]

On Thu, 2022-12-01 at 09:55 +0800, Fei Wang wrote:
> On Mon, 2022-11-28 at 13:20 +0000, Mark Thompson wrote:
> > On 14/11/2022 01:16, Fei Wang wrote:
> > > This can fix vp9 decode image corruption when the frame size is
> > > change,
> > > but the pervious frames still be referenced.
> > > 
> > > Surfaces don't need to be bound to vaContext only after VAAPI
> > > 1.0.0:
> > > https://github.com/intel/libva/commit/492b692005ccd0d8da190209d5b3ae7b7825f4b8
> > > 
> > > Signed-off-by: Fei Wang <fei.w.wang@intel.com>
> > > ---
> > >   libavcodec/vaapi_decode.c | 11 ++++++++---
> > >   libavcodec/vaapi_decode.h |  1 +
> > >   libavcodec/vaapi_vp9.c    |  4 ++++
> > >   3 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > This always segfaults immediately on anything unsupported.  E.g.
> > with
> > fate/hevc/paramchange_yuv420p_yuv420p10.hevc:
> > 
> > [hevc @ 0x555557c0e7c0] Format vaapi chosen by get_format().
> > [hevc @ 0x555557c0e7c0] Format vaapi requires hwaccel
> > initialisation.
> > [hevc @ 0x555557c0e7c0] Hardware does not support image size
> > 1056x8440 (constraints: width 0-4096 height 0-4096).
> > 
> > Thread 20 "av:hevc:df0" received signal SIGSEGV, Segmentation
> > fault.
> > [Switching to Thread 0x7fffb4ff9700 (LWP 509456)]
> > ff_vaapi_decode_uninit (avctx=0x555557c0e7c0) at
> > src/libavcodec/vaapi_decode.c:714
> > 714             vas = vaDestroyContext(ctx->hwctx->display, ctx-
> > > va_context);
> > (gdb) bt
> > #0  ff_vaapi_decode_uninit (avctx=0x555557c0e7c0) at
> > src/libavcodec/vaapi_decode.c:714
> > #1  0x00005555563073d7 in ff_vaapi_decode_init
> > (avctx=0x555557c0e7c0)
> > at src/libavcodec/vaapi_decode.c:704
> > #2  0x0000555555e62fee in hwaccel_init (avctx=0x555557c0e7c0,
> > hw_config=0x55555728f770 <__compound_literal.0>) at
> > src/libavcodec/decode.c:1121
> > #3  0x0000555555e634ec in ff_get_format (avctx=0x555557c0e7c0,
> > fmt=0x7fffb4ff8ccc) at src/libavcodec/decode.c:1261
> > #4  0x00005555561ca829 in ff_thread_get_format
> > (avctx=0x555557c0e7c0,
> > fmt=0x7fffb4ff8ccc) at src/libavcodec/pthread_frame.c:1048
> > #5  0x0000555555f68f37 in get_format (s=0x555557c3e6c0,
> > sps=0x555557c21f80) at src/libavcodec/hevcdec.c:505
> > #6  0x0000555555f69621 in hls_slice_header (s=0x555557c3e6c0) at
> > src/libavcodec/hevcdec.c:618
> > #7  0x0000555555f7472d in decode_nal_unit (s=0x555557c3e6c0,
> > nal=0x7fff8802e920) at src/libavcodec/hevcdec.c:3173
> > #8  0x0000555555f7508a in decode_nal_units (s=0x555557c3e6c0,
> > buf=0x7ffff637c010 "", length=159280) at
> > src/libavcodec/hevcdec.c:3355
> > #9  0x0000555555f756d6 in hevc_decode_frame (avctx=0x555557c0e7c0,
> > rframe=0x555557c0ecc0, got_output=0x555557c0d690,
> > avpkt=0x555557c0ef40) at src/libavcodec/hevcdec.c:3497
> > #10 0x00005555561c839c in frame_worker_thread (arg=0x555557c0d580)
> > at
> > src/libavcodec/pthread_frame.c:241
> > #11 0x00007ffff68ccea7 in start_thread (arg=<optimized out>) at
> > pthread_create.c:477
> > #12 0x00007ffff67ecaef in clone () at
> > ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
> > (gdb)
> 
> Thanks, will fix this in next version.
> 
> > 
> > Also, I don't see how this is testing whether the driver supports
> > changing the resolution at runtime?  The note in libva that you
> > cite
> > allows new switching render targets in the context, but I don't see
> > why different resolution would be allowed given that it's a
> > parameter
> > passed to vaCreateContext()?
> > 
> > Looking at the Mesa driver it appears that the internally-allocated
> > references are not going to allow size changes (where does the
> > template width get updated?).  I don't have any hardware to test
> > that, though - are you able to try this on recent AMD hardware with
> > VP9 support?
> 
> I checked on AMD RX6700XT, it can get correct output when decoding
> multi-resolution vp9 clips only after apply this patchset. For
> example,
> by using clip from:
> https://storage.googleapis.com/downloads.webmproject.org/vp9/decoder-test-streams/Profile_0_8bit.zip
> 
> VP9 native decode result:
> ffmpeg -i
> Profile_0_8bit/frm_resize/crowd_run_1080X512_fr30_bd8_frm_resize_l3.w
> eb
> m -pix_fmt yuv420p -autoscale 0 -f md5 -y -
> [...]
> MD5=51b3393fa98ad9ab99c0b45ef705ebc4
> [...]
> 
> Without this patchset:
> ffmpeg -v verbose -hwaccel vaapi -i
> Profile_0_8bit/frm_resize/crowd_run_1080X512_fr30_bd8_frm_resize_l3.w
> eb
> m -pix_fmt yuv420p -autoscale 0 -f md5 -y -
> [...]
> [AVHWDeviceContext @ 0x56526336a000] VAAPI driver: Mesa Gallium
> driver
> 22.3.0-rc4 for AMD Radeon RX 6700 XT (navi22, LLVM 11.0.0, DRM 3.44,
> 5.13.0-40-generic).
> [...]
> MD5=2e799f0f916195f86a356907f7e4eae1 (change from time to time, but
> never same with native decode result)
> [...]
> 
> With this patchset:
> ffmpeg -v verbose -hwaccel vaapi -i
> Profile_0_8bit/frm_resize/crowd_run_1080X512_fr30_bd8_frm_resize_l3.w
> eb
> m -pix_fmt yuv420p -autoscale 0 -f md5 -y -
> [...]
> [AVHWDeviceContext @ 0x561c08e7a000] VAAPI driver: Mesa Gallium
> driver
> 22.3.0-rc4 for AMD Radeon RX 6700 XT (navi22, LLVM 11.0.0, DRM 3.44,
> 5.13.0-40-generic).
> [...]
> MD5=51b3393fa98ad9ab99c0b45ef705ebc4
> [...]
> 
> That means both Intel and AMD driver implementation doesn't limit
> surface's resolution must be same with vacontext. So I think we can
> add
> some description to libva to declare that.

Hi Mark,

New interface added to libva to query driver's attributes. Could you
give your comments on this PR if you have?
https://github.com/intel/libva/pull/636

Thanks
Fei

> 
> > What have you done to verify the METHOD_HW_FRAMES_CTX
> > behaviour?  This has changed so that vaapi_decode_make_config() is
> > no
> > longer called, which feels possibly-bad though I'm unsure of the
> > exact consequences.
> > 
> > As I said last time, I do think you should only do this in exactly
> > the places where it is required, and not change any other
> > behaviour.
> 
> vaapi_decode_make_config() is called by ff_decode_get_hw_frames_ctx()
> in ff_vaapi_decode_init. So the change to vaapi_decode_make_config
> doesn't impact the final result.
> To avoid confusion, I will not change its behaviour in next version.
> 
> Thanks
> Fei
> 
> > Thanks,
> > 
> > - Mark
> > 
> > > diff --git a/libavcodec/vaapi_decode.c
> > > b/libavcodec/vaapi_decode.c
> > > index 134f10eca5..d950471b6d 100644
> > > --- a/libavcodec/vaapi_decode.c
> > > +++ b/libavcodec/vaapi_decode.c
> > > @@ -658,9 +658,6 @@ int ff_vaapi_decode_init(AVCodecContext
> > > *avctx)
> > >       VAStatus vas;
> > >       int err;
> > >   
> > > -    ctx->va_config  = VA_INVALID_ID;
> > > -    ctx->va_context = VA_INVALID_ID;
> > > -
> > >       err = ff_decode_get_hw_frames_ctx(avctx,
> > > AV_HWDEVICE_TYPE_VAAPI);
> > >       if (err < 0)
> > >           goto fail;
> > > @@ -670,6 +667,12 @@ int ff_vaapi_decode_init(AVCodecContext
> > > *avctx)
> > >       ctx->device = ctx->frames->device_ctx;
> > >       ctx->hwctx  = ctx->device->hwctx;
> > >   
> > > +    if (ctx->inited)
> > > +        return 0;
> > > +
> > > +    ctx->va_config  = VA_INVALID_ID;
> > > +    ctx->va_context = VA_INVALID_ID;
> > > +
> > >       err = vaapi_decode_make_config(avctx, ctx->frames-
> > > > device_ref,
> > >                                      &ctx->va_config, NULL);
> > >       if (err)
> > > @@ -691,6 +694,8 @@ int ff_vaapi_decode_init(AVCodecContext
> > > *avctx)
> > >       av_log(avctx, AV_LOG_DEBUG, "Decode context initialised: "
> > >              "%#x/%#x.\n", ctx->va_config, ctx->va_context);
> > >   
> > > +    ctx->inited = 1;
> > > +
> > >       return 0;
> > >   
> > >   fail:
> > > diff --git a/libavcodec/vaapi_decode.h
> > > b/libavcodec/vaapi_decode.h
> > > index 6beda14e52..62a4f37ed9 100644
> > > --- a/libavcodec/vaapi_decode.h
> > > +++ b/libavcodec/vaapi_decode.h
> > > @@ -61,6 +61,7 @@ typedef struct VAAPIDecodeContext {
> > >       int                   surface_count;
> > >   
> > >       VASurfaceAttrib       pixel_format_attribute;
> > > +    int                   inited;
> > >   } VAAPIDecodeContext;
> > >   
> > >   
> > > diff --git a/libavcodec/vaapi_vp9.c b/libavcodec/vaapi_vp9.c
> > > index 776382f683..245b7a1b3a 100644
> > > --- a/libavcodec/vaapi_vp9.c
> > > +++ b/libavcodec/vaapi_vp9.c
> > > @@ -181,5 +181,9 @@ const AVHWAccel ff_vp9_vaapi_hwaccel = {
> > >       .uninit               = ff_vaapi_decode_uninit,
> > >       .frame_params         = ff_vaapi_common_frame_params,
> > >       .priv_data_size       = sizeof(VAAPIDecodeContext),
> > > +#if VA_CHECK_VERSION(1, 0, 0)
> > > +    .caps_internal        = HWACCEL_CAP_ASYNC_SAFE |
> > > HWACCEL_CAP_RESET_WITHOUT_UNINIT,
> > > +#else
> > >       .caps_internal        = HWACCEL_CAP_ASYNC_SAFE,
> > > +#endif
> > >   };
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@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-request@ffmpeg.org with subject "unsubscribe".