[FFmpeg-devel] [PATCH 1/2] lavc/vaapi_encode: correct the HRD buffer size calculate.

[Mark Thompson]

On 31/10/17 15:11, Jun Zhao wrote:> On 2017/10/31 18:19, Mark Thompson wrote:
>> On 31/10/17 02:37, Jun Zhao wrote:
>>> From 7eef9be1c8a92bf625d62a0f97f762f1342c6d78 Mon Sep 17 00:00:00 2001
>>> From: Jun Zhao <jun.zhao at intel.com>
>>> Date: Tue, 31 Oct 2017 10:13:42 +0800
>>> Subject: [PATCH 1/2] lavc/vaapi_encode: correct the HRD buffer size calculate.
>>>
>>> when rc_buffer_size didn't setting, always use the max bit rate
>>> per second as HRD buffer size.
>>>
>>> Signed-off-by: Jun Zhao <jun.zhao at intel.com>
>>> Signed-off-by: Wang, Yi A <yi.a.wang at intel.com>
>>> ---
>>>  libavcodec/vaapi_encode.c | 21 ++++++++++-----------
>>>  1 file changed, 10 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
>>> index 590f4be4ed..d5f89ef346 100644
>>> --- a/libavcodec/vaapi_encode.c
>>> +++ b/libavcodec/vaapi_encode.c
>>> @@ -1144,19 +1144,9 @@ static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx)
>>>          return AVERROR(EINVAL);
>>>      }
>>>  
>>> -    if (avctx->rc_buffer_size)
>>> -        hrd_buffer_size = avctx->rc_buffer_size;
>>> -    else
>>> -        hrd_buffer_size = avctx->bit_rate;
>>> -    if (avctx->rc_initial_buffer_occupancy)
>>> -        hrd_initial_buffer_fullness = avctx->rc_initial_buffer_occupancy;
>>> -    else
>>> -        hrd_initial_buffer_fullness = hrd_buffer_size * 3 / 4;
>>> -
>>>      if (ctx->va_rc_mode == VA_RC_CBR) {
>>>          rc_bits_per_second   = avctx->bit_rate;
>>>          rc_target_percentage = 100;
>>> -        rc_window_size       = 1000;
>>>      } else {
>>>          if (avctx->rc_max_rate < avctx->bit_rate) {
>>>              // Max rate is unset or invalid, just use the normal bitrate.
>>> @@ -1166,8 +1156,17 @@ static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx)
>>>              rc_bits_per_second   = avctx->rc_max_rate;
>>>              rc_target_percentage = (avctx->bit_rate * 100) / rc_bits_per_second;
>>>          }
>>> -        rc_window_size = (hrd_buffer_size * 1000) / avctx->bit_rate;
>>>      }
>>> +    rc_window_size = (rc_bits_per_second * 1000) / avctx->bit_rate;
>>> +
>>> +    if (avctx->rc_buffer_size)
>>> +        hrd_buffer_size = avctx->rc_buffer_size;
>>> +    else
>>> +        hrd_buffer_size = rc_bits_per_second;
>>> +    if (avctx->rc_initial_buffer_occupancy)
>>> +        hrd_initial_buffer_fullness = avctx->rc_initial_buffer_occupancy;
>>> +    else
>>> +        hrd_initial_buffer_fullness = hrd_buffer_size * 3 / 4;
>>>  
>>>      ctx->rc_params.misc.type = VAEncMiscParameterTypeRateControl;
>>>      ctx->rc_params.rc = (VAEncMiscParameterRateControl) {
>>> -- 
>>> 2.14.1
>>>
>> Why?  If the RC buffer size is unset it currently guesses one second of the target bitrate - in what way is the peak bitrate any more appropriate as a guess?
> In VBR mode, when rc_max_rate more than bit_rate, I think HDR size need
> to use rc_max_rate, not the bit_rate.

Ok, why do you think that?  I'm not necessarily against this change (in cases where it matters the user will provide rc_buffer_size explicitly), but please provide some reasoning for it.

- Mark