Draft: src/Makefile.am: export JNI_OnLoad/Unload for android
Those functions need to be exported and visible for the Android loader to call them when the native library is loaded. This is optional when libvlccore is built statically since there's no linkage anyway, but mandatory as soon as we're building a dynamic libvlccore.so.
Merge request reports
Activity
changed milestone to %4.0
added Component::Build system Platform::Android labels
added MRStatus::Reviewable label
libvlccore
is meant to be loaded by native modules, and for lack of cleaner option, by native libvlc. It makes no sense to load it directly with JNI.Not only that but dynamically generating the symbol list is guaranteed to cause build system bugs. Indeed, this patch already seems to fail to clean properly, and who knows what more subtle problems it brings.
I'm opening the thread to prevent merging for now.
Yes, I agree with you, that's an ugly interception of the link layer and it opens to many unusual issues. Yet, I think we need a solution to have proper module support here, as it's supposed to be the default.
I'm open to variation and can provide time to adapt other part of the code instead.
The main blocker is the access to the JVM object in the core before the calls to
libvlc_new
, and the access to the class loader from the main thread. So it needs a way (either through an API or through link time interception points) to initialize those platform objects. It could be important for listing zip-aligned plugins from an apk before dlopen-ing them, much like the darwin framework support.libvlccore
is meant to be loaded by native modules, and for lack of cleaner option, by native libvlc. It makes no sense to load it directly with JNI.Actually, it's currently loaded by libvlcjni as a standard link dependency, in a context where libvlc, libvlccore and the plugins are merged in a single shared object. What I change here is made for the case where libvlc.so is loaded as a link dependency from libvlcjni.so and libvlccore.so is pulled afterwards. The main issue is that the interception point in this revision is only available on Android and doesn't exist on every other platform, leading to the potential buildsystem issues.
Just so that it's clear, the JavaVM doesn't load
libvlccore
to uselibvlccore
API through JNI, but instead loadslibvlccore
as a dependency oflibvlc
and provideslibvlccore
the ability to use JNI code. Libvlccore has no interest in exposing JNI features through its API.I think we're saying the same here but there's room for confusion here.
What I mentioned above is that if the JavaVM cannot inject itself into the libvlccore runtime, the only other injection point available is through libvlc, but it should happen before
libvlc_new
is called.I don't think that
libvlccore
even should access the JVM. That seems more like something of interest to some modules. Not that I'd know, admittedly.But regardless, I don't believe that libraries should expose JNI entry points unless they are loaded directly by JNI.
libdl
calls ''regular'' constructors ondlopen()
, not well-known JNI identifiers. It might work by accident if no other library in the namespace exposes the same JNI identifier, but that's really brittle.AFAICT, the way that's supposed to work is that
libvlcjni
exposes the JNI entry points. From there, they can be ''shared'' withlibvlc
,libvlccore
and modules through a common dependency, a bit likelibvlc_vdpau
orlibvlcpulse
.Edited by Rémi Denis-CourmontHi,
I don't think that
libvlccore
even should access the JVM. That seems more like something of interest to some modules. Not that I'd know, admittedly.I partially agree, the current design is made as-is because of:
- the JavaVM exposure limitation, you need a place where both side can share the JavaVM.
- the java classloader implementation: you can only access the classloader from the thread you're in, so said otherwise you can only load the java class from the thread loading the library, which you can reduce to loading and storing the classloader from the loading thread (you can use it to load in other threads afterwards).
- the prevalence of the vlc-android binding
But also, libvlccore is supposed to access the runtime context, in particular in
config_GetUserDir
orvlc_getProxyUrl
, and potentially from files (resources or plugins) coming from the .apk materializing the application being run, or even from android application extension concept (app bundle iirc).Where I join your point is that it should not be mandatory to have JNI available to run the core, or even compile the core, since we can also launch application the usual way on Android.
I like the separate library approach to expose new API endpoints, I don't know if binding maintainers also agree with this (@mfkl, @Aza, about adding a separate libvlc_android_init endpoint that would eg. be called in JNI_OnLoad and stored in a separate libvlc_android.so shared object) since it would result in a new shared object, but it makes a lot of sense. I still need to see how to handle the different case above though.
But regardless, I don't believe that libraries should expose JNI entry points unless they are loaded directly by JNI.
libdl
calls ''regular'' constructors ondlopen()
, not well-known JNI identifiers. It might work by accident if no other library in the namespace exposes the same JNI identifier, but that's really brittle.Indeed, I didn't realize this, but when doing dynamic plugins in the past, I needed to load the library in the correct order from the JVM. I was thinking it might have been a limitation of the android ELF loader, but clearly your explanation makes a lot more sense.
I like the separate library approach to expose new API endpoints, I don't know if binding maintainers also agree with this (@mfkl, @Aza, about adding a separate libvlc_android_init endpoint that would eg. be called in JNI_OnLoad and stored in a separate libvlc_android.so shared object) since it would result in a new shared object, but it makes a lot of sense.
Doesn't sound like a problem to me. We already have other libs anyway (
libc++_shared.so
). For the record, in libvlcsharp, libvlc loading is currently done like so:Java.Lang.JavaSystem.LoadLibrary("c++_shared"); Native.JniOnLoad(JniRuntime.CurrentRuntime.InvocationPointer);
added MRStatus::InReview label and removed MRStatus::Reviewable label
528 528 libvlccore_la_LIBADD += $(GCRYPT_LIBS) 529 529 endif 530 530 531 BUILT_SOURCES += $(builddir)/libvlccore.sym.out 531 532 libvlccore_la_LDFLAGS = \ 532 533 $(LDFLAGS_libvlccore) \ 533 534 -no-undefined \ 534 -export-symbols $(srcdir)/libvlccore.sym \ mentioned in merge request !3166