From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 6147A49747 for ; Sun, 18 Feb 2024 19:50:14 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 158D768D378; Sun, 18 Feb 2024 21:50:13 +0200 (EET) Received: from mail-wr1-f44.google.com (mail-wr1-f44.google.com [209.85.221.44]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id DCF5468D2FC for ; Sun, 18 Feb 2024 21:50:06 +0200 (EET) Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-3394bec856fso2654745f8f.0 for ; Sun, 18 Feb 2024 11:50:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jkqxz-net.20230601.gappssmtp.com; s=20230601; t=1708285806; x=1708890606; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=NHPk7gWLfNmJwIBQjS13KxxhDQUH4MSYBwRUg/rVFC0=; b=fSmTZpAac7vCF6tiCdZRoqebs2u9W80ZW3ABSFRVV6MDSfIuruQmKlChkmKqQYOyvM Sq8crBZu+pwsR24CNjj/gE45KrIHOFXr8/0iU2OPVYADLSzooF+yzNha7ej1ZnUvZftn HhyBKOAdsFVkBOHHu50DZXr0GcK2+8FQq6OU7eXP0GzzlgATonTg5iU/gG4sk/p+ast1 8MZ0l0lJVWCcAwbKdPyDx27PR08Opg4eiZdF0Zc8FLCyOQq9zZqxWW9Xs2O95NaXT3OJ 4jo+1isqbTuIXMRA3CNaLa/6EF1kxheFBsIT7IiZAnNRrDWkGFUV6FbPcELBa0Ea4D+x lh8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708285806; x=1708890606; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=NHPk7gWLfNmJwIBQjS13KxxhDQUH4MSYBwRUg/rVFC0=; b=p2JTKowIWFbmw3JlEmWROSJCrvAhath45RhpdmvgycgQhhm8K6pv8zMJJwYG/ftabe ix3BiZj75dXBrwqj042pjJ/fEjXNCX38oqeRO2zVwQ4sjNfo+EzatGbUQxMi9ZoIxswI PSZXQA/elDx0tziC5r+pGI8c3zgllOQODKP4e++XHNHpgaDqj4pQZIlMsMEpckx9Jz6T QHs97GdOpn07ZL9tQN/ECBHHeWkyIpMTx1ei0ov/F/7vpompyXjAgPGjuAANtXjAH1tl j9ESSDvIO1TQgnnXGC+OMX6c6/u+qUMLA8g6rulZLp6RbhNkq38+i0MgV8LtXyntXbMn bG5Q== X-Gm-Message-State: AOJu0YzhFTGVXoxUAu/Ebly/NGCanBPAW3IxJ1uyneEYFXkncFh6IuIk sEt3NmlM8hQlDUu6mvxomRKl9+r8l2G1/zsth+16EoNcaVN55K2eHtzbKj1oLUhae2veUARUO1B u X-Google-Smtp-Source: AGHT+IGsQTmIAjOKWPuO/z4R1vi9J/3Fgiln6hrpwIzTocl0bGfqg/uMBG0zjja5nYIGmfBvmvlLLA== X-Received: by 2002:a5d:6907:0:b0:33c:e45c:f17f with SMTP id t7-20020a5d6907000000b0033ce45cf17fmr11506552wru.13.1708285805419; Sun, 18 Feb 2024 11:50:05 -0800 (PST) Received: from [192.168.0.15] (cpc92302-cmbg19-2-0-cust1183.5-4.cable.virginm.net. [82.1.212.160]) by smtp.gmail.com with ESMTPSA id b1-20020a05600c11c100b00410cc2f5550sm9012611wmi.19.2024.02.18.11.50.05 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 18 Feb 2024 11:50:05 -0800 (PST) Message-ID: Date: Sun, 18 Feb 2024 19:50:31 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20240218084529.554-1-tong1.wu@intel.com> <20240218084529.554-4-tong1.wu@intel.com> From: Mark Thompson In-Reply-To: <20240218084529.554-4-tong1.wu@intel.com> Subject: Re: [FFmpeg-devel] [PATCH v5 4/9] avcodec/vaapi_encode: extract rc parameter configuration to base layer X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 18/02/2024 08:45, tong1.wu-at-intel.com@ffmpeg.org wrote: > From: Tong Wu > > VAAPI and D3D12VA can share rate control configuration codes. Hence, it > can be moved to base layer for simplification. > > Signed-off-by: Tong Wu > --- > 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".