diff mbox

[FFmpeg-devel] nvenc: Make AUD optional for h264_nvenc and hevc_nvenc

Message ID 586579DE.2090909@email.cz
State Superseded
Headers show

Commit Message

Miroslav Slugeň Dec. 29, 2016, 9:02 p.m. UTC
Somebody changed AUD to active in NVENC by default, which is not very 
clever, libx264 also has this future disabled, so we should stay in sync 
with libx264 behavior.

Enabled AUD will work only without B-frames. There is BUG in Nvidia 
NVENC when you use AUD for H264 with B-frames, it will return corrupted 
stream, because NVIDIA is inserting AUD type 0 (I-frame) before B-frames 
instead of AUD type 7 (any-frame).

H264 encoded with B-frames and AUD active will not play for example on 
Panasonic TX-AS640E, other decoders just ignore wrong AUD type.

Comments

Ali KIZIL Dec. 30, 2016, 4:48 a.m. UTC | #1
2016-12-30 0:02 GMT+03:00 Miroslav Slugeň <thunder.m@email.cz>:

> Somebody changed AUD to active in NVENC by default, which is not very
> clever, libx264 also has this future disabled, so we should stay in sync
> with libx264 behavior.
>
> Enabled AUD will work only without B-frames. There is BUG in Nvidia NVENC
> when you use AUD for H264 with B-frames, it will return corrupted stream,
> because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead of
> AUD type 7 (any-frame).
>
> H264 encoded with B-frames and AUD active will not play for example on
> Panasonic TX-AS640E, other decoders just ignore wrong AUD type.
>
>
> --
> Miroslav Slugeň
>
>
AUD set to active by default was for LG and Sony like UHD TVs which could
not decode NVENC encoded HEVC MPEGTS properly.
If patch sets AUD active by default if there is no B-Frames (as so in NVENC
HEVC encoding), patch will be ok in common.
Miroslav Slugeň Dec. 30, 2016, 8:38 a.m. UTC | #2
Dne 30.12.2016 v 05:48 Ali KIZIL napsal(a):
> 2016-12-30 0:02 GMT+03:00 Miroslav Slugeň <thunder.m@email.cz>:
>
>> Somebody changed AUD to active in NVENC by default, which is not very
>> clever, libx264 also has this future disabled, so we should stay in sync
>> with libx264 behavior.
>>
>> Enabled AUD will work only without B-frames. There is BUG in Nvidia NVENC
>> when you use AUD for H264 with B-frames, it will return corrupted stream,
>> because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead of
>> AUD type 7 (any-frame).
>>
>> H264 encoded with B-frames and AUD active will not play for example on
>> Panasonic TX-AS640E, other decoders just ignore wrong AUD type.
>>
>>
>> --
>> Miroslav Slugeň
>>
>>
> AUD set to active by default was for LG and Sony like UHD TVs which could
> not decode NVENC encoded HEVC MPEGTS properly.
> If patch sets AUD active by default if there is no B-Frames (as so in NVENC
> HEVC encoding), patch will be ok in common.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

I know that it was for LG tvs (i hav seen original enabling patch), and 
what about to enable AUD only for HEVC which doesn't have any B-frames 
in NVENC?

Because I think that we should always stay in sync between h264_nvenc 
and libx264 parameters.
Miroslav Slugeň Dec. 30, 2016, 9:07 a.m. UTC | #3
Dne 30.12.2016 v 05:48 Ali KIZIL napsal(a):
> 2016-12-30 0:02 GMT+03:00 Miroslav Slugeň <thunder.m@email.cz>:
>
>> Somebody changed AUD to active in NVENC by default, which is not very
>> clever, libx264 also has this future disabled, so we should stay in sync
>> with libx264 behavior.
>>
>> Enabled AUD will work only without B-frames. There is BUG in Nvidia NVENC
>> when you use AUD for H264 with B-frames, it will return corrupted stream,
>> because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead of
>> AUD type 7 (any-frame).
>>
>> H264 encoded with B-frames and AUD active will not play for example on
>> Panasonic TX-AS640E, other decoders just ignore wrong AUD type.
>>
>>
>> --
>> Miroslav Slugeň
>>
>>
> AUD set to active by default was for LG and Sony like UHD TVs which could
> not decode NVENC encoded HEVC MPEGTS properly.
> If patch sets AUD active by default if there is no B-Frames (as so in NVENC
> HEVC encoding), patch will be ok in common.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
I checked how libx264 and libx265 works

For libx265 default is off:

void x265_param_default(x265_param* param) {
param->bEnableAccessUnitDelimiters = 0;
}

Only for UHD BLURAY is on:

if (p->uhdBluray) {
p->bEnableAccessUnitDelimiters = 1;
}

For libx264 default is off:

void x264_param_default( x264_param_t *param ) {
param->b_aud = 0;
}

Only for BLURAY and AVC-Intra compat

if( h->param.b_bluray_compat ) {
h->param.b_aud = 1;
}

/* 200,100,50 */
if( h->param.i_avcintra_class ) {
h->param.b_aud = 1;
}

So how we should implement this to NVENC? :)

I still think we should default AUD to off and set in on only in special 
cases like BLURAY and AVC-Intra.
Ali KIZIL Dec. 30, 2016, 11 a.m. UTC | #4
2016-12-30 12:07 GMT+03:00 Miroslav Slugeň <thunder.m@email.cz>:

> Dne 30.12.2016 v 05:48 Ali KIZIL napsal(a):
>
>> 2016-12-30 0:02 GMT+03:00 Miroslav Slugeň <thunder.m@email.cz>:
>>
>>
>> Somebody changed AUD to active in NVENC by default, which is not very
>>> clever, libx264 also has this future disabled, so we should stay in sync
>>> with libx264 behavior.
>>>
>>> Enabled AUD will work only without B-frames. There is BUG in Nvidia NVENC
>>> when you use AUD for H264 with B-frames, it will return corrupted stream,
>>> because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead
>>> of
>>> AUD type 7 (any-frame).
>>>
>>> H264 encoded with B-frames and AUD active will not play for example on
>>> Panasonic TX-AS640E, other decoders just ignore wrong AUD type.
>>>
>>>
>>> --
>>> Miroslav Slugeň
>>>
>>>
>>> AUD set to active by default was for LG and Sony like UHD TVs which could
>> not decode NVENC encoded HEVC MPEGTS properly.
>> If patch sets AUD active by default if there is no B-Frames (as so in
>> NVENC
>> HEVC encoding), patch will be ok in common.
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> I checked how libx264 and libx265 works
>
> For libx265 default is off:
>
> void x265_param_default(x265_param* param) {
> param->bEnableAccessUnitDelimiters = 0;
> }
>
> Only for UHD BLURAY is on:
>
> if (p->uhdBluray) {
> p->bEnableAccessUnitDelimiters = 1;
> }
>
> For libx264 default is off:
>
> void x264_param_default( x264_param_t *param ) {
> param->b_aud = 0;
> }
>
> Only for BLURAY and AVC-Intra compat
>
> if( h->param.b_bluray_compat ) {
> h->param.b_aud = 1;
> }
>
> /* 200,100,50 */
> if( h->param.i_avcintra_class ) {
> h->param.b_aud = 1;
> }
>
> So how we should implement this to NVENC? :)
>
> I still think we should default AUD to off and set in on only in special
> cases like BLURAY and AVC-Intra.
>
>
> --
> Miroslav Slugeň
> +420 724 825 885
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

From my opinion, we can set AUD on by default for HEVC as there is no
B-Frames. When AUD is on at HEVC, I checked many TVs (Samsung, LG, Sony,
Philips etc.) all look OK.
It will be better to hear comments from others as well.
Mark Thompson Dec. 30, 2016, 2:29 p.m. UTC | #5
On 30/12/16 11:00, Ali KIZIL wrote:
> 2016-12-30 12:07 GMT+03:00 Miroslav Slugeň <thunder.m@email.cz>:
> 
>> Dne 30.12.2016 v 05:48 Ali KIZIL napsal(a):
>>
>>> 2016-12-30 0:02 GMT+03:00 Miroslav Slugeň <thunder.m@email.cz>:
>>>
>>>
>>> Somebody changed AUD to active in NVENC by default, which is not very
>>>> clever, libx264 also has this future disabled, so we should stay in sync
>>>> with libx264 behavior.
>>>>
>>>> Enabled AUD will work only without B-frames. There is BUG in Nvidia NVENC
>>>> when you use AUD for H264 with B-frames, it will return corrupted stream,
>>>> because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead
>>>> of
>>>> AUD type 7 (any-frame).
>>>>
>>>> H264 encoded with B-frames and AUD active will not play for example on
>>>> Panasonic TX-AS640E, other decoders just ignore wrong AUD type.
>>>>
>>>>
>>>> --
>>>> Miroslav Slugeň
>>>>
>>>>
>>>> AUD set to active by default was for LG and Sony like UHD TVs which could
>>> not decode NVENC encoded HEVC MPEGTS properly.
>>> If patch sets AUD active by default if there is no B-Frames (as so in
>>> NVENC
>>> HEVC encoding), patch will be ok in common.
>>>
>>> I checked how libx264 and libx265 works
>>
>> For libx265 default is off:
>>
>> void x265_param_default(x265_param* param) {
>> param->bEnableAccessUnitDelimiters = 0;
>> }
>>
>> Only for UHD BLURAY is on:
>>
>> if (p->uhdBluray) {
>> p->bEnableAccessUnitDelimiters = 1;
>> }
>>
>> For libx264 default is off:
>>
>> void x264_param_default( x264_param_t *param ) {
>> param->b_aud = 0;
>> }
>>
>> Only for BLURAY and AVC-Intra compat
>>
>> if( h->param.b_bluray_compat ) {
>> h->param.b_aud = 1;
>> }
>>
>> /* 200,100,50 */
>> if( h->param.i_avcintra_class ) {
>> h->param.b_aud = 1;
>> }
>>
>> So how we should implement this to NVENC? :)
>>
>> I still think we should default AUD to off and set in on only in special
>> cases like BLURAY and AVC-Intra.
>>
> 
> From my opinion, we can set AUD on by default for HEVC as there is no
> B-Frames. When AUD is on at HEVC, I checked many TVs (Samsung, LG, Sony,
> Philips etc.) all look OK.
> It will be better to hear comments from others as well.

Wouldn't that then revisit the same problem with some future implementation which adds support for B frames in H.265?  (I assume that's an implementation omission, not an API one.)

To my mind AUDs are a niche feature which should not be enabled by default unless specifically requested (hence the code in x26[45] enabling it only for bluray encodes, which do require them).  If some broken decoders are unable to function without them that is unfortunate, but not really ffmpeg's problem (and the workaround is known for anyone encountering it).

Therefore, I agree with the original patch.

Thanks,

