On 2016-10-25 23:30:50 +0200, Diego Biurrun wrote:
> On Fri, Sep 09, 2016 at 08:08:55PM +0200, Janne Grunau wrote:
> > On 2016-09-05 20:31:39 +0200, Diego Biurrun wrote:
> > > Previously all external library dependencies were added as link-time
> > > dependencies to all components. This adds bogus dependencies to some
> > > components, so only add dependencies to components if said components
> > > actually make use of a dependency.
> > > ---
> > >
> > > This can still be improved a bit I guess. Better function names welcome
> > > for
> > > starters. Janne had some more substantive comments. Our pkg-config files
> > > have
> > > many more issues, even after this patch...
> >
> > I don't think this is the right approach. The fundamental problem in
> > this patch is that it mixes the checks functions/libraries with the
> > information which part of libav uses the library. The basic idea would
> > be to set fn_$(func)_extralibs in check_lib(2) and have components
> > depend fn_$(func). We can't check for multiple functions in
> > check_lib(2) after that at once but we don't do it atm. The
> > dep_extralibs handling in check_deps() needs to be modified (looks like
> > it is missing in this patch).
>
> I'm not sure if we're on the same page here. I attached a very rough sketch
> of what I interpret to be your idea. It's not exactly turning out simpler,
> so if I'm going down the wrong path, I'm all ears for better suggestions.
It's more or less what I had in mind. I didn't thought of need to match
components back to libraries. But I never claimed that it would be
simpler. It does the right thing wrt to deps. Your previous patch would
still have linked libavformat against bzlib even then the matroska
demuxer was disabled (and bzlib is present and not disabled explicitly).
>
> diff --git a/configure b/configure
> index 6a04c33..ba8e3c8 100755
> --- a/configure
> +++ b/configure
> @@ -621,10 +621,11 @@ do_check_deps(){
> eval dep_sgs="\$${cfg}_suggest"
> eval dep_ifa="\$${cfg}_if"
> eval dep_ifn="\$${cfg}_if_any"
> + eval dep_lbs="\$${cfg}_libs"
>
> - pushvar cfg dep_all dep_any dep_sel dep_sgs dep_ifa dep_ifn
> + pushvar cfg dep_all dep_any dep_sel dep_sgs dep_ifa dep_ifn dep_lbs
> do_check_deps $dep_all $dep_any $dep_sel $dep_sgs $dep_ifa $dep_ifn
> - popvar cfg dep_all dep_any dep_sel dep_sgs dep_ifa dep_ifn
> + popvar cfg dep_all dep_any dep_sel dep_sgs dep_ifa dep_ifn dep_lbs
>
> [ -n "$dep_ifa" ] && { enabled_all $dep_ifa && enable_weak $cfg; }
> [ -n "$dep_ifn" ] && { enabled_any $dep_ifn && enable_weak $cfg; }
> @@ -635,6 +636,24 @@ do_check_deps(){
> if enabled $cfg; then
> enable_deep $dep_sel
> enable_deep_weak $dep_sgs
> + for lib in $dep_lbs; do
> + eval append dep_extralibs "\$${lib}_extralibs"
> + done
> + if test -n "$dep_extralibs"; then
> + case $cfg in
> + *coder|*parser|*bsf)
> + add_extralibs_lib avcodec $dep_extralibs ;;
> + *muxer)
> + add_extralibs_lib avformat $dep_extralibs ;;
> + *indev|*outdev)
> + add_extralibs_lib avdevice $dep_extralibs ;;
> + *filter)
> + add_extralibs_lib avfilter $dep_extralibs ;;
> + *)
> + add_extralibs $dep_extralibs ;;
> + esac
> + unset dep_extralibs
> + fi
> fi
>
> disable ${cfg}_checking
> @@ -648,8 +667,6 @@ check_deps(){
>
> for cfg in $allopts; do
> enabled $cfg || continue
> - eval dep_extralibs="\$${cfg}_extralibs"
> - test -n "$dep_extralibs" && add_extralibs $dep_extralibs
> done
> }
>
> @@ -739,6 +756,13 @@ add_extralibs(){
> prepend extralibs $($ldflags_filter "$@")
> }
>
> +add_extralibs_lib(){
> + lib=$1
> + shift
> + prepend extralibs_${lib} $($ldflags_filter "$@")
> + unique extralibs_${lib}
> +}
> +
> add_host_cppflags(){
> append host_cppflags "$@"
> }
> @@ -1001,10 +1025,11 @@ EOF
>
> check_lib(){
> log check_lib "$@"
> - headers="$1"
> - funcs="$2"
> - shift 2
> - check_func_headers "$headers" "$funcs" "$@" && add_extralibs "$@"
> + name="$1"
> + headers="$2"
> + func="$3"
> + shift 3
> + check_func_headers "$headers" "$func" "$@" && eval
> ${name}_extralibs="\$@"
I would have used $func as identifier for the extralibs instead of
introducing an extra identifier.
> }
>
> check_pkg_config(){
> @@ -1100,7 +1125,7 @@ require(){
> headers="$2"
> func="$3"
> shift 3
> - check_lib "$headers" $func "$@" || die "ERROR: $name not found"
> + check_lib $name "$headers" $func "$@" || die "ERROR: $name not found"
> }
>
> require_pkg_config(){
> @@ -2310,6 +2335,7 @@ ismv_muxer_select="mov_muxer"
> matroska_audio_muxer_select="matroska_muxer"
> matroska_demuxer_select="iso_media riffdec"
> matroska_demuxer_suggest="bzlib lzo zlib"
> +matroska_demuxer_libs="bzlib zlib"
> matroska_muxer_select="iso_media riffenc"
> mmf_muxer_select="riffenc"
> mov_demuxer_select="iso_media riffdec"
> @@ -2352,7 +2378,9 @@ xwma_demuxer_select="riffdec"
>
> # indevs / outdevs
> alsa_indev_deps="alsa_asoundlib_h snd_pcm_htimestamp"
> +alsa_indev_libs="alsa"
> alsa_outdev_deps="alsa_asoundlib_h"
> +alsa_outdev_libs="alsa"
To be honest I find these identifiers confusing. It's probably too short
alsa_outdev_extralibs="alsa_ldflags"
would imho less confusing
> avfoundation_indev_deps="AVFoundation_AVFoundation_h"
> bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h
> dev_video_bktr_ioctl_bt848_h dev_ic_bt8xx_h"
> dv1394_indev_deps="dv1394"
> @@ -2360,6 +2388,7 @@ dv1394_indev_select="dv_demuxer"
> fbdev_indev_deps="linux_fb_h"
> jack_indev_deps="jack_jack_h"
> jack_indev_deps_any="sem_timedwait dispatch_dispatch_h"
> +jack_indev_libs="jack"
> libcdio_indev_deps="libcdio"
> libdc1394_indev_deps="libdc1394"
> oss_indev_deps_any="soundcard_h sys_soundcard_h"
> @@ -4590,8 +4619,8 @@ fi
> enabled pthreads &&
> check_builtin sem_timedwait semaphore.h "sem_t *s; sem_init(s,0,0);
> sem_timedwait(s,0); sem_destroy(s)"
>
> -disabled zlib || check_lib zlib.h zlibVersion -lz || disable zlib
> -disabled bzlib || check_lib bzlib.h BZ2_bzlibVersion -lbz2 || disable bzlib
> +disabled zlib || check_lib zlib zlib.h zlibVersion -lz || disable
> zlib
> +disabled bzlib || check_lib bzlib bzlib.h BZ2_bzlibVersion -lbz2 || disable
> bzlib
>
> check_lib math.h sin -lm && LIBM="-lm"
>
> @@ -4756,9 +4785,9 @@ check_header sys/soundcard.h
> check_header soundcard.h
>
> enabled_any alsa_indev alsa_outdev &&
> - check_lib alsa/asoundlib.h snd_pcm_htimestamp -lasound
> + check_lib alsa alsa/asoundlib.h snd_pcm_htimestamp -lasound
I'm not really happy about these random additional identifiers. Not sure
if it is just bikeshedding. Using the function name makes somewhat
sense. This are the extralibs if you want to use $funtion
HTH
Janne
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
