>On 18/02/2024 08:45, tong1.wu-at-intel....@ffmpeg.org wrote:
>> From: Tong Wu <tong1...@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...@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.
Agreed.
>
>> +
>> + // 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.
>
Could you please explain more about your thoughts on why the percentage stuff
should not be in the common part? I think D3D12 can share the same code in
terms of percentage stuff and hrd stuff. I can let VAAPI do the AVBR stuff and
remove window_size from common part and keep everything else as-is.
>(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.)
>
It is presented in QVBR1 structure(defined in DirectX-header but not yet in
Windows SDK).
I can add this support afterwards maybe along with AV1 implementation.
>> +
>> +
>> 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
Thanks for your review and I'll reply to your comments regarding D3D12 HEVC
encoder patch later since there're indeed a lot of stuff you pointed out that I
need to take care of again.
BRs,
Tong
>_______________________________________________
>ffmpeg-devel mailing list
>ffmpeg-devel@ffmpeg.org
>https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>To unsubscribe, visit link above, or email
>ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
