* [FFmpeg-devel] [PATCH] libsvtav1: Enable 2-pass encoding
@ 2024-05-15 14:14 Fredrik Lundkvist via ffmpeg-devel
2024-05-15 14:57 ` James Almer
0 siblings, 1 reply; 5+ messages in thread
From: Fredrik Lundkvist via ffmpeg-devel @ 2024-05-15 14:14 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Fredrik Lundkvist
Currently, libsvtav1 does not send pass number or stat buffer to SVT-AV1, which means that 2-pass encoding is not possible using FFMPEG;
if a user wants to do 2-pass encoding using SVT-AV1, they have to use SvtAv1EncApp.
This patch adds 2-pass encoding support to libsvtav1 using the stats buffers provided by the AVCodecContext.
When encoding the first pass using SVT-AV1, the following should be logged:
Svt[info]: SVT [config]: preset : Pass 1
Indicating that the encoder knows that it is performing the first pass, activating a special preset.
With the patch applied:
ffmpeg -i input.mp4 -c:v libsvtav1 -an -pass 1 -b:v 1M -f null /dev/null
[...]
Svt[info]: -------------------------------------------
Svt[info]: SVT [config]: preset : Pass 1
Svt[info]: -------------------------------------------
[...]
Signed-off-by: Fredrik Lundkvist <fredrik.lundkvist@svt.se<mailto:fredrik.lundkvist@svt.se>>
---
libavcodec/libsvtav1.c | 47 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
index 9bc165f0cf..be2189399f 100644
--- a/libavcodec/libsvtav1.c
+++ b/libavcodec/libsvtav1.c
@@ -25,6 +25,7 @@
#include <EbSvtAv1Enc.h>
#include <EbSvtAv1Metadata.h>
+#include "libavutil/base64.h"
#include "libavutil/common.h"
#include "libavutil/frame.h"
#include "libavutil/imgutils.h"
@@ -226,6 +227,34 @@ static int config_enc_params(EbSvtAv1EncConfiguration *param,
param->max_qp_allowed = avctx->qmax;
param->min_qp_allowed = avctx->qmin;
}
+
+ if (avctx->flags & AV_CODEC_FLAG_PASS2) {
+ int decode_size, ret;
+ if(!avctx->stats_in) {
+ av_log(avctx, AV_LOG_ERROR, "No stats file for second pass\n");
+ return AVERROR_INVALIDDATA;
+ }
+ param->rc_stats_buffer.sz = strlen(avctx->stats_in) * 3 / 4;
+ ret = av_reallocp(¶m->rc_stats_buffer.buf, param->rc_stats_buffer.sz);
+ if (ret < 0) {
+ av_log(avctx, AV_LOG_ERROR,
+ "Stat buffer alloc (%"SIZE_SPECIFIER" bytes) failed \n",
+ param->rc_stats_buffer.sz
+ );
+ param->rc_stats_buffer.sz = 0;
+ return ret;
+ }
+ decode_size = av_base64_decode(param->rc_stats_buffer.buf, avctx->stats_in, param->rc_stats_buffer.sz);
+ if (decode_size < 0) {
+ av_log(avctx, AV_LOG_ERROR, "Stat buffer decode failed\n");
+ return AVERROR_INVALIDDATA;
+ }
+ param->rc_stats_buffer.sz = decode_size;
+ param->pass = 2;
+ } else if (avctx->flags & AV_CODEC_FLAG_PASS1) {
+ param->pass = 1;
+ }
+
param->max_bit_rate = avctx->rc_max_rate;
if ((avctx->bit_rate > 0 || avctx->rc_max_rate > 0) && avctx->rc_buffer_size)
param->maximum_buffer_size_ms =
@@ -618,6 +647,23 @@ static int eb_receive_packet(AVCodecContext *avctx, AVPacket *pkt)
if (headerPtr->flags & EB_BUFFERFLAG_EOS) {
svt_enc->eos_flag = EOS_RECEIVED;
svt_av1_enc_release_out_buffer(&headerPtr);
+ if (avctx->flags & AV_CODEC_FLAG_PASS1) {
+ SvtAv1FixedBuf first_pass_stats;
+ EbErrorType ret = svt_av1_enc_get_stream_info(svt_enc->svt_handle, SVT_AV1_STREAM_INFO_FIRST_PASS_STATS_OUT, &first_pass_stats);
+ if (ret == EB_ErrorNone) {
+ size_t b64_size = AV_BASE64_SIZE(first_pass_stats.sz);
+
+ avctx->stats_out = av_malloc(b64_size);
+ if(!avctx->stats_out) {
+ av_log(avctx, AV_LOG_ERROR, "Stat buffer alloc (%"SIZE_SPECIFIER" bytes) failed\n",
+ b64_size);
+ return AVERROR(ENOMEM);
+ }
+ av_base64_encode(avctx->stats_out, b64_size, first_pass_stats.buf, first_pass_stats.sz);
+ } else {
+ av_log(avctx, AV_LOG_ERROR, "Failed to get stream info");
+ }
+ }
return AVERROR_EOF;
}
#endif
@@ -681,7 +727,6 @@ static av_cold int eb_enc_close(AVCodecContext *avctx)
svt_metadata_array_free(&svt_enc->in_buf->metadata);
av_freep(&svt_enc->in_buf);
}
-
av_buffer_pool_uninit(&svt_enc->pool);
av_frame_free(&svt_enc->frame);
ff_dovi_ctx_unref(&svt_enc->dovi);
--
2.39.3 (Apple Git-145)
_______________________________________________
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".
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] libsvtav1: Enable 2-pass encoding
2024-05-15 14:14 [FFmpeg-devel] [PATCH] libsvtav1: Enable 2-pass encoding Fredrik Lundkvist via ffmpeg-devel
@ 2024-05-15 14:57 ` James Almer
0 siblings, 0 replies; 5+ messages in thread
From: James Almer @ 2024-05-15 14:57 UTC (permalink / raw)
To: ffmpeg-devel
On 5/15/2024 11:14 AM, Fredrik Lundkvist via ffmpeg-devel wrote:
> Currently, libsvtav1 does not send pass number or stat buffer to SVT-AV1, which means that 2-pass encoding is not possible using FFMPEG;
> if a user wants to do 2-pass encoding using SVT-AV1, they have to use SvtAv1EncApp.
> This patch adds 2-pass encoding support to libsvtav1 using the stats buffers provided by the AVCodecContext.
> When encoding the first pass using SVT-AV1, the following should be logged:
> Svt[info]: SVT [config]: preset : Pass 1
> Indicating that the encoder knows that it is performing the first pass, activating a special preset.
> With the patch applied:
> ffmpeg -i input.mp4 -c:v libsvtav1 -an -pass 1 -b:v 1M -f null /dev/null
> [...]
> Svt[info]: -------------------------------------------
> Svt[info]: SVT [config]: preset : Pass 1
> Svt[info]: -------------------------------------------
> [...]
>
> Signed-off-by: Fredrik Lundkvist <fredrik.lundkvist@svt.se<mailto:fredrik.lundkvist@svt.se>>
> ---
> libavcodec/libsvtav1.c | 47 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
> index 9bc165f0cf..be2189399f 100644
> --- a/libavcodec/libsvtav1.c
> +++ b/libavcodec/libsvtav1.c
> @@ -25,6 +25,7 @@
> #include <EbSvtAv1Enc.h>
> #include <EbSvtAv1Metadata.h>
>
> +#include "libavutil/base64.h"
> #include "libavutil/common.h"
> #include "libavutil/frame.h"
> #include "libavutil/imgutils.h"
> @@ -226,6 +227,34 @@ static int config_enc_params(EbSvtAv1EncConfiguration *param,
> param->max_qp_allowed = avctx->qmax;
> param->min_qp_allowed = avctx->qmin;
> }
> +
> + if (avctx->flags & AV_CODEC_FLAG_PASS2) {
> + int decode_size, ret;
> + if(!avctx->stats_in) {
if (...
> + av_log(avctx, AV_LOG_ERROR, "No stats file for second pass\n");
> + return AVERROR_INVALIDDATA;
> + }
> + param->rc_stats_buffer.sz = strlen(avctx->stats_in) * 3 / 4;
nit: AV_BASE64_DECODE_SIZE
> + ret = av_reallocp(¶m->rc_stats_buffer.buf, param->rc_stats_buffer.sz);
Why realloc instead of malloc?
> + if (ret < 0) {
> + av_log(avctx, AV_LOG_ERROR,
> + "Stat buffer alloc (%"SIZE_SPECIFIER" bytes) failed \n",
> + param->rc_stats_buffer.sz
> + );
> + param->rc_stats_buffer.sz = 0;
> + return ret;
> + }
> + decode_size = av_base64_decode(param->rc_stats_buffer.buf, avctx->stats_in, param->rc_stats_buffer.sz);
> + if (decode_size < 0) {
Wouldn't it be safer to free rc_stats_buffer.buf and set
rc_stats_buffer.sz to 0 on failure here too?
> + av_log(avctx, AV_LOG_ERROR, "Stat buffer decode failed\n");
> + return AVERROR_INVALIDDATA;
> + }
> + param->rc_stats_buffer.sz = decode_size;
> + param->pass = 2;
> + } else if (avctx->flags & AV_CODEC_FLAG_PASS1) {
No brackets for single line blocks.
> + param->pass = 1;
> + }
> +
> param->max_bit_rate = avctx->rc_max_rate;
> if ((avctx->bit_rate > 0 || avctx->rc_max_rate > 0) && avctx->rc_buffer_size)
> param->maximum_buffer_size_ms =
> @@ -618,6 +647,23 @@ static int eb_receive_packet(AVCodecContext *avctx, AVPacket *pkt)
> if (headerPtr->flags & EB_BUFFERFLAG_EOS) {
> svt_enc->eos_flag = EOS_RECEIVED;
> svt_av1_enc_release_out_buffer(&headerPtr);
> + if (avctx->flags & AV_CODEC_FLAG_PASS1) {
> + SvtAv1FixedBuf first_pass_stats;
> + EbErrorType ret = svt_av1_enc_get_stream_info(svt_enc->svt_handle, SVT_AV1_STREAM_INFO_FIRST_PASS_STATS_OUT, &first_pass_stats);
svt_ret. Don't shadow ret.
Also, consider splitting this line.
> + if (ret == EB_ErrorNone) {
> + size_t b64_size = AV_BASE64_SIZE(first_pass_stats.sz);
> +
> + avctx->stats_out = av_malloc(b64_size);
> + if(!avctx->stats_out) {
Missing space after the if again.
> + av_log(avctx, AV_LOG_ERROR, "Stat buffer alloc (%"SIZE_SPECIFIER" bytes) failed\n",
> + b64_size);
> + return AVERROR(ENOMEM);
> + }
> + av_base64_encode(avctx->stats_out, b64_size, first_pass_stats.buf, first_pass_stats.sz);
Unchecked return value.
> + } else {
Single line block again.
> + av_log(avctx, AV_LOG_ERROR, "Failed to get stream info");
> + }
> + }
> return AVERROR_EOF;
> }
> #endif
> @@ -681,7 +727,6 @@ static av_cold int eb_enc_close(AVCodecContext *avctx)
> svt_metadata_array_free(&svt_enc->in_buf->metadata);
> av_freep(&svt_enc->in_buf);
> }
> -
Spurious change.
> av_buffer_pool_uninit(&svt_enc->pool);
> av_frame_free(&svt_enc->frame);
> ff_dovi_ctx_unref(&svt_enc->dovi);
> --
> 2.39.3 (Apple Git-145)
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] libsvtav1: Enable 2-pass encoding
@ 2024-05-16 6:31 Fredrik Lundkvist via ffmpeg-devel
2024-05-16 6:59 ` Fredrik Lundkvist via ffmpeg-devel
2024-05-16 10:13 ` Andreas Rheinhardt
0 siblings, 2 replies; 5+ messages in thread
From: Fredrik Lundkvist via ffmpeg-devel @ 2024-05-16 6:31 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Fredrik Lundkvist
> > + ret = av_reallocp(¶m->rc_stats_buffer.buf, param->rc_stats_buffer.sz);
> Why realloc instead of malloc?
As far as I gathered, rc_stats_buffer.buf is already allocated by SVT-AV1; if malloc is preferred, I can change it and test to make sure it doesn’t cause any obvious issues.
> Wouldn't it be safer to free rc_stats_buffer.buf and set rc_stats_buffer.sz to 0 on failure here too?
Absolutely agree on setting rc_stats_buffer.sz to 0, not quite sure about freeing rc_stats_buffer_buf it is allocated by SVT-AV1.
Thank you for the review, the other comments will be fixed in V2.
/Fredrik
_______________________________________________
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".
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] libsvtav1: Enable 2-pass encoding
2024-05-16 6:31 Fredrik Lundkvist via ffmpeg-devel
@ 2024-05-16 6:59 ` Fredrik Lundkvist via ffmpeg-devel
2024-05-16 10:13 ` Andreas Rheinhardt
1 sibling, 0 replies; 5+ messages in thread
From: Fredrik Lundkvist via ffmpeg-devel @ 2024-05-16 6:59 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Fredrik Lundkvist
To continue this discussion.
>> + av_base64_encode(avctx->stats_out, b64_size, first_pass_stats.buf, first_pass_stats.sz);
> Unchecked return value.
I don’t see any non-test instance in the code base where the return value of av_base64_encode is actually checked. Unchecked calls are however found in many places, f.ex. libaomenc, librav1e, and matroskadec. IMO it should be consistent across the code base, and thus stay as is in this patch.
/Fredrik
_______________________________________________
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".
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [FFmpeg-devel] [PATCH] libsvtav1: Enable 2-pass encoding
2024-05-16 6:31 Fredrik Lundkvist via ffmpeg-devel
2024-05-16 6:59 ` Fredrik Lundkvist via ffmpeg-devel
@ 2024-05-16 10:13 ` Andreas Rheinhardt
1 sibling, 0 replies; 5+ messages in thread
From: Andreas Rheinhardt @ 2024-05-16 10:13 UTC (permalink / raw)
To: ffmpeg-devel
Fredrik Lundkvist via ffmpeg-devel:
>>> + ret = av_reallocp(¶m->rc_stats_buffer.buf, param->rc_stats_buffer.sz);
>
>> Why realloc instead of malloc?
>
> As far as I gathered, rc_stats_buffer.buf is already allocated by SVT-AV1; if malloc is preferred, I can change it and test to make sure it doesn’t cause any obvious issues.
>
https://gitlab.com/AOMediaCodec/SVT-AV1/-/blob/master/Source/API/EbSvtAv1Enc.h?ref_type=heads#L163
says that this pointer is a non-ownership pointer. Which would imply
that you can't realloc the initial pointer at all and have to simply
replace it with something of our own.
(That we would probably also need to free on our own.)
>
>> Wouldn't it be safer to free rc_stats_buffer.buf and set rc_stats_buffer.sz to 0 on failure here too?
>
> Absolutely agree on setting rc_stats_buffer.sz to 0, not quite sure about freeing rc_stats_buffer_buf it is allocated by SVT-AV1.
>
> Thank you for the review, the other comments will be fixed in V2.
>
> /Fredrik
> _______________________________________________
> 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-16 10:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-15 14:14 [FFmpeg-devel] [PATCH] libsvtav1: Enable 2-pass encoding Fredrik Lundkvist via ffmpeg-devel
2024-05-15 14:57 ` James Almer
2024-05-16 6:31 Fredrik Lundkvist via ffmpeg-devel
2024-05-16 6:59 ` Fredrik Lundkvist via ffmpeg-devel
2024-05-16 10:13 ` Andreas Rheinhardt
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git