| Message ID | 20170307090201.11017-1-mfcc64@gmail.com |
|---|---|
| State | Accepted |
| Commit | e85e8408802dc3a474e6a610867df8e57c768339 |
| Headers | show |
On Tue, 7 Mar 2017 16:01:58 +0700 Muhammad Faiz <mfcc64@gmail.com> wrote: > use ff_thread_once > > Suggested-by: wm4 <nfxjfg@googlemail.com> > Signed-off-by: Muhammad Faiz <mfcc64@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.
On Tue, Mar 7, 2017 at 4:17 PM, wm4 <nfxjfg@googlemail.com> wrote: > On Tue, 7 Mar 2017 16:01:58 +0700 > Muhammad Faiz <mfcc64@gmail.com> wrote: > >> use ff_thread_once >> >> Suggested-by: wm4 <nfxjfg@googlemail.com> >> Signed-off-by: Muhammad Faiz <mfcc64@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.
On Tue, 7 Mar 2017 20:48:13 +0700 Muhammad Faiz <mfcc64@gmail.com> wrote: > On Tue, Mar 7, 2017 at 4:17 PM, wm4 <nfxjfg@googlemail.com> wrote: > > On Tue, 7 Mar 2017 16:01:58 +0700 > > Muhammad Faiz <mfcc64@gmail.com> wrote: > > > >> use ff_thread_once > >> > >> Suggested-by: wm4 <nfxjfg@googlemail.com> > >> Signed-off-by: Muhammad Faiz <mfcc64@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.)
On 3/7/2017 11:05 AM, wm4 wrote: > On Tue, 7 Mar 2017 20:48:13 +0700 > Muhammad Faiz <mfcc64@gmail.com> wrote: > >> On Tue, Mar 7, 2017 at 4:17 PM, wm4 <nfxjfg@googlemail.com> wrote: >>> On Tue, 7 Mar 2017 16:01:58 +0700 >>> Muhammad Faiz <mfcc64@gmail.com> wrote: >>> >>>> use ff_thread_once >>>> >>>> Suggested-by: wm4 <nfxjfg@googlemail.com> >>>> Signed-off-by: Muhammad Faiz <mfcc64@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.
On Tue, 7 Mar 2017 11:09:19 -0300 James Almer <jamrial@gmail.com> wrote: > On 3/7/2017 11:05 AM, wm4 wrote: > > On Tue, 7 Mar 2017 20:48:13 +0700 > > Muhammad Faiz <mfcc64@gmail.com> wrote: > > > >> On Tue, Mar 7, 2017 at 4:17 PM, wm4 <nfxjfg@googlemail.com> wrote: > >>> On Tue, 7 Mar 2017 16:01:58 +0700 > >>> Muhammad Faiz <mfcc64@gmail.com> wrote: > >>> > >>>> use ff_thread_once > >>>> > >>>> Suggested-by: wm4 <nfxjfg@googlemail.com> > >>>> Signed-off-by: Muhammad Faiz <mfcc64@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.
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); +}
use ff_thread_once Suggested-by: wm4 <nfxjfg@googlemail.com> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> --- libavcodec/allcodecs.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)