[FFmpeg-devel,13/24] vaapi_encode: Use gop_size consistently in RC parameters

The non-H.26[45] codecs already use this form.  Since we don't
currently generate I frames for codecs which support them separately
to IDR, the p_per_i variable is set to infinity by default so that it
doesn't interfere with any other calculation.  (All the code for I
frames still exists, and it works for H.264 if set manually.)

(cherry picked from commit 6af014f4028238b4c50f1731b3369a41d65fa9c4)
---
 libavcodec/vaapi_encode.c      | 3 +--
 libavcodec/vaapi_encode_h264.c | 4 ++--
 libavcodec/vaapi_encode_h265.c | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

On 2017/6/13 6:40, Mark Thompson wrote:
> The non-H.26[45] codecs already use this form.  Since we don't
> currently generate I frames for codecs which support them separately
> to IDR, the p_per_i variable is set to infinity by default so that it
> doesn't interfere with any other calculation.  (All the code for I
> frames still exists, and it works for H.264 if set manually.)
> 
> (cherry picked from commit 6af014f4028238b4c50f1731b3369a41d65fa9c4)
> ---
>  libavcodec/vaapi_encode.c      | 3 +--
>  libavcodec/vaapi_encode_h264.c | 4 ++--
>  libavcodec/vaapi_encode_h265.c | 4 ++--
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> index 7aaf263d25..2de5f76cab 100644
> --- a/libavcodec/vaapi_encode.c
> +++ b/libavcodec/vaapi_encode.c
> @@ -1435,8 +1435,7 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>      ctx->output_order = - ctx->output_delay - 1;
>  
>      // Currently we never generate I frames, only IDR.
> -    ctx->p_per_i = ((avctx->gop_size - 1 + avctx->max_b_frames) /
> -                    (avctx->max_b_frames + 1));
> +    ctx->p_per_i = INT_MAX;

Why don't remove p_per_i if don't use this field?

>      ctx->b_per_p = avctx->max_b_frames;
>  
>      if (ctx->codec->sequence_params_size > 0) {
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index 92e29554ed..f9fcd805a4 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -905,8 +905,8 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>              mseq->nal_hrd_parameters_present_flag = 0;
>          }
>  
> -        vseq->intra_period     = ctx->p_per_i * (ctx->b_per_p + 1);
> -        vseq->intra_idr_period = vseq->intra_period;
> +        vseq->intra_period     = avctx->gop_size;
> +        vseq->intra_idr_period = avctx->gop_size;
>          vseq->ip_period        = ctx->b_per_p + 1;
>      }
>  
> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
> index 6e008b7b9c..1d648a6d87 100644
> --- a/libavcodec/vaapi_encode_h265.c
> +++ b/libavcodec/vaapi_encode_h265.c
> @@ -832,8 +832,8 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>              vseq->vui_time_scale        = avctx->time_base.den;
>          }
>  
> -        vseq->intra_period     = ctx->p_per_i * (ctx->b_per_p + 1);
> -        vseq->intra_idr_period = vseq->intra_period;
> +        vseq->intra_period     = avctx->gop_size;
> +        vseq->intra_idr_period = avctx->gop_size;
>          vseq->ip_period        = ctx->b_per_p + 1;
>      }
>  
>

On 14/06/17 04:12, Jun Zhao wrote:
> On 2017/6/13 6:40, Mark Thompson wrote:
>> The non-H.26[45] codecs already use this form.  Since we don't
>> currently generate I frames for codecs which support them separately
>> to IDR, the p_per_i variable is set to infinity by default so that it
>> doesn't interfere with any other calculation.  (All the code for I
>> frames still exists, and it works for H.264 if set manually.)
>>
>> (cherry picked from commit 6af014f4028238b4c50f1731b3369a41d65fa9c4)
>> ---
>>  libavcodec/vaapi_encode.c      | 3 +--
>>  libavcodec/vaapi_encode_h264.c | 4 ++--
>>  libavcodec/vaapi_encode_h265.c | 4 ++--
>>  3 files changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>> index 7aaf263d25..2de5f76cab 100644
>> --- a/libavcodec/vaapi_encode.c
>> +++ b/libavcodec/vaapi_encode.c
>> @@ -1435,8 +1435,7 @@ av_cold int ff_vaapi_encode_init(AVCodecContext *avctx)
>>      ctx->output_order = - ctx->output_delay - 1;
>>  
>>      // Currently we never generate I frames, only IDR.
>> -    ctx->p_per_i = ((avctx->gop_size - 1 + avctx->max_b_frames) /
>> -                    (avctx->max_b_frames + 1));
>> +    ctx->p_per_i = INT_MAX;
> 
> Why don't remove p_per_i if don't use this field?

It's useful for testing the I frame support in H.264, which works but is currently inaccessible to the user.  I have vague plans to make it user controllable somehow, but I'm not yet sure how.  (There is <https://lists.libav.org/pipermail/libav-devel/2017-May/083691.html> oustanding to generate SEI recovery points so that I frames in open-GOP style are sensibly usable in H.264, but no options to actually enable it yet.)

>>      ctx->b_per_p = avctx->max_b_frames;
>>  
>>      if (ctx->codec->sequence_params_size > 0) {
>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
>> index 92e29554ed..f9fcd805a4 100644
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -905,8 +905,8 @@ static int vaapi_encode_h264_init_sequence_params(AVCodecContext *avctx)
>>              mseq->nal_hrd_parameters_present_flag = 0;
>>          }
>>  
>> -        vseq->intra_period     = ctx->p_per_i * (ctx->b_per_p + 1);
>> -        vseq->intra_idr_period = vseq->intra_period;
>> +        vseq->intra_period     = avctx->gop_size;
>> +        vseq->intra_idr_period = avctx->gop_size;
>>          vseq->ip_period        = ctx->b_per_p + 1;
>>      }
>>  
>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>> index 6e008b7b9c..1d648a6d87 100644
>> --- a/libavcodec/vaapi_encode_h265.c
>> +++ b/libavcodec/vaapi_encode_h265.c
>> @@ -832,8 +832,8 @@ static int vaapi_encode_h265_init_sequence_params(AVCodecContext *avctx)
>>              vseq->vui_time_scale        = avctx->time_base.den;
>>          }
>>  
>> -        vseq->intra_period     = ctx->p_per_i * (ctx->b_per_p + 1);
>> -        vseq->intra_idr_period = vseq->intra_period;
>> +        vseq->intra_period     = avctx->gop_size;
>> +        vseq->intra_idr_period = avctx->gop_size;
>>          vseq->ip_period        = ctx->b_per_p + 1;
>>      }
>>  
>>