[FFmpeg-devel] [PATCH 4/4] vaapi: add interfaces to properly initialize context.

[Gwenole Beauchesne]

Hi,

2015-08-17 19:17 GMT+02:00 Gwenole Beauchesne <gb.devel at gmail.com>:
> Add av_vaapi_context_init() and av_vaapi_context_alloc() helper functions
> so that to properly initialize the vaapi_context structure, and allow for
> future extensions without breaking the API/ABI.
>
> The new version and flags fields are inserted after the last known user
> initialized field, and actually useful field at all, i.e. VA context_id.
> This is safe because the vaapi_context structure was required to be zero
> initialized in the past. And, since those new helper functions and changes
> cause the AV_VAAPI_CONTEXT_VERSION to be bumped, we can track older struct
> requirements from within libavcodec.
>
> Besides, it is now required by design, and actual usage, that the vaapi
> context structure be completely initialized once and for all before the
> AVCodecContext.get_format() function ever completes.
>
> Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne at intel.com>
> ---
>  libavcodec/vaapi.c          | 30 +++++++++++++++++++++++
>  libavcodec/vaapi.h          | 59 +++++++++++++++++++++++++++++++++++++++++----
>  libavcodec/vaapi_internal.h |  1 +
>  3 files changed, 85 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/vaapi.c b/libavcodec/vaapi.c
> index 1ae71a5..4d3e2f3 100644
> --- a/libavcodec/vaapi.c
> +++ b/libavcodec/vaapi.c
> @@ -41,11 +41,41 @@ static void destroy_buffers(VADisplay display, VABufferID *buffers, unsigned int
>      }
>  }
>
> +/* Allocates a vaapi_context structure */
> +struct vaapi_context *av_vaapi_context_alloc(unsigned int flags)
> +{
> +    struct vaapi_context *vactx;
> +
> +    vactx = av_malloc(sizeof(*vactx));
> +    if (!vactx)
> +        return NULL;
> +
> +    av_vaapi_context_init(vactx, AV_VAAPI_CONTEXT_VERSION, flags);
> +    return vactx;
> +}
> +
> +/* Initializes a vaapi_context structure with safe defaults */
> +void av_vaapi_context_init(struct vaapi_context *vactx, unsigned int version,
> +                           unsigned int flags)
> +{
> +    vactx->display      = NULL;
> +    vactx->config_id    = VA_INVALID_ID;
> +    vactx->context_id   = VA_INVALID_ID;
> +
> +    if (version > 0) {
> +        vactx->version  = version;
> +        vactx->flags    = flags;
> +    }
> +}
> +
>  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;
>
> +    if (user_vactx->version > 0)
> +        vactx->flags            = user_vactx->flags;
> +
>      vactx->display              = user_vactx->display;
>      vactx->config_id            = user_vactx->config_id;
>      vactx->context_id           = user_vactx->context_id;
> diff --git a/libavcodec/vaapi.h b/libavcodec/vaapi.h
> index 4448a2e..1f032a0 100644
> --- a/libavcodec/vaapi.h
> +++ b/libavcodec/vaapi.h
> @@ -40,14 +40,16 @@
>   * @{
>   */
>
> +#define AV_VAAPI_CONTEXT_VERSION 1
> +
>  /**
>   * This structure is used to share data between the FFmpeg library and
>   * the client video application.
> - * This shall be zero-allocated and available as
> - * AVCodecContext.hwaccel_context. All user members can be set once
> - * during initialization or through each AVCodecContext.get_buffer()
> - * function call. In any case, they must be valid prior to calling
> - * decoding functions.
> + *
> + * This shall be initialized with av_vaapi_context_init(), or
> + * indirectly through av_vaapi_context_alloc(), and made available as
> + * AVCodecContext.hwaccel_context. All user members must be properly
> + * initialized before AVCodecContext.get_format() completes.
>   */
>  struct vaapi_context {
>      /**
> @@ -74,6 +76,29 @@ struct vaapi_context {
>       */
>      uint32_t context_id;
>
> +    /**
> +     * This field must be set to AV_VAAPI_CONTEXT_VERSION
> +     *
> +     * @since Version 1.
> +     *
> +     * - encoding: unused
> +     * - decoding: Set by user, through av_vaapi_context_init()
> +     */
> +    unsigned int version;
> +
> +    /**
> +     * A bit field configuring the internal context used by libavcodec
> +     *
> +     * This is a combination of flags from common AV_HWACCEL_FLAG_xxx and
> +     * from VA-API specific AV_VAAPI_FLAG_xxx.
> +     *
> +     * @since Version 1.
> +     *
> +     * - encoding: unused
> +     * - decoding: Set by user, through av_vaapi_context_init()
> +     */
> +    unsigned int flags;
> +
>  #if FF_API_VAAPI_CONTEXT
>      /**
>       * VAPictureParameterBuffer ID
> @@ -184,6 +209,30 @@ struct vaapi_context {
>  #endif
>  };
>
> +/**
> + * Allocates a vaapi_context structure.
> + *
> + * This function allocates and initializes a vaapi_context with safe
> + * defaults, e.g. with proper invalid ids for VA config and context.
> + *
> + * The resulting structure can be deallocated with av_freep().
> + *
> + * @param[in]  flags    zero, or a combination of AV_HWACCEL_FLAG_xxx or
> + *     AV_VAAPI_FLAG_xxx flags OR'd altogether.
> + * @return Newly-allocated struct vaapi_context, or NULL on failure
> + */
> +struct vaapi_context *av_vaapi_context_alloc(unsigned int flags);
> +
> +/**
> + * Initializes a vaapi_context structure with safe defaults.
> + *
> + * @param[in]  version  this must be set to AV_VAAPI_CONTEXT_VERSION
> + * @param[in]  flags    zero, or a combination of AV_HWACCEL_FLAG_xxx or
> + *     AV_VAAPI_FLAG_xxx flags OR'd altogether.
> + */
> +void av_vaapi_context_init(struct vaapi_context *vactx, unsigned int version,
> +                           unsigned int flags);
> +
>  /* @} */

For this one, I am not terribly decided yet. The new trend is that I'd
now prefer flags to be zero'ed by default and explicitly set through
the structure, and not through that helper. Rationale: cosmetically,
due to the name of the probable options here, it looks better to OR
those afterwards. :)

Opinions welcome. Thanks.

>
>  #endif /* AVCODEC_VAAPI_H */
> diff --git a/libavcodec/vaapi_internal.h b/libavcodec/vaapi_internal.h
> index 29f46ab..a7c69e6 100644
> --- a/libavcodec/vaapi_internal.h
> +++ b/libavcodec/vaapi_internal.h
> @@ -36,6 +36,7 @@
>   */
>
>  typedef struct {
> +    unsigned int flags;                 ///< Configuration flags
>      VADisplay display;                  ///< Windowing system dependent handle
>      VAConfigID config_id;               ///< Configuration ID
>      VAContextID context_id;             ///< Context ID (video decode pipeline)
> --
> 1.9.1
>



-- 
Gwenole Beauchesne
Intel Corporation SAS / 2 rue de Paris, 92196 Meudon Cedex, France
Registration Number (RCS): Nanterre B 302 456 199