> On Jul 6, 2023, at 18:23, Timo Rothenpieler <t...@rothenpieler.org> wrote:
>
> On 06/07/2023 14:00, Zhao Zhili wrote:
>> From: Zhao Zhili <zhiliz...@tencent.com>
>> Regression since 99dfdb45. intraRefreshPeriod access cc->gopLength,
>> which has been overwritten to NVENC_INFINITE_GOPLENGTH before.
>> Fixes #10445.
>> ---
>> libavcodec/nvenc.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> diff --git a/libavcodec/nvenc.c b/libavcodec/nvenc.c
>> index 06579a502b..13fafcd246 100644
>> --- a/libavcodec/nvenc.c
>> +++ b/libavcodec/nvenc.c
>> @@ -1173,6 +1173,7 @@ static av_cold int
>> nvenc_setup_h264_config(AVCodecContext *avctx)
>> h264->enableIntraRefresh = 1;
>> h264->intraRefreshPeriod = cc->gopLength;
>> h264->intraRefreshCnt = cc->gopLength - 1;
>> + cc->gopLength = NVENC_INFINITE_GOPLENGTH;
>> #ifdef NVENC_HAVE_SINGLE_SLICE_INTRA_REFRESH
>> h264->singleSliceIntraRefresh = ctx->single_slice_intra_refresh;
>> #endif
>> @@ -1297,6 +1298,7 @@ static av_cold int
>> nvenc_setup_hevc_config(AVCodecContext *avctx)
>> hevc->enableIntraRefresh = 1;
>> hevc->intraRefreshPeriod = cc->gopLength;
>> hevc->intraRefreshCnt = cc->gopLength - 1;
>> + cc->gopLength = NVENC_INFINITE_GOPLENGTH;
>> #ifdef NVENC_HAVE_SINGLE_SLICE_INTRA_REFRESH
>> hevc->singleSliceIntraRefresh = ctx->single_slice_intra_refresh;
>> #endif
>> @@ -1415,6 +1417,7 @@ static av_cold int
>> nvenc_setup_av1_config(AVCodecContext *avctx)
>> av1->enableIntraRefresh = 1;
>> av1->intraRefreshPeriod = cc->gopLength;
>> av1->intraRefreshCnt = cc->gopLength - 1;
>> + cc->gopLength = NVENC_INFINITE_GOPLENGTH;
>> av1->idrPeriod = NVENC_INFINITE_GOPLENGTH;
>> } else if (cc->gopLength > 0) {
>> @@ -1619,9 +1622,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
>> if(ctx->single_slice_intra_refresh)
>> ctx->intra_refresh = 1;
>> - if (ctx->intra_refresh)
>> - ctx->encode_config.gopLength = NVENC_INFINITE_GOPLENGTH;
>> -
>> nvenc_recalc_surfaces(avctx);
>> nvenc_setup_rate_control(avctx);
>
> Shouldn't it be enough to move this block down a bit, below
> nvenc_setup_codec_config?
> That way the codec specific configs can still access the value, and the logic
> of setting it to infinite for intra refresh mode doesn't have to be
> duplicated everywhere.
Then people lost the context and hard to get why it should be put after
nvenc_setup_codec_config(), and might move it before by accident.
Another benefit to set cc->gopLength at here is idrPeriod can be set
to gopLength unconditionally:
http://ffmpeg.org/pipermail/ffmpeg-devel/2023-July/311694.html
To reduce code duplication, the codec specific intra_refresh configure
can be replaced by a macro, but it doesn’t looks good IMO.
>
> Though I'm not sure which way I prefer. Having it in the intra_refresh
> specific block in each codec definitely also has its advantage of keeping
> that logic in one place.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
