On Wed, 19 Aug 2015 19:32:27 +0200
Gwenole Beauchesne <gb.de...@gmail.com> wrote:
> 2015-08-19 19:19 GMT+02:00 Gwenole Beauchesne <gb.de...@gmail.com>:
> > Hi,
> >
> > 2015-08-19 18:50 GMT+02:00 wm4 <nfx...@googlemail.com>:
> >> On Wed, 19 Aug 2015 18:01:36 +0200
> >> Gwenole Beauchesne <gb.de...@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.beauche...@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@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
>
>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
