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

[Nuo Mi]

On Tue, Sep 24, 2024 at 10:43 PM Nuo Mi <nuomi2021 at gmail.com> wrote:

>
>
> 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
>

Currently, the pure decode speed for 8K is around 27–30 fps.
To achieve stable 8K at 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.

>
>> > --
>> > 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".
>>
>>