regression: qsvvpp segfault with new FIFO API

[U. Artie Eoff]

Summary of the bug:

All qsvvpp operations segfault since:

commit 85c938fa287c61334d01adfb038ca47bed6d106c
Author: Anton Khirnov <anton@khirnov.net>
Date:   Fri Jan 7 11:41:42 2022 +0100

    lavfi/qsvvpp: switch to new FIFO API

If I revert this change, it works.

How to reproduce:

% ffmpeg -i input ... output
ffmpeg version
built on ...

ffmpeg -init_hw_device qsv=qsv:hw_any,child_device=/dev/dri/renderD128 -hwaccel qsv -v verbose -f rawvideo -pix_fmt nv12 -s:v 1280x720 -i NV12.yuv -vf 'format=nv12|qsv,hwupload=extra_hw_frames=16,vpp_qsv=procamp=1:brightness=-100.0,hwdownload,format=nv12' -pix_fmt nv12 -f rawvideo -vsync passthrough -an -vframes 10 -y brightness_NV12.yuv
ffmpeg version N-105504-g04cc7a5548fa Copyright (c) 2000-2022 the FFmpeg developers
  built with gcc 11 (GCC)
  configuration: --prefix=/home/uaeoff/Work/workspace/media/install --disable-static --enable-shared --enable-libdrm --enable-vaapi --enable-libmfx --disable-amf --disable-audiotoolbox --disable-cuda --disable-cuda-sdk --disable-cuvid --disable-d3d11va --disable-dxva2 --disable-libnpp --disable-mmal --disable-nvdec --disable-nvenc --disable-omx --disable-omx-rpi --disable-rkmpp --disable-v4l2-m2m --disable-vdpau --disable-videotoolbox --enable-gpl --enable-libx264 --enable-libx265
  libavutil      57. 21.100 / 57. 21.100
  libavcodec     59. 20.100 / 59. 20.100
  libavformat    59. 17.101 / 59. 17.101
  libavdevice    59.  5.100 / 59.  5.100
  libavfilter     8. 26.101 /  8. 26.101
  libswscale      6.  5.100 /  6.  5.100
  libswresample   4.  4.100 /  4.  4.100
  libpostproc    56.  4.100 / 56.  4.100
[AVHWDeviceContext @ 0x1130200] libva: VA-API version 1.14.0
[AVHWDeviceContext @ 0x1130200] libva: User requested driver 'iHD'
[AVHWDeviceContext @ 0x1130200] libva: Trying to open /home/uaeoff/Work/workspace/media/install/lib/dri/iHD_drv_video.so
[AVHWDeviceContext @ 0x1130200] libva: Found init function __vaDriverInit_1_14
[AVHWDeviceContext @ 0x1130200] libva: va_openDriver() returns 0
[AVHWDeviceContext @ 0x1130200] Initialised VAAPI connection: version 1.14
[AVHWDeviceContext @ 0x1130200] VAAPI driver: Intel iHD driver for Intel(R) Gen Graphics - 22.2.0 ().
[AVHWDeviceContext @ 0x1130200] Driver not found in known nonstandard list, using standard behaviour.
[AVHWDeviceContext @ 0x112fcc0] Initialize MFX session: API version is 1.35, implementation version is 1.35
[rawvideo @ 0x12047c0] Estimating duration from bitrate, this may be inaccurate
WARNING: defaulting hwaccel_output_format to qsv for compatibility with old commandlines. This behaviour is DEPRECATED and will be removed in the future. Please explicitly set "-hwaccel_output_format qsv".
Input #0, rawvideo, from '/home/uaeoff/Work/workspace/media/src/otc-media/assets/bat/yuv/720p5994_parkrun_ter_1280x720_NV12.yuv':
  Duration: 00:00:00.40, start: 0.000000, bitrate: 276480 kb/s
  Stream #0:0: Video: rawvideo, 1 reference frame (NV12 / 0x3231564E), nv12, 1280x720, 276480 kb/s, 25 tbr, 25 tbn
