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

[Zhao Zhili <quinkblack-at-foxmail.com@ffmpeg.org>]

> On Jan 25, 2025, at 03:08, Timo Rothenpieler <timo@rothenpieler.org> wrote:
> 
> 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@ffmpeg.org> wrote:
>>> 
>>> 
>>> 
>>>> On Jan 24, 2025, at 02:02, Timo Rothenpieler <timo@rothenpieler.org> wrote:
>>>> 
>>>> On 23.01.2025 15:17, Zhao Zhili wrote:
>>>>> From: Zhao Zhili <zhilizhao@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

Applied patch 1/3, thanks for the review.

Patch 2/3 is trial. I would greatly appreciate it if someone could help review patch 3/3. 

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".