[FFmpeg-devel] [PATCH 2/9] libavcodec: add amfdec.

[Mark Thompson]

On 14/02/2024 01:55, Dmitrii Ovchinnikov wrote:
> From: Evgeny Pavlov <lucenticus at gmail.com>
> 
> Added AMF based h264, hevc, av1 decoders.
> Co-authored-by: Dmitrii Ovchinnikov <ovchinnikov.dmitrii at gmail.com>
> ---
>   libavcodec/Makefile     |   4 +-
>   libavcodec/allcodecs.c  |   3 +
>   libavcodec/amfdec.c     | 667 ++++++++++++++++++++++++++++++++++++++++
>   libavcodec/amfdec.h     |  75 +++++
>   libavcodec/h264_slice.c |   3 +
>   libavcodec/h264dec.c    |   3 +
>   libavcodec/hwconfig.h   |   2 +
>   7 files changed, 755 insertions(+), 2 deletions(-)
>   create mode 100644 libavcodec/amfdec.c
>   create mode 100644 libavcodec/amfdec.h
> 
> ...
> +
> +static int amf_decode_init(AVCodecContext *avctx)
> +{
> +    AvAmfDecoderContext *ctx = avctx->priv_data;
> +    int ret;
> +    enum AVPixelFormat pix_fmts[3] = {
> +        AV_PIX_FMT_AMF,
> +        avctx->pix_fmt,
> +        AV_PIX_FMT_NONE };
> +
> +    ret = ff_get_format(avctx, pix_fmts);
> +    if (ret < 0) {
> +        avctx->pix_fmt = AV_PIX_FMT_NONE;
> +    }

I think you've misunderstood how decoder setup works.  AVCodecContext.pix_fmt happens to be set to an initial value in some cases which use libavformat (including the ffmpeg utility), but there is no requirement on the user to do so (see the doxy).  Also all of the format information can change at any moment mid-stream (consider adaptive streaming scenarios).

It is therefore necessary for the decoder to parse the input and determine the intended format before calling the get_format callback, and to do that again whenever the format changes.  Calling it once at the beginning does not work at all.

> ...
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index 8464a0b34c..d11821194f 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -864,6 +864,9 @@ static enum AVPixelFormat get_pixel_format(H264Context *h, int force_callback)
>   #if CONFIG_H264_NVDEC_HWACCEL
>           *fmt++ = AV_PIX_FMT_CUDA;
>   #endif
> +#if CONFIG_H264_AMFDEC_HWACCEL
> +        *fmt++ = AV_PIX_FMT_AMF;
> +#endif
>   #if CONFIG_H264_VIDEOTOOLBOX_HWACCEL
>           if (h->avctx->colorspace != AVCOL_SPC_RGB)
>               *fmt++ = AV_PIX_FMT_VIDEOTOOLBOX;
> diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
> index 9f5893c512..7a2c9eecef 100644
> --- a/libavcodec/h264dec.c
> +++ b/libavcodec/h264dec.c
> @@ -1137,6 +1137,9 @@ const FFCodec ff_h264_decoder = {
>   #if CONFIG_H264_NVDEC_HWACCEL
>                                  HWACCEL_NVDEC(h264),
>   #endif
> +#if CONFIG_H264_AMFDEC_HWACCEL
> +                               HWACCEL_AMFDEC(h264),
> +#endif
>   #if CONFIG_H264_VAAPI_HWACCEL
>                                  HWACCEL_VAAPI(h264),
>   #endif

I don't see any acceleration support here at all, this is entirely an offload decoder.

Thanks,

- Mark