Stream mapping:
  Stream #0:0 -> #0:0 (rawvideo (native) -> rawvideo (native))
Press [q] to stop, [?] for help
[graph 0 input from stream 0:0 @ 0x1225080] w:1280 h:720 pixfmt:nv12 tb:1/25 fr:25/1 sar:0/1
[AVHWDeviceContext @ 0x122a2c0] VAAPI driver: Intel iHD driver for Intel(R) Gen Graphics - 22.2.0 ().
[AVHWDeviceContext @ 0x122a2c0] Driver not found in known nonstandard list, using standard behaviour.
[AVHWDeviceContext @ 0x1234780] VAAPI driver: Intel iHD driver for Intel(R) Gen Graphics - 22.2.0 ().
[AVHWDeviceContext @ 0x1234780] Driver not found in known nonstandard list, using standard behaviour.
[Parsed_vpp_qsv_2 @ 0x1222640] VPP: input is video memory surface
[Parsed_vpp_qsv_2 @ 0x1222640] VPP: output is video memory surface
Segmentation fault (core dumped)

[mkver]

1. A backtrace would be nice.
2. I only have one clue as to what is wrong with the new code. Does MFXVideoVPP_RunFrameVPPAsync() expect the memory pointed to by the mfxSyncPoint *syncp parameter to stay valid until the corresponding MFXVideoCORE_SyncOperation() operation (according to a1335149fd610b16459d9281b611282cac51c950, this happens for MFXVideoENCODE_EncodeFrameAsync())? In this case 85c938fa287c61334d01adfb038ca47bed6d106c indeed changed something and the following diff should restore the old behaviour:

diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
index 35769dfd60..6e3ebcc7e0 100644
--- a/libavfilter/qsvvpp.c
+++ b/libavfilter/qsvvpp.c
@@ -795,7 +795,6 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *picr
     AVFilterContext  *ctx     = inlink->dst;
     AVFilterLink     *outlink = ctx->outputs[0];
     QSVAsyncFrame     aframe;
-    mfxSyncPoint      sync;
     QSVFrame         *in_frame, *out_frame, *tmp;
     int               ret, filter_ret;
 
@@ -832,7 +831,7 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *picr
 
         do {
             ret = MFXVideoVPP_RunFrameVPPAsync(s->session, &in_frame->surface,
-                                               &out_frame->surface, NULL, &sync);
+                                               &out_frame->surface, NULL, &aframe.sync);
             if (ret == MFX_WRN_DEVICE_BUSY)
                 av_usleep(500);
         } while (ret == MFX_WRN_DEVICE_BUSY);
@@ -847,7 +846,7 @@ int ff_qsvvpp_filter_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *picr
                                              default_tb, outlink->time_base);
 
         out_frame->queued++;
-        aframe = (QSVAsyncFrame){ sync, out_frame };
+        aframe.frame = out_frame;
         av_fifo_write(s->async_fifo, &aframe, 1);
 
         if (av_fifo_can_read(s->async_fifo) > s->async_depth) {

3. Should my guess in 2. turn out to be correct, then I am not sure whether the old code was indeed correct or just happened to work: Would one not need to store each sync point in a way that makes them valid until the corresponding MFXVideoCORE_SyncOperation() (e.g. in the way a1335149fd610b16459d9281b611282cac51c950 does it)?

[U. Artie Eoff]

Anton (elenril) is already working on a fix... see ​https://up.khirnov.net/QK.diff. That patch fixes it on my side.

[mkver]

Ok, forgetting to update the usages of tmp in 85c938fa287c61334d01adfb038ca47bed6d106c is obviously wrong.

[U. Artie Eoff]

Also discovered by Coverity: ​https://scan3.scan.coverity.com/reports.htm#v45412/p14862/fileInstanceId=67465325&defectInstanceId=11122424&mergedDefectId=373734

[mkver]

Fixed in aeaefc3728cb46dc1fb2c363785fed482f358d17.