> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-boun...@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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
