* [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress
@ 2024-04-08 19:51 Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API Andreas Rheinhardt
` (28 more replies)
0 siblings, 29 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 19:51 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
The API is similar to the ThreadFrame API, with the exception
that it no longer has an included AVFrame and that it has its
own mutexes and condition variables which makes it more independent
of pthread_frame.c. 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. This will typically be solved
by putting a ThreadProgress in a refcounted structure that is
shared between threads.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Now including its own mutex and condition variable. Hopefully
no system has constraints on the number of mutexes and condition
variables it supports.
libavcodec/threadprogress.c | 73 +++++++++++++++++++++++++++++
libavcodec/threadprogress.h | 91 +++++++++++++++++++++++++++++++++++++
2 files changed, 164 insertions(+)
create mode 100644 libavcodec/threadprogress.c
create mode 100644 libavcodec/threadprogress.h
diff --git a/libavcodec/threadprogress.c b/libavcodec/threadprogress.c
new file mode 100644
index 0000000000..dd2a4d2336
--- /dev/null
+++ b/libavcodec/threadprogress.c
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
+ *
+ * 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
+ */
+
+#include <limits.h>
+#include <stdatomic.h>
+
+#include "pthread_internal.h"
+#include "threadprogress.h"
+#include "libavutil/attributes.h"
+
+DEFINE_OFFSET_ARRAY(ThreadProgress, thread_progress, init,
+ (offsetof(ThreadProgress, progress_mutex)),
+ (offsetof(ThreadProgress, progress_cond)));
+
+av_cold int ff_thread_progress_init(ThreadProgress *pro, int init_mode)
+{
+ atomic_init(&pro->progress, init_mode ? -1 : INT_MAX);
+ if (!init_mode) {
+ pro->init = 0;
+ return 0;
+ }
+ return ff_pthread_init(pro, thread_progress_offsets);
+}
+
+av_cold void ff_thread_progress_destroy(ThreadProgress *pro)
+{
+ ff_pthread_free(pro, thread_progress_offsets);
+}
+
+void ff_thread_progress_report(ThreadProgress *pro, int n)
+{
+ if (atomic_load_explicit(&pro->progress, memory_order_relaxed) >= n)
+ return;
+
+ atomic_store_explicit(&pro->progress, n, memory_order_release);
+
+ pthread_mutex_lock(&pro->progress_mutex);
+ pthread_cond_broadcast(&pro->progress_cond);
+ pthread_mutex_unlock(&pro->progress_mutex);
+}
+
+void ff_thread_progress_await(const ThreadProgress *pro_c, int n)
+{
+ /* Casting const away here is safe, because we only read from progress
+ * and will leave pro_c in the same state upon leaving the function
+ * as it had at the beginning. */
+ ThreadProgress *pro = (ThreadProgress*)pro_c;
+
+ if (atomic_load_explicit(&pro->progress, memory_order_acquire) >= n)
+ return;
+
+ pthread_mutex_lock(&pro->progress_mutex);
+ while (atomic_load_explicit(&pro->progress, memory_order_relaxed) < n)
+ pthread_cond_wait(&pro->progress_cond, &pro->progress_mutex);
+ pthread_mutex_unlock(&pro->progress_mutex);
+}
diff --git a/libavcodec/threadprogress.h b/libavcodec/threadprogress.h
new file mode 100644
index 0000000000..786802cbf1
--- /dev/null
+++ b/libavcodec/threadprogress.h
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
+ *
+ * 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
+
+/**
+ * ThreadProgress is an API to easily notify other threads about progress
+ * of any kind as long as it can be packaged into an int and is consistent
+ * with the natural ordering of integers.
+ *
+ * Each initialized ThreadProgress can be in one of two modes: No-op mode
+ * or ordinary mode. In the former mode, ff_thread_report_progress() and
+ * ff_thread_await_progress() are no-ops to simply support usecases like
+ * non-frame-threading. Only in the latter case perform these functions
+ * what their name already implies.
+ */
+
+#include <limits.h>
+#include <stdatomic.h>
+#include "libavutil/thread.h"
+
+/**
+ * This struct should be treated as opaque by users.
+ */
+typedef struct ThreadProgress {
+ atomic_int progress;
+ unsigned init;
+ AVMutex progress_mutex;
+ AVCond progress_cond;
+} ThreadProgress;
+
+/**
+ * Initialize a ThreadProgress.
+ *
+ * @param init_mode If zero, the ThreadProgress will be initialized so that
+ * to be in no-op mode as described above. Otherwise
+ */
+int ff_thread_progress_init(ThreadProgress *pro, int init_mode);
+
+/**
+ * Destroy a ThreadProgress. Can be called on a ThreadProgress that
+ * has never been initialized provided that the ThreadProgress struct
+ * has been initially zeroed. Must be called even if ff_thread_progress_init()
+ * failed.
+ */
+void ff_thread_progress_destroy(ThreadProgress *pro);
+
+/**
+ * Reset the ThreadProgress's progress progress counter; must only be called
+ * if it is not in use in any way (e.g. no thread may wait on it via
+ * ff_thread_progress_await()).
+ */
+static inline void ff_thread_progress_reset(ThreadProgress *pro)
+{
+ atomic_init(&pro->progress, pro->init ? -1 : INT_MAX);
+}
+
+/**
+ * This function is a no-op in no-op mode; otherwise it notifies
+ * other threads that a certain level of progress has been reached.
+ * Later calls with lower values of progress have no effect.
+ */
+void ff_thread_progress_report(ThreadProgress *pro, int progress);
+
+/**
+ * This function is a no-op in no-op mode; otherwise it waits
+ * until other threads have reached a certain level of progress:
+ * This function will return after another thread has called
+ * ff_thread_progress_report() with the same or higher value for progress.
+ */
+void ff_thread_progress_await(const ThreadProgress *pro, int progress);
+
+#endif /* AVCODEC_THREADPROGRESS_H */
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-09 1:40 ` Michael Niedermayer
2024-04-10 7:01 ` Anton Khirnov
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 03/27] avcodec/mimic: Switch to ProgressFrames Andreas Rheinhardt
` (27 subsequent siblings)
28 siblings, 2 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Frame-threaded decoders with inter-frame dependencies
use the ThreadFrame API for syncing. It works as follows:
During init each thread allocates an AVFrame for every
ThreadFrame.
Thread A reads the header of its packet and allocates
a buffer for an AVFrame with ff_thread_get_ext_buffer()
(which also allocates a small structure that is shared
with other references to this frame) and sets its fields,
including side data. Then said thread calls ff_thread_finish_setup().
From that moment onward it is not allowed to change any
of the AVFrame fields at all any more, but it may change
fields which are an indirection away, like the content of
AVFrame.data or already existing side data.
After thread A has called ff_thread_finish_setup(),
another thread (the user one) calls the codec's update_thread_context
callback which in turn calls ff_thread_ref_frame() which
calls av_frame_ref() which reads every field of A's
AVFrame; hence the above restriction on modifications
of the AVFrame (as any modification of the AVFrame by A after
ff_thread_finish_setup() would be a data race). Of course,
this av_frame_ref() also incurs allocations and therefore
needs to be checked. ff_thread_ref_frame() also references
the small structure used for communicating progress.
This av_frame_ref() makes it awkward to propagate values that
only become known during decoding to later threads (in case of
frame reordering or other mechanisms of delayed output (like
show-existing-frames) it's not the decoding thread, but a later
thread that returns the AVFrame). E.g. for VP9 when exporting video
encoding parameters as side data the number of blocks only
becomes known during decoding, so one can't allocate the side data
before ff_thread_finish_setup(). It is currently being done afterwards
and this leads to a data race in the vp9-encparams test when using
frame-threading. Returning decode_error_flags is also complicated
by this.
To perform this exchange a buffer shared between the references
is needed (notice that simply giving the later threads a pointer
to the original AVFrame does not work, because said AVFrame will
be reused lateron when thread A decodes the next packet given to it).
One could extend the buffer already used for progress for this
or use a new one (requiring yet another allocation), yet both
of these approaches have the drawback of being unnatural, ugly
and requiring quite a lot of ad-hoc code. E.g. in case of the VP9
side data mentioned above one could not simply use the helper
that allocates and adds the side data to an AVFrame in one go.
The ProgressFrame API meanwhile offers a different solution to all
of this. It is based around the idea that the most natural
shared object for sharing information about an AVFrame between
decoding threads is the AVFrame itself. To actually implement this
the AVFrame needs to be reference counted. This is achieved by
putting a (ownership) pointer into a shared (and opaque) structure
that is managed by the RefStruct API and which also contains
the stuff necessary for progress reporting.
The users get a pointer to this AVFrame with the understanding
that the owner may set all the fields until it has indicated
that it has finished decoding this AVFrame; then the users are
allowed to read everything. Every decoder may of course employ
a different contract than the one outlined above.
Given that there is no underlying av_frame_ref(), creating
references to a ProgressFrame can't fail. Only
ff_thread_progress_get_buffer() can fail, but given that
it will replace calls to ff_thread_get_ext_buffer() it is
at places where errors are already expected and properly
taken care of.
The ProgressFrames are empty (i.e. the AVFrame pointer is NULL
and the AVFrames are not allocated during init at all)
while not being in use; ff_thread_progress_get_buffer() both
sets up the actual ProgressFrame and already calls
ff_thread_get_buffer(). So instead of checking for
ThreadFrame.f->data[0] or ThreadFrame.f->buf[0] being NULL
for "this reference frame is non-existing" one should check for
ProgressFrame.f.
This also implies that one can only set AVFrame properties
after having allocated the buffer. This restriction is not deep:
if it becomes onerous for any codec, ff_thread_progress_get_buffer()
can be broken up. The user would then have to get a buffer
himself.
In order to avoid unnecessary allocations, the shared structure
is pooled, so that both the structure as well as the AVFrame
itself are reused. This means that there won't be lots of
unnecessary allocations in case of non-frame-threaded decoding.
It might even turn out to have fewer than the current code
(the current code allocates AVFrames for every DPB slot, but
these are often excessively large and not completely used;
the new code allocates them on demand). Pooling relies on the
reset function of the RefStruct pool API, it would be impossible
to implement with the AVBufferPool API.
Finally, ProgressFrames have no notion of owner; they are built
on top of the ThreadProgress API which also lacks such a concept.
Instead every ThreadProgress and every ProgressFrame contains
its own mutex and condition variable, making it completely independent
of pthread_frame.c. Just like the ThreadFrame API it is simply
presumed that only the actual owner/producer of a frame reports
progress on said frame.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/Makefile | 1 +
libavcodec/avcodec.c | 1 +
libavcodec/codec_internal.h | 4 ++
libavcodec/decode.c | 122 +++++++++++++++++++++++++++++++++
libavcodec/internal.h | 2 +
libavcodec/progressframe.h | 133 ++++++++++++++++++++++++++++++++++++
libavcodec/pthread_frame.c | 1 +
libavcodec/tests/avcodec.c | 3 +-
8 files changed, 266 insertions(+), 1 deletion(-)
create mode 100644 libavcodec/progressframe.h
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 7f6de4470e..472568bd6c 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -56,6 +56,7 @@ OBJS = ac3_parser.o \
qsv_api.o \
raw.o \
refstruct.o \
+ threadprogress.o \
utils.o \
version.o \
vlc.o \
diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 525fe516bd..888dd76228 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -441,6 +441,7 @@ av_cold void ff_codec_close(AVCodecContext *avctx)
av_frame_free(&avci->recon_frame);
ff_refstruct_unref(&avci->pool);
+ ff_refstruct_pool_uninit(&avci->progress_frame_pool);
ff_hwaccel_uninit(avctx);
diff --git a/libavcodec/codec_internal.h b/libavcodec/codec_internal.h
index d6757e2def..832e6440d7 100644
--- a/libavcodec/codec_internal.h
+++ b/libavcodec/codec_internal.h
@@ -62,6 +62,10 @@
* Codec initializes slice-based threading with a main function
*/
#define FF_CODEC_CAP_SLICE_THREAD_HAS_MF (1 << 5)
+/**
+ * The decoder might make use of the ProgressFrame API.
+ */
+#define FF_CODEC_CAP_USES_PROGRESSFRAMES (1 << 11)
/*
* The codec supports frame threading and has inter-frame dependencies, so it
* uses ff_thread_report/await_progress().
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 255347766a..f18f85c33c 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -49,8 +49,10 @@
#include "hwconfig.h"
#include "internal.h"
#include "packet_internal.h"
+#include "progressframe.h"
#include "refstruct.h"
#include "thread.h"
+#include "threadprogress.h"
typedef struct DecodeContext {
AVCodecInternal avci;
@@ -1668,6 +1670,116 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
return ret;
}
+typedef struct ProgressInternal {
+ ThreadProgress progress;
+ struct AVFrame *f;
+} ProgressInternal;
+
+static void check_progress_consistency(const ProgressFrame *f)
+{
+ av_assert1(!!f->f == !!f->progress);
+ av_assert1(!f->progress || f->progress->f == f->f);
+}
+
+static int progress_frame_get(AVCodecContext *avctx, ProgressFrame *f)
+{
+ FFRefStructPool *pool = avctx->internal->progress_frame_pool;
+
+ av_assert1(!f->f && !f->progress);
+
+ f->progress = ff_refstruct_pool_get(pool);
+ if (!f->progress)
+ return AVERROR(ENOMEM);
+
+ f->f = f->progress->f;
+ return 0;
+}
+
+int ff_progress_frame_get_buffer(AVCodecContext *avctx, ProgressFrame *f, int flags)
+{
+ int ret;
+
+ ret = progress_frame_get(avctx, f);
+ if (ret < 0)
+ return ret;
+
+ ret = ff_thread_get_buffer(avctx, f->progress->f, flags);
+ if (ret < 0) {
+ f->f = NULL;
+ ff_refstruct_unref(&f->progress);
+ return ret;
+ }
+ return 0;
+}
+
+void ff_progress_frame_ref(ProgressFrame *dst, const ProgressFrame *src)
+{
+ av_assert1(src->progress && src->f && src->f == src->progress->f);
+ av_assert1(!dst->f && !dst->progress);
+ dst->f = src->f;
+ dst->progress = ff_refstruct_ref(src->progress);
+}
+
+void ff_progress_frame_unref(ProgressFrame *f)
+{
+ check_progress_consistency(f);
+ f->f = NULL;
+ ff_refstruct_unref(&f->progress);
+}
+
+void ff_progress_frame_replace(ProgressFrame *dst, const ProgressFrame *src)
+{
+ if (dst == src)
+ return;
+ ff_progress_frame_unref(dst);
+ check_progress_consistency(src);
+ if (src->f)
+ ff_progress_frame_ref(dst, src);
+}
+
+void ff_progress_frame_report(ProgressFrame *f, int n)
+{
+ ff_thread_progress_report(&f->progress->progress, n);
+}
+
+void ff_progress_frame_await(const ProgressFrame *f, int n)
+{
+ ff_thread_progress_await(&f->progress->progress, n);
+}
+
+static av_cold int progress_frame_pool_init_cb(FFRefStructOpaque opaque, void *obj)
+{
+ const AVCodecContext *avctx = opaque.nc;
+ ProgressInternal *progress = obj;
+ int ret;
+
+ ret = ff_thread_progress_init(&progress->progress, avctx->active_thread_type & FF_THREAD_FRAME);
+ if (ret < 0)
+ return ret;
+
+ progress->f = av_frame_alloc();
+ if (!progress->f)
+ return AVERROR(ENOMEM);
+
+ return 0;
+}
+
+static void progress_frame_pool_reset_cb(FFRefStructOpaque unused, void *obj)
+{
+ ProgressInternal *progress = obj;
+
+ ff_thread_progress_reset(&progress->progress);
+ av_frame_unref(progress->f);
+}
+
+static av_cold void progress_frame_pool_free_entry_cb(FFRefStructOpaque opaque, void *obj)
+{
+ ProgressInternal *progress = obj;
+
+ ff_thread_progress_destroy(&progress->progress);
+ av_frame_free(&progress->f);
+}
+
int ff_decode_preinit(AVCodecContext *avctx)
{
AVCodecInternal *avci = avctx->internal;
@@ -1766,6 +1878,16 @@ int ff_decode_preinit(AVCodecContext *avctx)
if (!avci->in_pkt || !avci->last_pkt_props)
return AVERROR(ENOMEM);
+ if (ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_USES_PROGRESSFRAMES) {
+ avci->progress_frame_pool =
+ ff_refstruct_pool_alloc_ext(sizeof(ProgressInternal),
+ FF_REFSTRUCT_POOL_FLAG_FREE_ON_INIT_ERROR,
+ avctx, progress_frame_pool_init_cb,
+ progress_frame_pool_reset_cb,
+ progress_frame_pool_free_entry_cb, NULL);
+ if (!avci->progress_frame_pool)
+ return AVERROR(ENOMEM);
+ }
ret = decode_bsfs_init(avctx);
if (ret < 0)
return ret;
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 64fe0122c8..bc20a797ae 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -61,6 +61,8 @@ typedef struct AVCodecInternal {
struct FramePool *pool;
+ struct FFRefStructPool *progress_frame_pool;
+
void *thread_ctx;
/**
diff --git a/libavcodec/progressframe.h b/libavcodec/progressframe.h
new file mode 100644
index 0000000000..dc841f30d2
--- /dev/null
+++ b/libavcodec/progressframe.h
@@ -0,0 +1,133 @@
+/*
+ * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
+ *
+ * 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_PROGRESSFRAME_H
+#define AVCODEC_PROGRESSFRAME_H
+
+/**
+ * ProgressFrame is an API to easily share frames without an underlying
+ * av_frame_ref(). Its main usecase is in frame-threading scenarios,
+ * yet it could also be used for purely single-threaded decoders that
+ * want to keep multiple references to the same frame.
+ *
+ * The underlying principle behind the API is that all that is needed
+ * to share a frame is a reference count and a contract between all parties.
+ * The ProgressFrame provides the reference count and the frame is unreferenced
+ * via ff_thread_release_buffer() when the reference count reaches zero.
+ *
+ * In order to make this API also usable for frame-threaded decoders it also
+ * provides a way of exchanging simple information about the state of
+ * decoding the frame via ff_thread_progress_report() and
+ * ff_thread_progress_await().
+ *
+ * The typical contract for frame-threaded decoders is as follows:
+ * Thread A initializes a ProgressFrame via ff_thread_progress_get_buffer()
+ * (which already allocates the AVFrame's data buffers), calls
+ * ff_thread_finish_setup() and starts decoding the frame. Later threads
+ * receive a reference to this frame, which means they get a pointer
+ * to the AVFrame and the internal reference count gets incremented.
+ * Later threads whose frames use A's frame as reference as well as
+ * the thread that will eventually output A's frame will wait for
+ * progress on said frame reported by A. As soon as A has reported
+ * that it has finished decoding its frame, it must no longer modify it
+ * (neither its data nor its properties).
+ *
+ * Because creating a reference with this API does not involve reads
+ * from the actual AVFrame, the decoding thread may modify the properties
+ * (i.e. non-data fields) until it has indicated to be done with this
+ * frame. This is important for e.g. propagating decode_error_flags;
+ * it also allows to add side-data late.
+ */
+
+struct AVCodecContext;
+
+typedef struct ProgressFrame {
+ struct AVFrame *f;
+ struct ProgressInternal *progress;
+} ProgressFrame;
+
+/**
+ * Notify later decoding threads when part of their reference frame is ready.
+ * Call this when some part of the frame is finished decoding.
+ * Later calls with lower values of progress have no effect.
+ *
+ * @param f The frame being decoded.
+ * @param progress Value, in arbitrary units, of how much of the frame has decoded.
+ *
+ * @warning Calling this on a blank ProgressFrame causes undefined behaviour
+ */
+void ff_progress_frame_report(ProgressFrame *f, int progress);
+
+/**
+ * Wait for earlier decoding threads to finish reference frames.
+ * Call this before accessing some part of a frame, with a given
+ * value for progress, and it will return after the responsible decoding
+ * thread calls ff_thread_progress_report() with the same or
+ * higher value for progress.
+ *
+ * @param f The frame being referenced.
+ * @param progress Value, in arbitrary units, to wait for.
+ *
+ * @warning Calling this on a blank ProgressFrame causes undefined behaviour
+ */
+void ff_progress_frame_await(const ProgressFrame *f, int progress);
+
+/**
+ * This function sets up the ProgressFrame, i.e. gets ProgressFrame.f
+ * and also calls ff_thread_get_buffer() on the frame.
+ *
+ * @note: This must only be called by codecs with the
+ * FF_CODEC_CAP_USES_PROGRESSFRAMES internal cap.
+ */
+int ff_progress_frame_get_buffer(struct AVCodecContext *avctx,
+ ProgressFrame *f, int flags);
+
+/**
+ * Give up a reference to the underlying frame contained in a ProgressFrame
+ * and reset the ProgressFrame, setting all pointers to NULL.
+ *
+ * @note: This implies that when using this API the check for whether
+ * a frame exists is by checking ProgressFrame.f and not
+ * ProgressFrame.f->data[0] or ProgressFrame.f->buf[0].
+ */
+void ff_progress_frame_unref(ProgressFrame *f);
+
+/**
+ * Set dst->f to src->f and make dst a co-owner of src->f.
+ * dst can then be used to wait on progress of the underlying frame.
+ *
+ * @note: There is no underlying av_frame_ref() here. dst->f and src->f
+ * really point to the same AVFrame. Typically this means that
+ * the decoding thread is allowed to set all the properties of
+ * the AVFrame until it has indicated to have finished decoding.
+ * Afterwards later threads may read all of these fields.
+ * Access to the frame's data is governed by
+ * ff_thread_progress_report/await().
+ */
+void ff_progress_frame_ref(ProgressFrame *dst, const ProgressFrame *src);
+
+/**
+ * Do nothing if dst and src already refer to the same AVFrame;
+ * otherwise unreference dst and if src is not blank, put a reference
+ * to src's AVFrame in its place (in case src is not blank).
+ */
+void ff_progress_frame_replace(ProgressFrame *dst, const ProgressFrame *src);
+
+#endif /* AVCODEC_PROGRESSFRAME_H */
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index fd356bd190..6b2c4312e0 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -779,6 +779,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
if (!copy->internal)
return AVERROR(ENOMEM);
copy->internal->thread_ctx = p;
+ copy->internal->progress_frame_pool = avctx->internal->progress_frame_pool;
copy->delay = avctx->delay;
diff --git a/libavcodec/tests/avcodec.c b/libavcodec/tests/avcodec.c
index 08ca507bf0..58f54cac74 100644
--- a/libavcodec/tests/avcodec.c
+++ b/libavcodec/tests/avcodec.c
@@ -141,7 +141,8 @@ int main(void){
ret = 1;
}
}
- if (codec2->caps_internal & (FF_CODEC_CAP_ALLOCATE_PROGRESS |
+ if (codec2->caps_internal & (FF_CODEC_CAP_USES_PROGRESSFRAMES |
+ FF_CODEC_CAP_ALLOCATE_PROGRESS |
FF_CODEC_CAP_SETS_PKT_DTS |
FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
FF_CODEC_CAP_EXPORTS_CROPPING |
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 03/27] avcodec/mimic: Switch to ProgressFrames
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 04/27] avcodec/vp3: " Andreas Rheinhardt
` (26 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Anton Khirnov, Andreas Rheinhardt
Avoids implicit av_frame_ref() and therefore allocations
and error checks.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/mimic.c | 61 +++++++++++++++++-----------------------------
1 file changed, 22 insertions(+), 39 deletions(-)
diff --git a/libavcodec/mimic.c b/libavcodec/mimic.c
index 8928f24022..2925aa50f7 100644
--- a/libavcodec/mimic.c
+++ b/libavcodec/mimic.c
@@ -34,8 +34,8 @@
#include "bswapdsp.h"
#include "hpeldsp.h"
#include "idctdsp.h"
+#include "progressframe.h"
#include "thread.h"
-#include "threadframe.h"
#define MIMIC_HEADER_SIZE 20
#define MIMIC_VLC_BITS 11
@@ -52,7 +52,7 @@ typedef struct MimicContext {
int cur_index;
int prev_index;
- ThreadFrame frames [16];
+ ProgressFrame frames[16];
DECLARE_ALIGNED(32, int16_t, dct_block)[64];
@@ -105,16 +105,12 @@ static const uint8_t col_zag[64] = {
static av_cold int mimic_decode_end(AVCodecContext *avctx)
{
MimicContext *ctx = avctx->priv_data;
- int i;
av_freep(&ctx->swap_buf);
ctx->swap_buf_size = 0;
- for (i = 0; i < FF_ARRAY_ELEMS(ctx->frames); i++) {
- if (ctx->frames[i].f)
- ff_thread_release_ext_buffer(&ctx->frames[i]);
- av_frame_free(&ctx->frames[i].f);
- }
+ for (int i = 0; i < FF_ARRAY_ELEMS(ctx->frames); i++)
+ ff_progress_frame_unref(&ctx->frames[i]);
return 0;
}
@@ -130,7 +126,6 @@ static av_cold int mimic_decode_init(AVCodecContext *avctx)
{
static AVOnce init_static_once = AV_ONCE_INIT;
MimicContext *ctx = avctx->priv_data;
- int i;
ctx->prev_index = 0;
ctx->cur_index = 15;
@@ -141,12 +136,6 @@ static av_cold int mimic_decode_init(AVCodecContext *avctx)
ff_idctdsp_init(&ctx->idsp, avctx);
ff_permute_scantable(ctx->permutated_scantable, col_zag, ctx->idsp.idct_permutation);
- for (i = 0; i < FF_ARRAY_ELEMS(ctx->frames); i++) {
- ctx->frames[i].f = av_frame_alloc();
- if (!ctx->frames[i].f)
- return AVERROR(ENOMEM);
- }
-
ff_thread_once(&init_static_once, mimic_init_static);
return 0;
@@ -156,7 +145,6 @@ static av_cold int mimic_decode_init(AVCodecContext *avctx)
static int mimic_decode_update_thread_context(AVCodecContext *avctx, const AVCodecContext *avctx_from)
{
MimicContext *dst = avctx->priv_data, *src = avctx_from->priv_data;
- int i, ret;
if (avctx == avctx_from)
return 0;
@@ -164,13 +152,10 @@ static int mimic_decode_update_thread_context(AVCodecContext *avctx, const AVCod
dst->cur_index = src->next_cur_index;
dst->prev_index = src->next_prev_index;
- for (i = 0; i < FF_ARRAY_ELEMS(dst->frames); i++) {
- ff_thread_release_ext_buffer(&dst->frames[i]);
- if (i != src->next_cur_index && src->frames[i].f->data[0]) {
- ret = ff_thread_ref_frame(&dst->frames[i], &src->frames[i]);
- if (ret < 0)
- return ret;
- }
+ for (int i = 0; i < FF_ARRAY_ELEMS(dst->frames); i++) {
+ ff_progress_frame_unref(&dst->frames[i]);
+ if (i != src->next_cur_index && src->frames[i].f)
+ ff_progress_frame_ref(&dst->frames[i], &src->frames[i]);
}
return 0;
@@ -293,11 +278,10 @@ static int decode(MimicContext *ctx, int quality, int num_coeffs,
} else {
unsigned int backref = get_bits(&ctx->gb, 4);
int index = (ctx->cur_index + backref) & 15;
- uint8_t *p = ctx->frames[index].f->data[0];
- if (index != ctx->cur_index && p) {
- ff_thread_await_progress(&ctx->frames[index],
- cur_row, 0);
+ if (index != ctx->cur_index && ctx->frames[index].f) {
+ const uint8_t *p = ctx->frames[index].f->data[0];
+ ff_progress_frame_await(&ctx->frames[index], cur_row);
p += src -
ctx->frames[ctx->prev_index].f->data[plane];
ctx->hdsp.put_pixels_tab[1][0](dst, p, stride, 8);
@@ -307,8 +291,7 @@ static int decode(MimicContext *ctx, int quality, int num_coeffs,
}
}
} else {
- ff_thread_await_progress(&ctx->frames[ctx->prev_index],
- cur_row, 0);
+ ff_progress_frame_await(&ctx->frames[ctx->prev_index], cur_row);
ctx->hdsp.put_pixels_tab[1][0](dst, src, stride, 8);
}
src += 8;
@@ -317,8 +300,7 @@ static int decode(MimicContext *ctx, int quality, int num_coeffs,
src += (stride - ctx->num_hblocks[plane]) << 3;
dst += (stride - ctx->num_hblocks[plane]) << 3;
- ff_thread_report_progress(&ctx->frames[ctx->cur_index],
- cur_row++, 0);
+ ff_progress_frame_report(&ctx->frames[ctx->cur_index], cur_row++);
}
}
@@ -392,17 +374,18 @@ static int mimic_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
return AVERROR_PATCHWELCOME;
}
- if (is_pframe && !ctx->frames[ctx->prev_index].f->data[0]) {
+ if (is_pframe && !ctx->frames[ctx->prev_index].f) {
av_log(avctx, AV_LOG_ERROR, "decoding must start with keyframe\n");
return AVERROR_INVALIDDATA;
}
- ff_thread_release_ext_buffer(&ctx->frames[ctx->cur_index]);
+ ff_progress_frame_unref(&ctx->frames[ctx->cur_index]);
+ res = ff_progress_frame_get_buffer(avctx, &ctx->frames[ctx->cur_index],
+ AV_GET_BUFFER_FLAG_REF);
+ if (res < 0)
+ return res;
ctx->frames[ctx->cur_index].f->pict_type = is_pframe ? AV_PICTURE_TYPE_P :
AV_PICTURE_TYPE_I;
- if ((res = ff_thread_get_ext_buffer(avctx, &ctx->frames[ctx->cur_index],
- AV_GET_BUFFER_FLAG_REF)) < 0)
- return res;
ctx->next_prev_index = ctx->cur_index;
ctx->next_cur_index = (ctx->cur_index - 1) & 15;
@@ -419,10 +402,10 @@ static int mimic_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
init_get_bits(&ctx->gb, ctx->swap_buf, swap_buf_size << 3);
res = decode(ctx, quality, num_coeffs, !is_pframe);
- ff_thread_report_progress(&ctx->frames[ctx->cur_index], INT_MAX, 0);
+ ff_progress_frame_report(&ctx->frames[ctx->cur_index], INT_MAX);
if (res < 0) {
if (!(avctx->active_thread_type & FF_THREAD_FRAME))
- ff_thread_release_ext_buffer(&ctx->frames[ctx->cur_index]);
+ ff_progress_frame_unref(&ctx->frames[ctx->cur_index]);
return res;
}
@@ -449,6 +432,6 @@ const FFCodec ff_mimic_decoder = {
FF_CODEC_DECODE_CB(mimic_decode_frame),
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
UPDATE_THREAD_CONTEXT(mimic_decode_update_thread_context),
- .caps_internal = FF_CODEC_CAP_ALLOCATE_PROGRESS |
+ .caps_internal = FF_CODEC_CAP_USES_PROGRESSFRAMES |
FF_CODEC_CAP_INIT_CLEANUP,
};
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 04/27] avcodec/vp3: Switch to ProgressFrames
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 03/27] avcodec/mimic: Switch to ProgressFrames Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 05/27] avcodec/vp9: " Andreas Rheinhardt
` (25 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Anton Khirnov, Andreas Rheinhardt
Avoids implicit av_frame_ref() and therefore allocations
and error checks. It also avoids explicitly allocating
the AVFrames (done implicitly when getting the buffer)
and it also allows to reuse the flushing code for freeing
the ProgressFrames.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/vp3.c | 147 +++++++++++++++++------------------------------
1 file changed, 53 insertions(+), 94 deletions(-)
diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
index 201bab0e32..2a5f68dfa8 100644
--- a/libavcodec/vp3.c
+++ b/libavcodec/vp3.c
@@ -49,9 +49,9 @@
#include "internal.h"
#include "jpegquanttables.h"
#include "mathops.h"
+#include "progressframe.h"
#include "refstruct.h"
#include "thread.h"
-#include "threadframe.h"
#include "videodsp.h"
#include "vp3data.h"
#include "vp4data.h"
@@ -201,9 +201,9 @@ typedef struct Vp3DecodeContext {
int version;
int width, height;
int chroma_x_shift, chroma_y_shift;
- ThreadFrame golden_frame;
- ThreadFrame last_frame;
- ThreadFrame current_frame;
+ ProgressFrame golden_frame;
+ ProgressFrame last_frame;
+ ProgressFrame current_frame;
int keyframe;
uint8_t idct_permutation[64];
uint8_t idct_scantable[64];
@@ -353,12 +353,9 @@ static void vp3_decode_flush(AVCodecContext *avctx)
{
Vp3DecodeContext *s = avctx->priv_data;
- if (s->golden_frame.f)
- ff_thread_release_ext_buffer(&s->golden_frame);
- if (s->last_frame.f)
- ff_thread_release_ext_buffer(&s->last_frame);
- if (s->current_frame.f)
- ff_thread_release_ext_buffer(&s->current_frame);
+ ff_progress_frame_unref(&s->golden_frame);
+ ff_progress_frame_unref(&s->last_frame);
+ ff_progress_frame_unref(&s->current_frame);
}
static av_cold int vp3_decode_end(AVCodecContext *avctx)
@@ -372,9 +369,6 @@ static av_cold int vp3_decode_end(AVCodecContext *avctx)
/* release all frames */
vp3_decode_flush(avctx);
- av_frame_free(&s->current_frame.f);
- av_frame_free(&s->last_frame.f);
- av_frame_free(&s->golden_frame.f);
ff_refstruct_unref(&s->coeff_vlc);
@@ -1908,10 +1902,9 @@ static void vp3_draw_horiz_band(Vp3DecodeContext *s, int y)
/* At the end of the frame, report INT_MAX instead of the height of
* the frame. This makes the other threads' ff_thread_await_progress()
* calls cheaper, because they don't have to clip their values. */
- ff_thread_report_progress(&s->current_frame,
- y_flipped == s->height ? INT_MAX
- : y_flipped - 1,
- 0);
+ ff_progress_frame_report(&s->current_frame,
+ y_flipped == s->height ? INT_MAX
+ : y_flipped - 1);
}
if (!s->avctx->draw_horiz_band)
@@ -1942,7 +1935,7 @@ static void vp3_draw_horiz_band(Vp3DecodeContext *s, int y)
static void await_reference_row(Vp3DecodeContext *s, const Vp3Fragment *fragment,
int motion_y, int y)
{
- const ThreadFrame *ref_frame;
+ const ProgressFrame *ref_frame;
int ref_row;
int border = motion_y & 1;
@@ -1955,7 +1948,7 @@ static void await_reference_row(Vp3DecodeContext *s, const Vp3Fragment *fragment
ref_row = y + (motion_y >> 1);
ref_row = FFMAX(FFABS(ref_row), ref_row + 8 + border);
- ff_thread_await_progress(ref_frame, ref_row, 0);
+ ff_progress_frame_await(ref_frame, ref_row);
}
#if CONFIG_VP4_DECODER
@@ -2066,12 +2059,12 @@ static void render_slice(Vp3DecodeContext *s, int slice)
int16_t *block = s->block;
int motion_x = 0xdeadbeef, motion_y = 0xdeadbeef;
/* When decoding keyframes, the earlier frames may not be available,
- * so to avoid using undefined pointer arithmetic on them we just
- * use the current frame instead. Nothing is ever read from these
- * frames in case of a keyframe. */
- const AVFrame *last_frame = s->last_frame.f->data[0] ?
+ * so we just use the current frame in this case instead;
+ * it also avoid using undefined pointer arithmetic. Nothing is
+ * ever read from these frames in case of a keyframe. */
+ const AVFrame *last_frame = s->last_frame.f ?
s->last_frame.f : s->current_frame.f;
- const AVFrame *golden_frame = s->golden_frame.f->data[0] ?
+ const AVFrame *golden_frame = s->golden_frame.f ?
s->golden_frame.f : s->current_frame.f;
int motion_halfpel_index;
int first_pixel;
@@ -2353,17 +2346,6 @@ static av_cold int allocate_tables(AVCodecContext *avctx)
return 0;
}
-static av_cold int init_frames(Vp3DecodeContext *s)
-{
- s->current_frame.f = av_frame_alloc();
- s->last_frame.f = av_frame_alloc();
- s->golden_frame.f = av_frame_alloc();
-
- if (!s->current_frame.f || !s->last_frame.f || !s->golden_frame.f)
- return AVERROR(ENOMEM);
-
- return 0;
-}
static av_cold void free_vlc_tables(FFRefStructOpaque unused, void *obj)
{
@@ -2382,10 +2364,6 @@ static av_cold int vp3_decode_init(AVCodecContext *avctx)
int c_height;
int y_fragment_count, c_fragment_count;
- ret = init_frames(s);
- if (ret < 0)
- return ret;
-
if (avctx->codec_tag == MKTAG('V', 'P', '4', '0')) {
s->version = 3;
#if !CONFIG_VP4_DECODER
@@ -2524,61 +2502,42 @@ static av_cold int vp3_decode_init(AVCodecContext *avctx)
}
/// Release and shuffle frames after decode finishes
-static int update_frames(AVCodecContext *avctx)
+static void update_frames(AVCodecContext *avctx)
{
Vp3DecodeContext *s = avctx->priv_data;
- int ret = 0;
- if (s->keyframe) {
- ff_thread_release_ext_buffer(&s->golden_frame);
- ret = ff_thread_ref_frame(&s->golden_frame, &s->current_frame);
- }
- /* shuffle frames */
- ff_thread_release_ext_buffer(&s->last_frame);
- FFSWAP(ThreadFrame, s->last_frame, s->current_frame);
+ if (s->keyframe)
+ ff_progress_frame_replace(&s->golden_frame, &s->current_frame);
- return ret;
+ /* shuffle frames */
+ ff_progress_frame_unref(&s->last_frame);
+ FFSWAP(ProgressFrame, s->last_frame, s->current_frame);
}
#if HAVE_THREADS
-static int ref_frame(ThreadFrame *dst, const ThreadFrame *src)
+static void ref_frames(Vp3DecodeContext *dst, const Vp3DecodeContext *src)
{
- ff_thread_release_ext_buffer(dst);
- if (src->f->data[0])
- return ff_thread_ref_frame(dst, src);
- return 0;
-}
-
-static int ref_frames(Vp3DecodeContext *dst, const Vp3DecodeContext *src)
-{
- int ret;
- if ((ret = ref_frame(&dst->current_frame, &src->current_frame)) < 0 ||
- (ret = ref_frame(&dst->golden_frame, &src->golden_frame)) < 0 ||
- (ret = ref_frame(&dst->last_frame, &src->last_frame)) < 0)
- return ret;
- return 0;
+ ff_progress_frame_replace(&dst->current_frame, &src->current_frame);
+ ff_progress_frame_replace(&dst->golden_frame, &src->golden_frame);
+ ff_progress_frame_replace(&dst->last_frame, &src->last_frame);
}
static int vp3_update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
{
Vp3DecodeContext *s = dst->priv_data;
const Vp3DecodeContext *s1 = src->priv_data;
- int qps_changed = 0, err;
+ int qps_changed = 0;
ff_refstruct_replace(&s->coeff_vlc, s1->coeff_vlc);
- if (!s1->current_frame.f->data[0] ||
+ // copy previous frame data
+ ref_frames(s, s1);
+ if (!s1->current_frame.f ||
s->width != s1->width || s->height != s1->height) {
- if (s != s1)
- ref_frames(s, s1);
return -1;
}
if (s != s1) {
- // copy previous frame data
- if ((err = ref_frames(s, s1)) < 0)
- return err;
-
s->keyframe = s1->keyframe;
// copy qscale data if necessary
@@ -2600,7 +2559,8 @@ static int vp3_update_thread_context(AVCodecContext *dst, const AVCodecContext *
}
}
- return update_frames(dst);
+ update_frames(dst);
+ return 0;
}
#endif
@@ -2691,15 +2651,19 @@ static int vp3_decode_frame(AVCodecContext *avctx, AVFrame *frame,
if (avctx->skip_frame >= AVDISCARD_NONKEY && !s->keyframe)
return buf_size;
+ ret = ff_progress_frame_get_buffer(avctx, &s->current_frame,
+ AV_GET_BUFFER_FLAG_REF);
+ if (ret < 0) {
+ // Don't goto error here, as one can't report progress on or
+ // unref a non-existent frame.
+ return ret;
+ }
s->current_frame.f->pict_type = s->keyframe ? AV_PICTURE_TYPE_I
: AV_PICTURE_TYPE_P;
if (s->keyframe)
s->current_frame.f->flags |= AV_FRAME_FLAG_KEY;
else
s->current_frame.f->flags &= ~AV_FRAME_FLAG_KEY;
- if ((ret = ff_thread_get_ext_buffer(avctx, &s->current_frame,
- AV_GET_BUFFER_FLAG_REF)) < 0)
- goto error;
if (!s->edge_emu_buffer) {
s->edge_emu_buffer = av_malloc(9 * FFABS(s->current_frame.f->linesize[0]));
@@ -2757,19 +2721,16 @@ static int vp3_decode_frame(AVCodecContext *avctx, AVFrame *frame,
#endif
}
} else {
- if (!s->golden_frame.f->data[0]) {
+ if (!s->golden_frame.f) {
av_log(s->avctx, AV_LOG_WARNING,
"vp3: first frame not a keyframe\n");
- s->golden_frame.f->pict_type = AV_PICTURE_TYPE_I;
- if ((ret = ff_thread_get_ext_buffer(avctx, &s->golden_frame,
- AV_GET_BUFFER_FLAG_REF)) < 0)
+ if ((ret = ff_progress_frame_get_buffer(avctx, &s->golden_frame,
+ AV_GET_BUFFER_FLAG_REF)) < 0)
goto error;
- ff_thread_release_ext_buffer(&s->last_frame);
- if ((ret = ff_thread_ref_frame(&s->last_frame,
- &s->golden_frame)) < 0)
- goto error;
- ff_thread_report_progress(&s->last_frame, INT_MAX, 0);
+ s->golden_frame.f->pict_type = AV_PICTURE_TYPE_I;
+ ff_progress_frame_replace(&s->last_frame, &s->golden_frame);
+ ff_progress_frame_report(&s->golden_frame, INT_MAX);
}
}
ff_thread_finish_setup(avctx);
@@ -2847,16 +2808,13 @@ static int vp3_decode_frame(AVCodecContext *avctx, AVFrame *frame,
*got_frame = 1;
- if (!HAVE_THREADS || !(s->avctx->active_thread_type & FF_THREAD_FRAME)) {
- ret = update_frames(avctx);
- if (ret < 0)
- return ret;
- }
+ if (!HAVE_THREADS || !(s->avctx->active_thread_type & FF_THREAD_FRAME))
+ update_frames(avctx);
return buf_size;
error:
- ff_thread_report_progress(&s->current_frame, INT_MAX, 0);
+ ff_progress_frame_report(&s->current_frame, INT_MAX);
if (!HAVE_THREADS || !(s->avctx->active_thread_type & FF_THREAD_FRAME))
av_frame_unref(s->current_frame.f);
@@ -3206,7 +3164,8 @@ const FFCodec ff_theora_decoder = {
.flush = vp3_decode_flush,
UPDATE_THREAD_CONTEXT(vp3_update_thread_context),
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
- FF_CODEC_CAP_EXPORTS_CROPPING | FF_CODEC_CAP_ALLOCATE_PROGRESS,
+ FF_CODEC_CAP_EXPORTS_CROPPING |
+ FF_CODEC_CAP_USES_PROGRESSFRAMES,
};
#endif
@@ -3224,7 +3183,7 @@ const FFCodec ff_vp3_decoder = {
.flush = vp3_decode_flush,
UPDATE_THREAD_CONTEXT(vp3_update_thread_context),
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
- FF_CODEC_CAP_ALLOCATE_PROGRESS,
+ FF_CODEC_CAP_USES_PROGRESSFRAMES,
};
#if CONFIG_VP4_DECODER
@@ -3242,6 +3201,6 @@ const FFCodec ff_vp4_decoder = {
.flush = vp3_decode_flush,
UPDATE_THREAD_CONTEXT(vp3_update_thread_context),
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
- FF_CODEC_CAP_ALLOCATE_PROGRESS,
+ FF_CODEC_CAP_USES_PROGRESSFRAMES,
};
#endif
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 05/27] avcodec/vp9: Switch to ProgressFrames
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (2 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 04/27] avcodec/vp3: " Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 06/27] avcodec/vp9: Fix race when attaching side-data for show-existing frame Andreas Rheinhardt
` (24 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Anton Khirnov, Andreas Rheinhardt
This already fixes a race in the vp9-encparams test. In this test,
side data is added to the current frame after having been decoded
(and therefore after ff_thread_finish_setup() has been called).
Yet the update_thread_context callback called ff_thread_ref_frame()
and therefore av_frame_ref() with this frame as source frame and
the ensuing read was unsynchronised with adding the side data,
i.e. there was a data race.
By switching to the ProgressFrame API the implicit av_frame_ref()
is removed and the race fixed except if this frame is later reused by
a show-existing-frame which uses an explicit av_frame_ref().
The vp9-encparams test does not cover this, so this commit
already fixes all the races in this test.
This decoder kept multiple references to the same ThreadFrames
in the same context and therefore had lots of implicit av_frame_ref()
even when decoding single-threaded. This incurred lots of small
allocations: When decoding an ordinary 10s video in single-threaded
mode the number of allocations reported by Valgrind went down
from 57,814 to 20,908; for 10 threads it went down from 84,223 to
21,901.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/dxva2_vp9.c | 4 +-
libavcodec/vaapi_vp9.c | 2 +-
libavcodec/vp9.c | 137 +++++++++++------------------------
libavcodec/vp9_mc_template.c | 2 +-
libavcodec/vp9block.c | 5 +-
libavcodec/vp9dec.h | 6 +-
libavcodec/vp9lpf.c | 1 +
libavcodec/vp9mvs.c | 4 +-
libavcodec/vp9recon.c | 19 ++---
libavcodec/vp9shared.h | 6 +-
10 files changed, 70 insertions(+), 116 deletions(-)
diff --git a/libavcodec/dxva2_vp9.c b/libavcodec/dxva2_vp9.c
index 1498deb3c8..ca8b3b136d 100644
--- a/libavcodec/dxva2_vp9.c
+++ b/libavcodec/dxva2_vp9.c
@@ -79,7 +79,7 @@ int ff_dxva2_vp9_fill_picture_parameters(const AVCodecContext *avctx, AVDXVACont
pp->Reserved8Bits = 0;
for (i = 0; i < 8; i++) {
- if (h->refs[i].f->buf[0]) {
+ if (h->refs[i].f) {
fill_picture_entry(&pp->ref_frame_map[i], ff_dxva2_get_surface_index(avctx, ctx, h->refs[i].f, 0), 0);
pp->ref_frame_coded_width[i] = h->refs[i].f->width;
pp->ref_frame_coded_height[i] = h->refs[i].f->height;
@@ -89,7 +89,7 @@ int ff_dxva2_vp9_fill_picture_parameters(const AVCodecContext *avctx, AVDXVACont
for (i = 0; i < 3; i++) {
uint8_t refidx = h->h.refidx[i];
- if (h->refs[refidx].f->buf[0])
+ if (h->refs[refidx].f)
fill_picture_entry(&pp->frame_refs[i], ff_dxva2_get_surface_index(avctx, ctx, h->refs[refidx].f, 0), 0);
else
pp->frame_refs[i].bPicEntry = 0xFF;
diff --git a/libavcodec/vaapi_vp9.c b/libavcodec/vaapi_vp9.c
index 9dc7d5e72b..b8e760c807 100644
--- a/libavcodec/vaapi_vp9.c
+++ b/libavcodec/vaapi_vp9.c
@@ -100,7 +100,7 @@ static int vaapi_vp9_start_frame(AVCodecContext *avctx,
}
for (i = 0; i < 8; i++) {
- if (h->refs[i].f->buf[0])
+ if (h->refs[i].f)
pic_param.reference_frames[i] = ff_vaapi_get_surface_id(h->refs[i].f);
else
pic_param.reference_frames[i] = VA_INVALID_ID;
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 6bcda8bfff..bd52478ce7 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -30,9 +30,9 @@
#include "hwaccel_internal.h"
#include "hwconfig.h"
#include "profiles.h"
+#include "progressframe.h"
#include "refstruct.h"
#include "thread.h"
-#include "threadframe.h"
#include "pthread_internal.h"
#include "videodsp.h"
@@ -100,7 +100,7 @@ static void vp9_tile_data_free(VP9TileData *td)
static void vp9_frame_unref(VP9Frame *f)
{
- ff_thread_release_ext_buffer(&f->tf);
+ ff_progress_frame_unref(&f->tf);
ff_refstruct_unref(&f->extradata);
ff_refstruct_unref(&f->hwaccel_picture_private);
f->segmentation_map = NULL;
@@ -111,7 +111,7 @@ static int vp9_frame_alloc(AVCodecContext *avctx, VP9Frame *f)
VP9Context *s = avctx->priv_data;
int ret, sz;
- ret = ff_thread_get_ext_buffer(avctx, &f->tf, AV_GET_BUFFER_FLAG_REF);
+ ret = ff_progress_frame_get_buffer(avctx, &f->tf, AV_GET_BUFFER_FLAG_REF);
if (ret < 0)
return ret;
@@ -147,13 +147,9 @@ fail:
return ret;
}
-static int vp9_frame_ref(VP9Frame *dst, VP9Frame *src)
+static void vp9_frame_ref(VP9Frame *dst, const VP9Frame *src)
{
- int ret;
-
- ret = ff_thread_ref_frame(&dst->tf, &src->tf);
- if (ret < 0)
- return ret;
+ ff_progress_frame_ref(&dst->tf, &src->tf);
dst->extradata = ff_refstruct_ref(src->extradata);
@@ -163,8 +159,13 @@ static int vp9_frame_ref(VP9Frame *dst, VP9Frame *src)
ff_refstruct_replace(&dst->hwaccel_picture_private,
src->hwaccel_picture_private);
+}
- return 0;
+static void vp9_frame_replace(VP9Frame *dst, const VP9Frame *src)
+{
+ vp9_frame_unref(dst);
+ if (src && src->tf.f)
+ vp9_frame_ref(dst, src);
}
static int update_size(AVCodecContext *avctx, int w, int h)
@@ -589,9 +590,9 @@ static int decode_frame_header(AVCodecContext *avctx,
s->s.h.signbias[1] = get_bits1(&s->gb) && !s->s.h.errorres;
s->s.h.refidx[2] = get_bits(&s->gb, 3);
s->s.h.signbias[2] = get_bits1(&s->gb) && !s->s.h.errorres;
- if (!s->s.refs[s->s.h.refidx[0]].f->buf[0] ||
- !s->s.refs[s->s.h.refidx[1]].f->buf[0] ||
- !s->s.refs[s->s.h.refidx[2]].f->buf[0]) {
+ if (!s->s.refs[s->s.h.refidx[0]].f ||
+ !s->s.refs[s->s.h.refidx[1]].f ||
+ !s->s.refs[s->s.h.refidx[2]].f) {
av_log(avctx, AV_LOG_ERROR, "Not all references are available\n");
return AVERROR_INVALIDDATA;
}
@@ -611,7 +612,8 @@ static int decode_frame_header(AVCodecContext *avctx,
// Note that in this code, "CUR_FRAME" is actually before we
// have formally allocated a frame, and thus actually represents
// the _last_ frame
- s->s.h.use_last_frame_mvs &= s->s.frames[CUR_FRAME].tf.f->width == w &&
+ s->s.h.use_last_frame_mvs &= s->s.frames[CUR_FRAME].tf.f &&
+ s->s.frames[CUR_FRAME].tf.f->width == w &&
s->s.frames[CUR_FRAME].tf.f->height == h;
if (get_bits1(&s->gb)) // display size
skip_bits(&s->gb, 32);
@@ -1240,16 +1242,12 @@ static av_cold int vp9_decode_free(AVCodecContext *avctx)
VP9Context *s = avctx->priv_data;
int i;
- for (i = 0; i < 3; i++) {
+ for (int i = 0; i < 3; i++)
vp9_frame_unref(&s->s.frames[i]);
- av_frame_free(&s->s.frames[i].tf.f);
- }
ff_refstruct_pool_uninit(&s->frame_extradata_pool);
for (i = 0; i < 8; i++) {
- ff_thread_release_ext_buffer(&s->s.refs[i]);
- av_frame_free(&s->s.refs[i].f);
- ff_thread_release_ext_buffer(&s->next_refs[i]);
- av_frame_free(&s->next_refs[i].f);
+ ff_progress_frame_unref(&s->s.refs[i]);
+ ff_progress_frame_unref(&s->next_refs[i]);
}
free_buffers(s);
@@ -1384,7 +1382,7 @@ static int decode_tiles(AVCodecContext *avctx,
// FIXME maybe we can make this more finegrained by running the
// loopfilter per-block instead of after each sbrow
// In fact that would also make intra pred left preparation easier?
- ff_thread_report_progress(&s->s.frames[CUR_FRAME].tf, row >> 3, 0);
+ ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, row >> 3);
}
}
return 0;
@@ -1561,12 +1559,13 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
int ret, i, j, ref;
int retain_segmap_ref = s->s.frames[REF_FRAME_SEGMAP].segmentation_map &&
(!s->s.h.segmentation.enabled || !s->s.h.segmentation.update_map);
+ const VP9Frame *src;
AVFrame *f;
if ((ret = decode_frame_header(avctx, data, size, &ref)) < 0) {
return ret;
} else if (ret == 0) {
- if (!s->s.refs[ref].f->buf[0]) {
+ if (!s->s.refs[ref].f) {
av_log(avctx, AV_LOG_ERROR, "Requested reference %d not available\n", ref);
return AVERROR_INVALIDDATA;
}
@@ -1574,33 +1573,19 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
return ret;
frame->pts = pkt->pts;
frame->pkt_dts = pkt->dts;
- for (i = 0; i < 8; i++) {
- if (s->next_refs[i].f->buf[0])
- ff_thread_release_ext_buffer(&s->next_refs[i]);
- if (s->s.refs[i].f->buf[0] &&
- (ret = ff_thread_ref_frame(&s->next_refs[i], &s->s.refs[i])) < 0)
- return ret;
- }
+ for (int i = 0; i < 8; i++)
+ ff_progress_frame_replace(&s->next_refs[i], &s->s.refs[i]);
*got_frame = 1;
return pkt->size;
}
data += ret;
size -= ret;
- if (!retain_segmap_ref || s->s.h.keyframe || s->s.h.intraonly) {
- if (s->s.frames[REF_FRAME_SEGMAP].tf.f->buf[0])
- vp9_frame_unref(&s->s.frames[REF_FRAME_SEGMAP]);
- if (!s->s.h.keyframe && !s->s.h.intraonly && !s->s.h.errorres && s->s.frames[CUR_FRAME].tf.f->buf[0] &&
- (ret = vp9_frame_ref(&s->s.frames[REF_FRAME_SEGMAP], &s->s.frames[CUR_FRAME])) < 0)
- return ret;
- }
- if (s->s.frames[REF_FRAME_MVPAIR].tf.f->buf[0])
- vp9_frame_unref(&s->s.frames[REF_FRAME_MVPAIR]);
- if (!s->s.h.intraonly && !s->s.h.keyframe && !s->s.h.errorres && s->s.frames[CUR_FRAME].tf.f->buf[0] &&
- (ret = vp9_frame_ref(&s->s.frames[REF_FRAME_MVPAIR], &s->s.frames[CUR_FRAME])) < 0)
- return ret;
- if (s->s.frames[CUR_FRAME].tf.f->buf[0])
- vp9_frame_unref(&s->s.frames[CUR_FRAME]);
+ src = !s->s.h.keyframe && !s->s.h.intraonly && !s->s.h.errorres ? &s->s.frames[CUR_FRAME] : NULL;
+ if (!retain_segmap_ref || s->s.h.keyframe || s->s.h.intraonly)
+ vp9_frame_replace(&s->s.frames[REF_FRAME_SEGMAP], src);
+ vp9_frame_replace(&s->s.frames[REF_FRAME_MVPAIR], src);
+ vp9_frame_unref(&s->s.frames[CUR_FRAME]);
if ((ret = vp9_frame_alloc(avctx, &s->s.frames[CUR_FRAME])) < 0)
return ret;
f = s->s.frames[CUR_FRAME].tf.f;
@@ -1610,7 +1595,8 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
f->flags &= ~AV_FRAME_FLAG_KEY;
f->pict_type = (s->s.h.keyframe || s->s.h.intraonly) ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
- if (s->s.frames[REF_FRAME_SEGMAP].tf.f->buf[0] &&
+ // Non-existent frames have the implicit dimension 0x0 != CUR_FRAME
+ if (!s->s.frames[REF_FRAME_MVPAIR].tf.f ||
(s->s.frames[REF_FRAME_MVPAIR].tf.f->width != s->s.frames[CUR_FRAME].tf.f->width ||
s->s.frames[REF_FRAME_MVPAIR].tf.f->height != s->s.frames[CUR_FRAME].tf.f->height)) {
vp9_frame_unref(&s->s.frames[REF_FRAME_SEGMAP]);
@@ -1618,15 +1604,9 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
// ref frame setup
for (i = 0; i < 8; i++) {
- if (s->next_refs[i].f->buf[0])
- ff_thread_release_ext_buffer(&s->next_refs[i]);
- if (s->s.h.refreshrefmask & (1 << i)) {
- ret = ff_thread_ref_frame(&s->next_refs[i], &s->s.frames[CUR_FRAME].tf);
- } else if (s->s.refs[i].f->buf[0]) {
- ret = ff_thread_ref_frame(&s->next_refs[i], &s->s.refs[i]);
- }
- if (ret < 0)
- return ret;
+ ff_progress_frame_replace(&s->next_refs[i],
+ s->s.h.refreshrefmask & (1 << i) ?
+ &s->s.frames[CUR_FRAME].tf : &s->s.refs[i]);
}
if (avctx->hwaccel) {
@@ -1736,7 +1716,7 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
{
ret = decode_tiles(avctx, data, size);
if (ret < 0) {
- ff_thread_report_progress(&s->s.frames[CUR_FRAME].tf, INT_MAX, 0);
+ ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
return ret;
}
}
@@ -1752,7 +1732,7 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
ff_thread_finish_setup(avctx);
}
} while (s->pass++ == 1);
- ff_thread_report_progress(&s->s.frames[CUR_FRAME].tf, INT_MAX, 0);
+ ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
if (s->td->error_info < 0) {
av_log(avctx, AV_LOG_ERROR, "Failed to decode tile data\n");
@@ -1767,13 +1747,8 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
finish:
// ref frame setup
- for (i = 0; i < 8; i++) {
- if (s->s.refs[i].f->buf[0])
- ff_thread_release_ext_buffer(&s->s.refs[i]);
- if (s->next_refs[i].f->buf[0] &&
- (ret = ff_thread_ref_frame(&s->s.refs[i], &s->next_refs[i])) < 0)
- return ret;
- }
+ for (int i = 0; i < 8; i++)
+ ff_progress_frame_replace(&s->s.refs[i], &s->next_refs[i]);
if (!s->s.h.invisible) {
if ((ret = av_frame_ref(frame, s->s.frames[CUR_FRAME].tf.f)) < 0)
@@ -1792,7 +1767,7 @@ static void vp9_decode_flush(AVCodecContext *avctx)
for (i = 0; i < 3; i++)
vp9_frame_unref(&s->s.frames[i]);
for (i = 0; i < 8; i++)
- ff_thread_release_ext_buffer(&s->s.refs[i]);
+ ff_progress_frame_unref(&s->s.refs[i]);
if (FF_HW_HAS_CB(avctx, flush))
FF_HW_SIMPLE_CALL(avctx, flush);
@@ -1814,42 +1789,18 @@ static av_cold int vp9_decode_init(AVCodecContext *avctx)
}
#endif
- for (int i = 0; i < 3; i++) {
- s->s.frames[i].tf.f = av_frame_alloc();
- if (!s->s.frames[i].tf.f)
- return AVERROR(ENOMEM);
- }
- for (int i = 0; i < 8; i++) {
- s->s.refs[i].f = av_frame_alloc();
- s->next_refs[i].f = av_frame_alloc();
- if (!s->s.refs[i].f || !s->next_refs[i].f)
- return AVERROR(ENOMEM);
- }
return 0;
}
#if HAVE_THREADS
static int vp9_decode_update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
{
- int i, ret;
VP9Context *s = dst->priv_data, *ssrc = src->priv_data;
- for (i = 0; i < 3; i++) {
- if (s->s.frames[i].tf.f->buf[0])
- vp9_frame_unref(&s->s.frames[i]);
- if (ssrc->s.frames[i].tf.f->buf[0]) {
- if ((ret = vp9_frame_ref(&s->s.frames[i], &ssrc->s.frames[i])) < 0)
- return ret;
- }
- }
- for (i = 0; i < 8; i++) {
- if (s->s.refs[i].f->buf[0])
- ff_thread_release_ext_buffer(&s->s.refs[i]);
- if (ssrc->next_refs[i].f->buf[0]) {
- if ((ret = ff_thread_ref_frame(&s->s.refs[i], &ssrc->next_refs[i])) < 0)
- return ret;
- }
- }
+ for (int i = 0; i < 3; i++)
+ vp9_frame_replace(&s->s.frames[i], &ssrc->s.frames[i]);
+ for (int i = 0; i < 8; i++)
+ ff_progress_frame_replace(&s->s.refs[i], &ssrc->next_refs[i]);
ff_refstruct_replace(&s->frame_extradata_pool, ssrc->frame_extradata_pool);
s->frame_extradata_pool_size = ssrc->frame_extradata_pool_size;
@@ -1889,7 +1840,7 @@ const FFCodec ff_vp9_decoder = {
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_SLICE_THREADS,
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
FF_CODEC_CAP_SLICE_THREAD_HAS_MF |
- FF_CODEC_CAP_ALLOCATE_PROGRESS,
+ FF_CODEC_CAP_USES_PROGRESSFRAMES,
.flush = vp9_decode_flush,
UPDATE_THREAD_CONTEXT(vp9_decode_update_thread_context),
.p.profiles = NULL_IF_CONFIG_SMALL(ff_vp9_profiles),
diff --git a/libavcodec/vp9_mc_template.c b/libavcodec/vp9_mc_template.c
index e654c0e5ed..81e4ed59c7 100644
--- a/libavcodec/vp9_mc_template.c
+++ b/libavcodec/vp9_mc_template.c
@@ -36,7 +36,7 @@ static void FN(inter_pred)(VP9TileData *td)
const VP9Context *s = td->s;
VP9Block *b = td->b;
int row = td->row, col = td->col;
- const ThreadFrame *tref1 = &s->s.refs[s->s.h.refidx[b->ref[0]]], *tref2;
+ const ProgressFrame *tref1 = &s->s.refs[s->s.h.refidx[b->ref[0]]], *tref2;
const AVFrame *ref1 = tref1->f, *ref2;
int w1 = ref1->width, h1 = ref1->height, w2, h2;
ptrdiff_t ls_y = td->y_stride, ls_uv = td->uv_stride;
diff --git a/libavcodec/vp9block.c b/libavcodec/vp9block.c
index 5743f048cc..3a694763ce 100644
--- a/libavcodec/vp9block.c
+++ b/libavcodec/vp9block.c
@@ -22,8 +22,9 @@
*/
#include "libavutil/avassert.h"
+#include "libavutil/frame.h"
-#include "threadframe.h"
+#include "progressframe.h"
#include "vp89_rac.h"
#include "vp9.h"
#include "vp9data.h"
@@ -113,7 +114,7 @@ static void decode_mode(VP9TileData *td)
uint8_t *refsegmap = s->s.frames[REF_FRAME_SEGMAP].segmentation_map;
if (!s->s.frames[REF_FRAME_SEGMAP].uses_2pass)
- ff_thread_await_progress(&s->s.frames[REF_FRAME_SEGMAP].tf, row >> 3, 0);
+ ff_progress_frame_await(&s->s.frames[REF_FRAME_SEGMAP].tf, row >> 3);
for (y = 0; y < h4; y++) {
int idx_base = (y + row) * 8 * s->sb_cols + col;
for (x = 0; x < w4; x++)
diff --git a/libavcodec/vp9dec.h b/libavcodec/vp9dec.h
index 013aac49eb..81dc801052 100644
--- a/libavcodec/vp9dec.h
+++ b/libavcodec/vp9dec.h
@@ -29,8 +29,8 @@
#include <stdatomic.h>
#include "libavutil/mem_internal.h"
+#include "libavutil/pixfmt.h"
#include "libavutil/thread.h"
-#include "libavutil/internal.h"
#include "get_bits.h"
#include "videodsp.h"
@@ -120,7 +120,7 @@ typedef struct VP9Context {
int w, h;
enum AVPixelFormat pix_fmt, last_fmt, gf_fmt;
unsigned sb_cols, sb_rows, rows, cols;
- ThreadFrame next_refs[8];
+ ProgressFrame next_refs[8];
struct {
uint8_t lim_lut[64];
@@ -245,7 +245,7 @@ void ff_vp9_decode_block(VP9TileData *td, int row, int col,
VP9Filter *lflvl, ptrdiff_t yoff, ptrdiff_t uvoff,
enum BlockLevel bl, enum BlockPartition bp);
-void ff_vp9_loopfilter_sb(AVCodecContext *avctx, VP9Filter *lflvl,
+void ff_vp9_loopfilter_sb(struct AVCodecContext *avctx, VP9Filter *lflvl,
int row, int col, ptrdiff_t yoff, ptrdiff_t uvoff);
void ff_vp9_intra_recon_8bpp(VP9TileData *td,
diff --git a/libavcodec/vp9lpf.c b/libavcodec/vp9lpf.c
index 414cede852..afeebebf59 100644
--- a/libavcodec/vp9lpf.c
+++ b/libavcodec/vp9lpf.c
@@ -21,6 +21,7 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include "avcodec.h"
#include "vp9dec.h"
static av_always_inline void filter_plane_cols(VP9Context *s, int col, int ss_h, int ss_v,
diff --git a/libavcodec/vp9mvs.c b/libavcodec/vp9mvs.c
index b93d878d6f..b706d1660f 100644
--- a/libavcodec/vp9mvs.c
+++ b/libavcodec/vp9mvs.c
@@ -21,7 +21,7 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
-#include "threadframe.h"
+#include "progressframe.h"
#include "vp89_rac.h"
#include "vp9data.h"
#include "vp9dec.h"
@@ -175,7 +175,7 @@ static void find_ref_mvs(VP9TileData *td,
VP9mvrefPair *mv = &s->s.frames[REF_FRAME_MVPAIR].mv[row * s->sb_cols * 8 + col];
if (!s->s.frames[REF_FRAME_MVPAIR].uses_2pass)
- ff_thread_await_progress(&s->s.frames[REF_FRAME_MVPAIR].tf, row >> 3, 0);
+ ff_progress_frame_await(&s->s.frames[REF_FRAME_MVPAIR].tf, row >> 3);
if (mv->ref[0] == ref)
RETURN_MV(mv->mv[0]);
else if (mv->ref[1] == ref)
diff --git a/libavcodec/vp9recon.c b/libavcodec/vp9recon.c
index 073c04b47d..ef08ed17c8 100644
--- a/libavcodec/vp9recon.c
+++ b/libavcodec/vp9recon.c
@@ -22,9 +22,10 @@
*/
#include "libavutil/avassert.h"
+#include "libavutil/frame.h"
#include "libavutil/mem_internal.h"
-#include "threadframe.h"
+#include "progressframe.h"
#include "videodsp.h"
#include "vp9data.h"
#include "vp9dec.h"
@@ -298,7 +299,7 @@ void ff_vp9_intra_recon_16bpp(VP9TileData *td, ptrdiff_t y_off, ptrdiff_t uv_off
static av_always_inline void mc_luma_unscaled(VP9TileData *td, const vp9_mc_func (*mc)[2],
uint8_t *dst, ptrdiff_t dst_stride,
const uint8_t *ref, ptrdiff_t ref_stride,
- const ThreadFrame *ref_frame,
+ const ProgressFrame *ref_frame,
ptrdiff_t y, ptrdiff_t x, const VP9mv *mv,
int bw, int bh, int w, int h, int bytesperpixel)
{
@@ -314,7 +315,7 @@ static av_always_inline void mc_luma_unscaled(VP9TileData *td, const vp9_mc_func
// we use +7 because the last 7 pixels of each sbrow can be changed in
// the longest loopfilter of the next sbrow
th = (y + bh + 4 * !!my + 7) >> 6;
- ff_thread_await_progress(ref_frame, FFMAX(th, 0), 0);
+ ff_progress_frame_await(ref_frame, FFMAX(th, 0));
// The arm/aarch64 _hv filters read one more row than what actually is
// needed, so switch to emulated edge one pixel sooner vertically
// (!!my * 5) than horizontally (!!mx * 4).
@@ -336,7 +337,7 @@ static av_always_inline void mc_chroma_unscaled(VP9TileData *td, const vp9_mc_fu
ptrdiff_t dst_stride,
const uint8_t *ref_u, ptrdiff_t src_stride_u,
const uint8_t *ref_v, ptrdiff_t src_stride_v,
- const ThreadFrame *ref_frame,
+ const ProgressFrame *ref_frame,
ptrdiff_t y, ptrdiff_t x, const VP9mv *mv,
int bw, int bh, int w, int h, int bytesperpixel)
{
@@ -353,7 +354,7 @@ static av_always_inline void mc_chroma_unscaled(VP9TileData *td, const vp9_mc_fu
// we use +7 because the last 7 pixels of each sbrow can be changed in
// the longest loopfilter of the next sbrow
th = (y + bh + 4 * !!my + 7) >> (6 - s->ss_v);
- ff_thread_await_progress(ref_frame, FFMAX(th, 0), 0);
+ ff_progress_frame_await(ref_frame, FFMAX(th, 0));
// The arm/aarch64 _hv filters read one more row than what actually is
// needed, so switch to emulated edge one pixel sooner vertically
// (!!my * 5) than horizontally (!!mx * 4).
@@ -407,7 +408,7 @@ static av_always_inline void mc_luma_scaled(VP9TileData *td, vp9_scaled_mc_func
const vp9_mc_func (*mc)[2],
uint8_t *dst, ptrdiff_t dst_stride,
const uint8_t *ref, ptrdiff_t ref_stride,
- const ThreadFrame *ref_frame,
+ const ProgressFrame *ref_frame,
ptrdiff_t y, ptrdiff_t x, const VP9mv *in_mv,
int px, int py, int pw, int ph,
int bw, int bh, int w, int h, int bytesperpixel,
@@ -444,7 +445,7 @@ static av_always_inline void mc_luma_scaled(VP9TileData *td, vp9_scaled_mc_func
// we use +7 because the last 7 pixels of each sbrow can be changed in
// the longest loopfilter of the next sbrow
th = (y + refbh_m1 + 4 + 7) >> 6;
- ff_thread_await_progress(ref_frame, FFMAX(th, 0), 0);
+ ff_progress_frame_await(ref_frame, FFMAX(th, 0));
// The arm/aarch64 _hv filters read one more row than what actually is
// needed, so switch to emulated edge one pixel sooner vertically
// (y + 5 >= h - refbh_m1) than horizontally (x + 4 >= w - refbw_m1).
@@ -467,7 +468,7 @@ static av_always_inline void mc_chroma_scaled(VP9TileData *td, vp9_scaled_mc_fun
ptrdiff_t dst_stride,
const uint8_t *ref_u, ptrdiff_t src_stride_u,
const uint8_t *ref_v, ptrdiff_t src_stride_v,
- const ThreadFrame *ref_frame,
+ const ProgressFrame *ref_frame,
ptrdiff_t y, ptrdiff_t x, const VP9mv *in_mv,
int px, int py, int pw, int ph,
int bw, int bh, int w, int h, int bytesperpixel,
@@ -514,7 +515,7 @@ static av_always_inline void mc_chroma_scaled(VP9TileData *td, vp9_scaled_mc_fun
// we use +7 because the last 7 pixels of each sbrow can be changed in
// the longest loopfilter of the next sbrow
th = (y + refbh_m1 + 4 + 7) >> (6 - s->ss_v);
- ff_thread_await_progress(ref_frame, FFMAX(th, 0), 0);
+ ff_progress_frame_await(ref_frame, FFMAX(th, 0));
// The arm/aarch64 _hv filters read one more row than what actually is
// needed, so switch to emulated edge one pixel sooner vertically
// (y + 5 >= h - refbh_m1) than horizontally (x + 4 >= w - refbw_m1).
diff --git a/libavcodec/vp9shared.h b/libavcodec/vp9shared.h
index b445a2a746..805668416f 100644
--- a/libavcodec/vp9shared.h
+++ b/libavcodec/vp9shared.h
@@ -29,8 +29,8 @@
#include "libavutil/mem_internal.h"
+#include "progressframe.h"
#include "vp9.h"
-#include "threadframe.h"
enum BlockPartition {
PARTITION_NONE, // [ ] <-.
@@ -63,7 +63,7 @@ typedef struct VP9mvrefPair {
} VP9mvrefPair;
typedef struct VP9Frame {
- ThreadFrame tf;
+ ProgressFrame tf;
void *extradata; ///< RefStruct reference
uint8_t *segmentation_map;
VP9mvrefPair *mv;
@@ -164,7 +164,7 @@ typedef struct VP9BitstreamHeader {
typedef struct VP9SharedContext {
VP9BitstreamHeader h;
- ThreadFrame refs[8];
+ ProgressFrame refs[8];
#define CUR_FRAME 0
#define REF_FRAME_MVPAIR 1
#define REF_FRAME_SEGMAP 2
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 06/27] avcodec/vp9: Fix race when attaching side-data for show-existing frame
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (3 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 05/27] avcodec/vp9: " Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-10 7:06 ` Anton Khirnov
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 07/27] avcodec/vp9: Reduce wait times Andreas Rheinhardt
` (23 subsequent siblings)
28 siblings, 1 reply; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
When outputting a show-existing frame, the VP9 decoder simply
created a reference to said frame and returned it immediately to
the caller, without waiting for it to have finished decoding.
In case of frame-threading it is possible for the frame to
only be decoded while it was waiting to be output.
This is normally benign.
But there is one case where it is not: If the user wants
video encoding parameters to be exported, said side data
will only be attached to the src AVFrame at the end of
decoding the frame that is actually being shown. Without
synchronisation adding said side data in the decoder thread
and the reads in av_frame_ref() in the output thread
constitute a data race. This happens e.g. when using the
venc_data_dump tool with vp90-2-10-show-existing-frame.webm
from the FATE-suite.
Fix this by actually waiting for the frame to be output.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/vp9.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index bd52478ce7..e0bc313301 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -1569,6 +1569,8 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
av_log(avctx, AV_LOG_ERROR, "Requested reference %d not available\n", ref);
return AVERROR_INVALIDDATA;
}
+ ff_progress_frame_await(&s->s.refs[ref], INT_MAX);
+
if ((ret = av_frame_ref(frame, s->s.refs[ref].f)) < 0)
return ret;
frame->pts = pkt->pts;
@@ -1715,10 +1717,8 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
#endif
{
ret = decode_tiles(avctx, data, size);
- if (ret < 0) {
- ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
- return ret;
- }
+ if (ret < 0)
+ goto fail;
}
// Sum all counts fields into td[0].counts for tile threading
@@ -1732,18 +1732,19 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
ff_thread_finish_setup(avctx);
}
} while (s->pass++ == 1);
- ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
if (s->td->error_info < 0) {
av_log(avctx, AV_LOG_ERROR, "Failed to decode tile data\n");
s->td->error_info = 0;
- return AVERROR_INVALIDDATA;
+ ret = AVERROR_INVALIDDATA;
+ goto fail;
}
if (avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) {
ret = vp9_export_enc_params(s, &s->s.frames[CUR_FRAME]);
if (ret < 0)
- return ret;
+ goto fail;
}
+ ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
finish:
// ref frame setup
@@ -1757,6 +1758,9 @@ finish:
}
return pkt->size;
+fail:
+ ff_progress_frame_report(&s->s.frames[CUR_FRAME].tf, INT_MAX);
+ return ret;
}
static void vp9_decode_flush(AVCodecContext *avctx)
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 07/27] avcodec/vp9: Reduce wait times
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (4 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 06/27] avcodec/vp9: Fix race when attaching side-data for show-existing frame Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 08/27] avcodec/vp9: Simplify replacing VP9Frame Andreas Rheinhardt
` (22 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/vp9.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index e0bc313301..bdfa543188 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -1569,14 +1569,15 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
av_log(avctx, AV_LOG_ERROR, "Requested reference %d not available\n", ref);
return AVERROR_INVALIDDATA;
}
+ for (int i = 0; i < 8; i++)
+ ff_progress_frame_replace(&s->next_refs[i], &s->s.refs[i]);
+ ff_thread_finish_setup(avctx);
ff_progress_frame_await(&s->s.refs[ref], INT_MAX);
if ((ret = av_frame_ref(frame, s->s.refs[ref].f)) < 0)
return ret;
frame->pts = pkt->pts;
frame->pkt_dts = pkt->dts;
- for (int i = 0; i < 8; i++)
- ff_progress_frame_replace(&s->next_refs[i], &s->s.refs[i]);
*got_frame = 1;
return pkt->size;
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 08/27] avcodec/vp9: Simplify replacing VP9Frame
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (5 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 07/27] avcodec/vp9: Reduce wait times Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 09/27] avcodec/vp9: Replace atomic_store() by atomic_init() Andreas Rheinhardt
` (21 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
ff_thread_progress_replace() can handle a blank ProgressFrame
as src (in which case it simply unreferences dst), but not
a NULL one. So add a blank frame to be used as source for
this case, so that we can use the replace functions
to simplify vp9_frame_replace().
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/vp9.c | 16 +++++-----------
libavcodec/vp9shared.h | 3 ++-
2 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index bdfa543188..443eb74c3c 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -147,11 +147,11 @@ fail:
return ret;
}
-static void vp9_frame_ref(VP9Frame *dst, const VP9Frame *src)
+static void vp9_frame_replace(VP9Frame *dst, const VP9Frame *src)
{
- ff_progress_frame_ref(&dst->tf, &src->tf);
+ ff_progress_frame_replace(&dst->tf, &src->tf);
- dst->extradata = ff_refstruct_ref(src->extradata);
+ ff_refstruct_replace(&dst->extradata, src->extradata);
dst->segmentation_map = src->segmentation_map;
dst->mv = src->mv;
@@ -161,13 +161,6 @@ static void vp9_frame_ref(VP9Frame *dst, const VP9Frame *src)
src->hwaccel_picture_private);
}
-static void vp9_frame_replace(VP9Frame *dst, const VP9Frame *src)
-{
- vp9_frame_unref(dst);
- if (src && src->tf.f)
- vp9_frame_ref(dst, src);
-}
-
static int update_size(AVCodecContext *avctx, int w, int h)
{
#define HWACCEL_MAX (CONFIG_VP9_DXVA2_HWACCEL + \
@@ -1584,7 +1577,8 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
data += ret;
size -= ret;
- src = !s->s.h.keyframe && !s->s.h.intraonly && !s->s.h.errorres ? &s->s.frames[CUR_FRAME] : NULL;
+ src = !s->s.h.keyframe && !s->s.h.intraonly && !s->s.h.errorres ?
+ &s->s.frames[CUR_FRAME] : &s->s.frames[BLANK_FRAME];
if (!retain_segmap_ref || s->s.h.keyframe || s->s.h.intraonly)
vp9_frame_replace(&s->s.frames[REF_FRAME_SEGMAP], src);
vp9_frame_replace(&s->s.frames[REF_FRAME_MVPAIR], src);
diff --git a/libavcodec/vp9shared.h b/libavcodec/vp9shared.h
index 805668416f..8a450c26a6 100644
--- a/libavcodec/vp9shared.h
+++ b/libavcodec/vp9shared.h
@@ -168,7 +168,8 @@ typedef struct VP9SharedContext {
#define CUR_FRAME 0
#define REF_FRAME_MVPAIR 1
#define REF_FRAME_SEGMAP 2
- VP9Frame frames[3];
+#define BLANK_FRAME 3
+ VP9Frame frames[4];
} VP9SharedContext;
#endif /* AVCODEC_VP9SHARED_H */
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 09/27] avcodec/vp9: Replace atomic_store() by atomic_init()
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (6 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 08/27] avcodec/vp9: Simplify replacing VP9Frame Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 10/27] avcodec/pthread_frame: Add API to share RefStruct refs just once Andreas Rheinhardt
` (20 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
This part of the code is not slice-threaded and they are
semantically an initialization, so use atomic_init()
instead of the potentially expensive atomic_store()
(which uses sequentially consistent memory ordering).
Also remove the initial initialization directly after
allocating this array.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/vp9.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/libavcodec/vp9.c b/libavcodec/vp9.c
index 443eb74c3c..3adfb98f2d 100644
--- a/libavcodec/vp9.c
+++ b/libavcodec/vp9.c
@@ -55,7 +55,6 @@ DEFINE_OFFSET_ARRAY(VP9Context, vp9_context, pthread_init_cnt,
static int vp9_alloc_entries(AVCodecContext *avctx, int n) {
VP9Context *s = avctx->priv_data;
- int i;
if (avctx->active_thread_type & FF_THREAD_SLICE) {
if (s->entries)
@@ -64,9 +63,6 @@ static int vp9_alloc_entries(AVCodecContext *avctx, int n) {
s->entries = av_malloc_array(n, sizeof(atomic_int));
if (!s->entries)
return AVERROR(ENOMEM);
-
- for (i = 0; i < n; i++)
- atomic_init(&s->entries[i], 0);
}
return 0;
}
@@ -1661,7 +1657,7 @@ static int vp9_decode_frame(AVCodecContext *avctx, AVFrame *frame,
#if HAVE_THREADS
if (avctx->active_thread_type & FF_THREAD_SLICE) {
for (i = 0; i < s->sb_rows; i++)
- atomic_store(&s->entries[i], 0);
+ atomic_init(&s->entries[i], 0);
}
#endif
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 10/27] avcodec/pthread_frame: Add API to share RefStruct refs just once
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (7 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 09/27] avcodec/vp9: Replace atomic_store() by atomic_init() Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 11/27] avcodec/wavpack: Use ThreadProgress API Andreas Rheinhardt
` (19 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
This is useful when the lifetime of the object to be shared
is the whole decoding process as it allows to avoid having
to sync them every time in update_thread_context.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/decode.c | 7 +++++++
libavcodec/pthread_frame.c | 20 ++++++++++++++++++++
libavcodec/thread.h | 30 ++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+)
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index f18f85c33c..e196c05f11 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1747,6 +1747,13 @@ void ff_progress_frame_await(const ProgressFrame *f, int n)
ff_thread_progress_await(&f->progress->progress, n);
}
+#if !HAVE_THREADS
+enum ThreadingStatus ff_thread_sync_ref(AVCodecContext *avctx, size_t offset)
+{
+ return FF_THREAD_NO_FRAME_THREADING;
+}
+#endif /* !HAVE_THREADS */
+
static av_cold int progress_frame_pool_init_cb(FFRefStructOpaque opaque, void *obj)
{
const AVCodecContext *avctx = opaque.nc;
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 6b2c4312e0..ee571be610 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -1001,3 +1001,23 @@ void ff_thread_release_ext_buffer(ThreadFrame *f)
if (f->f)
av_frame_unref(f->f);
}
+
+enum ThreadingStatus ff_thread_sync_ref(AVCodecContext *avctx, size_t offset)
+{
+ PerThreadContext *p;
+ const void *ref;
+
+ if (!avctx->internal->is_copy)
+ return avctx->active_thread_type & FF_THREAD_FRAME ?
+ FF_THREAD_IS_FIRST_THREAD : FF_THREAD_NO_FRAME_THREADING;
+
+ p = avctx->internal->thread_ctx;
+
+ av_assert1(memcpy(&ref, (char*)avctx->priv_data + offset, sizeof(ref)) && ref == NULL);
+
+ memcpy(&ref, (const char*)p->parent->threads[0].avctx->priv_data + offset, sizeof(ref));
+ av_assert1(ref);
+ ff_refstruct_replace((char*)avctx->priv_data + offset, ref);
+
+ return FF_THREAD_IS_COPY;
+}
diff --git a/libavcodec/thread.h b/libavcodec/thread.h
index f772d7ff18..5ab12848b4 100644
--- a/libavcodec/thread.h
+++ b/libavcodec/thread.h
@@ -84,4 +84,34 @@ int ff_slice_thread_init_progress(AVCodecContext *avctx);
void ff_thread_report_progress2(AVCodecContext *avctx, int field, int thread, int n);
void ff_thread_await_progress2(AVCodecContext *avctx, int field, int thread, int shift);
+enum ThreadingStatus {
+ FF_THREAD_IS_COPY,
+ FF_THREAD_IS_FIRST_THREAD,
+ FF_THREAD_NO_FRAME_THREADING,
+};
+
+/**
+ * Allows to synchronize objects whose lifetime is the whole decoding
+ * process among all frame threads.
+ *
+ * When called from a non-copy thread, do nothing.
+ * When called from another thread, place a new RefStruct reference
+ * at the given offset in the calling thread's private data from
+ * the RefStruct reference in the private data of the first decoding thread.
+ * The first thread must have a valid RefStruct reference at the given
+ * offset in its private data; the calling thread must not have
+ * a reference at this offset in its private data (must be NULL).
+ *
+ * @param avctx an AVCodecContext
+ * @param offset offset of the RefStruct reference in avctx's private data
+ *
+ * @retval FF_THREAD_IS_COPY if frame-threading is in use and the
+ * calling thread is a copy; in this case, the RefStruct reference
+ * will be set.
+ * @retval FF_THREAD_IS_MAIN_THREAD if frame-threading is in use
+ * and the calling thread is the main thread.
+ * @retval FF_THREAD_NO_FRAME_THREADING if frame-threading is not in use.
+ */
+enum ThreadingStatus ff_thread_sync_ref(AVCodecContext *avctx, size_t offset);
+
#endif /* AVCODEC_THREAD_H */
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 11/27] avcodec/wavpack: Use ThreadProgress API
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (8 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 10/27] avcodec/pthread_frame: Add API to share RefStruct refs just once Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 12/27] avcodec/wavpack: Move initializing DSD data to a better place Andreas Rheinhardt
` (18 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
It is more natural given that WavPack doesn't need the data of
the previous frame at all; it just needs the DSD context.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/wavpack.c | 133 ++++++++++++++++++++++++-------------------
1 file changed, 75 insertions(+), 58 deletions(-)
diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 73d69d66ff..83f42f392d 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -30,7 +30,7 @@
#include "get_bits.h"
#include "refstruct.h"
#include "thread.h"
-#include "threadframe.h"
+#include "threadprogress.h"
#include "unary.h"
#include "wavpack.h"
#include "dsd.h"
@@ -107,11 +107,11 @@ typedef struct WavpackContext {
int samples;
int ch_offset;
- AVFrame *frame;
- ThreadFrame curr_frame, prev_frame;
Modulation modulation;
DSDContext *dsdctx; ///< RefStruct reference
+ ThreadProgress *curr_progress, *prev_progress; ///< RefStruct references
+ FFRefStructPool *progress_pool; ///< RefStruct reference
int dsd_channels;
} WavpackContext;
@@ -994,6 +994,8 @@ static int wv_dsd_reset(WavpackContext *s, int channels)
s->dsd_channels = 0;
ff_refstruct_unref(&s->dsdctx);
+ ff_refstruct_unref(&s->curr_progress);
+ ff_refstruct_unref(&s->prev_progress);
if (!channels)
return 0;
@@ -1017,22 +1019,31 @@ static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
{
WavpackContext *fsrc = src->priv_data;
WavpackContext *fdst = dst->priv_data;
- int ret;
-
- if (dst == src)
- return 0;
-
- ff_thread_release_ext_buffer(&fdst->curr_frame);
- if (fsrc->curr_frame.f->data[0]) {
- if ((ret = ff_thread_ref_frame(&fdst->curr_frame, &fsrc->curr_frame)) < 0)
- return ret;
- }
+ ff_refstruct_replace(&fdst->curr_progress, fsrc->curr_progress);
ff_refstruct_replace(&fdst->dsdctx, fsrc->dsdctx);
fdst->dsd_channels = fsrc->dsd_channels;
return 0;
}
+
+static av_cold int progress_pool_init_cb(FFRefStructOpaque opaque, void *obj)
+{
+ ThreadProgress *progress = obj;
+ return ff_thread_progress_init(progress, 1);
+}
+
+static void progress_pool_reset_cb(FFRefStructOpaque opaque, void *obj)
+{
+ ThreadProgress *progress = obj;
+ ff_thread_progress_reset(progress);
+}
+
+static av_cold void progress_pool_free_entry_cb(FFRefStructOpaque opaque, void *obj)
+{
+ ThreadProgress *progress = obj;
+ ff_thread_progress_destroy(progress);
+}
#endif
static av_cold int wavpack_decode_init(AVCodecContext *avctx)
@@ -1043,11 +1054,17 @@ static av_cold int wavpack_decode_init(AVCodecContext *avctx)
s->fdec_num = 0;
- s->curr_frame.f = av_frame_alloc();
- s->prev_frame.f = av_frame_alloc();
-
- if (!s->curr_frame.f || !s->prev_frame.f)
- return AVERROR(ENOMEM);
+#if HAVE_THREADS
+ if (ff_thread_sync_ref(avctx, offsetof(WavpackContext, progress_pool)) == FF_THREAD_IS_FIRST_THREAD) {
+ s->progress_pool = ff_refstruct_pool_alloc_ext(sizeof(*s->curr_progress),
+ FF_REFSTRUCT_POOL_FLAG_FREE_ON_INIT_ERROR, NULL,
+ progress_pool_init_cb,
+ progress_pool_reset_cb,
+ progress_pool_free_entry_cb, NULL);
+ if (!s->progress_pool)
+ return AVERROR(ENOMEM);
+ }
+#endif
return 0;
}
@@ -1061,19 +1078,14 @@ static av_cold int wavpack_decode_end(AVCodecContext *avctx)
av_freep(&s->fdec);
s->fdec_num = 0;
- ff_thread_release_ext_buffer(&s->curr_frame);
- av_frame_free(&s->curr_frame.f);
-
- ff_thread_release_ext_buffer(&s->prev_frame);
- av_frame_free(&s->prev_frame.f);
-
- ff_refstruct_unref(&s->dsdctx);
+ ff_refstruct_pool_uninit(&s->progress_pool);
+ wv_dsd_reset(s, 0);
return 0;
}
-static int wavpack_decode_block(AVCodecContext *avctx, int block_no,
- const uint8_t *buf, int buf_size)
+static int wavpack_decode_block(AVCodecContext *avctx, AVFrame *frame, int block_no,
+ const uint8_t *buf, int buf_size, int *new_progress)
{
WavpackContext *wc = avctx->priv_data;
WavpackFrameContext *s;
@@ -1521,7 +1533,6 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no,
av_log(avctx, AV_LOG_ERROR, "Error reinitializing the DSD context\n");
return ret;
}
- ff_thread_release_ext_buffer(&wc->curr_frame);
ff_init_dsd_data();
}
av_channel_layout_copy(&avctx->ch_layout, &new_ch_layout);
@@ -1529,18 +1540,25 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no,
avctx->sample_fmt = sample_fmt;
avctx->bits_per_raw_sample = orig_bpp;
- ff_thread_release_ext_buffer(&wc->prev_frame);
- FFSWAP(ThreadFrame, wc->curr_frame, wc->prev_frame);
-
/* get output buffer */
- wc->curr_frame.f->nb_samples = s->samples;
- ret = ff_thread_get_ext_buffer(avctx, &wc->curr_frame,
- AV_GET_BUFFER_FLAG_REF);
+ frame->nb_samples = s->samples;
+ ret = ff_thread_get_buffer(avctx, frame, 0);
if (ret < 0)
return ret;
- wc->frame = wc->curr_frame.f;
- ff_thread_finish_setup(avctx);
+ av_assert1(!!wc->progress_pool == !!(avctx->active_thread_type & FF_THREAD_FRAME));
+ if (wc->progress_pool) {
+ if (wc->dsdctx) {
+ ff_refstruct_unref(&wc->prev_progress);
+ wc->prev_progress = ff_refstruct_pool_get(wc->progress_pool);
+ if (!wc->prev_progress)
+ return AVERROR(ENOMEM);
+ FFSWAP(ThreadProgress*, wc->prev_progress, wc->curr_progress);
+ *new_progress = 1;
+ }
+ av_assert1(!!wc->dsdctx == !!wc->curr_progress);
+ ff_thread_finish_setup(avctx);
+ }
}
if (wc->ch_offset + s->stereo >= avctx->ch_layout.nb_channels) {
@@ -1548,9 +1566,9 @@ static int wavpack_decode_block(AVCodecContext *avctx, int block_no,
return ((avctx->err_recognition & AV_EF_EXPLODE) || !wc->ch_offset) ? AVERROR_INVALIDDATA : 0;
}
- samples_l = wc->frame->extended_data[wc->ch_offset];
+ samples_l = frame->extended_data[wc->ch_offset];
if (s->stereo)
- samples_r = wc->frame->extended_data[wc->ch_offset + 1];
+ samples_r = frame->extended_data[wc->ch_offset + 1];
wc->ch_offset += 1 + s->stereo;
@@ -1602,25 +1620,27 @@ static int dsd_channel(AVCodecContext *avctx, void *frmptr, int jobnr, int threa
const WavpackContext *s = avctx->priv_data;
AVFrame *frame = frmptr;
- ff_dsd2pcm_translate (&s->dsdctx [jobnr], s->samples, 0,
+ ff_dsd2pcm_translate(&s->dsdctx[jobnr], s->samples, 0,
(uint8_t *)frame->extended_data[jobnr], 4,
(float *)frame->extended_data[jobnr], 1);
return 0;
}
-static int wavpack_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
+static int wavpack_decode_frame(AVCodecContext *avctx, AVFrame *frame,
int *got_frame_ptr, AVPacket *avpkt)
{
WavpackContext *s = avctx->priv_data;
const uint8_t *buf = avpkt->data;
int buf_size = avpkt->size;
int frame_size, ret, frame_flags;
+ int new_progress = 0;
+
+ av_assert1(!s->curr_progress || s->dsdctx);
if (avpkt->size <= WV_HEADER_SIZE)
return AVERROR_INVALIDDATA;
- s->frame = NULL;
s->block = 0;
s->ch_offset = 0;
@@ -1646,7 +1666,8 @@ static int wavpack_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
ret = AVERROR_INVALIDDATA;
goto error;
}
- if ((ret = wavpack_decode_block(avctx, s->block, buf, frame_size)) < 0)
+ ret = wavpack_decode_block(avctx, frame, s->block, buf, frame_size, &new_progress);
+ if (ret < 0)
goto error;
s->block++;
buf += frame_size;
@@ -1659,26 +1680,23 @@ static int wavpack_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
goto error;
}
- ff_thread_await_progress(&s->prev_frame, INT_MAX, 0);
- ff_thread_release_ext_buffer(&s->prev_frame);
-
- if (s->modulation == MODULATION_DSD)
- avctx->execute2(avctx, dsd_channel, s->frame, NULL, avctx->ch_layout.nb_channels);
-
- ff_thread_report_progress(&s->curr_frame, INT_MAX, 0);
-
- if ((ret = av_frame_ref(rframe, s->frame)) < 0)
- return ret;
+ if (s->dsdctx) {
+ if (s->prev_progress)
+ ff_thread_progress_await(s->prev_progress, INT_MAX);
+ avctx->execute2(avctx, dsd_channel, frame, NULL, avctx->ch_layout.nb_channels);
+ if (s->curr_progress)
+ ff_thread_progress_report(s->curr_progress, INT_MAX);
+ }
*got_frame_ptr = 1;
return avpkt->size;
error:
- if (s->frame) {
- ff_thread_await_progress(&s->prev_frame, INT_MAX, 0);
- ff_thread_release_ext_buffer(&s->prev_frame);
- ff_thread_report_progress(&s->curr_frame, INT_MAX, 0);
+ if (new_progress) {
+ if (s->prev_progress)
+ ff_thread_progress_await(s->prev_progress, INT_MAX);
+ ff_thread_progress_report(s->curr_progress, INT_MAX);
}
return ret;
@@ -1697,6 +1715,5 @@ const FFCodec ff_wavpack_decoder = {
UPDATE_THREAD_CONTEXT(update_thread_context),
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS |
AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_CHANNEL_CONF,
- .caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
- FF_CODEC_CAP_ALLOCATE_PROGRESS,
+ .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
};
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 12/27] avcodec/wavpack: Move initializing DSD data to a better place
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (9 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 11/27] avcodec/wavpack: Use ThreadProgress API Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 13/27] avcodec/wavpack: Only reset DSD context upon parameter change Andreas Rheinhardt
` (17 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Namely to code that is only executed if we are indeed
initializing a DSD context.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/wavpack.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 83f42f392d..6fd297a002 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -1011,6 +1011,8 @@ static int wv_dsd_reset(WavpackContext *s, int channels)
for (i = 0; i < channels; i++)
memset(s->dsdctx[i].buf, 0x69, sizeof(s->dsdctx[i].buf));
+ ff_init_dsd_data();
+
return 0;
}
@@ -1533,7 +1535,6 @@ static int wavpack_decode_block(AVCodecContext *avctx, AVFrame *frame, int block
av_log(avctx, AV_LOG_ERROR, "Error reinitializing the DSD context\n");
return ret;
}
- ff_init_dsd_data();
}
av_channel_layout_copy(&avctx->ch_layout, &new_ch_layout);
avctx->sample_rate = new_samplerate;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 13/27] avcodec/wavpack: Only reset DSD context upon parameter change
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (10 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 12/27] avcodec/wavpack: Move initializing DSD data to a better place Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 14/27] avcodec/wavpack: Optimize always-false comparison away Andreas Rheinhardt
` (16 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
The current code resets it all the time unless we are decoding
a DSD frame with identical parameters to the last frame.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/wavpack.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 6fd297a002..51ac943fe7 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -1526,10 +1526,10 @@ static int wavpack_decode_block(AVCodecContext *avctx, AVFrame *frame, int block
}
/* clear DSD state if stream properties change */
- if (new_ch_layout.nb_channels != wc->dsd_channels ||
- av_channel_layout_compare(&new_ch_layout, &avctx->ch_layout) ||
- new_samplerate != avctx->sample_rate ||
- !!got_dsd != !!wc->dsdctx) {
+ if ((wc->dsdctx && !got_dsd) ||
+ got_dsd && (new_ch_layout.nb_channels != wc->dsd_channels ||
+ av_channel_layout_compare(&new_ch_layout, &avctx->ch_layout) ||
+ new_samplerate != avctx->sample_rate)) {
ret = wv_dsd_reset(wc, got_dsd ? new_ch_layout.nb_channels : 0);
if (ret < 0) {
av_log(avctx, AV_LOG_ERROR, "Error reinitializing the DSD context\n");
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 14/27] avcodec/wavpack: Optimize always-false comparison away
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (11 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 13/27] avcodec/wavpack: Only reset DSD context upon parameter change Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 15/27] avcodec/wavpack: Move transient variable from context to stack Andreas Rheinhardt
` (15 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Also use the correct type limit SIZE_MAX; INT_MAX comes
from a time when this used av_buffer_allocz() which used
an int at the time.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/wavpack.c | 5 ++++-
libavcodec/wavpack.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 51ac943fe7..6ab9088213 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -1000,7 +1000,8 @@ static int wv_dsd_reset(WavpackContext *s, int channels)
if (!channels)
return 0;
- if (channels > INT_MAX / sizeof(*s->dsdctx))
+ if (WV_MAX_CHANNELS > SIZE_MAX / sizeof(*s->dsdctx) &&
+ channels > SIZE_MAX / sizeof(*s->dsdctx))
return AVERROR(EINVAL);
s->dsdctx = ff_refstruct_allocz(channels * sizeof(*s->dsdctx));
@@ -1433,6 +1434,7 @@ static int wavpack_decode_block(AVCodecContext *avctx, AVFrame *frame, int block
av_log(avctx, AV_LOG_ERROR, "Invalid channel info size %d\n",
size);
}
+ av_assert1(chan <= WV_MAX_CHANNELS);
break;
case WP_ID_SAMPLE_RATE:
if (size != 3) {
@@ -1524,6 +1526,7 @@ static int wavpack_decode_block(AVCodecContext *avctx, AVFrame *frame, int block
} else {
av_channel_layout_default(&new_ch_layout, s->stereo + 1);
}
+ av_assert1(new_ch_layout.nb_channels <= WV_MAX_CHANNELS);
/* clear DSD state if stream properties change */
if ((wc->dsdctx && !got_dsd) ||
diff --git a/libavcodec/wavpack.h b/libavcodec/wavpack.h
index 9f62f8406d..2efbb1fd06 100644
--- a/libavcodec/wavpack.h
+++ b/libavcodec/wavpack.h
@@ -57,6 +57,7 @@
#define WV_FLT_ZERO_SENT 0x08
#define WV_FLT_ZERO_SIGN 0x10
+#define WV_MAX_CHANNELS (1 << 12)
#define WV_MAX_SAMPLES 150000
enum WP_ID_Flags {
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 15/27] avcodec/wavpack: Move transient variable from context to stack
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (12 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 14/27] avcodec/wavpack: Optimize always-false comparison away Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 16/27] avcodec/vp8: Convert to ProgressFrame API Andreas Rheinhardt
` (14 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/wavpack.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 6ab9088213..d4cf489c0f 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -103,7 +103,6 @@ typedef struct WavpackContext {
WavpackFrameContext **fdec;
int fdec_num;
- int block;
int samples;
int ch_offset;
@@ -1638,14 +1637,13 @@ static int wavpack_decode_frame(AVCodecContext *avctx, AVFrame *frame,
const uint8_t *buf = avpkt->data;
int buf_size = avpkt->size;
int frame_size, ret, frame_flags;
- int new_progress = 0;
+ int block = 0, new_progress = 0;
av_assert1(!s->curr_progress || s->dsdctx);
if (avpkt->size <= WV_HEADER_SIZE)
return AVERROR_INVALIDDATA;
- s->block = 0;
s->ch_offset = 0;
/* determine number of samples */
@@ -1666,14 +1664,15 @@ static int wavpack_decode_frame(AVCodecContext *avctx, AVFrame *frame,
if (frame_size <= 0 || frame_size > buf_size) {
av_log(avctx, AV_LOG_ERROR,
"Block %d has invalid size (size %d vs. %d bytes left)\n",
- s->block, frame_size, buf_size);
+ block, frame_size, buf_size);
ret = AVERROR_INVALIDDATA;
goto error;
}
- ret = wavpack_decode_block(avctx, frame, s->block, buf, frame_size, &new_progress);
+ ret = wavpack_decode_block(avctx, frame, block, buf,
+ frame_size, &new_progress);
if (ret < 0)
goto error;
- s->block++;
+ block++;
buf += frame_size;
buf_size -= frame_size;
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 16/27] avcodec/vp8: Convert to ProgressFrame API
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (13 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 15/27] avcodec/wavpack: Move transient variable from context to stack Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 17/27] avcodec/vp8: Mark flushing functions as av_cold Andreas Rheinhardt
` (13 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/pthread_frame.c | 2 +-
libavcodec/vp8.c | 104 ++++++++++++-------------------------
libavcodec/vp8.h | 5 +-
libavcodec/webp.c | 3 +-
4 files changed, 40 insertions(+), 74 deletions(-)
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index ee571be610..e42a9563cd 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -974,7 +974,7 @@ int ff_thread_get_ext_buffer(AVCodecContext *avctx, ThreadFrame *f, int flags)
/* Hint: It is possible for this function to be called with codecs
* that don't support frame threading at all, namely in case
* a frame-threaded decoder shares code with codecs that are not.
- * This currently affects non-MPEG-4 mpegvideo codecs and and VP7.
+ * This currently affects non-MPEG-4 mpegvideo codecs.
* The following check will always be true for them. */
if (!(avctx->active_thread_type & FF_THREAD_FRAME))
return ff_get_buffer(avctx, f->f, flags);
diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index 539b5c5395..a1443f6571 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -35,9 +35,9 @@
#include "hwaccel_internal.h"
#include "hwconfig.h"
#include "mathops.h"
+#include "progressframe.h"
#include "refstruct.h"
#include "thread.h"
-#include "threadframe.h"
#include "vp8.h"
#include "vp89_rac.h"
#include "vp8data.h"
@@ -103,9 +103,9 @@ static void free_buffers(VP8Context *s)
static int vp8_alloc_frame(VP8Context *s, VP8Frame *f, int ref)
{
- int ret;
- if ((ret = ff_thread_get_ext_buffer(s->avctx, &f->tf,
- ref ? AV_GET_BUFFER_FLAG_REF : 0)) < 0)
+ int ret = ff_progress_frame_get_buffer(s->avctx, &f->tf,
+ ref ? AV_GET_BUFFER_FLAG_REF : 0);
+ if (ret < 0)
return ret;
if (!(f->seg_map = ff_refstruct_allocz(s->mb_width * s->mb_height)))
goto fail;
@@ -117,7 +117,7 @@ static int vp8_alloc_frame(VP8Context *s, VP8Frame *f, int ref)
fail:
ff_refstruct_unref(&f->seg_map);
- ff_thread_release_ext_buffer(&f->tf);
+ ff_progress_frame_unref(&f->tf);
return ret;
}
@@ -125,26 +125,9 @@ static void vp8_release_frame(VP8Frame *f)
{
ff_refstruct_unref(&f->seg_map);
ff_refstruct_unref(&f->hwaccel_picture_private);
- ff_thread_release_ext_buffer(&f->tf);
+ ff_progress_frame_unref(&f->tf);
}
-#if CONFIG_VP8_DECODER
-static int vp8_ref_frame(VP8Frame *dst, const VP8Frame *src)
-{
- int ret;
-
- vp8_release_frame(dst);
-
- if ((ret = ff_thread_ref_frame(&dst->tf, &src->tf)) < 0)
- return ret;
- ff_refstruct_replace(&dst->seg_map, src->seg_map);
- ff_refstruct_replace(&dst->hwaccel_picture_private,
- src->hwaccel_picture_private);
-
- return 0;
-}
-#endif /* CONFIG_VP8_DECODER */
-
static void vp8_decode_flush_impl(AVCodecContext *avctx, int free_mem)
{
VP8Context *s = avctx->priv_data;
@@ -184,7 +167,7 @@ static VP8Frame *vp8_find_free_buffer(VP8Context *s)
av_log(s->avctx, AV_LOG_FATAL, "Ran out of free frames!\n");
abort();
}
- if (frame->tf.f->buf[0])
+ if (frame->tf.f)
vp8_release_frame(frame);
return frame;
@@ -1830,7 +1813,7 @@ static const uint8_t subpel_idx[3][8] = {
*/
static av_always_inline
void vp8_mc_luma(VP8Context *s, VP8ThreadData *td, uint8_t *dst,
- const ThreadFrame *ref, const VP8mv *mv,
+ const ProgressFrame *ref, const VP8mv *mv,
int x_off, int y_off, int block_w, int block_h,
int width, int height, ptrdiff_t linesize,
vp8_mc_func mc_func[3][3])
@@ -1847,7 +1830,7 @@ void vp8_mc_luma(VP8Context *s, VP8ThreadData *td, uint8_t *dst,
y_off += mv->y >> 2;
// edge emulation
- ff_thread_await_progress(ref, (3 + y_off + block_h + subpel_idx[2][my]) >> 4, 0);
+ ff_progress_frame_await(ref, (3 + y_off + block_h + subpel_idx[2][my]) >> 4);
src += y_off * linesize + x_off;
if (x_off < mx_idx || x_off >= width - block_w - subpel_idx[2][mx] ||
y_off < my_idx || y_off >= height - block_h - subpel_idx[2][my]) {
@@ -1863,7 +1846,7 @@ void vp8_mc_luma(VP8Context *s, VP8ThreadData *td, uint8_t *dst,
}
mc_func[my_idx][mx_idx](dst, linesize, src, src_linesize, block_h, mx, my);
} else {
- ff_thread_await_progress(ref, (3 + y_off + block_h) >> 4, 0);
+ ff_progress_frame_await(ref, (3 + y_off + block_h) >> 4);
mc_func[0][0](dst, linesize, src + y_off * linesize + x_off,
linesize, block_h, 0, 0);
}
@@ -1888,7 +1871,7 @@ void vp8_mc_luma(VP8Context *s, VP8ThreadData *td, uint8_t *dst,
*/
static av_always_inline
void vp8_mc_chroma(VP8Context *s, VP8ThreadData *td, uint8_t *dst1,
- uint8_t *dst2, const ThreadFrame *ref, const VP8mv *mv,
+ uint8_t *dst2, const ProgressFrame *ref, const VP8mv *mv,
int x_off, int y_off, int block_w, int block_h,
int width, int height, ptrdiff_t linesize,
vp8_mc_func mc_func[3][3])
@@ -1905,7 +1888,7 @@ void vp8_mc_chroma(VP8Context *s, VP8ThreadData *td, uint8_t *dst1,
// edge emulation
src1 += y_off * linesize + x_off;
src2 += y_off * linesize + x_off;
- ff_thread_await_progress(ref, (3 + y_off + block_h + subpel_idx[2][my]) >> 3, 0);
+ ff_progress_frame_await(ref, (3 + y_off + block_h + subpel_idx[2][my]) >> 3);
if (x_off < mx_idx || x_off >= width - block_w - subpel_idx[2][mx] ||
y_off < my_idx || y_off >= height - block_h - subpel_idx[2][my]) {
s->vdsp.emulated_edge_mc(td->edge_emu_buffer,
@@ -1930,7 +1913,7 @@ void vp8_mc_chroma(VP8Context *s, VP8ThreadData *td, uint8_t *dst1,
mc_func[my_idx][mx_idx](dst2, linesize, src2, linesize, block_h, mx, my);
}
} else {
- ff_thread_await_progress(ref, (3 + y_off + block_h) >> 3, 0);
+ ff_progress_frame_await(ref, (3 + y_off + block_h) >> 3);
mc_func[0][0](dst1, linesize, src1 + y_off * linesize + x_off, linesize, block_h, 0, 0);
mc_func[0][0](dst2, linesize, src2 + y_off * linesize + x_off, linesize, block_h, 0, 0);
}
@@ -1938,7 +1921,7 @@ void vp8_mc_chroma(VP8Context *s, VP8ThreadData *td, uint8_t *dst1,
static av_always_inline
void vp8_mc_part(VP8Context *s, VP8ThreadData *td, uint8_t *const dst[3],
- const ThreadFrame *ref_frame, int x_off, int y_off,
+ const ProgressFrame *ref_frame, int x_off, int y_off,
int bx_off, int by_off, int block_w, int block_h,
int width, int height, const VP8mv *mv)
{
@@ -2003,7 +1986,7 @@ void inter_predict(VP8Context *s, VP8ThreadData *td, uint8_t *const dst[3],
{
int x_off = mb_x << 4, y_off = mb_y << 4;
int width = 16 * s->mb_width, height = 16 * s->mb_height;
- const ThreadFrame *ref = &s->framep[mb->ref_frame]->tf;
+ const ProgressFrame *ref = &s->framep[mb->ref_frame]->tf;
const VP8mv *bmv = mb->bmv;
switch (mb->partitioning) {
@@ -2423,7 +2406,7 @@ static av_always_inline int decode_mb_row_no_filter(AVCodecContext *avctx, void
// if we re-use the same map.
if (prev_frame && s->segmentation.enabled &&
!s->segmentation.update_map)
- ff_thread_await_progress(&prev_frame->tf, mb_y, 0);
+ ff_progress_frame_await(&prev_frame->tf, mb_y);
mb = s->macroblocks + (s->mb_height - mb_y - 1) * 2;
memset(mb - 1, 0, sizeof(*mb)); // zero left macroblock
AV_WN32A(s->intra4x4_pred_mode_left, DC_PRED * 0x01010101);
@@ -2631,7 +2614,7 @@ int vp78_decode_mb_row_sliced(AVCodecContext *avctx, void *tdata, int jobnr,
td->mv_bounds.mv_max.y -= 64 * num_jobs;
if (avctx->active_thread_type == FF_THREAD_FRAME)
- ff_thread_report_progress(&curframe->tf, mb_y, 0);
+ ff_progress_frame_report(&curframe->tf, mb_y);
}
return 0;
@@ -2699,7 +2682,7 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame,
// release no longer referenced frames
for (i = 0; i < 5; i++)
- if (s->frames[i].tf.f->buf[0] &&
+ if (s->frames[i].tf.f &&
&s->frames[i] != prev_frame &&
&s->frames[i] != s->framep[VP8_FRAME_PREVIOUS] &&
&s->frames[i] != s->framep[VP8_FRAME_GOLDEN] &&
@@ -2728,14 +2711,14 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame,
goto err;
}
+ if ((ret = vp8_alloc_frame(s, curframe, referenced)) < 0)
+ goto err;
if (s->keyframe)
curframe->tf.f->flags |= AV_FRAME_FLAG_KEY;
else
curframe->tf.f->flags &= ~AV_FRAME_FLAG_KEY;
curframe->tf.f->pict_type = s->keyframe ? AV_PICTURE_TYPE_I
: AV_PICTURE_TYPE_P;
- if ((ret = vp8_alloc_frame(s, curframe, referenced)) < 0)
- goto err;
// check if golden and altref are swapped
if (s->update_altref != VP8_FRAME_NONE)
@@ -2792,7 +2775,7 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame,
// if we re-use the same map.
if (prev_frame && s->segmentation.enabled &&
!s->segmentation.update_map)
- ff_thread_await_progress(&prev_frame->tf, 1, 0);
+ ff_progress_frame_await(&prev_frame->tf, 1);
if (is_vp7)
ret = vp7_decode_mv_mb_modes(avctx, curframe, prev_frame);
else
@@ -2823,7 +2806,7 @@ int vp78_decode_frame(AVCodecContext *avctx, AVFrame *rframe, int *got_frame,
num_jobs);
}
- ff_thread_report_progress(&curframe->tf, INT_MAX, 0);
+ ff_progress_frame_report(&curframe->tf, INT_MAX);
memcpy(&s->framep[0], &s->next_framep[0], sizeof(s->framep[0]) * 4);
skip_decode:
@@ -2860,24 +2843,8 @@ static int vp7_decode_frame(AVCodecContext *avctx, AVFrame *frame,
av_cold int ff_vp8_decode_free(AVCodecContext *avctx)
{
- VP8Context *s = avctx->priv_data;
- int i;
-
vp8_decode_flush_impl(avctx, 1);
- for (i = 0; i < FF_ARRAY_ELEMS(s->frames); i++)
- av_frame_free(&s->frames[i].tf.f);
-
- return 0;
-}
-static av_cold int vp8_init_frames(VP8Context *s)
-{
- int i;
- for (i = 0; i < FF_ARRAY_ELEMS(s->frames); i++) {
- s->frames[i].tf.f = av_frame_alloc();
- if (!s->frames[i].tf.f)
- return AVERROR(ENOMEM);
- }
return 0;
}
@@ -2885,7 +2852,6 @@ static av_always_inline
int vp78_decode_init(AVCodecContext *avctx, int is_vp7)
{
VP8Context *s = avctx->priv_data;
- int ret;
s->avctx = avctx;
s->pix_fmt = AV_PIX_FMT_NONE;
@@ -2909,11 +2875,6 @@ int vp78_decode_init(AVCodecContext *avctx, int is_vp7)
/* does not change for VP8 */
memcpy(s->prob[0].scan, ff_zigzag_scan, sizeof(s->prob[0].scan));
- if ((ret = vp8_init_frames(s)) < 0) {
- ff_vp8_decode_free(avctx);
- return ret;
- }
-
return 0;
}
@@ -2931,13 +2892,20 @@ av_cold int ff_vp8_decode_init(AVCodecContext *avctx)
#if CONFIG_VP8_DECODER
#if HAVE_THREADS
+static void vp8_replace_frame(VP8Frame *dst, const VP8Frame *src)
+{
+ ff_progress_frame_replace(&dst->tf, &src->tf);
+ ff_refstruct_replace(&dst->seg_map, src->seg_map);
+ ff_refstruct_replace(&dst->hwaccel_picture_private,
+ src->hwaccel_picture_private);
+}
+
#define REBASE(pic) ((pic) ? (pic) - &s_src->frames[0] + &s->frames[0] : NULL)
static int vp8_decode_update_thread_context(AVCodecContext *dst,
const AVCodecContext *src)
{
VP8Context *s = dst->priv_data, *s_src = src->priv_data;
- int i;
if (s->macroblocks_base &&
(s_src->mb_width != s->mb_width || s_src->mb_height != s->mb_height)) {
@@ -2952,13 +2920,8 @@ static int vp8_decode_update_thread_context(AVCodecContext *dst,
s->lf_delta = s_src->lf_delta;
memcpy(s->sign_bias, s_src->sign_bias, sizeof(s->sign_bias));
- for (i = 0; i < FF_ARRAY_ELEMS(s_src->frames); i++) {
- if (s_src->frames[i].tf.f->buf[0]) {
- int ret = vp8_ref_frame(&s->frames[i], &s_src->frames[i]);
- if (ret < 0)
- return ret;
- }
- }
+ for (int i = 0; i < FF_ARRAY_ELEMS(s_src->frames); i++)
+ vp8_replace_frame(&s->frames[i], &s_src->frames[i]);
s->framep[0] = REBASE(s_src->next_framep[0]);
s->framep[1] = REBASE(s_src->next_framep[1]);
@@ -2982,6 +2945,7 @@ const FFCodec ff_vp7_decoder = {
FF_CODEC_DECODE_CB(vp7_decode_frame),
.p.capabilities = AV_CODEC_CAP_DR1,
.flush = vp8_decode_flush,
+ .caps_internal = FF_CODEC_CAP_USES_PROGRESSFRAMES,
};
#endif /* CONFIG_VP7_DECODER */
@@ -2997,7 +2961,7 @@ const FFCodec ff_vp8_decoder = {
FF_CODEC_DECODE_CB(ff_vp8_decode_frame),
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS |
AV_CODEC_CAP_SLICE_THREADS,
- .caps_internal = FF_CODEC_CAP_ALLOCATE_PROGRESS,
+ .caps_internal = FF_CODEC_CAP_USES_PROGRESSFRAMES,
.flush = vp8_decode_flush,
UPDATE_THREAD_CONTEXT(vp8_decode_update_thread_context),
.hw_configs = (const AVCodecHWConfigInternal *const []) {
diff --git a/libavcodec/vp8.h b/libavcodec/vp8.h
index 798f67b3de..9bdef0aa88 100644
--- a/libavcodec/vp8.h
+++ b/libavcodec/vp8.h
@@ -31,8 +31,9 @@
#include "libavutil/mem_internal.h"
#include "libavutil/thread.h"
+#include "avcodec.h"
#include "h264pred.h"
-#include "threadframe.h"
+#include "progressframe.h"
#include "videodsp.h"
#include "vp8dsp.h"
#include "vpx_rac.h"
@@ -150,7 +151,7 @@ typedef struct VP8ThreadData {
} VP8ThreadData;
typedef struct VP8Frame {
- ThreadFrame tf;
+ ProgressFrame tf;
uint8_t *seg_map; ///< RefStruct reference
void *hwaccel_picture_private; ///< RefStruct reference
diff --git a/libavcodec/webp.c b/libavcodec/webp.c
index dbcc5e73eb..7c2a5f0111 100644
--- a/libavcodec/webp.c
+++ b/libavcodec/webp.c
@@ -1571,5 +1571,6 @@ const FFCodec ff_webp_decoder = {
FF_CODEC_DECODE_CB(webp_decode_frame),
.close = webp_decode_close,
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
- .caps_internal = FF_CODEC_CAP_ICC_PROFILES,
+ .caps_internal = FF_CODEC_CAP_ICC_PROFILES |
+ FF_CODEC_CAP_USES_PROGRESSFRAMES,
};
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 17/27] avcodec/vp8: Mark flushing functions as av_cold
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (14 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 16/27] avcodec/vp8: Convert to ProgressFrame API Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 18/27] avcodec/codec_internal: Remove FF_CODEC_CAP_ALLOCATE_PROGRESS Andreas Rheinhardt
` (12 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/vp8.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/vp8.c b/libavcodec/vp8.c
index a1443f6571..f37938ad27 100644
--- a/libavcodec/vp8.c
+++ b/libavcodec/vp8.c
@@ -128,7 +128,7 @@ static void vp8_release_frame(VP8Frame *f)
ff_progress_frame_unref(&f->tf);
}
-static void vp8_decode_flush_impl(AVCodecContext *avctx, int free_mem)
+static av_cold void vp8_decode_flush_impl(AVCodecContext *avctx, int free_mem)
{
VP8Context *s = avctx->priv_data;
int i;
@@ -144,7 +144,7 @@ static void vp8_decode_flush_impl(AVCodecContext *avctx, int free_mem)
FF_HW_SIMPLE_CALL(avctx, flush);
}
-static void vp8_decode_flush(AVCodecContext *avctx)
+static av_cold void vp8_decode_flush(AVCodecContext *avctx)
{
vp8_decode_flush_impl(avctx, 0);
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 18/27] avcodec/codec_internal: Remove FF_CODEC_CAP_ALLOCATE_PROGRESS
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (15 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 17/27] avcodec/vp8: Mark flushing functions as av_cold Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 19/27] avcodec/hevcdec: Switch to ProgressFrames Andreas Rheinhardt
` (11 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Before commit f025b8e110b36c1cdb4fb56c4cd57aeca1767b5b,
every frame-threaded decoder used ThreadFrames, even when
they did not have any inter-frame dependencies at all.
In order to distinguish those decoders that need the AVBuffer
for progress communication from those that do not (to avoid
the allocation for the latter), the former decoders were marked
with the FF_CODEC_CAP_ALLOCATE_PROGRESS internal codec cap.
Yet distinguishing these two can be done in a more natural way:
Don't use ThreadFrames when not needed and split ff_thread_get_buffer()
into a core function that calls the user's get_buffer2 callback
and a wrapper around it that also allocates the progress AVBuffer.
This has been done in 02220b88fc38ef9dd4f2d519f5d3e4151258b60c
and since that commit the ALLOCATE_PROGRESS cap was nearly redundant.
The only exception was WebP and VP8. WebP can contain VP8
and uses the VP8 decoder directly (i.e. they share the same
AVCodecContext). Both decoders are frame-threaded and VP8
has inter-frame dependencies (in general, not in valid WebP)
and therefore the ALLOCATE_PROGRESS cap. In order to avoid
allocating progress in case of a frame-threaded WebP decoder
the cap and the check for the cap has been kept in place.
Yet now the VP8 decoder has been switched to use ProgressFrames
and therefore there is just no reason any more for this check
and the cap. This commit therefore removes both.
Also change the value of FF_CODEC_CAP_USES_PROGRESSFRAMES
to leave no gaps.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
doc/multithreading.txt | 15 +++++++--------
libavcodec/codec_internal.h | 7 +------
libavcodec/ffv1dec.c | 3 +--
libavcodec/h264dec.c | 2 +-
libavcodec/hevcdec.c | 2 +-
libavcodec/mpeg4videodec.c | 3 +--
libavcodec/pngdec.c | 3 +--
libavcodec/pthread_frame.c | 12 +++++-------
libavcodec/rv30.c | 1 -
libavcodec/rv40.c | 1 -
libavcodec/tests/avcodec.c | 5 -----
11 files changed, 18 insertions(+), 36 deletions(-)
diff --git a/doc/multithreading.txt b/doc/multithreading.txt
index 6c65ca9651..842d331e4f 100644
--- a/doc/multithreading.txt
+++ b/doc/multithreading.txt
@@ -36,9 +36,9 @@ Frame threading -
* Codecs similar to ffv1, whose streams don't reset across frames,
will not work because their bitstreams cannot be decoded in parallel.
-* The contents of buffers must not be read before ff_thread_await_progress()
+* The contents of buffers must not be read before ff_progress_frame_await()
has been called on them. reget_buffer() and buffer age optimizations no longer work.
-* The contents of buffers must not be written to after ff_thread_report_progress()
+* The contents of buffers must not be written to after ff_progress_frame_report()
has been called on them. This includes draw_edges().
Porting codecs to frame threading
@@ -53,14 +53,13 @@ thread.
Add AV_CODEC_CAP_FRAME_THREADS to the codec capabilities. There will be very little
speed gain at this point but it should work.
-If there are inter-frame dependencies, so the codec calls
-ff_thread_report/await_progress(), set FF_CODEC_CAP_ALLOCATE_PROGRESS in
-FFCodec.caps_internal and use ff_thread_get_buffer() to allocate frames.
-Otherwise decode directly into the user-supplied frames.
+Use ff_thread_get_buffer() (or ff_progress_frame_get_buffer()
+in case you have inter-frame dependencies and use the ProgressFrame API)
+to allocate frame buffers.
-Call ff_thread_report_progress() after some part of the current picture has decoded.
+Call ff_progress_frame_report() after some part of the current picture has decoded.
A good place to put this is where draw_horiz_band() is called - add this if it isn't
called anywhere, as it's useful too and the implementation is trivial when you're
doing this. Note that draw_edges() needs to be called before reporting progress.
-Before accessing a reference frame or its MVs, call ff_thread_await_progress().
+Before accessing a reference frame or its MVs, call ff_progress_frame_await().
diff --git a/libavcodec/codec_internal.h b/libavcodec/codec_internal.h
index 832e6440d7..1cd1f684f9 100644
--- a/libavcodec/codec_internal.h
+++ b/libavcodec/codec_internal.h
@@ -65,12 +65,7 @@
/**
* The decoder might make use of the ProgressFrame API.
*/
-#define FF_CODEC_CAP_USES_PROGRESSFRAMES (1 << 11)
-/*
- * The codec supports frame threading and has inter-frame dependencies, so it
- * uses ff_thread_report/await_progress().
- */
-#define FF_CODEC_CAP_ALLOCATE_PROGRESS (1 << 6)
+#define FF_CODEC_CAP_USES_PROGRESSFRAMES (1 << 6)
/**
* Codec handles avctx->thread_count == 0 (auto) internally.
*/
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index ba535e800d..dd594e3f9f 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -1129,6 +1129,5 @@ const FFCodec ff_ffv1_decoder = {
UPDATE_THREAD_CONTEXT(update_thread_context),
.p.capabilities = AV_CODEC_CAP_DR1 |
AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_SLICE_THREADS,
- .caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
- FF_CODEC_CAP_ALLOCATE_PROGRESS,
+ .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
};
diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 727dc1a662..6330fb9c48 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -1156,7 +1156,7 @@ const FFCodec ff_h264_decoder = {
NULL
},
.caps_internal = FF_CODEC_CAP_EXPORTS_CROPPING |
- FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
+ FF_CODEC_CAP_INIT_CLEANUP,
.flush = h264_decode_flush,
UPDATE_THREAD_CONTEXT(ff_h264_update_thread_context),
UPDATE_THREAD_CONTEXT_FOR_USER(ff_h264_update_thread_context_for_user),
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 727b02f0f4..cedd09fe14 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3715,7 +3715,7 @@ const FFCodec ff_hevc_decoder = {
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_FRAME_THREADS,
.caps_internal = FF_CODEC_CAP_EXPORTS_CROPPING |
- FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP,
+ FF_CODEC_CAP_INIT_CLEANUP,
.p.profiles = NULL_IF_CONFIG_SMALL(ff_hevc_profiles),
.hw_configs = (const AVCodecHWConfigInternal *const []) {
#if CONFIG_HEVC_DXVA2_HWACCEL
diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
index 07de5d6d91..6a7a37e817 100644
--- a/libavcodec/mpeg4videodec.c
+++ b/libavcodec/mpeg4videodec.c
@@ -3861,8 +3861,7 @@ const FFCodec ff_mpeg4_decoder = {
FF_CODEC_DECODE_CB(ff_h263_decode_frame),
.p.capabilities = AV_CODEC_CAP_DRAW_HORIZ_BAND | AV_CODEC_CAP_DR1 |
AV_CODEC_CAP_DELAY | AV_CODEC_CAP_FRAME_THREADS,
- .caps_internal = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
- FF_CODEC_CAP_ALLOCATE_PROGRESS,
+ .caps_internal = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM,
.flush = ff_mpeg_flush,
.p.max_lowres = 3,
.p.profiles = NULL_IF_CONFIG_SMALL(ff_mpeg4_video_profiles),
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 16e35a8cc6..5a99b4a1c4 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -1940,7 +1940,6 @@ const FFCodec ff_apng_decoder = {
UPDATE_THREAD_CONTEXT(update_thread_context),
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
- FF_CODEC_CAP_ALLOCATE_PROGRESS |
FF_CODEC_CAP_ICC_PROFILES,
};
#endif
@@ -1958,7 +1957,7 @@ const FFCodec ff_png_decoder = {
UPDATE_THREAD_CONTEXT(update_thread_context),
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
.caps_internal = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
- FF_CODEC_CAP_ALLOCATE_PROGRESS | FF_CODEC_CAP_INIT_CLEANUP |
+ FF_CODEC_CAP_INIT_CLEANUP |
FF_CODEC_CAP_ICC_PROFILES,
};
#endif
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index e42a9563cd..4c073d5609 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -979,14 +979,12 @@ int ff_thread_get_ext_buffer(AVCodecContext *avctx, ThreadFrame *f, int flags)
if (!(avctx->active_thread_type & FF_THREAD_FRAME))
return ff_get_buffer(avctx, f->f, flags);
- if (ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_ALLOCATE_PROGRESS) {
- f->progress = ff_refstruct_allocz(sizeof(*f->progress));
- if (!f->progress)
- return AVERROR(ENOMEM);
+ f->progress = ff_refstruct_allocz(sizeof(*f->progress));
+ if (!f->progress)
+ return AVERROR(ENOMEM);
- atomic_init(&f->progress->progress[0], -1);
- atomic_init(&f->progress->progress[1], -1);
- }
+ atomic_init(&f->progress->progress[0], -1);
+ atomic_init(&f->progress->progress[1], -1);
ret = ff_thread_get_buffer(avctx, f->f, flags);
if (ret)
diff --git a/libavcodec/rv30.c b/libavcodec/rv30.c
index 9e13e71805..316962fbbb 100644
--- a/libavcodec/rv30.c
+++ b/libavcodec/rv30.c
@@ -304,5 +304,4 @@ const FFCodec ff_rv30_decoder = {
AV_CODEC_CAP_FRAME_THREADS,
.flush = ff_mpeg_flush,
UPDATE_THREAD_CONTEXT(ff_rv34_decode_update_thread_context),
- .caps_internal = FF_CODEC_CAP_ALLOCATE_PROGRESS,
};
diff --git a/libavcodec/rv40.c b/libavcodec/rv40.c
index e48aa1f684..19d4e742df 100644
--- a/libavcodec/rv40.c
+++ b/libavcodec/rv40.c
@@ -582,5 +582,4 @@ const FFCodec ff_rv40_decoder = {
AV_CODEC_CAP_FRAME_THREADS,
.flush = ff_mpeg_flush,
UPDATE_THREAD_CONTEXT(ff_rv34_decode_update_thread_context),
- .caps_internal = FF_CODEC_CAP_ALLOCATE_PROGRESS,
};
diff --git a/libavcodec/tests/avcodec.c b/libavcodec/tests/avcodec.c
index 58f54cac74..cd949f6385 100644
--- a/libavcodec/tests/avcodec.c
+++ b/libavcodec/tests/avcodec.c
@@ -142,7 +142,6 @@ int main(void){
}
}
if (codec2->caps_internal & (FF_CODEC_CAP_USES_PROGRESSFRAMES |
- FF_CODEC_CAP_ALLOCATE_PROGRESS |
FF_CODEC_CAP_SETS_PKT_DTS |
FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
FF_CODEC_CAP_EXPORTS_CROPPING |
@@ -172,10 +171,6 @@ int main(void){
AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE |
AV_CODEC_CAP_ENCODER_FLUSH))
ERR("Decoder %s has encoder-only capabilities\n");
- if (codec2->caps_internal & FF_CODEC_CAP_ALLOCATE_PROGRESS &&
- !(codec->capabilities & AV_CODEC_CAP_FRAME_THREADS))
- ERR("Decoder %s wants allocated progress without supporting"
- "frame threads\n");
if (codec2->cb_type != FF_CODEC_CB_TYPE_DECODE &&
codec2->caps_internal & FF_CODEC_CAP_SETS_PKT_DTS)
ERR("Decoder %s is marked as setting pkt_dts when it doesn't have"
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 19/27] avcodec/hevcdec: Switch to ProgressFrames
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (16 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 18/27] avcodec/codec_internal: Remove FF_CODEC_CAP_ALLOCATE_PROGRESS Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 20/27] avcodec/pngdec: " Andreas Rheinhardt
` (10 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Avoids implicit av_frame_ref() and therefore allocations
and error checks. It also avoids explicitly allocating
the AVFrames (done implicitly when getting the buffer).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/hevc_filter.c | 8 ++++----
libavcodec/hevc_mvs.c | 6 +++---
libavcodec/hevc_refs.c | 22 ++++++++++------------
libavcodec/hevcdec.c | 24 ++++++++++--------------
libavcodec/hevcdec.h | 4 ++--
5 files changed, 29 insertions(+), 35 deletions(-)
diff --git a/libavcodec/hevc_filter.c b/libavcodec/hevc_filter.c
index 0c45310ea6..b1e2ea7a66 100644
--- a/libavcodec/hevc_filter.c
+++ b/libavcodec/hevc_filter.c
@@ -26,7 +26,7 @@
#include "libavutil/internal.h"
#include "hevcdec.h"
-#include "threadframe.h"
+#include "progressframe.h"
#define LUMA 0
#define CB 1
@@ -874,15 +874,15 @@ void ff_hevc_hls_filter(HEVCLocalContext *lc, int x, int y, int ctb_size)
if (y && x_end) {
sao_filter_CTB(lc, s, x, y - ctb_size);
if (s->threads_type & FF_THREAD_FRAME )
- ff_thread_report_progress(&s->ref->tf, y, 0);
+ ff_progress_frame_report(&s->ref->tf, y);
}
if (x_end && y_end) {
sao_filter_CTB(lc, s, x , y);
if (s->threads_type & FF_THREAD_FRAME )
- ff_thread_report_progress(&s->ref->tf, y + ctb_size, 0);
+ ff_progress_frame_report(&s->ref->tf, y + ctb_size);
}
} else if (s->threads_type & FF_THREAD_FRAME && x_end)
- ff_thread_report_progress(&s->ref->tf, y + ctb_size - 4, 0);
+ ff_progress_frame_report(&s->ref->tf, y + ctb_size - 4);
}
void ff_hevc_hls_filters(HEVCLocalContext *lc, int x_ctb, int y_ctb, int ctb_size)
diff --git a/libavcodec/hevc_mvs.c b/libavcodec/hevc_mvs.c
index 0a8cc2c43d..5591919e2e 100644
--- a/libavcodec/hevc_mvs.c
+++ b/libavcodec/hevc_mvs.c
@@ -23,7 +23,7 @@
#include "hevc.h"
#include "hevcdec.h"
-#include "threadframe.h"
+#include "progressframe.h"
static const uint8_t l0_l1_cand_idx[12][2] = {
{ 0, 1, },
@@ -248,7 +248,7 @@ static int temporal_luma_motion_vector(const HEVCContext *s, int x0, int y0,
x &= ~15;
y &= ~15;
if (s->threads_type == FF_THREAD_FRAME)
- ff_thread_await_progress(&ref->tf, y, 0);
+ ff_progress_frame_await(&ref->tf, y);
x_pu = x >> s->ps.sps->log2_min_pu_size;
y_pu = y >> s->ps.sps->log2_min_pu_size;
temp_col = TAB_MVF(x_pu, y_pu);
@@ -262,7 +262,7 @@ static int temporal_luma_motion_vector(const HEVCContext *s, int x0, int y0,
x &= ~15;
y &= ~15;
if (s->threads_type == FF_THREAD_FRAME)
- ff_thread_await_progress(&ref->tf, y, 0);
+ ff_progress_frame_await(&ref->tf, y);
x_pu = x >> s->ps.sps->log2_min_pu_size;
y_pu = y >> s->ps.sps->log2_min_pu_size;
temp_col = TAB_MVF(x_pu, y_pu);
diff --git a/libavcodec/hevc_refs.c b/libavcodec/hevc_refs.c
index aed649933d..192d311696 100644
--- a/libavcodec/hevc_refs.c
+++ b/libavcodec/hevc_refs.c
@@ -26,18 +26,15 @@
#include "decode.h"
#include "hevc.h"
#include "hevcdec.h"
+#include "progressframe.h"
#include "refstruct.h"
-#include "threadframe.h"
void ff_hevc_unref_frame(HEVCFrame *frame, int flags)
{
- /* frame->frame can be NULL if context init failed */
- if (!frame->frame || !frame->frame->buf[0])
- return;
-
frame->flags &= ~flags;
if (!frame->flags) {
- ff_thread_release_ext_buffer(&frame->tf);
+ ff_progress_frame_unref(&frame->tf);
+ frame->frame = NULL;
av_frame_unref(frame->frame_grain);
frame->needs_fg = 0;
@@ -83,13 +80,14 @@ static HEVCFrame *alloc_frame(HEVCContext *s)
int i, j, ret;
for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
HEVCFrame *frame = &s->DPB[i];
- if (frame->frame->buf[0])
+ if (frame->frame)
continue;
- ret = ff_thread_get_ext_buffer(s->avctx, &frame->tf,
- AV_GET_BUFFER_FLAG_REF);
+ ret = ff_progress_frame_get_buffer(s->avctx, &frame->tf,
+ AV_GET_BUFFER_FLAG_REF);
if (ret < 0)
return NULL;
+ frame->frame = frame->tf.f;
frame->rpl = ff_refstruct_allocz(s->pkt.nb_nals * sizeof(*frame->rpl));
if (!frame->rpl)
@@ -135,7 +133,7 @@ int ff_hevc_set_new_ref(HEVCContext *s, AVFrame **frame, int poc)
for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
HEVCFrame *frame = &s->DPB[i];
- if (frame->frame->buf[0] && frame->sequence == s->seq_decode &&
+ if (frame->frame && frame->sequence == s->seq_decode &&
frame->poc == poc) {
av_log(s->avctx, AV_LOG_ERROR, "Duplicate POC in a sequence: %d.\n",
poc);
@@ -394,7 +392,7 @@ static HEVCFrame *find_ref_idx(HEVCContext *s, int poc, uint8_t use_msb)
for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
HEVCFrame *ref = &s->DPB[i];
- if (ref->frame->buf[0] && ref->sequence == s->seq_decode) {
+ if (ref->frame && ref->sequence == s->seq_decode) {
if ((ref->poc & mask) == poc && (use_msb || ref->poc != s->poc))
return ref;
}
@@ -441,7 +439,7 @@ static HEVCFrame *generate_missing_ref(HEVCContext *s, int poc)
frame->flags = 0;
if (s->threads_type == FF_THREAD_FRAME)
- ff_thread_report_progress(&frame->tf, INT_MAX, 0);
+ ff_progress_frame_report(&frame->tf, INT_MAX);
return frame;
}
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index cedd09fe14..5f0f50adf1 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -49,9 +49,9 @@
#include "hwconfig.h"
#include "internal.h"
#include "profiles.h"
+#include "progressframe.h"
#include "refstruct.h"
#include "thread.h"
-#include "threadframe.h"
static const uint8_t hevc_pel_weight[65] = { [2] = 0, [4] = 1, [6] = 2, [8] = 3, [12] = 4, [16] = 5, [24] = 6, [32] = 7, [48] = 8, [64] = 9 };
@@ -1867,7 +1867,7 @@ static void hevc_await_progress(const HEVCContext *s, const HEVCFrame *ref,
if (s->threads_type == FF_THREAD_FRAME ) {
int y = FFMAX(0, (mv->y >> 2) + y0 + height + 9);
- ff_thread_await_progress(&ref->tf, y, 0);
+ ff_progress_frame_await(&ref->tf, y);
}
}
@@ -3238,7 +3238,7 @@ static int decode_nal_units(HEVCContext *s, const uint8_t *buf, int length)
fail:
if (s->ref && s->threads_type == FF_THREAD_FRAME)
- ff_thread_report_progress(&s->ref->tf, INT_MAX, 0);
+ ff_progress_frame_report(&s->ref->tf, INT_MAX);
return ret;
}
@@ -3416,14 +3416,15 @@ static int hevc_ref_frame(HEVCFrame *dst, HEVCFrame *src)
{
int ret;
- ret = ff_thread_ref_frame(&dst->tf, &src->tf);
- if (ret < 0)
- return ret;
+ ff_progress_frame_ref(&dst->tf, &src->tf);
+ dst->frame = dst->tf.f;
if (src->needs_fg) {
ret = av_frame_ref(dst->frame_grain, src->frame_grain);
- if (ret < 0)
+ if (ret < 0) {
+ ff_hevc_unref_frame(dst, ~0);
return ret;
+ }
dst->needs_fg = 1;
}
@@ -3463,7 +3464,6 @@ static av_cold int hevc_decode_free(AVCodecContext *avctx)
for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
ff_hevc_unref_frame(&s->DPB[i], ~0);
- av_frame_free(&s->DPB[i].frame);
av_frame_free(&s->DPB[i].frame_grain);
}
@@ -3509,11 +3509,6 @@ static av_cold int hevc_init_context(AVCodecContext *avctx)
return AVERROR(ENOMEM);
for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
- s->DPB[i].frame = av_frame_alloc();
- if (!s->DPB[i].frame)
- return AVERROR(ENOMEM);
- s->DPB[i].tf.f = s->DPB[i].frame;
-
s->DPB[i].frame_grain = av_frame_alloc();
if (!s->DPB[i].frame_grain)
return AVERROR(ENOMEM);
@@ -3545,7 +3540,7 @@ static int hevc_update_thread_context(AVCodecContext *dst,
for (i = 0; i < FF_ARRAY_ELEMS(s->DPB); i++) {
ff_hevc_unref_frame(&s->DPB[i], ~0);
- if (s0->DPB[i].frame->buf[0]) {
+ if (s0->DPB[i].frame) {
ret = hevc_ref_frame(&s->DPB[i], &s0->DPB[i]);
if (ret < 0)
return ret;
@@ -3715,6 +3710,7 @@ const FFCodec ff_hevc_decoder = {
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_FRAME_THREADS,
.caps_internal = FF_CODEC_CAP_EXPORTS_CROPPING |
+ FF_CODEC_CAP_USES_PROGRESSFRAMES |
FF_CODEC_CAP_INIT_CLEANUP,
.p.profiles = NULL_IF_CONFIG_SMALL(ff_hevc_profiles),
.hw_configs = (const AVCodecHWConfigInternal *const []) {
diff --git a/libavcodec/hevcdec.h b/libavcodec/hevcdec.h
index e65a6180ca..24fcbf440a 100644
--- a/libavcodec/hevcdec.h
+++ b/libavcodec/hevcdec.h
@@ -40,7 +40,7 @@
#include "hevc_sei.h"
#include "hevcdsp.h"
#include "h274.h"
-#include "threadframe.h"
+#include "progressframe.h"
#include "videodsp.h"
#define SHIFT_CTB_WPP 2
@@ -354,7 +354,7 @@ typedef struct DBParams {
typedef struct HEVCFrame {
AVFrame *frame;
AVFrame *frame_grain;
- ThreadFrame tf;
+ ProgressFrame tf;
int needs_fg; /* 1 if grain needs to be applied by the decoder */
MvField *tab_mvf; ///< RefStruct reference
RefPicList *refPicList;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 20/27] avcodec/pngdec: Switch to ProgressFrames
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (17 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 19/27] avcodec/hevcdec: Switch to ProgressFrames Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 21/27] avcodec/ffv1dec: " Andreas Rheinhardt
` (9 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Avoids implicit av_frame_ref() and therefore allocations
and error checks. It also avoids explicitly allocating
the AVFrames (done implicitly when getting the buffer).
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/pngdec.c | 67 ++++++++++++++++++---------------------------
1 file changed, 27 insertions(+), 40 deletions(-)
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index 5a99b4a1c4..f7751223b8 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -42,8 +42,8 @@
#include "apng.h"
#include "png.h"
#include "pngdsp.h"
+#include "progressframe.h"
#include "thread.h"
-#include "threadframe.h"
#include "zlib_wrapper.h"
#include <zlib.h>
@@ -63,8 +63,8 @@ typedef struct PNGDecContext {
AVCodecContext *avctx;
GetByteContext gb;
- ThreadFrame last_picture;
- ThreadFrame picture;
+ ProgressFrame last_picture;
+ ProgressFrame picture;
AVDictionary *frame_metadata;
@@ -874,7 +874,7 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
s->bpp += byte_depth;
}
- ff_thread_release_ext_buffer(&s->picture);
+ ff_progress_frame_unref(&s->picture);
if (s->dispose_op == APNG_DISPOSE_OP_PREVIOUS) {
/* We only need a buffer for the current picture. */
ret = ff_thread_get_buffer(avctx, p, 0);
@@ -883,8 +883,8 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
} else if (s->dispose_op == APNG_DISPOSE_OP_BACKGROUND) {
/* We need a buffer for the current picture as well as
* a buffer for the reference to retain. */
- ret = ff_thread_get_ext_buffer(avctx, &s->picture,
- AV_GET_BUFFER_FLAG_REF);
+ ret = ff_progress_frame_get_buffer(avctx, &s->picture,
+ AV_GET_BUFFER_FLAG_REF);
if (ret < 0)
return ret;
ret = ff_thread_get_buffer(avctx, p, 0);
@@ -892,8 +892,9 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s,
return ret;
} else {
/* The picture output this time and the reference to retain coincide. */
- if ((ret = ff_thread_get_ext_buffer(avctx, &s->picture,
- AV_GET_BUFFER_FLAG_REF)) < 0)
+ ret = ff_progress_frame_get_buffer(avctx, &s->picture,
+ AV_GET_BUFFER_FLAG_REF);
+ if (ret < 0)
return ret;
ret = av_frame_ref(p, s->picture.f);
if (ret < 0)
@@ -1254,7 +1255,7 @@ static void handle_p_frame_png(PNGDecContext *s, AVFrame *p)
ls = FFMIN(ls, s->width * s->bpp);
- ff_thread_await_progress(&s->last_picture, INT_MAX, 0);
+ ff_progress_frame_await(&s->last_picture, INT_MAX);
for (j = 0; j < s->height; j++) {
for (i = 0; i < ls; i++)
pd[i] += pd_last[i];
@@ -1286,7 +1287,7 @@ static int handle_p_frame_apng(AVCodecContext *avctx, PNGDecContext *s,
return AVERROR_PATCHWELCOME;
}
- ff_thread_await_progress(&s->last_picture, INT_MAX, 0);
+ ff_progress_frame_await(&s->last_picture, INT_MAX);
// copy unchanged rectangles from the last frame
for (y = 0; y < s->y_offset; y++)
@@ -1674,7 +1675,7 @@ exit_loop:
}
/* handle P-frames only if a predecessor frame is available */
- if (s->last_picture.f->data[0]) {
+ if (s->last_picture.f) {
if ( !(avpkt->flags & AV_PKT_FLAG_KEY) && avctx->codec_tag != AV_RL32("MPNG")
&& s->last_picture.f->width == p->width
&& s->last_picture.f->height== p->height
@@ -1691,12 +1692,11 @@ exit_loop:
if (CONFIG_APNG_DECODER && s->dispose_op == APNG_DISPOSE_OP_BACKGROUND)
apng_reset_background(s, p);
- ff_thread_report_progress(&s->picture, INT_MAX, 0);
-
- return 0;
-
+ ret = 0;
fail:
- ff_thread_report_progress(&s->picture, INT_MAX, 0);
+ if (s->picture.f)
+ ff_progress_frame_report(&s->picture, INT_MAX);
+
return ret;
}
@@ -1783,8 +1783,8 @@ static int decode_frame_png(AVCodecContext *avctx, AVFrame *p,
goto the_end;
if (!(avctx->active_thread_type & FF_THREAD_FRAME)) {
- ff_thread_release_ext_buffer(&s->last_picture);
- FFSWAP(ThreadFrame, s->picture, s->last_picture);
+ ff_progress_frame_unref(&s->last_picture);
+ FFSWAP(ProgressFrame, s->picture, s->last_picture);
}
*got_frame = 1;
@@ -1835,12 +1835,9 @@ static int decode_frame_apng(AVCodecContext *avctx, AVFrame *p,
return ret;
if (!(avctx->active_thread_type & FF_THREAD_FRAME)) {
- if (s->dispose_op == APNG_DISPOSE_OP_PREVIOUS) {
- ff_thread_release_ext_buffer(&s->picture);
- } else {
- ff_thread_release_ext_buffer(&s->last_picture);
- FFSWAP(ThreadFrame, s->picture, s->last_picture);
- }
+ if (s->dispose_op != APNG_DISPOSE_OP_PREVIOUS)
+ FFSWAP(ProgressFrame, s->picture, s->last_picture);
+ ff_progress_frame_unref(&s->picture);
}
*got_frame = 1;
@@ -1853,8 +1850,7 @@ static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
{
PNGDecContext *psrc = src->priv_data;
PNGDecContext *pdst = dst->priv_data;
- ThreadFrame *src_frame = NULL;
- int ret;
+ const ProgressFrame *src_frame;
if (dst == src)
return 0;
@@ -1879,12 +1875,7 @@ static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
src_frame = psrc->dispose_op == APNG_DISPOSE_OP_PREVIOUS ?
&psrc->last_picture : &psrc->picture;
- ff_thread_release_ext_buffer(&pdst->last_picture);
- if (src_frame && src_frame->f->data[0]) {
- ret = ff_thread_ref_frame(&pdst->last_picture, src_frame);
- if (ret < 0)
- return ret;
- }
+ ff_progress_frame_replace(&pdst->last_picture, src_frame);
return 0;
}
@@ -1895,10 +1886,6 @@ static av_cold int png_dec_init(AVCodecContext *avctx)
PNGDecContext *s = avctx->priv_data;
s->avctx = avctx;
- s->last_picture.f = av_frame_alloc();
- s->picture.f = av_frame_alloc();
- if (!s->last_picture.f || !s->picture.f)
- return AVERROR(ENOMEM);
ff_pngdsp_init(&s->dsp);
@@ -1909,10 +1896,8 @@ static av_cold int png_dec_end(AVCodecContext *avctx)
{
PNGDecContext *s = avctx->priv_data;
- ff_thread_release_ext_buffer(&s->last_picture);
- av_frame_free(&s->last_picture.f);
- ff_thread_release_ext_buffer(&s->picture);
- av_frame_free(&s->picture.f);
+ ff_progress_frame_unref(&s->last_picture);
+ ff_progress_frame_unref(&s->picture);
av_freep(&s->buffer);
s->buffer_size = 0;
av_freep(&s->last_row);
@@ -1940,6 +1925,7 @@ const FFCodec ff_apng_decoder = {
UPDATE_THREAD_CONTEXT(update_thread_context),
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
+ FF_CODEC_CAP_USES_PROGRESSFRAMES |
FF_CODEC_CAP_ICC_PROFILES,
};
#endif
@@ -1958,6 +1944,7 @@ const FFCodec ff_png_decoder = {
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
.caps_internal = FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM |
FF_CODEC_CAP_INIT_CLEANUP |
+ FF_CODEC_CAP_USES_PROGRESSFRAMES |
FF_CODEC_CAP_ICC_PROFILES,
};
#endif
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 21/27] avcodec/ffv1dec: Switch to ProgressFrames
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (18 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 20/27] avcodec/pngdec: " Andreas Rheinhardt
@ 2024-04-08 20:13 ` Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 22/27] avcodec/qsv: Use RefStruct API for memory id (mids) array Andreas Rheinhardt
` (8 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:13 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Avoids implicit av_frame_ref() and therefore allocations
and error checks. It also avoids explicitly allocating
the AVFrames (done implicitly when getting the buffer).
It also fixes a data race: The AVFrame's sample_aspect_ratio
is currently updated after ff_thread_finish_setup()
and this write is unsynchronized with the read in av_frame_ref().
Removing the implicit av_frame_ref() fixed this.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/ffv1.h | 5 ++-
libavcodec/ffv1dec.c | 81 +++++++++++++++++++-------------------------
2 files changed, 36 insertions(+), 50 deletions(-)
diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
index 04869da5c9..acec22e83e 100644
--- a/libavcodec/ffv1.h
+++ b/libavcodec/ffv1.h
@@ -28,13 +28,12 @@
* FF Video Codec 1 (a lossless codec)
*/
-#include "libavutil/imgutils.h"
#include "avcodec.h"
#include "get_bits.h"
#include "mathops.h"
+#include "progressframe.h"
#include "put_bits.h"
#include "rangecoder.h"
-#include "threadframe.h"
#ifdef __INTEL_COMPILER
#undef av_flatten
@@ -87,7 +86,7 @@ typedef struct FFV1Context {
int flags;
int64_t picture_number;
int key_frame;
- ThreadFrame picture, last_picture;
+ ProgressFrame picture, last_picture;
struct FFV1Context *fsrc;
AVFrame *cur;
diff --git a/libavcodec/ffv1dec.c b/libavcodec/ffv1dec.c
index dd594e3f9f..7a0d1909aa 100644
--- a/libavcodec/ffv1dec.c
+++ b/libavcodec/ffv1dec.c
@@ -37,8 +37,8 @@
#include "golomb.h"
#include "mathops.h"
#include "ffv1.h"
+#include "progressframe.h"
#include "thread.h"
-#include "threadframe.h"
static inline av_flatten int get_symbol_inline(RangeCoder *c, uint8_t *state,
int is_signed)
@@ -264,8 +264,8 @@ static int decode_slice(AVCodecContext *c, void *arg)
for( si=0; fs != f->slice_context[si]; si ++)
;
- if(f->fsrc && !(p->flags & AV_FRAME_FLAG_KEY))
- ff_thread_await_progress(&f->last_picture, si, 0);
+ if (f->fsrc && !(p->flags & AV_FRAME_FLAG_KEY) && f->last_picture.f)
+ ff_progress_frame_await(&f->last_picture, si);
if(f->fsrc && !(p->flags & AV_FRAME_FLAG_KEY)) {
FFV1Context *fssrc = f->fsrc->slice_context[si];
@@ -370,7 +370,7 @@ static int decode_slice(AVCodecContext *c, void *arg)
}
}
- ff_thread_report_progress(&f->picture, si, 0);
+ ff_progress_frame_report(&f->picture, si);
return 0;
}
@@ -858,11 +858,6 @@ static av_cold int decode_init(AVCodecContext *avctx)
if ((ret = ff_ffv1_common_init(avctx)) < 0)
return ret;
- f->picture.f = av_frame_alloc();
- f->last_picture.f = av_frame_alloc();
- if (!f->picture.f || !f->last_picture.f)
- return AVERROR(ENOMEM);
-
if (avctx->extradata_size > 0 && (ret = read_extra_header(f)) < 0)
return ret;
@@ -879,31 +874,21 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
int buf_size = avpkt->size;
FFV1Context *f = avctx->priv_data;
RangeCoder *const c = &f->slice_context[0]->c;
- int i, ret;
+ int i, ret, key_frame;
uint8_t keystate = 128;
uint8_t *buf_p;
AVFrame *p;
- if (f->last_picture.f)
- ff_thread_release_ext_buffer(&f->last_picture);
- FFSWAP(ThreadFrame, f->picture, f->last_picture);
-
- f->cur = p = f->picture.f;
+ ff_progress_frame_unref(&f->last_picture);
+ FFSWAP(ProgressFrame, f->picture, f->last_picture);
- if (f->version < 3 && avctx->field_order > AV_FIELD_PROGRESSIVE) {
- /* we have interlaced material flagged in container */
- p->flags |= AV_FRAME_FLAG_INTERLACED;
- if (avctx->field_order == AV_FIELD_TT || avctx->field_order == AV_FIELD_TB)
- p->flags |= AV_FRAME_FLAG_TOP_FIELD_FIRST;
- }
f->avctx = avctx;
ff_init_range_decoder(c, buf, buf_size);
ff_build_rac_states(c, 0.05 * (1LL << 32), 256 - 8);
- p->pict_type = AV_PICTURE_TYPE_I; //FIXME I vs. P
if (get_rac(c, &keystate)) {
- p->flags |= AV_FRAME_FLAG_KEY;
+ key_frame = AV_FRAME_FLAG_KEY;
f->key_frame_ok = 0;
if ((ret = read_header(f)) < 0)
return ret;
@@ -914,7 +899,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
"Cannot decode non-keyframe without valid keyframe\n");
return AVERROR_INVALIDDATA;
}
- p->flags &= ~AV_FRAME_FLAG_KEY;
+ key_frame = 0;
}
if (f->ac != AC_GOLOMB_RICE) {
@@ -932,10 +917,23 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
return AVERROR_INVALIDDATA;
}
- ret = ff_thread_get_ext_buffer(avctx, &f->picture, AV_GET_BUFFER_FLAG_REF);
+ ret = ff_progress_frame_get_buffer(avctx, &f->picture,
+ AV_GET_BUFFER_FLAG_REF);
if (ret < 0)
return ret;
+ f->cur = p = f->picture.f;
+
+ p->pict_type = AV_PICTURE_TYPE_I; //FIXME I vs. P
+ p->flags = (p->flags & ~AV_FRAME_FLAG_KEY) | key_frame;
+
+ if (f->version < 3 && avctx->field_order > AV_FIELD_PROGRESSIVE) {
+ /* we have interlaced material flagged in container */
+ p->flags |= AV_FRAME_FLAG_INTERLACED;
+ if (avctx->field_order == AV_FIELD_TT || avctx->field_order == AV_FIELD_TB)
+ p->flags |= AV_FRAME_FLAG_TOP_FIELD_FIRST;
+ }
+
if (avctx->debug & FF_DEBUG_PICT_INFO)
av_log(avctx, AV_LOG_DEBUG, "ver:%d keyframe:%d coder:%d ec:%d slices:%d bps:%d\n",
f->version, !!(p->flags & AV_FRAME_FLAG_KEY), f->ac, f->ec, f->slice_count, f->avctx->bits_per_raw_sample);
@@ -954,7 +952,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
} else v = buf_p - c->bytestream_start;
if (buf_p - c->bytestream_start < v) {
av_log(avctx, AV_LOG_ERROR, "Slice pointer chain broken\n");
- ff_thread_report_progress(&f->picture, INT_MAX, 0);
+ ff_progress_frame_report(&f->picture, INT_MAX);
return AVERROR_INVALIDDATA;
}
buf_p -= v;
@@ -996,11 +994,11 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
for (i = f->slice_count - 1; i >= 0; i--) {
FFV1Context *fs = f->slice_context[i];
int j;
- if (fs->slice_damaged && f->last_picture.f->data[0]) {
+ if (fs->slice_damaged && f->last_picture.f) {
const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(avctx->pix_fmt);
const uint8_t *src[4];
uint8_t *dst[4];
- ff_thread_await_progress(&f->last_picture, INT_MAX, 0);
+ ff_progress_frame_await(&f->last_picture, INT_MAX);
for (j = 0; j < desc->nb_components; j++) {
int pixshift = desc->comp[j].depth > 8;
int sh = (j == 1 || j == 2) ? f->chroma_h_shift : 0;
@@ -1022,10 +1020,9 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *rframe,
fs->slice_height);
}
}
- ff_thread_report_progress(&f->picture, INT_MAX, 0);
+ ff_progress_frame_report(&f->picture, INT_MAX);
- if (f->last_picture.f)
- ff_thread_release_ext_buffer(&f->last_picture);
+ ff_progress_frame_unref(&f->last_picture);
if ((ret = av_frame_ref(rframe, f->picture.f)) < 0)
return ret;
@@ -1067,7 +1064,7 @@ static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
{
FFV1Context *fsrc = src->priv_data;
FFV1Context *fdst = dst->priv_data;
- int i, ret;
+ int i;
if (dst == src)
return 0;
@@ -1088,12 +1085,7 @@ static int update_thread_context(AVCodecContext *dst, const AVCodecContext *src)
av_assert1(fdst->max_slice_count == fsrc->max_slice_count);
-
- ff_thread_release_ext_buffer(&fdst->picture);
- if (fsrc->picture.f->data[0]) {
- if ((ret = ff_thread_ref_frame(&fdst->picture, &fsrc->picture)) < 0)
- return ret;
- }
+ ff_progress_frame_replace(&fdst->picture, &fsrc->picture);
fdst->fsrc = fsrc;
@@ -1105,15 +1097,9 @@ static av_cold int ffv1_decode_close(AVCodecContext *avctx)
{
FFV1Context *const s = avctx->priv_data;
- if (s->picture.f) {
- ff_thread_release_ext_buffer(&s->picture);
- av_frame_free(&s->picture.f);
- }
+ ff_progress_frame_unref(&s->picture);
+ ff_progress_frame_unref(&s->last_picture);
- if (s->last_picture.f) {
- ff_thread_release_ext_buffer(&s->last_picture);
- av_frame_free(&s->last_picture.f);
- }
return ff_ffv1_close(avctx);
}
@@ -1129,5 +1115,6 @@ const FFCodec ff_ffv1_decoder = {
UPDATE_THREAD_CONTEXT(update_thread_context),
.p.capabilities = AV_CODEC_CAP_DR1 |
AV_CODEC_CAP_FRAME_THREADS | AV_CODEC_CAP_SLICE_THREADS,
- .caps_internal = FF_CODEC_CAP_INIT_CLEANUP,
+ .caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
+ FF_CODEC_CAP_USES_PROGRESSFRAMES,
};
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 22/27] avcodec/qsv: Use RefStruct API for memory id (mids) array
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (19 preceding siblings ...)
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 21/27] avcodec/ffv1dec: " Andreas Rheinhardt
@ 2024-04-08 20:14 ` Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 23/27] avcodec/rkmppdec: Fix double-free on error Andreas Rheinhardt
` (7 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:14 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Avoids allocations and therefore error checks and cleanup code;
also avoids indirections.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/qsv.c | 55 +++++++++++++++------------------------
libavcodec/qsv_internal.h | 11 ++++----
libavcodec/qsvdec.c | 3 ++-
libavcodec/qsvenc.c | 3 ++-
4 files changed, 31 insertions(+), 41 deletions(-)
diff --git a/libavcodec/qsv.c b/libavcodec/qsv.c
index d9c81b7158..b07302cdf6 100644
--- a/libavcodec/qsv.c
+++ b/libavcodec/qsv.c
@@ -35,6 +35,7 @@
#include "avcodec.h"
#include "qsv_internal.h"
+#include "refstruct.h"
#define MFX_IMPL_VIA_MASK(impl) (0x0f00 & (impl))
#define QSV_HAVE_USER_PLUGIN !QSV_ONEVPL
@@ -740,20 +741,19 @@ int ff_qsv_init_internal_session(AVCodecContext *avctx, QSVSession *qs,
return 0;
}
-static void mids_buf_free(void *opaque, uint8_t *data)
+static void mids_buf_free(FFRefStructOpaque opaque, void *obj)
{
- AVBufferRef *hw_frames_ref = opaque;
+ AVBufferRef *hw_frames_ref = opaque.nc;
av_buffer_unref(&hw_frames_ref);
- av_freep(&data);
}
-static AVBufferRef *qsv_create_mids(AVBufferRef *hw_frames_ref)
+static QSVMid *qsv_create_mids(AVBufferRef *hw_frames_ref)
{
AVHWFramesContext *frames_ctx = (AVHWFramesContext*)hw_frames_ref->data;
AVQSVFramesContext *frames_hwctx = frames_ctx->hwctx;
int nb_surfaces = frames_hwctx->nb_surfaces;
- AVBufferRef *mids_buf, *hw_frames_ref1;
+ AVBufferRef *hw_frames_ref1;
QSVMid *mids;
int i;
@@ -761,35 +761,27 @@ static AVBufferRef *qsv_create_mids(AVBufferRef *hw_frames_ref)
if (!hw_frames_ref1)
return NULL;
- mids = av_calloc(nb_surfaces, sizeof(*mids));
+ mids = ff_refstruct_alloc_ext(nb_surfaces * sizeof(*mids), 0,
+ hw_frames_ref1, mids_buf_free);
if (!mids) {
av_buffer_unref(&hw_frames_ref1);
return NULL;
}
- mids_buf = av_buffer_create((uint8_t*)mids, nb_surfaces * sizeof(*mids),
- mids_buf_free, hw_frames_ref1, 0);
- if (!mids_buf) {
- av_buffer_unref(&hw_frames_ref1);
- av_freep(&mids);
- return NULL;
- }
-
for (i = 0; i < nb_surfaces; i++) {
QSVMid *mid = &mids[i];
mid->handle_pair = (mfxHDLPair*)frames_hwctx->surfaces[i].Data.MemId;
mid->hw_frames_ref = hw_frames_ref1;
}
- return mids_buf;
+ return mids;
}
static int qsv_setup_mids(mfxFrameAllocResponse *resp, AVBufferRef *hw_frames_ref,
- AVBufferRef *mids_buf)
+ QSVMid *mids)
{
AVHWFramesContext *frames_ctx = (AVHWFramesContext*)hw_frames_ref->data;
AVQSVFramesContext *frames_hwctx = frames_ctx->hwctx;
- QSVMid *mids = (QSVMid*)mids_buf->data;
int nb_surfaces = frames_hwctx->nb_surfaces;
int i;
@@ -810,12 +802,7 @@ static int qsv_setup_mids(mfxFrameAllocResponse *resp, AVBufferRef *hw_frames_re
return AVERROR(ENOMEM);
}
- resp->mids[resp->NumFrameActual + 1] = av_buffer_ref(mids_buf);
- if (!resp->mids[resp->NumFrameActual + 1]) {
- av_buffer_unref((AVBufferRef**)&resp->mids[resp->NumFrameActual]);
- av_freep(&resp->mids);
- return AVERROR(ENOMEM);
- }
+ resp->mids[resp->NumFrameActual + 1] = ff_refstruct_ref(mids);
return 0;
}
@@ -849,7 +836,7 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis, mfxFrameAllocRequest *req,
return MFX_ERR_UNSUPPORTED;
}
- ret = qsv_setup_mids(resp, ctx->hw_frames_ctx, ctx->mids_buf);
+ ret = qsv_setup_mids(resp, ctx->hw_frames_ctx, ctx->mids);
if (ret < 0) {
av_log(ctx->logctx, AV_LOG_ERROR,
"Error filling an external frame allocation request\n");
@@ -860,7 +847,8 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis, mfxFrameAllocRequest *req,
AVHWFramesContext *ext_frames_ctx = (AVHWFramesContext*)ctx->hw_frames_ctx->data;
mfxFrameInfo *i = &req->Info;
- AVBufferRef *frames_ref, *mids_buf;
+ AVBufferRef *frames_ref;
+ QSVMid *mids;
AVHWFramesContext *frames_ctx;
AVQSVFramesContext *frames_hwctx;
@@ -888,14 +876,14 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis, mfxFrameAllocRequest *req,
return MFX_ERR_MEMORY_ALLOC;
}
- mids_buf = qsv_create_mids(frames_ref);
- if (!mids_buf) {
+ mids = qsv_create_mids(frames_ref);
+ if (!mids) {
av_buffer_unref(&frames_ref);
return MFX_ERR_MEMORY_ALLOC;
}
- ret = qsv_setup_mids(resp, frames_ref, mids_buf);
- av_buffer_unref(&mids_buf);
+ ret = qsv_setup_mids(resp, frames_ref, mids);
+ ff_refstruct_unref(&mids);
av_buffer_unref(&frames_ref);
if (ret < 0) {
av_log(ctx->logctx, AV_LOG_ERROR,
@@ -912,7 +900,7 @@ static mfxStatus qsv_frame_alloc(mfxHDL pthis, mfxFrameAllocRequest *req,
static mfxStatus qsv_frame_free(mfxHDL pthis, mfxFrameAllocResponse *resp)
{
av_buffer_unref((AVBufferRef**)&resp->mids[resp->NumFrameActual]);
- av_buffer_unref((AVBufferRef**)&resp->mids[resp->NumFrameActual + 1]);
+ ff_refstruct_unref(&resp->mids[resp->NumFrameActual + 1]);
av_freep(&resp->mids);
return MFX_ERR_NONE;
}
@@ -1104,11 +1092,10 @@ int ff_qsv_init_session_frames(AVCodecContext *avctx, mfxSession *psession,
qsv_frames_ctx->logctx = avctx;
/* allocate the memory ids for the external frames */
- av_buffer_unref(&qsv_frames_ctx->mids_buf);
- qsv_frames_ctx->mids_buf = qsv_create_mids(qsv_frames_ctx->hw_frames_ctx);
- if (!qsv_frames_ctx->mids_buf)
+ ff_refstruct_unref(&qsv_frames_ctx->mids);
+ qsv_frames_ctx->mids = qsv_create_mids(qsv_frames_ctx->hw_frames_ctx);
+ if (!qsv_frames_ctx->mids)
return AVERROR(ENOMEM);
- qsv_frames_ctx->mids = (QSVMid*)qsv_frames_ctx->mids_buf->data;
qsv_frames_ctx->nb_mids = frames_hwctx->nb_surfaces;
err = MFXVideoCORE_SetFrameAllocator(session, &frame_allocator);
diff --git a/libavcodec/qsv_internal.h b/libavcodec/qsv_internal.h
index c2d301b4a2..d970cd20f0 100644
--- a/libavcodec/qsv_internal.h
+++ b/libavcodec/qsv_internal.h
@@ -115,11 +115,12 @@ typedef struct QSVFramesContext {
AVBufferRef *hw_frames_ctx;
void *logctx;
- /* The memory ids for the external frames.
- * Refcounted, since we need one reference owned by the QSVFramesContext
- * (i.e. by the encoder/decoder) and another one given to the MFX session
- * from the frame allocator. */
- AVBufferRef *mids_buf;
+ /**
+ * The memory ids for the external frames.
+ * Refcounted (via the RefStruct API), since we need one reference
+ * owned by the QSVFramesContext (i.e. by the encoder/decoder) and
+ * another one given to the MFX session from the frame allocator.
+ */
QSVMid *mids;
int nb_mids;
} QSVFramesContext;
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index fd9267c6f4..5528bcdc8c 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -50,6 +50,7 @@
#include "hwconfig.h"
#include "qsv.h"
#include "qsv_internal.h"
+#include "refstruct.h"
#if QSV_ONEVPL
#include <mfxdispatcher.h>
@@ -885,7 +886,7 @@ static void qsv_decode_close_qsvcontext(QSVContext *q)
ff_qsv_close_internal_session(&q->internal_qs);
av_buffer_unref(&q->frames_ctx.hw_frames_ctx);
- av_buffer_unref(&q->frames_ctx.mids_buf);
+ ff_refstruct_unref(&q->frames_ctx.mids);
av_buffer_pool_uninit(&q->pool);
}
diff --git a/libavcodec/qsvenc.c b/libavcodec/qsvenc.c
index 3a8607fca6..018d193495 100644
--- a/libavcodec/qsvenc.c
+++ b/libavcodec/qsvenc.c
@@ -41,6 +41,7 @@
#include "qsv.h"
#include "qsv_internal.h"
#include "qsvenc.h"
+#include "refstruct.h"
struct profile_names {
mfxU16 profile;
@@ -2649,7 +2650,7 @@ int ff_qsv_enc_close(AVCodecContext *avctx, QSVEncContext *q)
ff_qsv_close_internal_session(&q->internal_qs);
av_buffer_unref(&q->frames_ctx.hw_frames_ctx);
- av_buffer_unref(&q->frames_ctx.mids_buf);
+ ff_refstruct_unref(&q->frames_ctx.mids);
cur = q->work_frames;
while (cur) {
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 23/27] avcodec/rkmppdec: Fix double-free on error
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (20 preceding siblings ...)
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 22/27] avcodec/qsv: Use RefStruct API for memory id (mids) array Andreas Rheinhardt
@ 2024-04-08 20:14 ` Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 24/27] avcodec/rkmppdec: Check av_buffer_ref() Andreas Rheinhardt
` (6 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:14 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
After having created the AVBuffer that is put into frame->buf[0],
ownership of several objects (namely an AVDRMFrameDescriptor,
an MppFrame and some AVBufferRefs framecontextref and decoder_ref)
has passed to the AVBuffer and therefore to the frame.
Yet it has nevertheless been freed manually on error
afterwards, which would lead to a double-free as soon
as the AVFrame is unreferenced.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
The rockchip patches are still untested (except for compilation).
libavcodec/rkmppdec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/rkmppdec.c b/libavcodec/rkmppdec.c
index 7665098c6a..6889545b20 100644
--- a/libavcodec/rkmppdec.c
+++ b/libavcodec/rkmppdec.c
@@ -463,8 +463,8 @@ static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
frame->hw_frames_ctx = av_buffer_ref(decoder->frames_ref);
if (!frame->hw_frames_ctx) {
- ret = AVERROR(ENOMEM);
- goto fail;
+ av_frame_unref(frame);
+ return AVERROR(ENOMEM);
}
return 0;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 24/27] avcodec/rkmppdec: Check av_buffer_ref()
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (21 preceding siblings ...)
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 23/27] avcodec/rkmppdec: Fix double-free on error Andreas Rheinhardt
@ 2024-04-08 20:14 ` Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 25/27] avcodec/rkmppdec: Use RefStruct API for references to decoder itself Andreas Rheinhardt
` (5 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:14 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/rkmppdec.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/libavcodec/rkmppdec.c b/libavcodec/rkmppdec.c
index 6889545b20..4e14d09c7c 100644
--- a/libavcodec/rkmppdec.c
+++ b/libavcodec/rkmppdec.c
@@ -450,6 +450,10 @@ static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
// MPP decoder needs to be closed only when all frames have been released.
framecontext = (RKMPPFrameContext *)framecontextref->data;
framecontext->decoder_ref = av_buffer_ref(rk_context->decoder_ref);
+ if (!framecontext->decoder_ref) {
+ ret = AVERROR(ENOMEM);
+ goto fail;
+ }
framecontext->frame = mppframe;
frame->data[0] = (uint8_t *)desc;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 25/27] avcodec/rkmppdec: Use RefStruct API for references to decoder itself
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (22 preceding siblings ...)
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 24/27] avcodec/rkmppdec: Check av_buffer_ref() Andreas Rheinhardt
@ 2024-04-08 20:14 ` Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 26/27] avcodec/rkmppdec: Allocate AVDRMFrameDescriptor and frame ctx jointly Andreas Rheinhardt
` (4 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:14 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Avoids boilerplate code when creating the context
and avoids allocations and therefore whole error paths
when creating references to it. Also avoids an indirection
and improves type-safety.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/rkmppdec.c | 46 +++++++++++++++----------------------------
1 file changed, 16 insertions(+), 30 deletions(-)
diff --git a/libavcodec/rkmppdec.c b/libavcodec/rkmppdec.c
index 4e14d09c7c..e137e7e820 100644
--- a/libavcodec/rkmppdec.c
+++ b/libavcodec/rkmppdec.c
@@ -30,6 +30,7 @@
#include "codec_internal.h"
#include "decode.h"
#include "hwconfig.h"
+#include "refstruct.h"
#include "libavutil/buffer.h"
#include "libavutil/common.h"
#include "libavutil/frame.h"
@@ -57,12 +58,12 @@ typedef struct {
typedef struct {
AVClass *av_class;
- AVBufferRef *decoder_ref;
+ RKMPPDecoder *decoder; ///< RefStruct reference
} RKMPPDecodeContext;
typedef struct {
MppFrame frame;
- AVBufferRef *decoder_ref;
+ const RKMPPDecoder *decoder_ref; ///< RefStruct reference
} RKMPPFrameContext;
static MppCodingType rkmpp_get_codingtype(AVCodecContext *avctx)
@@ -90,7 +91,7 @@ static uint32_t rkmpp_get_frameformat(MppFrameFormat mppformat)
static int rkmpp_write_data(AVCodecContext *avctx, uint8_t *buffer, int size, int64_t pts)
{
RKMPPDecodeContext *rk_context = avctx->priv_data;
- RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
+ RKMPPDecoder *decoder = rk_context->decoder;
int ret;
MppPacket packet;
@@ -125,13 +126,13 @@ static int rkmpp_write_data(AVCodecContext *avctx, uint8_t *buffer, int size, in
static int rkmpp_close_decoder(AVCodecContext *avctx)
{
RKMPPDecodeContext *rk_context = avctx->priv_data;
- av_buffer_unref(&rk_context->decoder_ref);
+ ff_refstruct_unref(&rk_context->decoder);
return 0;
}
-static void rkmpp_release_decoder(void *opaque, uint8_t *data)
+static void rkmpp_release_decoder(FFRefStructOpaque unused, void *obj)
{
- RKMPPDecoder *decoder = (RKMPPDecoder *)data;
+ RKMPPDecoder *decoder = obj;
if (decoder->mpi) {
decoder->mpi->reset(decoder->ctx);
@@ -146,8 +147,6 @@ static void rkmpp_release_decoder(void *opaque, uint8_t *data)
av_buffer_unref(&decoder->frames_ref);
av_buffer_unref(&decoder->device_ref);
-
- av_free(decoder);
}
static int rkmpp_init_decoder(AVCodecContext *avctx)
@@ -162,19 +161,13 @@ static int rkmpp_init_decoder(AVCodecContext *avctx)
avctx->pix_fmt = AV_PIX_FMT_DRM_PRIME;
// create a decoder and a ref to it
- decoder = av_mallocz(sizeof(RKMPPDecoder));
+ decoder = ff_refstruct_alloc_ext(sizeof(*decoder), 0,
+ NULL, rkmpp_release_decoder);
if (!decoder) {
ret = AVERROR(ENOMEM);
goto fail;
}
-
- rk_context->decoder_ref = av_buffer_create((uint8_t *)decoder, sizeof(*decoder), rkmpp_release_decoder,
- NULL, AV_BUFFER_FLAG_READONLY);
- if (!rk_context->decoder_ref) {
- av_free(decoder);
- ret = AVERROR(ENOMEM);
- goto fail;
- }
+ rk_context->decoder = decoder;
av_log(avctx, AV_LOG_DEBUG, "Initializing RKMPP decoder.\n");
@@ -270,7 +263,7 @@ fail:
static int rkmpp_send_packet(AVCodecContext *avctx, const AVPacket *avpkt)
{
RKMPPDecodeContext *rk_context = avctx->priv_data;
- RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
+ RKMPPDecoder *decoder = rk_context->decoder;
int ret;
// handle EOF
@@ -312,7 +305,7 @@ static void rkmpp_release_frame(void *opaque, uint8_t *data)
RKMPPFrameContext *framecontext = (RKMPPFrameContext *)framecontextref->data;
mpp_frame_deinit(&framecontext->frame);
- av_buffer_unref(&framecontext->decoder_ref);
+ ff_refstruct_unref(&framecontext->decoder_ref);
av_buffer_unref(&framecontextref);
av_free(desc);
@@ -321,7 +314,7 @@ static void rkmpp_release_frame(void *opaque, uint8_t *data)
static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
{
RKMPPDecodeContext *rk_context = avctx->priv_data;
- RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
+ RKMPPDecoder *decoder = rk_context->decoder;
RKMPPFrameContext *framecontext = NULL;
AVBufferRef *framecontextref = NULL;
int ret;
@@ -449,11 +442,6 @@ static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
// MPP decoder needs to be closed only when all frames have been released.
framecontext = (RKMPPFrameContext *)framecontextref->data;
- framecontext->decoder_ref = av_buffer_ref(rk_context->decoder_ref);
- if (!framecontext->decoder_ref) {
- ret = AVERROR(ENOMEM);
- goto fail;
- }
framecontext->frame = mppframe;
frame->data[0] = (uint8_t *)desc;
@@ -464,6 +452,7 @@ static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
ret = AVERROR(ENOMEM);
goto fail;
}
+ framecontext->decoder_ref = ff_refstruct_ref(rk_context->decoder);
frame->hw_frames_ctx = av_buffer_ref(decoder->frames_ref);
if (!frame->hw_frames_ctx) {
@@ -488,9 +477,6 @@ fail:
if (mppframe)
mpp_frame_deinit(&mppframe);
- if (framecontext)
- av_buffer_unref(&framecontext->decoder_ref);
-
if (framecontextref)
av_buffer_unref(&framecontextref);
@@ -503,7 +489,7 @@ fail:
static int rkmpp_receive_frame(AVCodecContext *avctx, AVFrame *frame)
{
RKMPPDecodeContext *rk_context = avctx->priv_data;
- RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
+ RKMPPDecoder *decoder = rk_context->decoder;
int ret = MPP_NOK;
AVPacket pkt = {0};
RK_S32 usedslots, freeslots;
@@ -543,7 +529,7 @@ static int rkmpp_receive_frame(AVCodecContext *avctx, AVFrame *frame)
static void rkmpp_flush(AVCodecContext *avctx)
{
RKMPPDecodeContext *rk_context = avctx->priv_data;
- RKMPPDecoder *decoder = (RKMPPDecoder *)rk_context->decoder_ref->data;
+ RKMPPDecoder *decoder = rk_context->decoder;
int ret = MPP_NOK;
av_log(avctx, AV_LOG_DEBUG, "Flush.\n");
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 26/27] avcodec/rkmppdec: Allocate AVDRMFrameDescriptor and frame ctx jointly
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (23 preceding siblings ...)
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 25/27] avcodec/rkmppdec: Use RefStruct API for references to decoder itself Andreas Rheinhardt
@ 2024-04-08 20:14 ` Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 27/27] avcodec/v4l2_(m2m|buffers): Use RefStruct API for context references Andreas Rheinhardt
` (3 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:14 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Avoids an allocation and therefore one error path.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
libavcodec/rkmppdec.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/libavcodec/rkmppdec.c b/libavcodec/rkmppdec.c
index e137e7e820..47b076dbd8 100644
--- a/libavcodec/rkmppdec.c
+++ b/libavcodec/rkmppdec.c
@@ -301,12 +301,10 @@ static int rkmpp_send_packet(AVCodecContext *avctx, const AVPacket *avpkt)
static void rkmpp_release_frame(void *opaque, uint8_t *data)
{
AVDRMFrameDescriptor *desc = (AVDRMFrameDescriptor *)data;
- AVBufferRef *framecontextref = (AVBufferRef *)opaque;
- RKMPPFrameContext *framecontext = (RKMPPFrameContext *)framecontextref->data;
+ RKMPPFrameContext *framecontext = opaque;
mpp_frame_deinit(&framecontext->frame);
ff_refstruct_unref(&framecontext->decoder_ref);
- av_buffer_unref(&framecontextref);
av_free(desc);
}
@@ -315,12 +313,9 @@ static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
{
RKMPPDecodeContext *rk_context = avctx->priv_data;
RKMPPDecoder *decoder = rk_context->decoder;
- RKMPPFrameContext *framecontext = NULL;
- AVBufferRef *framecontextref = NULL;
int ret;
MppFrame mppframe = NULL;
MppBuffer buffer = NULL;
- AVDRMFrameDescriptor *desc = NULL;
AVDRMLayerDescriptor *layer = NULL;
int mode;
MppFrameFormat mppformat;
@@ -409,11 +404,21 @@ static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
// now setup the frame buffer info
buffer = mpp_frame_get_buffer(mppframe);
if (buffer) {
- desc = av_mallocz(sizeof(AVDRMFrameDescriptor));
- if (!desc) {
+ RKMPPFrameContext *framecontext;
+ AVDRMFrameDescriptor *desc;
+ // We allocate the descriptor in buf[0] jointly with a structure
+ // that will allow to hold additional information
+ // for properly releasing MPP frames and decoder.
+ struct {
+ AVDRMFrameDescriptor desc;
+ RKMPPFrameContext framecontext;
+ } *combined_desc = av_mallocz(sizeof(*combined_desc));
+ if (!combined_desc) {
ret = AVERROR(ENOMEM);
goto fail;
}
+ desc = &combined_desc->desc;
+ framecontext = &combined_desc->framecontext;
desc->nb_objects = 1;
desc->objects[0].fd = mpp_buffer_get_fd(buffer);
@@ -432,23 +437,15 @@ static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
layer->planes[1].offset = layer->planes[0].pitch * mpp_frame_get_ver_stride(mppframe);
layer->planes[1].pitch = layer->planes[0].pitch;
- // we also allocate a struct in buf[0] that will allow to hold additionnal information
- // for releasing properly MPP frames and decoder
- framecontextref = av_buffer_allocz(sizeof(*framecontext));
- if (!framecontextref) {
- ret = AVERROR(ENOMEM);
- goto fail;
- }
-
// MPP decoder needs to be closed only when all frames have been released.
- framecontext = (RKMPPFrameContext *)framecontextref->data;
framecontext->frame = mppframe;
frame->data[0] = (uint8_t *)desc;
frame->buf[0] = av_buffer_create((uint8_t *)desc, sizeof(*desc), rkmpp_release_frame,
- framecontextref, AV_BUFFER_FLAG_READONLY);
+ framecontext, AV_BUFFER_FLAG_READONLY);
if (!frame->buf[0]) {
+ av_free(combined_desc);
ret = AVERROR(ENOMEM);
goto fail;
}
@@ -477,12 +474,6 @@ fail:
if (mppframe)
mpp_frame_deinit(&mppframe);
- if (framecontextref)
- av_buffer_unref(&framecontextref);
-
- if (desc)
- av_free(desc);
-
return ret;
}
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v2 27/27] avcodec/v4l2_(m2m|buffers): Use RefStruct API for context references
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (24 preceding siblings ...)
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 26/27] avcodec/rkmppdec: Allocate AVDRMFrameDescriptor and frame ctx jointly Andreas Rheinhardt
@ 2024-04-08 20:14 ` Andreas Rheinhardt
2024-04-09 6:33 ` [FFmpeg-devel] [PATCH v3 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (2 subsequent siblings)
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-08 20:14 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
Avoids allocations and therefore error checks; also avoids
indirections and allows to remove the boilerplate code
for creating an object with a dedicated free function.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
This patch is also still untested.
libavcodec/v4l2_buffers.c | 7 +++----
libavcodec/v4l2_buffers.h | 6 +++---
libavcodec/v4l2_m2m.c | 25 +++++++++----------------
libavcodec/v4l2_m2m.h | 5 ++---
4 files changed, 17 insertions(+), 26 deletions(-)
diff --git a/libavcodec/v4l2_buffers.c b/libavcodec/v4l2_buffers.c
index 2277135699..23474ee143 100644
--- a/libavcodec/v4l2_buffers.c
+++ b/libavcodec/v4l2_buffers.c
@@ -29,6 +29,7 @@
#include <poll.h>
#include "libavcodec/avcodec.h"
#include "libavutil/pixdesc.h"
+#include "refstruct.h"
#include "v4l2_context.h"
#include "v4l2_buffers.h"
#include "v4l2_m2m.h"
@@ -229,7 +230,7 @@ static void v4l2_free_buffer(void *opaque, uint8_t *unused)
ff_v4l2_buffer_enqueue(avbuf);
}
- av_buffer_unref(&avbuf->context_ref);
+ ff_refstruct_unref(&avbuf->context_ref);
}
}
@@ -240,9 +241,7 @@ static int v4l2_buf_increase_ref(V4L2Buffer *in)
if (in->context_ref)
atomic_fetch_add(&in->context_refcount, 1);
else {
- in->context_ref = av_buffer_ref(s->self_ref);
- if (!in->context_ref)
- return AVERROR(ENOMEM);
+ in->context_ref = ff_refstruct_ref(s->self_ref);
in->context_refcount = 1;
}
diff --git a/libavcodec/v4l2_buffers.h b/libavcodec/v4l2_buffers.h
index 3d2ff1b9a5..e35b161309 100644
--- a/libavcodec/v4l2_buffers.h
+++ b/libavcodec/v4l2_buffers.h
@@ -28,7 +28,6 @@
#include <stddef.h>
#include <linux/videodev2.h>
-#include "libavutil/buffer.h"
#include "libavutil/frame.h"
#include "packet.h"
@@ -46,8 +45,9 @@ typedef struct V4L2Buffer {
struct V4L2Context *context;
/* This object is refcounted per-plane, so we need to keep track
- * of how many context-refs we are holding. */
- AVBufferRef *context_ref;
+ * of how many context-refs we are holding.
+ * This pointer is a RefStruct reference. */
+ const struct V4L2m2mContext *context_ref;
atomic_uint context_refcount;
/* keep track of the mmap address and mmap length */
diff --git a/libavcodec/v4l2_m2m.c b/libavcodec/v4l2_m2m.c
index ac086a7913..15415cfc4e 100644
--- a/libavcodec/v4l2_m2m.c
+++ b/libavcodec/v4l2_m2m.c
@@ -32,6 +32,7 @@
#include "libavutil/pixdesc.h"
#include "libavutil/imgutils.h"
#include "libavutil/pixfmt.h"
+#include "refstruct.h"
#include "v4l2_context.h"
#include "v4l2_fmt.h"
#include "v4l2_m2m.h"
@@ -247,9 +248,9 @@ int ff_v4l2_m2m_codec_reinit(V4L2m2mContext *s)
return 0;
}
-static void v4l2_m2m_destroy_context(void *opaque, uint8_t *context)
+static void v4l2_m2m_destroy_context(FFRefStructOpaque unused, void *context)
{
- V4L2m2mContext *s = (V4L2m2mContext*)context;
+ V4L2m2mContext *s = context;
ff_v4l2_context_release(&s->capture);
sem_destroy(&s->refsync);
@@ -258,8 +259,6 @@ static void v4l2_m2m_destroy_context(void *opaque, uint8_t *context)
close(s->fd);
av_frame_free(&s->frame);
av_packet_unref(&s->buf_pkt);
-
- av_free(s);
}
int ff_v4l2_m2m_codec_end(V4L2m2mPriv *priv)
@@ -283,7 +282,7 @@ int ff_v4l2_m2m_codec_end(V4L2m2mPriv *priv)
ff_v4l2_context_release(&s->output);
s->self_ref = NULL;
- av_buffer_unref(&priv->context_ref);
+ ff_refstruct_unref(&priv->context);
return 0;
}
@@ -328,17 +327,11 @@ int ff_v4l2_m2m_codec_init(V4L2m2mPriv *priv)
int ff_v4l2_m2m_create_context(V4L2m2mPriv *priv, V4L2m2mContext **s)
{
- *s = av_mallocz(sizeof(V4L2m2mContext));
+ *s = ff_refstruct_alloc_ext(sizeof(**s), 0, NULL,
+ &v4l2_m2m_destroy_context);
if (!*s)
return AVERROR(ENOMEM);
- priv->context_ref = av_buffer_create((uint8_t *) *s, sizeof(V4L2m2mContext),
- &v4l2_m2m_destroy_context, NULL, 0);
- if (!priv->context_ref) {
- av_freep(s);
- return AVERROR(ENOMEM);
- }
-
/* assign the context */
priv->context = *s;
(*s)->priv = priv;
@@ -346,13 +339,13 @@ int ff_v4l2_m2m_create_context(V4L2m2mPriv *priv, V4L2m2mContext **s)
/* populate it */
priv->context->capture.num_buffers = priv->num_capture_buffers;
priv->context->output.num_buffers = priv->num_output_buffers;
- priv->context->self_ref = priv->context_ref;
+ priv->context->self_ref = priv->context;
priv->context->fd = -1;
priv->context->frame = av_frame_alloc();
if (!priv->context->frame) {
- av_buffer_unref(&priv->context_ref);
- *s = NULL; /* freed when unreferencing context_ref */
+ ff_refstruct_unref(&priv->context);
+ *s = NULL; /* freed when unreferencing context */
return AVERROR(ENOMEM);
}
diff --git a/libavcodec/v4l2_m2m.h b/libavcodec/v4l2_m2m.h
index 04d86d7b92..4ba33dc335 100644
--- a/libavcodec/v4l2_m2m.h
+++ b/libavcodec/v4l2_m2m.h
@@ -62,7 +62,7 @@ typedef struct V4L2m2mContext {
AVFrame *frame;
/* Reference to self; only valid while codec is active. */
- AVBufferRef *self_ref;
+ struct V4L2m2mContext *self_ref;
/* reference back to V4L2m2mPriv */
void *priv;
@@ -71,8 +71,7 @@ typedef struct V4L2m2mContext {
typedef struct V4L2m2mPriv {
AVClass *class;
- V4L2m2mContext *context;
- AVBufferRef *context_ref;
+ V4L2m2mContext *context; ///< RefStruct reference
int num_output_buffers;
int num_capture_buffers;
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API Andreas Rheinhardt
@ 2024-04-09 1:40 ` Michael Niedermayer
2024-04-09 6:35 ` Andreas Rheinhardt
2024-04-10 7:01 ` Anton Khirnov
1 sibling, 1 reply; 40+ messages in thread
From: Michael Niedermayer @ 2024-04-09 1:40 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 8128 bytes --]
On Mon, Apr 08, 2024 at 10:13:40PM +0200, Andreas Rheinhardt wrote:
> Frame-threaded decoders with inter-frame dependencies
> use the ThreadFrame API for syncing. It works as follows:
>
> During init each thread allocates an AVFrame for every
> ThreadFrame.
>
> Thread A reads the header of its packet and allocates
> a buffer for an AVFrame with ff_thread_get_ext_buffer()
> (which also allocates a small structure that is shared
> with other references to this frame) and sets its fields,
> including side data. Then said thread calls ff_thread_finish_setup().
> From that moment onward it is not allowed to change any
> of the AVFrame fields at all any more, but it may change
> fields which are an indirection away, like the content of
> AVFrame.data or already existing side data.
>
> After thread A has called ff_thread_finish_setup(),
> another thread (the user one) calls the codec's update_thread_context
> callback which in turn calls ff_thread_ref_frame() which
> calls av_frame_ref() which reads every field of A's
> AVFrame; hence the above restriction on modifications
> of the AVFrame (as any modification of the AVFrame by A after
> ff_thread_finish_setup() would be a data race). Of course,
> this av_frame_ref() also incurs allocations and therefore
> needs to be checked. ff_thread_ref_frame() also references
> the small structure used for communicating progress.
>
> This av_frame_ref() makes it awkward to propagate values that
> only become known during decoding to later threads (in case of
> frame reordering or other mechanisms of delayed output (like
> show-existing-frames) it's not the decoding thread, but a later
> thread that returns the AVFrame). E.g. for VP9 when exporting video
> encoding parameters as side data the number of blocks only
> becomes known during decoding, so one can't allocate the side data
> before ff_thread_finish_setup(). It is currently being done afterwards
> and this leads to a data race in the vp9-encparams test when using
> frame-threading. Returning decode_error_flags is also complicated
> by this.
>
> To perform this exchange a buffer shared between the references
> is needed (notice that simply giving the later threads a pointer
> to the original AVFrame does not work, because said AVFrame will
> be reused lateron when thread A decodes the next packet given to it).
> One could extend the buffer already used for progress for this
> or use a new one (requiring yet another allocation), yet both
> of these approaches have the drawback of being unnatural, ugly
> and requiring quite a lot of ad-hoc code. E.g. in case of the VP9
> side data mentioned above one could not simply use the helper
> that allocates and adds the side data to an AVFrame in one go.
>
> The ProgressFrame API meanwhile offers a different solution to all
> of this. It is based around the idea that the most natural
> shared object for sharing information about an AVFrame between
> decoding threads is the AVFrame itself. To actually implement this
> the AVFrame needs to be reference counted. This is achieved by
> putting a (ownership) pointer into a shared (and opaque) structure
> that is managed by the RefStruct API and which also contains
> the stuff necessary for progress reporting.
> The users get a pointer to this AVFrame with the understanding
> that the owner may set all the fields until it has indicated
> that it has finished decoding this AVFrame; then the users are
> allowed to read everything. Every decoder may of course employ
> a different contract than the one outlined above.
>
> Given that there is no underlying av_frame_ref(), creating
> references to a ProgressFrame can't fail. Only
> ff_thread_progress_get_buffer() can fail, but given that
> it will replace calls to ff_thread_get_ext_buffer() it is
> at places where errors are already expected and properly
> taken care of.
>
> The ProgressFrames are empty (i.e. the AVFrame pointer is NULL
> and the AVFrames are not allocated during init at all)
> while not being in use; ff_thread_progress_get_buffer() both
> sets up the actual ProgressFrame and already calls
> ff_thread_get_buffer(). So instead of checking for
> ThreadFrame.f->data[0] or ThreadFrame.f->buf[0] being NULL
> for "this reference frame is non-existing" one should check for
> ProgressFrame.f.
>
> This also implies that one can only set AVFrame properties
> after having allocated the buffer. This restriction is not deep:
> if it becomes onerous for any codec, ff_thread_progress_get_buffer()
> can be broken up. The user would then have to get a buffer
> himself.
>
> In order to avoid unnecessary allocations, the shared structure
> is pooled, so that both the structure as well as the AVFrame
> itself are reused. This means that there won't be lots of
> unnecessary allocations in case of non-frame-threaded decoding.
> It might even turn out to have fewer than the current code
> (the current code allocates AVFrames for every DPB slot, but
> these are often excessively large and not completely used;
> the new code allocates them on demand). Pooling relies on the
> reset function of the RefStruct pool API, it would be impossible
> to implement with the AVBufferPool API.
>
> Finally, ProgressFrames have no notion of owner; they are built
> on top of the ThreadProgress API which also lacks such a concept.
> Instead every ThreadProgress and every ProgressFrame contains
> its own mutex and condition variable, making it completely independent
> of pthread_frame.c. Just like the ThreadFrame API it is simply
> presumed that only the actual owner/producer of a frame reports
> progress on said frame.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> libavcodec/Makefile | 1 +
> libavcodec/avcodec.c | 1 +
> libavcodec/codec_internal.h | 4 ++
> libavcodec/decode.c | 122 +++++++++++++++++++++++++++++++++
> libavcodec/internal.h | 2 +
> libavcodec/progressframe.h | 133 ++++++++++++++++++++++++++++++++++++
> libavcodec/pthread_frame.c | 1 +
> libavcodec/tests/avcodec.c | 3 +-
> 8 files changed, 266 insertions(+), 1 deletion(-)
> create mode 100644 libavcodec/progressframe.h
breaks build without threads
./configure --disable-pthreads --cc='ccache gcc' && make -j32
...
libavcodec/threadprogress.c: In function ‘ff_thread_progress_report’:
libavcodec/threadprogress.c:54:5: error: implicit declaration of function ‘pthread_mutex_lock’; did you mean ‘ff_mutex_lock’? [-Werror=implicit-function-declaration]
pthread_mutex_lock(&pro->progress_mutex);
^~~~~~~~~~~~~~~~~~
ff_mutex_lock
libavcodec/threadprogress.c:55:5: error: implicit declaration of function ‘pthread_cond_broadcast’; did you mean ‘ff_cond_broadcast’? [-Werror=implicit-function-declaration]
pthread_cond_broadcast(&pro->progress_cond);
^~~~~~~~~~~~~~~~~~~~~~
ff_cond_broadcast
libavcodec/threadprogress.c:56:5: error: implicit declaration of function ‘pthread_mutex_unlock’; did you mean ‘ff_mutex_unlock’? [-Werror=implicit-function-declaration]
pthread_mutex_unlock(&pro->progress_mutex);
^~~~~~~~~~~~~~~~~~~~
ff_mutex_unlock
libavcodec/threadprogress.c: In function ‘ff_thread_progress_await’:
libavcodec/threadprogress.c:71:9: error: implicit declaration of function ‘pthread_cond_wait’; did you mean ‘ff_cond_wait’? [-Werror=implicit-function-declaration]
pthread_cond_wait(&pro->progress_cond, &pro->progress_mutex);
^~~~~~~~~~~~~~~~~
ff_cond_wait
cc1: some warnings being treated as errors
ffbuild/common.mak:81: recipe for target 'libavcodec/threadprogress.o' failed
make: *** [libavcodec/threadprogress.o] Error 1
make: *** Waiting for unfinished jobs....
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The smallest minority on earth is the individual. Those who deny
individual rights cannot claim to be defenders of minorities. - Ayn Rand
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* [FFmpeg-devel] [PATCH v3 01/27] avcodec/threadprogress: Add new API for frame-threaded progress
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (25 preceding siblings ...)
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 27/27] avcodec/v4l2_(m2m|buffers): Use RefStruct API for context references Andreas Rheinhardt
@ 2024-04-09 6:33 ` Andreas Rheinhardt
2024-04-10 6:38 ` [FFmpeg-devel] [PATCH v2 " Anton Khirnov
2024-04-18 20:40 ` Andreas Rheinhardt
28 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-09 6:33 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: Andreas Rheinhardt
The API is similar to the ThreadFrame API, with the exception
that it no longer has an included AVFrame and that it has its
own mutexes and condition variables which makes it more independent
of pthread_frame.c. 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. This will typically be solved
by putting a ThreadProgress in a refcounted structure that is
shared between threads.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Now using ff_mutex_*/ff_cond_* instead of their pthread counterparts.
libavcodec/threadprogress.c | 79 ++++++++++++++++++++++++++++++++
libavcodec/threadprogress.h | 91 +++++++++++++++++++++++++++++++++++++
2 files changed, 170 insertions(+)
create mode 100644 libavcodec/threadprogress.c
create mode 100644 libavcodec/threadprogress.h
diff --git a/libavcodec/threadprogress.c b/libavcodec/threadprogress.c
new file mode 100644
index 0000000000..62c4fd898b
--- /dev/null
+++ b/libavcodec/threadprogress.c
@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
+ *
+ * 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
+ */
+
+#include <limits.h>
+#include <stdatomic.h>
+
+#include "pthread_internal.h"
+#include "threadprogress.h"
+#include "libavutil/attributes.h"
+#include "libavutil/thread.h"
+
+DEFINE_OFFSET_ARRAY(ThreadProgress, thread_progress, init,
+ (offsetof(ThreadProgress, progress_mutex)),
+ (offsetof(ThreadProgress, progress_cond)));
+
+av_cold int ff_thread_progress_init(ThreadProgress *pro, int init_mode)
+{
+ atomic_init(&pro->progress, init_mode ? -1 : INT_MAX);
+#if HAVE_THREADS
+ if (init_mode)
+ return ff_pthread_init(pro, thread_progress_offsets);
+#endif
+ pro->init = init_mode;
+ return 0;
+}
+
+av_cold void ff_thread_progress_destroy(ThreadProgress *pro)
+{
+#if HAVE_THREADS
+ ff_pthread_free(pro, thread_progress_offsets);
+#else
+ pro->init = 0;
+#endif
+}
+
+void ff_thread_progress_report(ThreadProgress *pro, int n)
+{
+ if (atomic_load_explicit(&pro->progress, memory_order_relaxed) >= n)
+ return;
+
+ atomic_store_explicit(&pro->progress, n, memory_order_release);
+
+ ff_mutex_lock(&pro->progress_mutex);
+ ff_cond_broadcast(&pro->progress_cond);
+ ff_mutex_unlock(&pro->progress_mutex);
+}
+
+void ff_thread_progress_await(const ThreadProgress *pro_c, int n)
+{
+ /* Casting const away here is safe, because we only read from progress
+ * and will leave pro_c in the same state upon leaving the function
+ * as it had at the beginning. */
+ ThreadProgress *pro = (ThreadProgress*)pro_c;
+
+ if (atomic_load_explicit(&pro->progress, memory_order_acquire) >= n)
+ return;
+
+ ff_mutex_lock(&pro->progress_mutex);
+ while (atomic_load_explicit(&pro->progress, memory_order_relaxed) < n)
+ ff_cond_wait(&pro->progress_cond, &pro->progress_mutex);
+ ff_mutex_unlock(&pro->progress_mutex);
+}
diff --git a/libavcodec/threadprogress.h b/libavcodec/threadprogress.h
new file mode 100644
index 0000000000..786802cbf1
--- /dev/null
+++ b/libavcodec/threadprogress.h
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
+ *
+ * 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
+
+/**
+ * ThreadProgress is an API to easily notify other threads about progress
+ * of any kind as long as it can be packaged into an int and is consistent
+ * with the natural ordering of integers.
+ *
+ * Each initialized ThreadProgress can be in one of two modes: No-op mode
+ * or ordinary mode. In the former mode, ff_thread_report_progress() and
+ * ff_thread_await_progress() are no-ops to simply support usecases like
+ * non-frame-threading. Only in the latter case perform these functions
+ * what their name already implies.
+ */
+
+#include <limits.h>
+#include <stdatomic.h>
+#include "libavutil/thread.h"
+
+/**
+ * This struct should be treated as opaque by users.
+ */
+typedef struct ThreadProgress {
+ atomic_int progress;
+ unsigned init;
+ AVMutex progress_mutex;
+ AVCond progress_cond;
+} ThreadProgress;
+
+/**
+ * Initialize a ThreadProgress.
+ *
+ * @param init_mode If zero, the ThreadProgress will be initialized so that
+ * to be in no-op mode as described above. Otherwise
+ */
+int ff_thread_progress_init(ThreadProgress *pro, int init_mode);
+
+/**
+ * Destroy a ThreadProgress. Can be called on a ThreadProgress that
+ * has never been initialized provided that the ThreadProgress struct
+ * has been initially zeroed. Must be called even if ff_thread_progress_init()
+ * failed.
+ */
+void ff_thread_progress_destroy(ThreadProgress *pro);
+
+/**
+ * Reset the ThreadProgress's progress progress counter; must only be called
+ * if it is not in use in any way (e.g. no thread may wait on it via
+ * ff_thread_progress_await()).
+ */
+static inline void ff_thread_progress_reset(ThreadProgress *pro)
+{
+ atomic_init(&pro->progress, pro->init ? -1 : INT_MAX);
+}
+
+/**
+ * This function is a no-op in no-op mode; otherwise it notifies
+ * other threads that a certain level of progress has been reached.
+ * Later calls with lower values of progress have no effect.
+ */
+void ff_thread_progress_report(ThreadProgress *pro, int progress);
+
+/**
+ * This function is a no-op in no-op mode; otherwise it waits
+ * until other threads have reached a certain level of progress:
+ * This function will return after another thread has called
+ * ff_thread_progress_report() with the same or higher value for progress.
+ */
+void ff_thread_progress_await(const ThreadProgress *pro, int progress);
+
+#endif /* AVCODEC_THREADPROGRESS_H */
--
2.40.1
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-09 1:40 ` Michael Niedermayer
@ 2024-04-09 6:35 ` Andreas Rheinhardt
0 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-09 6:35 UTC (permalink / raw)
To: ffmpeg-devel
Michael Niedermayer:
> On Mon, Apr 08, 2024 at 10:13:40PM +0200, Andreas Rheinhardt wrote:
>> Frame-threaded decoders with inter-frame dependencies
>> use the ThreadFrame API for syncing. It works as follows:
>>
>> During init each thread allocates an AVFrame for every
>> ThreadFrame.
>>
>> Thread A reads the header of its packet and allocates
>> a buffer for an AVFrame with ff_thread_get_ext_buffer()
>> (which also allocates a small structure that is shared
>> with other references to this frame) and sets its fields,
>> including side data. Then said thread calls ff_thread_finish_setup().
>> From that moment onward it is not allowed to change any
>> of the AVFrame fields at all any more, but it may change
>> fields which are an indirection away, like the content of
>> AVFrame.data or already existing side data.
>>
>> After thread A has called ff_thread_finish_setup(),
>> another thread (the user one) calls the codec's update_thread_context
>> callback which in turn calls ff_thread_ref_frame() which
>> calls av_frame_ref() which reads every field of A's
>> AVFrame; hence the above restriction on modifications
>> of the AVFrame (as any modification of the AVFrame by A after
>> ff_thread_finish_setup() would be a data race). Of course,
>> this av_frame_ref() also incurs allocations and therefore
>> needs to be checked. ff_thread_ref_frame() also references
>> the small structure used for communicating progress.
>>
>> This av_frame_ref() makes it awkward to propagate values that
>> only become known during decoding to later threads (in case of
>> frame reordering or other mechanisms of delayed output (like
>> show-existing-frames) it's not the decoding thread, but a later
>> thread that returns the AVFrame). E.g. for VP9 when exporting video
>> encoding parameters as side data the number of blocks only
>> becomes known during decoding, so one can't allocate the side data
>> before ff_thread_finish_setup(). It is currently being done afterwards
>> and this leads to a data race in the vp9-encparams test when using
>> frame-threading. Returning decode_error_flags is also complicated
>> by this.
>>
>> To perform this exchange a buffer shared between the references
>> is needed (notice that simply giving the later threads a pointer
>> to the original AVFrame does not work, because said AVFrame will
>> be reused lateron when thread A decodes the next packet given to it).
>> One could extend the buffer already used for progress for this
>> or use a new one (requiring yet another allocation), yet both
>> of these approaches have the drawback of being unnatural, ugly
>> and requiring quite a lot of ad-hoc code. E.g. in case of the VP9
>> side data mentioned above one could not simply use the helper
>> that allocates and adds the side data to an AVFrame in one go.
>>
>> The ProgressFrame API meanwhile offers a different solution to all
>> of this. It is based around the idea that the most natural
>> shared object for sharing information about an AVFrame between
>> decoding threads is the AVFrame itself. To actually implement this
>> the AVFrame needs to be reference counted. This is achieved by
>> putting a (ownership) pointer into a shared (and opaque) structure
>> that is managed by the RefStruct API and which also contains
>> the stuff necessary for progress reporting.
>> The users get a pointer to this AVFrame with the understanding
>> that the owner may set all the fields until it has indicated
>> that it has finished decoding this AVFrame; then the users are
>> allowed to read everything. Every decoder may of course employ
>> a different contract than the one outlined above.
>>
>> Given that there is no underlying av_frame_ref(), creating
>> references to a ProgressFrame can't fail. Only
>> ff_thread_progress_get_buffer() can fail, but given that
>> it will replace calls to ff_thread_get_ext_buffer() it is
>> at places where errors are already expected and properly
>> taken care of.
>>
>> The ProgressFrames are empty (i.e. the AVFrame pointer is NULL
>> and the AVFrames are not allocated during init at all)
>> while not being in use; ff_thread_progress_get_buffer() both
>> sets up the actual ProgressFrame and already calls
>> ff_thread_get_buffer(). So instead of checking for
>> ThreadFrame.f->data[0] or ThreadFrame.f->buf[0] being NULL
>> for "this reference frame is non-existing" one should check for
>> ProgressFrame.f.
>>
>> This also implies that one can only set AVFrame properties
>> after having allocated the buffer. This restriction is not deep:
>> if it becomes onerous for any codec, ff_thread_progress_get_buffer()
>> can be broken up. The user would then have to get a buffer
>> himself.
>>
>> In order to avoid unnecessary allocations, the shared structure
>> is pooled, so that both the structure as well as the AVFrame
>> itself are reused. This means that there won't be lots of
>> unnecessary allocations in case of non-frame-threaded decoding.
>> It might even turn out to have fewer than the current code
>> (the current code allocates AVFrames for every DPB slot, but
>> these are often excessively large and not completely used;
>> the new code allocates them on demand). Pooling relies on the
>> reset function of the RefStruct pool API, it would be impossible
>> to implement with the AVBufferPool API.
>>
>> Finally, ProgressFrames have no notion of owner; they are built
>> on top of the ThreadProgress API which also lacks such a concept.
>> Instead every ThreadProgress and every ProgressFrame contains
>> its own mutex and condition variable, making it completely independent
>> of pthread_frame.c. Just like the ThreadFrame API it is simply
>> presumed that only the actual owner/producer of a frame reports
>> progress on said frame.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> libavcodec/Makefile | 1 +
>> libavcodec/avcodec.c | 1 +
>> libavcodec/codec_internal.h | 4 ++
>> libavcodec/decode.c | 122 +++++++++++++++++++++++++++++++++
>> libavcodec/internal.h | 2 +
>> libavcodec/progressframe.h | 133 ++++++++++++++++++++++++++++++++++++
>> libavcodec/pthread_frame.c | 1 +
>> libavcodec/tests/avcodec.c | 3 +-
>> 8 files changed, 266 insertions(+), 1 deletion(-)
>> create mode 100644 libavcodec/progressframe.h
>
> breaks build without threads
>
> ./configure --disable-pthreads --cc='ccache gcc' && make -j32
> ...
> libavcodec/threadprogress.c: In function ‘ff_thread_progress_report’:
> libavcodec/threadprogress.c:54:5: error: implicit declaration of function ‘pthread_mutex_lock’; did you mean ‘ff_mutex_lock’? [-Werror=implicit-function-declaration]
> pthread_mutex_lock(&pro->progress_mutex);
> ^~~~~~~~~~~~~~~~~~
> ff_mutex_lock
> libavcodec/threadprogress.c:55:5: error: implicit declaration of function ‘pthread_cond_broadcast’; did you mean ‘ff_cond_broadcast’? [-Werror=implicit-function-declaration]
> pthread_cond_broadcast(&pro->progress_cond);
> ^~~~~~~~~~~~~~~~~~~~~~
> ff_cond_broadcast
> libavcodec/threadprogress.c:56:5: error: implicit declaration of function ‘pthread_mutex_unlock’; did you mean ‘ff_mutex_unlock’? [-Werror=implicit-function-declaration]
> pthread_mutex_unlock(&pro->progress_mutex);
> ^~~~~~~~~~~~~~~~~~~~
> ff_mutex_unlock
> libavcodec/threadprogress.c: In function ‘ff_thread_progress_await’:
> libavcodec/threadprogress.c:71:9: error: implicit declaration of function ‘pthread_cond_wait’; did you mean ‘ff_cond_wait’? [-Werror=implicit-function-declaration]
> pthread_cond_wait(&pro->progress_cond, &pro->progress_mutex);
> ^~~~~~~~~~~~~~~~~
> ff_cond_wait
> cc1: some warnings being treated as errors
> ffbuild/common.mak:81: recipe for target 'libavcodec/threadprogress.o' failed
> make: *** [libavcodec/threadprogress.o] Error 1
> make: *** Waiting for unfinished jobs....
>
Thanks for testing; fixed by v3 of patch #1.
- 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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (26 preceding siblings ...)
2024-04-09 6:33 ` [FFmpeg-devel] [PATCH v3 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
@ 2024-04-10 6:38 ` Anton Khirnov
2024-04-18 20:40 ` Andreas Rheinhardt
28 siblings, 0 replies; 40+ messages in thread
From: Anton Khirnov @ 2024-04-10 6:38 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Andreas Rheinhardt
Quoting Andreas Rheinhardt (2024-04-08 21:51:59)
> +/**
> + * ThreadProgress is an API to easily notify other threads about progress
> + * of any kind as long as it can be packaged into an int and is consistent
> + * with the natural ordering of integers.
> + *
> + * Each initialized ThreadProgress can be in one of two modes: No-op mode
> + * or ordinary mode. In the former mode, ff_thread_report_progress() and
> + * ff_thread_await_progress() are no-ops to simply support usecases like
> + * non-frame-threading. Only in the latter case perform these functions
Maybe 'strictly serial usecases' or 'non-parallel usecases' would sound
better.
> + * what their name already implies.
> + */
> +
> +#include <limits.h>
> +#include <stdatomic.h>
> +#include "libavutil/thread.h"
> +
> +/**
> + * This struct should be treated as opaque by users.
> + */
> +typedef struct ThreadProgress {
> + atomic_int progress;
> + unsigned init;
> + AVMutex progress_mutex;
> + AVCond progress_cond;
> +} ThreadProgress;
> +
> +/**
> + * Initialize a ThreadProgress.
> + *
> + * @param init_mode If zero, the ThreadProgress will be initialized so that
> + * to be in no-op mode as described above. Otherwise
Seems like something missing here.
LGTM otherwise.
--
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API Andreas Rheinhardt
2024-04-09 1:40 ` Michael Niedermayer
@ 2024-04-10 7:01 ` Anton Khirnov
2024-04-10 7:09 ` Andreas Rheinhardt
1 sibling, 1 reply; 40+ messages in thread
From: Anton Khirnov @ 2024-04-10 7:01 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Andreas Rheinhardt
Quoting Andreas Rheinhardt (2024-04-08 22:13:40)
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index fd356bd190..6b2c4312e0 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -779,6 +779,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
> if (!copy->internal)
> return AVERROR(ENOMEM);
> copy->internal->thread_ctx = p;
> + copy->internal->progress_frame_pool = avctx->internal->progress_frame_pool;
I'd still prefer every thread to have its own reference.
Looks good otherwise.
--
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 06/27] avcodec/vp9: Fix race when attaching side-data for show-existing frame
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 06/27] avcodec/vp9: Fix race when attaching side-data for show-existing frame Andreas Rheinhardt
@ 2024-04-10 7:06 ` Anton Khirnov
0 siblings, 0 replies; 40+ messages in thread
From: Anton Khirnov @ 2024-04-10 7:06 UTC (permalink / raw)
To: FFmpeg development discussions and patches; +Cc: Andreas Rheinhardt
Quoting Andreas Rheinhardt (2024-04-08 22:13:44)
> When outputting a show-existing frame, the VP9 decoder simply
> created a reference to said frame and returned it immediately to
> the caller, without waiting for it to have finished decoding.
> In case of frame-threading it is possible for the frame to
> only be decoded while it was waiting to be output.
> This is normally benign.
>
> But there is one case where it is not: If the user wants
> video encoding parameters to be exported, said side data
> will only be attached to the src AVFrame at the end of
> decoding the frame that is actually being shown. Without
> synchronisation adding said side data in the decoder thread
> and the reads in av_frame_ref() in the output thread
> constitute a data race. This happens e.g. when using the
> venc_data_dump tool with vp90-2-10-show-existing-frame.webm
> from the FATE-suite.
>
> Fix this by actually waiting for the frame to be output.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> libavcodec/vp9.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
LGTM, though it'd be nice of someone more familiar with this decoder
looked at it.
--
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-10 7:01 ` Anton Khirnov
@ 2024-04-10 7:09 ` Andreas Rheinhardt
2024-04-10 7:59 ` Anton Khirnov
0 siblings, 1 reply; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-10 7:09 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-04-08 22:13:40)
>> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
>> index fd356bd190..6b2c4312e0 100644
>> --- a/libavcodec/pthread_frame.c
>> +++ b/libavcodec/pthread_frame.c
>> @@ -779,6 +779,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
>> if (!copy->internal)
>> return AVERROR(ENOMEM);
>> copy->internal->thread_ctx = p;
>> + copy->internal->progress_frame_pool = avctx->internal->progress_frame_pool;
>
> I'd still prefer every thread to have its own reference.
>
> Looks good otherwise.
>
The opaque of this pool is the main AVCodecContext; if the main
AVCodecContext is destroyed, the pool is in a state where one can no
longer get new entries from it. So giving every thread its own reference
is pretending to make it an equal co-owner of the pool, but it is not as
the pool must not outlive the main AVCodecContext.
- 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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-10 7:09 ` Andreas Rheinhardt
@ 2024-04-10 7:59 ` Anton Khirnov
2024-04-10 8:02 ` Andreas Rheinhardt
0 siblings, 1 reply; 40+ messages in thread
From: Anton Khirnov @ 2024-04-10 7:59 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Andreas Rheinhardt (2024-04-10 09:09:00)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2024-04-08 22:13:40)
> >> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> >> index fd356bd190..6b2c4312e0 100644
> >> --- a/libavcodec/pthread_frame.c
> >> +++ b/libavcodec/pthread_frame.c
> >> @@ -779,6 +779,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
> >> if (!copy->internal)
> >> return AVERROR(ENOMEM);
> >> copy->internal->thread_ctx = p;
> >> + copy->internal->progress_frame_pool = avctx->internal->progress_frame_pool;
> >
> > I'd still prefer every thread to have its own reference.
> >
> > Looks good otherwise.
> >
>
> The opaque of this pool is the main AVCodecContext; if the main
> AVCodecContext is destroyed, the pool is in a state where one can no
> longer get new entries from it. So giving every thread its own reference
> is pretending to make it an equal co-owner of the pool, but it is not as
> the pool must not outlive the main AVCodecContext.
But the only use of that opaque is checking whether frame threading is
in use, which is a constant during decoder lifetime. Might be cleaner to
avoid using AVCodecContext as opaque.
In any case, this is not important, feel free to leave it as is.
--
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-10 7:59 ` Anton Khirnov
@ 2024-04-10 8:02 ` Andreas Rheinhardt
2024-04-10 8:06 ` Anton Khirnov
0 siblings, 1 reply; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-10 8:02 UTC (permalink / raw)
To: ffmpeg-devel
Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-04-10 09:09:00)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2024-04-08 22:13:40)
>>>> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
>>>> index fd356bd190..6b2c4312e0 100644
>>>> --- a/libavcodec/pthread_frame.c
>>>> +++ b/libavcodec/pthread_frame.c
>>>> @@ -779,6 +779,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
>>>> if (!copy->internal)
>>>> return AVERROR(ENOMEM);
>>>> copy->internal->thread_ctx = p;
>>>> + copy->internal->progress_frame_pool = avctx->internal->progress_frame_pool;
>>>
>>> I'd still prefer every thread to have its own reference.
>>>
>>> Looks good otherwise.
>>>
>>
>> The opaque of this pool is the main AVCodecContext; if the main
>> AVCodecContext is destroyed, the pool is in a state where one can no
>> longer get new entries from it. So giving every thread its own reference
>> is pretending to make it an equal co-owner of the pool, but it is not as
>> the pool must not outlive the main AVCodecContext.
>
> But the only use of that opaque is checking whether frame threading is
> in use, which is a constant during decoder lifetime. Might be cleaner to
> avoid using AVCodecContext as opaque.
> In any case, this is not important, feel free to leave it as is.
>
But whether frame threading is in use is only determined after
ff_decode_preinit().
- 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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-10 8:02 ` Andreas Rheinhardt
@ 2024-04-10 8:06 ` Anton Khirnov
2024-04-10 8:09 ` Andreas Rheinhardt
0 siblings, 1 reply; 40+ messages in thread
From: Anton Khirnov @ 2024-04-10 8:06 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting Andreas Rheinhardt (2024-04-10 10:02:55)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2024-04-10 09:09:00)
> >> Anton Khirnov:
> >>> Quoting Andreas Rheinhardt (2024-04-08 22:13:40)
> >>>> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> >>>> index fd356bd190..6b2c4312e0 100644
> >>>> --- a/libavcodec/pthread_frame.c
> >>>> +++ b/libavcodec/pthread_frame.c
> >>>> @@ -779,6 +779,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
> >>>> if (!copy->internal)
> >>>> return AVERROR(ENOMEM);
> >>>> copy->internal->thread_ctx = p;
> >>>> + copy->internal->progress_frame_pool = avctx->internal->progress_frame_pool;
> >>>
> >>> I'd still prefer every thread to have its own reference.
> >>>
> >>> Looks good otherwise.
> >>>
> >>
> >> The opaque of this pool is the main AVCodecContext; if the main
> >> AVCodecContext is destroyed, the pool is in a state where one can no
> >> longer get new entries from it. So giving every thread its own reference
> >> is pretending to make it an equal co-owner of the pool, but it is not as
> >> the pool must not outlive the main AVCodecContext.
> >
> > But the only use of that opaque is checking whether frame threading is
> > in use, which is a constant during decoder lifetime. Might be cleaner to
> > avoid using AVCodecContext as opaque.
> > In any case, this is not important, feel free to leave it as is.
> >
>
> But whether frame threading is in use is only determined after
> ff_decode_preinit().
I realize, which is why I'm not insisting you change this.
But it could be handled, e.g. by allocating the pool later, or splitting
off the part of thread init that determines active_thread_type.
--
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API
2024-04-10 8:06 ` Anton Khirnov
@ 2024-04-10 8:09 ` Andreas Rheinhardt
0 siblings, 0 replies; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-10 8:09 UTC (permalink / raw)
To: ffmpeg-devel
Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-04-10 10:02:55)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2024-04-10 09:09:00)
>>>> Anton Khirnov:
>>>>> Quoting Andreas Rheinhardt (2024-04-08 22:13:40)
>>>>>> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
>>>>>> index fd356bd190..6b2c4312e0 100644
>>>>>> --- a/libavcodec/pthread_frame.c
>>>>>> +++ b/libavcodec/pthread_frame.c
>>>>>> @@ -779,6 +779,7 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
>>>>>> if (!copy->internal)
>>>>>> return AVERROR(ENOMEM);
>>>>>> copy->internal->thread_ctx = p;
>>>>>> + copy->internal->progress_frame_pool = avctx->internal->progress_frame_pool;
>>>>>
>>>>> I'd still prefer every thread to have its own reference.
>>>>>
>>>>> Looks good otherwise.
>>>>>
>>>>
>>>> The opaque of this pool is the main AVCodecContext; if the main
>>>> AVCodecContext is destroyed, the pool is in a state where one can no
>>>> longer get new entries from it. So giving every thread its own reference
>>>> is pretending to make it an equal co-owner of the pool, but it is not as
>>>> the pool must not outlive the main AVCodecContext.
>>>
>>> But the only use of that opaque is checking whether frame threading is
>>> in use, which is a constant during decoder lifetime. Might be cleaner to
>>> avoid using AVCodecContext as opaque.
>>> In any case, this is not important, feel free to leave it as is.
>>>
>>
>> But whether frame threading is in use is only determined after
>> ff_decode_preinit().
>
> I realize, which is why I'm not insisting you change this.
>
> But it could be handled, e.g. by allocating the pool later, or splitting
> off the part of thread init that determines active_thread_type.
>
Which is IMO less elegant than simply doing what I did here.
- 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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
` (27 preceding siblings ...)
2024-04-10 6:38 ` [FFmpeg-devel] [PATCH v2 " Anton Khirnov
@ 2024-04-18 20:40 ` Andreas Rheinhardt
2024-04-18 21:30 ` epirat07
28 siblings, 1 reply; 40+ messages in thread
From: Andreas Rheinhardt @ 2024-04-18 20:40 UTC (permalink / raw)
To: ffmpeg-devel
Andreas Rheinhardt:
> The API is similar to the ThreadFrame API, with the exception
> that it no longer has an included AVFrame and that it has its
> own mutexes and condition variables which makes it more independent
> of pthread_frame.c. 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. This will typically be solved
> by putting a ThreadProgress in a refcounted structure that is
> shared between threads.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Now including its own mutex and condition variable. Hopefully
> no system has constraints on the number of mutexes and condition
> variables it supports.
>
> libavcodec/threadprogress.c | 73 +++++++++++++++++++++++++++++
> libavcodec/threadprogress.h | 91 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 164 insertions(+)
> create mode 100644 libavcodec/threadprogress.c
> create mode 100644 libavcodec/threadprogress.h
>
> diff --git a/libavcodec/threadprogress.c b/libavcodec/threadprogress.c
> new file mode 100644
> index 0000000000..dd2a4d2336
> --- /dev/null
> +++ b/libavcodec/threadprogress.c
> @@ -0,0 +1,73 @@
> +/*
> + * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> + *
> + * 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
> + */
> +
> +#include <limits.h>
> +#include <stdatomic.h>
> +
> +#include "pthread_internal.h"
> +#include "threadprogress.h"
> +#include "libavutil/attributes.h"
> +
> +DEFINE_OFFSET_ARRAY(ThreadProgress, thread_progress, init,
> + (offsetof(ThreadProgress, progress_mutex)),
> + (offsetof(ThreadProgress, progress_cond)));
> +
> +av_cold int ff_thread_progress_init(ThreadProgress *pro, int init_mode)
> +{
> + atomic_init(&pro->progress, init_mode ? -1 : INT_MAX);
> + if (!init_mode) {
> + pro->init = 0;
> + return 0;
> + }
> + return ff_pthread_init(pro, thread_progress_offsets);
> +}
> +
> +av_cold void ff_thread_progress_destroy(ThreadProgress *pro)
> +{
> + ff_pthread_free(pro, thread_progress_offsets);
> +}
> +
> +void ff_thread_progress_report(ThreadProgress *pro, int n)
> +{
> + if (atomic_load_explicit(&pro->progress, memory_order_relaxed) >= n)
> + return;
> +
> + atomic_store_explicit(&pro->progress, n, memory_order_release);
> +
> + pthread_mutex_lock(&pro->progress_mutex);
> + pthread_cond_broadcast(&pro->progress_cond);
> + pthread_mutex_unlock(&pro->progress_mutex);
> +}
> +
> +void ff_thread_progress_await(const ThreadProgress *pro_c, int n)
> +{
> + /* Casting const away here is safe, because we only read from progress
> + * and will leave pro_c in the same state upon leaving the function
> + * as it had at the beginning. */
> + ThreadProgress *pro = (ThreadProgress*)pro_c;
> +
> + if (atomic_load_explicit(&pro->progress, memory_order_acquire) >= n)
> + return;
> +
> + pthread_mutex_lock(&pro->progress_mutex);
> + while (atomic_load_explicit(&pro->progress, memory_order_relaxed) < n)
> + pthread_cond_wait(&pro->progress_cond, &pro->progress_mutex);
> + pthread_mutex_unlock(&pro->progress_mutex);
> +}
> diff --git a/libavcodec/threadprogress.h b/libavcodec/threadprogress.h
> new file mode 100644
> index 0000000000..786802cbf1
> --- /dev/null
> +++ b/libavcodec/threadprogress.h
> @@ -0,0 +1,91 @@
> +/*
> + * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> + *
> + * 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
> +
> +/**
> + * ThreadProgress is an API to easily notify other threads about progress
> + * of any kind as long as it can be packaged into an int and is consistent
> + * with the natural ordering of integers.
> + *
> + * Each initialized ThreadProgress can be in one of two modes: No-op mode
> + * or ordinary mode. In the former mode, ff_thread_report_progress() and
> + * ff_thread_await_progress() are no-ops to simply support usecases like
> + * non-frame-threading. Only in the latter case perform these functions
> + * what their name already implies.
> + */
> +
> +#include <limits.h>
> +#include <stdatomic.h>
> +#include "libavutil/thread.h"
> +
> +/**
> + * This struct should be treated as opaque by users.
> + */
> +typedef struct ThreadProgress {
> + atomic_int progress;
> + unsigned init;
> + AVMutex progress_mutex;
> + AVCond progress_cond;
> +} ThreadProgress;
> +
> +/**
> + * Initialize a ThreadProgress.
> + *
> + * @param init_mode If zero, the ThreadProgress will be initialized so that
> + * to be in no-op mode as described above. Otherwise
> + */
> +int ff_thread_progress_init(ThreadProgress *pro, int init_mode);
> +
> +/**
> + * Destroy a ThreadProgress. Can be called on a ThreadProgress that
> + * has never been initialized provided that the ThreadProgress struct
> + * has been initially zeroed. Must be called even if ff_thread_progress_init()
> + * failed.
> + */
> +void ff_thread_progress_destroy(ThreadProgress *pro);
> +
> +/**
> + * Reset the ThreadProgress's progress progress counter; must only be called
> + * if it is not in use in any way (e.g. no thread may wait on it via
> + * ff_thread_progress_await()).
> + */
> +static inline void ff_thread_progress_reset(ThreadProgress *pro)
> +{
> + atomic_init(&pro->progress, pro->init ? -1 : INT_MAX);
> +}
> +
> +/**
> + * This function is a no-op in no-op mode; otherwise it notifies
> + * other threads that a certain level of progress has been reached.
> + * Later calls with lower values of progress have no effect.
> + */
> +void ff_thread_progress_report(ThreadProgress *pro, int progress);
> +
> +/**
> + * This function is a no-op in no-op mode; otherwise it waits
> + * until other threads have reached a certain level of progress:
> + * This function will return after another thread has called
> + * ff_thread_progress_report() with the same or higher value for progress.
> + */
> +void ff_thread_progress_await(const ThreadProgress *pro, int progress);
> +
> +#endif /* AVCODEC_THREADPROGRESS_H */
Will apply this patchset tomorrow unless there are objections.
- 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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress
2024-04-18 20:40 ` Andreas Rheinhardt
@ 2024-04-18 21:30 ` epirat07
0 siblings, 0 replies; 40+ messages in thread
From: epirat07 @ 2024-04-18 21:30 UTC (permalink / raw)
To: FFmpeg development discussions and patches
On 18 Apr 2024, at 22:40, Andreas Rheinhardt wrote:
> Andreas Rheinhardt:
>> The API is similar to the ThreadFrame API, with the exception
>> that it no longer has an included AVFrame and that it has its
>> own mutexes and condition variables which makes it more independent
>> of pthread_frame.c. 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. This will typically be solved
>> by putting a ThreadProgress in a refcounted structure that is
>> shared between threads.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> Now including its own mutex and condition variable. Hopefully
>> no system has constraints on the number of mutexes and condition
>> variables it supports.
>>
>> libavcodec/threadprogress.c | 73 +++++++++++++++++++++++++++++
>> libavcodec/threadprogress.h | 91 +++++++++++++++++++++++++++++++++++++
>> 2 files changed, 164 insertions(+)
>> create mode 100644 libavcodec/threadprogress.c
>> create mode 100644 libavcodec/threadprogress.h
>>
>> diff --git a/libavcodec/threadprogress.c b/libavcodec/threadprogress.c
>> new file mode 100644
>> index 0000000000..dd2a4d2336
>> --- /dev/null
>> +++ b/libavcodec/threadprogress.c
>> @@ -0,0 +1,73 @@
>> +/*
>> + * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> + *
>> + * 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
>> + */
>> +
>> +#include <limits.h>
>> +#include <stdatomic.h>
>> +
>> +#include "pthread_internal.h"
>> +#include "threadprogress.h"
>> +#include "libavutil/attributes.h"
>> +
>> +DEFINE_OFFSET_ARRAY(ThreadProgress, thread_progress, init,
>> + (offsetof(ThreadProgress, progress_mutex)),
>> + (offsetof(ThreadProgress, progress_cond)));
>> +
>> +av_cold int ff_thread_progress_init(ThreadProgress *pro, int init_mode)
>> +{
>> + atomic_init(&pro->progress, init_mode ? -1 : INT_MAX);
>> + if (!init_mode) {
>> + pro->init = 0;
>> + return 0;
>> + }
>> + return ff_pthread_init(pro, thread_progress_offsets);
>> +}
>> +
>> +av_cold void ff_thread_progress_destroy(ThreadProgress *pro)
>> +{
>> + ff_pthread_free(pro, thread_progress_offsets);
>> +}
>> +
>> +void ff_thread_progress_report(ThreadProgress *pro, int n)
>> +{
>> + if (atomic_load_explicit(&pro->progress, memory_order_relaxed) >= n)
>> + return;
>> +
>> + atomic_store_explicit(&pro->progress, n, memory_order_release);
>> +
>> + pthread_mutex_lock(&pro->progress_mutex);
>> + pthread_cond_broadcast(&pro->progress_cond);
>> + pthread_mutex_unlock(&pro->progress_mutex);
>> +}
>> +
>> +void ff_thread_progress_await(const ThreadProgress *pro_c, int n)
>> +{
>> + /* Casting const away here is safe, because we only read from progress
>> + * and will leave pro_c in the same state upon leaving the function
>> + * as it had at the beginning. */
>> + ThreadProgress *pro = (ThreadProgress*)pro_c;
>> +
>> + if (atomic_load_explicit(&pro->progress, memory_order_acquire) >= n)
>> + return;
>> +
>> + pthread_mutex_lock(&pro->progress_mutex);
>> + while (atomic_load_explicit(&pro->progress, memory_order_relaxed) < n)
>> + pthread_cond_wait(&pro->progress_cond, &pro->progress_mutex);
>> + pthread_mutex_unlock(&pro->progress_mutex);
>> +}
>> diff --git a/libavcodec/threadprogress.h b/libavcodec/threadprogress.h
>> new file mode 100644
>> index 0000000000..786802cbf1
>> --- /dev/null
>> +++ b/libavcodec/threadprogress.h
>> @@ -0,0 +1,91 @@
>> +/*
>> + * Copyright (c) 2022 Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> + *
>> + * 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
>> +
>> +/**
>> + * ThreadProgress is an API to easily notify other threads about progress
>> + * of any kind as long as it can be packaged into an int and is consistent
>> + * with the natural ordering of integers.
>> + *
>> + * Each initialized ThreadProgress can be in one of two modes: No-op mode
>> + * or ordinary mode. In the former mode, ff_thread_report_progress() and
>> + * ff_thread_await_progress() are no-ops to simply support usecases like
>> + * non-frame-threading. Only in the latter case perform these functions
>> + * what their name already implies.
>> + */
>> +
>> +#include <limits.h>
>> +#include <stdatomic.h>
>> +#include "libavutil/thread.h"
>> +
>> +/**
>> + * This struct should be treated as opaque by users.
>> + */
>> +typedef struct ThreadProgress {
>> + atomic_int progress;
>> + unsigned init;
>> + AVMutex progress_mutex;
>> + AVCond progress_cond;
>> +} ThreadProgress;
>> +
>> +/**
>> + * Initialize a ThreadProgress.
>> + *
>> + * @param init_mode If zero, the ThreadProgress will be initialized so that
>> + * to be in no-op mode as described above. Otherwise
Nit: Stray „Otherwise“ here at the end?
>> + */
>> +int ff_thread_progress_init(ThreadProgress *pro, int init_mode);
>> +
>> +/**
>> + * Destroy a ThreadProgress. Can be called on a ThreadProgress that
>> + * has never been initialized provided that the ThreadProgress struct
>> + * has been initially zeroed. Must be called even if ff_thread_progress_init()
>> + * failed.
>> + */
>> +void ff_thread_progress_destroy(ThreadProgress *pro);
>> +
>> +/**
>> + * Reset the ThreadProgress's progress progress counter; must only be called
I think you can use a proper reference here: ::ThreadProgress.progress
>> + * if it is not in use in any way (e.g. no thread may wait on it via
>> + * ff_thread_progress_await()).
>> + */
>> +static inline void ff_thread_progress_reset(ThreadProgress *pro)
>> +{
>> + atomic_init(&pro->progress, pro->init ? -1 : INT_MAX);
>> +}
>> +
>> +/**
>> + * This function is a no-op in no-op mode; otherwise it notifies
>> + * other threads that a certain level of progress has been reached.
>> + * Later calls with lower values of progress have no effect.
>> + */
>> +void ff_thread_progress_report(ThreadProgress *pro, int progress);
>> +
>> +/**
>> + * This function is a no-op in no-op mode; otherwise it waits
>> + * until other threads have reached a certain level of progress:
>> + * This function will return after another thread has called
>> + * ff_thread_progress_report() with the same or higher value for progress.
>> + */
>> +void ff_thread_progress_await(const ThreadProgress *pro, int progress);
>> +
>> +#endif /* AVCODEC_THREADPROGRESS_H */
>
> Will apply this patchset tomorrow unless there are objections.
>
> - 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-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 40+ messages in thread
end of thread, other threads:[~2024-04-18 21:30 UTC | newest]
Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-08 19:51 [FFmpeg-devel] [PATCH v2 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 02/27] avcodec/decode: Add new ProgressFrame API Andreas Rheinhardt
2024-04-09 1:40 ` Michael Niedermayer
2024-04-09 6:35 ` Andreas Rheinhardt
2024-04-10 7:01 ` Anton Khirnov
2024-04-10 7:09 ` Andreas Rheinhardt
2024-04-10 7:59 ` Anton Khirnov
2024-04-10 8:02 ` Andreas Rheinhardt
2024-04-10 8:06 ` Anton Khirnov
2024-04-10 8:09 ` Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 03/27] avcodec/mimic: Switch to ProgressFrames Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 04/27] avcodec/vp3: " Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 05/27] avcodec/vp9: " Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 06/27] avcodec/vp9: Fix race when attaching side-data for show-existing frame Andreas Rheinhardt
2024-04-10 7:06 ` Anton Khirnov
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 07/27] avcodec/vp9: Reduce wait times Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 08/27] avcodec/vp9: Simplify replacing VP9Frame Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 09/27] avcodec/vp9: Replace atomic_store() by atomic_init() Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 10/27] avcodec/pthread_frame: Add API to share RefStruct refs just once Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 11/27] avcodec/wavpack: Use ThreadProgress API Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 12/27] avcodec/wavpack: Move initializing DSD data to a better place Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 13/27] avcodec/wavpack: Only reset DSD context upon parameter change Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 14/27] avcodec/wavpack: Optimize always-false comparison away Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 15/27] avcodec/wavpack: Move transient variable from context to stack Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 16/27] avcodec/vp8: Convert to ProgressFrame API Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 17/27] avcodec/vp8: Mark flushing functions as av_cold Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 18/27] avcodec/codec_internal: Remove FF_CODEC_CAP_ALLOCATE_PROGRESS Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 19/27] avcodec/hevcdec: Switch to ProgressFrames Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 20/27] avcodec/pngdec: " Andreas Rheinhardt
2024-04-08 20:13 ` [FFmpeg-devel] [PATCH v2 21/27] avcodec/ffv1dec: " Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 22/27] avcodec/qsv: Use RefStruct API for memory id (mids) array Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 23/27] avcodec/rkmppdec: Fix double-free on error Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 24/27] avcodec/rkmppdec: Check av_buffer_ref() Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 25/27] avcodec/rkmppdec: Use RefStruct API for references to decoder itself Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 26/27] avcodec/rkmppdec: Allocate AVDRMFrameDescriptor and frame ctx jointly Andreas Rheinhardt
2024-04-08 20:14 ` [FFmpeg-devel] [PATCH v2 27/27] avcodec/v4l2_(m2m|buffers): Use RefStruct API for context references Andreas Rheinhardt
2024-04-09 6:33 ` [FFmpeg-devel] [PATCH v3 01/27] avcodec/threadprogress: Add new API for frame-threaded progress Andreas Rheinhardt
2024-04-10 6:38 ` [FFmpeg-devel] [PATCH v2 " Anton Khirnov
2024-04-18 20:40 ` Andreas Rheinhardt
2024-04-18 21:30 ` epirat07
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git