- Mark
Mark Thompson Dec. 30, 2016, 2:32 p.m. UTC | #6
On 29/12/16 21:02, Miroslav Slugeň wrote:
> Somebody changed AUD to active in NVENC by default, which is not very clever, libx264 also has this future disabled, so we should stay in sync with libx264 behavior.
> 
> Enabled AUD will work only without B-frames. There is BUG in Nvidia NVENC when you use AUD for H264 with B-frames, it will return corrupted stream, because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead of AUD type 7 (any-frame).
> 
> H264 encoded with B-frames and AUD active will not play for example on Panasonic TX-AS640E, other decoders just ignore wrong AUD type.
> 
> From 8205523dfa477eaaeda3e10d59a42e024dafbfdb Mon Sep 17 00:00:00 2001
> From: Miroslav Slugen <thunder.m@email.cz>
> Date: Thu, 29 Dec 2016 21:50:13 +0100
> Subject: [PATCH 1/1] NVENC: Make AUD optional
> 
> ---
>  libavcodec/nvenc.c      | 4 ++--
>  libavcodec/nvenc.h      | 1 +
>  libavcodec/nvenc_h264.c | 1 +
>  libavcodec/nvenc_hevc.c | 1 +
>  4 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
> index f64fd8a..d57a90c 100644
> --- a/libavcodec/nvenc.c
> +++ b/libavcodec/nvenc.c
> @@ -756,7 +756,7 @@ static av_cold int nvenc_setup_h264_config(AVCodecContext *avctx)
>  
>      h264->disableSPSPPS = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 1 : 0;
>      h264->repeatSPSPPS  = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 0 : 1;
> -    h264->outputAUD     = 1;
> +    h264->outputAUD     = (ctx->aud == 1) ? 1 : 0;

Just write ctx->aud, no need for the conditional.

>  
>      if (avctx->refs >= 0) {
>          /* 0 means "let the hardware decide" */
> @@ -840,7 +840,7 @@ static av_cold int nvenc_setup_hevc_config(AVCodecContext *avctx)
>  
>      hevc->disableSPSPPS = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 1 : 0;
>      hevc->repeatSPSPPS  = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 0 : 1;
> -    hevc->outputAUD     = 1;
> +    hevc->outputAUD     = (ctx->aud == 1) ? 1 : 0;

Similarly here.

>  
>      if (avctx->refs >= 0) {
>          /* 0 means "let the hardware decide" */
> diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
> index 5bc0cba..c435e05 100644
> --- a/libavcodec/nvenc.h
> +++ b/libavcodec/nvenc.h
> @@ -153,6 +153,7 @@ typedef struct NvencContext
>      int strict_gop;
>      int aq_strength;
>      int quality;
> +    int aud;
>  } NvencContext;
>  
>  int ff_nvenc_encode_init(AVCodecContext *avctx);
> diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c
> index 71e27fd..cae27aa 100644
> --- a/libavcodec/nvenc_h264.c
> +++ b/libavcodec/nvenc_h264.c
> @@ -107,6 +107,7 @@ static const AVOption options[] = {
>                                                              OFFSET(aq_strength),  AV_OPT_TYPE_INT,   { .i64 = 8 }, 1, 15, VE },
>      { "cq",           "Set target quality level (0 to 51, 0 means automatic) for constant quality mode in VBR rate control",
>                                                              OFFSET(quality),      AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, 51, VE },
> +    { "aud",          "Use access unit delimiters",         OFFSET(aud),          AV_OPT_TYPE_BOOL,  { .i64 = 0 }, 0, 1, VE },
>      { NULL }
>  };
>  
> diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
> index ef739b6..e44ca7e 100644
> --- a/libavcodec/nvenc_hevc.c
> +++ b/libavcodec/nvenc_hevc.c
> @@ -104,6 +104,7 @@ static const AVOption options[] = {
>                                                              OFFSET(aq_strength),  AV_OPT_TYPE_INT,   { .i64 = 8 }, 1, 15, VE },
>      { "cq",           "Set target quality level (0 to 51, 0 means automatic) for constant quality mode in VBR rate control",
>                                                              OFFSET(quality),      AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, 51, VE },
> +    { "aud",          "Use access unit delimiters",         OFFSET(aud),          AV_OPT_TYPE_BOOL,  { .i64 = 0 }, 0, 1, VE },
>      { NULL }
>  };
>  
> -- 
> 2.1.4
> 

LGTM otherwise.

Thanks,

