[FFmpeg-devel] [PATCH v3 1/3] avcodec/nvenc: Enable recovery point SEI for intra refresh mode

[Timo Rothenpieler]

On 24.01.2025 16:05, Kieran Kunhya via ffmpeg-devel wrote:
> On Fri, Jan 24, 2025 at 3:59 AM Zhao Zhili
> <quinkblack-at-foxmail.com at ffmpeg.org> wrote:
>>
>>
>>
>>> On Jan 24, 2025, at 02:02, Timo Rothenpieler <timo at rothenpieler.org> wrote:
>>>
>>> On 23.01.2025 15:17, Zhao Zhili wrote:
>>>> From: Zhao Zhili <zhilizhao at tencent.com>
>>>> Otherwise all frames can be dropped after seek without the
>>>> output_corrupt/showall flags.
>>>> ---
>>>>   libavcodec/nvenc.c | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
>>>> index 06e3fb81a4..68b1b661e4 100644
>>>> --- a/libavcodec/nvenc.c
>>>> +++ b/libavcodec/nvenc.c
>>>> @@ -1247,6 +1247,7 @@ static av_cold int nvenc_setup_h264_config(AVCodecContext *avctx)
>>>>           h264->intraRefreshPeriod = cc->gopLength;
>>>>           h264->intraRefreshCnt = cc->gopLength - 1;
>>>>           cc->gopLength = NVENC_INFINITE_GOPLENGTH;
>>>> +        h264->outputRecoveryPointSEI = 1;
>>>>   #ifdef NVENC_HAVE_SINGLE_SLICE_INTRA_REFRESH
>>>>           h264->singleSliceIntraRefresh = ctx->single_slice_intra_refresh;
>>>>   #endif
>>>> @@ -1380,6 +1381,7 @@ static av_cold int nvenc_setup_hevc_config(AVCodecContext *avctx)
>>>>           hevc->intraRefreshPeriod = cc->gopLength;
>>>>           hevc->intraRefreshCnt = cc->gopLength - 1;
>>>>           cc->gopLength = NVENC_INFINITE_GOPLENGTH;
>>>> +        hevc->outputRecoveryPointSEI = 1;
>>>>   #ifdef NVENC_HAVE_SINGLE_SLICE_INTRA_REFRESH
>>>>           hevc->singleSliceIntraRefresh = ctx->single_slice_intra_refresh;
>>>>   #endif
>>>
>>> Is there any reason someone might not want this?
>>> Or could it break certain scenarios?
>>> Reads to me like it'd only make the bitstream slightly larger, or are there other implications?
>>
>> For H.264 and H.265, GDR is intra refresh encoding together with recovery point SEI. SEI is optional for
>> decoding, but it’s critical for GDR decoding. For H.266, GDR is a NALU type, which makes this
>> technique complete.
>>
>> Without recovery point SEI, we can try decoding at any point blindly by specify -flags output_corrupt,
>> which isn’t the default behavior of our decoder. Otherwise all frames can be dropped and
>> never recovery.
>>
>> x264 don’t make option to disable recovery point SEI, which I think is the right design.
>> On the other hand, both x265 and hevc_nvenc don’t output recovery point SEI or output
>> recovery point SEI with useless fields, which makes user who want to use intra refresh
>> encoding must specify -flags output_corrupt for decoding. That is, without recovery point
>> SEI, user should have prior knowledge on encoding configuration before decoding.
>>
>> Anyway, even incorrect recovery point SEI is helpful so we don’t need to specify -flags
>> output_corrupt manually. Follow x264’s design, I don’t think save a few bytes worth the
>> trouble of disable recovery point SEI.
> 
> I don't know anything about nvenc specifically, but I think this
> decision about recovery point SEI is correct.

Sounds reasonable to me as well, so LGTM