On Mon, 04 Mar 2013 14:16:11 +0100, Diego Biurrun <[email protected]> wrote: > On Mon, Mar 04, 2013 at 11:35:04AM +0100, Anton Khirnov wrote: > > From: "Ronald S. Bultje" <[email protected]> > > > > These could be used for reference counting, or for keeping track of > > decoding progress in references in multithreaded decoders. > > > > Support is provided by gcc/msvc/suncc intrinsics, with a fallback using > > pthread mutexes. > > Why not use the fallback in all cases? Performance? >
Yes. The atomic intrinsics are MUCH faster than a mutex > > --- a/configure > > +++ b/configure > > @@ -3440,6 +3443,10 @@ check_func strerror_r > > check_func strtok_r > > check_func sched_getaffinity > > +# cannot use check_func, because those are builtins > > +check_code ld "" "__sync_synchronize()" && enable sync_synchronize > > +check_code ld mbarrier.h "__machine_rw_barrier()" && enable > > machine_rw_barrier > > +check_code ld windows.h "MemoryBarrier()" && enable MemoryBarrier > > What is the problem with check_func? It declares the checked function as extern int $func(); This does not work for those builtins, since they are not functions. > > > --- a/libavutil/Makefile > > +++ b/libavutil/Makefile > > @@ -100,6 +101,7 @@ SKIPHEADERS = old_pix_fmts.h > > > > TESTPROGS = adler32 \ > > aes \ > > + atomic \ > > avstring \ > > base64 \ > > If you add a test program, add a FATE test as well. > Added > > --- /dev/null > > +++ b/libavutil/atomic.c > > @@ -0,0 +1,120 @@ > > + > > +#include "atomic.h" > > + > > +#if !HAVE_MEMORYBARRIER && !HAVE_SYNC_SYNCHRONIZE && > > !HAVE_MACHINE_RW_BARRIER > > + > > +#if HAVE_PTHREADS > > + > > +#else > > +// assume there is no threading at all, noop implementations of everything > > That assumption would seem to be wrong. We support more than just the > pthreads thread implementation. > > > +#endif // HAVE_PTHREADS > > + > > +#endif > > Please /* */ comment all the #endifs. > Fixed > > +#ifdef TEST > > +#include <assert.h> > > + > > +int main(int argc, char *argv[]) > > The arguments are unused. > Changed to void > > --- /dev/null > > +++ b/libavutil/atomic.h > > @@ -0,0 +1,75 @@ > > + > > +/** > > + * Loads the current value stored in an atomic integer. > > Load > > > + * @note this acts as a memory barrier > > Capitalize, end in a period. > > > +/** > > + * Stores a new value in an atomic integer. > > Store > > > + * @param ptr atomic integer > > + * @param val the value to store in the atomic integer > > + * @note this acts as a memory barrier > > Capitalize, end in a period. > > > +/** > > + * Adds up a value to an atomic integer. > > Add > > What do you mean by "add up" here - I assume it should be plain "add"? > Right, changed to just 'Add a value...' > > + * @param ptr atomic integer > > + * @param inc the value to add up to the atomic integer (may be > > + * negative) > > same > > > + * @note this does NOT act as a memory barrier. This is primarily > > + * intended for reference counting. > > Capitalize, end in a period. > All the above fixed > > +/** > > + * Atomic pointer compare and swap. > > + * > > + * @param ptr pointer to the pointer to operate on > > + * @param oldval do the swap if the current value of *ptr equals to oldval > > + * @param newval value to replace *ptr with > > + * @return the value of *ptr before comparison > > + */ > > +void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void > > *newval); > > This is a void function, why the @return? It is not. -- Anton Khirnov _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
