| Message ID | decde190-5b63-c581-748d-2e6d655ad717@m1stereo.tv |
|---|---|
| State | New |
| Headers | show |
2018-02-20 17:32 GMT+01:00 Maksym Veremeyenko <verem@m1stereo.tv>: > attached patch implement dynamic loading of NewTek library and > drop dependencies from NewTek SDK (if previous patch with > including headers applied) This patch is not ok assuming the runtime library is not open source and has a license compatible with the GPL. I suggest you leave the code as-is, Carl Eugen
On Tue, 20 Feb 2018, Carl Eugen Hoyos wrote: > 2018-02-20 17:32 GMT+01:00 Maksym Veremeyenko <verem@m1stereo.tv>: > >> attached patch implement dynamic loading of NewTek library and >> drop dependencies from NewTek SDK (if previous patch with >> including headers applied) > > This patch is not ok assuming the runtime library is not open source > and has a license compatible with the GPL. The patch might has merits even if the library remains in the NONFREE section, no? Regards, Marton
2018-02-20 23:35 GMT+01:00 Marton Balint <cus@passwd.hu>: > > On Tue, 20 Feb 2018, Carl Eugen Hoyos wrote: > >> 2018-02-20 17:32 GMT+01:00 Maksym Veremeyenko <verem@m1stereo.tv>: >> >>> attached patch implement dynamic loading of NewTek library and >>> drop dependencies from NewTek SDK (if previous patch with >>> including headers applied) >> >> This patch is not ok assuming the runtime library is not open >> source and has a license compatible with the GPL. > > The patch might has merits even if the library remains in the > NONFREE section, no? It might, I do not immediately see them, though. Carl Eugen
21.02.2018 0:39, Carl Eugen Hoyos пише: > 2018-02-20 23:35 GMT+01:00 Marton Balint <cus@passwd.hu>: >> >> On Tue, 20 Feb 2018, Carl Eugen Hoyos wrote: >> >>> 2018-02-20 17:32 GMT+01:00 Maksym Veremeyenko <verem@m1stereo.tv>: >>> >>>> attached patch implement dynamic loading of NewTek library and >>>> drop dependencies from NewTek SDK (if previous patch with >>>> including headers applied) >>> >>> This patch is not ok assuming the runtime library is not open >>> source and has a license compatible with the GPL. >> >> The patch might has merits even if the library remains in the >> NONFREE section, no? > > It might, I do not immediately see them, though. patch without altering *EXTERNAL_LIBRARY_NONFREE_LIST* has any chance to be reviewed?
On Wed, 21 Feb 2018, Maksym Veremeyenko wrote: > 21.02.2018 0:39, Carl Eugen Hoyos пише: >> 2018-02-20 23:35 GMT+01:00 Marton Balint <cus@passwd.hu>: >>> >>> On Tue, 20 Feb 2018, Carl Eugen Hoyos wrote: >>> >>>> 2018-02-20 17:32 GMT+01:00 Maksym Veremeyenko <verem@m1stereo.tv>: >>>> >>>>> attached patch implement dynamic loading of NewTek library and >>>>> drop dependencies from NewTek SDK (if previous patch with >>>>> including headers applied) >>>> >>>> This patch is not ok assuming the runtime library is not open >>>> source and has a license compatible with the GPL. >>> >>> The patch might has merits even if the library remains in the >>> NONFREE section, no? >> >> It might, I do not immediately see them, though. > > patch without altering *EXTERNAL_LIBRARY_NONFREE_LIST* has any chance to > be reviewed? Sure: > From 8c0337878bdb8a1ccbc56ede42686e2a4d8e882e Mon Sep 17 00:00:00 2001 > From: Maksym Veremeyenko <verem@m1.tv> > Date: Tue, 20 Feb 2018 17:16:46 +0200 > Subject: [PATCH 2/2] Implement dynamic loading of NewTek NDI library > > --- > configure | 8 +-- > libavdevice/Makefile | 4 +- > libavdevice/libndi_newtek_common.c | 105 +++++++++++++++++++++++++++++++++++++ > libavdevice/libndi_newtek_common.h | 4 +- > libavdevice/libndi_newtek_dec.c | 32 ++++++----- > libavdevice/libndi_newtek_enc.c | 16 ++++-- > 6 files changed, 144 insertions(+), 25 deletions(-) > create mode 100644 libavdevice/libndi_newtek_common.c > > diff --git a/configure b/configure > index 013308c..4782c77 100755 > --- a/configure > +++ b/configure > @@ -1569,7 +1569,6 @@ EXTERNAL_LIBRARY_GPL_LIST=" > > EXTERNAL_LIBRARY_NONFREE_LIST=" > decklink > - libndi_newtek > libfdk_aac > openssl > libtls > @@ -1648,6 +1647,7 @@ EXTERNAL_LIBRARY_LIST=" > mediacodec > openal > opengl > + libndi_newtek > " Some people disagree with this, so better leave it in NONFREE for now. > > HWACCEL_AUTODETECT_LIBRARY_LIST=" > @@ -3093,10 +3093,11 @@ decklink_indev_deps="decklink threads" > decklink_indev_extralibs="-lstdc++" > decklink_outdev_deps="decklink threads" > decklink_outdev_extralibs="-lstdc++" > +libndi_newtek_deps_any="libdl LoadLibrary" > libndi_newtek_indev_deps="libndi_newtek" > -libndi_newtek_indev_extralibs="-lndi" > +libndi_newtek_indev_extralibs="" I believe you can simply delete this line. > libndi_newtek_outdev_deps="libndi_newtek" > -libndi_newtek_outdev_extralibs="-lndi" > +libndi_newtek_outdev_extralibs="" And this. > dshow_indev_deps="IBaseFilter" > dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi" > fbdev_indev_deps="linux_fb_h" > @@ -5866,7 +5867,6 @@ enabled cuda_sdk && require cuda_sdk cuda.h cuCtxCreate -lcuda > enabled chromaprint && require chromaprint chromaprint.h chromaprint_get_version -lchromaprint > enabled decklink && { require_header DeckLinkAPI.h && > { check_cpp_condition DeckLinkAPIVersion.h "BLACKMAGIC_DECKLINK_API_VERSION >= 0x0a060100" || die "ERROR: Decklink API version must be >= 10.6.1."; } } > -enabled libndi_newtek && require_header Processing.NDI.Lib.h As other already pointed out, external headers in ffmpeg source tree are not welcome anymore, so I guess you should keep this check. Maybe you should also check the version of the headers, because you require SDK version V3 from now on, right? > enabled frei0r && require_header frei0r.h > enabled gmp && require gmp gmp.h mpz_export -lgmp > enabled gnutls && require_pkg_config gnutls gnutls gnutls/gnutls.h gnutls_global_init > diff --git a/libavdevice/Makefile b/libavdevice/Makefile > index 8228d62..2d3322e 100644 > --- a/libavdevice/Makefile > +++ b/libavdevice/Makefile > @@ -19,8 +19,8 @@ OBJS-$(CONFIG_BKTR_INDEV) += bktr.o > OBJS-$(CONFIG_CACA_OUTDEV) += caca.o > OBJS-$(CONFIG_DECKLINK_OUTDEV) += decklink_enc.o decklink_enc_c.o decklink_common.o > OBJS-$(CONFIG_DECKLINK_INDEV) += decklink_dec.o decklink_dec_c.o decklink_common.o > -OBJS-$(CONFIG_LIBNDI_NEWTEK_OUTDEV) += libndi_newtek_enc.o > -OBJS-$(CONFIG_LIBNDI_NEWTEK_INDEV) += libndi_newtek_dec.o > +OBJS-$(CONFIG_LIBNDI_NEWTEK_OUTDEV) += libndi_newtek_enc.o libndi_newtek_common.o > +OBJS-$(CONFIG_LIBNDI_NEWTEK_INDEV) += libndi_newtek_dec.o libndi_newtek_common.o > OBJS-$(CONFIG_DSHOW_INDEV) += dshow_crossbar.o dshow.o dshow_enummediatypes.o \ > dshow_enumpins.o dshow_filter.o \ > dshow_pin.o dshow_common.o > diff --git a/libavdevice/libndi_newtek_common.c b/libavdevice/libndi_newtek_common.c > new file mode 100644 > index 0000000..5202993 > --- /dev/null > +++ b/libavdevice/libndi_newtek_common.c > @@ -0,0 +1,105 @@ > +/* > + * NewTek NDI common code > + * Copyright (c) 2018 Maksym Veremeyenko > + * > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#include "libavformat/avformat.h" > +#include "libavformat/internal.h" > +#include "libavutil/opt.h" > +#include "libavutil/imgutils.h" > + > +#ifdef _WIN32 > +#include <windows.h> > +#else > +#include <dlfcn.h> > +#endif > + > +#include "libndi_newtek_common.h" > + > +#define NDI_LIB_LOAD_ERROR_TEXT "\nPlease re-install the NewTek NDI Runtimes from " NDILIB_REDIST_URL " to use this functionality." > + > +const NDIlib_v3* ndi_lib_load(AVFormatContext *avctx) { Use the ff_ prefix for global functions. > + char *path = NULL, *e; > + const NDIlib_v3* (*NDIlib_v3_load)(void) = NULL; > +#ifdef _WIN32 > + HMODULE > +#else > + void* > +#endif > + hNDILib; The library handle should be returned, and freed when the device is freed, otherwise you are leaking it. > + > + e = getenv(NDILIB_REDIST_FOLDER); > + if (!e) { > + path = av_strdup(NDILIB_LIBRARY_NAME); > + if (!path) > + return NULL; > + } > + else { > + int s = strlen(NDILIB_LIBRARY_NAME) + 1 + strlen(e) + 1; > + path = av_malloc(s); > + if (!path) > + return NULL; > + snprintf(path, s, "%s" > +#ifdef _WIN32 > + "\\" > +#else > + "/" > +#endif > + "%s", e, NDILIB_LIBRARY_NAME); > + } You can use a simpler approach to do this using av_asprintf which allocates the result (you need to define DS to "\\" or "/" depending on arch): if (e) path = av_asprintf("%s%s%s", e, DS, NDILIB_LIBRARY_NAME); else path = av_strdup(NDILIB_LIBRARY_NAME); > + > + > +#ifdef _WIN32 > + /* Try to load the library */ > + hNDILib = LoadLibrary(path); > + > + if (!hNDILib) > + av_log(avctx, AV_LOG_ERROR, "LoadLibrary(%s) failed. " NDI_LIB_LOAD_ERROR_TEXT "\n", path); > + else { > + > + /* get NDIlib_v3_load address */ > + *((FARPROC*)&NDIlib_v3_load) = GetProcAddress(hNDILib, "NDIlib_v3_load"); > + > + if (!NDIlib_v3_load) { > + av_log(avctx, AV_LOG_ERROR, "GetProcAddress(NDIlib_v3_load) failed in file [%s]. " NDI_LIB_LOAD_ERROR_TEXT "\n", path); > + FreeLibrary(hNDILib); > + } > + } > +#else > + /* Try to load the library */ > + hNDILib = dlopen(path, RTLD_LOCAL | RTLD_LAZY); > + > + if (!hNDILib) > + av_log(avctx, AV_LOG_ERROR, "dlopen(%s) failed. " NDI_LIB_LOAD_ERROR_TEXT "\n", path); > + else { > + > + /* get NDIlib_v3_load address */ > + *((void**)&NDIlib_v3_load) = dlsym(hNDILib, "NDIlib_v3_load"); > + > + if (!NDIlib_v3_load) { > + av_log(avctx, AV_LOG_ERROR, "dlsym(NDIlib_v3_load) failed in file[%s]. " NDI_LIB_LOAD_ERROR_TEXT "\n", path); > + dlclose(hNDILib); > + } > + } > +#endif You should be able to load the library using common code if you use the compat/w32dlfcn.h wrapper for WIN32, similarly how amfenc or avisynth does it. However, the win32 wrapper expects a single library name, so you might have to extend the wrapper so that it detects if there is a backslash in the name, and act accordingly. Regards, Marton
Marton Balint (2018-02-20): > The patch might has merits even if the library remains in the NONFREE > section, no? I see more code and easier circumvention of the GPL, but no merit. Please be more specific. Regards,
On Wed, 21 Feb 2018, Nicolas George wrote: > Marton Balint (2018-02-20): >> The patch might has merits even if the library remains in the NONFREE >> section, no? > > I see more code and easier circumvention of the GPL, but no merit. > Please be more specific. I guess the biggest advantage of dynamic loading is that it can use the environment variable for finding the library, this approach is documented / encouraged in the NDI SDK. Also the author of the ndi patches prefers this approach, that matters as well. I am not sure I see a lot of difference when it comes to GPL circumvention, if the lib is in non-free, then the stuff becomes non-redistributable, period. Added complexity is marginal IMHO. Regards, Marton
2018-02-22 20:26 GMT+01:00 Marton Balint <cus@passwd.hu>: > > On Wed, 21 Feb 2018, Nicolas George wrote: > >> Marton Balint (2018-02-20): >>> >>> The patch might has merits even if the library remains in the NONFREE >>> section, no? >> >> I see more code and easier circumvention of the GPL, but no merit. >> Please be more specific. > > I guess the biggest advantage of dynamic loading is that it can use the > environment variable for finding the library, this approach is documented / > encouraged in the NDI SDK. This doesn't sound convincing imo: Does the company distribute the dynamic library with different names? Carl Eugen
On 22 February 2018 at 22:22, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2018-02-22 20:26 GMT+01:00 Marton Balint <cus@passwd.hu>: > > > > On Wed, 21 Feb 2018, Nicolas George wrote: > > > >> Marton Balint (2018-02-20): > >>> > >>> The patch might has merits even if the library remains in the NONFREE > >>> section, no? > >> > >> I see more code and easier circumvention of the GPL, but no merit. > >> Please be more specific. > > > > I guess the biggest advantage of dynamic loading is that it can use the > > environment variable for finding the library, this approach is > documented / > > encouraged in the NDI SDK. > > This doesn't sound convincing imo: Does the company distribute > the dynamic library with different names? > Yes, different names for each OS. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Thu, 22 Feb 2018, Carl Eugen Hoyos wrote: > 2018-02-22 20:26 GMT+01:00 Marton Balint <cus@passwd.hu>: >> >> On Wed, 21 Feb 2018, Nicolas George wrote: >> >>> Marton Balint (2018-02-20): >>>> >>>> The patch might has merits even if the library remains in the NONFREE >>>> section, no? >>> >>> I see more code and easier circumvention of the GPL, but no merit. >>> Please be more specific. >> >> I guess the biggest advantage of dynamic loading is that it can use the >> environment variable for finding the library, this approach is documented / >> encouraged in the NDI SDK. > > This doesn't sound convincing imo: Does the company distribute > the dynamic library with different names? No. In fact, now they only provide a .so file, so static linking is no longer possible. Regards, Marton
On Thu, 22 Feb 2018, Ricardo Constantino wrote: > On 22 February 2018 at 22:22, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2018-02-22 20:26 GMT+01:00 Marton Balint <cus@passwd.hu>: >> > >> > On Wed, 21 Feb 2018, Nicolas George wrote: >> > >> >> Marton Balint (2018-02-20): >> >>> >> >>> The patch might has merits even if the library remains in the NONFREE >> >>> section, no? >> >> >> >> I see more code and easier circumvention of the GPL, but no merit. >> >> Please be more specific. >> > >> > I guess the biggest advantage of dynamic loading is that it can use the >> > environment variable for finding the library, this approach is >> documented / >> > encouraged in the NDI SDK. >> >> This doesn't sound convincing imo: Does the company distribute >> the dynamic library with different names? >> > > Yes, different names for each OS. I think what CE ment was static v.s. dynamic lib names, but what you write is true as well, the library has different names on different OS-es/archs, I am not even sure NDI stuff builds for windows, I always tested linux only. With dynamic loading we can rely on the library name defined in the NDI SDK headers which should be the proper one for each OS/arch. Regards, Marton
Marton Balint (2018-02-22): > I think what CE ment was static v.s. dynamic lib names I am pretty sure he asked if the file name of the library could be different at run time than at compile time, which would be the only reason to justify using an environment variable. By the way, using an environment variable to load a dynamic library is a terrible idea in terms of security. Let us not do it. > With dynamic loading we can rely on the library name defined in the NDI SDK > headers which should be the proper one for each OS/arch. So basically, we should add 100+ lines of ugly code just because these guys are unable to follow the same rules as any other library? I vote no. Regards,
2018-02-22 23:57 GMT+01:00 Nicolas George <george@nsup.org>: > Marton Balint (2018-02-22): >> I think what CE ment was static v.s. dynamic lib names > > I am pretty sure he asked if the file name of the library could be > different at run time than at compile time, which would be the only > reason to justify using an environment variable. Correct. Carl Eugen
On Thu, 22 Feb 2018, Nicolas George wrote: > Marton Balint (2018-02-22): >> I think what CE ment was static v.s. dynamic lib names > > I am pretty sure he asked if the file name of the library could be > different at run time than at compile time, which would be the only > reason to justify using an environment variable. The environment variable contains the directory to the library, not the library name itself. > By the way, using an environment variable to load a dynamic library is a > terrible idea in terms of security. Let us not do it. LD_LIBRARY_PATH, LD_PRELOAD is an environment variable as well, so I don't see why it is different from security standpoint. And it's not unprecedented, there is FREI0R_PATH, or LADSPA_PATH for example. Okay, these are plugins, so not entirely the same thing, yet the approach is similar. > >> With dynamic loading we can rely on the library name defined in the NDI SDK >> headers which should be the proper one for each OS/arch. > > So basically, we should add 100+ lines of ugly code just because these > guys are unable to follow the same rules as any other library? I'd rather say that we should add 100+ lines of code because: - it fixes some portability issues with the library names - dynamic loading alone has some pros (e.g. not needing the SDK on every machine where you use your built ffmpeg) - we can use the SDK suggested way to find the library using an environment variable - the author who is the de facto maintainer prefers it this way I also tried to help making the code "less ugly" in my review. Regards, Marton
Marton Balint (2018-02-23): > The environment variable contains the directory to the library, not the > library name itself. So it reinvents LD_LIBRARY_PATH. Probably badly. > LD_LIBRARY_PATH, LD_PRELOAD is an environment variable as well, so I don't > see why it is different from security standpoint. It is not different in concept, but the details were designed by very competent hackers. Yet, over the years, numerous security issues related to these features have been found and fixed. > And it's not unprecedented, there is FREI0R_PATH, or LADSPA_PATH for > example. Okay, these are plugins, so not entirely the same thing, yet the > approach is similar. I had not yet thought of it. This is worrisome. > I'd rather say that we should add 100+ lines of code because: > - it fixes some portability issues with the library names So, in order to fix the portability issues of this library, any application that uses it is supposed to implement these 100+ lines of code. I say: let them fix their crap. > - dynamic loading alone has some pros (e.g. not needing the SDK > on every machine where you use your built ffmpeg) Sorry, you are wrong, this is not how dynamic loading works. You need the shared library (and possibly supporting data files), whether you load it through dlopen() or through NEEDED. > - we can use the SDK suggested way to find the library using an > environment variable Let them fix their crap. > - the author who is the de facto maintainer prefers it this way Let them fix their tastes. > I also tried to help making the code "less ugly" in my review. Les ugly is still ugly. Regards,
On 22 February 2018 at 22:53, Marton Balint <cus@passwd.hu> wrote: > > > On Thu, 22 Feb 2018, Ricardo Constantino wrote: > > On 22 February 2018 at 22:22, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> >> 2018-02-22 20:26 GMT+01:00 Marton Balint <cus@passwd.hu>: >>> > >>> > On Wed, 21 Feb 2018, Nicolas George wrote: >>> > >>> >> Marton Balint (2018-02-20): >>> >>> >>> >>> The patch might has merits even if the library remains in the NONFREE >>> >>> section, no? >>> >> >>> >> I see more code and easier circumvention of the GPL, but no merit. >>> >> Please be more specific. >>> > >>> > I guess the biggest advantage of dynamic loading is that it can use the >>> > environment variable for finding the library, this approach is >>> documented / >>> > encouraged in the NDI SDK. >>> >>> This doesn't sound convincing imo: Does the company distribute >>> the dynamic library with different names? >>> >>> >> Yes, different names for each OS. >> > > I think what CE ment was static v.s. dynamic lib names, but what you write > is true as well, the library has different names on different OS-es/archs, > I am not even sure NDI stuff builds for windows, I always tested linux only. > > With dynamic loading we can rely on the library name defined in the NDI > SDK headers which should be the proper one for each OS/arch. > Except non-MSVC compilers for Windows, which Newtek seems to think is the exclusive way to compile for Windows. > > Regards, > Marton > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Fri, Feb 23, 2018 at 11:47 AM, Nicolas George <george@nsup.org> wrote: > >> - dynamic loading alone has some pros (e.g. not needing the SDK >> on every machine where you use your built ffmpeg) > > Sorry, you are wrong, this is not how dynamic loading works. You need > the shared library (and possibly supporting data files), whether you > load it through dlopen() or through NEEDED. > > When you use dlopen, you can write code that gracefully fails when the library is not present, instead of failing to load ffmpeg/avdevice entirely. This is obviously what is meant here. - Hendrik
diff --git a/configure b/configure index 013308c..4782c77 100755 --- a/configure +++ b/configure @@ -1569,7 +1569,6 @@ EXTERNAL_LIBRARY_GPL_LIST=" EXTERNAL_LIBRARY_NONFREE_LIST=" decklink - libndi_newtek libfdk_aac openssl libtls @@ -1648,6 +1647,7 @@ EXTERNAL_LIBRARY_LIST=" mediacodec openal opengl + libndi_newtek " HWACCEL_AUTODETECT_LIBRARY_LIST=" @@ -3093,10 +3093,11 @@ decklink_indev_deps="decklink threads" decklink_indev_extralibs="-lstdc++" decklink_outdev_deps="decklink threads" decklink_outdev_extralibs="-lstdc++" +libndi_newtek_deps_any="libdl LoadLibrary" libndi_newtek_indev_deps="libndi_newtek" -libndi_newtek_indev_extralibs="-lndi" +libndi_newtek_indev_extralibs="" libndi_newtek_outdev_deps="libndi_newtek" -libndi_newtek_outdev_extralibs="-lndi" +libndi_newtek_outdev_extralibs="" dshow_indev_deps="IBaseFilter" dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi" fbdev_indev_deps="linux_fb_h" @@ -5866,7 +5867,6 @@ enabled cuda_sdk && require cuda_sdk cuda.h cuCtxCreate -lcuda enabled chromaprint && require chromaprint chromaprint.h chromaprint_get_version -lchromaprint enabled decklink && { require_header DeckLinkAPI.h && { check_cpp_condition DeckLinkAPIVersion.h "BLACKMAGIC_DECKLINK_API_VERSION >= 0x0a060100" || die "ERROR: Decklink API version must be >= 10.6.1."; } } -enabled libndi_newtek && require_header Processing.NDI.Lib.h enabled frei0r && require_header frei0r.h enabled gmp && require gmp gmp.h mpz_export -lgmp enabled gnutls && require_pkg_config gnutls gnutls gnutls/gnutls.h gnutls_global_init diff --git a/libavdevice/Makefile b/libavdevice/Makefile index 8228d62..2d3322e 100644 --- a/libavdevice/Makefile +++ b/libavdevice/Makefile @@ -19,8 +19,8 @@ OBJS-$(CONFIG_BKTR_INDEV) += bktr.o OBJS-$(CONFIG_CACA_OUTDEV) += caca.o OBJS-$(CONFIG_DECKLINK_OUTDEV) += decklink_enc.o decklink_enc_c.o decklink_common.o OBJS-$(CONFIG_DECKLINK_INDEV) += decklink_dec.o decklink_dec_c.o decklink_common.o -OBJS-$(CONFIG_LIBNDI_NEWTEK_OUTDEV) += libndi_newtek_enc.o -OBJS-$(CONFIG_LIBNDI_NEWTEK_INDEV) += libndi_newtek_dec.o +OBJS-$(CONFIG_LIBNDI_NEWTEK_OUTDEV) += libndi_newtek_enc.o libndi_newtek_common.o +OBJS-$(CONFIG_LIBNDI_NEWTEK_INDEV) += libndi_newtek_dec.o libndi_newtek_common.o OBJS-$(CONFIG_DSHOW_INDEV) += dshow_crossbar.o dshow.o dshow_enummediatypes.o \ dshow_enumpins.o dshow_filter.o \ dshow_pin.o dshow_common.o diff --git a/libavdevice/libndi_newtek_common.c b/libavdevice/libndi_newtek_common.c new file mode 100644 index 0000000..5202993 --- /dev/null +++ b/libavdevice/libndi_newtek_common.c @@ -0,0 +1,105 @@ +/* + * NewTek NDI common code + * Copyright (c) 2018 Maksym Veremeyenko + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "libavformat/avformat.h" +#include "libavformat/internal.h" +#include "libavutil/opt.h" +#include "libavutil/imgutils.h" + +#ifdef _WIN32 +#include <windows.h> +#else +#include <dlfcn.h> +#endif + +#include "libndi_newtek_common.h" + +#define NDI_LIB_LOAD_ERROR_TEXT "\nPlease re-install the NewTek NDI Runtimes from " NDILIB_REDIST_URL " to use this functionality." + +const NDIlib_v3* ndi_lib_load(AVFormatContext *avctx) { + char *path = NULL, *e; + const NDIlib_v3* (*NDIlib_v3_load)(void) = NULL; +#ifdef _WIN32 + HMODULE +#else + void* +#endif + hNDILib; + + e = getenv(NDILIB_REDIST_FOLDER); + if (!e) { + path = av_strdup(NDILIB_LIBRARY_NAME); + if (!path) + return NULL; + } + else { + int s = strlen(NDILIB_LIBRARY_NAME) + 1 + strlen(e) + 1; + path = av_malloc(s); + if (!path) + return NULL; + snprintf(path, s, "%s" +#ifdef _WIN32 + "\\" +#else + "/" +#endif + "%s", e, NDILIB_LIBRARY_NAME); + } + + +#ifdef _WIN32 + /* Try to load the library */ + hNDILib = LoadLibrary(path); + + if (!hNDILib) + av_log(avctx, AV_LOG_ERROR, "LoadLibrary(%s) failed. " NDI_LIB_LOAD_ERROR_TEXT "\n", path); + else { + + /* get NDIlib_v3_load address */ + *((FARPROC*)&NDIlib_v3_load) = GetProcAddress(hNDILib, "NDIlib_v3_load"); + + if (!NDIlib_v3_load) { + av_log(avctx, AV_LOG_ERROR, "GetProcAddress(NDIlib_v3_load) failed in file [%s]. " NDI_LIB_LOAD_ERROR_TEXT "\n", path); + FreeLibrary(hNDILib); + } + } +#else + /* Try to load the library */ + hNDILib = dlopen(path, RTLD_LOCAL | RTLD_LAZY); + + if (!hNDILib) + av_log(avctx, AV_LOG_ERROR, "dlopen(%s) failed. " NDI_LIB_LOAD_ERROR_TEXT "\n", path); + else { + + /* get NDIlib_v3_load address */ + *((void**)&NDIlib_v3_load) = dlsym(hNDILib, "NDIlib_v3_load"); + + if (!NDIlib_v3_load) { + av_log(avctx, AV_LOG_ERROR, "dlsym(NDIlib_v3_load) failed in file[%s]. " NDI_LIB_LOAD_ERROR_TEXT "\n", path); + dlclose(hNDILib); + } + } +#endif + + av_free(path); + + return NDIlib_v3_load ? NDIlib_v3_load() : NULL; +} diff --git a/libavdevice/libndi_newtek_common.h b/libavdevice/libndi_newtek_common.h index 8990317..16f794d 100644 --- a/libavdevice/libndi_newtek_common.h +++ b/libavdevice/libndi_newtek_common.h @@ -22,7 +22,9 @@ #ifndef AVDEVICE_LIBNDI_NEWTEK_COMMON_H #define AVDEVICE_LIBNDI_NEWTEK_COMMON_H -#include <Processing.NDI.Lib.h> +#include "compat/libndi_newtek/Processing.NDI.Lib.h" + +const NDIlib_v3* ndi_lib_load(AVFormatContext *avctx); #define NDI_TIME_BASE 10000000 #define NDI_TIME_BASE_Q (AVRational){1, NDI_TIME_BASE} diff --git a/libavdevice/libndi_newtek_dec.c b/libavdevice/libndi_newtek_dec.c index 4fb7197..beeb3fc 100644 --- a/libavdevice/libndi_newtek_dec.c +++ b/libavdevice/libndi_newtek_dec.c @@ -35,6 +35,7 @@ struct NDIContext { int allow_video_fields; /* Runtime */ + const NDIlib_v3* lib; NDIlib_recv_create_t *recv; NDIlib_find_instance_t ndi_find; @@ -87,7 +88,7 @@ static int ndi_set_audio_packet(AVFormatContext *avctx, NDIlib_audio_frame_t *a, dst.reference_level = 0; dst.p_data = (short *)pkt->data; - NDIlib_util_audio_to_interleaved_16s(a, &dst); + ctx->lib->NDIlib_util_audio_to_interleaved_16s(a, &dst); return 0; } @@ -102,7 +103,7 @@ static int ndi_find_sources(AVFormatContext *avctx, const char *name, NDIlib_sou .p_groups = NULL, .p_extra_ips = NULL }; if (!ctx->ndi_find) - ctx->ndi_find = NDIlib_find_create2(&find_create_desc); + ctx->ndi_find = ctx->lib->NDIlib_find_create_v2(&find_create_desc); if (!ctx->ndi_find) { av_log(avctx, AV_LOG_ERROR, "NDIlib_find_create failed.\n"); return AVERROR(EIO); @@ -112,13 +113,13 @@ static int ndi_find_sources(AVFormatContext *avctx, const char *name, NDIlib_sou { int f, t = ctx->wait_sources / 1000; av_log(avctx, AV_LOG_DEBUG, "Waiting for sources %d miliseconds\n", t); - f = NDIlib_find_wait_for_sources(ctx->ndi_find, t); + f = ctx->lib->NDIlib_find_wait_for_sources(ctx->ndi_find, t); av_log(avctx, AV_LOG_DEBUG, "NDIlib_find_wait_for_sources returns %d\n", f); if (!f) break; }; - ndi_srcs = NDIlib_find_get_current_sources(ctx->ndi_find, &n); + ndi_srcs = ctx->lib->NDIlib_find_get_current_sources(ctx->ndi_find, &n); if (ctx->find_sources) av_log(avctx, AV_LOG_INFO, "Found %d NDI sources:\n", n); @@ -143,7 +144,12 @@ static int ndi_read_header(AVFormatContext *avctx) const NDIlib_tally_t tally_state = { .on_program = true, .on_preview = false }; struct NDIContext *ctx = avctx->priv_data; - if (!NDIlib_initialize()) { + ctx->lib = ndi_lib_load(avctx); + if (!ctx->lib) { + return AVERROR_EXTERNAL; + } + + if (!ctx->lib->NDIlib_initialize()) { av_log(avctx, AV_LOG_ERROR, "NDIlib_initialize failed.\n"); return AVERROR_EXTERNAL; } @@ -162,14 +168,14 @@ static int ndi_read_header(AVFormatContext *avctx) recv_create_desc.allow_video_fields = ctx->allow_video_fields; /* Create the receiver */ - ctx->recv = NDIlib_recv_create(&recv_create_desc); + ctx->recv = ctx->lib->NDIlib_recv_create(&recv_create_desc); if (!ctx->recv) { av_log(avctx, AV_LOG_ERROR, "NDIlib_recv_create2 failed.\n"); return AVERROR(EIO); } /* Set tally */ - NDIlib_recv_set_tally(ctx->recv, &tally_state); + ctx->lib->NDIlib_recv_set_tally(ctx->recv, &tally_state); avctx->ctx_flags |= AVFMTCTX_NOHEADER; @@ -267,7 +273,7 @@ static int ndi_read_packet(AVFormatContext *avctx, AVPacket *pkt) NDIlib_frame_type_e t; av_log(avctx, AV_LOG_DEBUG, "NDIlib_recv_capture...\n"); - t = NDIlib_recv_capture(ctx->recv, &v, &a, &m, 40); + t = ctx->lib->NDIlib_recv_capture(ctx->recv, &v, &a, &m, 40); av_log(avctx, AV_LOG_DEBUG, "NDIlib_recv_capture=%d\n", t); if (t == NDIlib_frame_type_video) { @@ -275,7 +281,7 @@ static int ndi_read_packet(AVFormatContext *avctx, AVPacket *pkt) ret = ndi_create_video_stream(avctx, &v); if (!ret) ret = ndi_set_video_packet(avctx, &v, pkt); - NDIlib_recv_free_video(ctx->recv, &v); + ctx->lib->NDIlib_recv_free_video(ctx->recv, &v); break; } else if (t == NDIlib_frame_type_audio) { @@ -283,11 +289,11 @@ static int ndi_read_packet(AVFormatContext *avctx, AVPacket *pkt) ret = ndi_create_audio_stream(avctx, &a); if (!ret) ret = ndi_set_audio_packet(avctx, &a, pkt); - NDIlib_recv_free_audio(ctx->recv, &a); + ctx->lib->NDIlib_recv_free_audio(ctx->recv, &a); break; } else if (t == NDIlib_frame_type_metadata) - NDIlib_recv_free_metadata(ctx->recv, &m); + ctx->lib->NDIlib_recv_free_metadata(ctx->recv, &m); else if (t == NDIlib_frame_type_error){ av_log(avctx, AV_LOG_ERROR, "NDIlib_recv_capture failed with error\n"); ret = AVERROR(EIO); @@ -302,10 +308,10 @@ static int ndi_read_close(AVFormatContext *avctx) struct NDIContext *ctx = (struct NDIContext *)avctx->priv_data; if (ctx->recv) - NDIlib_recv_destroy(ctx->recv); + ctx->lib->NDIlib_recv_destroy(ctx->recv); if (ctx->ndi_find) - NDIlib_find_destroy(ctx->ndi_find); + ctx->lib->NDIlib_find_destroy(ctx->ndi_find); return 0; } diff --git a/libavdevice/libndi_newtek_enc.c b/libavdevice/libndi_newtek_enc.c index f3603f5..1243dbf 100644 --- a/libavdevice/libndi_newtek_enc.c +++ b/libavdevice/libndi_newtek_enc.c @@ -33,6 +33,7 @@ struct NDIContext { int reference_level; int clock_video, clock_audio; + const NDIlib_v3* lib; NDIlib_video_frame_t *video; NDIlib_audio_frame_interleaved_16s_t *audio; NDIlib_send_instance_t ndi_send; @@ -44,7 +45,7 @@ static int ndi_write_trailer(AVFormatContext *avctx) struct NDIContext *ctx = avctx->priv_data; if (ctx->ndi_send) { - NDIlib_send_destroy(ctx->ndi_send); + ctx->lib->NDIlib_send_destroy(ctx->ndi_send); av_frame_free(&ctx->last_avframe); } @@ -93,7 +94,7 @@ static int ndi_write_video_packet(AVFormatContext *avctx, AVStream *st, AVPacket /* asynchronous for one frame, but will block if a second frame is given before the first one has been sent */ - NDIlib_send_send_video_async(ctx->ndi_send, ctx->video); + ctx->lib->NDIlib_send_send_video_async(ctx->ndi_send, ctx->video); av_frame_free(&ctx->last_avframe); ctx->last_avframe = avframe; @@ -112,7 +113,7 @@ static int ndi_write_audio_packet(AVFormatContext *avctx, AVStream *st, AVPacket av_log(avctx, AV_LOG_DEBUG, "%s: pkt->pts=%"PRId64", timecode=%"PRId64", st->time_base=%d/%d\n", __func__, pkt->pts, ctx->audio->timecode, st->time_base.num, st->time_base.den); - NDIlib_util_send_send_audio_interleaved_16s(ctx->ndi_send, ctx->audio); + ctx->lib->NDIlib_util_send_send_audio_interleaved_16s(ctx->ndi_send, ctx->audio); return 0; } @@ -236,7 +237,12 @@ static int ndi_write_header(AVFormatContext *avctx) const NDIlib_send_create_t ndi_send_desc = { .p_ndi_name = avctx->url, .p_groups = NULL, .clock_video = ctx->clock_video, .clock_audio = ctx->clock_audio }; - if (!NDIlib_initialize()) { + ctx->lib = ndi_lib_load(avctx); + if (!ctx->lib) { + return AVERROR_EXTERNAL; + } + + if (!ctx->lib->NDIlib_initialize()) { av_log(avctx, AV_LOG_ERROR, "NDIlib_initialize failed.\n"); return AVERROR_EXTERNAL; } @@ -258,7 +264,7 @@ static int ndi_write_header(AVFormatContext *avctx) } } - ctx->ndi_send = NDIlib_send_create(&ndi_send_desc); + ctx->ndi_send = ctx->lib->NDIlib_send_create(&ndi_send_desc); if (!ctx->ndi_send) { av_log(avctx, AV_LOG_ERROR, "Failed to create NDI output %s\n", avctx->url); ret = AVERROR_EXTERNAL;