Re: [libav-devel] [Libav-devel][Patch] Patch for SDK 7 for NVENC
On 16/09/16 14:50, Yogender Gupta wrote: Thanks Diego, Luca. Will take care of your comments for the next submissions. Few points based on your review : 1. We can keep the default as HQ only instead of medium (mapping of medium is already to HQ), however we found of lot of people comfortable using slow, medium and fast, similar to what they find in x264. And that is the motivation of adding these, and making one of these defaults. At some point we may even have these kind of presets only, nothing decided as yet. That can be split in a separate patch I assume people using sdk 6 might like that. 2. Am bad at writing, will try to improve the log message next time Adding few lines to explain the change is usually quite appreciated =) 3. I couldn't find any entry for Range extensions for HEVC, so added, if you feel it already exists let me know, and I will try to put a patch with the existing one. I spun that in a separate patch, the change itself is ok. 4. The preset change may not be stray, the GUIDs have to be in the array in the same order as they are referred by. LLHP and LLHQ somehow got interchanged and that is why this change, let me know if I got this wrong. It is ok, I'll spun this as a stand-alone patch with a bump in the version requirement in configure. 5. Will follow the notation more closely as suggested by you. Much appreciated =) lu - updating to sdk 7 now. ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
Re: [libav-devel] [Libav-devel][Patch] Patch for SDK 7 for NVENC
Please don't top-post :)
On Fri, Sep 16, 2016 at 12:50:43PM +, Yogender Gupta wrote:
> Few points based on your review :
> 1. We can keep the default as HQ only instead of medium (mapping of
> medium is already to HQ), however we found of lot of people comfortable
> using slow, medium and fast, similar to what they find in x264. And that
> is the motivation of adding these, and making one of these defaults. At
> some point we may even have these kind of presets only, nothing decided
> as yet.
This should be a separate change and not grouped in with the other
changes.
> 4. The preset change may not be stray, the GUIDs have to be in the
> array in the same order as they are referred by. LLHP and LLHQ somehow got
> interchanged and that is why this change, let me know if I got this wrong.
May or may not? :)
This should be a separate change as well.
Bug fixes and behavior changes do not belong together with an API
update.
> -Original Message-
> From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of Diego
> Biurrun
> Sent: Friday, September 16, 2016 1:27 AM
> To: libav development
> Subject: Re: [libav-devel] [Libav-devel][Patch] Patch for SDK 7 for NVENC
>
> Some comments for next time as Luca said he wanted to take care of it.
> Please try to maintain the style of the file you are working in.
>
> > --- a/libavcodec/nvenc_hevc.c
> > +++ b/libavcodec/nvenc_hevc.c
> > @@ -27,8 +27,11 @@
> > #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> > static const AVOption options[] = {
> > -{ "preset", "Set the encoding preset", OFFSET(preset),
> > AV_OPT_TYPE_INT,{ .i64 = PRESET_HQ }, PRESET_DEFAULT,
> > PRESET_LOSSLESS_HP, VE, "preset" },
> > +{ "preset", "Set the encoding preset", OFFSET(preset),
> > AV_OPT_TYPE_INT,{ .i64 = PRESET_MEDIUM }, PRESET_DEFAULT,
> > PRESET_LOSSLESS_HP, VE, "preset" },
> > --- a/libavcodec/nvenc_h264.c
> > +++ b/libavcodec/nvenc_h264.c
> > @@ -27,8 +27,11 @@
> > #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> > static const AVOption options[] = {
> > -{ "preset", "Set the encoding preset", OFFSET(preset),
> > AV_OPT_TYPE_INT,{ .i64 = PRESET_HQ }, PRESET_DEFAULT,
> > PRESET_LOSSLESS_HP, VE, "preset" },
> > +{ "preset", "Set the encoding preset", OFFSET(preset),
> > AV_OPT_TYPE_INT,{ .i64 = PRESET_MEDIUM }, PRESET_DEFAULT,
> > PRESET_LOSSLESS_HP, VE, "preset" },
>
> Why are you changing the default from HQ to MEDIUM?
>
> On Thu, Sep 15, 2016 at 10:40:10AM +, Yogender Gupta wrote:
> > From cc4f606cfba7b1ced644fe597b1b424f2af0f2bc Mon Sep 17 00:00:00 2001
> > From: Yogender Gupta <ygu...@nvidia.com>
> > Date: Thu, 15 Sep 2016 16:06:44 +0530
> > Subject: [PATCH] Patch for SDK 7 for NVENC
>
> The log message should be
>
> nvenc: Update for SDK 7
>
> or similar.
>
> > ---
> > libavcodec/avcodec.h| 1 +
> > libavcodec/nvenc.c | 117
> > ++--
> > libavcodec/nvenc.h | 23 +-
> > libavcodec/nvenc_h264.c | 15 ++- libavcodec/nvenc_hevc.c | 19
> > ++--
> > 5 files changed, 166 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index
> > 7a5f10f..607688c 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -2961,6 +2961,7 @@ typedef struct AVCodecContext {
> > #define FF_PROFILE_HEVC_MAIN1
> > #define FF_PROFILE_HEVC_MAIN_10 2
> > #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE 3
> > +#define FF_PROFILE_HEVC_REXT4
>
> This is an installed header, so there should be a matching APIchanges entry.
>
> > --- a/libavcodec/nvenc.c
> > +++ b/libavcodec/nvenc.c
> > @@ -463,12 +471,15 @@ static int nvec_map_preset(NVENCContext *ctx)
> > GUIDTuple presets[] = {
> > { NV_ENC_PRESET_BD_GUID },
> > { NV_ENC_PRESET_LOW_LATENCY_DEFAULT_GUID, NVENC_LOWLATENCY },
> > -{ NV_ENC_PRESET_LOW_LATENCY_HP_GUID, NVENC_LOWLATENCY },
> > { NV_ENC_PRESET_LOW_LATENCY_HQ_GUID, NVENC_LOWLATENCY },
> > +{ NV_ENC_PRESET_LOW_LATENCY_HP_GUID, NVENC_LOWLATENCY },
> > { NV_ENC_PRESET_LOSSLESS_DEFAULT_GUID,NVENC_LOSSLESS },
> > { NV_ENC_PRESET_LOSSLESS_HP_GUID,
Re: [libav-devel] [Libav-devel][Patch] Patch for SDK 7 for NVENC
Thanks Diego, Luca. Will take care of your comments for the next submissions.
Few points based on your review :
1. We can keep the default as HQ only instead of medium (mapping of medium is
already to HQ), however we found of lot of people comfortable using slow,
medium and fast, similar to what they find in x264. And that is the motivation
of adding these, and making one of these defaults. At some point we may even
have these kind of presets only, nothing decided as yet.
2. Am bad at writing, will try to improve the log message next time
3. I couldn't find any entry for Range extensions for HEVC, so added, if you
feel it already exists let me know, and I will try to put a patch with the
existing one.
4. The preset change may not be stray, the GUIDs have to be in the array in the
same order as they are referred by. LLHP and LLHQ somehow got interchanged and
that is why this change, let me know if I got this wrong.
5. Will follow the notation more closely as suggested by you.
Thanks,
Yogender
-Original Message-
From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of Diego
Biurrun
Sent: Friday, September 16, 2016 1:27 AM
To: libav development
Subject: Re: [libav-devel] [Libav-devel][Patch] Patch for SDK 7 for NVENC
Some comments for next time as Luca said he wanted to take care of it.
Please try to maintain the style of the file you are working in.
> --- a/libavcodec/nvenc_hevc.c
> +++ b/libavcodec/nvenc_hevc.c
> @@ -27,8 +27,11 @@
> #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> static const AVOption options[] = {
> -{ "preset", "Set the encoding preset", OFFSET(preset),
> AV_OPT_TYPE_INT,{ .i64 = PRESET_HQ }, PRESET_DEFAULT,
> PRESET_LOSSLESS_HP, VE, "preset" },
> +{ "preset", "Set the encoding preset", OFFSET(preset),
> AV_OPT_TYPE_INT,{ .i64 = PRESET_MEDIUM }, PRESET_DEFAULT,
> PRESET_LOSSLESS_HP, VE, "preset" },
> --- a/libavcodec/nvenc_h264.c
> +++ b/libavcodec/nvenc_h264.c
> @@ -27,8 +27,11 @@
> #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> static const AVOption options[] = {
> -{ "preset", "Set the encoding preset", OFFSET(preset),
> AV_OPT_TYPE_INT,{ .i64 = PRESET_HQ }, PRESET_DEFAULT,
> PRESET_LOSSLESS_HP, VE, "preset" },
> +{ "preset", "Set the encoding preset", OFFSET(preset),
> AV_OPT_TYPE_INT,{ .i64 = PRESET_MEDIUM }, PRESET_DEFAULT,
> PRESET_LOSSLESS_HP, VE, "preset" },
Why are you changing the default from HQ to MEDIUM?
On Thu, Sep 15, 2016 at 10:40:10AM +, Yogender Gupta wrote:
> From cc4f606cfba7b1ced644fe597b1b424f2af0f2bc Mon Sep 17 00:00:00 2001
> From: Yogender Gupta <ygu...@nvidia.com>
> Date: Thu, 15 Sep 2016 16:06:44 +0530
> Subject: [PATCH] Patch for SDK 7 for NVENC
The log message should be
nvenc: Update for SDK 7
or similar.
> ---
> libavcodec/avcodec.h| 1 +
> libavcodec/nvenc.c | 117
> ++--
> libavcodec/nvenc.h | 23 +-
> libavcodec/nvenc_h264.c | 15 ++- libavcodec/nvenc_hevc.c | 19
> ++--
> 5 files changed, 166 insertions(+), 9 deletions(-)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index
> 7a5f10f..607688c 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2961,6 +2961,7 @@ typedef struct AVCodecContext {
> #define FF_PROFILE_HEVC_MAIN1
> #define FF_PROFILE_HEVC_MAIN_10 2
> #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE 3
> +#define FF_PROFILE_HEVC_REXT4
This is an installed header, so there should be a matching APIchanges entry.
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -463,12 +471,15 @@ static int nvec_map_preset(NVENCContext *ctx)
> GUIDTuple presets[] = {
> { NV_ENC_PRESET_BD_GUID },
> { NV_ENC_PRESET_LOW_LATENCY_DEFAULT_GUID, NVENC_LOWLATENCY },
> -{ NV_ENC_PRESET_LOW_LATENCY_HP_GUID, NVENC_LOWLATENCY },
> { NV_ENC_PRESET_LOW_LATENCY_HQ_GUID, NVENC_LOWLATENCY },
> +{ NV_ENC_PRESET_LOW_LATENCY_HP_GUID, NVENC_LOWLATENCY },
> { NV_ENC_PRESET_LOSSLESS_DEFAULT_GUID,NVENC_LOSSLESS },
> { NV_ENC_PRESET_LOSSLESS_HP_GUID, NVENC_LOSSLESS },
That looks like a stray change.
> @@ -589,6 +600,44 @@ static void
> nvenc_setup_rate_control(AVCodecContext *avctx)
>
> if (rc->averageBitRate > 0)
> avctx->bit_rate = rc->averageBitRate;
> +
> +if (ctx->aq)
> +{
if (ctx->aq) {
like in the rest of the file, more
Re: [libav-devel] [Libav-devel][Patch] Patch for SDK 7 for NVENC
Some comments for next time as Luca said he wanted to take care of it.
Please try to maintain the style of the file you are working in.
> --- a/libavcodec/nvenc_hevc.c
> +++ b/libavcodec/nvenc_hevc.c
> @@ -27,8 +27,11 @@
> #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> static const AVOption options[] = {
> -{ "preset", "Set the encoding preset", OFFSET(preset),
> AV_OPT_TYPE_INT,{ .i64 = PRESET_HQ }, PRESET_DEFAULT,
> PRESET_LOSSLESS_HP, VE, "preset" },
> +{ "preset", "Set the encoding preset", OFFSET(preset),
> AV_OPT_TYPE_INT,{ .i64 = PRESET_MEDIUM }, PRESET_DEFAULT,
> PRESET_LOSSLESS_HP, VE, "preset" },
> --- a/libavcodec/nvenc_h264.c
> +++ b/libavcodec/nvenc_h264.c
> @@ -27,8 +27,11 @@
> #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> static const AVOption options[] = {
> -{ "preset", "Set the encoding preset", OFFSET(preset),
> AV_OPT_TYPE_INT,{ .i64 = PRESET_HQ }, PRESET_DEFAULT,
> PRESET_LOSSLESS_HP, VE, "preset" },
> +{ "preset", "Set the encoding preset", OFFSET(preset),
> AV_OPT_TYPE_INT,{ .i64 = PRESET_MEDIUM }, PRESET_DEFAULT,
> PRESET_LOSSLESS_HP, VE, "preset" },
Why are you changing the default from HQ to MEDIUM?
On Thu, Sep 15, 2016 at 10:40:10AM +, Yogender Gupta wrote:
> From cc4f606cfba7b1ced644fe597b1b424f2af0f2bc Mon Sep 17 00:00:00 2001
> From: Yogender Gupta
> Date: Thu, 15 Sep 2016 16:06:44 +0530
> Subject: [PATCH] Patch for SDK 7 for NVENC
The log message should be
nvenc: Update for SDK 7
or similar.
> ---
> libavcodec/avcodec.h| 1 +
> libavcodec/nvenc.c | 117
> ++--
> libavcodec/nvenc.h | 23 +-
> libavcodec/nvenc_h264.c | 15 ++-
> libavcodec/nvenc_hevc.c | 19 ++--
> 5 files changed, 166 insertions(+), 9 deletions(-)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 7a5f10f..607688c 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2961,6 +2961,7 @@ typedef struct AVCodecContext {
> #define FF_PROFILE_HEVC_MAIN1
> #define FF_PROFILE_HEVC_MAIN_10 2
> #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE 3
> +#define FF_PROFILE_HEVC_REXT4
This is an installed header, so there should be a matching APIchanges
entry.
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -463,12 +471,15 @@ static int nvec_map_preset(NVENCContext *ctx)
> GUIDTuple presets[] = {
> { NV_ENC_PRESET_BD_GUID },
> { NV_ENC_PRESET_LOW_LATENCY_DEFAULT_GUID, NVENC_LOWLATENCY },
> -{ NV_ENC_PRESET_LOW_LATENCY_HP_GUID, NVENC_LOWLATENCY },
> { NV_ENC_PRESET_LOW_LATENCY_HQ_GUID, NVENC_LOWLATENCY },
> +{ NV_ENC_PRESET_LOW_LATENCY_HP_GUID, NVENC_LOWLATENCY },
> { NV_ENC_PRESET_LOSSLESS_DEFAULT_GUID,NVENC_LOSSLESS },
> { NV_ENC_PRESET_LOSSLESS_HP_GUID, NVENC_LOSSLESS },
That looks like a stray change.
> @@ -589,6 +600,44 @@ static void nvenc_setup_rate_control(AVCodecContext
> *avctx)
>
> if (rc->averageBitRate > 0)
> avctx->bit_rate = rc->averageBitRate;
> +
> +if (ctx->aq)
> +{
if (ctx->aq) {
like in the rest of the file, more below.
> +av_log(avctx, AV_LOG_INFO, "AQ Enabled\n");
"enabled" :)
> +av_log(avctx, AV_LOG_WARNING, "Lookahead Not Enabled. Increase
> Buffer Delay (-delay) \n");
Let's not capitalize every single word :)
> +}
> +else {
} else {
> +ctx->config.rcParams.enableLookahead = 1;
> +ctx->config.rcParams.lookaheadDepth = av_clip(ctx->rc_lookahead,
> 0, lkd_bound);
> +ctx->config.rcParams.disableIadapt = ctx->no_scenecut;
> +ctx->config.rcParams.disableBadapt = !ctx->b_adapt;
Align the = for better readability.
> +av_log(avctx, AV_LOG_INFO, "Lookahead Enabled with depth %d,
> Scenecut %s, B-Adapt %s\n",
> +ctx->config.rcParams.lookaheadDepth,
> ctx->config.rcParams.disableIadapt ? "Disabled" : "Enabled",
> +ctx->config.rcParams.disableBadapt ? "Disabled" : "Enabled");
Indent the next line to align with the function parameters, like so:
av_log(avctx, AV_LOG_INFO, "Lookahead Enabled with depth %d, Scenecut %s,
B-Adapt %s\n",
ctx->config.rcParams.lookaheadDepth,
ctx->config.rcParams.disableIadapt ? "Disabled" : "Enabled",
ctx->config.rcParams.disableBadapt ? "Disabled" : "Enabled");
> @@ -692,9 +741,38 @@ static int nvenc_setup_hevc_config(AVCodecContext *avctx)
>
> -/* No other profile is supported in the current SDK version 5 */
> -cc->profileGUID = NV_ENC_HEVC_PROFILE_MAIN_GUID;
> -avctx->profile = FF_PROFILE_HEVC_MAIN;
> +switch(ctx->profile) {
switch (
> --- a/libavcodec/nvenc.h
> +++ b/libavcodec/nvenc.h
>
Re: [libav-devel] [Libav-devel][Patch] Patch for SDK 7 for NVENC
On 15/09/16 12:40, Yogender Gupta wrote: > Attached is patch for SDK 7 for NVENC. > I'll need the weekend to update my system to try it, there are a couple of style nits that I'll address while at it beside that seems fine. Thanks a lot! lu ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
[libav-devel] [Libav-devel][Patch] Patch for SDK 7 for NVENC
Attached is patch for SDK 7 for NVENC. Thanks, Yogender --- 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. --- 0001-Patch-for-SDK-7-for-NVENC.patch Description: 0001-Patch-for-SDK-7-for-NVENC.patch ___ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
