[FFmpeg-devel] [RFC 3/4] vaapi: add new API to configure a VA pipeline.
wm4
nfxjfg at googlemail.com
Fri Aug 21 11:27:20 CEST 2015
On Wed, 19 Aug 2015 23:25:17 +0200
Gwenole Beauchesne <gb.devel at gmail.com> wrote:
> Hi,
>
> 2015-08-19 21:57 GMT+02:00 wm4 <nfxjfg at googlemail.com>:
> > 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?
Sorry, I completely forgot to reply to this...
> No, since it's specified to work that way. :)
>
> However, I can tell you that: AFAIK, all VA drivers will work as
> described (vaCreateContext() + fixed number of VA surfaces required
> for decode) except the open source Intel HD Graphics VA driver.
> Though, iirc even with it I had to store extra data in the VA context
> but only for H.264 MVC decoding on anything older than Haswell.
OK. The vdpau wrapper driver doesn't need it either.
> >> > 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.
>
> This is exactly what I would like to know, based on experience: why
> would you want to go the pain with (i) determine & map surface
> parameters to use, (ii) allocate & maintain your own pool of VA
> surfaces, (iii) determine & map decoder profile to use if libavcodec
> can do all of that for free for you?
>
> As I mentioned, I can concede to:
> 1. Expose an av_vaapi_get_surface_params() function that gives you the
> right surface chroma format and size params to use ;
(Assuming this suggestion is for use with the proposed libavcodec vaapi
surface allocator.)
This would have to work on an AVFrame, because everything else would
cause too much of a mess due to the disconnect between decoder
reinitialization and return of decoded frames. (The same reason API
users are encouraged to use AVFrame fields instead of AVCodecContext
fields.)
From what I know, there's no way to retrieve this information from an
opaque vaapi surface, though.
What do you think of adding an additional HW surface info struct to
each AVFrame? For example, AVFrame.data[2] could point to an allocated
struct, which has fields describing the HW-API specific properties of
the surface in AVFrame.data[3].
> 2. Expose an av_vaapi_get_decoder_params() function that gives you the
> right profile to use based on the actual underlying HW capabilities.
>
> With that, this can already reduce quite a bit of code (and possible
> errors) in user code and (1) is just a matter of calling
> vaCreateSurfaces(), and (2) is just a matter of calling
> vaCreateContext(). Once you have that, this is not too far from
> calling av_vaapi_set_pipeline_params(). There is still some code to
> write for (ii) in user application if the user really wants such
> cases.
Sounds possible.
> That's why, I really would like to know why you would want to do it
> manually. This is technically possible, but I would like to understand
> the real benefits over just having lavc to manage that kind of things
> itself. I think that for the "advanced" user like you that really
> really wants to allocate your own VA surfaces (but why? again?), you
> can still use the old-school way, i.e. supply the VA context yourself
> then, but with the new API (hwaccel_context-free).
Well, it's not like I want to have all that extra code for managing
surfaces in user application code (including my own). So I'd like very
much to be convinced that it's unnecessary. I don't know the inner
working of vaapi well enough to judge how important it is to be able to
override surface creation completely.
But I know the API was extended relatively recently to pass a list of
surface attributes for vaCreateSurfaces(). There's an extendable list
of "surface attributes" (VASurfaceAttribType), for which at least
VASurfaceAttribMemoryType, VASurfaceAttribUsageHint, and
VASurfaceAttribExternalBufferDescriptor matter for surface creation.
(The latter sounds pretty scary - everything down to the pixel data
pointer can be configured.)
Can these things really served by the API?
(Not like I plan to use them if I don't have to. But the API shouldn't
be a "toy" API either, that is useful for the simplest cases only, and
requires going all manual if you want a little bit more control.)
I don't insist on any of this - but in my opinion this should be
thought through. Being able to retrieve basic information like
underlying hardware pixel format is essential, though.
> Having said that, even if I do (1) and (2), they will live in a
> separate/subsequent patch as I wanted to move towards this "lavc
> managed HW resources" path gradually. i.e. I don't want to change the
> comment as this exactly matches what lavc would do/user has to do at
> this very precise point in the series.
>
> >> 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
> >>
> >>
> >>
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
>
More information about the ffmpeg-devel
mailing list