[FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency. Made minor changes to get the decklink avdevice code to build using Visual C++.
Marton Balint
cus at passwd.hu
Thu Apr 13 04:51:54 EEST 2017
On Wed, 12 Apr 2017, Aaron Levinson wrote:
> From bcc0ec4a0bb64e74b9c9369087708f55569bf50e Mon Sep 17 00:00:00 2001
> From: Aaron Levinson <alevinsn at aracnet.com>
> Date: Wed, 12 Apr 2017 16:41:53 -0700
> Subject: [PATCH] avdevice/decklink: Removed pthread dependency. Made minor changes to get the decklink avdevice code to build using Visual C++.
>
> Purpose: avdevice/decklink: Removed pthread dependency by
> replacing semaphore used in code appropriately. Doing so makes it easier to
> build ffmpeg using Visual C++ on Windows. In addition, made other minor
> changes to get the decklink avdevice code to build using Visual C++. This is
> a contination of Kyle Schwarz's "avdevice/decklink: Remove pthread
> dependency" patch that is available at
> https://patchwork.ffmpeg.org/patch/2654/ . This patch wasn't accepted, and
> as far as I can tell, there was no follow-up after it was rejected.
>
> Notes: Used Visual Studio 2015 (with update 3) for this.
Could you split this patch into two separete patches, one that removes the
pthread dependancy, and another which fixes MSVC build issues?
>
> Comments:
>
> -- configure:
>
> a) Eliminated pthreads dependency for decklink_indev_deps and
> decklink_outdev_deps
> b) For MSVC (Microsoft Visual C++) builds, now reinitializing
> decklink_indev_extralibs and decklink_outdev_extralibs to empty and
> then later, following the similar procedure used for mingw by
> adding -lole32 and -loleaut32. This is necessary in order for it
> to link properly when building with Visual C++.
>
> -- libavdevice/decklink_common.cpp / .h:
> a) Eliminated semaphore and replaced with a combination of a mutex,
> condition variable, and a counter (frames_buffer_available_spots).
> b) Switched the order of the include of libavformat/internal.h to
> workaround build issues with Visual C++. See comment in
> decklink_common.cpp for more details.
> c) Removed include of pthread.h and semaphore.h and now using
> libavutil/thread.h instead.
>
> -- libavdevice/decklink_dec.cpp:
> a) Eliminated include of pthread.h and semaphore.h.
> b) Rearranged the include of libavformat/internal.h (for reasons as
> described above).
> c) Made slight alteration to an argument for call to av_rescale_q() to
> workaround a compiler error with Visual C++. This appears to only
> be an issue when building C++ files with Visual C++. See comments
> in code for more details.
>
> -- libavdevice/decklink_enc.cpp:
> a) Eliminated include of pthread.h and semaphore.h.
> b) Rearranged the include of libavformat/internal.h (for reasons as
> described above).
> c) 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.
> ---
> configure | 8 ++++++--
> libavdevice/decklink_common.cpp | 13 +++++++++----
> libavdevice/decklink_common.h | 5 ++++-
> libavdevice/decklink_dec.cpp | 20 +++++++++++++++-----
> libavdevice/decklink_enc.cpp | 27 +++++++++++++++++++--------
> 5 files changed, 53 insertions(+), 20 deletions(-)
>
> diff --git a/configure b/configure
> index b0f7b1a..5b76a33 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"
> decklink_outdev_extralibs="-lstdc++"
> dshow_indev_deps="IBaseFilter"
> dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi"
> @@ -3646,6 +3646,8 @@ case "$toolchain" in
> ld_default="$source_path/compat/windows/mslink"
> nm_default="dumpbin -symbols"
> ar_default="lib"
> + decklink_indev_extralibs=""
> + decklink_outdev_extralibs=""
> case "$arch" in
> arm*)
> as_default="armasm"
> @@ -4902,6 +4904,8 @@ case $target_os in
> fi
> enabled x86_32 && check_ldflags -LARGEADDRESSAWARE
> shlibdir_default="$bindir_default"
> + decklink_outdev_extralibs="$decklink_outdev_extralibs -lole32 -loleaut32"
> + decklink_indev_extralibs="$decklink_indev_extralibs -lole32 -loleaut32"
> SLIBPREF=""
> SLIBSUF=".dll"
> SLIBNAME_WITH_VERSION='$(SLIBPREF)$(FULLNAME)-$(LIBVERSION)$(SLIBSUF)'
> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
> index c9107c0..523217c 100644
> --- a/libavdevice/decklink_common.cpp
> +++ b/libavdevice/decklink_common.cpp
> @@ -19,6 +19,15 @@
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> +// Moved inclusion of internal.h here in order to get it to build successfully
> +// when using Visual C++ to build--otherwise, compilation errors result
> +// due to winsock.h (which is included indirectly by DeckLinkAPI.h and
> +// DeckLinkAPI_i.c) conflicting with winsock2.h, which is included by
> +// internal.h. If winsock2.h is included first, then the conflict is resolved.
For multiline comments generally the /* */ syntax is preferred. You might
also consider shortening the text a bit, e.g.:
/* Include internal.h first to avoid conflict of winsock.h (used by DeckLink)
and winsock2.h (used by libavformat) when building with MSVC++ */
> +extern "C" {
> +#include "libavformat/internal.h"
> +}
> +
> #include <DeckLinkAPI.h>
> #ifdef _WIN32
> #include <DeckLinkAPI_i.c>
> @@ -26,12 +35,8 @@
> #include <DeckLinkAPIDispatch.cpp>
> #endif
>
> -#include <pthread.h>
> -#include <semaphore.h>
> -
> extern "C" {
> #include "libavformat/avformat.h"
> -#include "libavformat/internal.h"
> #include "libavutil/imgutils.h"
> #include "libavutil/intreadwrite.h"
> #include "libavutil/bswap.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 8cc1bdf..a663029 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -19,15 +19,15 @@
> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> */
>
> -#include <DeckLinkAPI.h>
> +extern "C" {
> +#include "libavformat/internal.h"
> +}
>
> -#include <pthread.h>
> -#include <semaphore.h>
> +#include <DeckLinkAPI.h>
>
> extern "C" {
> #include "config.h"
> #include "libavformat/avformat.h"
> -#include "libavformat/internal.h"
> #include "libavutil/avutil.h"
> #include "libavutil/common.h"
> #include "libavutil/imgutils.h"
> @@ -265,8 +265,18 @@ static int64_t get_pkt_pts(IDeckLinkVideoInputFrame *videoFrame,
> res = videoFrame->GetHardwareReferenceTimestamp(time_base.den, &bmd_pts, &bmd_duration);
> break;
> case PTS_SRC_WALLCLOCK:
> - pts = av_rescale_q(wallclock, AV_TIME_BASE_Q, time_base);
> + {
> + // doing the following rather than using AV_TIME_BASE_Q because
> + // AV_TIME_BASE_Q doesn't work when building with Visual C++
> + // for C++ files (works for C files). When building C++ files,
> + // it results in compiler error C4576. At least, this is the case
> + // with Visual C++ 2015.
> + AVRational timebase;
> + timebase.num = 1;
> + timebase.den = AV_TIME_BASE;
> + pts = av_rescale_q(wallclock, timebase, time_base);
> break;
> + }
You should indent this block more. Also, consider shortening
the comment, e.g:
// MSVC does not support compound literals like AV_TIME_BASE_Q in C++
> }
> if (res == S_OK)
> pts = bmd_pts / time_base.num;
> diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
> index 18ef905..9ffb718 100644
> --- a/libavdevice/decklink_enc.cpp
> +++ b/libavdevice/decklink_enc.cpp
> @@ -22,10 +22,11 @@
> #include <atomic>
> using std::atomic;
>
> -#include <DeckLinkAPI.h>
> +extern "C" {
> +#include "libavformat/internal.h"
> +}
>
> -#include <pthread.h>
> -#include <semaphore.h>
> +#include <DeckLinkAPI.h>
>
> extern "C" {
> #include "libavformat/avformat.h"
> @@ -91,7 +92,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;
> }
> @@ -133,7 +137,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;
> @@ -141,7 +144,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);
> @@ -211,7 +216,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);
>
> @@ -247,7 +253,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,
> --
> 2.10.1.windows.1
Thanks,
Marton
More information about the ffmpeg-devel
mailing list