After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 786403 - gstlibav.la contains references to -lbz2 for android
gstlibav.la contains references to -lbz2 for android
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal blocker
: 1.13.91
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-08-17 08:48 UTC by Matthew Waters (ystreet00)
Modified: 2018-03-09 08:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
build: fix .la generator for android (1.33 KB, patch)
2018-02-13 03:22 UTC, Matthew Waters (ystreet00)
none Details | Review
build: prefer .la references (2.28 KB, patch)
2018-03-06 04:25 UTC, Matthew Waters (ystreet00)
committed Details | Review

Description Matthew Waters (ystreet00) 2017-08-17 08:48:07 UTC
This currently breaks running an android application on a phone without libbz2.so with e.g. gst-player.
Comment 1 Sebastian Dröge (slomo) 2017-08-17 09:05:28 UTC
It should use the static libbz2 that we build as part of cerbero
Comment 2 Tim-Philipp Müller 2018-01-22 14:48:38 UTC
Is this still an issue?
Comment 3 Matthew Waters (ystreet00) 2018-01-31 05:37:46 UTC
Yes
Comment 4 Matthew Waters (ystreet00) 2018-01-31 05:40:49 UTC
It also contains references to -lz alongside libz.la
Comment 5 Matthew Waters (ystreet00) 2018-01-31 05:47:25 UTC
As does libav .la files itself:

~/Projects/cerbero/build/dist/android_universal $ find . -iname *.la -exec grep -H 'lbz2' \{\} \;
./arm/lib/libavformat.la:dependency_libs=' -L/home/matt/Projects/cerbero/build/dist/android_universal/arm/lib /home/matt/Projects/cerbero/build/dist/android_universal/arm/lib/libavcodec.la /home/matt/Projects/cerbero/build/dist/android_universal/arm/lib/libavutil.la -lm -lz -lbz2 -lm '
./x86_64/lib/libavformat.la:dependency_libs=' -L/home/matt/Projects/cerbero/build/dist/android_universal/x86_64/lib /home/matt/Projects/cerbero/build/dist/android_universal/x86_64/lib/libavcodec.la /home/matt/Projects/cerbero/build/dist/android_universal/x86_64/lib/libavutil.la -lm -lz -lbz2 -lm '
./x86/lib/libavformat.la:dependency_libs=' -L/home/matt/Projects/cerbero/build/dist/android_universal/x86/lib /home/matt/Projects/cerbero/build/dist/android_universal/x86/lib/libavcodec.la /home/matt/Projects/cerbero/build/dist/android_universal/x86/lib/libavutil.la -lm -lz -lbz2 -lm '
./armv7/lib/libavformat.la:dependency_libs=' -L/home/matt/Projects/cerbero/build/dist/android_universal/armv7/lib /home/matt/Projects/cerbero/build/dist/android_universal/armv7/lib/libavcodec.la /home/matt/Projects/cerbero/build/dist/android_universal/armv7/lib/libavutil.la -lm -lz -lbz2 -lm '
./arm64/lib/libavformat.la:dependency_libs=' -L/home/matt/Projects/cerbero/build/dist/android_universal/arm64/lib /home/matt/Projects/cerbero/build/dist/android_universal/arm64/lib/libavcodec.la /home/matt/Projects/cerbero/build/dist/android_universal/arm64/lib/libavutil.la -lm -lz -lbz2 -lm '
Comment 6 Edward Hervey 2018-02-12 15:36:00 UTC
Isn't the problem related to how you build the app ? It should pick up the libz and libbz2 from the dist when doing the final linking
Comment 7 Matthew Waters (ystreet00) 2018-02-13 00:13:04 UTC
The problem is the use of -lbz2 vs /path/to/lib/libbz2.la on android.  The former will expect libbz2.so on the android device (which does not exist) and the latter will include libbz2.a in the app (which is what we want).

