diff mbox

[FFmpeg-devel] libavfilter/vf_libvmaf.c The libvmaf filter tried to join on an invalid thread id

Message ID CACzQ+ub_q1jN7QqywLOFMq0VKHm5vFadjz6UskZOdQEWr2nC_w@mail.gmail.com
State New
Headers show

Commit Message

Kevin Wheatley April 30, 2018, 3:40 p.m. UTC
Following on from my report in the user list here is a better than a
quick hack suggestion to avoid trying to join on an invalid thread id.
This may not be the best solution, but it does avoid the SEGV on linux
when calling pthread_join()

Kevin

Comments

Kevin Wheatley May 3, 2018, 8:41 a.m. UTC | #1
Following up my own email with another question or so:

Could somebody point me at a suitable method of testing this within
the Fate framework?

I've started to try unpick how the fate tests are run, etc

What I need is a prototypical example which presumably would need to
factor in (what I figured out so far):

libvmaf is an optional component so the test should be too.
The test should probably live in tests/fate/filter-video.mak
The function (CMD) called should live in tests/fate-run.sh?

For the actual test I need two inputs an original and an encoded frame
(sequence is ok too), that can then feed into the vmaf invocation of
ffmpeg. Is there a set of inputs I can just reuse from what fate would
normally run so that the test need only run ffmpeg to compute the
output of vmaf, (or is the preference for every test to generate its
own input?)

Thanks

Kevin
Ronald S. Bultje May 3, 2018, 11:38 a.m. UTC | #2
Hi,

On Thu, May 3, 2018 at 4:41 AM, Kevin Wheatley <kevin.j.wheatley@gmail.com>
wrote:

> Following up my own email with another question or so:
>
> Could somebody point me at a suitable method of testing this within
> the Fate framework?


Why?

Your patch fixes a bug, I don't think we test bugs in fate, just features.

Patch LGTM.

Ronald
Kevin Wheatley May 3, 2018, 1:11 p.m. UTC | #3
On Thu, May 3, 2018 at 12:38 PM, Ronald S. Bultje <rsbultje@gmail.com> wrote:
> Why?
>
> Your patch fixes a bug, I don't think we test bugs in fate, just features.

I am perhaps making an incorrect assumption that the code has a bug
because it is never tested and that by adding a test that simply tries
to use it, the vmaf code is more likely to work. I'm happy to not
write a test if that is preferred!

Kevin
James Almer May 4, 2018, 9:31 p.m. UTC | #4
On 5/3/2018 8:38 AM, Ronald S. Bultje wrote:
> Hi,
> 
> On Thu, May 3, 2018 at 4:41 AM, Kevin Wheatley <kevin.j.wheatley@gmail.com>
> wrote:
> 
>> Following up my own email with another question or so:
>>
>> Could somebody point me at a suitable method of testing this within
>> the Fate framework?
> 
> 
> Why?
> 
> Your patch fixes a bug, I don't think we test bugs in fate, just features.
> 
> Patch LGTM.
> 
> Ronald

Pushed then.
diff mbox

Patch

From a94d0cb7ff4202487ea980a029fa613c58d6d7c3 Mon Sep 17 00:00:00 2001
From: Kevin Wheatley <kevin.j.wheatley@gmail.com>
Date: Mon, 30 Apr 2018 16:33:51 +0100
Subject: [PATCH] The libvmaf filter tried to join on an invalid thread id

The thread id was invalid because it was not initialised
during the calls to init_complex_filtergraph.

This adds a flag to check for initialisation before trying to
peform the join.

Signed-off-by: Kevin Wheatley <kevin.j.wheatley@gmail.com>
---
 libavfilter/vf_libvmaf.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/libavfilter/vf_libvmaf.c b/libavfilter/vf_libvmaf.c
index 42c6b66..5d47a74 100644
--- a/libavfilter/vf_libvmaf.c
+++ b/libavfilter/vf_libvmaf.c
@@ -43,6 +43,7 @@  typedef struct LIBVMAFContext {
     int width;
     int height;
     double vmaf_score;
+    int vmaf_thread_created;
     pthread_t vmaf_thread;
     pthread_mutex_t lock;
     pthread_cond_t cond;
@@ -228,6 +229,7 @@  static av_cold int init(AVFilterContext *ctx)
     s->gmain = av_frame_alloc();
     s->error = 0;
 
+    s->vmaf_thread_created = 0;
     pthread_mutex_init(&s->lock, NULL);
     pthread_cond_init (&s->cond, NULL);
 
@@ -275,6 +277,7 @@  static int config_input_ref(AVFilterLink *inlink)
         av_log(ctx, AV_LOG_ERROR, "Thread creation failed.\n");
         return AVERROR(EINVAL);
     }
+    s->vmaf_thread_created = 1;
 
     return 0;
 }
@@ -317,7 +320,11 @@  static av_cold void uninit(AVFilterContext *ctx)
     pthread_cond_signal(&s->cond);
     pthread_mutex_unlock(&s->lock);
 
-    pthread_join(s->vmaf_thread, NULL);
+    if (s->vmaf_thread_created)
+    {
+        pthread_join(s->vmaf_thread, NULL);
+        s->vmaf_thread_created = 0;
+    }
 
     av_frame_free(&s->gref);
     av_frame_free(&s->gmain);
-- 
1.7.1