[FFmpeg-devel,01/10,v3] avutil: add hwcontext_amf.

Adds hwcontext_amf, which allows to use shared AMF
context for the encoder, decoder and AMF-based filters,
without copy to the host memory.
It will also allow you to use some optimisations in
the interaction of components (for example, SAV) and make a more
manageable and optimal setup for using GPU devices with AMF
in the case of a fully AMF pipeline.
It will be a significant performance uplift when full AMF pipeline
with filters is used.

We also plan to add Compression artefact removal filter in near feature.
v2: cleanup header files
v3: an unnecessary class has been removed.
---
 libavutil/Makefile                 |   4 +
 libavutil/hwcontext.c              |   4 +
 libavutil/hwcontext.h              |   1 +
 libavutil/hwcontext_amf.c          | 585 +++++++++++++++++++++++++++++
 libavutil/hwcontext_amf.h          |  64 ++++
 libavutil/hwcontext_amf_internal.h |  44 +++
 libavutil/hwcontext_internal.h     |   1 +
 libavutil/pixdesc.c                |   4 +
 libavutil/pixfmt.h                 |   5 +
 9 files changed, 712 insertions(+)
 create mode 100644 libavutil/hwcontext_amf.c
 create mode 100644 libavutil/hwcontext_amf.h
 create mode 100644 libavutil/hwcontext_amf_internal.h

