diff mbox series

[FFmpeg-devel,1/2] avutil/executor: Allowing thread_count be zero

Message ID tencent_FF44F3E5B36C75DAC90DEEA7255073030805@qq.com
State New
Headers show
Series [FFmpeg-devel,1/2] avutil/executor: Allowing thread_count be zero | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Zhao Zhili June 24, 2024, 4:47 a.m. UTC
From: Zhao Zhili <zhilizhao@tencent.com>

Before the patch, disable threads support at configure/build time
was the only method to force zero thread in executor. However,
it's common practice for libavcodec to run on caller's thread when
user specify thread number to one. And for WASM environment, whether
threads are supported needs to be detected at runtime. So executor
should support zero thread at runtime.

A single thread executor can be useful, e.g., to handle network
protocol. So we can't take thread_count one as zero thread, which
disabled a valid usercase.

Other libraries take -threads 0 to mean auto. Executor as a low
level utils doesn't do cpu detect. So take thread_count zero as
zero thread, literally.

Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
---
 libavutil/executor.c | 28 ++++++++++++++++++----------
 libavutil/executor.h |  2 +-
 2 files changed, 19 insertions(+), 11 deletions(-)

Comments

Nuo Mi June 25, 2024, 12:36 p.m. UTC | #1
On Mon, Jun 24, 2024 at 12:48 PM Zhao Zhili <quinkblack@foxmail.com> wrote:

> From: Zhao Zhili <zhilizhao@tencent.com>
>
> Before the patch, disable threads support at configure/build time
> was the only method to force zero thread in executor. However,
> it's common practice for libavcodec to run on caller's thread when
> user specify thread number to one. And for WASM environment, whether
> threads are supported needs to be detected at runtime. So executor
> should support zero thread at runtime.
>
> A single thread executor can be useful, e.g., to handle network
> protocol. So we can't take thread_count one as zero thread, which
> disabled a valid usercase.
>
> Other libraries take -threads 0 to mean auto. Executor as a low
> level utils doesn't do cpu detect. So take thread_count zero as
> zero thread, literally.
>
> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
>
Hi Zhili,
Thank you for the v2
I will merge it in 3 days if there are no objections.
Nuo Mi June 27, 2024, 12:57 p.m. UTC | #2
On Tue, Jun 25, 2024 at 8:36 PM Nuo Mi <nuomi2021@gmail.com> wrote:

>
>
> On Mon, Jun 24, 2024 at 12:48 PM Zhao Zhili <quinkblack@foxmail.com>
> wrote:
>
>> From: Zhao Zhili <zhilizhao@tencent.com>
>>
>> Before the patch, disable threads support at configure/build time
>> was the only method to force zero thread in executor. However,
>> it's common practice for libavcodec to run on caller's thread when
>> user specify thread number to one. And for WASM environment, whether
>> threads are supported needs to be detected at runtime. So executor
>> should support zero thread at runtime.
>>
>> A single thread executor can be useful, e.g., to handle network
>> protocol. So we can't take thread_count one as zero thread, which
>> disabled a valid usercase.
>>
>> Other libraries take -threads 0 to mean auto. Executor as a low
>> level utils doesn't do cpu detect. So take thread_count zero as
>> zero thread, literally.
>>
>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
>>
> Hi Zhili,
> Thank you for the v2
> I will merge it in 3 days if there are no objections.
>
Applied

>
>
diff mbox series

Patch

diff --git a/libavutil/executor.c b/libavutil/executor.c
index 26691fe157..fb20104b58 100644
--- a/libavutil/executor.c
+++ b/libavutil/executor.c
@@ -82,9 +82,11 @@  static int run_one_task(AVExecutor *e, void *lc)
         /* nothing */;
     if (*prev) {
         AVTask *t = remove_task(prev, *prev);
-        ff_mutex_unlock(&e->lock);
+        if (e->thread_count > 0)
+            ff_mutex_unlock(&e->lock);
         cb->run(t, lc, cb->user_data);
-        ff_mutex_lock(&e->lock);
+        if (e->thread_count > 0)
+            ff_mutex_lock(&e->lock);
         return 1;
     }
     return 0;
@@ -146,14 +148,17 @@  AVExecutor* av_executor_alloc(const AVTaskCallbacks *cb, int thread_count)
         return NULL;
     e->cb = *cb;
 
-    e->local_contexts = av_calloc(thread_count, e->cb.local_context_size);
+    e->local_contexts = av_calloc(FFMAX(thread_count, 1), e->cb.local_context_size);
     if (!e->local_contexts)
         goto free_executor;
 
-    e->threads = av_calloc(thread_count, sizeof(*e->threads));
+    e->threads = av_calloc(FFMAX(thread_count, 1), sizeof(*e->threads));
     if (!e->threads)
         goto free_executor;
 
+    if (!thread_count)
+        return e;
+
     has_lock = !ff_mutex_init(&e->lock, NULL);
     has_cond = !ff_cond_init(&e->cond, NULL);
 
@@ -175,9 +180,12 @@  free_executor:
 
 void av_executor_free(AVExecutor **executor)
 {
+    int thread_count;
+
     if (!executor || !*executor)
         return;
-    executor_free(*executor, 1, 1);
+    thread_count = (*executor)->thread_count;
+    executor_free(*executor, thread_count, thread_count);
     *executor = NULL;
 }
 
@@ -195,9 +203,9 @@  void av_executor_execute(AVExecutor *e, AVTask *t)
     ff_cond_signal(&e->cond);
     ff_mutex_unlock(&e->lock);
 
-#if !HAVE_THREADS
-    // We are running in a single-threaded environment, so we must handle all tasks ourselves
-    while (run_one_task(e, e->local_contexts))
-        /* nothing */;
-#endif
+    if (!e->thread_count || !HAVE_THREADS) {
+        // We are running in a single-threaded environment, so we must handle all tasks ourselves
+        while (run_one_task(e, e->local_contexts))
+            /* nothing */;
+    }
 }
diff --git a/libavutil/executor.h b/libavutil/executor.h
index c602bcb613..0eb21c10c8 100644
--- a/libavutil/executor.h
+++ b/libavutil/executor.h
@@ -46,7 +46,7 @@  typedef struct AVTaskCallbacks {
 /**
  * Alloc executor
  * @param callbacks callback structure for executor
- * @param thread_count worker thread number
+ * @param thread_count worker thread number, 0 for run on caller's thread directly
  * @return return the executor
  */
 AVExecutor* av_executor_alloc(const AVTaskCallbacks *callbacks, int thread_count);