diff mbox

[FFmpeg-devel,10/16] lavfi/vf_libvmaf: convert to framesync2.

Message ID 20170810114642.26779-10-george@nsup.org
State New
Headers show

Commit Message

Nicolas George Aug. 10, 2017, 11:46 a.m. UTC 
After this commit, the code compiles, but on my setup it
segfaults before and after. It also prints the very worrying
warning:

src/libavfilter/vf_libvmaf.c:161:66: warning: passing argument 4 of ‘compute_vmaf’ from incompatible pointer type [-Wincompatible-pointer-types]
/tmp/i/include/libvmaf.h:26:8: note: expected ‘int (*)(float *, float *, float *, int,  void *)’ but argument is of type ‘int (*)(float *, float *, float *, int,  double *, void *)’

==12116== Thread 6:
==12116== Conditional jump or move depends on uninitialised value(s)
==12116==    at 0x526D432: cons_ (ocval.h:1188)
==12116==    by 0x526D432: GenericIT (ocval.h:1119)
==12116==    by 0x526D432: OC::TranslateForNumPyClassesToArray(OC::Val&) (pickleloader.h:92)
==12116==    by 0x5211F5D: loads (pickleloader.h:566)
==12116==    by 0x5211F5D: LoadValFromArray (chooseser.h:290)
==12116==    by 0x5211F5D: LoadValFromFile (chooseser.h:405)
==12116==    by 0x5211F5D: _read_and_assert_model(char const*, OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&) (vmaf.cpp:77)
==12116==    by 0x5212B0F: VmafRunner::run(Asset, int (*)(float*, float*, float*, int, void*), void*, bool, bool, bool, bool, bool) (vmaf.cpp:149)
==12116==    by 0x52165B6: RunVmaf(char const*, int, int, int (*)(float*, float*, float*, int, void*), void*, char const*, char const*, char const*, bool, bool, bool, bool, bool, char const*) (vmaf.cpp:645)
==12116==    by 0x518AFFF: compute_vmaf_score (vf_libvmaf.c:161)
==12116==    by 0x518AFFF: call_vmaf (vf_libvmaf.c:170)
==12116==    by 0x7967493: start_thread (pthread_create.c:333)
==12116==    by 0x7F69A8E: clone (clone.S:97)
==12116==
==12116== Conditional jump or move depends on uninitialised value(s)
==12116==    at 0x526D432: cons_ (ocval.h:1188)
==12116==    by 0x526D432: GenericIT (ocval.h:1119)
==12116==    by 0x526D432: OC::TranslateForNumPyClassesToArray(OC::Val&) (pickleloader.h:92)
==12116==    by 0x526D50D: OC::TranslateForNumPyClassesToArray(OC::Val&) (pickleloader.h:94)
==12116==    by 0x5211F5D: loads (pickleloader.h:566)
==12116==    by 0x5211F5D: LoadValFromArray (chooseser.h:290)
==12116==    by 0x5211F5D: LoadValFromFile (chooseser.h:405)
==12116==    by 0x5211F5D: _read_and_assert_model(char const*, OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&) (vmaf.cpp:77)
==12116==    by 0x5212B0F: VmafRunner::run(Asset, int (*)(float*, float*, float*, int, void*), void*, bool, bool, bool, bool, bool) (vmaf.cpp:149)
==12116==    by 0x52165B6: RunVmaf(char const*, int, int, int (*)(float*, float*, float*, int, void*), void*, char const*, char const*, char const*, bool, bool, bool, bool, bool, char const*) (vmaf.cpp:645)
==12116==    by 0x518AFFF: compute_vmaf_score (vf_libvmaf.c:161)
==12116==    by 0x518AFFF: call_vmaf (vf_libvmaf.c:170)
==12116==    by 0x7967493: start_thread (pthread_create.c:333)
==12116==    by 0x7F69A8E: clone (clone.S:97)
==12116==
==12116== Conditional jump or move depends on uninitialised value(s)
==12116==    at 0x526D432: cons_ (ocval.h:1188)
==12116==    by 0x526D432: GenericIT (ocval.h:1119)
==12116==    by 0x526D432: OC::TranslateForNumPyClassesToArray(OC::Val&) (pickleloader.h:92)
==12116==    by 0x526D50D: OC::TranslateForNumPyClassesToArray(OC::Val&) (pickleloader.h:94)
==12116==    by 0x526D50D: OC::TranslateForNumPyClassesToArray(OC::Val&) (pickleloader.h:94)
==12116==    by 0x5211F5D: loads (pickleloader.h:566)
==12116==    by 0x5211F5D: LoadValFromArray (chooseser.h:290)
==12116==    by 0x5211F5D: LoadValFromFile (chooseser.h:405)
==12116==    by 0x5211F5D: _read_and_assert_model(char const*, OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&) (vmaf.cpp:77)
==12116==    by 0x5212B0F: VmafRunner::run(Asset, int (*)(float*, float*, float*, int, void*), void*, bool, bool, bool, bool, bool) (vmaf.cpp:149)
==12116==    by 0x52165B6: RunVmaf(char const*, int, int, int (*)(float*, float*, float*, int, void*), void*, char const*, char const*, char const*, bool, bool, bool, bool, bool, char const*) (vmaf.cpp:645)
==12116==    by 0x518AFFF: compute_vmaf_score (vf_libvmaf.c:161)
==12116==    by 0x518AFFF: call_vmaf (vf_libvmaf.c:170)
==12116==    by 0x7967493: start_thread (pthread_create.c:333)
==12116==    by 0x7F69A8E: clone (clone.S:97)
==12116==
==12116== Use of uninitialised value of size 8
==12116==    at 0x518AC79: read_frame_8bit (vf_libvmaf.c:147)
==12116==    by 0x52AB5E8: combo (combo.c:149)
==12116==    by 0x5212E95: VmafRunner::run(Asset, int (*)(float*, float*, float*, int, void*), void*, bool, bool, bool, bool, bool) (vmaf.cpp:278)
==12116==    by 0x52165B6: RunVmaf(char const*, int, int, int (*)(float*, float*, float*, int, void*), void*, char const*, char const*, char const*, bool, bool, bool, bool, bool, char const*) (vmaf.cpp:645)
==12116==    by 0x518AFFF: compute_vmaf_score (vf_libvmaf.c:161)
==12116==    by 0x518AFFF: call_vmaf (vf_libvmaf.c:170)
==12116==    by 0x7967493: start_thread (pthread_create.c:333)
==12116==    by 0x7F69A8E: clone (clone.S:97)
==12116==
==12116== Invalid read of size 4
==12116==    at 0x518AC79: read_frame_8bit (vf_libvmaf.c:147)
==12116==    by 0x52AB5E8: combo (combo.c:149)
==12116==    by 0x5212E95: VmafRunner::run(Asset, int (*)(float*, float*, float*, int, void*), void*, bool, bool, bool, bool, bool) (vmaf.cpp:278)
==12116==    by 0x52165B6: RunVmaf(char const*, int, int, int (*)(float*, float*, float*, int, void*), void*, char const*, char const*, char const*, bool, bool, bool, bool, bool, char const*) (vmaf.cpp:645)
==12116==    by 0x518AFFF: compute_vmaf_score (vf_libvmaf.c:161)
==12116==    by 0x518AFFF: call_vmaf (vf_libvmaf.c:170)
==12116==    by 0x7967493: start_thread (pthread_create.c:333)
==12116==    by 0x7F69A8E: clone (clone.S:97)
==12116==  Address 0x40 is not stack'd, malloc'd or (recently) free'd
==12116==
==12116==
==12116== Process terminating with default action of signal 11 (SIGSEGV)
==12116==  Access not within mapped region at address 0x40
==12116==    at 0x518AC79: read_frame_8bit (vf_libvmaf.c:147)
==12116==    by 0x52AB5E8: combo (combo.c:149)
==12116==    by 0x5212E95: VmafRunner::run(Asset, int (*)(float*, float*, float*, int, void*), void*, bool, bool, bool, bool, bool) (vmaf.cpp:278)
==12116==    by 0x52165B6: RunVmaf(char const*, int, int, int (*)(float*, float*, float*, int, void*), void*, char const*, char const*, char const*, bool, bool, bool, bool, bool, char const*) (vmaf.cpp:645)
==12116==    by 0x518AFFF: compute_vmaf_score (vf_libvmaf.c:161)
==12116==    by 0x518AFFF: call_vmaf (vf_libvmaf.c:170)
==12116==    by 0x7967493: start_thread (pthread_create.c:333)
==12116==    by 0x7F69A8E: clone (clone.S:97)

