[FFmpeg-devel] [PATCH 1/9] lavu/hwcontext_qsv: update AVQSVFramesContext to support dynamic frame pool

[Xiang, Haihao]

On Do, 2024-05-02 at 20:35 +0100, Mark Thompson wrote:
> On 28/04/2024 08:39, Xiang, Haihao wrote:
> > From: Haihao Xiang <haihao.xiang at intel.com>
> > 
> > Add AVQSVFramesContext.info and update the description.
> > 
> > Signed-off-by: Haihao Xiang <haihao.xiang at intel.com>
> > ---
> >  doc/APIchanges            |  3 +++
> >  libavutil/hwcontext_qsv.c |  4 ++--
> >  libavutil/hwcontext_qsv.h | 28 +++++++++++++++++++++++++---
> >  libavutil/version.h       |  4 ++--
> >  4 files changed, 32 insertions(+), 7 deletions(-)
> > 
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 0566fcdcc5..4a434b2877 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-
> > 07
> >  
> >  API changes, most recent first:
> >  
> > +2024-04-xx - xxxxxxxxxx - lavu 59.17.100 - hwcontext_qsv.h
> > +  Add AVQSVFramesContext.info
> > +
> >  2024-04-24 - 8616cfe0890 - lavu 59.16.100 - opt.h
> >    Add AV_OPT_SERIALIZE_SEARCH_CHILDREN.
> >  
> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
> > index c7c7878644..f552811346 100644
> > --- a/libavutil/hwcontext_qsv.c
> > +++ b/libavutil/hwcontext_qsv.c
> > @@ -627,7 +627,7 @@ static mfxStatus frame_alloc(mfxHDL pthis,
> > mfxFrameAllocRequest *req,
> >      QSVFramesContext       *s = ctx->hwctx;
> >      AVQSVFramesContext *hwctx = &s->p;
> >      mfxFrameInfo *i  = &req->Info;
> > -    mfxFrameInfo *i1 = &hwctx->surfaces[0].Info;
> > +    mfxFrameInfo *i1 = hwctx->nb_surfaces ? &hwctx->surfaces[0].Info :
> > hwctx->info;
> >  
> >      if (!(req->Type & MFX_MEMTYPE_VIDEO_MEMORY_PROCESSOR_TARGET) ||
> >          !(req->Type & (MFX_MEMTYPE_FROM_VPPIN | MFX_MEMTYPE_FROM_VPPOUT))
> > ||
> > @@ -1207,7 +1207,7 @@ static int qsv_init_internal_session(AVHWFramesContext
> > *ctx,
> >                                MFX_IOPATTERN_OUT_SYSTEM_MEMORY;
> >      par.AsyncDepth = 1;
> >  
> > -    par.vpp.In = frames_hwctx->surfaces[0].Info;
> > +    par.vpp.In = frames_hwctx->nb_surfaces ? frames_hwctx->surfaces[0].Info
> > : *frames_hwctx->info;
> >  
> >      /* Apparently VPP requires the frame rate to be set to some value,
> > otherwise
> >       * init will fail (probably for the framerate conversion filter). Since
> > we
> > diff --git a/libavutil/hwcontext_qsv.h b/libavutil/hwcontext_qsv.h
> > index e2dba8ad83..9e43a237d4 100644
> > --- a/libavutil/hwcontext_qsv.h
> > +++ b/libavutil/hwcontext_qsv.h
> > @@ -25,8 +25,8 @@
> >   * @file
> >   * An API-specific header for AV_HWDEVICE_TYPE_QSV.
> >   *
> > - * This API does not support dynamic frame pools. AVHWFramesContext.pool
> > must
> > - * contain AVBufferRefs whose data pointer points to an mfxFrameSurface1
> > struct.
> > + * AVHWFramesContext.pool must contain AVBufferRefs whose data pointer
> > points
> > + * to a mfxFrameSurface1 struct.
> >   */
> >  
> >  /**
> > @@ -51,7 +51,29 @@ typedef struct AVQSVDeviceContext {
> >   * This struct is allocated as AVHWFramesContext.hwctx
> >   */
> >  typedef struct AVQSVFramesContext {
> > -    mfxFrameSurface1 *surfaces;
> > +    /**
> > +     * A pointer to a mfxFrameSurface1 or mfxFrameInfo struct
> > +     *
> > +     * When nb_surfaces is non-zero, it is a pointer to a mfxFrameSurface1
> > +     * struct.
> > +     *
> > +     * When nb_surfaces is 0, it is a pointer to a mfxFrameInfo struct, all
> > +     * buffers allocated from the pool have the same mfxFrameInfo.
> > +     */
> > +    union {
> > +        mfxFrameSurface1 *surfaces;
> > +        mfxFrameInfo     *info;
> > +    };
> 
> doc/developer.texi:
> 
> "FFmpeg is mainly programmed in the ISO C11 language, except for the public
> headers which must stay C99 compatible."
> 
> Anonymous unions are therefore not allowed in public headers.

Thanks for pointing it out, I missed this statement.

> 
> Can you explain what you need the info field for, though?  (What is needed but
> can't be inferred elsewhere?  VAAPI in particular is can be mapped here but
> doesn't contain any special information like this.)

The info is used in libavcodec and libavfilter. My thought is we may get the
info from surfaces directly for fixed frame pool in libavcodec and libavfilter,
It is better to have a field too for the info in dynamic frame pool so we
needn't infer the info in libavcodec and libavfilter.

Thanks
Haihao

> 
> > +
> > +    /**
> > +     * Number of frames in the pool
> > +     *
> > +     * It is 0 for dynamic frame pools or
> > AVHWFramesContext.initial_pool_size
> > +     * for fixed frame pools.
> > +     *
> > +     * Note only oneVPL GPU runtime 2.9+ can support dynamic frame pools
> > +     * on d3d11va or vaapi
> > +     */
> >      int            nb_surfaces;
> 
> +1 to adding proper documentation for these fields.
> 
> >  
> >      /**
> > diff --git a/libavutil/version.h b/libavutil/version.h
> > index 735f6832e3..3b5a2e7aaa 100644
> > --- a/libavutil/version.h
> > +++ b/libavutil/version.h
> > @@ -79,8 +79,8 @@
> >   */
> >  
> >  #define LIBAVUTIL_VERSION_MAJOR  59
> > -#define LIBAVUTIL_VERSION_MINOR  16
> > -#define LIBAVUTIL_VERSION_MICRO 101
> > +#define LIBAVUTIL_VERSION_MINOR  17
> > +#define LIBAVUTIL_VERSION_MICRO 100
> >  
> >  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> >                                                 LIBAVUTIL_VERSION_MINOR, \
> 
> Thanks,
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".