Message ID | 2a2f2a9e-9761-4aa5-acee-3b70b4cbb51d@gmail.com |
---|---|
State | New |
Headers | show |
On 31/10/17 02:37, Jun Zhao wrote: > From 7eef9be1c8a92bf625d62a0f97f762f1342c6d78 Mon Sep 17 00:00:00 2001 > From: Jun Zhao <jun.zhao@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@intel.com> > Signed-off-by: Wang, Yi A <yi.a.wang@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? - Mark
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@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@intel.com> >> Signed-off-by: Wang, Yi A <yi.a.wang@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. > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
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@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@intel.com> >>> Signed-off-by: Wang, Yi A <yi.a.wang@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
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.zhao@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@intel.com> >>>> Signed-off-by: Wang, Yi A <yi.a.wang@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. 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. > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
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.zhao@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@intel.com> >>>>> Signed-off-by: Wang, Yi A <yi.a.wang@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
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) {