Message ID | 20240223071442.170041-1-fei.w.wang@intel.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,v2] lavc/vaapi_encode: Enable Macroblock based bitrate control | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
On 23/02/2024 07:14, fei.w.wang-at-intel.com@ffmpeg.org wrote: > From: Fei Wang <fei.w.wang@intel.com> > > Signed-off-by: Fei Wang <fei.w.wang@intel.com> > --- > update: > Print mbbrc status with "ON/OFF" instead of "0/1". > > doc/encoders.texi | 3 +++ > libavcodec/vaapi_encode.c | 11 ++++++++++- > libavcodec/vaapi_encode.h | 9 ++++++++- > 3 files changed, 21 insertions(+), 2 deletions(-) Can you explain what the option actually does? The current documentation doesn't really give any hint of whether a user would want it on or off. It also seems like this is a legacy name and maybe should have a more generic name to make sense - the iHD driver at least accepts the option for H.265 and VP9, neither of which have macroblocks. > diff --git a/doc/encoders.texi b/doc/encoders.texi > index 9f477d7c53..3f11a80039 100644 > --- a/doc/encoders.texi > +++ b/doc/encoders.texi > @@ -4086,6 +4086,9 @@ Quality-defined variable-bitrate. > Average variable bitrate. > @end table > > +@item mbbrc > +Enable Macroblock based rate control. Invalid for CQP mode. > + > @end table > > Each encoder also has its own specific options: > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > index 86f4110cd2..e4bf15fea0 100644 > --- a/libavcodec/vaapi_encode.c > +++ b/libavcodec/vaapi_encode.c > @@ -1806,6 +1806,9 @@ static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx) > int i, first = 1, res; > > supported_va_rc_modes = rc_attr.value; > + if (ctx->mbbrc && !(supported_va_rc_modes & VA_RC_MB)) > + ctx->mbbrc = 0; Probably not a good idea to silently disable what the user explicitly requested. > + > for (i = 0; i < FF_ARRAY_ELEMS(vaapi_encode_rc_modes); i++) { > rc_mode = &vaapi_encode_rc_modes[i]; > if (supported_va_rc_modes & rc_mode->va_mode) { > @@ -2017,13 +2020,18 @@ rc_mode_found: > ctx->va_bit_rate = rc_bits_per_second; > > av_log(avctx, AV_LOG_VERBOSE, "RC mode: %s.\n", rc_mode->name); > + > + if (ctx->mbbrc && ctx->va_rc_mode == VA_RC_CQP) > + ctx->mbbrc = 0; > + av_log(avctx, AV_LOG_VERBOSE, "MB bitrate control: %s.\n", ctx->mbbrc ? "ON" : "OFF"); > + > if (rc_attr.value == VA_ATTRIB_NOT_SUPPORTED) { > // This driver does not want the RC mode attribute to be set. > } else { > ctx->config_attributes[ctx->nb_config_attributes++] = > (VAConfigAttrib) { > .type = VAConfigAttribRateControl, > - .value = ctx->va_rc_mode, > + .value = ctx->mbbrc ? ctx->va_rc_mode | VA_RC_MB : ctx->va_rc_mode, > }; > } > > @@ -2052,6 +2060,7 @@ rc_mode_found: > #if VA_CHECK_VERSION(1, 1, 0) > .ICQ_quality_factor = av_clip(rc_quality, 1, 51), > .max_qp = (avctx->qmax > 0 ? avctx->qmax : 0), > + .rc_flags.bits.mb_rate_control = ctx->mbbrc ? 1 : 2, What did the previous 0 "use default" do here? > #endif > #if VA_CHECK_VERSION(1, 3, 0) > .quality_factor = rc_quality, > diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h > index 6964055b93..9774cc86b1 100644 > --- a/libavcodec/vaapi_encode.h > +++ b/libavcodec/vaapi_encode.h > @@ -216,6 +216,9 @@ typedef struct VAAPIEncodeContext { > // available modes). > int explicit_rc_mode; > > + // Macroblock based rate control. > + int mbbrc; > + > // Explicitly-set QP, for use with the "qp" options. > // (Forces CQP mode when set, overriding everything else.) > int explicit_qp; > @@ -538,7 +541,11 @@ int ff_vaapi_encode_close(AVCodecContext *avctx); > VAAPI_ENCODE_RC_MODE(VBR, "Variable-bitrate"), \ > VAAPI_ENCODE_RC_MODE(ICQ, "Intelligent constant-quality"), \ > VAAPI_ENCODE_RC_MODE(QVBR, "Quality-defined variable-bitrate"), \ > - VAAPI_ENCODE_RC_MODE(AVBR, "Average variable-bitrate") > + VAAPI_ENCODE_RC_MODE(AVBR, "Average variable-bitrate"), \ > + { "mbbrc", \ > + "Macroblock based bitrate control",\ > + OFFSET(common.mbbrc), AV_OPT_TYPE_BOOL, \ > + { .i64 = 0 }, 0, 1, FLAGS } > > > #endif /* AVCODEC_VAAPI_ENCODE_H */ Thanks, - Mark
On Sun, 2024-02-25 at 19:15 +0000, Mark Thompson wrote: > On 23/02/2024 07:14, fei.w.wang-at-intel.com@ffmpeg.org wrote: > > From: Fei Wang <fei.w.wang@intel.com> > > > > Signed-off-by: Fei Wang <fei.w.wang@intel.com> > > --- > > update: > > Print mbbrc status with "ON/OFF" instead of "0/1". > > > > doc/encoders.texi | 3 +++ > > libavcodec/vaapi_encode.c | 11 ++++++++++- > > libavcodec/vaapi_encode.h | 9 ++++++++- > > 3 files changed, 21 insertions(+), 2 deletions(-) > > Can you explain what the option actually does? The current > documentation doesn't really give any hint of whether a user would > want it on or off. > > It also seems like this is a legacy name and maybe should have a more > generic name to make sense - the iHD driver at least accepts the > option for H.265 and VP9, neither of which have macroblocks. How about BLBRC(Block level bitrate control)? > > > diff --git a/doc/encoders.texi b/doc/encoders.texi > > index 9f477d7c53..3f11a80039 100644 > > --- a/doc/encoders.texi > > +++ b/doc/encoders.texi > > @@ -4086,6 +4086,9 @@ Quality-defined variable-bitrate. > > Average variable bitrate. > > @end table > > > > +@item mbbrc > > +Enable Macroblock based rate control. Invalid for CQP mode. > > + > > @end table > > > > Each encoder also has its own specific options: > > diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c > > index 86f4110cd2..e4bf15fea0 100644 > > --- a/libavcodec/vaapi_encode.c > > +++ b/libavcodec/vaapi_encode.c > > @@ -1806,6 +1806,9 @@ static av_cold int > > vaapi_encode_init_rate_control(AVCodecContext *avctx) > > int i, first = 1, res; > > > > supported_va_rc_modes = rc_attr.value; > > + if (ctx->mbbrc && !(supported_va_rc_modes & VA_RC_MB)) > > + ctx->mbbrc = 0; > > Probably not a good idea to silently disable what the user explicitly > requested. > > > + > > for (i = 0; i < FF_ARRAY_ELEMS(vaapi_encode_rc_modes); > > i++) { > > rc_mode = &vaapi_encode_rc_modes[i]; > > if (supported_va_rc_modes & rc_mode->va_mode) { > > @@ -2017,13 +2020,18 @@ rc_mode_found: > > ctx->va_bit_rate = rc_bits_per_second; > > > > av_log(avctx, AV_LOG_VERBOSE, "RC mode: %s.\n", rc_mode- > > >name); > > + > > + if (ctx->mbbrc && ctx->va_rc_mode == VA_RC_CQP) > > + ctx->mbbrc = 0; > > + av_log(avctx, AV_LOG_VERBOSE, "MB bitrate control: %s.\n", > > ctx->mbbrc ? "ON" : "OFF"); > > + > > if (rc_attr.value == VA_ATTRIB_NOT_SUPPORTED) { > > // This driver does not want the RC mode attribute to be > > set. > > } else { > > ctx->config_attributes[ctx->nb_config_attributes++] = > > (VAConfigAttrib) { > > .type = VAConfigAttribRateControl, > > - .value = ctx->va_rc_mode, > > + .value = ctx->mbbrc ? ctx->va_rc_mode | VA_RC_MB : > > ctx->va_rc_mode, > > }; > > } > > > > @@ -2052,6 +2060,7 @@ rc_mode_found: > > #if VA_CHECK_VERSION(1, 1, 0) > > .ICQ_quality_factor = av_clip(rc_quality, 1, 51), > > .max_qp = (avctx->qmax > 0 ? avctx->qmax > > : 0), > > + .rc_flags.bits.mb_rate_control = ctx->mbbrc ? 1 : 2, > > What did the previous 0 "use default" do here? 0 means auto decide by driver. This feature may has impact on frame psnr, so set it as off by default and expect user enable it explicitly. Thanks Fei > > > #endif > > #if VA_CHECK_VERSION(1, 3, 0) > > .quality_factor = rc_quality, > > diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h > > index 6964055b93..9774cc86b1 100644 > > --- a/libavcodec/vaapi_encode.h > > +++ b/libavcodec/vaapi_encode.h > > @@ -216,6 +216,9 @@ typedef struct VAAPIEncodeContext { > > // available modes). > > int explicit_rc_mode; > > > > + // Macroblock based rate control. > > + int mbbrc; > > + > > // Explicitly-set QP, for use with the "qp" options. > > // (Forces CQP mode when set, overriding everything else.) > > int explicit_qp; > > @@ -538,7 +541,11 @@ int ff_vaapi_encode_close(AVCodecContext > > *avctx); > > VAAPI_ENCODE_RC_MODE(VBR, "Variable-bitrate"), \ > > VAAPI_ENCODE_RC_MODE(ICQ, "Intelligent constant-quality"), \ > > VAAPI_ENCODE_RC_MODE(QVBR, "Quality-defined variable- > > bitrate"), \ > > - VAAPI_ENCODE_RC_MODE(AVBR, "Average variable-bitrate") > > + VAAPI_ENCODE_RC_MODE(AVBR, "Average variable-bitrate"), \ > > + { "mbbrc", \ > > + "Macroblock based bitrate control",\ > > + OFFSET(common.mbbrc), AV_OPT_TYPE_BOOL, \ > > + { .i64 = 0 }, 0, 1, FLAGS } > > > > > > #endif /* AVCODEC_VAAPI_ENCODE_H */ > > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/doc/encoders.texi b/doc/encoders.texi index 9f477d7c53..3f11a80039 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -4086,6 +4086,9 @@ Quality-defined variable-bitrate. Average variable bitrate. @end table +@item mbbrc +Enable Macroblock based rate control. Invalid for CQP mode. + @end table Each encoder also has its own specific options: diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c index 86f4110cd2..e4bf15fea0 100644 --- a/libavcodec/vaapi_encode.c +++ b/libavcodec/vaapi_encode.c @@ -1806,6 +1806,9 @@ static av_cold int vaapi_encode_init_rate_control(AVCodecContext *avctx) int i, first = 1, res; supported_va_rc_modes = rc_attr.value; + if (ctx->mbbrc && !(supported_va_rc_modes & VA_RC_MB)) + ctx->mbbrc = 0; + for (i = 0; i < FF_ARRAY_ELEMS(vaapi_encode_rc_modes); i++) { rc_mode = &vaapi_encode_rc_modes[i]; if (supported_va_rc_modes & rc_mode->va_mode) { @@ -2017,13 +2020,18 @@ rc_mode_found: ctx->va_bit_rate = rc_bits_per_second; av_log(avctx, AV_LOG_VERBOSE, "RC mode: %s.\n", rc_mode->name); + + if (ctx->mbbrc && ctx->va_rc_mode == VA_RC_CQP) + ctx->mbbrc = 0; + av_log(avctx, AV_LOG_VERBOSE, "MB bitrate control: %s.\n", ctx->mbbrc ? "ON" : "OFF"); + if (rc_attr.value == VA_ATTRIB_NOT_SUPPORTED) { // This driver does not want the RC mode attribute to be set. } else { ctx->config_attributes[ctx->nb_config_attributes++] = (VAConfigAttrib) { .type = VAConfigAttribRateControl, - .value = ctx->va_rc_mode, + .value = ctx->mbbrc ? ctx->va_rc_mode | VA_RC_MB : ctx->va_rc_mode, }; } @@ -2052,6 +2060,7 @@ rc_mode_found: #if VA_CHECK_VERSION(1, 1, 0) .ICQ_quality_factor = av_clip(rc_quality, 1, 51), .max_qp = (avctx->qmax > 0 ? avctx->qmax : 0), + .rc_flags.bits.mb_rate_control = ctx->mbbrc ? 1 : 2, #endif #if VA_CHECK_VERSION(1, 3, 0) .quality_factor = rc_quality, diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h index 6964055b93..9774cc86b1 100644 --- a/libavcodec/vaapi_encode.h +++ b/libavcodec/vaapi_encode.h @@ -216,6 +216,9 @@ typedef struct VAAPIEncodeContext { // available modes). int explicit_rc_mode; + // Macroblock based rate control. + int mbbrc; + // Explicitly-set QP, for use with the "qp" options. // (Forces CQP mode when set, overriding everything else.) int explicit_qp; @@ -538,7 +541,11 @@ int ff_vaapi_encode_close(AVCodecContext *avctx); VAAPI_ENCODE_RC_MODE(VBR, "Variable-bitrate"), \ VAAPI_ENCODE_RC_MODE(ICQ, "Intelligent constant-quality"), \ VAAPI_ENCODE_RC_MODE(QVBR, "Quality-defined variable-bitrate"), \ - VAAPI_ENCODE_RC_MODE(AVBR, "Average variable-bitrate") + VAAPI_ENCODE_RC_MODE(AVBR, "Average variable-bitrate"), \ + { "mbbrc", \ + "Macroblock based bitrate control",\ + OFFSET(common.mbbrc), AV_OPT_TYPE_BOOL, \ + { .i64 = 0 }, 0, 1, FLAGS } #endif /* AVCODEC_VAAPI_ENCODE_H */