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

[Nuo Mi]

On Tue, Sep 24, 2024 at 10:24 PM Zhao Zhili <quinkblack at foxmail.com> wrote:

>
>
> > On Sep 24, 2024, at 22:16, Nuo Mi <nuomi2021 at gmail.com> wrote:
> >
> > 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(-)
> >
> > 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;
>
> Unfortunately executor.h is a public header, this is API/ABI break.
>
No one uses it except the vvc decoder.
I can update the libavutils API version

>
> > --
> > 2.34.1
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
>