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

[Mark Thompson]

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 at 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 :)
>