diff mbox series

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

Message ID 20221114011605.1157707-2-fei.w.wang@intel.com
State New
Headers show
Series [FFmpeg-devel,v5,1/2] lavc: add HWACCEL_CAP_RESET_WITHOUT_UNINIT capacity for hwaccel | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Wang, Fei W Nov. 14, 2022, 1:16 a.m. UTC
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(-)

Comments

Mark Thompson Nov. 28, 2022, 1:20 p.m. UTC | #1
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)


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?

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.

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
>   };
Wang, Fei W Dec. 1, 2022, 1:55 a.m. UTC | #2
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.web
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.web
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.web
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.

> 
> 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".
Wang, Fei W Dec. 6, 2022, 8:57 a.m. UTC | #3
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".
diff mbox series

Patch

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
 };