diff mbox series

[FFmpeg-devel,4/5] lavu/hwcontext_amf: Engine selection support for AMF context

Message ID 20201015001625.32832-4-ovchinnikov.dmitrii@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/5] lavc/amfenc: HWConfig for amf encoder. | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Dmitrii Ovchinnikov Oct. 15, 2020, 12:16 a.m. UTC 
---
 libavcodec/amfenc.c       |   9 +++-
 libavcodec/amfenc.h       |   1 +
 libavcodec/amfenc_h264.c  |   8 +++
 libavcodec/amfenc_hevc.c  |   8 +++
 libavutil/hwcontext_amf.c | 104 ++++++++++++++++++++++++++++++--------
 libavutil/hwcontext_amf.h |   7 +++
 6 files changed, 116 insertions(+), 21 deletions(-)

Comments

Mark Thompson Oct. 28, 2020, 11:52 p.m. UTC | #1 
On 15/10/2020 01:16, OvchinnikovDmitrii wrote:
> ---
>   libavcodec/amfenc.c       |   9 +++-
>   libavcodec/amfenc.h       |   1 +
>   libavcodec/amfenc_h264.c  |   8 +++
>   libavcodec/amfenc_hevc.c  |   8 +++
>   libavutil/hwcontext_amf.c | 104 ++++++++++++++++++++++++++++++--------
>   libavutil/hwcontext_amf.h |   7 +++
>   6 files changed, 116 insertions(+), 21 deletions(-)

Why?  Given that you are already initialising from a specific device when one is given, what does this actually change?

(With patch 1 you can do "ffmpeg ... -init_hw_device d3d11:3 ... -c:v h264_amf ..." and it picks up the device you asked for.)

> diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
> index 767eab3d56..b7eb74811b 100644
> --- a/libavcodec/amfenc.c
> +++ b/libavcodec/amfenc.c
> @@ -82,6 +82,7 @@ static int amf_init_context(AVCodecContext *avctx)
>   {
>       AmfContext *ctx = avctx->priv_data;
>       AVAMFDeviceContext *amf_ctx;
> +    AVDictionary *dict = NULL;
>       int ret;
>   
>       ctx->delayed_frame = av_frame_alloc();
> @@ -123,10 +124,16 @@ static int amf_init_context(AVCodecContext *avctx)
>           if (ret < 0)
>               return ret;
>       } else {
> -        ret = av_hwdevice_ctx_create(&ctx->amf_device_ctx, AV_HWDEVICE_TYPE_AMF, NULL, NULL, 0);
> +#if defined(_WIN32)
> +    if (ctx->engine) {
> +        ret = av_dict_set_int(&dict, "engine", ctx->engine, 0);
>           if (ret < 0)
>               return ret;
>       }
> +#endif

Doesn't seem obviously Windows-dependent?

> +        ret = av_hwdevice_ctx_create(&ctx->amf_device_ctx, AV_HWDEVICE_TYPE_AMF, NULL, dict, 0);if (ret < 0)
> +            return ret;
> +    }
>       amf_ctx = ((AVHWDeviceContext*)ctx->amf_device_ctx->data)->hwctx;
>       ctx->context = amf_ctx->context;
>       ctx->factory = amf_ctx->factory;
> diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
> index 96d1942a37..f4897c9b2a 100644
> --- a/libavcodec/amfenc.h
> +++ b/libavcodec/amfenc.h
> @@ -73,6 +73,7 @@ typedef struct AmfContext {
>       int                 quality;
>       int                 b_frame_delta_qp;
>       int                 ref_b_frame_delta_qp;
> +    int                 engine;
>   
>       // Dynamic options, can be set after Init() call
>   
> diff --git a/libavcodec/amfenc_h264.c b/libavcodec/amfenc_h264.c
> index 622ee85946..9fb42323d8 100644
> --- a/libavcodec/amfenc_h264.c
> +++ b/libavcodec/amfenc_h264.c
> @@ -71,6 +71,14 @@ static const AVOption options[] = {
>       { "balanced",       "Balanced",                             0,                  AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_QUALITY_PRESET_BALANCED },    0, 0, VE, "quality" },
>       { "quality",        "Prefer Quality",                       0,                  AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_QUALITY_PRESET_QUALITY  },     0, 0, VE, "quality" },
>   
> +#if defined(_WIN32)
> +//preffered engine
> +    { "engine",         "Preffered engine",             OFFSET(engine), AV_OPT_TYPE_INT,   { .i64 = AMF_VIDEO_ENCODER_ENGINE_DEFAULT }, AMF_VIDEO_ENCODER_ENGINE_DEFAULT, AMF_VIDEO_ENCODER_ENGINE_VULKAN, VE, "engine" },
> +    { "dxva2",          "DirectX Video Acceleration 2", 0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_DXVA2   }, 0, 0, VE, "engine" },
> +    { "d3d11",          "Direct2D11",                   0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_D3D11   }, 0, 0, VE, "engine" },
> +    { "vulkan",         "Vulkan",                       0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_VULKAN  }, 0, 0, VE, "engine" },
> +#endif

