[FFmpeg-devel] [PATCH] Multi NVENC Split Frame Encoding in HEVC and AV1
Diego Felix de Souza
ddesouza at nvidia.com
Fri Apr 12 11:59:38 EEST 2024
Hi Timo,
Thank you for your review. Sorry for both superfulous spaces. Thank you for pointing them out, I will be more careful on the next patches. Please check my answers below.
Best regards,
Diego
From: Timo Rothenpieler <timo at rothenpieler.org>
Date: Thursday, 11. April 2024 at 15:50
To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
Cc: Diego Felix de Souza <ddesouza at nvidia.com>
Subject: Re: [FFmpeg-devel] [PATCH] Multi NVENC Split Frame Encoding in HEVC and AV1
External email: Use caution opening links or attachments
On 11/04/2024 13:58, Diego Felix de Souza via ffmpeg-devel wrote:
> From: Diego Felix de Souza <ddesouza at nvidia.com>
>
> When Split frame encoding is enabled, each input frame is partitioned into
> horizontal strips which are encoded independently and simultaneously by
> separate NVENCs, usually resulting in increased encoding speed compared to
> single NVENC encoding.
> ---
> libavcodec/nvenc.c | 16 ++++++++++++++++
> libavcodec/nvenc.h | 2 ++
> libavcodec/nvenc_av1.c | 8 ++++++++
> libavcodec/nvenc_hevc.c | 8 ++++++++
> 4 files changed, 34 insertions(+)
>
> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> index b6c5ed3e6b..f4d0d21715 100644
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -1696,6 +1696,22 @@ FF_ENABLE_DEPRECATION_WARNINGS
> if (ctx->weighted_pred == 1)
> ctx->init_encode_params.enableWeightedPrediction = 1;
>
> +#ifdef NVENC_HAVE_SPLIT_FRAME_ENCODING
> + if (avctx->codec->id != AV_CODEC_ID_H264 )
nit: superfluous space
In general, this check is probably not necessary, given the h264 encoder
does not have this AVOption so it'll always be zero-initialized there,
which should be fine?
Or would setting it to the equivalent of "auto" cause issues on h264?
You are right, this check is not necessary and the SDK will disregard the any value in the
splitEncodeMode for H.264. I put it to be clear that this functionality is not available for
H.264. I will remove it in the next patch for the sake of simplicity.
> + ctx->init_encode_params.splitEncodeMode = ctx->split_encode_mode;
> +
> + if ((ctx->split_encode_mode != NV_ENC_SPLIT_DISABLE_MODE) &&
> + ((ctx->weighted_pred == 1) && (avctx->codec->id == AV_CODEC_ID_HEVC ))) {
I think the ffmpeg recommended style would indent this line by 4 spaces.
What I usually do, even though it's technically against
style-guidelines, is indent the extra lines by 4, and then put the
opening { on its own line, since imo it greatly improves readability.
I remove the 4 spaces to fit in an 80 caracters line. However, I agree that it would
improve readability like you suggested. I will change it in the next version.
also a superfluous space at the end.
> + av_log(avctx, AV_LOG_WARNING, "Split encoding is not "
> + "supported if any of the following features: weighted prediction, "
> + "alpha layer encoding, subframe mode, output into video memory "
> + "buffer, picture timing/buffering period SEI message insertion "
> + "with DX12 interface are enabled in case of HEVC. For AV1, split "
> + "encoding is not supported when output into video memory buffer "
> + "is enabled.\n");
This message is a bit of a mouthful, and most of it is not applicable to
users.
I'd maybe shorten it to something like:
"Split encoding not supported if weighted prediction enabled."
Since that's the only option that can actually cause issues with the
current nvenc.c implementation.
Sure, I will short the warning message.
> + }
> +#endif
> +
> if (ctx->bluray_compat) {
> ctx->aud = 1;
> ctx->dpb_size = FFMIN(FFMAX(avctx->refs, 0), 6);
> diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
> index 85ecaf1b5f..09de00badc 100644
> --- a/libavcodec/nvenc.h
> +++ b/libavcodec/nvenc.h
> @@ -81,6 +81,7 @@ typedef void ID3D11Device;
> // SDK 12.1 compile time feature checks
> #if NVENCAPI_CHECK_VERSION(12, 1)
> #define NVENC_NO_DEPRECATED_RC
> +#define NVENC_HAVE_SPLIT_FRAME_ENCODING
> #endif
>
> // SDK 12.2 compile time feature checks
> @@ -280,6 +281,7 @@ typedef struct NvencContext
> int tf_level;
> int lookahead_level;
> int unidir_b;
> + int split_encode_mode;
> } NvencContext;
>
> int ff_nvenc_encode_init(AVCodecContext *avctx);
> diff --git a/libavcodec/nvenc_av1.c b/libavcodec/nvenc_av1.c
> index d37ee07bff..45dc3c26e0 100644
> --- a/libavcodec/nvenc_av1.c
> +++ b/libavcodec/nvenc_av1.c
> @@ -157,6 +157,14 @@ static const AVOption options[] = {
> { "1", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_LOOKAHEAD_LEVEL_1 }, 0, 0, VE, .unit = "lookahead_level" },
> { "2", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_LOOKAHEAD_LEVEL_2 }, 0, 0, VE, .unit = "lookahead_level" },
> { "3", "", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_LOOKAHEAD_LEVEL_3 }, 0, 0, VE, .unit = "lookahead_level" },
> +#endif
> +#ifdef NVENC_HAVE_SPLIT_FRAME_ENCODING
> + { "split_encode_mode", "Specifies the split encoding mode", OFFSET(split_encode_mode), AV_OPT_TYPE_INT, { .i64 = NV_ENC_SPLIT_DISABLE_MODE }, 0, NV_ENC_SPLIT_DISABLE_MODE, VE, .unit = "split_encode_mode" },
> + { "disabled", "Disabled for all configurations", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_DISABLE_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> + { "auto", "Enabled or disabled depending on the preset and tuning info", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_AUTO_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> + { "forced", "Enabled with number of horizontal strips selected by the driver", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_AUTO_FORCED_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> + { "2", "Enabled with number of horizontal strips forced to 2 when number of NVENCs > 1", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_TWO_FORCED_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> + { "3", "Enabled with number of horizontal strips forced to 3 when number of NVENCs > 2", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_THREE_FORCED_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> #endif
Shouldn't the default be "auto"?
At least I assume so far, without this option present, the field was
just zero initialized, which equates to auto.
So disabling it might surprise some users with degraded performance?
Or am I missing something?
You are correct. I will put the default option to be Auto in both HEVC and AV1.
> { NULL }
> };
> diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
> index bd8b6153f3..1f5e56ecd0 100644
> --- a/libavcodec/nvenc_hevc.c
> +++ b/libavcodec/nvenc_hevc.c
> @@ -216,6 +216,14 @@ static const AVOption options[] = {
> #endif
> #ifdef NVENC_HAVE_UNIDIR_B
> { "unidir_b", "Enable use of unidirectional B-Frames.", OFFSET(unidir_b), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
> +#endif
> +#ifdef NVENC_HAVE_SPLIT_FRAME_ENCODING
> + { "split_encode_mode", "Specifies the split encoding mode", OFFSET(split_encode_mode), AV_OPT_TYPE_INT, { .i64 = NV_ENC_SPLIT_DISABLE_MODE }, 0, NV_ENC_SPLIT_DISABLE_MODE, VE, .unit = "split_encode_mode" },
> + { "disabled", "Disabled for all configurations", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_DISABLE_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> + { "auto", "Enabled or disabled depending on the preset and tuning info", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_AUTO_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> + { "forced", "Enabled with number of horizontal strips selected by the driver", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_AUTO_FORCED_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> + { "2", "Enabled with number of horizontal strips forced to 2 when number of NVENCs > 1", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_TWO_FORCED_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> + { "3", "Enabled with number of horizontal strips forced to 3 when number of NVENCs > 2", 0, AV_OPT_TYPE_CONST, { .i64 = NV_ENC_SPLIT_THREE_FORCED_MODE }, 0, 0, VE, .unit = "split_encode_mode" },
> #endif
same as above
> { NULL }
> };
> --
> 2.34.1
>
> -----------------------------------------------------------------------------------
> NVIDIA GmbH
> Wuerselen
> Amtsgericht Aachen
> HRB 8361
> Managing Directors: Rebecca Peters, Donald Robertson, Janet Hall, Ludwig von Reiche
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may contain
> confidential information. Any unauthorized review, use, disclosure or distribution
> is prohibited. If you are not the intended recipient, please contact the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmpeg-devel&data=05%7C02%7Cddesouza%40nvidia.com%7C1a3192cf9f79411b8a8308dc5a2e4fa7%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638484402198960858%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=E9%2Fic1TW9%2FPq0c5LIWl%2FSfaYUqQ%2F%2F%2FR6mLEaZDfbGI0%3D&reserved=0<https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
-----------------------------------------------------------------------------------
NVIDIA GmbH
Wuerselen
Amtsgericht Aachen
HRB 8361
Managing Directors: Rebecca Peters, Donald Robertson, Janet Hall, Ludwig von Reiche
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
More information about the ffmpeg-devel
mailing list