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

[Mark Thompson]

On 20/11/17 21:09, Philip Langdale wrote:
> 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?

Ha, yeah - and that error is neatly hidden by the fact that the AVCodecHWConfig is the first element of AVCodecHWConfigInternal, so the returned pointer is still NULL after adding the struct offset.

Fixed.

Thanks,

- Mark