[FFmpeg-devel] [PATCH v4] avfilter: add XPSNR filter

[Helmrich, Christian]

Hi and thanks for taking a look at this!

> maybe you can add a fate test

I understand my colleague (who supports me on this) right, he's still looking into how to do that.
But to have some quick progress on your other comments:

> libavfilter/vf_xpsnr.c:38:10: fatal error: internal.h: No such file or directory
>   38 | #include "internal.h"

Thanks, fixed in v4 (attached).

> the casts are unneeded

Thanks, fixed in v4.

> av_free*(NULL) is safe

Thanks, fixed in v4.

Christian Helmrich

Fraunhofer HHI


________________________________
Von: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> im Auftrag von Michael Niedermayer <michael at niedermayer.cc>
Gesendet: Freitag, 23. August 2024 22:00
An: FFmpeg development discussions and patches
Betreff: Re: [FFmpeg-devel] [PATCH v3] avfilter: add XPSNR filter

On Thu, Jul 04, 2024 at 03:50:34PM +0000, Helmrich, Christian wrote:
> This is a continuation of last year's version of this filter patch, see also
>
> https://ffmpeg.org/pipermail/ffmpeg-devel/2023-January/305517.html
>
> It includes a fix in one of the stride variables and some cleanup in order
>
> to adhere even more to the FFmpeg coding guidelines.
>
>
> Christian Helmrich
>
> Fraunhofer HHI

>  doc/filters.texi                |   68 +++
>  libavfilter/Makefile            |    1
>  libavfilter/allfilters.c        |    1
>  libavfilter/vf_xpsnr.c          |  739 ++++++++++++++++++++++++++++++++++++++++
>  libavfilter/x86/Makefile        |    1
>  libavfilter/x86/vf_xpsnr_init.c |   43 ++
>  libavfilter/xpsnr.h             |   48 ++
>  7 files changed, 901 insertions(+)

maybe you can add a fate test


> 50878de1981bb30903785175d4030e4c065c6c85  v3-0001-avfilter-add-XPSNR-filter.patch
> From 6a020fc9279ab2fd66e6dd8596f566ee6578cb35 Mon Sep 17 00:00:00 2001
> From: Christian Helmrich <christian.helmrich at hhi.fraunhofer.de>
> Date: Thu, 4 Jul 2024 17:10:29 +0200
> Subject: [PATCH v3] avfilter: add XPSNR filter
>
> Add XPSNR video filter
> Register new filter xpsnr.
[...]

> +#include "libavutil/avstring.h"
> +#include "libavutil/file_open.h"
> +#include "libavutil/mem.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "avfilter.h"
> +#include "drawutils.h"
> +#include "framesync.h"

> +#include "internal.h"

libavfilter/vf_xpsnr.c:38:10: fatal error: internal.h: No such file or directory
   38 | #include "internal.h"

[...]
> +    /* prepare XPSNR calculations: allocate temporary picture and block memory */
> +    if (s->sse_luma == NULL)
> +        s->sse_luma = (double *) av_malloc_array(w_blk * h_blk, sizeof(double));
> +    if (s->weights  == NULL)
> +        s->weights  = (double *) av_malloc_array(w_blk * h_blk, sizeof(double));

the casts are unneeded


[...]
> +    if (s->sse_luma)
> +        av_freep(&s->sse_luma);
> +    if (s->weights )
> +        av_freep(&s->weights );

av_free*(NULL) is safe

thx

[...]

--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: v4-avfilter-add-XPSNR-filter.patch
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20240826/1255d0d2/attachment.ksh>