Lukas Fellechner: >> Gesendet: Montag, 05. September 2022 um 00:50 Uhr >> Von: "Andreas Rheinhardt" <andreas.rheinha...@outlook.com> >> An: ffmpeg-devel@ffmpeg.org >> Betreff: Re: [FFmpeg-devel] [PATCH v3 2/3] lavf/dashdec: Multithreaded DASH >> initialization >> Lukas Fellechner: >>> Andreas Rheinhardt andreas.rheinhardt at outlook.com >>> Wed Aug 31 05:54:12 EEST 2022 >>>> >>> >>>> 1. We actually have an API to process multiple tasks by different >>>> threads: Look at libavutil/slicethread.h. Why can't you reuse that? >>>> 2. In case initialization of one of the conditions/mutexes fails, you >>>> are nevertheless destroying them; you are even destroying completely >>>> uninitialized mutexes. This is undefined behaviour. Checking the result >>>> of it does not fix this. >>>> >>>> - Andreas >>> >>> 1. The slicethread implementation is pretty hard to understand. >>> I was not sure if it can be used for normal parallelization, because >>> it looked more like some kind of thread pool for continuous data >>> processing. But after taking a second look, I think I can use it here. >>> I will try and see if it works well. >>> >>> 2. I was not aware that this is undefined behavior. Will switch to >>> PTHREAD_MUTEX_INITIALIZER and PTHREAD_COND_INITIALIZER macros, >>> which return a safely initialized mutex/cond. >>> >> >> "The behavior is undefined if the value specified by the mutex argument >> to pthread_mutex_destroy() does not refer to an initialized mutex." >> (From >> https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_destroy.html) >> >> Furthermore: "In cases where default mutex attributes are appropriate, >> the macro PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes. >> The effect shall be equivalent to dynamic initialization by a call to >> pthread_mutex_init() with parameter attr specified as NULL, except that >> no error checks are performed." The last sentence sounds as if one would >> then have to check mutex locking. >> >> Moreover, older pthread standards did not allow to use >> PTHREAD_MUTEX_INITIALIZER with non-static mutexes, so I don't know >> whether we can use that. Also our pthreads-wrapper on top of >> OS/2-threads does not provide PTHREAD_COND_INITIALIZER (which is used >> nowhere in the codebase). > > I missed that detail about the initializer macro. Thank you for clearing > that up. > > After looking more into threads implementation in ffmpeg, I wonder if I > really need to check any results of init/destroy or other functions. > In slicethreads.c, there is zero checking on any of the lock functions. > The pthreads-based implementation does internally check the results of all > function calls and calls abort() in case of errors ("strict_" wrappers). > The Win32 implementation uses SRW locks which cannot even return errors. > And the OS2 implementation returns 0 on all calls as well. > > So right now, I think that I should continue with normal _init() calls > (no macros) and drop all error checking, just like slicethread does. > Are you fine with that approach?
Zero checking is our old approach; the new approach checks for errors and ensures that only mutexes/condition variables that have been properly initialized are destroyed. See ff_pthread_init/free in libavcodec/pthread.c (you can't use this in libavformat, because these functions are local to libavcodec). - Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
