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

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

[Zhao Zhili]

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
-- 
2.46.0

_______________________________________________
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".

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

[Zhao Zhili]

> 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".

Re: [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@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
_______________________________________________
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".

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

[Kieran Kunhya via ffmpeg-devel]

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.

Kieran
_______________________________________________
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".

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

[Zhao Zhili]

> 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.

> _______________________________________________
> 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".

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

[Timo Rothenpieler]

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?
_______________________________________________
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] [PATCH v3 1/3] avcodec/nvenc: Enable recovery point SEI for intra refresh mode

[Zhao Zhili]

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
-- 
2.46.0


_______________________________________________
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".