[FFmpeg-devel] [PATCH 2/4] lavfi/avfilter: simplify filter registration

[Hendrik Leppkes]

On Thu, Nov 30, 2017 at 12:54 PM, Rostislav Pehlivanov
<atomnuker at gmail.com> wrote:
> On 29 November 2017 at 16:40, Michael Niedermayer <michael at 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 <michael at 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 <atomnuker at 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 at 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 at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
> Could you test the attached patch? Pretty much everything is synchronized.
>

The only way to have two threads modify a linked list at the same time
is with a "compare exchange" (cas) instruction, so why not just keep
the old logic and replace the avpriv atomic with a stdatomic one?

ie.
while(*f || avpriv_atomic_ptr_cas((void * volatile *)f, NULL, filter))

becomes: (plus appropriate type changes/casts as required)
while(*f || atomic_compare_exchange_strong(f, NULL, filter) == false)

- Hendrik