Re: [FFmpeg-devel] [PATCH 35/42] avcodec/threadprogress: Add new API for frame-threaded progress
Quoting Andreas Rheinhardt (2023-09-19 21:57:27) > The API is very similar by the ProgressFrame API, with the exception > that it no longer has an included AVFrame. Instead one can wait > on anything via a ThreadProgress. One just has to ensure that > the lifetime of the object containing the ThreadProgress is long > enough (the corresponding problem for ProgressFrames is solved > by allocating the progress and giving each thread a reference > to it). This will typically be solved by putting a ThreadProgress > in a refcounted structure that is shared between threads. > It will be put to the test in the following commits. > > An alternative to the check for whether the owner exists > (meaning "do we use frame-threading?") would be to initialize > the progress to INT_MAX in case frame threading is not in use. > This would probably be preferable on arches where atomic reads > are cheap (like x86), but are there ones where it is not? > > One could also (guarded by e.g. an ASSERT_LEVEL check) actually > track the progress for non-framethreading, too, in order to > track errors even in single-threaded mode. > > Signed-off-by: Andreas Rheinhardt > --- > libavcodec/pthread_frame.c | 50 + > libavcodec/threadprogress.h | 37 +++ > 2 files changed, 87 insertions(+) > create mode 100644 libavcodec/threadprogress.h Again, I'd prefer to do without owner. -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 35/42] avcodec/threadprogress: Add new API for frame-threaded progress
Michael Niedermayer:
> On Tue, Sep 19, 2023 at 09:57:27PM +0200, Andreas Rheinhardt wrote:
>> The API is very similar by the ProgressFrame API, with the exception
>> that it no longer has an included AVFrame. Instead one can wait
>> on anything via a ThreadProgress. One just has to ensure that
>> the lifetime of the object containing the ThreadProgress is long
>> enough (the corresponding problem for ProgressFrames is solved
>> by allocating the progress and giving each thread a reference
>> to it). This will typically be solved by putting a ThreadProgress
>> in a refcounted structure that is shared between threads.
>> It will be put to the test in the following commits.
>>
>> An alternative to the check for whether the owner exists
>> (meaning "do we use frame-threading?") would be to initialize
>> the progress to INT_MAX in case frame threading is not in use.
>> This would probably be preferable on arches where atomic reads
>> are cheap (like x86), but are there ones where it is not?
>>
>> One could also (guarded by e.g. an ASSERT_LEVEL check) actually
>> track the progress for non-framethreading, too, in order to
>> track errors even in single-threaded mode.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavcodec/pthread_frame.c | 50 +
>> libavcodec/threadprogress.h | 37 +++
>> 2 files changed, 87 insertions(+)
>> create mode 100644 libavcodec/threadprogress.h
>
> Seems to break build here with shared libs / clang
>
> CClibavcodec/pthread_frame.o
> src/libavcodec/pthread_frame.c:1108:9: error: address argument to atomic
> operation must be a pointer to non-const _Atomic type ('const atomic_int *'
> (aka 'const _Atomic(int) *') invalid)
> atomic_load_explicit(&pro->progress, memory_order_acquire) >= n)
> ^~~
> /usr/lib/llvm-6.0/lib/clang/6.0.0/include/stdatomic.h:135:30: note: expanded
> from macro 'atomic_load_explicit'
> #define atomic_load_explicit __c11_atomic_load
> ^
> src/libavcodec/pthread_frame.c:1116:12: error: address argument to atomic
> operation must be a pointer to non-const _Atomic type ('const atomic_int *'
> (aka 'const _Atomic(int) *') invalid)
> while (atomic_load_explicit(&pro->progress, memory_order_relaxed) < n)
>^~~
> /usr/lib/llvm-6.0/lib/clang/6.0.0/include/stdatomic.h:135:30: note: expanded
> from macro 'atomic_load_explicit'
> #define atomic_load_explicit __c11_atomic_load
> ^
> 2 errors generated.
> src/ffbuild/common.mak:81: recipe for target 'libavcodec/pthread_frame.o'
> failed
> make: *** [libavcodec/pthread_frame.o] Error 1
> make: Target 'all' not remade because of errors.
>
The original version of C11 did not allow to use atomic_load() on
const-qualified atomic objects (presumably because on systems that don't
have native support for atomic operations this might involve locking a
mutex or so and this would involve writes); this has since been changed
(see defect report N1807), yet your (presumably) ancient toolchain does
not reflect that.
I'll cast const away here and add a comment.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 35/42] avcodec/threadprogress: Add new API for frame-threaded progress
On Tue, Sep 19, 2023 at 09:57:27PM +0200, Andreas Rheinhardt wrote:
> The API is very similar by the ProgressFrame API, with the exception
> that it no longer has an included AVFrame. Instead one can wait
> on anything via a ThreadProgress. One just has to ensure that
> the lifetime of the object containing the ThreadProgress is long
> enough (the corresponding problem for ProgressFrames is solved
> by allocating the progress and giving each thread a reference
> to it). This will typically be solved by putting a ThreadProgress
> in a refcounted structure that is shared between threads.
> It will be put to the test in the following commits.
>
> An alternative to the check for whether the owner exists
> (meaning "do we use frame-threading?") would be to initialize
> the progress to INT_MAX in case frame threading is not in use.
> This would probably be preferable on arches where atomic reads
> are cheap (like x86), but are there ones where it is not?
>
> One could also (guarded by e.g. an ASSERT_LEVEL check) actually
> track the progress for non-framethreading, too, in order to
> track errors even in single-threaded mode.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/pthread_frame.c | 50 +
> libavcodec/threadprogress.h | 37 +++
> 2 files changed, 87 insertions(+)
> create mode 100644 libavcodec/threadprogress.h
Seems to break build here with shared libs / clang
CC libavcodec/pthread_frame.o
src/libavcodec/pthread_frame.c:1108:9: error: address argument to atomic
operation must be a pointer to non-const _Atomic type ('const atomic_int *'
(aka 'const _Atomic(int) *') invalid)
atomic_load_explicit(&pro->progress, memory_order_acquire) >= n)
^~~
/usr/lib/llvm-6.0/lib/clang/6.0.0/include/stdatomic.h:135:30: note: expanded
from macro 'atomic_load_explicit'
#define atomic_load_explicit __c11_atomic_load
^
src/libavcodec/pthread_frame.c:1116:12: error: address argument to atomic
operation must be a pointer to non-const _Atomic type ('const atomic_int *'
(aka 'const _Atomic(int) *') invalid)
while (atomic_load_explicit(&pro->progress, memory_order_relaxed) < n)
^~~
/usr/lib/llvm-6.0/lib/clang/6.0.0/include/stdatomic.h:135:30: note: expanded
from macro 'atomic_load_explicit'
#define atomic_load_explicit __c11_atomic_load
^
2 errors generated.
src/ffbuild/common.mak:81: recipe for target 'libavcodec/pthread_frame.o' failed
make: *** [libavcodec/pthread_frame.o] Error 1
make: Target 'all' not remade because of errors.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 35/42] avcodec/threadprogress: Add new API for frame-threaded progress
The API is very similar by the ProgressFrame API, with the exception
that it no longer has an included AVFrame. Instead one can wait
on anything via a ThreadProgress. One just has to ensure that
the lifetime of the object containing the ThreadProgress is long
enough (the corresponding problem for ProgressFrames is solved
by allocating the progress and giving each thread a reference
to it). This will typically be solved by putting a ThreadProgress
in a refcounted structure that is shared between threads.
It will be put to the test in the following commits.
An alternative to the check for whether the owner exists
(meaning "do we use frame-threading?") would be to initialize
the progress to INT_MAX in case frame threading is not in use.
This would probably be preferable on arches where atomic reads
are cheap (like x86), but are there ones where it is not?
One could also (guarded by e.g. an ASSERT_LEVEL check) actually
track the progress for non-framethreading, too, in order to
track errors even in single-threaded mode.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/pthread_frame.c | 50 +
libavcodec/threadprogress.h | 37 +++
2 files changed, 87 insertions(+)
create mode 100644 libavcodec/threadprogress.h
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 9e827f0606..219ab16ccd 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -40,6 +40,7 @@
#include "refstruct.h"
#include "thread.h"
#include "threadframe.h"
+#include "threadprogress.h"
#include "version_major.h"
#include "libavutil/avassert.h"
@@ -1069,3 +1070,52 @@ void ff_thread_progress_await(const ProgressFrame *f,
int n)
pthread_cond_wait(&p->progress_cond, &p->progress_mutex);
pthread_mutex_unlock(&p->progress_mutex);
}
+
+void ff_thread_progress_init(ThreadProgress *pro, AVCodecContext *owner)
+{
+PerThreadContext *p = pro->owner = owner->active_thread_type &
FF_THREAD_FRAME ?
+ owner->internal->thread_ctx : NULL;
+if (!p)
+return;
+atomic_init(&pro->progress, -1);
+if (atomic_load_explicit(&p->debug_threads, memory_order_relaxed))
+av_log(owner, AV_LOG_DEBUG, "Initializing ThreadProgress %p\n",
(void*)pro);
+}
+
+void ff_thread_progress_report2(ThreadProgress *pro, int n)
+{
+PerThreadContext *p = pro->owner;
+
+if (!p ||
+atomic_load_explicit(&pro->progress, memory_order_relaxed) >= n)
+return;
+
+if (atomic_load_explicit(&p->debug_threads, memory_order_relaxed))
+av_log(p->avctx, AV_LOG_DEBUG,
+ "%p finished %d\n", (void*)pro, n);
+
+pthread_mutex_lock(&p->progress_mutex);
+
+atomic_store_explicit(&pro->progress, n, memory_order_release);
+
+pthread_cond_broadcast(&p->progress_cond);
+pthread_mutex_unlock(&p->progress_mutex);
+}
+
+void ff_thread_progress_await2(const ThreadProgress *pro, int n)
+{
+PerThreadContext *p = pro->owner;
+
+if (!p ||
+atomic_load_explicit(&pro->progress, memory_order_acquire) >= n)
+return;
+
+if (atomic_load_explicit(&p->debug_threads, memory_order_relaxed))
+av_log(p->avctx, AV_LOG_DEBUG,
+ "thread awaiting %d from %p\n", n, (void*)pro);
+
+pthread_mutex_lock(&p->progress_mutex);
+while (atomic_load_explicit(&pro->progress, memory_order_relaxed) < n)
+pthread_cond_wait(&p->progress_cond, &p->progress_mutex);
+pthread_mutex_unlock(&p->progress_mutex);
+}
diff --git a/libavcodec/threadprogress.h b/libavcodec/threadprogress.h
new file mode 100644
index 00..4f65e7da4d
--- /dev/null
+++ b/libavcodec/threadprogress.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2022 Andreas Rheinhardt
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVCODEC_THREADPROGRESS_H
+#define AVCODEC_THREADPROGRESS_H
+
+#include
+
+struct AVCodecContext;
+
+typedef struct ThreadProgress {
+atomic_int progress;
+struct PerThreadContext *owner;
+} ThreadProgress;
+
+void ff_thread_progress_init(ThreadProgress *pro, struct AVCodecContext
*owner);
+void ff_thread_progress_report2(ThreadProgress *pro, int progress);
+void ff_thread_progress_await2(const ThreadProgres
