Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-07-10 Thread Tobias Rapp

On 09.07.2017 19:42, Ashish Singh wrote:

Hi, added metadata scores and changed multipe string comparisons to few integer
comparisons.

---
 Changelog|   1 +
 doc/filters.texi |  20 +++
 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/ansnr.h  |  29 
 libavfilter/vf_ansnr.c   | 416 +++
 6 files changed, 468 insertions(+)
 create mode 100644 libavfilter/ansnr.h
 create mode 100644 libavfilter/vf_ansnr.c

diff --git a/Changelog b/Changelog
index 1778980..bfe848a 100644
--- a/Changelog
+++ b/Changelog
@@ -10,6 +10,7 @@ version :
 - config.log and other configuration files moved into ffbuild/ directory
 - update cuvid/nvenc headers to Video Codec SDK 8.0.14
 - afir audio filter
+- ansnr video filter

 version 3.3:
 - CrystalHD decoder moved to new decode API
diff --git a/doc/filters.texi b/doc/filters.texi
index 5985db6..7a0856b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -4419,6 +4419,26 @@ input reaches end of stream. This will cause problems if 
your encoding
 pipeline drops frames. If you're trying to apply an image as an
 overlay to a video stream, consider the @var{overlay} filter instead.

+@section ansnr
+
+Obtain the average ANSNR (Anti-Noise Signal to Noise
+Ratio) between two input videos.
+
+This filter takes in input two input videos.
+
+Both video inputs must have the same resolution and pixel format for
+this filter to work correctly. Also it assumes that both inputs
+have the same number of frames, which are compared one by one.
+
+The obtained average ANSNR is printed through the logging system.
+
+In the below example the input file @file{main.mpg} being processed is compared
+with the reference file @file{ref.mpg}.
+
+@example
+ffmpeg -i main.mpg -i ref.mpg -lavfi ansnr -f null -
+@end example
+
 @section ass

 Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index f7dfe8a..705e5a1 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -124,6 +124,7 @@ OBJS-$(CONFIG_ANULLSINK_FILTER)  += 
asink_anullsink.o
 # video filters
 OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
 OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
+OBJS-$(CONFIG_ANSNR_FILTER)  += vf_ansnr.o dualinput.o 
framesync.o
 OBJS-$(CONFIG_ASS_FILTER)+= vf_subtitles.o
 OBJS-$(CONFIG_ATADENOISE_FILTER) += vf_atadenoise.o
 OBJS-$(CONFIG_AVGBLUR_FILTER)+= vf_avgblur.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index cd35ae4..c1f67c4 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -136,6 +136,7 @@ static void register_all(void)

 REGISTER_FILTER(ALPHAEXTRACT,   alphaextract,   vf);
 REGISTER_FILTER(ALPHAMERGE, alphamerge, vf);
+REGISTER_FILTER(ANSNR,  ansnr,  vf);
 REGISTER_FILTER(ASS,ass,vf);
 REGISTER_FILTER(ATADENOISE, atadenoise, vf);
 REGISTER_FILTER(AVGBLUR,avgblur,vf);
