| Message ID | 91cb5c55-fdb5-460b-c640-0b11255fd2fd@aracnet.com |
|---|---|
| State | Accepted |
| Headers | show |
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@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. Regards, Marton
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@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.
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@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: ------------------------------------------------------------------------- -- 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
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@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.
On Sun, 16 Apr 2017 15:23:15 -0300 James Almer <jamrial@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@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.
On 4/16/2017 12:15 PM, wm4 wrote: > On Sun, 16 Apr 2017 15:23:15 -0300 > James Almer <jamrial@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@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
> Thanks, applied. > > Regards, > Marton This seems to have broken the coverity builds: https://travis-ci.org/FFmpeg/FFmpeg-Coverity/builds/222597943#L1103
On 4/16/2017 1:33 PM, Timo Rothenpieler wrote: >> Thanks, applied. >> >> Regards, >> Marton > > This seems to have broken the coverity builds: > https://travis-ci.org/FFmpeg/FFmpeg-Coverity/builds/222597943#L1103 It was suggested on IRC by James Almer that "it seems to complain about ubitux's 'strict' pthread implementation (for assert levels above 0). Maybe it needs to be disabled for C++ sources." Aaron
On Mon, Apr 17, 2017 at 12:26 AM, Aaron Levinson <alevinsn@aracnet.com> wrote: > On 4/16/2017 1:33 PM, Timo Rothenpieler wrote: >>> >>> Thanks, applied. >>> >>> Regards, >>> Marton >> >> >> This seems to have broken the coverity builds: >> https://travis-ci.org/FFmpeg/FFmpeg-Coverity/builds/222597943#L1103 > > > It was suggested on IRC by James Almer that "it seems to complain about > ubitux's 'strict' pthread implementation (for assert levels above 0). Maybe > it needs to be disabled for C++ sources." > The real problem is in av_err2str, which uses a temporary array, which is either not valid in C++ (likely), or not supported by MSVC for reasons (although in contrast to C support, its C++ support is generally pretty good). But considering how little C++ code we have, its probably best to somehow avoid using it instead of trying to fix it. - Hendrik
On 4/16/2017 3:31 PM, Hendrik Leppkes wrote: > On Mon, Apr 17, 2017 at 12:26 AM, Aaron Levinson <alevinsn@aracnet.com> wrote: >> On 4/16/2017 1:33 PM, Timo Rothenpieler wrote: >>>> >>>> Thanks, applied. >>>> >>>> Regards, >>>> Marton >>> >>> >>> This seems to have broken the coverity builds: >>> https://travis-ci.org/FFmpeg/FFmpeg-Coverity/builds/222597943#L1103 >> >> >> It was suggested on IRC by James Almer that "it seems to complain about >> ubitux's 'strict' pthread implementation (for assert levels above 0). Maybe >> it needs to be disabled for C++ sources." >> > > The real problem is in av_err2str, which uses a temporary array, which > is either not valid in C++ (likely), or not supported by MSVC for > reasons (although in contrast to C support, its C++ support is > generally pretty good). > But considering how little C++ code we have, its probably best to > somehow avoid using it instead of trying to fix it. > > - Hendrik I don't think this has much to do with MSVC here, because coverity is doing a gcc/g++ build. I suspect that it is failing with any gcc/g++ build because of the patch. I'll work on this now. Aaron
On 4/16/2017 4:25 PM, Aaron Levinson wrote: > On 4/16/2017 3:31 PM, Hendrik Leppkes wrote: >> On Mon, Apr 17, 2017 at 12:26 AM, Aaron Levinson >> <alevinsn@aracnet.com> wrote: >>> On 4/16/2017 1:33 PM, Timo Rothenpieler wrote: >>>>> >>>>> Thanks, applied. >>>>> >>>>> Regards, >>>>> Marton >>>> >>>> >>>> This seems to have broken the coverity builds: >>>> https://travis-ci.org/FFmpeg/FFmpeg-Coverity/builds/222597943#L1103 >>> >>> >>> It was suggested on IRC by James Almer that "it seems to complain about >>> ubitux's 'strict' pthread implementation (for assert levels above 0). >>> Maybe >>> it needs to be disabled for C++ sources." >>> >> >> The real problem is in av_err2str, which uses a temporary array, which >> is either not valid in C++ (likely), or not supported by MSVC for >> reasons (although in contrast to C support, its C++ support is >> generally pretty good). >> But considering how little C++ code we have, its probably best to >> somehow avoid using it instead of trying to fix it. >> >> - Hendrik > > I don't think this has much to do with MSVC here, because coverity is > doing a gcc/g++ build. I suspect that it is failing with any gcc/g++ > build because of the patch. I'll work on this now. > > Aaron The build issue occurs with g++ and an assert level greater than 1, although it would also occur with MSVC++ if pthreads is enabled. I will submit a separate patch e-mail that fixes the build error. Aaron
diff --git a/configure b/configure index 0af4a29..18d79ab 100755 --- a/configure +++ b/configure @@ -3000,9 +3000,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++" dshow_indev_deps="IBaseFilter" dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi" diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp index 587a321..523217c 100644 --- a/libavdevice/decklink_common.cpp +++ b/libavdevice/decklink_common.cpp @@ -35,9 +35,6 @@ extern "C" { #include <DeckLinkAPIDispatch.cpp> #endif -#include <pthread.h> -#include <semaphore.h> - extern "C" { #include "libavformat/avformat.h" #include "libavutil/imgutils.h" diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h index 4753287..c12cf18 100644 --- a/libavdevice/decklink_common.h +++ b/libavdevice/decklink_common.h @@ -24,6 +24,7 @@ #include <DeckLinkAPIVersion.h> +#include "libavutil/thread.h" #include "decklink_common_c.h" class decklink_output_callback; @@ -89,7 +90,9 @@ struct decklink_ctx { int frames_preroll; int frames_buffer; - sem_t semaphore; + pthread_mutex_t mutex; + pthread_cond_t cond; + int frames_buffer_available_spots; int channels; }; diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp index f219d27..a663029 100644 --- a/libavdevice/decklink_dec.cpp +++ b/libavdevice/decklink_dec.cpp @@ -25,9 +25,6 @@ extern "C" { #include <DeckLinkAPI.h> -#include <pthread.h> -#include <semaphore.h> - extern "C" { #include "config.h" #include "libavformat/avformat.h" diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp index 4881cf2..3bd6357 100644 --- a/libavdevice/decklink_enc.cpp +++ b/libavdevice/decklink_enc.cpp @@ -28,9 +28,6 @@ extern "C" { #include <DeckLinkAPI.h> -#include <pthread.h> -#include <semaphore.h> - extern "C" { #include "libavformat/avformat.h" #include "libavutil/imgutils.h" @@ -94,7 +91,10 @@ public: av_frame_unref(avframe); - sem_post(&ctx->semaphore); + pthread_mutex_lock(&ctx->mutex); + ctx->frames_buffer_available_spots++; + pthread_cond_broadcast(&ctx->cond); + pthread_mutex_unlock(&ctx->mutex); return S_OK; } @@ -136,7 +136,6 @@ static int decklink_setup_video(AVFormatContext *avctx, AVStream *st) ctx->output_callback = new decklink_output_callback(); ctx->dlo->SetScheduledFrameCompletionCallback(ctx->output_callback); - /* Start video semaphore. */ ctx->frames_preroll = st->time_base.den * ctx->preroll; if (st->time_base.den > 1000) ctx->frames_preroll /= 1000; @@ -144,7 +143,9 @@ static int decklink_setup_video(AVFormatContext *avctx, AVStream *st) /* Buffer twice as many frames as the preroll. */ ctx->frames_buffer = ctx->frames_preroll * 2; ctx->frames_buffer = FFMIN(ctx->frames_buffer, 60); - sem_init(&ctx->semaphore, 0, ctx->frames_buffer); + pthread_mutex_init(&ctx->mutex, NULL); + pthread_cond_init(&ctx->cond, NULL); + ctx->frames_buffer_available_spots = ctx->frames_buffer; /* The device expects the framerate to be fixed. */ avpriv_set_pts_info(st, 64, st->time_base.num, st->time_base.den); @@ -214,7 +215,8 @@ av_cold int ff_decklink_write_trailer(AVFormatContext *avctx) if (ctx->output_callback) delete ctx->output_callback; - sem_destroy(&ctx->semaphore); + pthread_mutex_destroy(&ctx->mutex); + pthread_cond_destroy(&ctx->cond); av_freep(&cctx->ctx); @@ -250,7 +252,12 @@ static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt) } /* Always keep at most one second of frames buffered. */ - sem_wait(&ctx->semaphore); + pthread_mutex_lock(&ctx->mutex); + while (ctx->frames_buffer_available_spots == 0) { + pthread_cond_wait(&ctx->cond, &ctx->mutex); + } + ctx->frames_buffer_available_spots--; + pthread_mutex_unlock(&ctx->mutex); /* Schedule frame for playback. */ hr = ctx->dlo->ScheduleVideoFrame((struct IDeckLinkVideoFrame *) frame,