[FFmpeg-devel] [RFC 3/4] vaapi: add new API to configure a VA pipeline.

wm4 nfxjfg at googlemail.com
Wed Aug 19 21:57:14 CEST 2015


On Wed, 19 Aug 2015 19:32:27 +0200
Gwenole Beauchesne <gb.devel at gmail.com> wrote:

> 2015-08-19 19:19 GMT+02:00 Gwenole Beauchesne <gb.devel at gmail.com>:
> > Hi,
> >
> > 2015-08-19 18:50 GMT+02:00 wm4 <nfxjfg at googlemail.com>:
> >> On Wed, 19 Aug 2015 18:01:36 +0200
> >> Gwenole Beauchesne <gb.devel at gmail.com> wrote:
> >>
> >>> Add av_vaapi_set_pipeline_params() interface to initialize the internal
> >>> FFVAContext with a valid VA display, and optional parameters including
> >>> a user-supplied VA context id.
> >>>
> >>> This is preparatory work for delegating VA pipeline (decoder) creation
> >>> to libavcodec. Meanwhile, if this new interface is used, the user shall
> >>> provide the VA context id and a valid AVCodecContext.get_buffer2() hook.
> >>> Otherwise, the older vaapi_context approach is still supported.
> >>>
> >>> This is an API change. The whole vaapi_context structure is no longer
> >>> needed, and thus deprecated.
> >>>
> >>> Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne at intel.com>
> >>> ---
> >>>  doc/APIchanges              |  6 ++--
> >>>  libavcodec/vaapi.c          | 87 ++++++++++++++++++++++++++++++++++++++++-----
> >>>  libavcodec/vaapi.h          | 49 +++++++++++++++++++++++--
> >>>  libavcodec/vaapi_internal.h |  4 +++
> >>>  4 files changed, 134 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/doc/APIchanges b/doc/APIchanges
> >>> index aa92b69..fcdaa26 100644
> >>> --- a/doc/APIchanges
> >>> +++ b/doc/APIchanges
> >>> @@ -16,8 +16,10 @@ libavutil:     2014-08-09
> >>>  API changes, most recent first:
> >>>
> >>>  2015-xx-xx - lavc 56.58.100 - vaapi.h
> >>> -  Deprecate old VA-API context (vaapi_context) fields that were only
> >>> -  set and used by libavcodec. They are all managed internally now.
> >>> +  Add new API to configure the hwaccel/vaapi pipeline, currently
> >>> +  useful for decoders: av_vaapi_set_pipeline_params()
> >>> +  Deprecate old VA-API context (vaapi_context) structure, which is no
> >>> +  longer used for initializing a VA-API decode pipeline.
> >>>
> >>>  2015-xx-xx - lavu 54.31.100 - pixfmt.h
> >>>    Add a unique pixel format for VA-API (AV_PIX_FMT_VAAPI) that
> >>> diff --git a/libavcodec/vaapi.c b/libavcodec/vaapi.c
> >>> index c081bb5..afddbb3 100644
> >>> --- a/libavcodec/vaapi.c
> >>> +++ b/libavcodec/vaapi.c
> >>> @@ -41,24 +41,95 @@ static void destroy_buffers(VADisplay display, VABufferID *buffers, unsigned int
> >>>      }
> >>>  }
> >>>
> >>> +/** @name VA-API pipeline parameters (internal) */
> >>> +/**@{*/
> >>> +/** Pipeline configuration flags (AV_HWACCEL_FLAG_*|AV_VAAPI_PIPELINE_FLAG_*) */
> >>> +#define AV_VAAPI_PIPELINE_PARAM_FLAGS           "flags"
> >>> +/** User-supplied VA display handle */
> >>> +#define AV_VAAPI_PIPELINE_PARAM_DISPLAY         "display"
> >>> +/**@}*/
> >>> +
> >>> +#define OFFSET(x) offsetof(FFVAContext, x)
> >>> +static const AVOption FFVAContextOptions[] = {
> >>> +    { AV_VAAPI_PIPELINE_PARAM_FLAGS, "flags", OFFSET(flags),
> >>> +      AV_OPT_TYPE_INT, { .i64 = 0 }, 0, UINT32_MAX },
> >>> +    { AV_VAAPI_PIPELINE_PARAM_DISPLAY, "VA display", OFFSET(user_display),
> >>> +      AV_OPT_TYPE_INT64, { .i64 = 0 }, 0, UINTPTR_MAX },
> >>> +    { AV_VAAPI_PIPELINE_PARAM_CONTEXT, "VA context id", OFFSET(user_context_id),
> >>> +      AV_OPT_TYPE_INT, { .i64 = VA_INVALID_ID }, 0, UINT32_MAX },
> >>> +    { NULL, }
> >>> +};
> >>> +#undef OFFSET
> >>> +
> >>> +static const AVClass FFVAContextClass = {
> >>> +    .class_name = "FFVAContext",
> >>> +    .item_name  = av_default_item_name,
> >>> +    .option     = FFVAContextOptions,
> >>> +    .version    = LIBAVUTIL_VERSION_INT,
> >>> +};
> >>> +
> >>> +int av_vaapi_set_pipeline_params(AVCodecContext *avctx, VADisplay display,
> >>> +                                 uint32_t flags, AVDictionary **params)
> >>> +{
> >>> +    int ret;
> >>> +
> >>> +    if (!display) {
> >>> +        av_log(avctx, AV_LOG_ERROR, "No valid VA display supplied.\n");
> >>> +        return AVERROR(EINVAL);
> >>> +    }
> >>> +
> >>> +    if (params && *params)
> >>> +        av_dict_copy(&avctx->internal->hwaccel_config, *params, 0);
> >>> +
> >>> +    ret = av_dict_set_int(&avctx->internal->hwaccel_config,
> >>> +                          AV_VAAPI_PIPELINE_PARAM_FLAGS, flags, 0);
> >>> +    if (ret != 0)
> >>> +        return ret;
> >>> +
> >>> +    return av_dict_set_int(&avctx->internal->hwaccel_config,
> >>> +                           AV_VAAPI_PIPELINE_PARAM_DISPLAY,
> >>> +                           (int64_t)(intptr_t)display, 0);
> >>> +}
> >>> +
> >>>  int ff_vaapi_context_init(AVCodecContext *avctx)
> >>>  {
> >>>      FFVAContext * const vactx = ff_vaapi_get_context(avctx);
> >>> -    const struct vaapi_context * const user_vactx = avctx->hwaccel_context;
> >>> +    int ret;
> >>>
> >>> -    if (!user_vactx) {
> >>> -        av_log(avctx, AV_LOG_ERROR, "Hardware acceleration context (hwaccel_context) does not exist.\n");
> >>> -        return AVERROR(ENOSYS);
> >>> -    }
> >>> +    vactx->klass = &FFVAContextClass;
> >>> +    av_opt_set_defaults(vactx);
> >>>
> >>> -    vactx->display              = user_vactx->display;
> >>> -    vactx->config_id            = user_vactx->config_id;
> >>> -    vactx->context_id           = user_vactx->context_id;
> >>> +#if FF_API_VAAPI_CONTEXT
> >>> +FF_DISABLE_DEPRECATION_WARNINGS
> >>> +    if (avctx->hwaccel_context) {
> >>> +        const struct vaapi_context * const user_vactx = avctx->hwaccel_context;
> >>> +        vactx->user_display     = (uintptr_t)user_vactx->display;
> >>> +        vactx->user_context_id  = user_vactx->context_id;
> >>> +    }
> >>> +FF_ENABLE_DEPRECATION_WARNINGS
> >>> +#endif
> >>>
> >>> +    vactx->context_id           = VA_INVALID_ID;
> >>>      vactx->pic_param_buf_id     = VA_INVALID_ID;
> >>>      vactx->iq_matrix_buf_id     = VA_INVALID_ID;
> >>>      vactx->bitplane_buf_id      = VA_INVALID_ID;
> >>>
> >>> +    ret = av_opt_set_dict(vactx, &avctx->internal->hwaccel_config);
> >>> +    if (ret != 0)
> >>> +        return ret;
> >>> +
> >>> +    vactx->display              = (void *)(uintptr_t)vactx->user_display;
> >>> +    vactx->context_id           = vactx->user_context_id;
> >>> +
> >>> +    if (!vactx->display) {
> >>> +        av_log(avctx, AV_LOG_ERROR, "No valid VA display found.\n");
> >>> +        return AVERROR(EINVAL);
> >>> +    }
> >>> +
> >>> +    if (vactx->context_id != VA_INVALID_ID && !avctx->get_buffer2) {
> >>> +        av_log(avctx, AV_LOG_ERROR, "No AVCodecContext.get_buffer2() hooked defined.\n");
> >>> +        return AVERROR(EINVAL);
> >>> +    }
> >>>      return 0;
> >>>  }
> >>>
> >>> diff --git a/libavcodec/vaapi.h b/libavcodec/vaapi.h
> >>> index 4448a2e..43f8381 100644
> >>> --- a/libavcodec/vaapi.h
> >>> +++ b/libavcodec/vaapi.h
> >>> @@ -32,7 +32,9 @@
> >>>
> >>>  #include <stdint.h>
> >>>  #include <libavutil/attributes.h>
> >>> +#include <va/va.h>
> >>>  #include "version.h"
> >>> +#include "avcodec.h"
> >>>
> >>>  /**
> >>>   * @defgroup lavc_codec_hwaccel_vaapi VA API Decoding
> >>> @@ -48,7 +50,11 @@
> >>>   * during initialization or through each AVCodecContext.get_buffer()
> >>>   * function call. In any case, they must be valid prior to calling
> >>>   * decoding functions.
> >>> + *
> >>> + * This structure is deprecated. Please refer to pipeline parameters
> >>> + * and associated accessors, e.g. av_vaapi_set_pipeline_params().
> >>>   */
> >>> +#if FF_API_VAAPI_CONTEXT
> >>>  struct vaapi_context {
> >>>      /**
> >>>       * Window system dependent data
> >>> @@ -56,6 +62,7 @@ struct vaapi_context {
> >>>       * - encoding: unused
> >>>       * - decoding: Set by user
> >>>       */
> >>> +    attribute_deprecated
> >>>      void *display;
> >>>
> >>>      /**
> >>> @@ -64,6 +71,7 @@ struct vaapi_context {
> >>>       * - encoding: unused
> >>>       * - decoding: Set by user
> >>>       */
> >>> +    attribute_deprecated
> >>>      uint32_t config_id;
> >>>
> >>>      /**
> >>> @@ -72,9 +80,9 @@ struct vaapi_context {
> >>>       * - encoding: unused
> >>>       * - decoding: Set by user
> >>>       */
> >>> +    attribute_deprecated
> >>>      uint32_t context_id;
> >>>
> >>> -#if FF_API_VAAPI_CONTEXT
> >>>      /**
> >>>       * VAPictureParameterBuffer ID
> >>>       *
> >>> @@ -181,8 +189,45 @@ struct vaapi_context {
> >>>       */
> >>>      attribute_deprecated
> >>>      uint32_t slice_data_size;
> >>> -#endif
> >>>  };
> >>> +#endif
> >>> +
> >>> +/** @name VA-API pipeline parameters */
> >>> +/**@{*/
> >>> +/**
> >>> + * VA context id (uint32_t) [default: VA_INVALID_ID]
> >>> + *
> >>> + * This defines the VA context id to use for decoding. If set, then
> >>> + * the user allocates and owns the handle, and shall supply VA surfaces
> >>> + * through an appropriate hook to AVCodecContext.get_buffer2().
> >>> + */
> >>> +#define AV_VAAPI_PIPELINE_PARAM_CONTEXT         "context"
> >>
> >> I don't understand this. Wouldn't it also be possible to use a
> >> user-defined get_buffer2 function, but with a context created by
> >> libavcodec?
> >
> > Historically, you cannot create a decode pipeline without supplying a
> > fixed number of VA surfaces to vaCreateContext(). You cannot have any
> > guarantee either that re-creating a VA context with an increased
> > number of VA surfaces would still get things working. I have an
> > example of a pretty old and semi-opensource driver that stuffs various
> > local states into the VA context even if they are VA surface specific.

I see... I guess we still want to support these. Does the vaapi API
provide a way to know whether this is required?

> > If we wanted to call get_buffer2() multiple times to get an initial
> > set of VA surfaces that you could use to create a VA context, (i) this
> > would be ugly IMHO, and (ii) there is no guarantee either that the
> > user get_buffer2() impl wouldn't hang/wait until his pool of free VA
> > surfaces fills in again.

I don't think it's ugly... seems like a nice (and optional) hack to
support some older hardware without complicating the normal API too
much.

> For that very particular situation, I believe we could introduce an
> AV_GET_BUFFER_FLAG_NOWAIT flag. But yet, we have no guarantee either
> that the user would actually support this flag if needed. :)

Sounds like a good solution. These surfaces are special as in they need
to stick around until end of decoding too.

Personally I don't see much value in the new API without having
libavcodec handle decoder creation (including profile handling).

This could be skewed by my personal experience; most of my own
hwaccel-specific vaapi code is for decoder creation and profile
handling. I'd like to do my own surface allocation handling, because
vaapi doesn't seem to provide a way to retrieve certain surface
parameters like chroma format and actual surface size. Other API
users will have similar issues. But maybe I'm somehow misled.

> And, I prefer to restrict to clearly defined usages that work, rather
> than allowing multiple combos that would complexify test coverage... I
> believe the ones I exposed in the cover letter are the only practical
> ones to ever be useful, based on a compiled set of feedbacks over past
> years. But I can be wrong. If so, please tell me why would you
> particularly want to have lavc allocate the VA context and you
> providing your own VA surfaces. If creating a VA context is a problem
> for the user, I certainly can expose a utility function that provides
> the necessary decode params. Actually, it was in the plan too:
> av_vaapi_get_decode_params().
> 
> > Because of that, I believe it's better to restrict the usages to what
> > it is strictly permitted, for historical reasons. So, either user
> > creates VA surfaces + VA context, or he lets lavc do so but for both
> > VA surfaces & VA context as well. Besides, as I further distilled in
> > one of the patches, lavc naturally works through allocating buffers
> > itself for SW decoders, that can work, and this is something desired
> > to achieve with hwaccel too. At least for vaapi. i.e. you'd only need
> > to provide a VADisplay handle.

