On Thu, Jun 16, 2022 at 02:04:47PM +0200, Tomas Härdin wrote: > > libavcodec/avcodec.c | 10 ++++++---- > libavcodec/pthread_slice.c | 9 +++++---- > libavfilter/pthread.c | 3 ++- > libavutil/slicethread.c | 34 +++++++++++++++++++++------------- > libavutil/slicethread.h | 6 +++--- > libswscale/swscale.c | 5 +++-- > libswscale/swscale_internal.h | 4 ++-- > tests/ref/fate/fic-avi | 30 +++++++++++++----------------- > 8 files changed, 55 insertions(+), 46 deletions(-) > be6eb9df96feec948b97acb76d2a9f7abc0cec52 0001-Make-execute-and-execute2-return-FFMIN-of-thread-ret.patch > From 2895c86dd908f6ddf3562d81050c50ea697a0bad Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= > Date: Thu, 16 Jun 2022 12:16:44 +0200 > Subject: [PATCH] Make execute() and execute2() return FFMIN() of thread return > codes > > At the moment only fic.c actually checks return code of execute() hence the change to its FATE reference [...] > diff --git a/libavutil/slicethread.c b/libavutil/slicethread.c > index ea1c9c8311..83a98a7ae7 100644 > --- a/libavutil/slicethread.c > +++ b/libavutil/slicethread.c > @@ -32,6 +32,7 @@ typedef struct WorkerContext { > pthread_cond_t cond; > pthread_t thread; > int done; > + int ret; > } WorkerContext; > > struct AVSliceThread { > @@ -48,11 +49,11 @@ struct AVSliceThread { > int finished; > > void *priv; > - void (*worker_func)(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads); > - void (*main_func)(void *priv); > + int (*worker_func)(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads); > + int (*main_func)(void *priv); > }; > > -static int run_jobs(AVSliceThread *ctx) > +static int run_jobs(AVSliceThread *ctx, int *ret_out) > { > unsigned nb_jobs = ctx->nb_jobs; > unsigned nb_active_threads = ctx->nb_active_threads; > @@ -60,7 +61,8 @@ static int run_jobs(AVSliceThread *ctx) > unsigned current_job = first_job; > > do { > - ctx->worker_func(ctx->priv, current_job, first_job, nb_jobs, nb_active_threads); > + int ret = ctx->worker_func(ctx->priv, current_job, first_job, nb_jobs, nb_active_threads); > + *ret_out = FFMIN(*ret_out, ret); > } while ((current_job = atomic_fetch_add_explicit(&ctx->current_job, 1, memory_order_acq_rel)) < nb_jobs); > > return current_job == nb_jobs + nb_active_threads - 1; > @@ -84,7 +86,7 @@ static void *attribute_align_arg thread_worker(void *v) > return NULL; > } > > - if (run_jobs(ctx)) { > + if (run_jobs(ctx, &w->ret)) { > pthread_mutex_lock(&ctx->done_mutex); > ctx->done = 1; > pthread_cond_signal(&ctx->done_cond); > @@ -94,8 +96,8 @@ static void *attribute_align_arg thread_worker(void *v) > } > > int avpriv_slicethread_create(AVSliceThread **pctx, void *priv, > - void (*worker_func)(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads), > - void (*main_func)(void *priv), > + int (*worker_func)(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads), > + int (*main_func)(void *priv), > int nb_threads) > { > AVSliceThread *ctx; > @@ -163,9 +165,9 @@ int avpriv_slicethread_create(AVSliceThread **pctx, void *priv, > return nb_threads; > } > > -void avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main) > +int avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main) > { > - int nb_workers, i, is_last = 0; > + int nb_workers, i, is_last = 0, ret = 0; > > av_assert0(nb_jobs > 0); > ctx->nb_jobs = nb_jobs; > @@ -180,14 +182,15 @@ void avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_mai > WorkerContext *w = &ctx->workers[i]; > pthread_mutex_lock(&w->mutex); > w->done = 0; > + w->ret = 0; > pthread_cond_signal(&w->cond); > pthread_mutex_unlock(&w->mutex); > } > > if (ctx->main_func && execute_main) > - ctx->main_func(ctx->priv); > + ret = ctx->main_func(ctx->priv); > else > - is_last = run_jobs(ctx); > + is_last = run_jobs(ctx, &ret); > > if (!is_last) { > pthread_mutex_lock(&ctx->done_mutex); > @@ -196,6 +199,11 @@ void avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_mai > ctx->done = 0; > pthread_mutex_unlock(&ctx->done_mutex); > } > + > + for (i = 0; i < nb_workers; i++) > + ret = FFMIN(ret, ctx->workers[i].ret); > + > + return ret; > } > > void avpriv_slicethread_free(AVSliceThread **pctx) > @@ -236,8 +244,8 @@ void avpriv_slicethread_free(AVSliceThread **pctx) > #else /* HAVE_PTHREADS || HAVE_W32THREADS || HAVE_OS32THREADS */ > > int avpriv_slicethread_create(AVSliceThread **pctx, void *priv, > - void (*worker_func)(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads), > - void (*main_func)(void *priv), > + int (*worker_func)(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads), > + int (*main_func)(void *priv), > int nb_threads) > { > *pctx = NULL; You forgot to update the fallback code when threads are disabled [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates