[FFmpeg-devel] [PATCH] avcodec/nvenc: fix intraRefreshPeriod setting

[Timo Rothenpieler]

On 06/07/2023 14:00, Zhao Zhili wrote:
> From: Zhao Zhili <zhilizhao at 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.

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.