diff --git a/libavfilter/ansnr.h b/libavfilter/ansnr.h
new file mode 100644
index 000..44fb3ba
--- /dev/null
+++ b/libavfilter/ansnr.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2017 Ronald S. Bultje 
+ * Copyright (c) 2017 Ashish Pratap Singh 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVFILTER_ANSNR_H
+#define AVFILTER_ANSNR_H
+
+static int compute_ansnr(const void *ref, const void *dis, int w,
+ int h, int ref_stride, int dis_stride, double *score,
+ double *score_psnr, double peak, double psnr_max, 
void *ctx);
+
+#endif /* AVFILTER_ANSNR_H */
diff --git a/libavfilter/vf_ansnr.c b/libavfilter/vf_ansnr.c
new file mode 100644
index 000..78c71e1
--- /dev/null
+++ b/libavfilter/vf_ansnr.c
@@ -0,0 +1,416 @@
+/*
+ * Copyright (c) 2017 Ronald S. Bultje 
+ * Copyright (c) 2017 Ashish Pratap Singh 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU 

Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-07-04 Thread Tobias Rapp

On 03.07.2017 18:01, Ashish Singh wrote:

Added ansnr section in doc/filters.texi and changelog and fixed issues.

---
 Changelog|   1 +
 doc/filters.texi |  24 +++
 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/ansnr.h  |  29 
 libavfilter/vf_ansnr.c   | 403 +++
 6 files changed, 459 insertions(+)
 create mode 100644 libavfilter/ansnr.h
 create mode 100644 libavfilter/vf_ansnr.c

diff --git a/Changelog b/Changelog
index 1778980..bfe848a 100644
--- a/Changelog
+++ b/Changelog
@@ -10,6 +10,7 @@ version :
 - config.log and other configuration files moved into ffbuild/ directory
 - update cuvid/nvenc headers to Video Codec SDK 8.0.14
 - afir audio filter
+- ansnr video filter

 version 3.3:
 - CrystalHD decoder moved to new decode API
diff --git a/doc/filters.texi b/doc/filters.texi
index 5985db6..a8068df 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -4419,6 +4419,30 @@ input reaches end of stream. This will cause problems if 
your encoding
 pipeline drops frames. If you're trying to apply an image as an
 overlay to a video stream, consider the @var{overlay} filter instead.

+@section ansnr
+
+Obtain the average ANSNR (Anti-Noise Signal to Noise
+Ratio) between two input videos.
+
+This filter takes in input two input videos, the first input is
+considered the "main" source and is passed unchanged to the
+output. The second input is used as a "reference" video for computing
+the ANSNR.
+
+Both video inputs must have the same resolution and pixel format for
+this filter to work correctly. Also it assumes that both inputs
+have the same number of frames, which are compared one by one.
+
+The obtained average ANSNR is printed through the logging system.
+
+For example:
+@example
+ffmpeg -i main.mpg -i ref.mpg -lavfi ansnr -f null -
+@end example
+
+On this example the input file @file{main.mpg} being processed is compared 
with the
+reference file @file{ref.mpg}.
+
 @section ass

 Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index f7dfe8a..705e5a1 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -124,6 +124,7 @@ OBJS-$(CONFIG_ANULLSINK_FILTER)  += 
asink_anullsink.o
 # video filters
 OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
 OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
+OBJS-$(CONFIG_ANSNR_FILTER)  += vf_ansnr.o dualinput.o 
framesync.o
 OBJS-$(CONFIG_ASS_FILTER)+= vf_subtitles.o
 OBJS-$(CONFIG_ATADENOISE_FILTER) += vf_atadenoise.o
 OBJS-$(CONFIG_AVGBLUR_FILTER)+= vf_avgblur.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index cd35ae4..c1f67c4 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -136,6 +136,7 @@ static void register_all(void)

 REGISTER_FILTER(ALPHAEXTRACT,   alphaextract,   vf);
 REGISTER_FILTER(ALPHAMERGE, alphamerge, vf);
+REGISTER_FILTER(ANSNR,  ansnr,  vf);
 REGISTER_FILTER(ASS,ass,vf);
 REGISTER_FILTER(ATADENOISE, atadenoise, vf);
 REGISTER_FILTER(AVGBLUR,avgblur,vf);
diff --git a/libavfilter/ansnr.h b/libavfilter/ansnr.h
new file mode 100644
index 000..44fb3ba
--- /dev/null
+++ b/libavfilter/ansnr.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2017 Ronald S. Bultje 
+ * Copyright (c) 2017 Ashish Pratap Singh 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVFILTER_ANSNR_H
+#define AVFILTER_ANSNR_H
+
+static int compute_ansnr(const void *ref, const void *dis, int w,
+ int h, int ref_stride, int dis_stride, double *score,
+ double *score_psnr, double peak, double psnr_max, 
void *ctx);
+
+#endif /* AVFILTER_ANSNR_H */
diff --git a/libavfilter/vf_ansnr.c b/libavfilter/vf_ansnr.c
new file mode 100644
index 000..88d33b2
--- /dev/null
+++ b/libavfilter/vf_ansnr.c
@@ -0,0 +1,403 @@
+/*
+ * Copyright (c) 2017 Ronald S. Bultje 
+ * Copyright (c) 2017 Ashish Pratap Singh 

Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-07-03 Thread Moritz Barsnick
On Mon, Jul 03, 2017 at 18:39:38 +0530, Ashish Singh wrote:
> This is ANSNR filter, one of the component filters of VMAF.

Would you please expand the abbreviation in at least one place?
Optimally in doc/filters.texi, the content of which is missing (as it
still is in your VMAF patch). (And perhaps in the header of the .c
file.)

> +++ b/libavfilter/ansnr.h
[...]
> +#ifndef AVFILTER_PSNR_H
> +#define AVFILTER_PSNR_H

Incorrect header guard.

> +const float ansnr_filter2d_dis[5 * 5] = {
> +2.0 / 571.0,  7.0 / 571.0,  12.0 / 571.0,  7.0 / 571.0,  2.0 / 571.0,
> +7.0 / 571.0,  31.0 / 571.0, 52.0 / 571.0,  31.0 / 571.0, 7.0 / 571.0,
> +12.0 / 571.0, 52.0 / 571.0, 127.0 / 571.0, 52.0 / 571.0, 12.0 / 571.0,
> +7.0 / 571.0,  31.0 / 571.0, 52.0 / 571.0,  31.0 / 571.0, 7.0 / 571.0,
> +2.0 / 571.0,  7.0 / 571.0,  12.0 / 571.0,  7.0 / 571.0,  2.0 / 571.0

If you can align these along the decimal dots, it's easier to read.

> +for (i = 0; i < h; ++i) {
> +for (j = 0; j < w; ++j) {

I believe ffmpeg style prefers "i++".

> +signal_sum   += pow_2(ref[ref_ind]);
> +noise_sum += pow_2(ref[ref_ind] - dis[dis_ind]);

Whitespace.

> +*score = (noise==0) ? (psnr_max) : (10.0 * log10(signal / noise));
> +
> +*score_psnr = FFMIN(10 * log10(pow_2(peak) * w * h / FFMAX(noise, eps)),
> +psnr_max);

Inconsistent: Why 10.0 in the first case, 10 in the second?

> +double score = 0.0;
> +double score_psnr = 0;

Inconsistent: Why 0.0 in the first case, 0 in the second?

Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-04-03 Thread Ronald S. Bultje
Hi Betty,

On Mon, Apr 3, 2017 at 11:00 AM, Betty Wu  wrote:

> +typedef double number_t;
>

Why?

+static int ansnr_filter(const uint8_t* src_image, number_t* dst_image, int
> img_row, int img_col, const double *filter, int stride)
>
[..]

> +number_t **imme_image;
> +if(!(imme_image = (number_t **)av_malloc((size_t)imme_row *
> sizeof(number_t *
> +return AVERROR(ENOMEM);
> +
> +for(int i = 0; i < imme_row; i++) {
> +if(!(imme_image[i] = (number_t *)av_malloc((size_t)imme_col *
> sizeof(number_t
> +return AVERROR(ENOMEM);
> +}
>

In ffmpeg, we typically don't allocate lines and cols in separate arrays.
You can just use a 1D array and use y*stride+x for indexing, which is what
we do elsewhere.

These arrays should also be initialized once in the init function and then
reuse, instead of being re-allocated for each frame.

+number_t *anc_after;
> +if(!(anc_after = (number_t *)av_malloc((size_t)row * col *
> sizeof(number_t {
> +av_free(anc_after);
> +return AVERROR(ENOMEM);
> +}
>

Why free if allocation failed? And this also should be allocated once
during init, not re-allocated for each frame.


> +static int query_formats(AVFilterContext *ctx)
> +{
> +static const enum AVPixelFormat pix_fmts[] = {
> +AV_PIX_FMT_GRAY8, AV_PIX_FMT_GRAY16,
> +#define PF_NOALPHA(suf) AV_PIX_FMT_YUV420##suf,  AV_PIX_FMT_YUV422##suf,
> AV_PIX_FMT_YUV444##suf
> +#define PF_ALPHA(suf)   AV_PIX_FMT_YUVA420##suf, AV_PIX_FMT_YUVA422##suf,
> AV_PIX_FMT_YUVA444##suf
> +#define PF(suf) PF_NOALPHA(suf), PF_ALPHA(suf)
> +PF(P), PF(P9), PF(P10), PF_NOALPHA(P12), PF_NOALPHA(P14), PF(P16),
> +AV_PIX_FMT_YUV440P, AV_PIX_FMT_YUV411P, AV_PIX_FMT_YUV410P,
> +AV_PIX_FMT_YUVJ411P, AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P,
> +AV_PIX_FMT_YUVJ440P, AV_PIX_FMT_YUVJ444P,
> +AV_PIX_FMT_GBRP, AV_PIX_FMT_GBRP9, AV_PIX_FMT_GBRP10,
> +AV_PIX_FMT_GBRP12, AV_PIX_FMT_GBRP14, AV_PIX_FMT_GBRP16,
> +AV_PIX_FMT_GBRAP, AV_PIX_FMT_GBRAP16,
> +AV_PIX_FMT_NONE
> +};
>

Have all of these been tested? Since ANSNR is Y-only, it doesn't work on
GBR. Likewise, I believe the code is 8-bit only so it shouldn't work on
gray16 or any of the P9/10/12/14/16 formats.

For SIMD purposes, I would probably encourage you to work in fixed-point
integer. Maybe for the qualification task we can skip that (I hadn't really
thought about it), but I think for the final implementation, fixed-point
will be much faster, and speed is a significant goal here. An interesting
idea for speed is to make the filter decomposable (i.e. split the
horizontal/vertical pass) if that's possible. At least for the 3-tap
filter, that should be trivial (it's just a 121 lowpass in both
directions), and should give some speed gains because you go from n^2 to 2n
+ an extra load/store pair in terms of complexity (whether it's actually
faster remains to be proven). The other filter should also be decomposable
but I don't see from the top of my head what it decomposes into and I don't
feel like writing a script to figure it out. :-).

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-04-03 Thread Steven Liu
2017-04-03 23:00 GMT+08:00 Betty Wu :

> A new filter ANSNR is added. libavfilter/Makefile is changed.
> Run 'ffmpeg -i input1 -i input2 -lavfi ansnr -f null -' to get an overall
> score while per-frame value is stored but not printed.
> This implementation is for constructing the vmaf filter later since ANSNR
> is one of individual tools used in vmaf.
>
> Signed-off-by: Betty Wu 
> ---
>  libavfilter/Makefile |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/vf_ansnr.c   | 425 ++
> +
>  3 files changed, 427 insertions(+)
>  create mode 100644 libavfilter/vf_ansnr.c
>
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 9c15ed62d2..5416e0f34f 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -123,6 +123,7 @@ OBJS-$(CONFIG_ANULLSINK_FILTER)  +=
> asink_anullsink.o
>  # video filters
>  OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
>  OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
> +OBJS-$(CONFIG_ANSNR_FILTER)  += vf_ansnr.o dualinput.o
> framesync.o
>  OBJS-$(CONFIG_ASS_FILTER)+= vf_subtitles.o
>  OBJS-$(CONFIG_ATADENOISE_FILTER) += vf_atadenoise.o
>  OBJS-$(CONFIG_AVGBLUR_FILTER)+= vf_avgblur.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 64b634e8f3..fbd9ec026e 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -134,6 +134,7 @@ static void register_all(void)
>
>  REGISTER_FILTER(ALPHAEXTRACT,   alphaextract,   vf);
>  REGISTER_FILTER(ALPHAMERGE, alphamerge, vf);
> +REGISTER_FILTER(ANSNR,  ansnr,  vf);
>  REGISTER_FILTER(ASS,ass,vf);
>  REGISTER_FILTER(ATADENOISE, atadenoise, vf);
>  REGISTER_FILTER(AVGBLUR,avgblur,vf);
> diff --git a/libavfilter/vf_ansnr.c b/libavfilter/vf_ansnr.c
> new file mode 100644
> index 00..cfea6efbd4
> --- /dev/null
> +++ b/libavfilter/vf_ansnr.c
> @@ -0,0 +1,425 @@
> +/*
> + * Copyright (c) 2017 Betty Wu
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +/*
> + * @file
> + * Caculate the ANSNR between two input videos.
> + * @author Betty Wu
> + */
> +
> +#include "libavutil/avstring.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "avfilter.h"
> +#include "dualinput.h"
> +#include "drawutils.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +#define OPT_RANGE_PIXEL_OFFSET -128
> +
> +typedef double number_t;
> +
> +typedef struct ANSNRContext {
> +const AVClass *class;
> +FFDualInputContext dinput;
> +uint64_t nb_frames;
> +FILE *stats_file;
> +char *stats_file_str;
> +int stats_version;
> +int stats_header_written;
> +int stats_add_max;
> +int is_rgb;
> +uint8_t rgba_map[4];
> +double score;
> +double score_total;
> +char comps[4];
> +int nb_components;
> +int planewidth[4];
> +int planeheight[4];
> +
> +} ANSNRContext;
> +#define OFFSET(x) offsetof(ANSNRContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +
> +static const AVOption ansnr_options[] = {
> +{"stats_file", "Set file where to store per-frame difference
> information", OFFSET(stats_file_str), AV_OPT_TYPE_STRING, {.str=NULL}, 0,
> 0, FLAGS },
> +{"f",  "Set file where to store per-frame difference
> information", OFFSET(stats_file_str), AV_OPT_TYPE_STRING, {.str=NULL}, 0,
> 0, FLAGS },
> +{ NULL }
> +};
>
Document please.

