[FFmpeg-devel,1/4] avcodec/util: use a mutex instead of atomics in avcodec_register()

Submitted by James Almer on Jan. 4, 2018, 6:19 p.m.

Details

Message ID 20180104181904.5816-1-jamrial@gmail.com
State Superseded
Headers show

Commit Message

James Almer Jan. 4, 2018, 6:19 p.m.
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/utils.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Michael Niedermayer Jan. 4, 2018, 11:52 p.m.
On Thu, Jan 04, 2018 at 03:19:01PM -0300, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/utils.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index dfbfe98d63..61acf36c4b 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -26,7 +26,6 @@
>   */
>  
>  #include "config.h"
> -#include "libavutil/atomic.h"
>  #include "libavutil/attributes.h"
>  #include "libavutil/avassert.h"
>  #include "libavutil/avstring.h"
> @@ -95,7 +94,6 @@ void av_fast_padded_mallocz(void *ptr, unsigned int *size, size_t min_size)
>  
>  /* encoder management */
>  static AVCodec *first_avcodec = NULL;
> -static AVCodec **last_avcodec = &first_avcodec;
>  
>  AVCodec *av_codec_next(const AVCodec *c)
>  {
> @@ -127,16 +125,21 @@ int av_codec_is_decoder(const AVCodec *codec)
>      return codec && (codec->decode || codec->receive_frame);
>  }
>  
> +static AVMutex codec_register_mutex = AV_MUTEX_INITIALIZER;
> +
>  av_cold void avcodec_register(AVCodec *codec)
>  {
>      AVCodec **p;
>      avcodec_init();
> -    p = last_avcodec;
> -    codec->next = NULL;
> +    ff_mutex_lock(&codec_register_mutex);
> +    p = &first_avcodec;
>  
> -    while(*p || avpriv_atomic_ptr_cas((void * volatile *)p, NULL, codec))
> +    while (*p)
>          p = &(*p)->next;
> -    last_avcodec = &codec->next;
> +    *p          = codec;
> +    codec->next = NULL;
> +
> +    ff_mutex_unlock(&codec_register_mutex);

the code should keep track of the last entry of the list as previously and not
search through the list each time for the end, especially not under a mutex
this is not as negligible fast as you may think 
adding 1000 entries to a list like this will need ~ half millon iterations

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index dfbfe98d63..61acf36c4b 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -26,7 +26,6 @@ 
  */
 
 #include "config.h"
-#include "libavutil/atomic.h"
 #include "libavutil/attributes.h"
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
@@ -95,7 +94,6 @@  void av_fast_padded_mallocz(void *ptr, unsigned int *size, size_t min_size)
 
 /* encoder management */
 static AVCodec *first_avcodec = NULL;
-static AVCodec **last_avcodec = &first_avcodec;
 
 AVCodec *av_codec_next(const AVCodec *c)
 {
@@ -127,16 +125,21 @@  int av_codec_is_decoder(const AVCodec *codec)
     return codec && (codec->decode || codec->receive_frame);
 }
 
+static AVMutex codec_register_mutex = AV_MUTEX_INITIALIZER;
+
 av_cold void avcodec_register(AVCodec *codec)
 {
     AVCodec **p;
     avcodec_init();
-    p = last_avcodec;
-    codec->next = NULL;
+    ff_mutex_lock(&codec_register_mutex);
+    p = &first_avcodec;
 
-    while(*p || avpriv_atomic_ptr_cas((void * volatile *)p, NULL, codec))
+    while (*p)
         p = &(*p)->next;
-    last_avcodec = &codec->next;
+    *p          = codec;
+    codec->next = NULL;
+
+    ff_mutex_unlock(&codec_register_mutex);
 
     if (codec->init_static_data)
         codec->init_static_data(codec);