I agree, but there are some minor details that perhaps need to be taken
care of. (See what I wrote above for some remarks.)

> >>> +/**@}*/
> >>> +
> >>> +/**
> >>> + * Defines VA processing pipeline parameters
> >>> + *
> >>> + * This function binds the supplied VA @a display to a codec context
> >>> + * @a avctx.
> >>> + *
> >>> + * The user retains full ownership of the display, and thus shall
> >>> + * ensure the VA-API subsystem was initialized with vaInitialize(),
> >>> + * make due diligence to keep it live until it is no longer needed,
> >>> + * and dispose the associated resources with vaTerminate() whenever
> >>> + * appropriate.
> >>> + *
> >>> + * @note This function has no effect if it is called outside of an
> >>> + * AVCodecContext.get_format() hook.
> >>> + *
> >>> + * @param[in] avctx     the codec context being used for decoding the stream
> >>> + * @param[in] display   the VA display handle to use for decoding
> >>> + * @param[in] flags     zero or more OR'd AV_HWACCEL_FLAG_* or
> >>> + *     AV_VAAPI_PIPELINE_FLAG_* flags
> >>> + * @param[in] params    optional parameters to configure the pipeline
> >>> + * @return 0 on success, an AVERROR code on failure.
> >>> + */
> >>> +int av_vaapi_set_pipeline_params(AVCodecContext *avctx, VADisplay display,
> >>> +                                 uint32_t flags, AVDictionary **params);
> >>>
> >>>  /* @} */
> >>>
> >>> diff --git a/libavcodec/vaapi_internal.h b/libavcodec/vaapi_internal.h
> >>> index 29f46ab..958246c 100644
> >>> --- a/libavcodec/vaapi_internal.h
> >>> +++ b/libavcodec/vaapi_internal.h
> >>> @@ -36,6 +36,10 @@
> >>>   */
> >>>
> >>>  typedef struct {
> >>> +    const void *klass;
> >>> +    uint32_t flags;                     ///< Pipeline flags
> >>> +    uint64_t user_display;              ///< User-supplied VA display
> >>> +    uint32_t user_context_id;           ///< User-supplied VA context ID
> >>>      VADisplay display;                  ///< Windowing system dependent handle
> >>>      VAConfigID config_id;               ///< Configuration ID
> >>>      VAContextID context_id;             ///< Context ID (video decode pipeline)
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel at ffmpeg.org
> >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > Regards,
> > --
> > Gwenole Beauchesne
> > Intel Corporation SAS / 2 rue de Paris, 92196 Meudon Cedex, France
> > Registration Number (RCS): Nanterre B 302 456 199
> 
> 
> 



More information about the ffmpeg-devel mailing list