On Mon, 27 Mar 2017 21:38:12 +0100
Mark Thompson <s...@jkqxz.net> wrote:
> This also adds a new field to AVHWAccel to set supported hardware device
> types, so that we can query the right hwaccel.
> ---
> Not entirely sure that the AVCodecContext should be the first argument of
> probe_hw() - AVCodecParameters might be nicer, but we are likely to end up
> needing an AVCodecContext anyway. (More generally, please bikeshed freely
> over naming and structures used.)
>
> The probe_hw() function on the D*VA hwaccel would be where the extra
> alignment gets set (when AVCodecContext.hw_device_ctx is not used).
>
>
> doc/APIchanges | 3 +++
> libavcodec/avcodec.h | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
> libavcodec/decode.c | 53
> ++++++++++++++++++++++++++++++++++++++++++++-------
> libavcodec/encode.c | 13 +++++++++++++
> libavcodec/internal.h | 10 ++++++++++
> libavcodec/utils.c | 15 +++++++++++++++
> 6 files changed, 138 insertions(+), 7 deletions(-)
>
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 5da821c90..649d35a08 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -13,6 +13,9 @@ libavutil: 2017-03-23
>
> API changes, most recent first:
>
> +2017-04-xx - xxxxxxx - lavc 58.x+1.0 - avcodec.h
> + Add AVHWAccel.device_type and avcodec_probe_hw().
> +
> 2017-04-xx - xxxxxxx - lavc 58.x.y+1 - avcodec.h
> Add AV_HWACCEL_FLAG_ALLOW_PROFILE_MISMATCH.
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 2aa70ca4f..8f7293d02 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -35,6 +35,7 @@
> #include "libavutil/cpu.h"
> #include "libavutil/dict.h"
> #include "libavutil/frame.h"
> +#include "libavutil/hwcontext.h"
> #include "libavutil/log.h"
> #include "libavutil/pixfmt.h"
> #include "libavutil/rational.h"
> @@ -2857,6 +2858,15 @@ typedef struct AVCodec {
> * packets before decoding.
> */
> const char *bsfs;
> +
> + /**
> + * Test support for the given encode/decode parameters, and if possible
> + * set the fields in the frames context to a set of usable values.
> + *
> + * If a hwaccel is used with a decoder and it also has a probe_hw()
> + * function, that will be called instead of this.
> + */
> + int (*probe_hw)(AVCodecContext *avctx, AVHWFramesContext *hw_frames);
> } AVCodec;
>
> /**
> @@ -2898,6 +2908,11 @@ typedef struct AVHWAccel {
> */
> int capabilities;
>
> + /**
> + * Hardware device type which this hwaccel can use.
> + */
> + enum AVHWDeviceType device_type;
> +
> /*****************************************************************
> * No fields below this line are part of the public API. They
> * may not be used outside of libavcodec and can be changed and
> @@ -2988,6 +3003,11 @@ typedef struct AVHWAccel {
> * Internal hwaccel capabilities.
> */
> int caps_internal;
> +
> + /**
> + * Probe hardware for support and parameters.
> + */
> + int (*probe_hw)(AVCodecContext *avctx, AVHWFramesContext *hw_frames);
> } AVHWAccel;
>
> /**
> @@ -4927,6 +4947,37 @@ const AVCodecDescriptor
> *avcodec_descriptor_get_by_name(const char *name);
> AVCPBProperties *av_cpb_properties_alloc(size_t *size);
>
> /**
> + * Probe hardware support for the given codec parameters.
> + *
> + * For decoders, it tests whether a stream with the given parameters can be
> + * decoded on the given device. For encoders, it tests whether encoding with
> + * the given parameters is possible on the device.
> + *
> + * On successful return, the fields of hw_frames_ref are set such that, once
> + * initialised, it will be usable as AVCodecContext.hw_frames_ctx. If the
> + * user has additional requirements to apply to the frames context (for
> + * example, for a larger pool size or for frames created with some system-
> + * specific attribute) then they should be applied after this call but before
> + * calling av_hwframe_ctx_init().
> + *
> + * The device to use is provided as the allocating context of hw_frames_ref.
> + * (Even if set, the hw_frames_ctx and hw_device_ctx fields of avctx are
> + * ignored.)
> + *
> + * @param avctx Codec context with relevant fields set. It need not have
> been
> + * opened.
> + * @param hw_frames_ref A reference to a newly-allocated AVHWFramesContext,
> + * which will be used to return the result.
> + * @return 0 on success, otherwise negative error code:
> + * AVERROR(ENOSYS): This function is not implemented (hardware support
> + * may or may not be available).
> + * AVERROR(ENODEV): The device is not usable.
> + * AVERROR(EINVAL): The codec parameters are not supported.
> + * Other errors: Probing failed for some other reason.
> + */
> +int avcodec_probe_hw(AVCodecContext *avctx, AVBufferRef *hw_frames_ref);
This API still makes me a bit uneasy. Regular get_format hwaccel init
relies on the fact that the decoder has set some fields from parsing
the bit stream, like the current sw_pixfmt or frame size. These aren't
necessarily available on a "fresh" AVCodecContext (not even if
extradata is available).
So normally, the only time this will work properly is in the get_format
callback. Which is ok, but which I think isn't the entire intention? I
think I'd be fine with allowing this call only during get_format or so.
Or maybe it's supposed to return "approximate" results outside of
get_format in the first place.
(For encoding on the other hand it's probably fine - though I'm not
sure if this is supposed to be called before or after opening the
context.)
As a minor nit, this can't distinguish between required initial pool
size and upper bound on pool size. Users who want to preallocate frames
with e.g. vdpau would want to know the latter. (Well, I don't want
that, and see no reason why anyone would want it, so I officially don't
care about this argument.)
I'll make full use of my permission to bikeshed, so here is a different
API that impractically changes everything:
Proposal 1:
Add new init_hw_device and init_hw_frames callbacks:
/*
* Create a hw device and set ctx->hw_device_ctx. This might be used
* for hardware decoding. If ENOSYS is returned, the next hwaccel
* method is tried, and if all fail, software decoding is used.
*
* If hw_device_ctx is already set, this callback is not used, and
* does not need to be set.
*
* If internal reinit is done (like on flush or if the codec
* parameters change), hw_device_ctx is cleared, and this callback
* can be called again.
*/
int (*init_hw_device)(AVCodecContext *ctx, AVHWDeviceType type);
/*
* When this is called, ctx->hw_frames_ctx is already setup in a basic
* way (such as frame dimensions and surface format). This callback
* can be used to refine the frame parameters further.
*/
int (*init_hw_frames)(AVCodecContext *ctx);
Proposal 2:
Add a mechanism to let the caller setup parameters in advance, and
letting the hwaccel code use these parameters without needing callbacks
back to user code.
Proposal 2a:
Add these fields to AVCodecContext:
AVDictionary *hw_frames_params;
void *hw_frames_opaque_params; // AVDictionary can't have pointers
AVHWDeviceType hw_frames_opaque_params_type;
Proposal 2b:
Add this field to AVCodecContext:
/*
* When allocating a new hw_frames_ctx, use the parameters found in
* this context as defaults (if the device type matches). The pixel
* format and size will be changed, and the number of initial pool
* frames can be extended.
*/
AHWFramesContext *hw_frames_template;
Proposal 3:
Use your API, and restrict when/where it's allowed to be called.
> +
> +/**
> * @}
> */
>
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index e4f6a0d72..4029683e6 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -701,13 +701,6 @@ static int is_hwaccel_pix_fmt(enum AVPixelFormat pix_fmt)
> return desc->flags & AV_PIX_FMT_FLAG_HWACCEL;
> }
>
> -enum AVPixelFormat avcodec_default_get_format(struct AVCodecContext *s,
> const enum AVPixelFormat *fmt)
> -{
> - while (*fmt != AV_PIX_FMT_NONE && is_hwaccel_pix_fmt(*fmt))
> - ++fmt;
> - return fmt[0];
> -}
> -
> static AVHWAccel *find_hwaccel(enum AVCodecID codec_id,
> enum AVPixelFormat pix_fmt)
> {
> @@ -753,6 +746,28 @@ static int setup_hwaccel(AVCodecContext *avctx,
> return 0;
> }
>
> +enum AVPixelFormat avcodec_default_get_format(struct AVCodecContext *s,
> const enum AVPixelFormat *fmt)
> +{
> + if (s->hw_device_ctx) {
> + const AVHWDeviceContext *device = (const
> AVHWDeviceContext*)s->hw_device_ctx->data;
> + const AVHWAccel *hwaccel;
> + int i;
> + for (i = 0; fmt[i] != AV_PIX_FMT_NONE; i++) {
> + if (!is_hwaccel_pix_fmt(fmt[i]))
> + continue;
> + hwaccel = find_hwaccel(s->codec_id, fmt[i]);
> + if (!hwaccel)
> + continue;
> + if (hwaccel->device_type == device->type)
> + return fmt[i];
> + }
> + }
> +
> + while (*fmt != AV_PIX_FMT_NONE && is_hwaccel_pix_fmt(*fmt))
> + ++fmt;
> + return fmt[0];
> +}
Does this change normal decode behavior to using hwaccels if
hw_device_ctx is used and get_format isn't set by the user? That's
great, but could use a mention in the commit message. (And I suppose
some would say this should be a separate commit.)
> +
> int ff_get_format(AVCodecContext *avctx, const enum AVPixelFormat *fmt)
> {
> const AVPixFmtDescriptor *desc;
> @@ -1210,3 +1225,27 @@ void ff_decode_bsfs_uninit(AVCodecContext *avctx)
> av_freep(&s->bsfs);
> s->nb_bsfs = 0;
> }
> +
> +int ff_decode_probe_hw(AVCodecContext *avctx, AVHWFramesContext *hw_frames)
> +{
> + const AVCodec *codec = avctx->codec;
> + const AVHWAccel *hwaccel = NULL;
> +
> + if (!codec)
> + return AVERROR(EINVAL);
> +
> + if (!hw_frames->device_ctx)
> + return AVERROR(ENODEV);
> +
> + while ((hwaccel = av_hwaccel_next(hwaccel))) {
> + if (hwaccel->id == avctx->codec_id &&
> + hwaccel->device_type == hw_frames->device_ctx->type)
> + break;
> + }
> + if (hwaccel && hwaccel->probe_hw)
> + return hwaccel->probe_hw(avctx, hw_frames);
> + else if (codec->probe_hw)
> + return codec->probe_hw(avctx, hw_frames);
> + else
> + return AVERROR(ENOSYS);
> +}
> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> index 9bb7ae5bd..d4914d7da 100644
> --- a/libavcodec/encode.c
> +++ b/libavcodec/encode.c
> @@ -354,3 +354,16 @@ int attribute_align_arg
> avcodec_receive_packet(AVCodecContext *avctx, AVPacket *
> avctx->internal->buffer_pkt_valid = 0;
> return 0;
> }
> +
> +int ff_encode_probe_hw(AVCodecContext *avctx, AVHWFramesContext *hw_frames)
> +{
> + const AVCodec *codec = avctx->codec;
> +
> + if (!codec)
> + return AVERROR(EINVAL);
> +
> + if (codec->probe_hw)
> + return codec->probe_hw(avctx, hw_frames);
> + else
> + return AVERROR(ENOSYS);
> +}
> diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> index 403fb4a09..930b32fd6 100644
> --- a/libavcodec/internal.h
> +++ b/libavcodec/internal.h
> @@ -285,4 +285,14 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame
> *frame);
> */
> AVCPBProperties *ff_add_cpb_side_data(AVCodecContext *avctx);
>
> +/**
> + * Probe decoder/hwaccel hardware support.
> + */
> +int ff_decode_probe_hw(AVCodecContext *avctx, AVHWFramesContext *hw_frames);
> +
> +/**
> + * Probe encoder hardware support.
> + */
> +int ff_encode_probe_hw(AVCodecContext *avctx, AVHWFramesContext *hw_frames);
> +
> #endif /* AVCODEC_INTERNAL_H */
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index bc421f67f..2a73098ec 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1685,3 +1685,18 @@ int avcodec_parameters_to_context(AVCodecContext
> *codec,
>
> return 0;
> }
> +
> +int avcodec_probe_hw(AVCodecContext *avctx, AVBufferRef *hw_frames_ref)
> +{
> + AVHWFramesContext *hw_frames = (AVHWFramesContext*)hw_frames_ref->data;
> +
> + switch (avctx->codec_type) {
> + case AVMEDIA_TYPE_VIDEO:
> + if (av_codec_is_encoder(avctx->codec))
> + return ff_encode_probe_hw(avctx, hw_frames);
> + else
> + return ff_decode_probe_hw(avctx, hw_frames);
> + default:
> + return AVERROR(ENOSYS);
> + }
> +}
Otherwise all seems fine.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