> +AVFILTER_DEFINE_CLASS(ansnr);
> +
> +const int ans_filter3_stride = 3;
> +const int ans_filter5_stride = 5;
> +
> +const double ans_filter3[ans_filter3_stride*ans_filter3_stride] = {
> +1.0 / 16.0, 2.0 / 16.0, 1.0 / 16.0,
> +2.0 / 16.0, 4.0 / 16.0, 2.0 / 16.0,
> +1.0 / 16.0, 2.0 / 16.0, 1.0 / 16.0
> +};
> +
> +const double ans_filter5[ans_filter5_stride*ans_filter5_stride] = {
> +2.0 / 571.0,  7.0 / 571.0,  12.0 / 571.0,  7.0 / 571.0,  2.0 / 571.0,
> +7.0 / 571.0,  31.0 / 571.0, 52.0 / 571.0,  31.0 / 571.0, 7.0 / 571.0,
> +12.0 / 571.0, 52.0 

Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-04-01 Thread Moritz Barsnick
On Thu, Mar 30, 2017 at 23:06:36 +0200, Betty Wu wrote:
> A new filter ANSNR is added. libavfilter/Makefile is changed.

That line isn't really needed in a commit message (too obvious).

> +/*
> + * Copyright 2016-2017 Netflix, Inc.
> + * Copyright (c) 2017 Betty Wu
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.

Are you the copyright holder of this code? It looks very similar to
https://github.com/Netflix/vmaf/blob/master/feature/src/ansnr.c and its
dependencies, which are licensed Apache 2.0. IINAL, but you can't
"just" change the license, unless given permission to do so.

> +/* Whether to use border replication instead of zero extension. */
> +/* #define ANSNR_OPT_BORDER_REPLICATE */
> +
> +/* Whether to save intermediate results to files. */
> +/* #define ANSNR_OPT_DEBUG_DUMP */
> +
> +/* Whether to use a 1-D approximation of filters. */
> +/* #define ANSNR_OPT_FILTER_1D */
> +
> +/* Whether to normalize result by dividing against maximum ANSNR. */
> +/* #define ANSNR_OPT_NORMALIZE */
> +
> +/* Whether to use single precision for computation. */
> +#define ANSNR_OPT_SINGLE_PRECISION
> +//#define ANSNR_OPT_DOUBLE_PRECISION

This produces a lot of dead code. Should these not be run-time options,
so that the user can make use of all the code? (Single vs. double
precision could possibly be maintained as two separate filters in one
source file, instead of a run-time option, if there's any reasoning for
keeping both.)

> +#endif /* ANSNR_OPTIONS_H_ */

This shows that this is a verbatim copy of ansnr_options.h from the
named link, and that you don't know what these #define guards are good
for. ;-)


