Re: [FFmpeg-devel] [PATCH v5 4/9] avcodec/vaapi_encode: extract rc parameter configuration to base layer

[Mark Thompson <sw@jkqxz.net>]

On 18/02/2024 08:45, tong1.wu-at-intel.com@ffmpeg.org wrote:
> From: Tong Wu <tong1.wu@intel.com>
> 
> VAAPI and D3D12VA can share rate control configuration codes. Hence, it
> can be moved to base layer for simplification.
> 
> Signed-off-by: Tong Wu <tong1.wu@intel.com>
> ---
>   libavcodec/hw_base_encode.c    | 151 ++++++++++++++++++++++++
>   libavcodec/hw_base_encode.h    |  34 ++++++
>   libavcodec/vaapi_encode.c      | 210 ++++++---------------------------
>   libavcodec/vaapi_encode.h      |  14 +--
>   libavcodec/vaapi_encode_av1.c  |   2 +-
>   libavcodec/vaapi_encode_h264.c |   2 +-
>   libavcodec/vaapi_encode_vp9.c  |   2 +-
>   7 files changed, 227 insertions(+), 188 deletions(-)
> 
> diff --git a/libavcodec/hw_base_encode.c b/libavcodec/hw_base_encode.c
> index f0e4ef9655..c20c47bf55 100644
> --- a/libavcodec/hw_base_encode.c
> +++ b/libavcodec/hw_base_encode.c
> @@ -631,6 +631,157 @@ end:
>       return 0;
>   }
>   
> +int ff_hw_base_rc_mode_configure(AVCodecContext *avctx, const HWBaseEncodeRCMode *rc_mode,
> +                                 int default_quality, HWBaseEncodeRCConfigure *rc_conf)
> +{
> +    HWBaseEncodeContext *ctx = avctx->priv_data;
> +
> +    if (!rc_mode || !rc_conf)
> +        return -1;
> +
> +    if (rc_mode->bitrate) {
> +        if (avctx->bit_rate <= 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Bitrate must be set for %s "
> +                   "RC mode.\n", rc_mode->name);
> +            return AVERROR(EINVAL);
> +        }
> +
> +        if (rc_mode->mode == RC_MODE_AVBR) {
> +            // For maximum confusion AVBR is hacked into the existing API
> +            // by overloading some of the fields with completely different
> +            // meanings.

You definitely don't want this absurd wart from libva to leak into the common API parts.  Make new fields which have the desired meaning in the common parts and let the VAAPI code deal with this stupidity.

> +
> +            // Target percentage does not apply in AVBR mode.
> +            rc_conf->rc_bits_per_second = avctx->bit_rate;
> +
> +            // Accuracy tolerance range for meeting the specified target
> +            // bitrate.  It's very unclear how this is actually intended
> +            // to work - since we do want to get the specified bitrate,
> +            // set the accuracy to 100% for now.
> +            rc_conf->rc_target_percentage = 100;
> +
> +            // Convergence period in frames.  The GOP size reflects the
> +            // user's intended block size for cutting, so reusing that
> +            // as the convergence period seems a reasonable default.
> +            rc_conf->rc_window_size = avctx->gop_size > 0 ? avctx->gop_size : 60;
> +
> +        } else if (rc_mode->maxrate) {
> +            if (avctx->rc_max_rate > 0) {
> +                if (avctx->rc_max_rate < avctx->bit_rate) {
> +                    av_log(avctx, AV_LOG_ERROR, "Invalid bitrate settings: "
> +                           "bitrate (%"PRId64") must not be greater than "
> +                           "maxrate (%"PRId64").\n", avctx->bit_rate,
> +                           avctx->rc_max_rate);
> +                    return AVERROR(EINVAL);
> +                }
> +                rc_conf->rc_bits_per_second   = avctx->rc_max_rate;
> +                rc_conf->rc_target_percentage = (avctx->bit_rate * 100) /
> +                                                 avctx->rc_max_rate;
> +            } else {
> +                // We only have a target bitrate, but this mode requires
> +                // that a maximum rate be supplied as well.  Since the
> +                // user does not want this to be a constraint, arbitrarily
> +                // pick a maximum rate of double the target rate.
> +                rc_conf->rc_bits_per_second   = 2 * avctx->bit_rate;
> +                rc_conf->rc_target_percentage = 50;
> +            }
> +        } else {
> +            if (avctx->rc_max_rate > avctx->bit_rate) {
> +                av_log(avctx, AV_LOG_WARNING, "Max bitrate is ignored "
> +                       "in %s RC mode.\n", rc_mode->name);
> +            }
> +            rc_conf->rc_bits_per_second   = avctx->bit_rate;
> +            rc_conf->rc_target_percentage = 100;
> +        }
> +    } else {
> +        rc_conf->rc_bits_per_second   = 0;
> +        rc_conf->rc_target_percentage = 100;
> +    }
> +
> +    if (rc_mode->quality) {
> +        if (ctx->explicit_qp) {
> +            rc_conf->rc_quality = ctx->explicit_qp;
> +        } else if (avctx->global_quality > 0) {
> +            rc_conf->rc_quality = avctx->global_quality;
> +        } else {
> +            rc_conf->rc_quality = default_quality;
> +            av_log(avctx, AV_LOG_WARNING, "No quality level set; "
> +                   "using default (%d).\n", rc_conf->rc_quality);
> +        }
> +    } else {
> +        rc_conf->rc_quality = 0;
> +    }
> +
> +    if (rc_mode->hrd) {
> +        if (avctx->rc_buffer_size)
> +            rc_conf->hrd_buffer_size = avctx->rc_buffer_size;
> +        else if (avctx->rc_max_rate > 0)
> +            rc_conf->hrd_buffer_size = avctx->rc_max_rate;
> +        else
> +            rc_conf->hrd_buffer_size = avctx->bit_rate;
> +        if (avctx->rc_initial_buffer_occupancy) {
> +            if (avctx->rc_initial_buffer_occupancy > rc_conf->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, rc_conf->hrd_buffer_size);
> +                return AVERROR(EINVAL);
> +            }
> +            rc_conf->hrd_initial_buffer_fullness = avctx->rc_initial_buffer_occupancy;
> +        } else {
> +            rc_conf->hrd_initial_buffer_fullness = rc_conf->hrd_buffer_size * 3 / 4;
> +        }
> +
> +        rc_conf->rc_window_size = (rc_conf->hrd_buffer_size * 1000) / rc_conf->rc_bits_per_second;
> +    } else {
> +        if (avctx->rc_buffer_size || avctx->rc_initial_buffer_occupancy) {
> +            av_log(avctx, AV_LOG_WARNING, "Buffering settings are ignored "
> +                   "in %s RC mode.\n", rc_mode->name);
> +        }
> +
> +        rc_conf->hrd_buffer_size             = 0;
> +        rc_conf->hrd_initial_buffer_fullness = 0;
> +
> +        if (rc_mode->mode != RC_MODE_AVBR) {
> +            // Already set (with completely different meaning) for AVBR.
> +            rc_conf->rc_window_size = 1000;
> +        }
> +    }
> +
> +    if (rc_conf->rc_bits_per_second          > UINT32_MAX ||
> +        rc_conf->hrd_buffer_size             > UINT32_MAX ||
> +        rc_conf->hrd_initial_buffer_fullness > UINT32_MAX) {
> +        av_log(avctx, AV_LOG_ERROR, "RC parameters of 2^32 or "
> +               "greater are not supported by hardware.\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    av_log(avctx, AV_LOG_VERBOSE, "RC mode: %s.\n", rc_mode->name);
> +
> +    if (rc_mode->quality)
> +        av_log(avctx, AV_LOG_VERBOSE, "RC quality: %d.\n", rc_conf->rc_quality);
> +
> +    if (rc_mode->hrd) {
> +        av_log(avctx, AV_LOG_VERBOSE, "RC buffer: %"PRId64" bits, "
> +               "initial fullness %"PRId64" bits.\n",
> +               rc_conf->hrd_buffer_size, rc_conf->hrd_initial_buffer_fullness);
> +    }
> +
> +    if (avctx->framerate.num > 0 && avctx->framerate.den > 0)
> +        av_reduce(&rc_conf->fr_num, &rc_conf->fr_den,
> +                  avctx->framerate.num, avctx->framerate.den, 65535);
> +    else
> +        av_reduce(&rc_conf->fr_num, &rc_conf->fr_den,
> +                  avctx->time_base.den, avctx->time_base.num, 65535);
> +
> +    av_log(avctx, AV_LOG_VERBOSE, "RC framerate: %d/%d (%.2f fps).\n",
> +           rc_conf->fr_num, rc_conf->fr_den, (double)rc_conf->fr_num / rc_conf->fr_den);
> +
> +    ctx->rc_quality  = rc_conf->rc_quality;
> +
> +    return 0;
> +}
> +
>   int ff_hw_base_encode_init(AVCodecContext *avctx)
>   {
>       HWBaseEncodeContext *ctx = avctx->priv_data;
> diff --git a/libavcodec/hw_base_encode.h b/libavcodec/hw_base_encode.h
> index b836b22e6b..4072b514d3 100644
> --- a/libavcodec/hw_base_encode.h
> +++ b/libavcodec/hw_base_encode.h
> @@ -72,6 +72,37 @@ enum {
>       RC_MODE_MAX = RC_MODE_AVBR,
>   };
>   
> +typedef struct HWBaseEncodeRCMode {
> +    // Mode from above enum (RC_MODE_*).
> +    int mode;
> +    // Name.
> +    const char *name;
> +    // Uses bitrate parameters.
> +    int bitrate;
> +    // Supports maxrate distinct from bitrate.
> +    int maxrate;
> +    // Uses quality value.
> +    int quality;
> +    // Supports HRD/VBV parameters.
> +    int hrd;
> +} HWBaseEncodeRCMode;
> +
> +typedef struct HWBaseEncodeRCConfigure
> +{
> +    int64_t rc_bits_per_second;
> +    int rc_target_percentage;
> +    int rc_window_size;
> +
> +    int rc_quality;
> +
> +    int64_t hrd_buffer_size;
> +    int64_t hrd_initial_buffer_fullness;
> +
> +    int fr_num;
> +    int fr_den;
> +} HWBaseEncodeRCConfigure;

The set of fields here needs more thought to match up the common parts of the APIs.

Just have target rate and maxrate and let VAAPI deal with the percentage stuff maybe?  Not sure what to do with window_size which isn't present at all in D3D12.  The convergence/accuracy stuff for VAAPI AVBR is also unclear.

(Do you know why QVBR is missing the VBV parameters in D3D12?  On the face of it that doesn't make any sense, but maybe I'm missing something.)

> +
> +
>   typedef struct HWBaseEncodePicture {
>       struct HWBaseEncodePicture *next;
>   
> @@ -242,6 +273,9 @@ int ff_hw_base_encode_set_output_property(AVCodecContext *avctx, HWBaseEncodePic
>   
>   int ff_hw_base_encode_receive_packet(AVCodecContext *avctx, AVPacket *pkt);
>   
> +int ff_hw_base_rc_mode_configure(AVCodecContext *avctx, const HWBaseEncodeRCMode *rc_mode,
> +                                 int default_quality, HWBaseEncodeRCConfigure *rc_conf);
> +
>   int ff_hw_base_encode_init(AVCodecContext *avctx);
>   
>   int ff_hw_base_encode_close(AVCodecContext *avctx);
> diff --git a/libavcodec/vaapi_encode.c b/libavcodec/vaapi_encode.c
> ...

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".