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. ? > 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. > 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. Thanks, - Mark _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