Dmitrii Ovchinnikov:
> Adds hwcontext_amf, which allows to use shared AMF
> context for the encoder, decoder and AMF-based filters,
> without copy to the host memory.
> It will also allow you to use some optimisations in
> the interaction of components (for example, SAV) and make a more
> manageable and optimal setup for using GPU devices with AMF
> in the case of a fully AMF pipeline.
> It will be a significant performance uplift when full AMF pipeline
> with filters is used.
> 
> We also plan to add Compression artefact removal filter in near feature.
> v2: cleanup header files
> v3: an unnecessary class has been removed.
> ---
>  libavutil/Makefile                 |   4 +
>  libavutil/hwcontext.c              |   4 +
>  libavutil/hwcontext.h              |   1 +
>  libavutil/hwcontext_amf.c          | 585 +++++++++++++++++++++++++++++
>  libavutil/hwcontext_amf.h          |  64 ++++
>  libavutil/hwcontext_amf_internal.h |  44 +++
>  libavutil/hwcontext_internal.h     |   1 +
>  libavutil/pixdesc.c                |   4 +
>  libavutil/pixfmt.h                 |   5 +
>  9 files changed, 712 insertions(+)
>  create mode 100644 libavutil/hwcontext_amf.c
>  create mode 100644 libavutil/hwcontext_amf.h
>  create mode 100644 libavutil/hwcontext_amf_internal.h
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 6e6fa8d800..13c318560d 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -45,6 +45,7 @@ HEADERS = adler32.h                                                     \
>            hwcontext_d3d12va.h                                           \
>            hwcontext_drm.h                                               \
>            hwcontext_dxva2.h                                             \
> +          hwcontext_amf.h                                               \
>            hwcontext_qsv.h                                               \
>            hwcontext_mediacodec.h                                        \
>            hwcontext_opencl.h                                            \
> @@ -196,6 +197,7 @@ OBJS-$(CONFIG_CUDA)                     += hwcontext_cuda.o
>  OBJS-$(CONFIG_D3D11VA)                  += hwcontext_d3d11va.o
>  OBJS-$(CONFIG_D3D12VA)                  += hwcontext_d3d12va.o
>  OBJS-$(CONFIG_DXVA2)                    += hwcontext_dxva2.o
> +OBJS-$(CONFIG_AMF)                      += hwcontext_amf.o
>  OBJS-$(CONFIG_LIBDRM)                   += hwcontext_drm.o
>  OBJS-$(CONFIG_MACOS_KPERF)              += macos_kperf.o
>  OBJS-$(CONFIG_MEDIACODEC)               += hwcontext_mediacodec.o
> @@ -220,6 +222,8 @@ SKIPHEADERS-$(CONFIG_CUDA)             += hwcontext_cuda_internal.h     \
>  SKIPHEADERS-$(CONFIG_D3D11VA)          += hwcontext_d3d11va.h
>  SKIPHEADERS-$(CONFIG_D3D12VA)          += hwcontext_d3d12va.h
>  SKIPHEADERS-$(CONFIG_DXVA2)            += hwcontext_dxva2.h
> +SKIPHEADERS-$(CONFIG_AMF)              += hwcontext_amf.h               \
> +                                          hwcontext_amf_internal
>  SKIPHEADERS-$(CONFIG_QSV)              += hwcontext_qsv.h
>  SKIPHEADERS-$(CONFIG_OPENCL)           += hwcontext_opencl.h
>  SKIPHEADERS-$(CONFIG_VAAPI)            += hwcontext_vaapi.h
> diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c
> index fa99a0d8a4..f06d49c45c 100644
> --- a/libavutil/hwcontext.c
> +++ b/libavutil/hwcontext.c
> @@ -65,6 +65,9 @@ static const HWContextType * const hw_table[] = {
>  #endif
>  #if CONFIG_VULKAN
>      &ff_hwcontext_type_vulkan,
> +#endif
> +#if CONFIG_AMF
> +    &ff_hwcontext_type_amf,
>  #endif
>      NULL,
>  };
> @@ -82,6 +85,7 @@ static const char *const hw_type_names[] = {
>      [AV_HWDEVICE_TYPE_VIDEOTOOLBOX] = "videotoolbox",
>      [AV_HWDEVICE_TYPE_MEDIACODEC] = "mediacodec",
>      [AV_HWDEVICE_TYPE_VULKAN] = "vulkan",
> +    [AV_HWDEVICE_TYPE_AMF] = "amf",
>  };
>  
>  typedef struct FFHWDeviceContext {
> diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
> index bac30debae..96042ba197 100644
> --- a/libavutil/hwcontext.h
> +++ b/libavutil/hwcontext.h
> @@ -38,6 +38,7 @@ enum AVHWDeviceType {
>      AV_HWDEVICE_TYPE_MEDIACODEC,
>      AV_HWDEVICE_TYPE_VULKAN,
>      AV_HWDEVICE_TYPE_D3D12VA,
> +    AV_HWDEVICE_TYPE_AMF,
>  };
>  
>  /**
> diff --git a/libavutil/hwcontext_amf.c b/libavutil/hwcontext_amf.c
> new file mode 100644
> index 0000000000..1c589669e1
> --- /dev/null
> +++ b/libavutil/hwcontext_amf.c
> @@ -0,0 +1,585 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include "buffer.h"
> +#include "common.h"
> +#include "hwcontext.h"
> +#include "hwcontext_amf.h"
> +#include "hwcontext_internal.h"
> +#include "hwcontext_amf_internal.h"
> +#if CONFIG_VULKAN
> +#include "hwcontext_vulkan.h"
> +#endif
> +#if CONFIG_D3D11VA
> +#include "libavutil/hwcontext_d3d11va.h"
> +#endif
> +#if CONFIG_DXVA2
> +#define COBJMACROS
> +#include "libavutil/hwcontext_dxva2.h"
> +#endif
> +#include "mem.h"
> +#include "pixdesc.h"
> +#include "pixfmt.h"
> +#include "imgutils.h"
> +#include "libavutil/avassert.h"
> +#include <AMF/core/Surface.h>
> +#include <AMF/core/Trace.h>
> +#ifdef _WIN32
> +#include "compat/w32dlfcn.h"
> +#else
> +#include <dlfcn.h>
> +#endif
> +#define FFMPEG_AMF_WRITER_ID L"ffmpeg_amf"
> +int av_amf_load_library(AVAMFDeviceContext* amf_ctx,  void* avcl);
> +
> +typedef struct AMFFramesContext {
> +    AMFSurface * surfaces;
> +    int            nb_surfaces;
> +} AMFFramesContext;
> +
> +typedef struct AmfTraceWriter {
> +    AMFTraceWriterVtbl  *vtbl;
> +    void                *avctx;
> +} AmfTraceWriter;
> +
> +static void AMF_CDECL_CALL AMFTraceWriter_Write(AMFTraceWriter *pThis,
> +    const wchar_t *scope, const wchar_t *message)
> +{
> +    AmfTraceWriter *tracer = (AmfTraceWriter*)pThis;
> +    av_log(tracer->avctx, AV_LOG_DEBUG, "%ls: %ls", scope, message); // \n is provided from AMF
> +}
> +
> +static void AMF_CDECL_CALL AMFTraceWriter_Flush(AMFTraceWriter *pThis)
> +{
> +}
> +
> +AmfTraceWriter * amf_writer_alloc(void  *avctx)

This function should be declared as static (in fact, how does this even
compile given that we add -Werror=missing-prototypes for both GCC and
clang?). The same applies to several other functions here.

> +{
> +    AmfTraceWriter * writer = av_mallocz(sizeof(AmfTraceWriter));
> +    if (!writer)
> +        return NULL;
> +    writer->vtbl = av_mallocz(sizeof(AmfTraceWriter));

1. Wrong sizeof.
2. I don't see a reason why there should be separate allocations at all: Use
struct AmfTraceWriter {
    AMFTraceWriterVtbl *vtblp;
    void               *avctx;
    AMFTraceWriterVtbl  vtbl;
} AmfTraceWriter;
and make vtblp point to vtbl.

> +    if (writer->vtbl) {
> +        av_freep(&writer);
> +        return NULL;
> +    }
> +    writer->vtbl->Write = AMFTraceWriter_Write;
> +    writer->vtbl->Flush = AMFTraceWriter_Flush;
> +    writer->avctx = avctx;
> +    return writer;
> +}
> +
> +void amf_writer_free(void  *opaque)
> +{
> +    AmfTraceWriter *writer = (AmfTraceWriter *)opaque;
> +    av_freep(&writer->vtbl);
> +    av_freep(&writer);
> +}
> +
> +
> +typedef struct AVAMFFormatMap {
> +    enum AVPixelFormat       av_format;
> +    enum AMF_SURFACE_FORMAT  amf_format;
> +} FormatMap;
> +
> +const FormatMap format_map[] =
> +{
> +    { AV_PIX_FMT_NONE,          AMF_SURFACE_UNKNOWN },
> +    { AV_PIX_FMT_NV12,          AMF_SURFACE_NV12 },
> +    { AV_PIX_FMT_BGR0,          AMF_SURFACE_BGRA },
> +    { AV_PIX_FMT_RGB0,          AMF_SURFACE_RGBA },
> +    { AV_PIX_FMT_BGRA,          AMF_SURFACE_BGRA },
> +    { AV_PIX_FMT_ARGB,          AMF_SURFACE_ARGB },
> +    { AV_PIX_FMT_RGBA,          AMF_SURFACE_RGBA },
> +    { AV_PIX_FMT_GRAY8,         AMF_SURFACE_GRAY8 },
> +    { AV_PIX_FMT_YUV420P,       AMF_SURFACE_YUV420P },
> +    { AV_PIX_FMT_YUYV422,       AMF_SURFACE_YUY2 },
> +    { AV_PIX_FMT_P010,          AMF_SURFACE_P010 },
> +};
> +
> +enum AMF_SURFACE_FORMAT av_amf_av_to_amf_format(enum AVPixelFormat fmt)
> +{
> +    int i;
> +    for (i = 0; i < amf_countof(format_map); i++) {
> +        if (format_map[i].av_format == fmt) {
> +            return format_map[i].amf_format;
> +        }
> +    }
> +    return AMF_SURFACE_UNKNOWN;
> +}
> +
> +enum AVPixelFormat av_amf_to_av_format(enum AMF_SURFACE_FORMAT fmt)
> +{
> +    int i;
> +    for (i = 0; i < amf_countof(format_map); i++) {
> +        if (format_map[i].amf_format == fmt) {
> +            return format_map[i].av_format;
> +        }
> +    }
> +    return AMF_SURFACE_UNKNOWN;
> +}
> +
> +static const enum AVPixelFormat supported_formats[] = {
> +    AV_PIX_FMT_NV12,
> +    AV_PIX_FMT_YUV420P,
> +    AV_PIX_FMT_BGRA,
> +    AV_PIX_FMT_P010,
> +    AV_PIX_FMT_YUV420P10,
> +#if CONFIG_D3D11VA
> +    AV_PIX_FMT_D3D11,
> +#endif
> +#if CONFIG_DXVA2
> +    AV_PIX_FMT_DXVA2_VLD,
> +#endif
> +    AV_PIX_FMT_AMF_SURFACE
> +};
> +
> +static int amf_frames_get_constraints(AVHWDeviceContext *ctx,
> +                                       const void *hwconfig,
> +                                       AVHWFramesConstraints *constraints)
> +{
> +    int i;
> +
> +    constraints->valid_sw_formats = av_malloc_array(FF_ARRAY_ELEMS(supported_formats) + 1,
> +                                                    sizeof(*constraints->valid_sw_formats));
> +    if (!constraints->valid_sw_formats)
> +        return AVERROR(ENOMEM);
> +
> +    for (i = 0; i < FF_ARRAY_ELEMS(supported_formats); i++)
> +        constraints->valid_sw_formats[i] = supported_formats[i];
> +    constraints->valid_sw_formats[FF_ARRAY_ELEMS(supported_formats)] = AV_PIX_FMT_NONE;
> +
> +    constraints->valid_hw_formats = av_malloc_array(2, sizeof(*constraints->valid_hw_formats));
> +    if (!constraints->valid_hw_formats)
> +        return AVERROR(ENOMEM);
> +
> +    constraints->valid_hw_formats[0] = AV_PIX_FMT_AMF_SURFACE;
> +    constraints->valid_hw_formats[1] = AV_PIX_FMT_NONE;
> +
> +    return 0;
> +}
> +
> +static void amf_dummy_free(void *opaque, uint8_t *data)
> +{
> +
> +}
> +
> +static AVBufferRef *amf_pool_alloc(void *opaque, size_t size)
> +{
> +    AVHWFramesContext *hwfc = (AVHWFramesContext *)opaque;
> +    AVBufferRef *buf;
> +
> +    buf = av_buffer_create(NULL, NULL, amf_dummy_free, hwfc, AV_BUFFER_FLAG_READONLY);
> +    if (!buf) {
> +        av_log(hwfc, AV_LOG_ERROR, "Failed to create buffer for AMF context.\n");
> +        return NULL;
> +    }
> +    return buf;
> +}
> +
> +static int amf_frames_init(AVHWFramesContext *ctx)
> +{
> +    int i;
> +
> +    for (i = 0; i < FF_ARRAY_ELEMS(supported_formats); i++) {
> +        if (ctx->sw_format == supported_formats[i])
> +            break;
> +    }
> +    if (i == FF_ARRAY_ELEMS(supported_formats)) {
> +        av_log(ctx, AV_LOG_ERROR, "Pixel format '%s' is not supported\n",
> +               av_get_pix_fmt_name(ctx->sw_format));
> +        return AVERROR(ENOSYS);
> +    }
> +
> +    ffhwframesctx(ctx)->pool_internal =
> +            av_buffer_pool_init2(sizeof(AMFSurface), ctx,
> +                                 &amf_pool_alloc, NULL);
> +
> +    return 0;
> +}
> +
> +int amf_context_create(  AVAMFDeviceContext * amf_ctx,
> +                                void* avcl,
> +                                const char *device,
> +                                AVDictionary *opts, int flags)
> +{
> +    AMF_RESULT         res;
> +
> +    amf_ctx->trace->pVtbl->EnableWriter(amf_ctx->trace, AMF_TRACE_WRITER_CONSOLE, 0);
> +    amf_ctx->trace->pVtbl->SetGlobalLevel(amf_ctx->trace, AMF_TRACE_TRACE);
> +
> +     // connect AMF logger to av_log
> +    amf_ctx->trace_writer = amf_writer_alloc(avcl);
> +    amf_ctx->trace->pVtbl->RegisterWriter(amf_ctx->trace, FFMPEG_AMF_WRITER_ID, (AMFTraceWriter*)amf_ctx->trace_writer, 1);
> +    amf_ctx->trace->pVtbl->SetWriterLevel(amf_ctx->trace, FFMPEG_AMF_WRITER_ID, AMF_TRACE_TRACE);
> +
> +    res = amf_ctx->factory->pVtbl->CreateContext(amf_ctx->factory, &amf_ctx->context);
> +    AMF_RETURN_IF_FALSE(avcl, res == AMF_OK, AVERROR_UNKNOWN, "CreateContext() failed with error %d\n", res);
> +
> +    return 0;
> +}
> +
> +static int amf_get_buffer(AVHWFramesContext *ctx, AVFrame *frame)
> +{
> +    frame->buf[0] = av_buffer_pool_get(ctx->pool);
> +    if (!frame->buf[0])
> +        return AVERROR(ENOMEM);
> +
> +    frame->data[0] = frame->buf[0]->data;
> +    frame->format  = AV_PIX_FMT_AMF_SURFACE;
> +    frame->width   = ctx->width;
> +    frame->height  = ctx->height;
> +    return 0;
> +}
> +
> +static int amf_transfer_get_formats(AVHWFramesContext *ctx,
> +                                     enum AVHWFrameTransferDirection dir,
> +                                     enum AVPixelFormat **formats)
> +{
> +    enum AVPixelFormat *fmts;
> +
> +    fmts = av_malloc_array(2, sizeof(*fmts));
> +    if (!fmts)
> +        return AVERROR(ENOMEM);
> +
> +    fmts[0] = ctx->sw_format;
> +    fmts[1] = AV_PIX_FMT_NONE;
> +
> +    *formats = fmts;
> +
> +    return 0;
> +}
> +
> +int av_amf_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> +                                 const AVFrame *src)
> +{
> +    AMFSurface* surface = (AMFSurface*)dst->data[0];
> +    AMFPlane *plane;
> +    uint8_t  *dst_data[4];
> +    int       dst_linesize[4];
> +    int       planes;
> +    int       i;
> +    int w = FFMIN(dst->width,  src->width);
> +    int h = FFMIN(dst->height, src->height);
> +
> +    planes = (int)surface->pVtbl->GetPlanesCount(surface);
> +    av_assert0(planes < FF_ARRAY_ELEMS(dst_data));
> +
> +    for (i = 0; i < planes; i++) {
> +        plane = surface->pVtbl->GetPlaneAt(surface, i);
> +        dst_data[i] = plane->pVtbl->GetNative(plane);
> +        dst_linesize[i] = plane->pVtbl->GetHPitch(plane);
> +    }
> +    av_image_copy(dst_data, dst_linesize,
> +        (const uint8_t**)src->data, src->linesize, src->format,
> +        w, h);

av_image_copy2

> +
> +    return 0;
> +}
> +int av_amf_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> +                                    const AVFrame *src)
> +{
> +    AMFSurface* surface = (AMFSurface*)src->data[0];
> +    AMFPlane *plane;
> +    uint8_t  *src_data[4];
> +    int       src_linesize[4];
> +    int       planes;
> +    int       i;
> +    int w = FFMIN(dst->width,  src->width);
> +    int h = FFMIN(dst->height, src->height);
> +    int ret;
> +
> +    ret = surface->pVtbl->Convert(surface, AMF_MEMORY_HOST);
> +    AMF_RETURN_IF_FALSE(ctx, ret == AMF_OK, AVERROR_UNKNOWN, "Convert(amf::AMF_MEMORY_HOST) failed with error %d\n", AVERROR_UNKNOWN);
> +
> +    planes = (int)surface->pVtbl->GetPlanesCount(surface);
> +    av_assert0(planes < FF_ARRAY_ELEMS(src_data));
> +
> +    for (i = 0; i < planes; i++) {
> +        plane = surface->pVtbl->GetPlaneAt(surface, i);
> +        src_data[i] = plane->pVtbl->GetNative(plane);
> +        src_linesize[i] = plane->pVtbl->GetHPitch(plane);
> +    }
> +    av_image_copy(dst->data, dst->linesize,
> +                  (const uint8_t **)src_data, src_linesize, dst->format,
> +                  w, h);
> +    surface->pVtbl->Release(surface);
> +    return 0;
> +}
> +
> +
> +static void amf_device_uninit(AVHWDeviceContext *device_ctx)
> +{
> +    AVAMFDeviceContext      *amf_ctx = device_ctx->hwctx;
> +    av_amf_context_free(0, (uint8_t *)amf_ctx);
> +}
> +
> +static int amf_device_init(AVHWDeviceContext *ctx)
> +{
> +    AVAMFDeviceContext *amf_ctx = ctx->hwctx;
> +    return av_amf_context_init(amf_ctx, ctx);
> +}
> +
> +static int amf_device_create(AVHWDeviceContext *device_ctx,
> +                              const char *device,
> +                              AVDictionary *opts, int flags)
> +{
> +    AVAMFDeviceContext        *ctx = device_ctx->hwctx;
> +    int ret;
> +    if ((ret = av_amf_load_library(ctx, device_ctx)) == 0) {
> +        if ((ret = amf_context_create(ctx, device_ctx, "", opts, flags)) == 0){
> +            return 0;
> +        }
> +    }
> +    amf_device_uninit(device_ctx);
> +    return ret;
> +}
> +
> +static int amf_device_derive(AVHWDeviceContext *device_ctx,
> +                              AVHWDeviceContext *child_device_ctx, AVDictionary *opts,
> +                              int flags)
> +{
> +    AVAMFDeviceContext        *ctx = device_ctx->hwctx;
> +    int ret;
> +
> +    ret = amf_device_create(device_ctx, "", opts, flags);
> +    if(ret < 0)
> +        return ret;
> +
> +    return av_amf_context_derive(ctx, child_device_ctx, opts, flags);
> +}
> +
> +#if CONFIG_DXVA2
> +static int amf_init_from_dxva2_device(AVAMFDeviceContext * amf_ctx, AVDXVA2DeviceContext *hwctx)
> +{
> +    IDirect3DDevice9    *device;
> +    HANDLE              device_handle;
> +    HRESULT             hr;
> +    AMF_RESULT          res;
> +    int ret;
> +
> +    hr = IDirect3DDeviceManager9_OpenDeviceHandle(hwctx->devmgr, &device_handle);
> +    if (FAILED(hr)) {
> +        av_log(hwctx, AV_LOG_ERROR, "Failed to open device handle for Direct3D9 device: %lx.\n", (unsigned long)hr);
> +        return AVERROR_EXTERNAL;
> +    }
> +
> +    hr = IDirect3DDeviceManager9_LockDevice(hwctx->devmgr, device_handle, &device, FALSE);
> +    if (SUCCEEDED(hr)) {
> +        IDirect3DDeviceManager9_UnlockDevice(hwctx->devmgr, device_handle, FALSE);
> +        ret = 0;
> +    } else {
> +        av_log(hwctx, AV_LOG_ERROR, "Failed to lock device handle for Direct3D9 device: %lx.\n", (unsigned long)hr);
> +        ret = AVERROR_EXTERNAL;
> +    }
> +
> +
> +    IDirect3DDeviceManager9_CloseDeviceHandle(hwctx->devmgr, device_handle);
> +
> +    if (ret < 0)
> +        return ret;
> +
> +    res = amf_ctx->context->pVtbl->InitDX9(amf_ctx->context, device);
> +
> +    IDirect3DDevice9_Release(device);
> +
> +    if (res != AMF_OK) {
> +        if (res == AMF_NOT_SUPPORTED)
> +            av_log(hwctx, AV_LOG_ERROR, "AMF via D3D9 is not supported on the given device.\n");
> +        else
> +            av_log(hwctx, AV_LOG_ERROR, "AMF failed to initialise on given D3D9 device: %d.\n", res);
> +        return AVERROR(ENODEV);
> +    }
> +    amf_ctx->mem_type = AMF_MEMORY_DX9;
> +    return 0;
> +}
> +#endif
> +
> +#if CONFIG_D3D11VA
> +static int amf_init_from_d3d11_device(AVAMFDeviceContext* amf_ctx, AVD3D11VADeviceContext *hwctx)
> +{
> +    AMF_RESULT res;
> +    res = amf_ctx->context->pVtbl->InitDX11(amf_ctx->context, hwctx->device, AMF_DX11_1);
> +    if (res != AMF_OK) {
> +        if (res == AMF_NOT_SUPPORTED)
> +            av_log(hwctx, AV_LOG_ERROR, "AMF via D3D11 is not supported on the given device.\n");
> +        else
> +            av_log(hwctx, AV_LOG_ERROR, "AMF failed to initialise on the given D3D11 device: %d.\n", res);
> +        return AVERROR(ENODEV);
> +    }
> +    amf_ctx->mem_type = AMF_MEMORY_DX11;
> +    return 0;
> +}
> +#endif
> +
> +int av_amf_context_init(AVAMFDeviceContext* amf_ctx, void* avcl)
> +{
> +     AMFContext1 *context1 = NULL;
> +     AMF_RESULT res;
> +
> +    res = amf_ctx->context->pVtbl->InitDX11(amf_ctx->context, NULL, AMF_DX11_1);
> +    if (res == AMF_OK) {
> +        amf_ctx->mem_type = AMF_MEMORY_DX11;
> +        av_log(avcl, AV_LOG_VERBOSE, "AMF initialisation succeeded via D3D11.\n");
> +    } else {
> +        res = amf_ctx->context->pVtbl->InitDX9(amf_ctx->context, NULL);
> +        if (res == AMF_OK) {
> +            amf_ctx->mem_type = AMF_MEMORY_DX9;
> +            av_log(avcl, AV_LOG_VERBOSE, "AMF initialisation succeeded via D3D9.\n");
> +        } else {
> +            AMFGuid guid = IID_AMFContext1();
> +            res = amf_ctx->context->pVtbl->QueryInterface(amf_ctx->context, &guid, (void**)&context1);
> +            AMF_RETURN_IF_FALSE(avcl, res == AMF_OK, AVERROR_UNKNOWN, "CreateContext1() failed with error %d\n", res);
> +
> +            res = context1->pVtbl->InitVulkan(context1, NULL);
> +            context1->pVtbl->Release(context1);
> +            if (res != AMF_OK) {
> +                if (res == AMF_NOT_SUPPORTED)
> +                    av_log(avcl, AV_LOG_ERROR, "AMF via Vulkan is not supported on the given device.\n");
> +                 else
> +                    av_log(avcl, AV_LOG_ERROR, "AMF failed to initialise on the given Vulkan device: %d.\n", res);
> +                 return AVERROR(ENOSYS);
> +            }
> +            amf_ctx->mem_type = AMF_MEMORY_VULKAN;
> +            av_log(avcl, AV_LOG_VERBOSE, "AMF initialisation succeeded via Vulkan.\n");
> +         }
> +     }
> +     return 0;
> +}
> +int av_amf_load_library(AVAMFDeviceContext* amf_ctx,  void* avcl)

This function will be exported due to its name, although it is not
declared in a public header. Rename it. In fact, make it static.

> +{
> +    AMFInit_Fn         init_fun;
> +    AMFQueryVersion_Fn version_fun;
> +    AMF_RESULT         res;
> +
> +    amf_ctx->library = dlopen(AMF_DLL_NAMEA, RTLD_NOW | RTLD_LOCAL);
> +    AMF_RETURN_IF_FALSE(avcl, amf_ctx->library != NULL,
> +        AVERROR_UNKNOWN, "DLL %s failed to open\n", AMF_DLL_NAMEA);
> +
> +    init_fun = (AMFInit_Fn)dlsym(amf_ctx->library, AMF_INIT_FUNCTION_NAME);
> +    AMF_RETURN_IF_FALSE(avcl, init_fun != NULL, AVERROR_UNKNOWN, "DLL %s failed to find function %s\n", AMF_DLL_NAMEA, AMF_INIT_FUNCTION_NAME);
> +
> +    version_fun = (AMFQueryVersion_Fn)dlsym(amf_ctx->library, AMF_QUERY_VERSION_FUNCTION_NAME);
> +    AMF_RETURN_IF_FALSE(avcl, version_fun != NULL, AVERROR_UNKNOWN, "DLL %s failed to find function %s\n", AMF_DLL_NAMEA, AMF_QUERY_VERSION_FUNCTION_NAME);
> +
> +    res = version_fun(&amf_ctx->version);
> +    AMF_RETURN_IF_FALSE(avcl, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with error %d\n", AMF_QUERY_VERSION_FUNCTION_NAME, res);
> +    res = init_fun(AMF_FULL_VERSION, &amf_ctx->factory);
> +    AMF_RETURN_IF_FALSE(avcl, res == AMF_OK, AVERROR_UNKNOWN, "%s failed with error %d\n", AMF_INIT_FUNCTION_NAME, res);
> +    res = amf_ctx->factory->pVtbl->GetTrace(amf_ctx->factory, &amf_ctx->trace);
> +    AMF_RETURN_IF_FALSE(avcl, res == AMF_OK, AVERROR_UNKNOWN, "GetTrace() failed with error %d\n", res);
> +    res = amf_ctx->factory->pVtbl->GetDebug(amf_ctx->factory, &amf_ctx->debug);
> +    AMF_RETURN_IF_FALSE(avcl, res == AMF_OK, AVERROR_UNKNOWN, "GetDebug() failed with error %d\n", res);
> +    return 0;
> +}
> +
> +int av_amf_context_derive(AVAMFDeviceContext * amf_ctx,
> +                               AVHWDeviceContext *child_device_ctx, AVDictionary *opts,
> +                               int flags)
> +{
> +
> +    switch (child_device_ctx->type) {
> +
> +#if CONFIG_DXVA2
> +    case AV_HWDEVICE_TYPE_DXVA2:
> +        {
> +            AVDXVA2DeviceContext *child_device_hwctx = child_device_ctx->hwctx;
> +            return amf_init_from_dxva2_device(amf_ctx, child_device_hwctx);
> +        }
> +        break;
> +#endif
> +
> +#if CONFIG_D3D11VA
> +    case AV_HWDEVICE_TYPE_D3D11VA:
> +        {
> +            AVD3D11VADeviceContext *child_device_hwctx = child_device_ctx->hwctx;
> +            return amf_init_from_d3d11_device(amf_ctx, child_device_hwctx);
> +        }
> +        break;
> +#endif
> +    default:
> +        {
> +            av_log(child_device_ctx, AV_LOG_ERROR, "AMF initialisation from a %s device is not supported.\n",
> +                av_hwdevice_get_type_name(child_device_ctx->type));
> +            return AVERROR(ENOSYS);
> +        }
> +    }
> +    return 0;
> +}
> +
> +int av_amf_context_create(AVAMFDeviceContext * context,
> +                            void* avcl,
> +                            const char *device,
> +                            AVDictionary *opts, int flags)
> +{
> +    int ret;
> +    if ((ret = av_amf_load_library(context, avcl)) == 0) {
> +        if ((ret = amf_context_create(context, avcl, "", opts, flags)) == 0){
> +            return 0;
> +        }
> +    }
> +    av_amf_context_free(0, (uint8_t *)context);
> +    return ret;
> +}
> +
> +void av_amf_context_free(void *opaque, uint8_t *data)
> +{
> +    AVAMFDeviceContext *amf_ctx = (AVAMFDeviceContext *)data;
> +    if (amf_ctx->context) {
> +        amf_ctx->context->pVtbl->Terminate(amf_ctx->context);
> +        amf_ctx->context->pVtbl->Release(amf_ctx->context);
> +        amf_ctx->context = NULL;
> +    }
> +
> +    if (amf_ctx->trace) {
> +        amf_ctx->trace->pVtbl->UnregisterWriter(amf_ctx->trace, FFMPEG_AMF_WRITER_ID);
> +    }
> +
> +    if(amf_ctx->library) {
> +        dlclose(amf_ctx->library);
> +        amf_ctx->library = NULL;
> +    }
> +    if (amf_ctx->trace_writer) {
> +        amf_writer_free(amf_ctx->trace_writer);
> +    }
> +
> +    amf_ctx->debug = NULL;
> +    amf_ctx->version = 0;
> +}
> +
> +
> +const HWContextType ff_hwcontext_type_amf = {
> +    .type                 = AV_HWDEVICE_TYPE_AMF,
> +    .name                 = "AMF",
> +
> +    .device_hwctx_size    = sizeof(AVAMFDeviceContext),
> +    .frames_hwctx_size    = sizeof(AMFFramesContext),
> +
> +    .device_create        = amf_device_create,
> +    .device_derive        = amf_device_derive,
> +    .device_init          = amf_device_init,
> +    .device_uninit        = amf_device_uninit,
> +    .frames_get_constraints = amf_frames_get_constraints,
> +    .frames_init          = amf_frames_init,
> +    .frames_get_buffer    = amf_get_buffer,
> +    .transfer_get_formats = amf_transfer_get_formats,
> +    .transfer_data_to     = av_amf_transfer_data_to,
> +    .transfer_data_from   = av_amf_transfer_data_from,
> +
> +    .pix_fmts             = (const enum AVPixelFormat[]){ AV_PIX_FMT_AMF_SURFACE, AV_PIX_FMT_NONE },
> +};
> diff --git a/libavutil/hwcontext_amf_internal.h b/libavutil/hwcontext_amf_internal.h
> new file mode 100644
> index 0000000000..b991f357a6
> --- /dev/null
> +++ b/libavutil/hwcontext_amf_internal.h
> @@ -0,0 +1,44 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +
> +#ifndef AVUTIL_HWCONTEXT_AMF_INTERNAL_H
> +#define AVUTIL_HWCONTEXT_AMF_INTERNAL_H
> +#include <AMF/core/Factory.h>
> +#include <AMF/core/Context.h>
> +
> +/**
> +* Error handling helper
> +*/
> +#define AMF_RETURN_IF_FALSE(avctx, exp, ret_value, /*message,*/ ...) \
> +    if (!(exp)) { \
> +        av_log(avctx, AV_LOG_ERROR, __VA_ARGS__); \
> +        return ret_value; \
> +    }
> +
> +#define AMF_GOTO_FAIL_IF_FALSE(avctx, exp, ret_value, /*message,*/ ...) \
> +    if (!(exp)) { \
> +        av_log(avctx, AV_LOG_ERROR, __VA_ARGS__); \
> +        ret = ret_value; \
> +        goto fail; \
> +    }
> +
> +#define AMF_TIME_BASE_Q          (AVRational){1, AMF_SECOND}
> +
> +
> +#endif /* AVUTIL_HWCONTEXT_AMF_INTERNAL_H */
> \ No newline at end of file

IIRC this is against the C spec.

On 30/05/2024 15:08, Dmitrii Ovchinnikov wrote:
> Adds hwcontext_amf, which allows to use shared AMF
> context for the encoder, decoder and AMF-based filters,
> without copy to the host memory.
> It will also allow you to use some optimisations in
> the interaction of components (for example, SAV) and make a more
> manageable and optimal setup for using GPU devices with AMF
> in the case of a fully AMF pipeline.
> It will be a significant performance uplift when full AMF pipeline
> with filters is used.
> 
> We also plan to add Compression artefact removal filter in near feature.
> v2: cleanup header files
> v3: an unnecessary class has been removed.
> ---
>   libavutil/Makefile                 |   4 +
>   libavutil/hwcontext.c              |   4 +
>   libavutil/hwcontext.h              |   1 +
>   libavutil/hwcontext_amf.c          | 585 +++++++++++++++++++++++++++++
>   libavutil/hwcontext_amf.h          |  64 ++++
>   libavutil/hwcontext_amf_internal.h |  44 +++
>   libavutil/hwcontext_internal.h     |   1 +
>   libavutil/pixdesc.c                |   4 +
>   libavutil/pixfmt.h                 |   5 +
>   9 files changed, 712 insertions(+)
>   create mode 100644 libavutil/hwcontext_amf.c
>   create mode 100644 libavutil/hwcontext_amf.h
>   create mode 100644 libavutil/hwcontext_amf_internal.h

Still no answer to my question?

DX12 and Vulkan native encoders will expose less features compare to AMF,

at least in foreseeable feature. The missing features include low latency,

PreAnalysis including look-ahead etc. AMF context on Windows allows fully

enable SAV - ability to utilize VCNs in dGPU and APU in a single session.

AMF components including encoder and decoder has some internal optimizations

in the area of memory access for APUs that are not available in standard

3D APIs.

Eventually specialized multimedia AMD cards could be added seamlessly to

FFmpeg with AMF integration.

AMF FSR(VSR) includes YUV version with focus on videos which is not

available in AMD FSR aimed for gaming.

More advanced filters that are not available in standard 3D APIs are coming.

On 30/05/2024 18:06, Dmitrii Ovchinnikov wrote:
> DX12 and Vulkan native encoders will expose less features compare to 
> AMF,____
> 
> at least in foreseeable feature. The missing features include low 
> latency,____

That's plainly not true.

> PreAnalysis including look-ahead etc. AMF context on Windows allows 
> fully____
>
> enable SAV - ability to utilize VCNs in dGPU and APU in a single 
> session.____

You should try talking internally to learn what is in progress.

> AMF components including encoder and decoder has some internal 
> optimizations ____
> 
> in the area of memory access for APUs that are not available in standard 
> ____
> 
> 3D APIs.____

This isn't OpenGL.

> Eventually specialized multimedia AMD cards could be added seamlessly to____
> 
> FFmpeg with AMF integration.____
> 
> AMF FSR(VSR) includes YUV version with focus on videos which is not____
> 
> available in AMD FSR aimed for gaming.____

Why don't you open source it then?

> More advanced filters that are not available in standard 3D APIs are 
> coming. __
We could have them as Vulkan filters.


I'm not objecting on this patch, but I am concerned that it's more 
proprietary code which is soon going to be redundant.

I will have to review it properly at some point.

I would appreciate your review.
Just to clarify: The information I provided is coming from AMF and driver
developers.

On 30/05/2024 14:08, Dmitrii Ovchinnikov wrote:
> Adds hwcontext_amf, which allows to use shared AMF
> context for the encoder, decoder and AMF-based filters,
> without copy to the host memory.
> It will also allow you to use some optimisations in
> the interaction of components (for example, SAV) and make a more
> manageable and optimal setup for using GPU devices with AMF
> in the case of a fully AMF pipeline.
> It will be a significant performance uplift when full AMF pipeline
> with filters is used.
> 
> We also plan to add Compression artefact removal filter in near feature.
> v2: cleanup header files
> v3: an unnecessary class has been removed.
> ---
>  libavutil/Makefile                 |   4 +
>  libavutil/hwcontext.c              |   4 +
>  libavutil/hwcontext.h              |   1 +
>  libavutil/hwcontext_amf.c          | 585 +++++++++++++++++++++++++++++
>  libavutil/hwcontext_amf.h          |  64 ++++
>  libavutil/hwcontext_amf_internal.h |  44 +++
>  libavutil/hwcontext_internal.h     |   1 +
>  libavutil/pixdesc.c                |   4 +
>  libavutil/pixfmt.h                 |   5 +
>  9 files changed, 712 insertions(+)
>  create mode 100644 libavutil/hwcontext_amf.c
>  create mode 100644 libavutil/hwcontext_amf.h
>  create mode 100644 libavutil/hwcontext_amf_internal.h
> 
> ...
> +
> +static void amf_dummy_free(void *opaque, uint8_t *data)
> +{
> +
> +}
> +
> +static AVBufferRef *amf_pool_alloc(void *opaque, size_t size)
> +{
> +    AVHWFramesContext *hwfc = (AVHWFramesContext *)opaque;
> +    AVBufferRef *buf;
> +
> +    buf = av_buffer_create(NULL, NULL, amf_dummy_free, hwfc, AV_BUFFER_FLAG_READONLY);
> +    if (!buf) {
> +        av_log(hwfc, AV_LOG_ERROR, "Failed to create buffer for AMF context.\n");
> +        return NULL;
> +    }
> +    return buf;
> +}

You're still allocating nothing here?

I think what this means is that you don't actually want to implement frames context creation at all because it doesn't do anything.

If it is not possible to make an AMFSurface as anything other than an output from an AMF component then this would make sense, the decoder can allocate the internals.

Look at the DRM hwcontext for an example that works like this - the DRM objects can only be made as outputs from devices or by mapping, so there is no frame context implementation.

> +
> ...
> diff --git a/libavutil/hwcontext_amf.h b/libavutil/hwcontext_amf.h
> new file mode 100644
> index 0000000000..ef2118dd4e
> --- /dev/null
> +++ b/libavutil/hwcontext_amf.h
> @@ -0,0 +1,64 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +
> +#ifndef AVUTIL_HWCONTEXT_AMF_H
> +#define AVUTIL_HWCONTEXT_AMF_H
> +
> +#include "pixfmt.h"
> +#include "hwcontext.h"
> +#include <AMF/core/Factory.h>
> +#include <AMF/core/Context.h>
> +#include <AMF/core/Trace.h>
> +#include <AMF/core/Debug.h>
> +
> +/**
> + * This struct is allocated as AVHWDeviceContext.hwctx
> + */
> +typedef struct AVAMFDeviceContext {
> +    HMODULE            library;

What is this type?  (It looks like a Windows type, but I thought this was cross-platform.)

> +    AMFFactory         *factory;
> +    AMFDebug           *debug;
> +    AMFTrace           *trace;
> +    void               *trace_writer;

Are all of these objects necessary to operation of the AMF device?  Please remove elements which are not necessary and add them to the private context if they are otherwise useful.

> +
> +    int64_t            version; ///< version of AMF runtime

Why is the version necessary to expose in the public API?  Is it not possible to call the QueryVersion function after starting?

> +    AMFContext         *context;
> +    int                mem_type;
Is mem_type really necessary to expose in the piblic API?  Can the user not determine this by some API call?

> +} AVAMFDeviceContext;
> +
> +enum AMF_SURFACE_FORMAT av_amf_av_to_amf_format(enum AVPixelFormat fmt);
> +enum AVPixelFormat av_amf_to_av_format(enum AMF_SURFACE_FORMAT fmt);
> +

All of the following functions should not be public symbols.  You want to implement the hwcontext functions so that these all work without needing special implementation for AMF, they should not be individually callable because that is not useful.

> +int av_amf_context_create(AVAMFDeviceContext * context,
> +                          void* avcl,
> +                          const char *device,
> +                          AVDictionary *opts, int flags);

Use device_create.

> +int av_amf_context_init(AVAMFDeviceContext* internal, void* avcl);

Use device_init.

> +void av_amf_context_free(void *opaque, uint8_t *data);

Use device_uninit (or maybe an AVBuffer destructor?).

> +int av_amf_context_derive(AVAMFDeviceContext * internal,
> +                          AVHWDeviceContext *child_device_ctx, AVDictionary *opts,
> +                          int flags);

Use device_derive.

> +
> +int av_amf_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst,
> +                                    const AVFrame *src);

Use transfer_data_from.

> +
> +int av_amf_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> +                                 const AVFrame *src);

Use transfer_data_to.

> +
> +#endif /* AVUTIL_HWCONTEXT_AMF_H */
> ...

Thanks,

- Mark