I think the problem is our custom .la generator in gst-libav.
Comment 8 Nicolas Dufresne (ndufresne) 2018-02-13 00:59:32 UTC
The custom .la generator is not supposed to differ that much from the Cerbero custom one we had before. Best would be to compare before / after. Though, -l should make no difference, we have a libtool static-libtool-libs equivalent makefile that is supposed to prefer the static lib with .la files. So I suspect that libbz2.la does not exist or some -L is gone missing.
Comment 9 Matthew Waters (ystreet00) 2018-02-13 01:19:28 UTC
(In reply to Nicolas Dufresne (stormer) from comment #8)
> Though, -l should make no difference, we have a libtool static-libtool-libs
> equivalent makefile that is supposed to prefer the static lib with .la
> files.

This only works for .la references, not -l references.  And libbz2.a exists.  We build it: https://cgit.freedesktop.org/gstreamer/cerbero/tree/recipes/bzip2.recipe
Comment 10 Matthew Waters (ystreet00) 2018-02-13 02:19:51 UTC
libgstlibav.la from 1.12.4 binaries:

dependency_libs=' ... /home/jan/devel/gstreamer/cerbero/build/dist/android_universal/armv7/lib/libz.la /home/jan/devel/gstreamer/cerbero/build/dist/android_universal/armv7/lib/libbz2.la'

With no -lbz2 or -lz reference

From master:

dependency_libs=' ... -lbz2 /home/matt/Projects/cerbero/build/dist/android_universal/armv7/lib/libswresample.la -lz /home/matt/Projects/cerbero/build/dist/android_universal/armv7/lib/libavutil.la -lm /home/matt/Projects/cerbero/build/dist/android_universal/armv7/lib/libz.la /home/matt/Projects/cerbero/build/dist/android_universal/armv7/lib/libbz2.la'

Which contains both -lbz2 and libbz2.la (and the same for libz).
Comment 11 Matthew Waters (ystreet00) 2018-02-13 03:22:18 UTC
Created attachment 368284 [details] [review]
build: fix .la generator for android
Comment 12 Nicolas Dufresne (ndufresne) 2018-02-13 13:06:21 UTC
Review of attachment 368284 [details] [review]:

That seems incomplete, FFMPEG still depends on zlib and bzip2, which is now completely absent from the .la file.
Comment 13 Nicolas Dufresne (ndufresne) 2018-02-13 13:10:39 UTC
Can you document what adds this .la file, since it's not us, there must be something else, cerbero recipe ? I bet this will break other platform, that's why I comment negatively on this patch. But if you want to try it out on the CI go for it.

If you give a try to the original static libs resolver, libtool, it will handle -l the same as any .la reference (by finding a static lib first). This is also what our replicate is supposed to do, though apparently it didn't work, or wasn't happy to have a duplicate.
Comment 14 Matthew Waters (ystreet00) 2018-02-13 13:24:01 UTC
(In reply to Nicolas Dufresne (stormer) from comment #12)
> Review of attachment 368284 [details] [review] [review]:
> 
> That seems incomplete, FFMPEG still depends on zlib and bzip2, which is now
> completely absent from the .la file.

The resulting .la file still has references to libz.la and libbz2.la so they're not 'completely absent'

(In reply to Nicolas Dufresne (stormer) from comment #13)
> Can you document what adds this .la file, since it's not us, there must be
> something else, cerbero recipe ? I bet this will break other platform,
> that's why I comment negatively on this patch. But if you want to try it out
> on the CI go for it.

'this .la file' == libgstlibav.la ?  The one you added to the gst-libav Makefile as a result of removing the .la generator from cerbero? https://cgit.freedesktop.org/gstreamer/gst-libav/log/gst-libs/ext/Makefile.am and https://cgit.freedesktop.org/gstreamer/cerbero/commit/recipes/gst-libav-1.0.recipe?id=afd2f5017c16203a5c152e5771bc2599901f128c

> If you give a try to the original static libs resolver, libtool, it will
> handle -l the same as any .la reference (by finding a static lib first).
> This is also what our replicate is supposed to do, though apparently it
> didn't work, or wasn't happy to have a duplicate.

The replicator is a dumb Makefile substitution which doesn't know anything about external libraries and so does not know how to transform -lz into /path/to/libz.la and thus copies the -lz and BZ2_LIBS (-lbz2) directly into the resulting libgstlibav.la.
Comment 15 Nicolas Dufresne (ndufresne) 2018-02-13 13:31:50 UTC
(In reply to Matthew Waters (ystreet00) from comment #14)
> 'this .la file' == libgstlibav.la ?  The one you added to the gst-libav

No, the generator is for the FFMPEG libraries. The libgstlibav.la is still generated by libtool.
Comment 16 Matthew Waters (ystreet00) 2018-02-13 13:42:42 UTC
(In reply to Nicolas Dufresne (stormer) from comment #15)
> (In reply to Matthew Waters (ystreet00) from comment #14)
> > 'this .la file' == libgstlibav.la ?  The one you added to the gst-libav
> 
> No, the generator is for the FFMPEG libraries. The libgstlibav.la is still
> generated by libtool.

Right, the FFMPEG generator produces .la files with -lbz2 which when parsed with libtool will produce references to static libraries.  However, when parsed with our Android Makefile based .la parser, will simply passthrough the -lbz2 reference and fail at runtime to link to a non-existent shared library.
Comment 17 Nicolas Dufresne (ndufresne) 2018-02-13 14:24:49 UTC
Ok, I didn't expect this behaviour. It worked for me though when I published this change, not sure what changed. Adding -l to .la is valid btw, we are lucky it didn't break elsewhere.
Comment 18 Sebastian Dröge (slomo) 2018-02-26 11:10:49 UTC
Let's get the patch in then for now, Matthew? Or can someone think of a nicer solution?
Comment 19 Matthew Waters (ystreet00) 2018-03-06 04:25:45 UTC
Created attachment 369370 [details] [review]
build: prefer .la references

How about this?

It seems to work in both my android setup and in gst-uninstalled.
Comment 20 Nicolas Dufresne (ndufresne) 2018-03-06 04:36:38 UTC
Review of attachment 369370 [details] [review]:

It's been a long time I haven't look at makefile programming, but that should work. It's probably what libtool would do.
Comment 21 Sebastian Dröge (slomo) 2018-03-06 07:14:22 UTC
Comment on attachment 369370 [details] [review]
build: prefer .la references

Let's get that in then?
Comment 22 Matthew Waters (ystreet00) 2018-03-06 07:23:24 UTC
commit e2bce0f3ec07d86e980cb85233c4f7fb819698c5
Author: Matthew Waters <matthew@centricular.com>
Date:   Tue Mar 6 14:40:20 2018 +1100

    build: prefer using *.la references when creating our own libtool files
    
    Otherwise we will reference the dependant libraries with -lfoo rather than
    /path/to/libfoo.la which breaks with the Android-based .la parser which
    simply passes through all -l libraries.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=786403
Comment 23 Solen Win 2018-03-09 08:21:34 UTC
Comment on attachment 368284 [details] [review]
build: fix .la generator for android

solenwin2@gmail.com