Message ID | 20180105014044.2132-1-jamrial@gmail.com |
---|---|
State | Accepted |
Commit | 9ed4ebc530dde681943389b1f97db90b546ad38d |
Headers | show |
On Thu, 4 Jan 2018 22:40:44 -0300 James Almer <jamrial@gmail.com> wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/utils.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index dfbfe98d63..4d736d2e7d 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" > @@ -127,17 +126,24 @@ 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(); > + > + ff_mutex_lock(&codec_register_mutex); > p = last_avcodec; > - codec->next = NULL; > > - while(*p || avpriv_atomic_ptr_cas((void * volatile *)p, NULL, codec)) > + while (*p) > p = &(*p)->next; > + *p = codec; > + codec->next = NULL; > last_avcodec = &codec->next; > > + ff_mutex_unlock(&codec_register_mutex); > + > if (codec->init_static_data) > codec->init_static_data(codec); > } Seems good, but should init_static_data really be outside of the lock? (Sure, it wasn't under a look before either, but still.)
On 1/5/2018 1:02 PM, wm4 wrote: > On Thu, 4 Jan 2018 22:40:44 -0300 > James Almer <jamrial@gmail.com> wrote: > >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/utils.c | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c >> index dfbfe98d63..4d736d2e7d 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" >> @@ -127,17 +126,24 @@ 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(); >> + >> + ff_mutex_lock(&codec_register_mutex); >> p = last_avcodec; >> - codec->next = NULL; >> >> - while(*p || avpriv_atomic_ptr_cas((void * volatile *)p, NULL, codec)) >> + while (*p) >> p = &(*p)->next; >> + *p = codec; >> + codec->next = NULL; >> last_avcodec = &codec->next; >> >> + ff_mutex_unlock(&codec_register_mutex); >> + >> if (codec->init_static_data) >> codec->init_static_data(codec); >> } > > Seems good, but should init_static_data really be outside of the lock? > (Sure, it wasn't under a look before either, but still.) I don't think calling avcodec_register() twice with the same AVCodec is defined or supported at all, so outside of creating the linked list i don't see any other potential race here. But i may be wrong. In any case, that's something for a separate patch as it's a change in behavior (and may make things slower), so pushed as is.
diff --git a/libavcodec/utils.c b/libavcodec/utils.c index dfbfe98d63..4d736d2e7d 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" @@ -127,17 +126,24 @@ 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(); + + ff_mutex_lock(&codec_register_mutex); p = last_avcodec; - codec->next = NULL; - while(*p || avpriv_atomic_ptr_cas((void * volatile *)p, NULL, codec)) + while (*p) p = &(*p)->next; + *p = codec; + codec->next = NULL; last_avcodec = &codec->next; + ff_mutex_unlock(&codec_register_mutex); + if (codec->init_static_data) codec->init_static_data(codec); }
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/utils.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)