[FFmpeg-devel] [PATCH 4/5] lavu/hwcontext_amf: Engine selection support for AMF context
Mark Thompson
sw at jkqxz.net
Thu Oct 29 01:52:08 EET 2020
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
More information about the ffmpeg-devel
mailing list