[FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes

[Michael Niedermayer]

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?= <git at haerdin.se>
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20220616/31647e23/attachment.sig>