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

Paul B Mahol onemda at gmail.com
Tue Mar 7 15:42:52 EET 2017


On 3/7/17, Michael Niedermayer <michael at niedermayer.cc> wrote:
> 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

In that case I will fork FFmpeg as AGPL only.


More information about the ffmpeg-devel mailing list