Re: [FFmpeg-devel] [PATCH v3 1/3] lavc/utils.c: use C11 atomics for entangled thread handling
On Sat, Nov 25, 2017 at 11:05:22PM -0300, James Almer wrote: > On 11/25/2017 10:56 PM, Michael Niedermayer wrote: > > On Sat, Nov 25, 2017 at 05:01:55PM +, Rostislav Pehlivanov wrote: > >> Signed-off-by: Rostislav Pehlivanov > >> --- > >> libavcodec/utils.c | 9 + > >> 1 file changed, 5 insertions(+), 4 deletions(-) > > > > LGTM > > > > The whole lock manager should possibly be simplified using some > > more standard mutex stuff from C11 or use that as default. > > > > thx > > If you mean threads.h stuff, afaik it's optional and glibc doesn't > include it, so hardly an option today. that is sad [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB While the State exists there can be no freedom; when there is freedom there will be no State. -- Vladimir Lenin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 1/3] lavc/utils.c: use C11 atomics for entangled thread handling
On 26 November 2017 at 01:56, Michael Niedermayer wrote: > On Sat, Nov 25, 2017 at 05:01:55PM +, Rostislav Pehlivanov wrote: > > Signed-off-by: Rostislav Pehlivanov > > --- > > libavcodec/utils.c | 9 + > > 1 file changed, 5 insertions(+), 4 deletions(-) > > LGTM > > The whole lock manager should possibly be simplified using some > more standard mutex stuff from C11 or use that as default. > > thx > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Let us carefully observe those good qualities wherein our enemies excel us > and endeavor to excel them, by avoiding what is faulty, and imitating what > is excellent in them. -- Plutarch > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > pushed, thanks ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 1/3] lavc/utils.c: use C11 atomics for entangled thread handling
On 11/25/2017 10:56 PM, Michael Niedermayer wrote: > On Sat, Nov 25, 2017 at 05:01:55PM +, Rostislav Pehlivanov wrote: >> Signed-off-by: Rostislav Pehlivanov >> --- >> libavcodec/utils.c | 9 + >> 1 file changed, 5 insertions(+), 4 deletions(-) > > LGTM > > The whole lock manager should possibly be simplified using some > more standard mutex stuff from C11 or use that as default. > > thx If you mean threads.h stuff, afaik it's optional and glibc doesn't include it, so hardly an option today. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 1/3] lavc/utils.c: use C11 atomics for entangled thread handling
On Sat, Nov 25, 2017 at 05:01:55PM +, Rostislav Pehlivanov wrote: > Signed-off-by: Rostislav Pehlivanov > --- > libavcodec/utils.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) LGTM The whole lock manager should possibly be simplified using some more standard mutex stuff from C11 or use that as default. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Let us carefully observe those good qualities wherein our enemies excel us and endeavor to excel them, by avoiding what is faulty, and imitating what is excellent in them. -- Plutarch signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 1/3] lavc/utils.c: use C11 atomics for entangled thread handling
On 25 November 2017 at 18:37, James Almer wrote: > On 11/25/2017 2:01 PM, Rostislav Pehlivanov wrote: > > Signed-off-by: Rostislav Pehlivanov > > --- > > libavcodec/utils.c | 9 + > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > > index e50de6e89b..3a0f3c11f5 100644 > > --- a/libavcodec/utils.c > > +++ b/libavcodec/utils.c > > @@ -56,6 +56,7 @@ > > #include "version.h" > > #include > > #include > > +#include > > #include > > #include > > #if CONFIG_ICONV > > @@ -114,7 +115,7 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp > op) = NULL; > > > > > > volatile int ff_avcodec_locked; > > -static int volatile entangled_thread_counter = 0; > > +static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0); > > static void *codec_mutex; > > static void *avformat_mutex; > > > > @@ -1944,11 +1945,11 @@ int ff_lock_avcodec(AVCodecContext *log_ctx, > const AVCodec *codec) > > return -1; > > } > > > > -if (avpriv_atomic_int_add_and_fetch(&entangled_thread_counter, 1) > != 1) { > > +if (atomic_fetch_add(&entangled_thread_counter, 1)) { > > av_log(log_ctx, AV_LOG_ERROR, > > "Insufficient thread locking. At least %d threads are " > > "calling avcodec_open2() at the same time right now.\n", > > - entangled_thread_counter); > > + atomic_load(&entangled_thread_counter)); > > if (!lockmgr_cb) > > av_log(log_ctx, AV_LOG_ERROR, "No lock manager is set, > please see av_lockmgr_register()\n"); > > ff_avcodec_locked = 1; > > @@ -1967,7 +1968,7 @@ int ff_unlock_avcodec(const AVCodec *codec) > > > > av_assert0(ff_avcodec_locked); > > ff_avcodec_locked = 0; > > -avpriv_atomic_int_add_and_fetch(&entangled_thread_counter, -1); > > +atomic_fetch_add(&entangled_thread_counter, -1); > > Nit: You could use atomic_fetch_sub() instead of adding -1. > > > if (lockmgr_cb) { > > if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE)) > > return -1; > > > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > Fixed locally ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v3 1/3] lavc/utils.c: use C11 atomics for entangled thread handling
On 11/25/2017 2:01 PM, Rostislav Pehlivanov wrote: > Signed-off-by: Rostislav Pehlivanov > --- > libavcodec/utils.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index e50de6e89b..3a0f3c11f5 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -56,6 +56,7 @@ > #include "version.h" > #include > #include > +#include > #include > #include > #if CONFIG_ICONV > @@ -114,7 +115,7 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) > = NULL; > > > volatile int ff_avcodec_locked; > -static int volatile entangled_thread_counter = 0; > +static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0); > static void *codec_mutex; > static void *avformat_mutex; > > @@ -1944,11 +1945,11 @@ int ff_lock_avcodec(AVCodecContext *log_ctx, const > AVCodec *codec) > return -1; > } > > -if (avpriv_atomic_int_add_and_fetch(&entangled_thread_counter, 1) != 1) { > +if (atomic_fetch_add(&entangled_thread_counter, 1)) { > av_log(log_ctx, AV_LOG_ERROR, > "Insufficient thread locking. At least %d threads are " > "calling avcodec_open2() at the same time right now.\n", > - entangled_thread_counter); > + atomic_load(&entangled_thread_counter)); > if (!lockmgr_cb) > av_log(log_ctx, AV_LOG_ERROR, "No lock manager is set, please > see av_lockmgr_register()\n"); > ff_avcodec_locked = 1; > @@ -1967,7 +1968,7 @@ int ff_unlock_avcodec(const AVCodec *codec) > > av_assert0(ff_avcodec_locked); > ff_avcodec_locked = 0; > -avpriv_atomic_int_add_and_fetch(&entangled_thread_counter, -1); > +atomic_fetch_add(&entangled_thread_counter, -1); Nit: You could use atomic_fetch_sub() instead of adding -1. > if (lockmgr_cb) { > if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE)) > return -1; > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v3 1/3] lavc/utils.c: use C11 atomics for entangled thread handling
Signed-off-by: Rostislav Pehlivanov --- libavcodec/utils.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libavcodec/utils.c b/libavcodec/utils.c index e50de6e89b..3a0f3c11f5 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -56,6 +56,7 @@ #include "version.h" #include #include +#include #include #include #if CONFIG_ICONV @@ -114,7 +115,7 @@ static int (*lockmgr_cb)(void **mutex, enum AVLockOp op) = NULL; volatile int ff_avcodec_locked; -static int volatile entangled_thread_counter = 0; +static atomic_int entangled_thread_counter = ATOMIC_VAR_INIT(0); static void *codec_mutex; static void *avformat_mutex; @@ -1944,11 +1945,11 @@ int ff_lock_avcodec(AVCodecContext *log_ctx, const AVCodec *codec) return -1; } -if (avpriv_atomic_int_add_and_fetch(&entangled_thread_counter, 1) != 1) { +if (atomic_fetch_add(&entangled_thread_counter, 1)) { av_log(log_ctx, AV_LOG_ERROR, "Insufficient thread locking. At least %d threads are " "calling avcodec_open2() at the same time right now.\n", - entangled_thread_counter); + atomic_load(&entangled_thread_counter)); if (!lockmgr_cb) av_log(log_ctx, AV_LOG_ERROR, "No lock manager is set, please see av_lockmgr_register()\n"); ff_avcodec_locked = 1; @@ -1967,7 +1968,7 @@ int ff_unlock_avcodec(const AVCodec *codec) av_assert0(ff_avcodec_locked); ff_avcodec_locked = 0; -avpriv_atomic_int_add_and_fetch(&entangled_thread_counter, -1); +atomic_fetch_add(&entangled_thread_counter, -1); if (lockmgr_cb) { if ((*lockmgr_cb)(&codec_mutex, AV_LOCK_RELEASE)) return -1; -- 2.15.0.417.g466bffb3ac ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel