* [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
@ 2022-06-30 12:42 Tomas Härdin
2022-07-02 9:43 ` Anton Khirnov
0 siblings, 1 reply; 13+ messages in thread
From: Tomas Härdin @ 2022-06-30 12:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1: Type: text/plain, Size: 270 bytes --]
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.
/Tomas
[-- Attachment #2: 0001-Make-execute-and-execute2-return-FFMIN-of-thread-ret.patch --]
[-- Type: text/x-patch, Size: 14541 bytes --]
From 872354ff70f6e880d77e8bddd28bfa7bc6582ab2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@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
---
libavcodec/avcodec.c | 10 ++++++----
libavcodec/pthread_slice.c | 12 ++++++------
libavfilter/pthread.c | 3 ++-
libavutil/slicethread.c | 37 ++++++++++++++++++++++-------------
libavutil/slicethread.h | 6 +++---
libswscale/swscale.c | 5 +++--
libswscale/swscale_internal.h | 4 ++--
tests/ref/fate/fic-avi | 30 ++++++++++++----------------
8 files changed, 58 insertions(+), 49 deletions(-)
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 5f6e71a39e..49f0fd06fb 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -44,28 +44,30 @@
int avcodec_default_execute(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2), void *arg, int *ret, int count, int size)
{
- int i;
+ int i, rr = 0;
for (i = 0; i < count; i++) {
int r = func(c, (char *)arg + i * size);
+ rr = FFMIN(rr, r);
if (ret)
ret[i] = r;
}
emms_c();
- return 0;
+ return rr;
}
int avcodec_default_execute2(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2, int jobnr, int threadnr), void *arg, int *ret, int count)
{
- int i;
+ int i, rr = 0;
for (i = 0; i < count; i++) {
int r = func(c, arg, i, 0);
+ rr = FFMIN(rr, r);
if (ret)
ret[i] = r;
}
emms_c();
- return 0;
+ return rr;
}
static AVMutex codec_mutex = AV_MUTEX_INITIALIZER;
diff --git a/libavcodec/pthread_slice.c b/libavcodec/pthread_slice.c
index 0ad1965a22..e2903846eb 100644
--- a/libavcodec/pthread_slice.c
+++ b/libavcodec/pthread_slice.c
@@ -57,13 +57,13 @@ typedef struct SliceThreadContext {
pthread_mutex_t *progress_mutex;
} SliceThreadContext;
-static void main_function(void *priv) {
+static int main_function(void *priv) {
AVCodecContext *avctx = priv;
SliceThreadContext *c = avctx->internal->thread_ctx;
- c->mainfunc(avctx);
+ return c->mainfunc(avctx);
}
-static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
+static int worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
{
AVCodecContext *avctx = priv;
SliceThreadContext *c = avctx->internal->thread_ctx;
@@ -73,6 +73,7 @@ static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb
: c->func2(avctx, c->args, jobnr, threadnr);
if (c->rets)
c->rets[jobnr] = ret;
+ return ret;
}
void ff_slice_thread_free(AVCodecContext *avctx)
@@ -108,8 +109,7 @@ static int thread_execute(AVCodecContext *avctx, action_func* func, void *arg, i
c->func = func;
c->rets = ret;
- avpriv_slicethread_execute(c->thread, job_count, !!c->mainfunc );
- return 0;
+ return avpriv_slicethread_execute(c->thread, job_count, !!c->mainfunc );
}
static int thread_execute2(AVCodecContext *avctx, action_func2* func2, void *arg, int *ret, int job_count)
@@ -131,7 +131,7 @@ int ff_slice_thread_init(AVCodecContext *avctx)
{
SliceThreadContext *c;
int thread_count = avctx->thread_count;
- void (*mainfunc)(void *);
+ int (*mainfunc)(void *);
// We cannot do this in the encoder init as the threads are created before
if (av_codec_is_encoder(avctx->codec) &&
diff --git a/libavfilter/pthread.c b/libavfilter/pthread.c
index 1a063d3cc0..8cec278be0 100644
--- a/libavfilter/pthread.c
+++ b/libavfilter/pthread.c
@@ -43,12 +43,13 @@ typedef struct ThreadContext {
int *rets;
} ThreadContext;
-static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
+static int worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
{
ThreadContext *c = priv;
int ret = c->func(c->ctx, c->arg, jobnr, nb_jobs);
if (c->rets)
c->rets[jobnr] = ret;
+ return ret;
}
static void slice_thread_uninit(ThreadContext *c)
diff --git a/libavutil/slicethread.c b/libavutil/slicethread.c
index ea1c9c8311..2e78d32ab8 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,17 +244,18 @@ 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;
return AVERROR(ENOSYS);
}
-void avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main)
+int avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main)
{
av_assert0(0);
+ return AVERROR(ENOSYS);
}
void avpriv_slicethread_free(AVSliceThread **pctx)
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),
int nb_threads);
/**
@@ -41,7 +41,7 @@ int avpriv_slicethread_create(AVSliceThread **pctx, void *priv,
* @param nb_jobs number of jobs, must be > 0
* @param execute_main also execute main_func
*/
-void avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main);
+int avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main);
/**
* Destroy slice threading context.
diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 7b40f49da4..2f9a0b5a7c 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -1211,8 +1211,8 @@ int attribute_align_arg sws_scale(struct SwsContext *c,
dst, dstStride, 0, c->dstH);
}
-void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
- int nb_jobs, int nb_threads)
+int ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
+ int nb_jobs, int nb_threads)
{
SwsContext *parent = priv;
SwsContext *c = parent->slice_ctx[threadnr];
@@ -1241,4 +1241,5 @@ void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
}
parent->slice_err[threadnr] = err;
+ return err;
}
diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index e118b54457..eab3e26331 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -1135,8 +1135,8 @@ void ff_init_vscale_pfn(SwsContext *c, yuv2planar1_fn yuv2plane1, yuv2planarX_fn
yuv2interleavedX_fn yuv2nv12cX, yuv2packed1_fn yuv2packed1, yuv2packed2_fn yuv2packed2,
yuv2packedX_fn yuv2packedX, yuv2anyX_fn yuv2anyX, int use_mmx);
-void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
- int nb_jobs, int nb_threads);
+int ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
+ int nb_jobs, int nb_threads);
//number of extra lines to process
#define MAX_LINES_AHEAD 4
diff --git a/tests/ref/fate/fic-avi b/tests/ref/fate/fic-avi
index df55789d54..4546f230b1 100644
--- a/tests/ref/fate/fic-avi
+++ b/tests/ref/fate/fic-avi
@@ -76,19 +76,18 @@
0, 70, 70, 1, 1566720, 0x40f7d39a
0, 71, 71, 1, 1566720, 0x40f7d39a
0, 72, 72, 1, 1566720, 0x40f7d39a
-0, 73, 73, 1, 1566720, 0xa7d6e25f
-0, 74, 74, 1, 1566720, 0xa7d6e25f
-0, 75, 75, 1, 1566720, 0xa7d6e25f
-0, 76, 76, 1, 1566720, 0xa7d6e25f
-0, 77, 77, 1, 1566720, 0xa7d6e25f
-0, 78, 78, 1, 1566720, 0xa7d6e25f
-0, 79, 79, 1, 1566720, 0xa7d6e25f
-0, 80, 80, 1, 1566720, 0xa7d6e25f
-0, 81, 81, 1, 1566720, 0xa7d6e25f
-0, 82, 82, 1, 1566720, 0xa7d6e25f
-0, 83, 83, 1, 1566720, 0xa7d6e25f
-0, 84, 84, 1, 1566720, 0xa7d6e25f
-0, 85, 85, 1, 1566720, 0xa7d6e25f
+0, 74, 74, 1, 1566720, 0x40f7d39a
+0, 75, 75, 1, 1566720, 0x40f7d39a
+0, 76, 76, 1, 1566720, 0x40f7d39a
+0, 77, 77, 1, 1566720, 0x40f7d39a
+0, 78, 78, 1, 1566720, 0x40f7d39a
+0, 79, 79, 1, 1566720, 0x40f7d39a
+0, 80, 80, 1, 1566720, 0x40f7d39a
+0, 81, 81, 1, 1566720, 0x40f7d39a
+0, 82, 82, 1, 1566720, 0x40f7d39a
+0, 83, 83, 1, 1566720, 0x40f7d39a
+0, 84, 84, 1, 1566720, 0x40f7d39a
+0, 85, 85, 1, 1566720, 0x40f7d39a
0, 86, 86, 1, 1566720, 0xa7d6e25f
0, 87, 87, 1, 1566720, 0xa7d6e25f
0, 88, 88, 1, 1566720, 0xa7d6e25f
@@ -104,7 +103,6 @@
0, 98, 98, 1, 1566720, 0xa7d6e25f
0, 99, 99, 1, 1566720, 0xa7d6e25f
0, 100, 100, 1, 1566720, 0xeaf8d207
-0, 101, 101, 1, 1566720, 0x6724983e
0, 102, 102, 1, 1566720, 0x0e95d209
0, 103, 103, 1, 1566720, 0x0e95d209
0, 104, 104, 1, 1566720, 0x0e95d209
@@ -121,6 +119,4 @@
0, 115, 115, 1, 1566720, 0xfe83b964
0, 116, 116, 1, 1566720, 0xfe83b964
0, 117, 117, 1, 1566720, 0xfe83b964
-0, 118, 118, 1, 1566720, 0x25dc30a6
-0, 119, 119, 1, 1566720, 0x25dc30a6
-0, 120, 120, 1, 1566720, 0x25dc30a6
+0, 119, 119, 1, 1566720, 0xfe83b964
--
2.30.2
[-- Attachment #3: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-30 12:42 [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes Tomas Härdin
@ 2022-07-02 9:43 ` Anton Khirnov
2022-07-04 10:46 ` Tomas Härdin
0 siblings, 1 reply; 13+ messages in thread
From: Anton Khirnov @ 2022-07-02 9:43 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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
> 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.
--
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-07-02 9:43 ` Anton Khirnov
@ 2022-07-04 10:46 ` Tomas Härdin
2022-07-05 16:42 ` Anton Khirnov
0 siblings, 1 reply; 13+ messages in thread
From: Tomas Härdin @ 2022-07-04 10:46 UTC (permalink / raw)
To: FFmpeg development discussions and patches
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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-07-04 10:46 ` Tomas Härdin
@ 2022-07-05 16:42 ` Anton Khirnov
0 siblings, 0 replies; 13+ messages in thread
From: Anton Khirnov @ 2022-07-05 16:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Tomas Härdin (2022-07-04 12:46:04)
> 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.
Adding a new function is ok I suppose.
Though longer-term I'd like to see that API cleaned up and made properly
public.
> 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
I'm all in favor of that.
--
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-21 7:55 ` Anton Khirnov
@ 2022-06-21 8:05 ` Tomas Härdin
0 siblings, 0 replies; 13+ messages in thread
From: Tomas Härdin @ 2022-06-21 8:05 UTC (permalink / raw)
To: FFmpeg development discussions and patches
tis 2022-06-21 klockan 09:55 +0200 skrev Anton Khirnov:
> Quoting Tomas Härdin (2022-06-21 09:51:55)
> > lör 2022-06-18 klockan 16:38 +0200 skrev Anton Khirnov:
> > > Quoting Tomas Härdin (2022-06-17 11:42:25)
> > > > fre 2022-06-17 klockan 01:38 +0200 skrev Michael Niedermayer:
> > > > > On Thu, Jun 16, 2022 at 11:04:01PM +0200, Tomas Härdin wrote:
> > > > > > tor 2022-06-16 klockan 20:27 +0200 skrev Michael
> > > > > > Niedermayer:
> > > > > > >
> > > > > > > >
> > > > > > > > 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
> > > > > >
> > > > > > Uhm, the existing code just abort()s if threads are
> > > > > > disabled?
> > > > > > I'm
> > > > > > not
> > > > > > really sure if there anything that can or should be done
> > > > > > there
> > > > >
> > > > > Before your patches fate passes with --disable-pthreads
> > > > > afterwards it will fail during build because the function
> > > > > mismatches,
> > > > > the abort should not be reachable i hope
> > > >
> > > > Updated patch attached
> > > >
> > > > /Tomas
> > > >
> > > > From e3729f70d016b8e5c9d1bdb6014506ddd8d4eb56 Mon Sep 17
> > > > 00:00:00
> > > > 2001
> > > > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@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
> > >
> > > Wait, how does this follow?
> > >
> >
> > fic.c actually gets informed that one or more fic_decode_slice()
> > jobs
> > failed and so it successfully fails on frames that don't decode
> > correctly
>
> Why does it fail though? The sample we use for the test is actually
> broken?
>
Not sure, and I have to head off. But I get this error at least:
[avi @ 0x7f40c4000cc0] Packet corrupt (stream = 0, dts = 120)
/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-21 7:51 ` Tomas Härdin
@ 2022-06-21 7:55 ` Anton Khirnov
2022-06-21 8:05 ` Tomas Härdin
0 siblings, 1 reply; 13+ messages in thread
From: Anton Khirnov @ 2022-06-21 7:55 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Tomas Härdin (2022-06-21 09:51:55)
> lör 2022-06-18 klockan 16:38 +0200 skrev Anton Khirnov:
> > Quoting Tomas Härdin (2022-06-17 11:42:25)
> > > fre 2022-06-17 klockan 01:38 +0200 skrev Michael Niedermayer:
> > > > On Thu, Jun 16, 2022 at 11:04:01PM +0200, Tomas Härdin wrote:
> > > > > tor 2022-06-16 klockan 20:27 +0200 skrev Michael Niedermayer:
> > > > > >
> > > > > > >
> > > > > > > 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
> > > > >
> > > > > Uhm, the existing code just abort()s if threads are disabled?
> > > > > I'm
> > > > > not
> > > > > really sure if there anything that can or should be done there
> > > >
> > > > Before your patches fate passes with --disable-pthreads
> > > > afterwards it will fail during build because the function
> > > > mismatches,
> > > > the abort should not be reachable i hope
> > >
> > > Updated patch attached
> > >
> > > /Tomas
> > >
> > > From e3729f70d016b8e5c9d1bdb6014506ddd8d4eb56 Mon Sep 17 00:00:00
> > > 2001
> > > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@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
> >
> > Wait, how does this follow?
> >
>
> fic.c actually gets informed that one or more fic_decode_slice() jobs
> failed and so it successfully fails on frames that don't decode
> correctly
Why does it fail though? The sample we use for the test is actually broken?
--
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-18 14:38 ` Anton Khirnov
@ 2022-06-21 7:51 ` Tomas Härdin
2022-06-21 7:55 ` Anton Khirnov
0 siblings, 1 reply; 13+ messages in thread
From: Tomas Härdin @ 2022-06-21 7:51 UTC (permalink / raw)
To: FFmpeg development discussions and patches
lör 2022-06-18 klockan 16:38 +0200 skrev Anton Khirnov:
> Quoting Tomas Härdin (2022-06-17 11:42:25)
> > fre 2022-06-17 klockan 01:38 +0200 skrev Michael Niedermayer:
> > > On Thu, Jun 16, 2022 at 11:04:01PM +0200, Tomas Härdin wrote:
> > > > tor 2022-06-16 klockan 20:27 +0200 skrev Michael Niedermayer:
> > > > >
> > > > > >
> > > > > > 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
> > > >
> > > > Uhm, the existing code just abort()s if threads are disabled?
> > > > I'm
> > > > not
> > > > really sure if there anything that can or should be done there
> > >
> > > Before your patches fate passes with --disable-pthreads
> > > afterwards it will fail during build because the function
> > > mismatches,
> > > the abort should not be reachable i hope
> >
> > Updated patch attached
> >
> > /Tomas
> >
> > From e3729f70d016b8e5c9d1bdb6014506ddd8d4eb56 Mon Sep 17 00:00:00
> > 2001
> > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@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
>
> Wait, how does this follow?
>
fic.c actually gets informed that one or more fic_decode_slice() jobs
failed and so it successfully fails on frames that don't decode
correctly
/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-17 9:42 ` Tomas Härdin
@ 2022-06-18 14:38 ` Anton Khirnov
2022-06-21 7:51 ` Tomas Härdin
0 siblings, 1 reply; 13+ messages in thread
From: Anton Khirnov @ 2022-06-18 14:38 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Tomas Härdin (2022-06-17 11:42:25)
> fre 2022-06-17 klockan 01:38 +0200 skrev Michael Niedermayer:
> > On Thu, Jun 16, 2022 at 11:04:01PM +0200, Tomas Härdin wrote:
> > > tor 2022-06-16 klockan 20:27 +0200 skrev Michael Niedermayer:
> > > >
> > > > >
> > > > > 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
> > >
> > > Uhm, the existing code just abort()s if threads are disabled? I'm
> > > not
> > > really sure if there anything that can or should be done there
> >
> > Before your patches fate passes with --disable-pthreads
> > afterwards it will fail during build because the function mismatches,
> > the abort should not be reachable i hope
>
> Updated patch attached
>
> /Tomas
>
> From e3729f70d016b8e5c9d1bdb6014506ddd8d4eb56 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@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
Wait, how does this follow?
--
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-16 23:38 ` Michael Niedermayer
@ 2022-06-17 9:42 ` Tomas Härdin
2022-06-18 14:38 ` Anton Khirnov
0 siblings, 1 reply; 13+ messages in thread
From: Tomas Härdin @ 2022-06-17 9:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1: Type: text/plain, Size: 1637 bytes --]
fre 2022-06-17 klockan 01:38 +0200 skrev Michael Niedermayer:
> On Thu, Jun 16, 2022 at 11:04:01PM +0200, Tomas Härdin wrote:
> > tor 2022-06-16 klockan 20:27 +0200 skrev Michael Niedermayer:
> > >
> > > >
> > > > 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
> >
> > Uhm, the existing code just abort()s if threads are disabled? I'm
> > not
> > really sure if there anything that can or should be done there
>
> Before your patches fate passes with --disable-pthreads
> afterwards it will fail during build because the function mismatches,
> the abort should not be reachable i hope
Updated patch attached
/Tomas
[-- Attachment #2: 0001-Make-execute-and-execute2-return-FFMIN-of-thread-ret.patch --]
[-- Type: text/x-patch, Size: 14115 bytes --]
From e3729f70d016b8e5c9d1bdb6014506ddd8d4eb56 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@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
---
libavcodec/avcodec.c | 10 ++++++----
libavcodec/pthread_slice.c | 9 +++++----
libavfilter/pthread.c | 3 ++-
libavutil/slicethread.c | 37 ++++++++++++++++++++++-------------
libavutil/slicethread.h | 6 +++---
libswscale/swscale.c | 5 +++--
libswscale/swscale_internal.h | 4 ++--
tests/ref/fate/fic-avi | 30 ++++++++++++----------------
8 files changed, 57 insertions(+), 47 deletions(-)
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 5f6e71a39e..49f0fd06fb 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -44,28 +44,30 @@
int avcodec_default_execute(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2), void *arg, int *ret, int count, int size)
{
- int i;
+ int i, rr = 0;
for (i = 0; i < count; i++) {
int r = func(c, (char *)arg + i * size);
+ rr = FFMIN(rr, r);
if (ret)
ret[i] = r;
}
emms_c();
- return 0;
+ return rr;
}
int avcodec_default_execute2(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2, int jobnr, int threadnr), void *arg, int *ret, int count)
{
- int i;
+ int i, rr = 0;
for (i = 0; i < count; i++) {
int r = func(c, arg, i, 0);
+ rr = FFMIN(rr, r);
if (ret)
ret[i] = r;
}
emms_c();
- return 0;
+ return rr;
}
static AVMutex codec_mutex = AV_MUTEX_INITIALIZER;
diff --git a/libavcodec/pthread_slice.c b/libavcodec/pthread_slice.c
index 0ad1965a22..5f02b9b6a1 100644
--- a/libavcodec/pthread_slice.c
+++ b/libavcodec/pthread_slice.c
@@ -57,13 +57,13 @@ typedef struct SliceThreadContext {
pthread_mutex_t *progress_mutex;
} SliceThreadContext;
-static void main_function(void *priv) {
+static int main_function(void *priv) {
AVCodecContext *avctx = priv;
SliceThreadContext *c = avctx->internal->thread_ctx;
- c->mainfunc(avctx);
+ return c->mainfunc(avctx);
}
-static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
+static int worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
{
AVCodecContext *avctx = priv;
SliceThreadContext *c = avctx->internal->thread_ctx;
@@ -73,6 +73,7 @@ static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb
: c->func2(avctx, c->args, jobnr, threadnr);
if (c->rets)
c->rets[jobnr] = ret;
+ return ret;
}
void ff_slice_thread_free(AVCodecContext *avctx)
@@ -131,7 +132,7 @@ int ff_slice_thread_init(AVCodecContext *avctx)
{
SliceThreadContext *c;
int thread_count = avctx->thread_count;
- void (*mainfunc)(void *);
+ int (*mainfunc)(void *);
// We cannot do this in the encoder init as the threads are created before
if (av_codec_is_encoder(avctx->codec) &&
diff --git a/libavfilter/pthread.c b/libavfilter/pthread.c
index 1a063d3cc0..8cec278be0 100644
--- a/libavfilter/pthread.c
+++ b/libavfilter/pthread.c
@@ -43,12 +43,13 @@ typedef struct ThreadContext {
int *rets;
} ThreadContext;
-static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
+static int worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
{
ThreadContext *c = priv;
int ret = c->func(c->ctx, c->arg, jobnr, nb_jobs);
if (c->rets)
c->rets[jobnr] = ret;
+ return ret;
}
static void slice_thread_uninit(ThreadContext *c)
diff --git a/libavutil/slicethread.c b/libavutil/slicethread.c
index ea1c9c8311..2e78d32ab8 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,17 +244,18 @@ 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;
return AVERROR(ENOSYS);
}
-void avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main)
+int avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main)
{
av_assert0(0);
+ return AVERROR(ENOSYS);
}
void avpriv_slicethread_free(AVSliceThread **pctx)
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),
int nb_threads);
/**
@@ -41,7 +41,7 @@ int avpriv_slicethread_create(AVSliceThread **pctx, void *priv,
* @param nb_jobs number of jobs, must be > 0
* @param execute_main also execute main_func
*/
-void avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main);
+int avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main);
/**
* Destroy slice threading context.
diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 7b40f49da4..2f9a0b5a7c 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -1211,8 +1211,8 @@ int attribute_align_arg sws_scale(struct SwsContext *c,
dst, dstStride, 0, c->dstH);
}
-void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
- int nb_jobs, int nb_threads)
+int ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
+ int nb_jobs, int nb_threads)
{
SwsContext *parent = priv;
SwsContext *c = parent->slice_ctx[threadnr];
@@ -1241,4 +1241,5 @@ void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
}
parent->slice_err[threadnr] = err;
+ return err;
}
diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index e118b54457..eab3e26331 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -1135,8 +1135,8 @@ void ff_init_vscale_pfn(SwsContext *c, yuv2planar1_fn yuv2plane1, yuv2planarX_fn
yuv2interleavedX_fn yuv2nv12cX, yuv2packed1_fn yuv2packed1, yuv2packed2_fn yuv2packed2,
yuv2packedX_fn yuv2packedX, yuv2anyX_fn yuv2anyX, int use_mmx);
-void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
- int nb_jobs, int nb_threads);
+int ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
+ int nb_jobs, int nb_threads);
//number of extra lines to process
#define MAX_LINES_AHEAD 4
diff --git a/tests/ref/fate/fic-avi b/tests/ref/fate/fic-avi
index df55789d54..4546f230b1 100644
--- a/tests/ref/fate/fic-avi
+++ b/tests/ref/fate/fic-avi
@@ -76,19 +76,18 @@
0, 70, 70, 1, 1566720, 0x40f7d39a
0, 71, 71, 1, 1566720, 0x40f7d39a
0, 72, 72, 1, 1566720, 0x40f7d39a
-0, 73, 73, 1, 1566720, 0xa7d6e25f
-0, 74, 74, 1, 1566720, 0xa7d6e25f
-0, 75, 75, 1, 1566720, 0xa7d6e25f
-0, 76, 76, 1, 1566720, 0xa7d6e25f
-0, 77, 77, 1, 1566720, 0xa7d6e25f
-0, 78, 78, 1, 1566720, 0xa7d6e25f
-0, 79, 79, 1, 1566720, 0xa7d6e25f
-0, 80, 80, 1, 1566720, 0xa7d6e25f
-0, 81, 81, 1, 1566720, 0xa7d6e25f
-0, 82, 82, 1, 1566720, 0xa7d6e25f
-0, 83, 83, 1, 1566720, 0xa7d6e25f
-0, 84, 84, 1, 1566720, 0xa7d6e25f
-0, 85, 85, 1, 1566720, 0xa7d6e25f
+0, 74, 74, 1, 1566720, 0x40f7d39a
+0, 75, 75, 1, 1566720, 0x40f7d39a
+0, 76, 76, 1, 1566720, 0x40f7d39a
+0, 77, 77, 1, 1566720, 0x40f7d39a
+0, 78, 78, 1, 1566720, 0x40f7d39a
+0, 79, 79, 1, 1566720, 0x40f7d39a
+0, 80, 80, 1, 1566720, 0x40f7d39a
+0, 81, 81, 1, 1566720, 0x40f7d39a
+0, 82, 82, 1, 1566720, 0x40f7d39a
+0, 83, 83, 1, 1566720, 0x40f7d39a
+0, 84, 84, 1, 1566720, 0x40f7d39a
+0, 85, 85, 1, 1566720, 0x40f7d39a
0, 86, 86, 1, 1566720, 0xa7d6e25f
0, 87, 87, 1, 1566720, 0xa7d6e25f
0, 88, 88, 1, 1566720, 0xa7d6e25f
@@ -104,7 +103,6 @@
0, 98, 98, 1, 1566720, 0xa7d6e25f
0, 99, 99, 1, 1566720, 0xa7d6e25f
0, 100, 100, 1, 1566720, 0xeaf8d207
-0, 101, 101, 1, 1566720, 0x6724983e
0, 102, 102, 1, 1566720, 0x0e95d209
0, 103, 103, 1, 1566720, 0x0e95d209
0, 104, 104, 1, 1566720, 0x0e95d209
@@ -121,6 +119,4 @@
0, 115, 115, 1, 1566720, 0xfe83b964
0, 116, 116, 1, 1566720, 0xfe83b964
0, 117, 117, 1, 1566720, 0xfe83b964
-0, 118, 118, 1, 1566720, 0x25dc30a6
-0, 119, 119, 1, 1566720, 0x25dc30a6
-0, 120, 120, 1, 1566720, 0x25dc30a6
+0, 119, 119, 1, 1566720, 0xfe83b964
--
2.30.2
[-- Attachment #3: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-16 21:04 ` Tomas Härdin
@ 2022-06-16 23:38 ` Michael Niedermayer
2022-06-17 9:42 ` Tomas Härdin
0 siblings, 1 reply; 13+ messages in thread
From: Michael Niedermayer @ 2022-06-16 23:38 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1519 bytes --]
On Thu, Jun 16, 2022 at 11:04:01PM +0200, Tomas Härdin wrote:
> tor 2022-06-16 klockan 20:27 +0200 skrev Michael Niedermayer:
> >
> > >
> > > 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
>
> Uhm, the existing code just abort()s if threads are disabled? I'm not
> really sure if there anything that can or should be done there
Before your patches fate passes with --disable-pthreads
afterwards it will fail during build because the function mismatches,
the abort should not be reachable i hope
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-16 18:27 ` Michael Niedermayer
@ 2022-06-16 21:04 ` Tomas Härdin
2022-06-16 23:38 ` Michael Niedermayer
0 siblings, 1 reply; 13+ messages in thread
From: Tomas Härdin @ 2022-06-16 21:04 UTC (permalink / raw)
To: FFmpeg development discussions and patches
tor 2022-06-16 klockan 20:27 +0200 skrev Michael Niedermayer:
>
> >
> > 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
Uhm, the existing code just abort()s if threads are disabled? I'm not
really sure if there anything that can or should be done there
/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
2022-06-16 12:04 Tomas Härdin
@ 2022-06-16 18:27 ` Michael Niedermayer
2022-06-16 21:04 ` Tomas Härdin
0 siblings, 1 reply; 13+ messages in thread
From: Michael Niedermayer @ 2022-06-16 18:27 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 6046 bytes --]
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@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
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
* [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes
@ 2022-06-16 12:04 Tomas Härdin
2022-06-16 18:27 ` Michael Niedermayer
0 siblings, 1 reply; 13+ messages in thread
From: Tomas Härdin @ 2022-06-16 12:04 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0001-Make-execute-and-execute2-return-FFMIN-of-thread-ret.patch --]
[-- Type: text/x-patch, Size: 13804 bytes --]
From 2895c86dd908f6ddf3562d81050c50ea697a0bad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@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
---
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(-)
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 5f6e71a39e..49f0fd06fb 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -44,28 +44,30 @@
int avcodec_default_execute(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2), void *arg, int *ret, int count, int size)
{
- int i;
+ int i, rr = 0;
for (i = 0; i < count; i++) {
int r = func(c, (char *)arg + i * size);
+ rr = FFMIN(rr, r);
if (ret)
ret[i] = r;
}
emms_c();
- return 0;
+ return rr;
}
int avcodec_default_execute2(AVCodecContext *c, int (*func)(AVCodecContext *c2, void *arg2, int jobnr, int threadnr), void *arg, int *ret, int count)
{
- int i;
+ int i, rr = 0;
for (i = 0; i < count; i++) {
int r = func(c, arg, i, 0);
+ rr = FFMIN(rr, r);
if (ret)
ret[i] = r;
}
emms_c();
- return 0;
+ return rr;
}
static AVMutex codec_mutex = AV_MUTEX_INITIALIZER;
diff --git a/libavcodec/pthread_slice.c b/libavcodec/pthread_slice.c
index 0ad1965a22..5f02b9b6a1 100644
--- a/libavcodec/pthread_slice.c
+++ b/libavcodec/pthread_slice.c
@@ -57,13 +57,13 @@ typedef struct SliceThreadContext {
pthread_mutex_t *progress_mutex;
} SliceThreadContext;
-static void main_function(void *priv) {
+static int main_function(void *priv) {
AVCodecContext *avctx = priv;
SliceThreadContext *c = avctx->internal->thread_ctx;
- c->mainfunc(avctx);
+ return c->mainfunc(avctx);
}
-static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
+static int worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
{
AVCodecContext *avctx = priv;
SliceThreadContext *c = avctx->internal->thread_ctx;
@@ -73,6 +73,7 @@ static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb
: c->func2(avctx, c->args, jobnr, threadnr);
if (c->rets)
c->rets[jobnr] = ret;
+ return ret;
}
void ff_slice_thread_free(AVCodecContext *avctx)
@@ -131,7 +132,7 @@ int ff_slice_thread_init(AVCodecContext *avctx)
{
SliceThreadContext *c;
int thread_count = avctx->thread_count;
- void (*mainfunc)(void *);
+ int (*mainfunc)(void *);
// We cannot do this in the encoder init as the threads are created before
if (av_codec_is_encoder(avctx->codec) &&
diff --git a/libavfilter/pthread.c b/libavfilter/pthread.c
index 1a063d3cc0..8cec278be0 100644
--- a/libavfilter/pthread.c
+++ b/libavfilter/pthread.c
@@ -43,12 +43,13 @@ typedef struct ThreadContext {
int *rets;
} ThreadContext;
-static void worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
+static int worker_func(void *priv, int jobnr, int threadnr, int nb_jobs, int nb_threads)
{
ThreadContext *c = priv;
int ret = c->func(c->ctx, c->arg, jobnr, nb_jobs);
if (c->rets)
c->rets[jobnr] = ret;
+ return ret;
}
static void slice_thread_uninit(ThreadContext *c)
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;
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),
int nb_threads);
/**
@@ -41,7 +41,7 @@ int avpriv_slicethread_create(AVSliceThread **pctx, void *priv,
* @param nb_jobs number of jobs, must be > 0
* @param execute_main also execute main_func
*/
-void avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main);
+int avpriv_slicethread_execute(AVSliceThread *ctx, int nb_jobs, int execute_main);
/**
* Destroy slice threading context.
diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 7b40f49da4..2f9a0b5a7c 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -1211,8 +1211,8 @@ int attribute_align_arg sws_scale(struct SwsContext *c,
dst, dstStride, 0, c->dstH);
}
-void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
- int nb_jobs, int nb_threads)
+int ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
+ int nb_jobs, int nb_threads)
{
SwsContext *parent = priv;
SwsContext *c = parent->slice_ctx[threadnr];
@@ -1241,4 +1241,5 @@ void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
}
parent->slice_err[threadnr] = err;
+ return err;
}
diff --git a/libswscale/swscale_internal.h b/libswscale/swscale_internal.h
index e118b54457..eab3e26331 100644
--- a/libswscale/swscale_internal.h
+++ b/libswscale/swscale_internal.h
@@ -1135,8 +1135,8 @@ void ff_init_vscale_pfn(SwsContext *c, yuv2planar1_fn yuv2plane1, yuv2planarX_fn
yuv2interleavedX_fn yuv2nv12cX, yuv2packed1_fn yuv2packed1, yuv2packed2_fn yuv2packed2,
yuv2packedX_fn yuv2packedX, yuv2anyX_fn yuv2anyX, int use_mmx);
-void ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
- int nb_jobs, int nb_threads);
+int ff_sws_slice_worker(void *priv, int jobnr, int threadnr,
+ int nb_jobs, int nb_threads);
//number of extra lines to process
#define MAX_LINES_AHEAD 4
diff --git a/tests/ref/fate/fic-avi b/tests/ref/fate/fic-avi
index df55789d54..4546f230b1 100644
--- a/tests/ref/fate/fic-avi
+++ b/tests/ref/fate/fic-avi
@@ -76,19 +76,18 @@
0, 70, 70, 1, 1566720, 0x40f7d39a
0, 71, 71, 1, 1566720, 0x40f7d39a
0, 72, 72, 1, 1566720, 0x40f7d39a
-0, 73, 73, 1, 1566720, 0xa7d6e25f
-0, 74, 74, 1, 1566720, 0xa7d6e25f
-0, 75, 75, 1, 1566720, 0xa7d6e25f
-0, 76, 76, 1, 1566720, 0xa7d6e25f
-0, 77, 77, 1, 1566720, 0xa7d6e25f
-0, 78, 78, 1, 1566720, 0xa7d6e25f
-0, 79, 79, 1, 1566720, 0xa7d6e25f
-0, 80, 80, 1, 1566720, 0xa7d6e25f
-0, 81, 81, 1, 1566720, 0xa7d6e25f
-0, 82, 82, 1, 1566720, 0xa7d6e25f
-0, 83, 83, 1, 1566720, 0xa7d6e25f
-0, 84, 84, 1, 1566720, 0xa7d6e25f
-0, 85, 85, 1, 1566720, 0xa7d6e25f
+0, 74, 74, 1, 1566720, 0x40f7d39a
+0, 75, 75, 1, 1566720, 0x40f7d39a
+0, 76, 76, 1, 1566720, 0x40f7d39a
+0, 77, 77, 1, 1566720, 0x40f7d39a
+0, 78, 78, 1, 1566720, 0x40f7d39a
+0, 79, 79, 1, 1566720, 0x40f7d39a
+0, 80, 80, 1, 1566720, 0x40f7d39a
+0, 81, 81, 1, 1566720, 0x40f7d39a
+0, 82, 82, 1, 1566720, 0x40f7d39a
+0, 83, 83, 1, 1566720, 0x40f7d39a
+0, 84, 84, 1, 1566720, 0x40f7d39a
+0, 85, 85, 1, 1566720, 0x40f7d39a
0, 86, 86, 1, 1566720, 0xa7d6e25f
0, 87, 87, 1, 1566720, 0xa7d6e25f
0, 88, 88, 1, 1566720, 0xa7d6e25f
@@ -104,7 +103,6 @@
0, 98, 98, 1, 1566720, 0xa7d6e25f
0, 99, 99, 1, 1566720, 0xa7d6e25f
0, 100, 100, 1, 1566720, 0xeaf8d207
-0, 101, 101, 1, 1566720, 0x6724983e
0, 102, 102, 1, 1566720, 0x0e95d209
0, 103, 103, 1, 1566720, 0x0e95d209
0, 104, 104, 1, 1566720, 0x0e95d209
@@ -121,6 +119,4 @@
0, 115, 115, 1, 1566720, 0xfe83b964
0, 116, 116, 1, 1566720, 0xfe83b964
0, 117, 117, 1, 1566720, 0xfe83b964
-0, 118, 118, 1, 1566720, 0x25dc30a6
-0, 119, 119, 1, 1566720, 0x25dc30a6
-0, 120, 120, 1, 1566720, 0x25dc30a6
+0, 119, 119, 1, 1566720, 0xfe83b964
--
2.30.2
[-- Attachment #3: Type: text/plain, Size: 251 bytes --]
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-07-05 16:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-30 12:42 [FFmpeg-devel] [PATCH] Make execute() and execute2() return FFMIN() of thread return codes Tomas Härdin
2022-07-02 9:43 ` Anton Khirnov
2022-07-04 10:46 ` Tomas Härdin
2022-07-05 16:42 ` Anton Khirnov
-- strict thread matches above, loose matches on Subject: below --
2022-06-16 12:04 Tomas Härdin
2022-06-16 18:27 ` Michael Niedermayer
2022-06-16 21:04 ` Tomas Härdin
2022-06-16 23:38 ` Michael Niedermayer
2022-06-17 9:42 ` Tomas Härdin
2022-06-18 14:38 ` Anton Khirnov
2022-06-21 7:51 ` Tomas Härdin
2022-06-21 7:55 ` Anton Khirnov
2022-06-21 8:05 ` Tomas Härdin
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git