[FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency

[Aaron Levinson]

On 4/16/2017 12:15 PM, wm4 wrote:
> On Sun, 16 Apr 2017 15:23:15 -0300
> James Almer <jamrial at 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 <alevinsn at 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.

I believe that semaphores on OS X are handled via dispatch semaphore. 
See instances of "dispatch" in configure and compat/dispatch_semaphore.

Aaron