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

2022-12-06 Thread Wang, Fei W 
On Thu, 2022-12-01 at 09:55 +0800, Fei Wang wrote:
> On Mon, 2022-11-28 at 13:20 +, 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 
> > > ---
> > >   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 @ 0x57c0e7c0] Format vaapi chosen by get_format().
> > [hevc @ 0x57c0e7c0] Format vaapi requires hwaccel
> > initialisation.
> > [hevc @ 0x57c0e7c0] 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=0x57c0e7c0) at
> > src/libavcodec/vaapi_decode.c:714
> > 714 vas = vaDestroyContext(ctx->hwctx->display, ctx-
> > > va_context);
> > (gdb) bt
> > #0  ff_vaapi_decode_uninit (avctx=0x57c0e7c0) at
> > src/libavcodec/vaapi_decode.c:714
> > #1  0x563073d7 in ff_vaapi_decode_init
> > (avctx=0x57c0e7c0)
> > at src/libavcodec/vaapi_decode.c:704
> > #2  0x55e62fee in hwaccel_init (avctx=0x57c0e7c0,
> > hw_config=0x5728f770 <__compound_literal.0>) at
> > src/libavcodec/decode.c:1121
> > #3  0x55e634ec in ff_get_format (avctx=0x57c0e7c0,
> > fmt=0x7fffb4ff8ccc) at src/libavcodec/decode.c:1261
> > #4  0x561ca829 in ff_thread_get_format
> > (avctx=0x57c0e7c0,
> > fmt=0x7fffb4ff8ccc) at src/libavcodec/pthread_frame.c:1048
> > #5  0x55f68f37 in get_format (s=0x57c3e6c0,
> > sps=0x57c21f80) at src/libavcodec/hevcdec.c:505
> > #6  0x55f69621 in hls_slice_header (s=0x57c3e6c0) at
> > src/libavcodec/hevcdec.c:618
> > #7  0x55f7472d in decode_nal_unit (s=0x57c3e6c0,
> > nal=0x7fff8802e920) at src/libavcodec/hevcdec.c:3173
> > #8  0x55f7508a in decode_nal_units (s=0x57c3e6c0,
> > buf=0x7637c010 "", length=159280) at
> > src/libavcodec/hevcdec.c:3355
> > #9  0x55f756d6 in hevc_decode_frame (avctx=0x57c0e7c0,
> > rframe=0x57c0ecc0, got_output=0x57c0d690,
> > avpkt=0x57c0ef40) at src/libavcodec/hevcdec.c:3497
> > #10 0x561c839c in frame_worker_thread (arg=0x57c0d580)
> > at
> > src/libavcodec/pthread_frame.c:241
> > #11 0x768ccea7 in start_thread (arg=) at
> > pthread_create.c:477
> > #12 0x767ecaef 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

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

2022-11-30 Thread Wang, Fei W 
On Mon, 2022-11-28 at 13:20 +, 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 
> > ---
> >   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 @ 0x57c0e7c0] Format vaapi chosen by get_format().
> [hevc @ 0x57c0e7c0] Format vaapi requires hwaccel initialisation.
> [hevc @ 0x57c0e7c0] 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=0x57c0e7c0) at
> src/libavcodec/vaapi_decode.c:714
> 714 vas = vaDestroyContext(ctx->hwctx->display, ctx-
> >va_context);
> (gdb) bt
> #0  ff_vaapi_decode_uninit (avctx=0x57c0e7c0) at
> src/libavcodec/vaapi_decode.c:714
> #1  0x563073d7 in ff_vaapi_decode_init (avctx=0x57c0e7c0)
> at src/libavcodec/vaapi_decode.c:704
> #2  0x55e62fee in hwaccel_init (avctx=0x57c0e7c0,
> hw_config=0x5728f770 <__compound_literal.0>) at
> src/libavcodec/decode.c:1121
> #3  0x55e634ec in ff_get_format (avctx=0x57c0e7c0,
> fmt=0x7fffb4ff8ccc) at src/libavcodec/decode.c:1261
> #4  0x561ca829 in ff_thread_get_format (avctx=0x57c0e7c0,
> fmt=0x7fffb4ff8ccc) at src/libavcodec/pthread_frame.c:1048
> #5  0x55f68f37 in get_format (s=0x57c3e6c0,
> sps=0x57c21f80) at src/libavcodec/hevcdec.c:505
> #6  0x55f69621 in hls_slice_header (s=0x57c3e6c0) at
> src/libavcodec/hevcdec.c:618
> #7  0x55f7472d in decode_nal_unit (s=0x57c3e6c0,
> nal=0x7fff8802e920) at src/libavcodec/hevcdec.c:3173
> #8  0x55f7508a in decode_nal_units (s=0x57c3e6c0,
> buf=0x7637c010 "", length=159280) at
> src/libavcodec/hevcdec.c:3355
> #9  0x55f756d6 in hevc_decode_frame (avctx=0x57c0e7c0,
> rframe=0x57c0ecc0, got_output=0x57c0d690,
> avpkt=0x57c0ef40) at src/libavcodec/hevcdec.c:3497
> #10 0x561c839c in frame_worker_thread (arg=0x57c0d580) at
> src/libavcodec/pthread_frame.c:241
> #11 0x768ccea7 in start_thread (arg=) at
> pthread_create.c:477
> #12 0x767ecaef 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

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

2022-11-28 Thread Mark Thompson 

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 
---
  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 @ 0x57c0e7c0] Format vaapi chosen by get_format().
[hevc @ 0x57c0e7c0] Format vaapi requires hwaccel initialisation.
[hevc @ 0x57c0e7c0] 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=0x57c0e7c0) at 
src/libavcodec/vaapi_decode.c:714
714 vas = vaDestroyContext(ctx->hwctx->display, ctx->va_context);
(gdb) bt
#0  ff_vaapi_decode_uninit (avctx=0x57c0e7c0) at 
src/libavcodec/vaapi_decode.c:714
#1  0x563073d7 in ff_vaapi_decode_init (avctx=0x57c0e7c0) at 
src/libavcodec/vaapi_decode.c:704
#2  0x55e62fee in hwaccel_init (avctx=0x57c0e7c0, 
hw_config=0x5728f770 <__compound_literal.0>) at src/libavcodec/decode.c:1121
#3  0x55e634ec in ff_get_format (avctx=0x57c0e7c0, 
fmt=0x7fffb4ff8ccc) at src/libavcodec/decode.c:1261
#4  0x561ca829 in ff_thread_get_format (avctx=0x57c0e7c0, 
fmt=0x7fffb4ff8ccc) at src/libavcodec/pthread_frame.c:1048
#5  0x55f68f37 in get_format (s=0x57c3e6c0, sps=0x57c21f80) at 
src/libavcodec/hevcdec.c:505
#6  0x55f69621 in hls_slice_header (s=0x57c3e6c0) at 
src/libavcodec/hevcdec.c:618
#7  0x55f7472d in decode_nal_unit (s=0x57c3e6c0, 
nal=0x7fff8802e920) at src/libavcodec/hevcdec.c:3173
#8  0x55f7508a in decode_nal_units (s=0x57c3e6c0, buf=0x7637c010 
"", length=159280) at src/libavcodec/hevcdec.c:3355
#9  0x55f756d6 in hevc_decode_frame (avctx=0x57c0e7c0, 
rframe=0x57c0ecc0, got_output=0x57c0d690, avpkt=0x57c0ef40) at 
src/libavcodec/hevcdec.c:3497
#10 0x561c839c in frame_worker_thread (arg=0x57c0d580) at 
src/libavcodec/pthread_frame.c:241
#11 0x768ccea7 in start_thread (arg=) at 
pthread_create.c:477
#12 0x767ecaef 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,
 >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;

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

2022-11-13 Thread Fei Wang 
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 
---
 libavcodec/vaapi_decode.c | 11 ---
 libavcodec/vaapi_decode.h |  1 +
 libavcodec/vaapi_vp9.c|  4 
 3 files changed, 13 insertions(+), 3 deletions(-)

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

___
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".