On Sun, 16 Apr 2017 15:23:15 -0300 James Almer <jamr...@gmail.com> wrote:
> On 4/16/2017 2:17 PM, Aaron Levinson wrote: > > On 4/15/2017 9:32 PM, James Almer wrote: > >> On 4/15/2017 7:41 AM, Marton Balint wrote: > >>> > >>> On Thu, 13 Apr 2017, Aaron Levinson wrote: > >>> > >>>> On 4/13/2017 3:40 PM, Marton Balint wrote: > >>>>> > >>>>> On Thu, 13 Apr 2017, Aaron Levinson wrote: > >>>>> > >>>>>> On 4/13/2017 2:12 AM, Hendrik Leppkes wrote: > >>>>>>> On Thu, Apr 13, 2017 at 10:36 AM, Aaron Levinson > >>>>>>> <alevi...@aracnet.com> > >>>>>> wrote: > >>>>>>> Give it some time for the other changes to be reviewed by the people > >>>>>>> that actually know decklink itself, you can include that in any new > >>>>>>> versions of the patch then, no need to send one for that right now. > >>>>>>> > >>>>>>> - Hendrik > >>>>>> > >>>>>> Actually, the coding changes made to the decklink source files in this > >>>>>> patch have pretty much nothing to do with decklink itself, and anyone > >>>>>> with familiarity with semaphores and pthread could review those > >>>>>> changes. > >>>>>> In fact, all I really did was use already existing approaches for > >>>>>> replacing a semaphore with the combination of a condition variable, > >>>>>> mutex, and counter, and there are plenty of examples of how to do this > >>>>>> on the Web. > >>>>>> > >>>>> > >>>>> Yeah, the changes look fine, please send an updated patch, I will apply > >>>>> it after some final testing. > >>>>> > >>>>> Thanks, > >>>>> Marton > >>>> > >>>> Here's a new version of the patch with the pthreads dependency replaced > >>>> with threads. > >>>> > >>> > >>> Thanks, applied. > >> > >> Wouldn't it be simpler to add posix semaphore emulation to w32threads > >> and os2threads? > >> The former should be trivial, and probably even without the need to > >> use mutexes or conditional variables given there's CreateSemaphore > >> and ReleaseSemaphore for this purpose. Not sure about the latter. > > > > I addressed this in one of the commit log messages: > > Yes. I made the mistake of reading the commit message after > reading the code and writing a reply, sorry about that :P > > > > > ------------------------------------------------------------------------- > > > > -- libavdevice/decklink_enc.cpp: > > a) Eliminated include of pthread.h and semaphore.h. > > b) Replaced use of semaphore with the equivalent using a combination > > of a mutex, condition variable, and a counter > > (frames_buffer_available_spots). In theory, libavutil/thread.h and > > the associated code could have been modified instead to add > > cross-platform implementations of the sem_ functions, but an > > inspection of the ffmpeg source base indicates that there are only > > two cases in which semaphores are used (including this one that was > > replaced), so it was deemed to not be worth the effort. > > > > -------------------------------------------------------------------------- > > > > I considered this approach, but the thought of having to do this in > > os2threads.h pretty much dissuaded me. I had thought that OS/2 was pretty > > much dead, but it appears that I was wrong. There is a yearly conference, > > and a new version of OS/2 will soon be released. > > > > However, the two places in ffmpeg that the sem_ functions were/are used are > > likely not relevant to OS/2 anyway: > > > > -- DeckLink: Blackmagic only provides drivers/kernel modules for Windows, > > Linux, and OS/X > > -- avdevice/jack.c: This pertains to www.jackaudio.org, and at least based > > on what I can see at https://github.com/jackaudio/jack2, there doesn't > > appear to be any build configuration for OS/2. jack support also > > apparently needs sem_timedwait(). > > > > As such, someone could likely get away with just implementing the sem_ > > functions in w32threads.h. > > > > Aaron > > We have a developer that keeps os2threads.h up to date, so he > will probably add sem_t compat wrappers to it, if not now when > a module that works on OS/2 needs it. > > IMO, if you can and want then add win32 wrappers to w32threads > right now. The pthread_once wrapper was added for one use case > then as soon as it became available it started to see more use > elsewhere in the tree. The same might happen to Semaphores. Beware that semaphores don't work on OSX. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