The presence of options shouldn't be platform dependent, even if they don't actually get used in other places.

Also, "preferred" is normally spelled with one 'f' and two 'r's (also in more places below).

> +
>       // Dynamic
>       /// Rate Control Method
>       { "rc",             "Rate Control Method",                  OFFSET(rate_control_mode), AV_OPT_TYPE_INT,   { .i64 = AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_UNKNOWN }, AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_UNKNOWN, AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_LATENCY_CONSTRAINED_VBR, VE, "rc" },
> diff --git a/libavcodec/amfenc_hevc.c b/libavcodec/amfenc_hevc.c
> index bb224c5fec..fc2c40a6ed 100644
> --- a/libavcodec/amfenc_hevc.c
> +++ b/libavcodec/amfenc_hevc.c
> @@ -58,6 +58,14 @@ static const AVOption options[] = {
>       { "speed",          "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_SPEED    }, 0, 0, VE, "quality" },
>       { "quality",        "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_QUALITY  }, 0, 0, VE, "quality" },
>   
> +#if defined(_WIN32)
> +//preffered engine
> +    { "engine",         "Preffered engine",             OFFSET(engine), AV_OPT_TYPE_INT,   { .i64 = AMF_VIDEO_ENCODER_ENGINE_DEFAULT }, AMF_VIDEO_ENCODER_ENGINE_DEFAULT, AMF_VIDEO_ENCODER_ENGINE_VULKAN, VE, "engine" },
> +    { "dxva2",          "DirectX Video Acceleration 2", 0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_DXVA2   }, 0, 0, VE, "engine" },
> +    { "d3d11",          "Direct2D11",                   0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_D3D11   }, 0, 0, VE, "engine" },
> +    { "vulkan",         "Vulkan",                       0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_VULKAN  }, 0, 0, VE, "engine" },
> +#endif

Can this be common rather than duplicated?  It doesn't have the crazy option dependency on the codec like all of the AMF options.