Signed-off-by: Nicolas George <george@nsup.org>
---
 libavfilter/Makefile     |  2 +-
 libavfilter/vf_libvmaf.c | 45 +++++++++++++++++++++++++--------------------
 2 files changed, 26 insertions(+), 21 deletions(-)


Unchanged. Will remove the backtrace if confirmed to work.

Comments

Nicolas George Aug. 20, 2017, 9:52 a.m. UTC | #1 
Le tridi 23 thermidor, an CCXXV, Nicolas George a écrit :
> After this commit, the code compiles, but on my setup it
> segfaults before and after. It also prints the very worrying
> warning:
> 
> src/libavfilter/vf_libvmaf.c:161:66: warning: passing argument 4 of ‘compute_vmaf’ from incompatible pointer type [-Wincompatible-pointer-types]
> /tmp/i/include/libvmaf.h:26:8: note: expected ‘int (*)(float *, float *, float *, int,  void *)’ but argument is of type ‘int (*)(float *, float *, float *, int,  double *, void *)’
> 
> ==12116== Thread 6:
> ==12116== Conditional jump or move depends on uninitialised value(s)
> ==12116==    at 0x526D432: cons_ (ocval.h:1188)
> ==12116==    by 0x526D432: GenericIT (ocval.h:1119)
> ==12116==    by 0x526D432: OC::TranslateForNumPyClassesToArray(OC::Val&) (pickleloader.h:92)
> ==12116==    by 0x5211F5D: loads (pickleloader.h:566)
> ==12116==    by 0x5211F5D: LoadValFromArray (chooseser.h:290)
> ==12116==    by 0x5211F5D: LoadValFromFile (chooseser.h:405)
> ==12116==    by 0x5211F5D: _read_and_assert_model(char const*, OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&) (vmaf.cpp:77)
> ==12116==    by 0x5212B0F: VmafRunner::run(Asset, int (*)(float*, float*, float*, int, void*), void*, bool, bool, bool, bool, bool) (vmaf.cpp:149)
> ==12116==    by 0x52165B6: RunVmaf(char const*, int, int, int (*)(float*, float*, float*, int, void*), void*, char const*, char const*, char const*, bool, bool, bool, bool, bool, char const*) (vmaf.cpp:645)
> ==12116==    by 0x518AFFF: compute_vmaf_score (vf_libvmaf.c:161)
> ==12116==    by 0x518AFFF: call_vmaf (vf_libvmaf.c:170)
> ==12116==    by 0x7967493: start_thread (pthread_create.c:333)
> ==12116==    by 0x7F69A8E: clone (clone.S:97)
<snip>

