diff mbox series

[FFmpeg-devel,1/2] avutils/executor: remove unused ready callback

Message ID 20240924141658.39574-1-nuomi2021@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] avutils/executor: remove unused ready callback | expand

Checks

Context Check Description
andriy/commit_msg_x86 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Nuo Mi Sept. 24, 2024, 2:16 p.m. UTC 
Due to the nature of multithreading, using a "ready check" mechanism may introduce a deadlock. For example:

Suppose all tasks have been submitted to the executor, and the last thread checks the entire list and finds
no ready tasks. It then goes to sleep, waiting for a new task. However, for some multithreading-related reason,
a task becomes ready after the check. Since no other thread is aware of this and no new tasks are being added to
the executor, a deadlock occurs.

In VVC, this function is unnecessary because we use a scoreboard. All tasks submitted to the executor are ready tasks.
---
 libavcodec/vvc/thread.c | 8 --------
 libavutil/executor.c    | 6 ++----
 libavutil/executor.h    | 3 ---
 3 files changed, 2 insertions(+), 15 deletions(-)

Comments

Nuo Mi Oct. 1, 2024, 6:59 a.m. UTC | #1 
On Wed, Sep 25, 2024 at 8:47 PM Anton Khirnov <anton@khirnov.net> wrote:

> Quoting Nuo Mi (2024-09-25 14:37:43)
> > On Tue, Sep 24, 2024 at 11:44 PM Anton Khirnov <anton@khirnov.net>
> wrote:
> >
> > > Quoting Nuo Mi (2024-09-24 16:56:46)
> > > > On Tue, Sep 24, 2024 at 10:43 PM Nuo Mi <nuomi2021@gmail.com> wrote:
> > > >
> > > > Currently, the pure decode speed for 8K is around 27–30 fps.
> > > > To achieve stable 8K@30 playback, we may need to go through several
> > > rounds
> > > > of refactoring and optimization for both the decoder and executor,
> with
> > > > each refactor potentially breaking the API/ABI.
> > >
> > > API and ABI breaks are not allowed outside of major bumps.
> > >
> > Okay, let me explore alternative options. Thank you for pointing that out
>
> You could also copy the entire API into libavcodec, then you could
> modify it as you wish.
>
> It might eventually be useful elsewhere though.
>
Hi Anton,
Thank you for the suggestion.
V2 has been sent.

>
> --
> 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".
>
diff mbox series

Patch

diff --git a/libavcodec/vvc/thread.c b/libavcodec/vvc/thread.c
index 86a7753c6a..6208cc1811 100644
--- a/libavcodec/vvc/thread.c
+++ b/libavcodec/vvc/thread.c
@@ -372,13 +372,6 @@  static int task_is_stage_ready(VVCTask *t, int add)
     return task_has_target_score(t, stage, score);
 }
 
-static int task_ready(const AVTask *_t, void *user_data)
-{
-    VVCTask *t = (VVCTask*)_t;
-
-    return task_is_stage_ready(t, 0);
-}
-
 #define CHECK(a, b)                         \
     do {                                    \
         if ((a) != (b))                     \
@@ -689,7 +682,6 @@  AVExecutor* ff_vvc_executor_alloc(VVCContext *s, const int thread_count)
         s,
         sizeof(VVCLocalContext),
         task_priority_higher,
-        task_ready,
         task_run,
     };
     return av_executor_alloc(&callbacks, thread_count);
diff --git a/libavutil/executor.c b/libavutil/executor.c
index bfce2ac444..64e6cc0775 100644
--- a/libavutil/executor.c
+++ b/libavutil/executor.c
@@ -79,10 +79,8 @@  static void add_task(AVTask **prev, AVTask *t)
 static int run_one_task(AVExecutor *e, void *lc)
 {
     AVTaskCallbacks *cb = &e->cb;
-    AVTask **prev;
+    AVTask **prev = &e->tasks;
 
-    for (prev = &e->tasks; *prev && !cb->ready(*prev, cb->user_data); prev = &(*prev)->next)
-        /* nothing */;
     if (*prev) {
         AVTask *t = remove_task(prev, *prev);
         if (e->thread_count > 0)
@@ -143,7 +141,7 @@  AVExecutor* av_executor_alloc(const AVTaskCallbacks *cb, int thread_count)
 {
     AVExecutor *e;
     int has_lock = 0, has_cond = 0;
-    if (!cb || !cb->user_data || !cb->ready || !cb->run || !cb->priority_higher)
+    if (!cb || !cb->user_data || !cb->run || !cb->priority_higher)
         return NULL;
 
     e = av_mallocz(sizeof(*e));
diff --git a/libavutil/executor.h b/libavutil/executor.h
index 0eb21c10c8..7af53c92ce 100644
--- a/libavutil/executor.h
+++ b/libavutil/executor.h
@@ -36,9 +36,6 @@  typedef struct AVTaskCallbacks {
     // return 1 if a's priority > b's priority
     int (*priority_higher)(const AVTask *a, const AVTask *b);
 
-    // task is ready for run
-    int (*ready)(const AVTask *t, void *user_data);
-
     // run the task
     int (*run)(AVTask *t, void *local_context, void *user_data);
 } AVTaskCallbacks;