[FFmpeg-devel] [PATCH 1/4] avcodec/allcodecs: make avcodec_register_all thread safe

[Michael Niedermayer]

On Tue, Mar 07, 2017 at 07:44:43AM +0100, wm4 wrote:
> On Tue,  7 Mar 2017 02:47:36 +0700
> Muhammad Faiz <mfcc64 at gmail.com> wrote:
> 
> > Signed-off-by: Muhammad Faiz <mfcc64 at gmail.com>
> > ---
> >  libavcodec/allcodecs.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index eee322b..1d05fc1 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -24,6 +24,8 @@
> >   * Provide registration of all codecs, parsers and bitstream filters for libavcodec.
> >   */
> >  
> > +#include <stdatomic.h>
> > +
> >  #include "config.h"
> >  #include "avcodec.h"
> >  #include "version.h"
> > @@ -60,11 +62,14 @@
> >  
> >  void avcodec_register_all(void)
> >  {
> > -    static int initialized;
> > +    static atomic_int initialized;
> > +    static atomic_int ready;
> >  
> > -    if (initialized)
> > +    if (atomic_exchange_explicit(&initialized, 1, memory_order_relaxed)) {
> > +        while (!atomic_load_explicit(&ready, memory_order_relaxed))
> > +            ;
> >          return;
> > -    initialized = 1;
> > +    }
> >  
> >      /* hardware accelerators */
> >      REGISTER_HWACCEL(H263_VAAPI,        h263_vaapi);
> > @@ -716,4 +721,6 @@ void avcodec_register_all(void)
> >      REGISTER_PARSER(VP8,                vp8);
> >      REGISTER_PARSER(VP9,                vp9);
> >      REGISTER_PARSER(XMA,                xma);
> > +
> > +    atomic_store_explicit(&ready, 1, memory_order_relaxed);
> >  }
> 

> avcodec_register() is already "safe" by attempting to do lock-free and
> wait-free list insertion (not very convincing IMHO, but it's there).
> Should that code be removed?

that code is needed, otherwise avcodec_register() would not be thread
safe


> Does anyone know why avcodec_register() is
> even still public API? The same affects some other things.

It is public so that user written codecs can be registered

Ideally we would have a real plugin system and for example rune could
maintain his cinepack decoder as a plugin in his github repo.
Then he would likely not have left FFmpeg today.

Removing avcodec_register() IMO is a step in the wrong direction as
after that theres no way to register user written codecs and then the
API also would no longer need to be unchanged and once the API changes
frequently adding a real plugin interface becomes MUCH harder.

The real problem behind all this is of course more complex and a
combination of many of our developers being rather aggressive and
having strong oppinions on code outside the area they actively maintain.
That drives some contributors away and now the lack of a plugin
interface means they are driven over the ledge and a lost as
contributors to FFmpeg.

I would very much like to see some effort to add a real plugin
interface instead of the opposite direction


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170307/113cbff0/attachment.sig>