diff mbox series

[FFmpeg-devel,v2] lavc/vaapi_encode: Enable Macroblock based bitrate control

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

Checks

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

Commit Message

Wang, Fei W Feb. 23, 2024, 7:14 a.m. UTC 
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(-)

Comments

Mark Thompson Feb. 25, 2024, 7:15 p.m. UTC | #1 
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
Wang, Fei W Feb. 26, 2024, 2:30 a.m. UTC | #2 
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 mbox series

Patch

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 */