- Mark
Miroslav Slugeň Dec. 30, 2016, 4:12 p.m. UTC | #7
Dne 30.12.2016 v 15:32 Mark Thompson napsal(a):
> On 29/12/16 21:02, Miroslav Slugeň wrote:
>> Somebody changed AUD to active in NVENC by default, which is not very clever, libx264 also has this future disabled, so we should stay in sync with libx264 behavior.
>>
>> Enabled AUD will work only without B-frames. There is BUG in Nvidia NVENC when you use AUD for H264 with B-frames, it will return corrupted stream, because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead of AUD type 7 (any-frame).
>>
>> H264 encoded with B-frames and AUD active will not play for example on Panasonic TX-AS640E, other decoders just ignore wrong AUD type.
>>
>>  From 8205523dfa477eaaeda3e10d59a42e024dafbfdb Mon Sep 17 00:00:00 2001
>> From: Miroslav Slugen <thunder.m@email.cz>
>> Date: Thu, 29 Dec 2016 21:50:13 +0100
>> Subject: [PATCH 1/1] NVENC: Make AUD optional
>>
>> ---
>>   libavcodec/nvenc.c      | 4 ++--
>>   libavcodec/nvenc.h      | 1 +
>>   libavcodec/nvenc_h264.c | 1 +
>>   libavcodec/nvenc_hevc.c | 1 +
>>   4 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
>> index f64fd8a..d57a90c 100644
>> --- a/libavcodec/nvenc.c
>> +++ b/libavcodec/nvenc.c
>> @@ -756,7 +756,7 @@ static av_cold int nvenc_setup_h264_config(AVCodecContext *avctx)
>>   
>>       h264->disableSPSPPS = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 1 : 0;
>>       h264->repeatSPSPPS  = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 0 : 1;
>> -    h264->outputAUD     = 1;
>> +    h264->outputAUD     = (ctx->aud == 1) ? 1 : 0;
> Just write ctx->aud, no need for the conditional.

I understand, but in our implementation we reserved -1 for autodetection 
which could be done later based on other parameters of encoder. Should i 
then rewrite it to: h264->outputAUD = ctx->aud; now, until autodetection 
is complete?
>
>>   
>>       if (avctx->refs >= 0) {
>>           /* 0 means "let the hardware decide" */
>> @@ -840,7 +840,7 @@ static av_cold int nvenc_setup_hevc_config(AVCodecContext *avctx)
>>   
>>       hevc->disableSPSPPS = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 1 : 0;
>>       hevc->repeatSPSPPS  = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 0 : 1;
>> -    hevc->outputAUD     = 1;
>> +    hevc->outputAUD     = (ctx->aud == 1) ? 1 : 0;
> Similarly here.
>
>>   
>>       if (avctx->refs >= 0) {
>>           /* 0 means "let the hardware decide" */
>> diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
>> index 5bc0cba..c435e05 100644
>> --- a/libavcodec/nvenc.h
>> +++ b/libavcodec/nvenc.h
>> @@ -153,6 +153,7 @@ typedef struct NvencContext
>>       int strict_gop;
>>       int aq_strength;
>>       int quality;
>> +    int aud;
>>   } NvencContext;
>>   
>>   int ff_nvenc_encode_init(AVCodecContext *avctx);
>> diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c
>> index 71e27fd..cae27aa 100644
>> --- a/libavcodec/nvenc_h264.c
>> +++ b/libavcodec/nvenc_h264.c
>> @@ -107,6 +107,7 @@ static const AVOption options[] = {
>>                                                               OFFSET(aq_strength),  AV_OPT_TYPE_INT,   { .i64 = 8 }, 1, 15, VE },
>>       { "cq",           "Set target quality level (0 to 51, 0 means automatic) for constant quality mode in VBR rate control",
>>                                                               OFFSET(quality),      AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, 51, VE },
>> +    { "aud",          "Use access unit delimiters",         OFFSET(aud),          AV_OPT_TYPE_BOOL,  { .i64 = 0 }, 0, 1, VE },
>>       { NULL }
>>   };
>>   
>> diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
>> index ef739b6..e44ca7e 100644
>> --- a/libavcodec/nvenc_hevc.c
>> +++ b/libavcodec/nvenc_hevc.c
>> @@ -104,6 +104,7 @@ static const AVOption options[] = {
>>                                                               OFFSET(aq_strength),  AV_OPT_TYPE_INT,   { .i64 = 8 }, 1, 15, VE },
>>       { "cq",           "Set target quality level (0 to 51, 0 means automatic) for constant quality mode in VBR rate control",
>>                                                               OFFSET(quality),      AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, 51, VE },
>> +    { "aud",          "Use access unit delimiters",         OFFSET(aud),          AV_OPT_TYPE_BOOL,  { .i64 = 0 }, 0, 1, VE },
>>       { NULL }
>>   };
>>   
>> -- 
>> 2.1.4
>>
> LGTM otherwise.
>
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Thanks :)
Mark Thompson Dec. 30, 2016, 4:38 p.m. UTC | #8
On 30/12/16 16:12, Miroslav Slugeň wrote:
> Dne 30.12.2016 v 15:32 Mark Thompson napsal(a):
>> On 29/12/16 21:02, Miroslav Slugeň wrote:
>>> Somebody changed AUD to active in NVENC by default, which is not very clever, libx264 also has this future disabled, so we should stay in sync with libx264 behavior.
>>>
>>> Enabled AUD will work only without B-frames. There is BUG in Nvidia NVENC when you use AUD for H264 with B-frames, it will return corrupted stream, because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead of AUD type 7 (any-frame).
>>>
>>> H264 encoded with B-frames and AUD active will not play for example on Panasonic TX-AS640E, other decoders just ignore wrong AUD type.
>>>
>>>  From 8205523dfa477eaaeda3e10d59a42e024dafbfdb Mon Sep 17 00:00:00 2001
>>> From: Miroslav Slugen <thunder.m@email.cz>
>>> Date: Thu, 29 Dec 2016 21:50:13 +0100
>>> Subject: [PATCH 1/1] NVENC: Make AUD optional
>>>
>>> ---
>>>   libavcodec/nvenc.c      | 4 ++--
>>>   libavcodec/nvenc.h      | 1 +
>>>   libavcodec/nvenc_h264.c | 1 +
>>>   libavcodec/nvenc_hevc.c | 1 +
>>>   4 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
>>> index f64fd8a..d57a90c 100644
>>> --- a/libavcodec/nvenc.c
>>> +++ b/libavcodec/nvenc.c
>>> @@ -756,7 +756,7 @@ static av_cold int nvenc_setup_h264_config(AVCodecContext *avctx)
>>>         h264->disableSPSPPS = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 1 : 0;
>>>       h264->repeatSPSPPS  = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 0 : 1;
>>> -    h264->outputAUD     = 1;
>>> +    h264->outputAUD     = (ctx->aud == 1) ? 1 : 0;
>> Just write ctx->aud, no need for the conditional.
> 
> I understand, but in our implementation we reserved -1 for autodetection which could be done later based on other parameters of encoder. Should i then rewrite it to: h264->outputAUD = ctx->aud; now, until autodetection is complete?

I don't mind; if it makes a following patch look simpler then sure, leave it.

I'm curious how this autodetection can work, though.  In general you prefer never to have AUD enabled unless required (to avoid issues like your own problem this is fixing, and because it is just redundant padding which increases the size of the stream to no gain), so the cases where you want it enabled are going to be where it is an absolute requirement anyway (such as bluray), hence I see no meaningful "auto" case.

>>>         if (avctx->refs >= 0) {
>>>           /* 0 means "let the hardware decide" */
>>> @@ -840,7 +840,7 @@ static av_cold int nvenc_setup_hevc_config(AVCodecContext *avctx)
>>>         hevc->disableSPSPPS = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 1 : 0;
>>>       hevc->repeatSPSPPS  = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 0 : 1;
>>> -    hevc->outputAUD     = 1;
>>> +    hevc->outputAUD     = (ctx->aud == 1) ? 1 : 0;
>> Similarly here.
>>
>>>         if (avctx->refs >= 0) {
>>>           /* 0 means "let the hardware decide" */
>>> diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
>>> index 5bc0cba..c435e05 100644
>>> --- a/libavcodec/nvenc.h
>>> +++ b/libavcodec/nvenc.h
>>> @@ -153,6 +153,7 @@ typedef struct NvencContext
>>>       int strict_gop;
>>>       int aq_strength;
>>>       int quality;
>>> +    int aud;
>>>   } NvencContext;
>>>     int ff_nvenc_encode_init(AVCodecContext *avctx);
>>> diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c
>>> index 71e27fd..cae27aa 100644
>>> --- a/libavcodec/nvenc_h264.c
>>> +++ b/libavcodec/nvenc_h264.c
>>> @@ -107,6 +107,7 @@ static const AVOption options[] = {
>>>                                                               OFFSET(aq_strength),  AV_OPT_TYPE_INT,   { .i64 = 8 }, 1, 15, VE },
>>>       { "cq",           "Set target quality level (0 to 51, 0 means automatic) for constant quality mode in VBR rate control",
>>>                                                               OFFSET(quality),      AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, 51, VE },
>>> +    { "aud",          "Use access unit delimiters",         OFFSET(aud),          AV_OPT_TYPE_BOOL,  { .i64 = 0 }, 0, 1, VE },
>>>       { NULL }
>>>   };
>>>   diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
>>> index ef739b6..e44ca7e 100644
>>> --- a/libavcodec/nvenc_hevc.c
>>> +++ b/libavcodec/nvenc_hevc.c
>>> @@ -104,6 +104,7 @@ static const AVOption options[] = {
>>>                                                               OFFSET(aq_strength),  AV_OPT_TYPE_INT,   { .i64 = 8 }, 1, 15, VE },
>>>       { "cq",           "Set target quality level (0 to 51, 0 means automatic) for constant quality mode in VBR rate control",
>>>                                                               OFFSET(quality),      AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, 51, VE },
>>> +    { "aud",          "Use access unit delimiters",         OFFSET(aud),          AV_OPT_TYPE_BOOL,  { .i64 = 0 }, 0, 1, VE },
>>>       { NULL }
>>>   };
>>>   -- 
>>> 2.1.4
>>>
>> LGTM otherwise.
>>
>> Thanks,
>>
>> - Mark
> 
> Thanks :)
>
Yogender Gupta Jan. 5, 2017, 10:29 a.m. UTC | #9
>> There is BUG in Nvidia NVENC when you use AUD for H264 with B-frames, it will return corrupted stream, because NVIDIA is inserting AUD type 0 (I-frame) before B-frames instead of AUD type 7 (any-frame).


Thanks for bringing this to notice. We have added a fix in the Nvidia driver and the newer driver releases will insert the correct AUD type.

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.
-----------------------------------------------------------------------------------
Ali KIZIL June 13, 2017, 1:17 p.m. UTC | #10
2017-01-05 13:29 GMT+03:00 Yogender Gupta <ygupta@nvidia.com>:

> >> There is BUG in Nvidia NVENC when you use AUD for H264 with B-frames,
> it will return corrupted stream, because NVIDIA is inserting AUD type 0
> (I-frame) before B-frames instead of AUD type 7 (any-frame).
>
> Thanks for bringing this to notice. We have added a fix in the Nvidia
> driver and the newer driver releases will insert the correct AUD type.
>
> 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.
> ------------------------------------------------------------
> -----------------------
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Yogender, is new driver released for correct AUD type insertion ?
Yogender Kumar Gupta June 13, 2017, 1:36 p.m. UTC | #11
Yes, please try the latest driver version.

Thanks
Yogender

> On 13-Jun-2017, at 6:47 PM, Ali KIZIL <alikizil@gmail.com> wrote:
> 
> 2017-01-05 13:29 GMT+03:00 Yogender Gupta <ygupta@nvidia.com>:
> 
>>>> There is BUG in Nvidia NVENC when you use AUD for H264 with B-frames,
>> it will return corrupted stream, because NVIDIA is inserting AUD type 0
>> (I-frame) before B-frames instead of AUD type 7 (any-frame).
>> 
>> Thanks for bringing this to notice. We have added a fix in the Nvidia
>> driver and the newer driver releases will insert the correct AUD type.
>> 
>> 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.
>> ------------------------------------------------------------
>> -----------------------
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
> 
> Yogender, is new driver released for correct AUD type insertion ?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