Just some random comments to the source code, I didn't try to
understand the functionality:

> +if (ii < 0) ii = -ii;

FFABS()

> +imgcoeff = (float)src[ii * src_px_stride + 
> jj]+OPT_RANGE_PIXEL_OFFSET;

Please leave spaces around operators for readability.

> +if (SIZE_MAX / buf_sz_one < 3)
> +{
> +goto fail;
> +}

ffmpeg's bracket style is different (and some say they could be omitted
here, some say not).

> +ref_filtr = (number_t *)data_top; data_top += buf_sz_one;

I believe it is preferred not to combine two statements onto one line.


> +if( !strcmp(frame_format,"yuv420p") || !strcmp(frame_format,"yuv422p") 
> || !strcmp(frame_format,"yuv444p"))
> +psnr_max=60;
> +if( !strcmp(frame_format,"yuv420p10le") || 
> !strcmp(frame_format,"yuv422p10le") || !strcmp(frame_format,"yuv444p10le"))
> +psnr_max=72;

Bracket style.

> +s->score=score;

Whitespace.

> +//if (ARCH_X86)
> +//ff_ansnr_init_x86(>dsp, desc->comp[0].depth);

Unused code shouldn't go in.

Another observation:
Is stats_version initialized anywhere?

Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-03-30 Thread Steven Liu
2017-03-31 5:06 GMT+08:00 Betty Wu :

> A new filter ANSNR is added. libavfilter/Makefile is changed.
> Run 'ffmpeg -i input1 -i input2 -lavfi ansnr -f null -' to get an overall
> score while per-frame value is stored but not printed.
> This implementation is for constructing the vmaf filter later since ANSNR
> is one of individual tools used in vmaf.
>
> Signed-off-by: Betty Wu 
> ---
>  libavfilter/Makefile |   1 +
>  libavfilter/allfilters.c |   1 +
>  libavfilter/vf_ansnr.c   | 727 ++
> +
>  3 files changed, 729 insertions(+)
>  create mode 100644 libavfilter/vf_ansnr.c
>

need add options describes into document doc/filters.texi

>
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 9c15ed62d2..5416e0f34f 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -123,6 +123,7 @@ OBJS-$(CONFIG_ANULLSINK_FILTER)  +=
> asink_anullsink.o
>  # video filters
>  OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
>  OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
> +OBJS-$(CONFIG_ANSNR_FILTER)  += vf_ansnr.o dualinput.o
> framesync.o
>  OBJS-$(CONFIG_ASS_FILTER)+= vf_subtitles.o
>  OBJS-$(CONFIG_ATADENOISE_FILTER) += vf_atadenoise.o
>  OBJS-$(CONFIG_AVGBLUR_FILTER)+= vf_avgblur.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 64b634e8f3..fbd9ec026e 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -134,6 +134,7 @@ static void register_all(void)
>
>  REGISTER_FILTER(ALPHAEXTRACT,   alphaextract,   vf);
>  REGISTER_FILTER(ALPHAMERGE, alphamerge, vf);
> +REGISTER_FILTER(ANSNR,  ansnr,  vf);
>  REGISTER_FILTER(ASS,ass,vf);
>  REGISTER_FILTER(ATADENOISE, atadenoise, vf);
>  REGISTER_FILTER(AVGBLUR,avgblur,vf);
> diff --git a/libavfilter/vf_ansnr.c b/libavfilter/vf_ansnr.c
> new file mode 100644
> index 00..3c53b726d2
> --- /dev/null
> +++ b/libavfilter/vf_ansnr.c
> @@ -0,0 +1,727 @@
> +/*
> + * Copyright 2016-2017 Netflix, Inc.
> + * Copyright (c) 2017 Betty Wu
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +/*
> + * @file
> + * Caculate the ANSNR between two input videos.
> + * @author Betty Wu
> + */
> +
> +#include "libavutil/avstring.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "avfilter.h"
> +#include "dualinput.h"
> +#include "drawutils.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +#define MAX_ALIGN 32
> +#define ALIGN_CEIL(x) ((x) + ((x) % MAX_ALIGN ? MAX_ALIGN - (x) %
> MAX_ALIGN : 0))
> +#define OPT_RANGE_PIXEL_OFFSET -128
> +
> +#ifndef ANSNR_OPTIONS_H_
> +#define ANSNR_OPTIONS_H_
> +
> +/* Whether to use border replication instead of zero extension. */
> +/* #define ANSNR_OPT_BORDER_REPLICATE */
> +
> +/* Whether to save intermediate results to files. */
> +/* #define ANSNR_OPT_DEBUG_DUMP */
> +
> +/* Whether to use a 1-D approximation of filters. */
> +/* #define ANSNR_OPT_FILTER_1D */
> +
> +/* Whether to normalize result by dividing against maximum ANSNR. */
> +/* #define ANSNR_OPT_NORMALIZE */
> +
> +/* Whether to use single precision for computation. */
> +#define ANSNR_OPT_SINGLE_PRECISION
> +//#define ANSNR_OPT_DOUBLE_PRECISION
> +
> +#endif /* ANSNR_OPTIONS_H_ */
> +
> +#ifndef ANSNR_TOOLS_H_
> +#define ANSNR_TOOLS_H_
> +#endif
> +
> +#ifdef ANSNR_OPT_SINGLE_PRECISION
> +  typedef float number_t;
> +  #define read_image_b   read_image_b2s
> +  #define read_image_w   read_image_w2s
> +  #define ansnr_filter1d_ref ansnr_filter1d_ref_s
> +  #define ansnr_filter1d_dis ansnr_filter1d_dis_s
> +  #define ansnr_filter2d_ref ansnr_filter2d_ref_s
> +  #define ansnr_filter2d_dis ansnr_filter2d_dis_s
> +  #define ansnr_filter1d ansnr_filter1d_s
> +  #define ansnr_filter2d ansnr_filter2d_s
> +  #define ansnr_mse  ansnr_mse_s
> +#else
> +  typedef double number_t;
> +  #define read_image_b   read_image_b2d
> +  #define read_image_w   read_image_w2d
> +  #define 

Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-03-30 Thread Michael Niedermayer
On Thu, Mar 30, 2017 at 11:06:36PM +0200, Betty Wu wrote:
> A new filter ANSNR is added. libavfilter/Makefile is changed.
> Run 'ffmpeg -i input1 -i input2 -lavfi ansnr -f null -' to get an overall 
> score while per-frame value is stored but not printed.
> This implementation is for constructing the vmaf filter later since ANSNR is 
> one of individual tools used in vmaf.
> 
> Signed-off-by: Betty Wu 
> ---
>

[...]

> +static void *aligned_malloc(size_t size, size_t alignment)
> +{
> +void *ptr;
> +
> +if (posix_memalign(, alignment, size))
> +return 0;
> +else
> +return ptr;
> +}

please use av_malloc()

posix_memalign is not portable


> +
> +static void aligned_free(void *ptr)
> +{
> +free(ptr);
> +}

use av_freep()

also please see tools/patcheck


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

2017-03-30 Thread Steven Liu
2017-03-30 22:44 GMT+08:00 Betty Wu :

> A new filter ANSNR is added. Makefile is changed.
> Run like 'ffmpeg -i input1 -i input2 -lavfi ansnr -f null -'.
> The implementation is for further constructing vmaf filter since ANSNR is
> one of individual tools used in vmaf.
>
> Signed-off-by: Betty Wu 
> ---
>  libavfilter/Makefile | 1 +
>  libavfilter/allfilters.c | 1 +
>  2 files changed, 2 insertions(+)
>
maybe you forgot add the filter source file?
use git add before commit

>
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 9c15ed62d2..5416e0f34f 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -123,6 +123,7 @@ OBJS-$(CONFIG_ANULLSINK_FILTER)  +=
> asink_anullsink.o
>  # video filters
>  OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
>  OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
> +OBJS-$(CONFIG_ANSNR_FILTER)  += vf_ansnr.o dualinput.o
> framesync.o
>  OBJS-$(CONFIG_ASS_FILTER)+= vf_subtitles.o
>  OBJS-$(CONFIG_ATADENOISE_FILTER) += vf_atadenoise.o
>  OBJS-$(CONFIG_AVGBLUR_FILTER)+= vf_avgblur.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index 64b634e8f3..fbd9ec026e 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -134,6 +134,7 @@ static void register_all(void)
>
>  REGISTER_FILTER(ALPHAEXTRACT,   alphaextract,   vf);
>  REGISTER_FILTER(ALPHAMERGE, alphamerge, vf);
> +REGISTER_FILTER(ANSNR,  ansnr,  vf);
>  REGISTER_FILTER(ASS,ass,vf);
>  REGISTER_FILTER(ATADENOISE, atadenoise, vf);
>  REGISTER_FILTER(AVGBLUR,avgblur,vf);
> --
> 2.12.1
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel