On 27/04/18 05:10, Xiang, Haihao wrote: > On Sun, 2018-04-22 at 16:29 +0100, Mark Thompson wrote: >> Query which modes are supported and select between VBR and CBR based >> on that. Removes all of the codec-specific rate control mode selection >> code. Also ensures that target percentage and window size match the >> target bitrate and RC buffer size in all cases where they are not >> explicitly specified. >> --- >> doc/encoders.texi | 2 - >> libavcodec/vaapi_encode.c | 166 >> ++++++++++++++++++++++++++++----------- >> - >> libavcodec/vaapi_encode.h | 4 +- >> libavcodec/vaapi_encode_h264.c | 12 --- >> libavcodec/vaapi_encode_h265.c | 12 --- >> libavcodec/vaapi_encode_mjpeg.c | 3 +- >> libavcodec/vaapi_encode_mpeg2.c | 3 +- >> libavcodec/vaapi_encode_vp8.c | 11 --- >> libavcodec/vaapi_encode_vp9.c | 11 --- >> 9 files changed, 119 insertions(+), 105 deletions(-) >> >> ... >> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c >> index fc83d3db11..6188423935 100644 >> --- a/libavcodec/vaapi_encode.c >> +++ b/libavcodec/vaapi_encode.c >> ... >> vaapi_encode_config_attributes(AVCodecContext *avctx) >> static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx) >> { >> VAAPIEncodeContext *ctx = avctx->priv_data; >> - int rc_bits_per_second; >> - int rc_target_percentage; >> - int rc_window_size; >> - int hrd_buffer_size; >> - int hrd_initial_buffer_fullness; >> + int64_t rc_bits_per_second; >> + int rc_target_percentage; >> + int rc_window_size; >> + int64_t hrd_buffer_size; >> + int64_t hrd_initial_buffer_fullness; >> int fr_num, fr_den; >> + VAConfigAttrib rc_attr = { VAConfigAttribRateControl }; >> + VAStatus vas; >> + >> + vas = vaGetConfigAttributes(ctx->hwctx->display, >> + ctx->va_profile, ctx->va_entrypoint, >> + &rc_attr, 1); >> + if (vas != VA_STATUS_SUCCESS) { >> + av_log(avctx, AV_LOG_ERROR, "Failed to query rate control " >> + "config attribute: %d (%s).\n", vas, vaErrorStr(vas)); >> + return AVERROR_EXTERNAL; >> + } >> >> - if (avctx->bit_rate > INT32_MAX) { >> - av_log(avctx, AV_LOG_ERROR, "Target bitrate of 2^31 bps or " >> - "higher is not supported.\n"); >> + if (rc_attr.value == VA_ATTRIB_NOT_SUPPORTED) { >> + av_log(avctx, AV_LOG_VERBOSE, "Driver does not report any " >> + "supported rate control modes: assuming >> constant-quality.\n"); >> + ctx->va_rc_mode = VA_RC_CQP; >> + return 0; >> + } >> + if (avctx->flags & AV_CODEC_FLAG_QSCALE || >> + avctx->bit_rate <= 0) { >> + if (rc_attr.value & VA_RC_CQP) { >> + av_log(avctx, AV_LOG_VERBOSE, "Using constant-quality mode.\n"); >> + ctx->va_rc_mode = VA_RC_CQP; >> + return 0; >> + } else { >> + av_log(avctx, AV_LOG_ERROR, "Driver does not support " >> + "constant-quality mode (%#x).\n", rc_attr.value); >> + return AVERROR(EINVAL); >> + } >> + } >> + >> + if (!(rc_attr.value & (VA_RC_CBR | VA_RC_VBR))) { >> + av_log(avctx, AV_LOG_ERROR, "Driver does not support any " >> + "bitrate-targetted rate control modes.\n"); >> return AVERROR(EINVAL); >> } >> >> @@ -1320,27 +1323,90 @@ static av_cold int >> vaapi_encode_init_rate_control(AVCodecContext *avctx) >> hrd_buffer_size = avctx->rc_buffer_size; >> else >> hrd_buffer_size = avctx->bit_rate; >> - if (avctx->rc_initial_buffer_occupancy) >> + if (avctx->rc_initial_buffer_occupancy) { >> + if (avctx->rc_initial_buffer_occupancy > hrd_buffer_size) { >> + av_log(avctx, AV_LOG_ERROR, "Invalid RC buffer settings: " >> + "must have initial buffer size (%d) < " >> + "buffer size (%"PRId64").\n", >> + avctx->rc_initial_buffer_occupancy, hrd_buffer_size); >> + return AVERROR(EINVAL); >> + } >> hrd_initial_buffer_fullness = avctx->rc_initial_buffer_occupancy; >> - else >> + } else { >> hrd_initial_buffer_fullness = hrd_buffer_size * 3 / 4; >> + } >> + >> + if (avctx->rc_max_rate && avctx->rc_max_rate < avctx->bit_rate) { >> + av_log(avctx, AV_LOG_ERROR, "Invalid bitrate settings: must have " >> + "bitrate (%"PRId64") <= maxrate (%"PRId64").\n", >> + avctx->bit_rate, avctx->rc_max_rate); >> + return AVERROR(EINVAL); >> + } >> + >> + if (avctx->rc_max_rate > avctx->bit_rate) { >> + if (!(rc_attr.value & VA_RC_VBR)) { >> + av_log(avctx, AV_LOG_WARNING, "Driver does not support " >> + "VBR mode (%#x), using CBR mode instead.\n", >> + rc_attr.value); >> + ctx->va_rc_mode = VA_RC_CBR; >> + } else { >> + ctx->va_rc_mode = VA_RC_VBR; >> + } >> + >> + rc_bits_per_second = avctx->rc_max_rate; >> + rc_target_percentage = (avctx->bit_rate * 100) / avctx->rc_max_rate; >> + >> + } else if (avctx->rc_max_rate == avctx->bit_rate) { >> + if (!(rc_attr.value & VA_RC_CBR)) { >> + av_log(avctx, AV_LOG_WARNING, "Driver does not support " >> + "CBR mode (%#x), using VBR mode instead.\n", >> + rc_attr.value); >> + ctx->va_rc_mode = VA_RC_VBR; >> + } else { >> + ctx->va_rc_mode = VA_RC_CBR; >> + } >> >> - 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. >> + if (rc_attr.value & VA_RC_VBR) { >> + ctx->va_rc_mode = VA_RC_VBR; >> + >> + rc_bits_per_second = avctx->bit_rate + hrd_buffer_size; > > Why hrd_buffer_size is added? User may set a large hrd buffer size.
Given a buffer size and a target bitrate, the maximum bitrate in a second is achieved by completely filling an empty buffer and also using all the bits available from that second. >> + rc_target_percentage = (avctx->bit_rate * 100) / >> rc_bits_per_second; >> + } else { >> + ctx->va_rc_mode = VA_RC_CBR; >> + >> rc_bits_per_second = avctx->bit_rate; >> rc_target_percentage = 100; >> - } else { >> - 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 = ((int64_t)hrd_buffer_size * 100 * 1000) / >> + (rc_target_percentage * rc_bits_per_second); > > I think it should be (hrd_buffer_size * 1000) / rc_bits_per_second. Could you > explain why is it multiplied by (100 / rc_target_percentage) here? The target bitrate (avctx->bit_rate == rc_bits_per_second * rc_target_percentage / 100) is the fill-rate of the buffer, so that's what we need to divide the buffer size by to find how long it takes to fill and therefore the effective window size. >> + >> + av_log(avctx, AV_LOG_VERBOSE, "RC mode: %s, %d%% of %"PRId64" bps " >> + "over %d ms.\n", ctx->va_rc_mode == VA_RC_VBR ? "VBR" : "CBR", >> + rc_target_percentage, rc_bits_per_second, rc_window_size); >> + av_log(avctx, AV_LOG_VERBOSE, "RC buffer: %"PRId64" bits, " >> + "initial fullness %"PRId64" bits.\n", >> + hrd_buffer_size, hrd_initial_buffer_fullness); >> + >> + if (rc_bits_per_second > UINT32_MAX || >> + hrd_buffer_size > UINT32_MAX || >> + hrd_initial_buffer_fullness > UINT32_MAX) { >> + av_log(avctx, AV_LOG_ERROR, "RC parameters of 2^32 or " >> + "greater are not supported by VAAPI.\n"); >> + return AVERROR(EINVAL); >> + } >> + >> + ctx->config_attributes[ctx->nb_config_attributes++] = >> + (VAConfigAttrib) { >> + .type = VAConfigAttribRateControl, >> + .value = ctx->va_rc_mode, >> + }; >> + >> ctx->rc_params.misc.type = VAEncMiscParameterTypeRateControl; >> ctx->rc_params.rc = (VAEncMiscParameterRateControl) { >> .bits_per_second = rc_bits_per_second, Thanks, - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel