Re: [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  wrote:

> On 3/7/2017 11:05 AM, wm4 wrote:
> > On Tue, 7 Mar 2017 20:48:13 +0700
> > Muhammad Faiz  wrote:
> >   
> >> On Tue, Mar 7, 2017 at 4:17 PM, wm4  wrote:  
> >>> On Tue,  7 Mar 2017 16:01:58 +0700
> >>> Muhammad Faiz  wrote:
> >>>
>  use ff_thread_once
> 
>  Suggested-by: wm4 
>  Signed-off-by: Muhammad Faiz 
>  ---
>   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(_##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(, 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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[James Almer]

On 3/7/2017 11:05 AM, wm4 wrote:
> On Tue, 7 Mar 2017 20:48:13 +0700
> Muhammad Faiz  wrote:
> 
>> On Tue, Mar 7, 2017 at 4:17 PM, wm4  wrote:
>>> On Tue,  7 Mar 2017 16:01:58 +0700
>>> Muhammad Faiz  wrote:
>>>  
 use ff_thread_once

 Suggested-by: wm4 
 Signed-off-by: Muhammad Faiz 
 ---
  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(_##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(, 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.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Tue, 7 Mar 2017 20:48:13 +0700
Muhammad Faiz  wrote:

> On Tue, Mar 7, 2017 at 4:17 PM, wm4  wrote:
> > On Tue,  7 Mar 2017 16:01:58 +0700
> > Muhammad Faiz  wrote:
> >  
> >> use ff_thread_once
> >>
> >> Suggested-by: wm4 
> >> Signed-off-by: Muhammad Faiz 
> >> ---
> >>  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(_##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(, 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.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Muhammad Faiz]

On Tue, Mar 7, 2017 at 4:17 PM, wm4  wrote:
> On Tue,  7 Mar 2017 16:01:58 +0700
> Muhammad Faiz  wrote:
>
>> use ff_thread_once
>>
>> Suggested-by: wm4 
>> Signed-off-by: Muhammad Faiz 
>> ---
>>  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(_##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(, 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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[wm4]

On Tue,  7 Mar 2017 16:01:58 +0700
Muhammad Faiz  wrote:

> use ff_thread_once
> 
> Suggested-by: wm4 
> Signed-off-by: Muhammad Faiz 
> ---
>  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(_##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(, 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.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Muhammad Faiz]

use ff_thread_once

Suggested-by: wm4 
Signed-off-by: Muhammad Faiz 
---
 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(_##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(, register_all);
+}
-- 
2.9.3

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel