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

[wm4]

On Tue, 7 Mar 2017 11:09:19 -0300
James Almer <jamrial at gmail.com> wrote:

> On 3/7/2017 11:05 AM, wm4 wrote:
> > On Tue, 7 Mar 2017 20:48:13 +0700
> > Muhammad Faiz <mfcc64 at gmail.com> wrote:
> >   
> >> On Tue, Mar 7, 2017 at 4:17 PM, wm4 <nfxjfg at googlemail.com> wrote:  
> >>> On Tue,  7 Mar 2017 16:01:58 +0700
> >>> Muhammad Faiz <mfcc64 at gmail.com> wrote:
> >>>    
> >>>> use ff_thread_once
> >>>>
> >>>> Suggested-by: wm4 <nfxjfg at googlemail.com>
> >>>> Signed-off-by: Muhammad Faiz <mfcc64 at gmail.com>
> >>>> ---
> >>>>  libavcodec/allcodecs.c | 16 +++++++++-------
> >>>>  1 file changed, 9 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> >>>> index eee322b..6ec7e79 100644
> >>>> --- a/libavcodec/allcodecs.c
> >>>> +++ b/libavcodec/allcodecs.c
> >>>> @@ -25,6 +25,7 @@
> >>>>   */
> >>>>
> >>>>  #include "config.h"
> >>>> +#include "libavutil/thread.h"
> >>>>  #include "avcodec.h"
> >>>>  #include "version.h"
> >>>>
> >>>> @@ -58,14 +59,8 @@
> >>>>              av_register_codec_parser(&ff_##x##_parser);                 \
> >>>>      }
> >>>>
> >>>> -void avcodec_register_all(void)
> >>>> +static void register_all(void)
> >>>>  {
> >>>> -    static int initialized;
> >>>> -
> >>>> -    if (initialized)
> >>>> -        return;
> >>>> -    initialized = 1;
> >>>> -
> >>>>      /* hardware accelerators */
> >>>>      REGISTER_HWACCEL(H263_VAAPI,        h263_vaapi);
> >>>>      REGISTER_HWACCEL(H263_VIDEOTOOLBOX, h263_videotoolbox);
> >>>> @@ -717,3 +712,10 @@ void avcodec_register_all(void)
> >>>>      REGISTER_PARSER(VP9,                vp9);
> >>>>      REGISTER_PARSER(XMA,                xma);
> >>>>  }
> >>>> +
> >>>> +void avcodec_register_all(void)
> >>>> +{
> >>>> +    static AVOnce control = AV_ONCE_INIT;
> >>>> +
> >>>> +    ff_thread_once(&control, register_all);
> >>>> +}    
> >>>
> >>> Looks all good to me.
> >>>
> >>> Of course this doesn't change that the thread-safety of this whole
> >>> "list of components" code is still questionable, but it doesn't hurt to
> >>> do this either.    
> >>
> >> Patchset applied.
> >>
> >> Thank's.  
> > 
> > I'm not the maintainer, though. Looks like you violated the development
> > rules. (Not that I blame you, it's just an observation.)  
> 
> There's no listed maintainer for all*.c files, so your review was enough
> for a simple patchset like this.

Hm, I guess with strict reading that's true... all is fine, then.