Re: [FFmpeg-devel] libavc/libx264: add support to propagate SSE values through encoder stats
Hi Anton,
On Tue, 2023-10-10 at 12:54 +0200, Anton Khirnov wrote:
>
> Quoting Carotti, Elias via ffmpeg-devel (2023-10-02 19:35:09)
> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> > index 77a9f173b4..4c643c9066 100644
> > --- a/libavcodec/libx264.c
> > +++ b/libavcodec/libx264.c
> > @@ -129,6 +129,8 @@ typedef struct X264Context {
> > int roi_warned;
> >
> > int mb_info;
> > +
> > + int64_t sse[3];
>
> The values don't need to be preserved across frames, so might as well
> put this on stack in the block calling
> ff_side_data_set_encoder_stats().
Agreed.
>
> > } X264Context;
> >
> > static void X264_log(void *p, int level, const char *fmt, va_list
> > args)
> > @@ -726,7 +728,40 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >
> > pkt->flags |= AV_PKT_FLAG_KEY*pic_out.b_keyframe;
> > if (ret) {
> > - ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 -
> > 1) * FF_QP2LAMBDA, NULL, 0, pict_type);
> > + const AVPixFmtDescriptor *pix_desc =
> > av_pix_fmt_desc_get(csp_to_pixfmt(pic_out.img.i_csp));
> > + int error_count = 0;
> > + int64_t *errors = NULL;
> > +
> > + if (ctx->flags & AV_CODEC_FLAG_PSNR) {
> > + double scale[3] = { 1,
> > + (1 << pix_desc->log2_chroma_h) * (double)(1 <<
> > pix_desc->log2_chroma_w),
> > + (1 << pix_desc->log2_chroma_h) * (double)(1 <<
> > pix_desc->log2_chroma_w),
>
> Any particular reason the cast is on the second value? It looks
> strange.
>
Just my habit. Fixed.
> > + };
> > + double mse;
> > + int i;
> > +
> > + error_count = pix_desc->nb_components;
> > +
> > + av_log(ctx, AV_LOG_DEBUG, "PSNR values from libx264:
> > %.3f %.3f %.3f.\n",
> > + pic_out.prop.f_psnr[0], pic_out.prop.f_psnr[1],
> > pic_out.prop.f_psnr[2]);
> > +
> > + for (i = 0; i < pix_desc->nb_components; ++i) {
>
> for (int i
Agreed.
I also found the - (minus) sign in the mse formula was wrong and I
removed it.
Numbers seem to be coherent with those from libx264.
Please find attached a new patch rebased against the latest master with
the above fixes.
There is an increasing error (over increasing PSNRs and resolutions)
when reconstructing the PSNR from the SSE as computed above due to the
approximations and the roundings back and forth, however it seems to
yield similar values as those computed by libx264.
Best
NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese
di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR
i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
From 8c9456042e0cd333702b8e77d3e80767a4c0b7cf Mon Sep 17 00:00:00 2001
From: Elias Carotti
Date: Fri, 15 Sep 2023 20:05:43 +0200
Subject: [PATCH] avcodec/libx264: Add the SSE computation for libx264.
Since libx264 only provides a per-frame per-channel PSNR, this is inverted to get back the SSE.
---
libavcodec/libx264.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 77a9f173b4..85bd870f5d 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -726,7 +726,39 @@ FF_ENABLE_DEPRECATION_WARNINGS
pkt->flags |= AV_PKT_FLAG_KEY*pic_out.b_keyframe;
if (ret) {
-ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 - 1) * FF_QP2LAMBDA, NULL, 0, pict_type);
+const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(csp_to_pixfmt(pic_out.img.i_csp));
+int error_count = 0;
+int64_t *errors = NULL;
+int64_t sse[3] = {0};
+
+if (ctx->flags & AV_CODEC_FLAG_PSNR) {
+double scale[3] = { 1,
+(double)(1 << pix_desc->log2_chroma_h) * (1 << pix_desc->log2_chroma_w),
+(double)(1 << pix_desc->log2_chroma_h) * (1 << pix_desc->log2_chroma_w),
+};
+
+error_count = pix_desc->nb_components;
+
+av_log(ctx, AV_LOG_DEBUG, "PSNR values from libx264: %.3f %.3f %.3f.\n",
+ pic_out.prop.f_psnr[0], pic_out.prop.f_psnr[1], pic_out.prop.f_psnr[2]);
+
+for (int i = 0; i < pix_desc->nb_components; ++i) {
+double max_value = (double)(1 << pix_desc->comp[i].depth) - 1.0;
+double plane_size = ctx->width * (double)ctx->height / scale[i];
+
+/* psnr = 10 * log10(max_value * max_value / mse) */
+double mse = (max_value * max
Re: [FFmpeg-devel] libavc/libx264: add support to propagate SSE values through encoder stats
On Fri, 2023-10-13 at 16:16 +0200, Anton Khirnov wrote:
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender
> and know the content is safe.
>
>
>
> Quoting Carotti, Elias via ffmpeg-devel (2023-10-11 12:54:21)
> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> > index 77a9f173b4..85bd870f5d 100644
> > --- a/libavcodec/libx264.c
> > +++ b/libavcodec/libx264.c
> > @@ -726,7 +726,39 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >
> > pkt->flags |= AV_PKT_FLAG_KEY*pic_out.b_keyframe;
> > if (ret) {
> > - ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 -
> > 1) * FF_QP2LAMBDA, NULL, 0, pict_type);
> > + const AVPixFmtDescriptor *pix_desc =
> > av_pix_fmt_desc_get(csp_to_pixfmt(pic_out.img.i_csp));
>
> There's a problem here - we do not handle all values of i_csp.
> E.g. we have no equivalent of X264_CSP_NV12 | X264_CSP_HIGH_DEPTH,
> which
> x264 will use for YUV420P10 input.
>
> The best solution is probably to use AVCodecContext.pix_fmt and
> assume
> that x264 doesn't do any nontrivial (i.e. other than interleaving and
> such) pixel format transformations internally.
>
I see. Wouldn't not outputting the SSE values when csp_to_pixfmt()
returns AV_PIX_FMT_NONE work better?
That wouldn't be worse than it is today (meaning that right now we
don't get those values for any pix_fmt).
Anyway, I did as you suggested and used AVCodecContext.pix_fmt.
> >
> > + av_log(ctx, AV_LOG_DEBUG, "PSNR values from libx264:
> > %.3f %.3f %.3f.\n",
> > + pic_out.prop.f_psnr[0], pic_out.prop.f_psnr[1],
> > pic_out.prop.f_psnr[2]);
>
> In my tests libx264 prints these values by itself, so this seems
> redundant.
removed.
NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese
di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR
i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
From b702fcd76cf0626f75a941875f31add50d08894d Mon Sep 17 00:00:00 2001
From: Elias Carotti
Date: Fri, 15 Sep 2023 20:05:43 +0200
Subject: [PATCH] avcodec/libx264: Add the SSE computation for libx264.
Since libx264 only provides a per-frame per-channel PSNR, this is inverted to get back the SSE.
---
libavcodec/libx264.c | 31 ++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 77a9f173b4..6ebe210039 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -726,7 +726,36 @@ FF_ENABLE_DEPRECATION_WARNINGS
pkt->flags |= AV_PKT_FLAG_KEY*pic_out.b_keyframe;
if (ret) {
-ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 - 1) * FF_QP2LAMBDA, NULL, 0, pict_type);
+const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(ctx->pix_fmt);
+int error_count = 0;
+int64_t *errors = NULL;
+int64_t sse[3] = {0};
+
+if (ctx->flags & AV_CODEC_FLAG_PSNR) {
+double scale[3] = { 1,
+(double)(1 << pix_desc->log2_chroma_h) * (1 << pix_desc->log2_chroma_w),
+(double)(1 << pix_desc->log2_chroma_h) * (1 << pix_desc->log2_chroma_w),
+};
+
+error_count = pix_desc->nb_components;
+
+for (int i = 0; i < pix_desc->nb_components; ++i) {
+double max_value = (double)(1 << pix_desc->comp[i].depth) - 1.0;
+double plane_size = ctx->width * (double)ctx->height / scale[i];
+
+/* psnr = 10 * log10(max_value * max_value / mse) */
+double mse = (max_value * max_value) / pow(10, pic_out.prop.f_psnr[i] / 10.0);
+
+/* SSE = MSE * width * height / scale -> because of possible chroma downsampling */
+sse[i] = (int64_t)floor(mse * plane_size + .5);
+};
+
+errors = sse;
+}
+
+ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 - 1) * FF_QP2LAMBDA,
+ errors, error_count, pict_type);
+
if (wallclock)
ff_side_data_set_prft(pkt, wallclock);
}
--
2.34.1
___
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".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
Hi
-Original Message-
From: ffmpeg-devel On Behalf Of Stefano
Sabatini
Sent: Friday, August 25, 2023 12:01 PM
To: FFmpeg development discussions and patches
Cc: Stefano Sabatini
Subject: [EXTERNAL] [FFmpeg-devel] [PATCH] lavc/libx264: enable
x4->params.analyse.b_fast_pskip if mb_info is set
CAUTION: This email originated from outside of the organization. Do not click
links or open attachments unless you can confirm the sender and know the
content is safe.
x4->params.analyse.b_fast_pskip should only be forced in case mb_info
is set.
Fix output change introduced in 418c954e318.
---
libavcodec/libx264.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index
1a7dc7bdd5..a2877d7f75 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -1190,7 +1190,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
}
x4->params.analyse.b_mb_info = x4->mb_info;
-x4->params.analyse.b_fast_pskip = 1;
+if (x4->mb_info) {
+x4->params.analyse.b_fast_pskip = x4->mb_info;
+}
// update AVCodecContext with x264 parameters
avctx->has_b_frames = x4->params.i_bframe ?
--
2.25.1
Sorry for the delay. I agree, this was missing in the patch.
Best
Elias
___
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".
NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese
di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR
i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
___
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".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
On Thu, 2023-08-31 at 19:09 +0200, Stefano Sabatini wrote: > > > > In particular why are you turning on fast_pskip silently based on a > > completely different setting? > > The patch is fixing the regression introduced by the unconditional > setting of b_fast_pskip. > > Now the question is if it makes sense to set mb_info without > b_fast_pskip (in both case this should be probably documented). > > @Elias can you comment about the mb_info/b_fast_pskip use case? Sorry again for the delay in responding. We can safely remove it altogether. It's true we don't need to set it along with mb_info. However, it doesn't do any harm, since fast_pskip is by default set to true by libx264 and only turned off either explicitly by the user, or when using the placebo preset, or when doing lossless encoding (constant QP == 0.) So, I agree, let's remove these three lines. Elias NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico ___ 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".
Re: [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set
-Original Message- From: ffmpeg-devel On Behalf Of Stefano Sabatini Sent: Saturday, September 2, 2023 5:45 PM To: ffmpeg-devel@ffmpeg.org Cc: kier...@obe.tv; Carotti, Elias Subject: RE: [EXTERNAL] [FFmpeg-devel] [PATCH] lavc/libx264: enable x4->params.analyse.b_fast_pskip if mb_info is set CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe. On date Saturday 2023-09-02 09:20:08 +, Carotti, Elias wrote: > On Thu, 2023-08-31 at 19:09 +0200, Stefano Sabatini wrote: > > > > > > > > In particular why are you turning on fast_pskip silently based on > > > a completely different setting? > > > > The patch is fixing the regression introduced by the unconditional > > setting of b_fast_pskip. > > > > Now the question is if it makes sense to set mb_info without > > b_fast_pskip (in both case this should be probably documented). > > > > @Elias can you comment about the mb_info/b_fast_pskip use case? > > Sorry again for the delay in responding. > We can safely remove it altogether. It's true we don't need to set it > along with mb_info. > However, it doesn't do any harm, since fast_pskip is by default set to > true by libx264 and only turned off either explicitly by the user, or > when using the placebo preset, or when doing lossless encoding > (constant QP == 0.) So, I agree, let's remove these three lines. Thanks, updated. Ok for me Elias NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico ___ 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] libavc/libx264: add support to propagate SSE values through encoder stats
Hi,
please find attached a patch to propagate the SSE for a frame into the
encoder stats.
Since libx264 already provides PSNR values, this is done by basically
inverting the formula to recover the SSE values.
Would it be possible to also append other values to the errors vector?
E.g., libx264 also computes SSIM but other values could be provided.
Best,
Elias
NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese
di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR
i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
From dfd47efb7f7cb264fef62d0d8fb70ff8168bdfd7 Mon Sep 17 00:00:00 2001
From: Elias Carotti
Date: Fri, 15 Sep 2023 20:05:43 +0200
Subject: [PATCH] Add the SSE calculation for libx264.
Since libx264 only provides a per-frame per-channel PSNR, this is inverted to get back the SSE.
---
libavcodec/libx264.c | 37 -
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 77a9f173b4..0fae155ff2 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -129,6 +129,8 @@ typedef struct X264Context {
int roi_warned;
int mb_info;
+
+int64_t sse[3];
} X264Context;
static void X264_log(void *p, int level, const char *fmt, va_list args)
@@ -726,7 +728,40 @@ FF_ENABLE_DEPRECATION_WARNINGS
pkt->flags |= AV_PKT_FLAG_KEY*pic_out.b_keyframe;
if (ret) {
-ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 - 1) * FF_QP2LAMBDA, NULL, 0, pict_type);
+const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(csp_to_pixfmt(pic_out.img.i_csp));
+int error_count = 0;
+int64_t *errors = NULL;
+
+if (ctx->flags & AV_CODEC_FLAG_PSNR) {
+double scale[3] = { 1,
+(1 << pix_desc->log2_chroma_h) * (double)(1 << pix_desc->log2_chroma_w),
+(1 << pix_desc->log2_chroma_h) * (double)(1 << pix_desc->log2_chroma_w),
+};
+double mse;
+int i;
+
+error_count = pix_desc->nb_components;
+
+av_log(ctx, AV_LOG_DEBUG, "PSNR values from libx264: %.3f %.3f %.3f.\n",
+ pic_out.prop.f_psnr[0], pic_out.prop.f_psnr[1], pic_out.prop.f_psnr[2]);
+
+for (i = 0; i < pix_desc->nb_components; ++i) {
+double max_value = (double)(1 << pix_desc->comp[i].depth) - 1.0;
+double plane_size = ctx->width * ctx->height / scale[i];
+
+/* psnr = -10 * log10(max_value * max_value / mse) */
+mse = (max_value * max_value) / pow(10, -pic_out.prop.f_psnr[i] / 10.0);
+
+/* SSE = MSE * width * height / scale -> because of possible chroma downsampling */
+x4->sse[i] = (int64_t)floor(mse * plane_size);
+};
+
+errors = x4->sse;
+}
+
+ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 - 1) * FF_QP2LAMBDA,
+ errors, error_count, pict_type);
+
if (wallclock)
ff_side_data_set_prft(pkt, wallclock);
}
--
2.34.1
___
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".
Re: [FFmpeg-devel] libavc/libx264: add support to propagate SSE values through encoder stats
> Hi,
> please find attached a patch to propagate the SSE for a frame into
> the
> encoder stats.
> Since libx264 already provides PSNR values, this is done by basically
> inverting the formula to recover the SSE values.
>
> Would it be possible to also append other values to the errors
> vector?
> E.g., libx264 also computes SSIM but other values could be provided.
>
> Best,
> Elias
Hi,
very minor fix to a minor patch.
I noticed I wasn't rounding values properly.
Could someone have a look, please?
Elias
NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese
di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR
i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
From 3eb487cbcaefa703aa362390ae0b90b58024a023 Mon Sep 17 00:00:00 2001
From: Elias Carotti
Date: Fri, 15 Sep 2023 20:05:43 +0200
Subject: [PATCH] Add the SSE computation for libx264.
Since libx264 only provides a per-frame per-channel PSNR, this is inverted to get back the SSE.
---
libavcodec/libx264.c | 37 -
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 77a9f173b4..4c643c9066 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -129,6 +129,8 @@ typedef struct X264Context {
int roi_warned;
int mb_info;
+
+int64_t sse[3];
} X264Context;
static void X264_log(void *p, int level, const char *fmt, va_list args)
@@ -726,7 +728,40 @@ FF_ENABLE_DEPRECATION_WARNINGS
pkt->flags |= AV_PKT_FLAG_KEY*pic_out.b_keyframe;
if (ret) {
-ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 - 1) * FF_QP2LAMBDA, NULL, 0, pict_type);
+const AVPixFmtDescriptor *pix_desc = av_pix_fmt_desc_get(csp_to_pixfmt(pic_out.img.i_csp));
+int error_count = 0;
+int64_t *errors = NULL;
+
+if (ctx->flags & AV_CODEC_FLAG_PSNR) {
+double scale[3] = { 1,
+(1 << pix_desc->log2_chroma_h) * (double)(1 << pix_desc->log2_chroma_w),
+(1 << pix_desc->log2_chroma_h) * (double)(1 << pix_desc->log2_chroma_w),
+};
+double mse;
+int i;
+
+error_count = pix_desc->nb_components;
+
+av_log(ctx, AV_LOG_DEBUG, "PSNR values from libx264: %.3f %.3f %.3f.\n",
+ pic_out.prop.f_psnr[0], pic_out.prop.f_psnr[1], pic_out.prop.f_psnr[2]);
+
+for (i = 0; i < pix_desc->nb_components; ++i) {
+double max_value = (double)(1 << pix_desc->comp[i].depth) - 1.0;
+double plane_size = ctx->width * (double)ctx->height / scale[i];
+
+/* psnr = -10 * log10(max_value * max_value / mse) */
+mse = (max_value * max_value) / pow(10, -pic_out.prop.f_psnr[i] / 10.0);
+
+/* SSE = MSE * width * height / scale -> because of possible chroma downsampling */
+x4->sse[i] = (int64_t)floor(mse * plane_size + .5);
+};
+
+errors = x4->sse;
+}
+
+ff_side_data_set_encoder_stats(pkt, (pic_out.i_qpplus1 - 1) * FF_QP2LAMBDA,
+ errors, error_count, pict_type);
+
if (wallclock)
ff_side_data_set_prft(pkt, wallclock);
}
--
2.34.1
___
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".
