From patchwork Fri Apr 14 01:17:57 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Levinson X-Patchwork-Id: 3409 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.3.129 with SMTP id 123csp45439vsd; Thu, 13 Apr 2017 18:18:13 -0700 (PDT) X-Received: by 10.28.176.5 with SMTP id z5mr12965180wme.3.1492132693448; Thu, 13 Apr 2017 18:18:13 -0700 (PDT) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id w35si610265wrc.60.2017.04.13.18.18.12; Thu, 13 Apr 2017 18:18:13 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B4DF36899A8; Fri, 14 Apr 2017 04:18:03 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from white.spiritone.com (white.spiritone.com [216.99.193.38]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1C9BF689983 for ; Fri, 14 Apr 2017 04:17:57 +0300 (EEST) Received: from [192.168.3.100] (184-100-162-109.ptld.qwest.net [184.100.162.109]) by white.spiritone.com (Postfix) with ESMTPSA id EE456734033B for ; Thu, 13 Apr 2017 18:18:01 -0700 (PDT) To: FFmpeg development discussions and patches References: <4776a36e-4571-6257-5a54-169db893063f@aracnet.com> <3ef289a8-c819-7a17-0307-839049caebb4@aracnet.com> From: Aaron Levinson Message-ID: <91cb5c55-fdb5-460b-c640-0b11255fd2fd@aracnet.com> Date: Thu, 13 Apr 2017 18:17:57 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] avdevice/decklink: Removed pthread dependency X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 >> 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, Aaron ----------------------------------------------------------------------- From 7ecc4c280f2185dc1c19131a1dbf9f833ebb42b3 Mon Sep 17 00:00:00 2001 From: Aaron Levinson Date: Thu, 13 Apr 2017 18:08:17 -0700 Subject: [PATCH] avdevice/decklink: Removed pthread dependency 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. 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. Comments: -- configure: Eliminated pthreads dependency for decklink_indev_deps and decklink_outdev_deps and replaced with threads dependency -- 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) Removed include of pthread.h and semaphore.h and now using libavutil/thread.h instead. -- libavdevice/decklink_dec.cpp: Eliminated include of pthread.h and semaphore.h. -- 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. --- configure | 4 ++-- libavdevice/decklink_common.cpp | 3 --- libavdevice/decklink_common.h | 5 ++++- libavdevice/decklink_dec.cpp | 3 --- libavdevice/decklink_enc.cpp | 23 +++++++++++++++-------- 5 files changed, 21 insertions(+), 17 deletions(-) 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 #endif -#include -#include - 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 +#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 -#include -#include - 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 -#include -#include - 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,