On Thu, 26 Oct 2017 13:27:01 +0100 Mark Thompson <[email protected]> wrote:
> On 26/10/17 11:36, wm4 wrote: > > On Thu, 26 Oct 2017 00:18:39 +0100 > > Mark Thompson <[email protected]> wrote: > > > >> --- > >> Rebased on the frame parameter changes, and incorporating all review > >> comments from last time. > >> > >> > >> doc/APIchanges | 3 +++ > >> libavcodec/avcodec.h | 74 > >> ++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> libavcodec/hwaccel.h | 18 +++++++++++++ > >> libavcodec/utils.c | 12 +++++++++ > >> 4 files changed, 107 insertions(+) > >> > >> diff --git a/doc/APIchanges b/doc/APIchanges > >> index 9f3a1f246..490c71cc2 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.x+1.0 - avcodec.h > >> + Add AVCodecHWConfig and avcodec_get_hw_config(). > >> + > >> 2017-xx-xx - xxxxxxx - lavc 58.5.0 - avcodec.h > >> Add avcodec_get_hw_frames_parameters(). > >> > >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > >> index 562483502..b4bd4ad16 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" > >> @@ -2735,6 +2736,61 @@ typedef struct AVProfile { > >> const char *name; ///< short name for the profile > >> } AVProfile; > >> > >> +enum { > >> + /** > >> + * The codec supports this format via the hw_device_ctx interface. > >> + * > >> + * When selecting this format, AVCodecContext.hw_device_ctx should > >> + * have been set to a device of the specified type before calling > >> + * avcodec_open2(). > >> + */ > >> + AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX = 0x01, > >> + /** > >> + * The codec supports this format via the hw_frames_ctx interface. > >> + * > >> + * When selecting this format for a decoder, > >> + * AVCodecContext.hw_frames_ctx should be set to a suitable frames > >> + * context inside the get_format() callback. The frames context > >> + * must have been created on a device of the specified type. > >> + */ > >> + AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX = 0x02, > >> + /** > >> + * The codec supports this format by some internal method. > >> + * > >> + * This format can be selected without any additional configuration - > >> + * no device or frames are required. > >> + */ > >> + AV_CODEC_HW_CONFIG_METHOD_INTERNAL = 0x04, > >> + /** > >> + * The codec supports this format by some ad-hoc method. > >> + * > >> + * Additional settings and/or function calls are required. See the > >> + * codec-specific documentation for details. (Methods requiring > >> + * this sort of configuration are deprecated and others should be > >> + * used in preference.) > >> + */ > >> + AV_CODEC_HW_CONFIG_METHOD_AD_HOC = 0x08, > >> +}; > >> + > >> +typedef struct AVCodecHWConfig { > >> + /** > >> + * A hardware pixel format which the codec can use. > >> + */ > >> + enum AVPixelFormat pix_fmt; > >> + /** > >> + * Bit set of AV_CODEC_HW_CONFIG_METHOD_* flags, describing the > >> possible > >> + * setup methods which can be used with this configuration. > >> + */ > >> + int methods; > >> + /** > >> + * The device type associated with the configuration. > >> + * > >> + * Must be set for AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX and > >> + * AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX, otherwise unused. > >> + */ > >> + enum AVHWDeviceType device_type; > >> +} AVCodecHWConfig; > > > > All patches LGTM, just 3 things: > > > > 1. I'd really like if the following two things were exposed as metadata: > > - whether the AVCodec supports software decoding at all (fallbacks > > etc.) > > - whether the software decoder is native to Libav, or something > > external (for example I think qsvdec can do software decoding?) > > Yeah, that would make sense. > > Another field "flags", then something like: > * AV_CODEC_HW_CONFIG_FLAG_SUPPORTS_FALLBACK: can switch between this and and > the internal software decoder via get_format(). > * AV_CODEC_HW_CONFIG_FLAG_EXTERNAL_SOFTWARE: external decoder can be a > software implementation. > > ? Looks good. In fact both flags would make sense. > > > 2. Should AVCodecHWConfig have a name that identifies each entry? (The > > name would be unique for the same AVCodec, but not globally.) As we > > move away from identifying hwaccels via only a pixfmt or device type, > > and use AVCodecHWConfig pointers instead, this might help with > > introspection, debugging, command line mappings. > > Can you suggest the sort of names you're thinking of here? I think this > might depend on (3) as well to be useful. I was thinking maybe "hwaccel_vaapi" etc. > > 3. Should there be no "methods" bit field, and instead expose each mode > > as a separate AVCodecHWConfig? It's a bit weird that one entry can > > somehow represent multiple methods. > > I did consider this initially, but it doesn't work very nicely with the > existing pixfmt-based selection (get_format, etc.) because there are then > multiple entries for each pixfmt. With select_config() instead of > get_format(), maybe it would be the right way to go. I'm undecided on what exactly to do too. I guess at a later point we could disallow AVCodecHWConfig entries that conflate too much, and split the existing ones. That bikeshed is closed for now, then. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
