| Message ID | CAB0OVGrdeHjXUSDB+CX7qeKGjHT2ErEdtVpmx9vONkePusDT7w@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
On 11/26/2017 1:35 PM, Carl Eugen Hoyos wrote: > Attached patch adds a missing dependency to libvmaf, I don't know if > other threads also work. This should also be filed as a bug against libvmaf, since its pkg-config file isn't complete, then. - Derek
2017-11-26 14:40 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: > On 11/26/2017 1:35 PM, Carl Eugen Hoyos wrote: >> Attached patch adds a missing dependency to libvmaf, I don't know if >> other threads also work. > > This should also be filed as a bug against libvmaf, since its pkg-config > file isn't complete, then. Sorry, I don't understand how pkg-config is related to the missing dependency of our configure script: Please explain. Thank you, Carl Eugen
On 11/26/2017 1:50 PM, Carl Eugen Hoyos wrote: > Sorry, I don't understand how pkg-config is related to the missing > dependency of our configure script: Please explain. Because pthreads is a dependency of libvmaf. Looking at libvmaf, it does list pthreads as a dependency: https://github.com/Netflix/vmaf/blob/master/wrapper/libvmaf.pc It should work as-is. Is there a way I can reproduce this? - Derek
2017-11-26 14:53 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: > On 11/26/2017 1:50 PM, Carl Eugen Hoyos wrote: >> Sorry, I don't understand how pkg-config is related to the missing >> dependency of our configure script: Please explain. > > Because pthreads is a dependency of libvmaf. > > Looking at libvmaf, it does list pthreads as a dependency: > > https://github.com/Netflix/vmaf/blob/master/wrapper/libvmaf.pc > > It should work as-is. Is there a way I can reproduce this? The following produces an ffmpeg binary that loads pthreads at start-time: $ ./configure --enable-libvmaf --disable-pthreads Carl Eugen
Carl Eugen Hoyos (2017-11-26): > The following produces an ffmpeg binary that loads pthreads > at start-time: > $ ./configure --enable-libvmaf --disable-pthreads That is not a problem. The fact that it produces a binary with pthreads symbols in it is. Regards,
2017-11-26 14:59 GMT+01:00 Nicolas George <george@nsup.org>: > Carl Eugen Hoyos (2017-11-26): >> The following produces an ffmpeg binary that loads pthreads >> at start-time: >> $ ./configure --enable-libvmaf --disable-pthreads > > That is not a problem. The fact that it produces a binary with > pthreads symbols in it is. The way I understand (Linux) dynamic libraries, one implies the other. Carl Eugen
Carl Eugen Hoyos (2017-11-26): > The way I understand (Linux) dynamic libraries, one implies > the other. Yes, but not the other way around. If a library uses threads internally but provides an interface that can be used without threads, then it is not our problem. If a library does not use threads but our calls to the library require threads, then we must consider it a dependency. Regards,
2017-11-26 15:05 GMT+01:00 Nicolas George <george@nsup.org>: > Carl Eugen Hoyos (2017-11-26): >> The way I understand (Linux) dynamic libraries, one implies >> the other. > > Yes, but not the other way around. > > If a library uses threads internally but provides an interface that can > be used without threads, then it is not our problem. > > If a library does not use threads but our calls to the library require > threads, then we must consider it a dependency. In this case the library and our interface both depend on pthreads but configure ignores this. Carl Eugen
On 11/26/2017 2:02 PM, Carl Eugen Hoyos wrote: > The way I understand (Linux) dynamic libraries, one implies > the other. The problem is that libvmaf's .pc file put all of its deps in Libs instead of splitting them out into Libs.private, which is used only when static linking. Stuff like -lpthreads is only needed if static linking, and stuff like -lstdc++ is just wrong on any system that doesn't use libstdc++ (and also only used for static linking). I assume this is what Nicholas is referring to. - Derek
On 11/26/2017 2:05 PM, Nicolas George wrote: > If a library does not use threads but our calls to the library require > threads, then we must consider it a dependency. Netflix made their pkg-config file incorrectly, which causes this. It should be fixed there, but do we want to work around that in the mean time like this? I have no real strong opinion on that, I just thought it should be reported upstream so they could fix it. -Derek
2017-11-26 15:06 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: > On 11/26/2017 2:02 PM, Carl Eugen Hoyos wrote: >> The way I understand (Linux) dynamic libraries, one implies >> the other. > > The problem is that libvmaf's .pc file put all of its deps in > Libs instead of splitting them out into Libs.private, which > is used only when static linking. > Stuff like -lpthreads is > only needed if static linking, and stuff like -lstdc++ is > just wrong on any system that doesn't use libstdc++ (and > also only used for static linking). As said before, it is non-trivial to find such a system (it worked fine here on osx when I tested last). > I assume this is what Nicholas is referring to. So is the patch ok? Carl Eugen
On 11/26/2017 2:06 PM, Carl Eugen Hoyos wrote: > In this case the library and our interface both depend on pthreads > but configure ignores this. Sorry, I wasn't aware our own wrapper code use pthreads too. Patch should be OK then. Upstream pkg-config file is still broken, though. :) - Derek
Derek Buitenhuis (2017-11-26): > The problem is that libvmaf's .pc file put all of its deps in > Libs instead of splitting them out into Libs.private, which > is used only when static linking. Stuff like -lpthreads is > only needed if static linking, and stuff like -lstdc++ is > just wrong on any system that doesn't use libstdc++ (and > also only used for static linking). > > I assume this is what Nicholas is referring to. No, the problem is that you are not speaking of the same thing. $ grep -c pthread libavfilter/*vmaf* libavfilter/vf_libvmaf.c:22 libavfilter/vf_vmafmotion.c:0 libavfilter/vmaf_motion.h:0 -> our code depends on pthreads for this filters, it must be expressed in configure: Carl Eugen's patch is right, there is no need to bugreport anything. His explanations later were wrong, but it may only be caused by the confusion you brought. What you describe is possibly true, but not related. Regards,
On 11/26/2017 2:09 PM, Carl Eugen Hoyos wrote: > As said before, it is non-trivial to find such a system > (it worked fine here on osx when I tested last). OS X does not ship with libstdc++, IIRC. You must have been using a ports-build toolchain? Using clang-cl or MSVC on windows will also not use libstdc++. FreeBSD does not use libstdc++. - Derek
On 11/26/2017 2:10 PM, Nicolas George wrote: > -> our code depends on pthreads for this filters, it must be expressed > in configure: Carl Eugen's patch is right, there is no need to bugreport > anything. His explanations later were wrong, but it may only be caused > by the confusion you brought. Yeah I didn't realize that. Apologies on that. > > What you describe is possibly true, but not related. Correct - I will still report it upstream. - Derek
2017-11-26 15:11 GMT+01:00 Derek Buitenhuis <derek.buitenhuis@gmail.com>: > On 11/26/2017 2:09 PM, Carl Eugen Hoyos wrote: >> As said before, it is non-trivial to find such a system >> (it worked fine here on osx when I tested last). > > OS X does not ship with libstdc++, IIRC. You must > have been using a ports-build toolchain? I believe I have explained before that I always only use vanilla toolchains because that's the only thing users typically have access to. Carl Eugen
On 11/26/2017 2:15 PM, Carl Eugen Hoyos wrote: > I believe I have explained before that I always only use > vanilla toolchains because that's the only thing users > typically have access to. It's possible the OS X version was outdated then, since the system clang has used libc++ as default since OS X 10.9. But I digress, this is now off-topic, and no longer related to the patch at hand. - Derek
2017-11-26 15:10 GMT+01:00 Nicolas George <george@nsup.org>: > $ grep -c pthread libavfilter/*vmaf* > libavfilter/vf_libvmaf.c:22 > libavfilter/vf_vmafmotion.c:0 > libavfilter/vmaf_motion.h:0 > > -> our code depends on pthreads for this filters, it must be expressed > in configure: Carl Eugen's patch is right Is the correct dependency pthreads or threads? Carl Eugen
2017-11-26 15:10 GMT+01:00 Nicolas George <george@nsup.org>: > -> our code depends on pthreads for this filters, it must be expressed > in configure: Carl Eugen's patch is right Pushed with pthreads, as this is currently used. Thank you, Carl Eugen
From 4ee0fe8778c67a0f623b352a257e68485dd1d559 Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <ceffmpeg@gmail.com> Date: Sun, 26 Nov 2017 14:29:27 +0100 Subject: [PATCH] configure: libvmaf depends on pthreads. --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 0cc97eb..46ff3e8 100755 --- a/configure +++ b/configure @@ -3290,7 +3290,7 @@ uspp_filter_deps="gpl avcodec" vaguedenoiser_filter_deps="gpl" vidstabdetect_filter_deps="libvidstab" vidstabtransform_filter_deps="libvidstab" -libvmaf_filter_deps="libvmaf" +libvmaf_filter_deps="libvmaf threads" zmq_filter_deps="libzmq" zoompan_filter_deps="swscale" zscale_filter_deps="libzimg const_nan" -- 1.7.10.4