From f29623786b871807a663ce922d9afbf0d716a387 Mon Sep 17 00:00:00 2001 From: wm4 Date: Sat, 29 Feb 2020 21:04:24 +0100 Subject: [PATCH] filter: decide how multi-threading is supposed to work Instead of vague ideas about making different filter graphs on different threads interact directly, this have no direct support. Instead, helpers are required (such as added with the next commit). Document it. Different root filters (i.e. separate filter graphs) are now considered to be part of separate threads, so assert() if they're found to accidentally interact. --- filters/filter.c | 11 +++++++++++ filters/filter.h | 21 +++++++++++++-------- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/filters/filter.c b/filters/filter.c index 84596b723e..dfe6df953a 100644 --- a/filters/filter.c +++ b/filters/filter.c @@ -183,6 +183,8 @@ bool mp_filter_run(struct mp_filter *filter) r->filtering = true; + // Note: some filters may call mp_filter_wakeup() from process on themselves + // to queue a wakeup again later. So do not call this in the loop. flush_async_notifications(r); while (r->num_pending) { @@ -311,6 +313,8 @@ static struct mp_pin *find_connected_end(struct mp_pin *p) // state flags. static void init_connection(struct mp_pin *p) { + struct filter_runner *runner = p->owner->in->runner; + if (p->dir == MP_PIN_IN) p = p->other; @@ -321,6 +325,12 @@ static void init_connection(struct mp_pin *p) assert(!in->user_conn); assert(!out->user_conn); + // This and similar checks enforce the same root filter requirement. + if (in->manual_connection) + assert(in->manual_connection->in->runner == runner); + if (out->manual_connection) + assert(out->manual_connection->in->runner == runner); + // Logicaly, the ends are always manual connections. A pin chain without // manual connections at the ends is still disconnected (or if this // attempted to extend an existing connection, becomes dangling and gets @@ -339,6 +349,7 @@ static void init_connection(struct mp_pin *p) assert(!cur->data.type); // unused for in pins assert(!cur->other->data_requested); // unset for unconnected out pins assert(!cur->other->data.type); // unset for unconnected out pins + assert(cur->owner->in->runner == runner); cur->within_conn = cur->other->within_conn = true; cur = cur->other->user_conn; } diff --git a/filters/filter.h b/filters/filter.h index 5e09a17ee7..34146af98d 100644 --- a/filters/filter.h +++ b/filters/filter.h @@ -217,12 +217,6 @@ const char *mp_pin_get_name(struct mp_pin *p); * call the first filter's process() function, which will filter and output * the frame, and the frame is iteratively filtered until it reaches the output. * - * (The mp_pin_* calls can recursively start filtering, but this is only the - * case if you access a separate graph with a different filter root. Most - * importantly, calling them from outside the filter's process() function (e.g. - * an outside filter user) will enter filtering. Within the filter, mp_pin_* - * will usually only set or query flags.) - * * --- General rules for thread safety: * * Filters are by default not thread safe. However, some filters can be @@ -231,6 +225,14 @@ const char *mp_pin_get_name(struct mp_pin *p); * for some utility functions explicitly marked as such, and which are meant * to make implementing threaded filters easier. * + * (Semi-)automatic filter communication such as pins must always be within the + * same root filter. This is meant to help with ensuring thread-safety. Every + * thread that wants to run filters "on its own" should use a different filter + * graph, and disallowing different root filters ensures these graphs are not + * accidentally connected using non-thread safe mechanisms. Actual threaded + * filter graphs would use several independent graphs connected by asynchronous + * helpers (such as queues instead of mp_pin connections). + * * --- Rules for manual connections: * * A pin can be marked for manual connection via mp_pin_set_manual_connection(). @@ -272,6 +274,11 @@ const char *mp_pin_get_name(struct mp_pin *p); * For running parts of a filter graph on a different thread, f_async_queue.h * can be used. * + * With different filter graphs working asynchronously, reset handling and start + * of filtering becomes more difficult. Since filtering is always triggered by + * requesting output from a filter, a simple way to solve this is to trigger + * resets from the consumer, and to synchronously reset the producer. + * * --- Format conversions and mid-stream format changes: * * Generally, all filters must support all formats, as well as mid-stream @@ -402,8 +409,6 @@ struct AVBufferRef *mp_filter_load_hwdec_device(struct mp_filter *f, int avtype) // Perform filtering. This runs until the filter graph is blocked (due to // missing external input or unread output). It returns whether any outside // pins have changed state. -// Does not perform recursive filtering to connected filters with different -// root filter, though it notifies them. bool mp_filter_run(struct mp_filter *f); // Create a root dummy filter with no inputs or outputs. This fulfills the