[FFmpeg-trac] #9629(avfilter:new): regression: qsvvpp segfault with new FIFO API

[FFmpeg]

#9629: regression: qsvvpp segfault with new FIFO API
-------------------------------------+-------------------------------------
             Reporter:  U. Artie     |                    Owner:  (none)
  Eoff                               |
                 Type:  defect       |                   Status:  new
             Priority:  critical     |                Component:  avfilter
              Version:  unspecified  |               Resolution:
             Keywords:  qsvvpp, qsv  |               Blocked By:
             Blocking:               |  Reproduced by developer:  0
Analyzed by developer:  0            |
-------------------------------------+-------------------------------------
Comment (by 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)?
-- 
Ticket URL: <https://trac.ffmpeg.org/ticket/9629#comment:1>
FFmpeg <https://ffmpeg.org>
FFmpeg issue tracker