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

[Tomas Härdin]

lör 2022-07-02 klockan 11:43 +0200 skrev Anton Khirnov:
> Quoting Tomas Härdin (2022-06-30 14:42:42)
> > Hi
> > 
> > Previous version of this patch failed fate-fic-avi with
> > THREAD_TYPE=slice THREADS=2 due to thread_execute() always
> > returning 0.
> > Fixed in this version.
> > 
> > The fic sample appears to indeed be broken. Some packets are
> > truncated,
> > including one zero-length packet.
> 
> maybe mention this fact for posterity above the relevant test in
> tests/fate/scren.mak

Sure

> 
> > diff --git a/libavutil/slicethread.h b/libavutil/slicethread.h
> > index f6f6f302c4..5c8f197932 100644
> > --- a/libavutil/slicethread.h
> > +++ b/libavutil/slicethread.h
> > @@ -31,8 +31,8 @@ typedef struct AVSliceThread AVSliceThread;
> >   * @return return number of threads or negative AVERROR on failure
> >   */
> >  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),
> 
> This is an ABI break.
> 

You're right. I was under the impression that avpriv functions could be
changed more freely but obviously not when they're shared between
libraries.

This could be worked around with a new function called say
avpriv_slicethread_create2() and a minor bump. Another approach is to
av_fast_malloc() an array for rets when none is given. The number of
allocations would thereby be quite limited.

I'd like to see all users of execute() and execute2() start checking
their return values, and making that easier to do is another step
toward more correctness of the code

/Tomas