[FFmpeg-devel] [PATCH 01/13] lavc: Add codec metadata to indicate hardware support

[Philip Langdale]

On 2017-11-18 10:47, Mark Thompson wrote:
> ---
>  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 d336f6ce22..a3c5e21765 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,9 @@ libavutil:     2017-10-21
> 
>  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.3.100 - avcodec.h
>    Add avcodec_get_hw_frames_parameters().
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 442b558d4b..5bbeb67a0d 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -36,6 +36,7 @@
>  #include "libavutil/channel_layout.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"
> @@ -3279,6 +3280,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 context is 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;
> +
>  typedef struct AVCodecDefault AVCodecDefault;
> 
>  struct AVSubtitle;
> @@ -3404,6 +3460,15 @@ typedef struct AVCodec {
>       * packets before decoding.
>       */
>      const char *bsfs;
> +
> +    /**
> +     * Array of pointers to hardware configurations supported by the 
> codec,
> +     * or NULL if no hardware supported.  The array is terminated by a 
> NULL
> +     * pointer.
> +     *
> +     * The user can only access this field via 
> avcodec_get_hw_config().
> +     */
> +    const struct AVCodecHWConfigInternal **hw_configs;
>  } AVCodec;
> 
>  #if FF_API_CODEC_GET_SET
> @@ -3414,6 +3479,15 @@ int av_codec_get_max_lowres(const AVCodec 
> *codec);
>  struct MpegEncContext;
> 
>  /**
> + * Retrieve supported hardware configurations for a codec.
> + *
> + * Values of index from zero to some maximum return the indexed 
> configuration
> + * descriptor; all other values return NULL.  If the codec does not 
> support
> + * any hardware configurations then it will always return NULL.
> + */
> +const AVCodecHWConfig *avcodec_get_hw_config(const AVCodec *codec, int 
> index);
> +
> +/**
>   * @defgroup lavc_hwaccel AVHWAccel
>   * @{
>   */
> diff --git a/libavcodec/hwaccel.h b/libavcodec/hwaccel.h
> index 124fbbf1fd..0198c7f858 100644
> --- a/libavcodec/hwaccel.h
> +++ b/libavcodec/hwaccel.h
> @@ -19,6 +19,24 @@
>  #ifndef AVCODEC_HWACCEL_H
>  #define AVCODEC_HWACCEL_H
> 
> +#include "avcodec.h"
> +
> +
>  #define HWACCEL_CAP_ASYNC_SAFE      (1 << 0)
> 
> +
> +typedef struct AVCodecHWConfigInternal {
> +    /**
> +     * This is the structure which will be returned to the user by
> +     * avcodec_get_hw_config().
> +     */
> +    AVCodecHWConfig public;
> +    /**
> +     * If this configuration uses a hwaccel, a pointer to it.
> +     * If not, NULL.
> +     */
> +    const AVHWAccel *hwaccel;
> +} AVCodecHWConfigInternal;
> +
> +
>  #endif /* AVCODEC_HWACCEL_H */
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index e50de6e89b..01bf7556f7 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -45,6 +45,7 @@
>  #include "libavutil/thread.h"
>  #include "avcodec.h"
>  #include "decode.h"
> +#include "hwaccel.h"
>  #include "libavutil/opt.h"
>  #include "me_cmp.h"
>  #include "mpegvideo.h"
> @@ -1885,6 +1886,17 @@ int ff_match_2uint16(const uint16_t(*tab)[2],
> int size, int a, int b)
>      return i;
>  }
> 
> +const AVCodecHWConfig *avcodec_get_hw_config(const AVCodec *codec, int 
> index)
> +{
> +    int i;
> +    if (!codec->hw_configs || index < 0)
> +        return NULL;
> +    for (i = 0; i < index; i++)
> +        if (!codec->hw_configs[i])
> +            return NULL;
> +    return &codec->hw_configs[index]->public;

What if 'index' is exactly the NULL terminator. You would dereference 
it, right?
Could use 'i <= index' perhaps?

> +}
> +
>  static AVHWAccel *first_hwaccel = NULL;
>  static AVHWAccel **last_hwaccel = &first_hwaccel;

Otherwise looks fine.

--phil