Without confirmation it is fixed, I will push as is, with the dump in
the commit message. Please test and report.

Regards,
Ronald S. Bultje Aug. 30, 2017, 1:25 p.m. UTC | #2 
Hi,

On Sun, Aug 20, 2017 at 5:52 AM, Nicolas George <george@nsup.org> wrote:

> Le tridi 23 thermidor, an CCXXV, Nicolas George a écrit :
> > After this commit, the code compiles, but on my setup it
> > segfaults before and after. It also prints the very worrying
> > warning:
> >
> > src/libavfilter/vf_libvmaf.c:161:66: warning: passing argument 4 of
> ‘compute_vmaf’ from incompatible pointer type [-Wincompatible-pointer-types]
> > /tmp/i/include/libvmaf.h:26:8: note: expected ‘int (*)(float *, float *,
> float *, int,  void *)’ but argument is of type ‘int (*)(float *, float *,
> float *, int,  double *, void *)’
> >
> > ==12116== Thread 6:
> > ==12116== Conditional jump or move depends on uninitialised value(s)
> > ==12116==    at 0x526D432: cons_ (ocval.h:1188)
> > ==12116==    by 0x526D432: GenericIT (ocval.h:1119)
> > ==12116==    by 0x526D432: OC::TranslateForNumPyClassesToArray(OC::Val&)
> (pickleloader.h:92)
> > ==12116==    by 0x5211F5D: loads (pickleloader.h:566)
> > ==12116==    by 0x5211F5D: LoadValFromArray (chooseser.h:290)
> > ==12116==    by 0x5211F5D: LoadValFromFile (chooseser.h:405)
> > ==12116==    by 0x5211F5D: _read_and_assert_model(char const*, OC::Val&,
> OC::Val&, OC::Val&, OC::Val&, OC::Val&, OC::Val&) (vmaf.cpp:77)
> > ==12116==    by 0x5212B0F: VmafRunner::run(Asset, int (*)(float*,
> float*, float*, int, void*), void*, bool, bool, bool, bool, bool)
> (vmaf.cpp:149)
> > ==12116==    by 0x52165B6: RunVmaf(char const*, int, int, int
> (*)(float*, float*, float*, int, void*), void*, char const*, char const*,
> char const*, bool, bool, bool, bool, bool, char const*) (vmaf.cpp:645)
> > ==12116==    by 0x518AFFF: compute_vmaf_score (vf_libvmaf.c:161)
> > ==12116==    by 0x518AFFF: call_vmaf (vf_libvmaf.c:170)
> > ==12116==    by 0x7967493: start_thread (pthread_create.c:333)
> > ==12116==    by 0x7F69A8E: clone (clone.S:97)
> <snip>
>
> Without confirmation it is fixed, I will push as is, with the dump in
> the commit message. Please test and report.


