Quoting wm4 (2017-09-26 17:09:56)
> From: wm4 <[email protected]>
>
> This adds a new API, which allows the API user to query the required
> AVHWFramesContext parameters. This also reduces code duplication across
> the hwaccels by introducing ff_decode_get_hw_frames_ctx(), which uses
> the new API function. It takes care of initializing the hw_frames_ctx
> if needed, and does additional error handling and API usage checking.
>
> Support for VDA and Cuvid missing.
> ---
> Added the rest of the codecs, fixed the other TODO items.
>
> Cuvid should be able to support it, but I still didn't manage to
> install the Cuda SDK. I don't care about VDA, it should just be
> removed.
>
> Ready to be applied, if you ask me.
> ---
> doc/APIchanges | 3 +
> libavcodec/avcodec.h | 82 +++++++++++++++++
> libavcodec/decode.c | 67 ++++++++++++++
> libavcodec/decode.h | 9 ++
> libavcodec/dxva2.c | 55 +++++------
> libavcodec/dxva2_h264.c | 3 +
> libavcodec/dxva2_hevc.c | 3 +
> libavcodec/dxva2_internal.h | 3 +
> libavcodec/dxva2_mpeg2.c | 3 +
> libavcodec/dxva2_vc1.c | 5 +
> libavcodec/vaapi_decode.c | 216
> +++++++++++++++++++++-----------------------
> libavcodec/vaapi_decode.h | 5 +-
> libavcodec/vaapi_h264.c | 1 +
> libavcodec/vaapi_hevc.c | 1 +
> libavcodec/vaapi_mpeg2.c | 1 +
> libavcodec/vaapi_mpeg4.c | 2 +
> libavcodec/vaapi_vc1.c | 2 +
> libavcodec/vaapi_vp8.c | 1 +
> libavcodec/vdpau.c | 58 ++++++------
> libavcodec/vdpau_h264.c | 1 +
> libavcodec/vdpau_hevc.c | 1 +
> libavcodec/vdpau_internal.h | 2 +
> libavcodec/vdpau_mpeg12.c | 1 +
> libavcodec/vdpau_mpeg4.c | 1 +
> libavcodec/vdpau_vc1.c | 2 +
> libavcodec/version.h | 2 +-
> 26 files changed, 349 insertions(+), 181 deletions(-)
>
Okay, this is less icky than I expected.
> diff --git a/doc/APIchanges b/doc/APIchanges
> index fa27007f44..aa419dc0e9 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil: 2017-03-23
>
> API changes, most recent first:
>
> +2017-xx-xx - xxxxxxx - lavc 58.5.0 - avcodec.h
> + Add avcodec_fill_hw_frames_parameters().
> +
> 2017-xx-xx - xxxxxxx - lavu 56.6.0 - pixdesc.h
> Add av_color_range_from_name(), av_color_primaries_from_name(),
> av_color_transfer_from_name(), av_color_space_from_name(), and
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 162f1abe4b..0d1e1e5254 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2990,6 +2990,16 @@ typedef struct AVHWAccel {
> * Internal hwaccel capabilities.
> */
> int caps_internal;
> +
> + /**
> + * Fill the given hw_frames context with current codec parameters. Called
> + * from get_format. Refer to avcodec_fill_hw_frames_parameters() for
> + * details.
> + *
> + * This CAN be called before AVHWAccel.init is called, and you must
> assume
> + * that avctx->hwaccel_priv_data is invalid.
> + */
> + int (*frame_params)(AVCodecContext *avctx, AVBufferRef *hw_frames_ctx);
> } AVHWAccel;
>
> /**
> @@ -3436,6 +3446,78 @@ int avcodec_open2(AVCodecContext *avctx, const AVCodec
> *codec, AVDictionary **op
> */
> int avcodec_close(AVCodecContext *avctx);
>
> +/**
> + * Fill the given hw_frames_ctx (or actually, the AVHWFramesContext pointed
> to
> + * by it) with values adequate for hardware decoding. This is meant to get
> + * called from the get_format callback, and is a helper for preparing a
> + * AVHWFramesContext for AVCodecContext.hw_frames_ctx. This API is for
> decoding
> + * with certain hardware acceleration modes/APIs only. Calling this function
> is
> + * not a requirement, but strongly encouraged, unless you know what you are
> + * doing.
I mildly dislike the last sentence. This should be a convenience
function intended to simplify user's lives in common cases, with manual
creation still fully supported and allowed. Statements like these in the
docs might make people think manual creation is something like a second
class citizen, which it certainly should not be.
> + *
> + * Alternatively to this, an API user can set AVCodecContext.hw_device_ctx,
> + * which sets up AVCodecContext.hw_frames_ctx fully automatically, and makes
> + * it unnecessary to call this function or having to care about
> + * AVHWFramesContext initialization at all.
> + *
> + * There are a number of requirements for calling this function:
> + *
> + * - It must be called from get_format with the same avctx parameter you are
> + * going to use for decoding. Calling it outside of get_format is not
> allowed,
> + * and can trigger undefined behavior.
> + * - If the decoder does not support this functionality, AVERROR(ENOENT) will
> + * be returned. This happens only if the format is a software format, or if
> + * the decoder does not support custom allocated AVHWFramesContext
> properly.
> + * Be aware that even if this returns successfully, hwaccel initialization
> + * could fail later.
> + * - The hw_pix_fmt must be one of the choices suggested by get_format. If
> the
> + * user decides to use a hw_frames_ctx prepared with this API function, the
> + * user must return the same hw_pix_fmt from get_format.
> + * - The hw_frames_ctx must be allocated from a device type that supports the
> + * given hw_pix_fmt.
> + * - The passed hw_frames_ctx must not have been initialized yet.
> + * - The API function may overwrite any fields in the hw_frames_ctx. It will
> not
> + * actually initialize the context. A user calls this API function to get
> + * basic parameters, and can afterwards modify the parameters as needed.
> + * - After calling this API function, it is the user's responsibility to
> + * initialize the hw_frames_ctx, and to set AVCodecContext.hw_frames_ctx
> + * to it. If done, this must be done before returning from get_format (this
> + * is implied by the normal AVCodecContext.hw_frames_ctx API rules).
> + * - The hw_frames_ctx parameters may change every time time get_format is
> + * called. Also, AVCodecContext.hw_frames_ctx is reset before get_format.
> So
> + * you are inherently required to go through this process again on every
> + * get_format call.
> + * - It is perfectly possible to call this function without actually using
> + * the resulting hw_frames_ctx. One use-case might be trying to reuse a
> + * previously initialized hw_frames_ctx, and calling this API function only
> + * to test whether the required frame parameters have changed.
> + *
> + * The function will set at least the following fields on hw_frames_ctx
> + * (potentially more, depending on hwaccel API):
> + *
> + * - Set the format field to hw_pix_fmt.
> + * - Set the sw_format field to the most suited and most versatile format.
> (An
> + * implication is that this will prefer generic formats over opaque formats
> + * with arbitrary restrictions, if possible.)
> + * - Set the width/height fields to the coded frame size, rounded up to the
> + * API-specific minimum alignment.
> + * - Only _if_ the hwaccel requires a pre-allocated pool: set the
> initial_pool_size
> + * field to the number of maximum reference surfaces possible with the
> codec,
> + * plus 1 surface for the user to work (meaning the user can safely
> reference
> + * at most 1 decoded surface at a time), plus additional buffering
> introduced
> + * by frame threading. If the hwaccel does not require pre-allocation, the
> + * field is left to 0, and the decoder will allocate new surfaces on demand
> + * during decoding.
> + *
> + * @param avctx The context which is currently calling get_format, and which
> + * implicitly contains all state needed for filling
> hw_frames_ctx
> + * properly.
> + * @param hw_pix_fmt The hwaccel format you are going to return from
> get_format.
> + * @param hw_frames_ctx A reference to an _uninitialized_ AVHWFramesContext.
> + * Fields will be set to values required for decoding.
The frames context can potentially contain user-allocated content which
is then freed in the user-set free() callback. The semantics of that
memory management is then unclear.
Is there any reason the av_hwframe_ctx_alloc() call must be done by the
user? Doing it inside this function (and so changing the last parameter
to AVBufferRef**) seems to avoid plenty of weird corner cases with no
disadvantages.
Then we have to say whether this function is allowed to set the
opaque/free fields and any possibly-allocated fields in the
type-specific contexts (e.g. attributes for vaapi). Forbidding it makes
it all simpler but may bite us later when it turns out they are
necessary because some hwaccel is particularly insane.
--
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