From 8205523dfa477eaaeda3e10d59a42e024dafbfdb Mon Sep 17 00:00:00 2001
From: Miroslav Slugen <thunder.m@email.cz>
Date: Thu, 29 Dec 2016 21:50:13 +0100
Subject: [PATCH 1/1] NVENC: Make AUD optional

---
 libavcodec/nvenc.c      | 4 ++--
 libavcodec/nvenc.h      | 1 +
 libavcodec/nvenc_h264.c | 1 +
 libavcodec/nvenc_hevc.c | 1 +
 4 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
index f64fd8a..d57a90c 100644
--- a/libavcodec/nvenc.c
+++ b/libavcodec/nvenc.c
@@ -756,7 +756,7 @@  static av_cold int nvenc_setup_h264_config(AVCodecContext *avctx)
 
     h264->disableSPSPPS = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 1 : 0;
     h264->repeatSPSPPS  = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 0 : 1;
-    h264->outputAUD     = 1;
+    h264->outputAUD     = (ctx->aud == 1) ? 1 : 0;
 
     if (avctx->refs >= 0) {
         /* 0 means "let the hardware decide" */
@@ -840,7 +840,7 @@  static av_cold int nvenc_setup_hevc_config(AVCodecContext *avctx)
 
     hevc->disableSPSPPS = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 1 : 0;
     hevc->repeatSPSPPS  = (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) ? 0 : 1;
-    hevc->outputAUD     = 1;
+    hevc->outputAUD     = (ctx->aud == 1) ? 1 : 0;
 
     if (avctx->refs >= 0) {
         /* 0 means "let the hardware decide" */
diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
index 5bc0cba..c435e05 100644
--- a/libavcodec/nvenc.h
+++ b/libavcodec/nvenc.h
@@ -153,6 +153,7 @@  typedef struct NvencContext
     int strict_gop;
     int aq_strength;
     int quality;
+    int aud;
 } NvencContext;
 
 int ff_nvenc_encode_init(AVCodecContext *avctx);
diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c
index 71e27fd..cae27aa 100644
--- a/libavcodec/nvenc_h264.c
+++ b/libavcodec/nvenc_h264.c
@@ -107,6 +107,7 @@  static const AVOption options[] = {
                                                             OFFSET(aq_strength),  AV_OPT_TYPE_INT,   { .i64 = 8 }, 1, 15, VE },
     { "cq",           "Set target quality level (0 to 51, 0 means automatic) for constant quality mode in VBR rate control",
                                                             OFFSET(quality),      AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, 51, VE },
+    { "aud",          "Use access unit delimiters",         OFFSET(aud),          AV_OPT_TYPE_BOOL,  { .i64 = 0 }, 0, 1, VE },
     { NULL }
 };
 
diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
index ef739b6..e44ca7e 100644
--- a/libavcodec/nvenc_hevc.c
+++ b/libavcodec/nvenc_hevc.c
@@ -104,6 +104,7 @@  static const AVOption options[] = {
                                                             OFFSET(aq_strength),  AV_OPT_TYPE_INT,   { .i64 = 8 }, 1, 15, VE },
     { "cq",           "Set target quality level (0 to 51, 0 means automatic) for constant quality mode in VBR rate control",
                                                             OFFSET(quality),      AV_OPT_TYPE_INT,   { .i64 = 0 }, 0, 51, VE },
+    { "aud",          "Use access unit delimiters",         OFFSET(aud),          AV_OPT_TYPE_BOOL,  { .i64 = 0 }, 0, 1, VE },
     { NULL }
 };
 
-- 
2.1.4