Sorry, that was my fault, Ashish had submitted a patch and I had not merged
it. Fixed now, all works correctly. Thanks for the framesync2 conversion.

Ronald
diff mbox

Patch

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 13c0c2dbd2..47e649cdc4 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -217,7 +217,7 @@  OBJS-$(CONFIG_INTERLACE_FILTER)              += vf_interlace.o
 OBJS-$(CONFIG_INTERLEAVE_FILTER)             += f_interleave.o
 OBJS-$(CONFIG_KERNDEINT_FILTER)              += vf_kerndeint.o
 OBJS-$(CONFIG_LENSCORRECTION_FILTER)         += vf_lenscorrection.o
-OBJS-$(CONFIG_LIBVMAF_FILTER)                += vf_libvmaf.o dualinput.o framesync.o
+OBJS-$(CONFIG_LIBVMAF_FILTER)                += vf_libvmaf.o framesync2.o
 OBJS-$(CONFIG_LIMITER_FILTER)                += vf_limiter.o
 OBJS-$(CONFIG_LOOP_FILTER)                   += f_loop.o
 OBJS-$(CONFIG_LUMAKEY_FILTER)                += vf_lumakey.o
diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
index 3345cad670..2165561c5a 100644
--- a/libavfilter/vf_libvmaf.c
+++ b/libavfilter/vf_libvmaf.c
@@ -31,15 +31,15 @@ 
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
 #include "avfilter.h"
-#include "dualinput.h"
 #include "drawutils.h"
 #include "formats.h"
+#include "framesync2.h"
 #include "internal.h"
 #include "video.h"
 
 typedef struct LIBVMAFContext {
     const AVClass *class;
-    FFDualInputContext dinput;
+    FFFrameSync fs;
     const AVPixFmtDescriptor *desc;
     char *format;
     int width;
@@ -81,7 +81,7 @@  static const AVOption libvmaf_options[] = {
     { NULL }
 };
 
-AVFILTER_DEFINE_CLASS(libvmaf);
+FRAMESYNC_DEFINE_CLASS(libvmaf, LIBVMAFContext, fs);
 
 #define read_frame_fn(type, bits)                                               \
     static int read_frame_##bits##bit(float *ref_data, float *main_data,            \
@@ -172,9 +172,18 @@  static void *call_vmaf(void *ctx)
     pthread_exit(NULL);
 }
 