> +
>       { "rc",             "Set the rate control mode",            OFFSET(rate_control_mode), AV_OPT_TYPE_INT, { .i64 = AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_UNKNOWN }, AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_UNKNOWN, AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR, VE, "rc" },
>       { "cqp",            "Constant Quantization Parameter",      0, AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CONSTANT_QP             }, 0, 0, VE, "rc" },
>       { "cbr",            "Constant Bitrate",                     0, AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR                     }, 0, 0, VE, "rc" },
> diff --git a/libavutil/hwcontext_amf.c b/libavutil/hwcontext_amf.c
> index b70ee90d40..faae57dda3 100644
> --- a/libavutil/hwcontext_amf.c
> +++ b/libavutil/hwcontext_amf.c
> @@ -41,6 +41,7 @@
>   #else
>   #include <dlfcn.h>
>   #endif
> +#include "libavutil/opt.h"
>   
>   /**
>   * Error handling helper
> @@ -151,6 +152,51 @@ static int amf_init_device_ctx_object(AVHWDeviceContext *ctx)
>       return 0;
>   }
>   
> +static AMF_RESULT amf_context_init_d3d11(AVHWDeviceContext * avctx)
> +{
> +    AMF_RESULT res;
> +    AVAMFDeviceContext * ctx = avctx->hwctx;
> +    res = ctx->context->pVtbl->InitDX11(ctx->context, NULL, AMF_DX11_1);
> +    if (res == AMF_OK){
> +        av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via D3D11.\n");
> +    }
> +    return res;
> +}
> +
> +static AMF_RESULT amf_context_init_dxva2(AVHWDeviceContext * avctx)
> +{
> +    AMF_RESULT res;
> +    AVAMFDeviceContext * ctx = avctx->hwctx;
> +    res = ctx->context->pVtbl->InitDX9(ctx->context, NULL);
> +    if (res == AMF_OK){
> +        av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via DXVA2.\n");
> +    }
> +    return res;
> +}
> +
> +static AMF_RESULT amf_context_init_vulkan(AVHWDeviceContext * avctx)
> +{
> +    AMF_RESULT res;
> +    AVAMFDeviceContext * ctx = avctx->hwctx;
> +    AMFContext1 *context1 = NULL;
> +    AMFGuid guid = IID_AMFContext1();
> +
> +    res = ctx->context->pVtbl->QueryInterface(ctx->context, &guid, (void**)&context1);
> +    AMFAV_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "CreateContext1() fialed 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(avctx, AV_LOG_VERBOSE, "AMF via vulkan is not supported on the given device.\n");
> +        else
> +            av_log(avctx, AV_LOG_VERBOSE, "AMF failed to initialise on the given vulkan device. %d.\n", res);
> +        return AMF_FAIL;
> +    }
> +    av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via vulkan.\n");
> +    return res;
> +}
> +
>   static int amf_device_create(AVHWDeviceContext *ctx, const char *device,
>                                   AVDictionary *opts, int flags)
>   {
> @@ -158,35 +204,53 @@ static int amf_device_create(AVHWDeviceContext *ctx, const char *device,
>       AMFContext1 *context1 = NULL;
>       AMF_RESULT res;
>       int err;
> +    AVDictionaryEntry *e;
> +    int preffered_engine = AMF_VIDEO_ENCODER_ENGINE_DEFAULT;
>   
>       err = amf_init_device_ctx_object(ctx);
>       if(err < 0)
>           return err;
>   
> -    res = amf_ctx->context->pVtbl->InitDX11(amf_ctx->context, NULL, AMF_DX11_1);
> -    if (res == AMF_OK) {
> -        av_log(ctx, AV_LOG_VERBOSE, "AMF initialisation succeeded via D3D11.\n");
> -    } else {
> -        res = amf_ctx->context->pVtbl->InitDX9(amf_ctx->context, NULL);
> -        if (res == AMF_OK) {
> -            av_log(ctx, 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);
> -            AMFAV_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "CreateContext1() failed with error %d\n", res);
> -
> -            res = context1->pVtbl->InitVulkan(context1, NULL);
> -            context1->pVtbl->Release(context1);
> +    e = av_dict_get(opts, "engine", NULL, 0);
> +    if (!!e) {
> +        preffered_engine = atoi(e->value);
> +    }
> +
> +    res = AMF_FAIL;
> +    switch(preffered_engine) {
> +        case AMF_VIDEO_ENCODER_ENGINE_D3D11:
> +            res = amf_context_init_d3d11(ctx);
> +            break;
> +        case AMF_VIDEO_ENCODER_ENGINE_DXVA2:
> +            res = amf_context_init_dxva2(ctx);
> +            break;
> +        case AMF_VIDEO_ENCODER_ENGINE_VULKAN:
> +            res = amf_context_init_vulkan(ctx);
> +            break;
> +        default:
> +            break;
> +    }
> +    if (res != AMF_OK) {

What is the intent of the fallback?  If you asked for a particular backend then presumably you had a reason for that.

> +        if (preffered_engine != AMF_VIDEO_ENCODER_ENGINE_DEFAULT)
> +            av_log(ctx, AV_LOG_ERROR, "AMF failed to initialise via preffered engine.\n");
> +
> +        if (preffered_engine != AMF_VIDEO_ENCODER_ENGINE_D3D11)//already tried in switch case
> +            res = amf_context_init_d3d11(ctx);
> +
> +        if (res != AMF_OK) {
> +            if (preffered_engine != AMF_VIDEO_ENCODER_ENGINE_DXVA2)
> +            res = amf_context_init_dxva2(ctx);
> +
>               if (res != AMF_OK) {
> -                if (res == AMF_NOT_SUPPORTED)
> -                    av_log(ctx, AV_LOG_ERROR, "AMF via Vulkan is not supported on the given device.\n");
> -                else
> -                    av_log(ctx, AV_LOG_ERROR, "AMF failed to initialise on the given Vulkan device: %d.\n", res);
> -                return AVERROR(ENOSYS);
> +                if (preffered_engine != AMF_VIDEO_ENCODER_ENGINE_VULKAN)
> +                    res = amf_context_init_vulkan(ctx);
>               }
> -            av_log(ctx, AV_LOG_VERBOSE, "AMF initialisation succeeded via Vulkan.\n");
>           }
>       }
> +
> +    if (res != AMF_OK) {
> +        return AVERROR(ENOSYS);
> +    }
>       return 0;
>   }
>   
> diff --git a/libavutil/hwcontext_amf.h b/libavutil/hwcontext_amf.h
> index ae2a37f425..82e2ce7abb 100644
> --- a/libavutil/hwcontext_amf.h
> +++ b/libavutil/hwcontext_amf.h
> @@ -30,6 +30,13 @@
>   #include "AMF/core/Context.h"
>   #include "AMF/core/Factory.h"
>   
> +enum AMF_VIDEO_ENCODER_PREFFERED_ENGINE
> +{
> +    AMF_VIDEO_ENCODER_ENGINE_DEFAULT = 0,
> +    AMF_VIDEO_ENCODER_ENGINE_DXVA2,
> +    AMF_VIDEO_ENCODER_ENGINE_D3D11,
> +    AMF_VIDEO_ENCODER_ENGINE_VULKAN
> +};

This enum would need namespacing.  I suspect that passing strings around would be simpler and easier to reason about (e.g. "ffmpeg -init_hw_device amf:,engine=d3d11").

>   
>   /**
>    * This struct is allocated as AVHWDeviceContext.hwctx
> 

- Mark
Dmitrii Ovchinnikov Oct. 29, 2020, 12:23 a.m. UTC | #2 
>Why?  Given that you are already initialising from a specific device when
one is given, what does this actually change?
This code was created quite a long time ago in order to allow the user to
use the vulkan encoder on Windows.
For now, it looks like you can use AVVulkanDeviceContext instead.I will
make the appropriate patch and send it later, instead of this patch. Thank
you for the review!
Soft Works Oct. 29, 2020, 12:30 a.m. UTC | #3 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Mark Thompson
> Sent: Thursday, October 29, 2020 12:52 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 4/5] lavu/hwcontext_amf: Engine
> selection support for AMF context
> 
> On 15/10/2020 01:16, OvchinnikovDmitrii wrote:
> > ---
> >   libavcodec/amfenc.c       |   9 +++-
> >   libavcodec/amfenc.h       |   1 +
> >   libavcodec/amfenc_h264.c  |   8 +++
> >   libavcodec/amfenc_hevc.c  |   8 +++
> >   libavutil/hwcontext_amf.c | 104 ++++++++++++++++++++++++++++++---
> -----
> >   libavutil/hwcontext_amf.h |   7 +++
> >   6 files changed, 116 insertions(+), 21 deletions(-)
> 
> Why?  Given that you are already initialising from a specific device when one
> is given, what does this actually change?
> 
> (With patch 1 you can do "ffmpeg ... -init_hw_device d3d11:3 ... -c:v
> h264_amf ..." and it picks up the device you asked for.)
> 
> > diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c index
> > 767eab3d56..b7eb74811b 100644
> > --- a/libavcodec/amfenc.c
> > +++ b/libavcodec/amfenc.c
> > @@ -82,6 +82,7 @@ static int amf_init_context(AVCodecContext *avctx)
> >   {
> >       AmfContext *ctx = avctx->priv_data;
> >       AVAMFDeviceContext *amf_ctx;
> > +    AVDictionary *dict = NULL;
> >       int ret;
> >
> >       ctx->delayed_frame = av_frame_alloc(); @@ -123,10 +124,16 @@
> > static int amf_init_context(AVCodecContext *avctx)
> >           if (ret < 0)
> >               return ret;
> >       } else {
> > -        ret = av_hwdevice_ctx_create(&ctx->amf_device_ctx,
> AV_HWDEVICE_TYPE_AMF, NULL, NULL, 0);
> > +#if defined(_WIN32)
> > +    if (ctx->engine) {
> > +        ret = av_dict_set_int(&dict, "engine", ctx->engine, 0);
> >           if (ret < 0)
> >               return ret;
> >       }
> > +#endif
> 
> Doesn't seem obviously Windows-dependent?

Probably, because on Linux there is no choice (only Vulkan).

> 
> > +        ret = av_hwdevice_ctx_create(&ctx->amf_device_ctx,
> AV_HWDEVICE_TYPE_AMF, NULL, dict, 0);if (ret < 0)
> > +            return ret;
> > +    }
> >       amf_ctx = ((AVHWDeviceContext*)ctx->amf_device_ctx->data)-
> >hwctx;
> >       ctx->context = amf_ctx->context;
> >       ctx->factory = amf_ctx->factory; diff --git
> > a/libavcodec/amfenc.h b/libavcodec/amfenc.h index
> > 96d1942a37..f4897c9b2a 100644
> > --- a/libavcodec/amfenc.h
> > +++ b/libavcodec/amfenc.h
> > @@ -73,6 +73,7 @@ typedef struct AmfContext {
> >       int                 quality;
> >       int                 b_frame_delta_qp;
> >       int                 ref_b_frame_delta_qp;
> > +    int                 engine;
> >
> >       // Dynamic options, can be set after Init() call
> >
> > diff --git a/libavcodec/amfenc_h264.c b/libavcodec/amfenc_h264.c index
> > 622ee85946..9fb42323d8 100644
> > --- a/libavcodec/amfenc_h264.c
> > +++ b/libavcodec/amfenc_h264.c
> > @@ -71,6 +71,14 @@ static const AVOption options[] = {
> >       { "balanced",       "Balanced",                             0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_QUALITY_PRESET_BALANCED },    0, 0, VE, "quality" },
> >       { "quality",        "Prefer Quality",                       0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_QUALITY_PRESET_QUALITY  },     0, 0, VE, "quality" },
> >
> > +#if defined(_WIN32)
> > +//preffered engine
> > +    { "engine",         "Preffered engine",             OFFSET(engine),
> AV_OPT_TYPE_INT,   { .i64 = AMF_VIDEO_ENCODER_ENGINE_DEFAULT },
> AMF_VIDEO_ENCODER_ENGINE_DEFAULT,
> AMF_VIDEO_ENCODER_ENGINE_VULKAN, VE, "engine" },
> > +    { "dxva2",          "DirectX Video Acceleration 2", 0,
> AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_DXVA2   }, 0,
> 0, VE, "engine" },
> > +    { "d3d11",          "Direct2D11",                   0,              AV_OPT_TYPE_CONST, {
> .i64 = AMF_VIDEO_ENCODER_ENGINE_D3D11   }, 0, 0, VE, "engine" },
> > +    { "vulkan",         "Vulkan",                       0,              AV_OPT_TYPE_CONST, {
> .i64 = AMF_VIDEO_ENCODER_ENGINE_VULKAN  }, 0, 0, VE, "engine" },
> > +#endif
> 
> The presence of options shouldn't be platform dependent, even if they don't
> actually get used in other places.

That doesn't make sense to me. Why show Direct3D options on Linux?
I don't think there is any other case where ffmpeg shows any D3D parameters on Linux.


> 
> Also, "preferred" is normally spelled with one 'f' and two 'r's (also in more
> places below).
> 
> > +
> >       // Dynamic
> >       /// Rate Control Method
> >       { "rc",             "Rate Control Method",
> OFFSET(rate_control_mode), AV_OPT_TYPE_INT,   { .i64 =
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_UNKNOWN },
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_UNKNOWN,
> AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_LATENCY_CONSTRAINE
> D_VBR, VE, "rc" },
> > diff --git a/libavcodec/amfenc_hevc.c b/libavcodec/amfenc_hevc.c index
> > bb224c5fec..fc2c40a6ed 100644
> > --- a/libavcodec/amfenc_hevc.c
> > +++ b/libavcodec/amfenc_hevc.c
> > @@ -58,6 +58,14 @@ static const AVOption options[] = {
> >       { "speed",          "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_SPEED    }, 0, 0, VE,
> "quality" },
> >       { "quality",        "", 0, AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_QUALITY  }, 0, 0, VE,
> "quality" },
> >
> > +#if defined(_WIN32)
> > +//preffered engine
> > +    { "engine",         "Preffered engine",             OFFSET(engine),
> AV_OPT_TYPE_INT,   { .i64 = AMF_VIDEO_ENCODER_ENGINE_DEFAULT },
> AMF_VIDEO_ENCODER_ENGINE_DEFAULT,
> AMF_VIDEO_ENCODER_ENGINE_VULKAN, VE, "engine" },
> > +    { "dxva2",          "DirectX Video Acceleration 2", 0,
> AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_DXVA2   }, 0,
> 0, VE, "engine" },
> > +    { "d3d11",          "Direct2D11",                   0,              AV_OPT_TYPE_CONST, {
> .i64 = AMF_VIDEO_ENCODER_ENGINE_D3D11   }, 0, 0, VE, "engine" },
> > +    { "vulkan",         "Vulkan",                       0,              AV_OPT_TYPE_CONST, {
> .i64 = AMF_VIDEO_ENCODER_ENGINE_VULKAN  }, 0, 0, VE, "engine" },
> > +#endif
> 
> Can this be common rather than duplicated?  It doesn't have the crazy option
> dependency on the codec like all of the AMF options.
> 
> > +
> >       { "rc",             "Set the rate control mode",
> OFFSET(rate_control_mode), AV_OPT_TYPE_INT, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_UNKNOWN },
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_UNKNOWN,
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR, VE, "rc" },
> >       { "cqp",            "Constant Quantization Parameter",      0,
> AV_OPT_TYPE_CONST, { .i64 =
> AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CONSTANT_QP
> }, 0, 0, VE, "rc" },
> >       { "cbr",            "Constant Bitrate",                     0, AV_OPT_TYPE_CONST, {
> .i64 = AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR
> }, 0, 0, VE, "rc" },
> > diff --git a/libavutil/hwcontext_amf.c b/libavutil/hwcontext_amf.c
> > index b70ee90d40..faae57dda3 100644
> > --- a/libavutil/hwcontext_amf.c
> > +++ b/libavutil/hwcontext_amf.c
> > @@ -41,6 +41,7 @@
> >   #else
> >   #include <dlfcn.h>
> >   #endif
> > +#include "libavutil/opt.h"
> >
> >   /**
> >   * Error handling helper
> > @@ -151,6 +152,51 @@ static int
> amf_init_device_ctx_object(AVHWDeviceContext *ctx)
> >       return 0;
> >   }
> >
> > +static AMF_RESULT amf_context_init_d3d11(AVHWDeviceContext *
> avctx) {
> > +    AMF_RESULT res;
> > +    AVAMFDeviceContext * ctx = avctx->hwctx;
> > +    res = ctx->context->pVtbl->InitDX11(ctx->context, NULL,
> AMF_DX11_1);
> > +    if (res == AMF_OK){
> > +        av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via
> D3D11.\n");
> > +    }
> > +    return res;
> > +}
> > +
> > +static AMF_RESULT amf_context_init_dxva2(AVHWDeviceContext *
> avctx) {
> > +    AMF_RESULT res;
> > +    AVAMFDeviceContext * ctx = avctx->hwctx;
> > +    res = ctx->context->pVtbl->InitDX9(ctx->context, NULL);
> > +    if (res == AMF_OK){
> > +        av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via
> DXVA2.\n");
> > +    }
> > +    return res;
> > +}
> > +
> > +static AMF_RESULT amf_context_init_vulkan(AVHWDeviceContext *
> avctx)
> > +{
> > +    AMF_RESULT res;
> > +    AVAMFDeviceContext * ctx = avctx->hwctx;
> > +    AMFContext1 *context1 = NULL;
> > +    AMFGuid guid = IID_AMFContext1();
> > +
> > +    res = ctx->context->pVtbl->QueryInterface(ctx->context, &guid,
> (void**)&context1);
> > +    AMFAV_RETURN_IF_FALSE(ctx, res == AMF_OK,
> AVERROR_UNKNOWN,
> > + "CreateContext1() fialed 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(avctx, AV_LOG_VERBOSE, "AMF via vulkan is not supported
> on the given device.\n");
> > +        else
> > +            av_log(avctx, AV_LOG_VERBOSE, "AMF failed to initialise on the
> given vulkan device. %d.\n", res);
> > +        return AMF_FAIL;
> > +    }
> > +    av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via
> vulkan.\n");
> > +    return res;
> > +}
> > +
> >   static int amf_device_create(AVHWDeviceContext *ctx, const char
> *device,
> >                                   AVDictionary *opts, int flags)
> >   {
> > @@ -158,35 +204,53 @@ static int
> amf_device_create(AVHWDeviceContext *ctx, const char *device,
> >       AMFContext1 *context1 = NULL;
> >       AMF_RESULT res;
> >       int err;
> > +    AVDictionaryEntry *e;
> > +    int preffered_engine = AMF_VIDEO_ENCODER_ENGINE_DEFAULT;
> >
> >       err = amf_init_device_ctx_object(ctx);
> >       if(err < 0)
> >           return err;
> >
> > -    res = amf_ctx->context->pVtbl->InitDX11(amf_ctx->context, NULL,
> AMF_DX11_1);
> > -    if (res == AMF_OK) {
> > -        av_log(ctx, AV_LOG_VERBOSE, "AMF initialisation succeeded via
> D3D11.\n");
> > -    } else {
> > -        res = amf_ctx->context->pVtbl->InitDX9(amf_ctx->context, NULL);
> > -        if (res == AMF_OK) {
> > -            av_log(ctx, 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);
> > -            AMFAV_RETURN_IF_FALSE(ctx, res == AMF_OK,
> AVERROR_UNKNOWN, "CreateContext1() failed with error %d\n", res);
> > -
> > -            res = context1->pVtbl->InitVulkan(context1, NULL);
> > -            context1->pVtbl->Release(context1);
> > +    e = av_dict_get(opts, "engine", NULL, 0);
> > +    if (!!e) {
> > +        preffered_engine = atoi(e->value);
> > +    }
> > +
> > +    res = AMF_FAIL;
> > +    switch(preffered_engine) {
> > +        case AMF_VIDEO_ENCODER_ENGINE_D3D11:
> > +            res = amf_context_init_d3d11(ctx);
> > +            break;
> > +        case AMF_VIDEO_ENCODER_ENGINE_DXVA2:
> > +            res = amf_context_init_dxva2(ctx);
> > +            break;
> > +        case AMF_VIDEO_ENCODER_ENGINE_VULKAN:
> > +            res = amf_context_init_vulkan(ctx);
> > +            break;
> > +        default:
> > +            break;
> > +    }
> > +    if (res != AMF_OK) {
> 
> What is the intent of the fallback?  If you asked for a particular backend then
> presumably you had a reason for that.

I very much agree that such fallbacks should not happen (when a specific choice have been made).

@Dmitri - Great that you guys are finally getting the ball rolling..!

Kind regards,
softworkz
diff mbox series

Patch

diff --git a/libavcodec/amfenc.c b/libavcodec/amfenc.c
index 767eab3d56..b7eb74811b 100644
--- a/libavcodec/amfenc.c
+++ b/libavcodec/amfenc.c
@@ -82,6 +82,7 @@  static int amf_init_context(AVCodecContext *avctx)
 {
     AmfContext *ctx = avctx->priv_data;
     AVAMFDeviceContext *amf_ctx;
+    AVDictionary *dict = NULL;
     int ret;
 
     ctx->delayed_frame = av_frame_alloc();
@@ -123,10 +124,16 @@  static int amf_init_context(AVCodecContext *avctx)
         if (ret < 0)
             return ret;
     } else {
-        ret = av_hwdevice_ctx_create(&ctx->amf_device_ctx, AV_HWDEVICE_TYPE_AMF, NULL, NULL, 0);
+#if defined(_WIN32)
+    if (ctx->engine) {
+        ret = av_dict_set_int(&dict, "engine", ctx->engine, 0);
         if (ret < 0)
             return ret;
     }
+#endif
+        ret = av_hwdevice_ctx_create(&ctx->amf_device_ctx, AV_HWDEVICE_TYPE_AMF, NULL, dict, 0);if (ret < 0)
+            return ret;
+    }
     amf_ctx = ((AVHWDeviceContext*)ctx->amf_device_ctx->data)->hwctx;
     ctx->context = amf_ctx->context;
     ctx->factory = amf_ctx->factory;
diff --git a/libavcodec/amfenc.h b/libavcodec/amfenc.h
index 96d1942a37..f4897c9b2a 100644
--- a/libavcodec/amfenc.h
+++ b/libavcodec/amfenc.h
@@ -73,6 +73,7 @@  typedef struct AmfContext {
     int                 quality;
     int                 b_frame_delta_qp;
     int                 ref_b_frame_delta_qp;
+    int                 engine;
 
     // Dynamic options, can be set after Init() call
 
diff --git a/libavcodec/amfenc_h264.c b/libavcodec/amfenc_h264.c
index 622ee85946..9fb42323d8 100644
--- a/libavcodec/amfenc_h264.c
+++ b/libavcodec/amfenc_h264.c
@@ -71,6 +71,14 @@  static const AVOption options[] = {
     { "balanced",       "Balanced",                             0,                  AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_QUALITY_PRESET_BALANCED },    0, 0, VE, "quality" },
     { "quality",        "Prefer Quality",                       0,                  AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_QUALITY_PRESET_QUALITY  },     0, 0, VE, "quality" },
 
+#if defined(_WIN32)
+//preffered engine
+    { "engine",         "Preffered engine",             OFFSET(engine), AV_OPT_TYPE_INT,   { .i64 = AMF_VIDEO_ENCODER_ENGINE_DEFAULT }, AMF_VIDEO_ENCODER_ENGINE_DEFAULT, AMF_VIDEO_ENCODER_ENGINE_VULKAN, VE, "engine" },
+    { "dxva2",          "DirectX Video Acceleration 2", 0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_DXVA2   }, 0, 0, VE, "engine" },
+    { "d3d11",          "Direct2D11",                   0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_D3D11   }, 0, 0, VE, "engine" },
+    { "vulkan",         "Vulkan",                       0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_VULKAN  }, 0, 0, VE, "engine" },
+#endif
+
     // Dynamic
     /// Rate Control Method
     { "rc",             "Rate Control Method",                  OFFSET(rate_control_mode), AV_OPT_TYPE_INT,   { .i64 = AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_UNKNOWN }, AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_UNKNOWN, AMF_VIDEO_ENCODER_RATE_CONTROL_METHOD_LATENCY_CONSTRAINED_VBR, VE, "rc" },
diff --git a/libavcodec/amfenc_hevc.c b/libavcodec/amfenc_hevc.c
index bb224c5fec..fc2c40a6ed 100644
--- a/libavcodec/amfenc_hevc.c
+++ b/libavcodec/amfenc_hevc.c
@@ -58,6 +58,14 @@  static const AVOption options[] = {
     { "speed",          "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_SPEED    }, 0, 0, VE, "quality" },
     { "quality",        "", 0, AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_HEVC_QUALITY_PRESET_QUALITY  }, 0, 0, VE, "quality" },
 
+#if defined(_WIN32)
+//preffered engine
+    { "engine",         "Preffered engine",             OFFSET(engine), AV_OPT_TYPE_INT,   { .i64 = AMF_VIDEO_ENCODER_ENGINE_DEFAULT }, AMF_VIDEO_ENCODER_ENGINE_DEFAULT, AMF_VIDEO_ENCODER_ENGINE_VULKAN, VE, "engine" },
+    { "dxva2",          "DirectX Video Acceleration 2", 0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_DXVA2   }, 0, 0, VE, "engine" },
+    { "d3d11",          "Direct2D11",                   0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_D3D11   }, 0, 0, VE, "engine" },
+    { "vulkan",         "Vulkan",                       0,              AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_ENGINE_VULKAN  }, 0, 0, VE, "engine" },
+#endif
+
     { "rc",             "Set the rate control mode",            OFFSET(rate_control_mode), AV_OPT_TYPE_INT, { .i64 = AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_UNKNOWN }, AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_UNKNOWN, AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR, VE, "rc" },
     { "cqp",            "Constant Quantization Parameter",      0, AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CONSTANT_QP             }, 0, 0, VE, "rc" },
     { "cbr",            "Constant Bitrate",                     0, AV_OPT_TYPE_CONST, { .i64 = AMF_VIDEO_ENCODER_HEVC_RATE_CONTROL_METHOD_CBR                     }, 0, 0, VE, "rc" },
diff --git a/libavutil/hwcontext_amf.c b/libavutil/hwcontext_amf.c
index b70ee90d40..faae57dda3 100644
--- a/libavutil/hwcontext_amf.c
+++ b/libavutil/hwcontext_amf.c
@@ -41,6 +41,7 @@ 
 #else
 #include <dlfcn.h>
 #endif
+#include "libavutil/opt.h"
 
 /**
 * Error handling helper
@@ -151,6 +152,51 @@  static int amf_init_device_ctx_object(AVHWDeviceContext *ctx)
     return 0;
 }
 
+static AMF_RESULT amf_context_init_d3d11(AVHWDeviceContext * avctx)
+{
+    AMF_RESULT res;
+    AVAMFDeviceContext * ctx = avctx->hwctx;
+    res = ctx->context->pVtbl->InitDX11(ctx->context, NULL, AMF_DX11_1);
+    if (res == AMF_OK){
+        av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via D3D11.\n");
+    }
+    return res;
+}
+
+static AMF_RESULT amf_context_init_dxva2(AVHWDeviceContext * avctx)
+{
+    AMF_RESULT res;
+    AVAMFDeviceContext * ctx = avctx->hwctx;
+    res = ctx->context->pVtbl->InitDX9(ctx->context, NULL);
+    if (res == AMF_OK){
+        av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via DXVA2.\n");
+    }
+    return res;
+}
+
+static AMF_RESULT amf_context_init_vulkan(AVHWDeviceContext * avctx)
+{
+    AMF_RESULT res;
+    AVAMFDeviceContext * ctx = avctx->hwctx;
+    AMFContext1 *context1 = NULL;
+    AMFGuid guid = IID_AMFContext1();
+
+    res = ctx->context->pVtbl->QueryInterface(ctx->context, &guid, (void**)&context1);
+    AMFAV_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "CreateContext1() fialed 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(avctx, AV_LOG_VERBOSE, "AMF via vulkan is not supported on the given device.\n");
+        else
+            av_log(avctx, AV_LOG_VERBOSE, "AMF failed to initialise on the given vulkan device. %d.\n", res);
+        return AMF_FAIL;
+    }
+    av_log(avctx, AV_LOG_VERBOSE, "AMF initialisation successed via vulkan.\n");
+    return res;
+}
+
 static int amf_device_create(AVHWDeviceContext *ctx, const char *device,
                                 AVDictionary *opts, int flags)
 {
@@ -158,35 +204,53 @@  static int amf_device_create(AVHWDeviceContext *ctx, const char *device,
     AMFContext1 *context1 = NULL;
     AMF_RESULT res;
     int err;
+    AVDictionaryEntry *e;
+    int preffered_engine = AMF_VIDEO_ENCODER_ENGINE_DEFAULT;
 
     err = amf_init_device_ctx_object(ctx);
     if(err < 0)
         return err;
 
-    res = amf_ctx->context->pVtbl->InitDX11(amf_ctx->context, NULL, AMF_DX11_1);
-    if (res == AMF_OK) {
-        av_log(ctx, AV_LOG_VERBOSE, "AMF initialisation succeeded via D3D11.\n");
-    } else {
-        res = amf_ctx->context->pVtbl->InitDX9(amf_ctx->context, NULL);
-        if (res == AMF_OK) {
-            av_log(ctx, 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);
-            AMFAV_RETURN_IF_FALSE(ctx, res == AMF_OK, AVERROR_UNKNOWN, "CreateContext1() failed with error %d\n", res);
-
-            res = context1->pVtbl->InitVulkan(context1, NULL);
-            context1->pVtbl->Release(context1);
+    e = av_dict_get(opts, "engine", NULL, 0);
+    if (!!e) {
+        preffered_engine = atoi(e->value);
+    }
+
+    res = AMF_FAIL;
+    switch(preffered_engine) {
+        case AMF_VIDEO_ENCODER_ENGINE_D3D11:
+            res = amf_context_init_d3d11(ctx);
+            break;
+        case AMF_VIDEO_ENCODER_ENGINE_DXVA2:
+            res = amf_context_init_dxva2(ctx);
+            break;
+        case AMF_VIDEO_ENCODER_ENGINE_VULKAN:
+            res = amf_context_init_vulkan(ctx);
+            break;
+        default:
+            break;
+    }
+    if (res != AMF_OK) {
+        if (preffered_engine != AMF_VIDEO_ENCODER_ENGINE_DEFAULT)
+            av_log(ctx, AV_LOG_ERROR, "AMF failed to initialise via preffered engine.\n");
+
+        if (preffered_engine != AMF_VIDEO_ENCODER_ENGINE_D3D11)//already tried in switch case
+            res = amf_context_init_d3d11(ctx);
+
+        if (res != AMF_OK) {
+            if (preffered_engine != AMF_VIDEO_ENCODER_ENGINE_DXVA2)
+            res = amf_context_init_dxva2(ctx);
+
             if (res != AMF_OK) {
-                if (res == AMF_NOT_SUPPORTED)
-                    av_log(ctx, AV_LOG_ERROR, "AMF via Vulkan is not supported on the given device.\n");
-                else
-                    av_log(ctx, AV_LOG_ERROR, "AMF failed to initialise on the given Vulkan device: %d.\n", res);
-                return AVERROR(ENOSYS);
+                if (preffered_engine != AMF_VIDEO_ENCODER_ENGINE_VULKAN)
+                    res = amf_context_init_vulkan(ctx);
             }
-            av_log(ctx, AV_LOG_VERBOSE, "AMF initialisation succeeded via Vulkan.\n");
         }
     }
+
+    if (res != AMF_OK) {
+        return AVERROR(ENOSYS);
+    }
     return 0;
 }
 
diff --git a/libavutil/hwcontext_amf.h b/libavutil/hwcontext_amf.h
index ae2a37f425..82e2ce7abb 100644
--- a/libavutil/hwcontext_amf.h
+++ b/libavutil/hwcontext_amf.h
@@ -30,6 +30,13 @@ 
 #include "AMF/core/Context.h"
 #include "AMF/core/Factory.h"
 
+enum AMF_VIDEO_ENCODER_PREFFERED_ENGINE
+{
+    AMF_VIDEO_ENCODER_ENGINE_DEFAULT = 0,
+    AMF_VIDEO_ENCODER_ENGINE_DXVA2,
+    AMF_VIDEO_ENCODER_ENGINE_D3D11,
+    AMF_VIDEO_ENCODER_ENGINE_VULKAN
+};
 
 /**
  * This struct is allocated as AVHWDeviceContext.hwctx