On 31/10/17 16:00, Jun Zhao wrote: > On 2017/10/31 23:45, Mark Thompson wrote: >> 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.z...@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.z...@intel.com> >>>>> Signed-off-by: Wang, Yi A <yi.a.w...@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. > I think the code logic have considered the explicit rc_buffer_size > setting, and this patch > correct the HRD buffer size with MAX(rc_max_rate, bit_rate) when no > setting rc_buffer_size in > VBR mode.
I agree that this is what your patch does. > In the old logic, in VBR mode, if user don't setting > rc_buffer_size > explicitly, the HRD buffer size always use avctx->bit_rate, it does not > make sense. Why not? - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel