It should use the static libbz2 that we build as part of cerbero

Is this still an issue?

Yes

It also contains references to -lz alongside libz.la

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 '

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

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.

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.

(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

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).

Created attachment 368284 [details] [review]
build: fix .la generator for android

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.

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.

(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.

(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.

(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.

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.

Let's get the patch in then for now, Matthew? Or can someone think of a nicer solution?

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.

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 on attachment 369370 [details] [review]
build: prefer .la references

Let's get that in then?

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 on attachment 368284 [details] [review]
build: fix .la generator for android

solenwin2@gmail.com