On 26/10/17 13:56, wm4 wrote: > 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.
I'll send a new version with that later. >>> 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. I feel like this sort of name is trivially generated from the other contents of the structure, so coding it here wouldn't really add anything? Also, if explicit names were added to the public API then people might try to use them for non-informational purposes. >>> 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. Sure, splitting them later is fine. - Mark _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
