[FFmpeg-devel] [PATCH v2] lavfi/qsvvpp: support async depth

[Linjie Fu]

Hi Fei,

On Mon, Mar 15, 2021 at 1:13 PM Fei Wang <fei.w.wang at intel.com> wrote:
>
> Async depth will allow qsv filter cache few frames, and avoid force
> switch and end filter task frame by frame. This change will improve
> performance for some multi-task case, for example 1:N transcode(
> decode + vpp + encode) with all QSV plugins.

Async depth support for qsv vpp is valuable for the performance of
whole qsv pipeline, since both decoding/encoding have already
supported the async_depth.

Hence, would you please help to elaborate more about the details about
the performance improvement for the whole pipeline?
(For examples,  before/after this patch, cmdline, platform and the fps ...)

> Signed-off-by: Fei Wang <fei.w.wang at intel.com>
> ---
> Change: combine used and queued into queued in QSVFrame.
>
>  libavfilter/qsvvpp.c             | 153 ++++++++++++++++++-------------
>  libavfilter/qsvvpp.h             |  41 ++++++++-
>  libavfilter/vf_deinterlace_qsv.c |  14 +--
>  libavfilter/vf_vpp_qsv.c         |  75 ++++++++++++---
>  4 files changed, 193 insertions(+), 90 deletions(-)
>
> diff --git a/libavfilter/qsvvpp.c b/libavfilter/qsvvpp.c
> index f216b3f248..e7c7a12cfa 100644
> --- a/libavfilter/qsvvpp.c
> +++ b/libavfilter/qsvvpp.c
> @@ -27,6 +27,7 @@
>  #include "libavutil/hwcontext_qsv.h"
>  #include "libavutil/time.h"
>  #include "libavutil/pixdesc.h"
> +#include "libavutil/fifo.h"

This seems to be redundant, since you're adding fifo.h in qsvvpp.h as well.

>  #include "internal.h"
>  #include "qsvvpp.h"
> @@ -37,37 +38,6 @@
>  #define IS_OPAQUE_MEMORY(mode) (mode & MFX_MEMTYPE_OPAQUE_FRAME)
>  #define IS_SYSTEM_MEMORY(mode) (mode & MFX_MEMTYPE_SYSTEM_MEMORY)
>
> -typedef struct QSVFrame {
> -    AVFrame          *frame;
> -    mfxFrameSurface1 *surface;
> -    mfxFrameSurface1  surface_internal;  /* for system memory */
> -    struct QSVFrame  *next;
> -} QSVFrame;
> -
> -/* abstract struct for all QSV filters */
> -struct QSVVPPContext {
> -    mfxSession          session;
> -    int (*filter_frame) (AVFilterLink *outlink, AVFrame *frame);/* callback */
> -    enum AVPixelFormat  out_sw_format;   /* Real output format */
> -    mfxVideoParam       vpp_param;
> -    mfxFrameInfo       *frame_infos;     /* frame info for each input */
> -
> -    /* members related to the input/output surface */
> -    int                 in_mem_mode;
> -    int                 out_mem_mode;
> -    QSVFrame           *in_frame_list;
> -    QSVFrame           *out_frame_list;
> -    int                 nb_surface_ptrs_in;
> -    int                 nb_surface_ptrs_out;
> -    mfxFrameSurface1  **surface_ptrs_in;
> -    mfxFrameSurface1  **surface_ptrs_out;
> -
> -    /* MFXVPP extern parameters */
> -    mfxExtOpaqueSurfaceAlloc opaque_alloc;
> -    mfxExtBuffer      **ext_buffers;
> -    int                 nb_ext_buffers;
> -};
> -
>  static const mfxHandleType handle_types[] = {
>      MFX_HANDLE_VA_DISPLAY,
>      MFX_HANDLE_D3D9_DEVICE_MANAGER,
> @@ -336,9 +306,11 @@ static int fill_frameinfo_by_link(mfxFrameInfo *frameinfo, AVFilterLink *link)
>  static void clear_unused_frames(QSVFrame *list)
>  {
>      while (list) {
> -        if (list->surface && !list->surface->Data.Locked) {
> -            list->surface = NULL;
> +        /* list->queued==1 means the frame is not cached in VPP
> +         * process any more, it can be released to pool. */
> +        if ((list->queued == 1) && !list->surface.Data.Locked) {
>              av_frame_free(&list->frame);
> +            list->queued = 0;
>          }
>          list = list->next;
>      }
> @@ -361,8 +333,10 @@ static QSVFrame *get_free_frame(QSVFrame **list)
>      QSVFrame *out = *list;
>
>      for (; out; out = out->next) {
> -        if (!out->surface)
> +        if (!out->queued) {
> +            out->queued = 1;
>              break;
> +        }
>      }
>
>      if (!out) {
> @@ -371,8 +345,9 @@ static QSVFrame *get_free_frame(QSVFrame **list)
>              av_log(NULL, AV_LOG_ERROR, "Can't alloc new output frame.\n");
>              return NULL;
>          }
> -        out->next  = *list;
> -        *list      = out;
> +        out->queued = 1;
> +        out->next   = *list;
> +        *list       = out;
>      }
>
>      return out;
> @@ -402,7 +377,7 @@ static QSVFrame *submit_frame(QSVVPPContext *s, AVFilterLink *inlink, AVFrame *p
>              return NULL;
>          }
>          qsv_frame->frame   = av_frame_clone(picref);
> -        qsv_frame->surface = (mfxFrameSurface1 *)qsv_frame->frame->data[3];
> +        qsv_frame->surface = *(mfxFrameSurface1 *)qsv_frame->frame->data[3];

The type of surface in struct QSVFrame  would be changed fron
*mfxFrameSurface1 to mfxFrameSurface1, and surface_internal would be
removed.
IMO separating the related changes for the structures into a single
commit would make it more explicit, since it's not closely related
with the implemetation of async fifo.

- linjie