On 29 November 2017 at 16:40, Michael Niedermayer <mich...@niedermayer.cc>
wrote:

> On Wed, Nov 29, 2017 at 03:51:34AM +0000, Rostislav Pehlivanov wrote:
> > On 28 November 2017 at 20:23, Michael Niedermayer <mich...@niedermayer.cc
> >
> > wrote:
> >
> > > On Mon, Nov 27, 2017 at 04:30:19AM +0000, Rostislav Pehlivanov wrote:
> > > > Atomics were entirely pointless and did nothing but slow and
> complicate
> > > > the process down. This could be improved further still but the main
> > > > objective of this commit is to simplify.
> > > >
> > > > Signed-off-by: Rostislav Pehlivanov <atomnu...@gmail.com>
> > > > ---
> > > >  libavfilter/avfilter.c | 8 +++++---
> > > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > *_register() can be called by the user app in a unsyncronized way
> > > for example from a module like vlc used AFAIK.
> > > or from libs loaded by an application which use libavfilter or other
> > > internally.
> > >
> > > These registration functions should stay thread safe
> > >
> > >
> > > [...]
> > >
> > > --
> > > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC7
> 87040B0FAB
> > >
> > > Those who would give up essential Liberty, to purchase a little
> > > temporary Safety, deserve neither Liberty nor Safety -- Benjamin
> Franklin
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > >
> > What variable actually needs to be protected?
>
> The linked list of registered "entities"
> That is if the addition is not synchronized by some means, an atomic
> exchange a mutex or something then a race can generally occur
> and only one addition might succeed or something else might get
> entangled
>
> This is in practical terms a very unlikely event to occur of course
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> You can kill me, but you cannot change the truth.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Could you test the attached patch? Pretty much everything is synchronized.
From fb9789206c76c876d6a79e81df68c6b8041f03c3 Mon Sep 17 00:00:00 2001
From: Rostislav Pehlivanov <atomnu...@gmail.com>
Date: Mon, 27 Nov 2017 04:12:26 +0000
Subject: [PATCH v2 2/2] lavfi/avfilter: simplify filter registration

Atomics were entirely pointless and did nothing but slow and complicate
the process down. This could be improved further still but the main
objective of this commit is to simplify.

Signed-off-by: Rostislav Pehlivanov <atomnu...@gmail.com>
---
 libavfilter/avfilter.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index b98b32bacb..76c9f12be9 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -19,7 +19,6 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#include "libavutil/atomic.h"
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
 #include "libavutil/buffer.h"
@@ -37,6 +36,7 @@
 #define FF_INTERNAL_FIELDS 1
 #include "framequeue.h"
 
+#include <stdatomic.h>
 #include "audio.h"
 #include "avfilter.h"
 #include "filters.h"
@@ -574,7 +574,7 @@ int avfilter_process_command(AVFilterContext *filter, const char *cmd, const cha
 }
 
 static AVFilter *first_filter;
-static AVFilter **last_filter = &first_filter;
+static _Atomic (AVFilter **) last_filter = ATOMIC_VAR_INIT(&first_filter);
 
 const AVFilter *avfilter_get_by_name(const char *name)
 {
@@ -592,16 +592,16 @@ const AVFilter *avfilter_get_by_name(const char *name)
 
 int avfilter_register(AVFilter *filter)
 {
-    AVFilter **f = last_filter;
-
     /* the filter must select generic or internal exclusively */
     av_assert0((filter->flags & AVFILTER_FLAG_SUPPORT_TIMELINE) != AVFILTER_FLAG_SUPPORT_TIMELINE);
 
     filter->next = NULL;
 
-    while(*f || avpriv_atomic_ptr_cas((void * volatile *)f, NULL, filter))
-        f = &(*f)->next;
-    last_filter = &filter->next;
+    /* Iterate through the list until the last entry has been reached */
+    do {
+        *atomic_load(&last_filter) = filter;
+        atomic_store(&last_filter, &filter->next);
+    } while (*atomic_load(&last_filter));
 
     return 0;
 }
-- 
2.15.1.424.g9478a66081

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to