diff mbox

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

Message ID c890bd1e-f926-f279-ee2f-216ea1ca6a46@aracnet.com
State Superseded
Headers show

Commit Message

Aaron Levinson April 13, 2017, 8:36 a.m. UTC
On 4/13/2017 1:21 AM, Hendrik Leppkes wrote:
> On Thu, Apr 13, 2017 at 5:32 AM, Aaron Levinson <alevinsn@aracnet.com> wrote:
>> diff --git a/configure b/configure
>> index b0f7b1a..adb0060 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2992,9 +2992,9 @@ avfoundation_indev_deps="pthreads"
>>  avfoundation_indev_extralibs="-framework Foundation -framework AVFoundation -framework CoreVideo -framework CoreMedia"
>>  bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"
>>  caca_outdev_deps="libcaca"
>> -decklink_indev_deps="decklink pthreads"
>> +decklink_indev_deps="decklink"
>>  decklink_indev_extralibs="-lstdc++"
>> -decklink_outdev_deps="decklink pthreads"
>> +decklink_outdev_deps="decklink"
>
> You should probably still have a dependency on "threads" (a combined
> dep for any threading support), or does the device work without any
> threading support now?

That makes sense, and I think I follow how the threads dependency is 
generated in configure.  Should I generate an entirely new patch?  The 
only difference would be the following, assuming all is okay with the 
actual code changes:

  dshow_indev_deps="IBaseFilter"
  dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 
-lshlwapi"

Thanks,
Aaron Levinson

Comments

Hendrik Leppkes April 13, 2017, 9:12 a.m. UTC | #1
On Thu, Apr 13, 2017 at 10:36 AM, Aaron Levinson <alevinsn@aracnet.com> wrote:
> On 4/13/2017 1:21 AM, Hendrik Leppkes wrote:
>>
>> On Thu, Apr 13, 2017 at 5:32 AM, Aaron Levinson <alevinsn@aracnet.com>
>> wrote:
>>>
>>> diff --git a/configure b/configure
>>> index b0f7b1a..adb0060 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2992,9 +2992,9 @@ avfoundation_indev_deps="pthreads"
>>>  avfoundation_indev_extralibs="-framework Foundation -framework
>>> AVFoundation -framework CoreVideo -framework CoreMedia"
>>>  bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h
>>> dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"
>>>  caca_outdev_deps="libcaca"
>>> -decklink_indev_deps="decklink pthreads"
>>> +decklink_indev_deps="decklink"
>>>  decklink_indev_extralibs="-lstdc++"
>>> -decklink_outdev_deps="decklink pthreads"
>>> +decklink_outdev_deps="decklink"
>>
>>
>> You should probably still have a dependency on "threads" (a combined
>> dep for any threading support), or does the device work without any
>> threading support now?
>
>
> That makes sense, and I think I follow how the threads dependency is
> generated in configure.  Should I generate an entirely new patch?  The only
> difference would be the following, assuming all is okay with the actual code
> changes:
>

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
Aaron Levinson April 13, 2017, 8:12 p.m. UTC | #2
On 4/13/2017 2:12 AM, Hendrik Leppkes wrote:
> On Thu, Apr 13, 2017 at 10:36 AM, Aaron Levinson <alevinsn@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.

Aaron
Marton Balint April 13, 2017, 10:40 p.m. UTC | #3
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@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
diff mbox

Patch

diff --git a/configure b/configure
index b0f7b1a..adb0060 100755
--- a/configure
+++ b/configure
@@ -2992,9 +2992,9 @@  avfoundation_indev_deps="pthreads"
  avfoundation_indev_extralibs="-framework Foundation -framework 
AVFoundation -framework CoreVideo -framework CoreMedia"
  bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h 
dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"
  caca_outdev_deps="libcaca"
-decklink_indev_deps="decklink pthreads"
+decklink_indev_deps="decklink threads"
  decklink_indev_extralibs="-lstdc++"
-decklink_outdev_deps="decklink pthreads"
+decklink_outdev_deps="decklink threads"
  decklink_outdev_extralibs="-lstdc++"