-static AVFrame *do_vmaf(AVFilterContext *ctx, AVFrame *main, const AVFrame *ref)
+static int do_vmaf(FFFrameSync *fs)
 {
+    AVFilterContext *ctx = fs->parent;
     LIBVMAFContext *s = ctx->priv;
+    AVFrame *main, *ref;
+    int ret;
+
+    ret = ff_framesync2_dualinput_get(fs, &main, &ref);
+    if (ret < 0)
+        return ret;
+    if (!ref)
+        return ff_filter_frame(ctx->outputs[0], main);
 
     pthread_mutex_lock(&s->lock);
 
@@ -190,7 +199,7 @@  static AVFrame *do_vmaf(AVFilterContext *ctx, AVFrame *main, const AVFrame *ref)
     pthread_cond_signal(&s->cond);
     pthread_mutex_unlock(&s->lock);
 
-    return main;
+    return ff_filter_frame(ctx->outputs[0], main);
 }
 
 static av_cold int init(AVFilterContext *ctx)
@@ -203,7 +212,7 @@  static av_cold int init(AVFilterContext *ctx)
     pthread_mutex_init(&s->lock, NULL);
     pthread_cond_init (&s->cond, NULL);
 
-    s->dinput.process = do_vmaf;
+    s->fs.on_event = do_vmaf;
     return 0;
 }
 
@@ -259,34 +268,31 @@  static int config_output(AVFilterLink *outlink)
     AVFilterLink *mainlink = ctx->inputs[0];
     int ret;
 
+    ret = ff_framesync2_init_dualinput(&s->fs, ctx);
+    if (ret < 0)
+        return ret;
     outlink->w = mainlink->w;
     outlink->h = mainlink->h;
     outlink->time_base = mainlink->time_base;
     outlink->sample_aspect_ratio = mainlink->sample_aspect_ratio;
     outlink->frame_rate = mainlink->frame_rate;
-    if ((ret = ff_dualinput_init(ctx, &s->dinput)) < 0)
+    if ((ret = ff_framesync2_configure(&s->fs)) < 0)
         return ret;
 
     return 0;
 }
 
-static int filter_frame(AVFilterLink *inlink, AVFrame *inpicref)
+static int activate(AVFilterContext *ctx)
 {
-    LIBVMAFContext *s = inlink->dst->priv;
-    return ff_dualinput_filter_frame(&s->dinput, inlink, inpicref);
-}
-
-static int request_frame(AVFilterLink *outlink)
-{
-    LIBVMAFContext *s = outlink->src->priv;
-    return ff_dualinput_request_frame(&s->dinput, outlink);
+    LIBVMAFContext *s = ctx->priv;
+    return ff_framesync2_activate(&s->fs);
 }
 
 static av_cold void uninit(AVFilterContext *ctx)
 {
     LIBVMAFContext *s = ctx->priv;
 
-    ff_dualinput_uninit(&s->dinput);
+    ff_framesync2_uninit(&s->fs);
 
     pthread_mutex_lock(&s->lock);
     s->eof = 1;
@@ -306,11 +312,9 @@  static const AVFilterPad libvmaf_inputs[] = {
     {
         .name         = "main",
         .type         = AVMEDIA_TYPE_VIDEO,
-        .filter_frame = filter_frame,
     },{
         .name         = "reference",
         .type         = AVMEDIA_TYPE_VIDEO,
-        .filter_frame = filter_frame,
         .config_props = config_input_ref,
     },
     { NULL }
@@ -321,7 +325,6 @@  static const AVFilterPad libvmaf_outputs[] = {
         .name          = "default",
         .type          = AVMEDIA_TYPE_VIDEO,
         .config_props  = config_output,
-        .request_frame = request_frame,
     },
     { NULL }
 };
@@ -329,9 +332,11 @@  static const AVFilterPad libvmaf_outputs[] = {
 AVFilter ff_vf_libvmaf = {
     .name          = "libvmaf",
     .description   = NULL_IF_CONFIG_SMALL("Calculate the VMAF between two video streams."),
+    .preinit       = libvmaf_framesync_preinit,
     .init          = init,
     .uninit        = uninit,
     .query_formats = query_formats,
+    .activate      = activate,
     .priv_size     = sizeof(LIBVMAFContext),
     .priv_class    = &libvmaf_class,
     .inputs        = libvmaf_inputs,