Skip to content
Snippets Groups Projects

Draft: src/Makefile.am: export JNI_OnLoad/Unload for android

2 unresolved threads

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.

Edited by Jean-Baptiste Kempf

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Alexandre Janniaux changed milestone to %4.0

    changed milestone to %4.0

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

    • Obviously something needs to be loaded by JNI. My point is that JNI has no business opening libvlccore directly, because its interfaces are really neither meant nor suited to JNI usage.

    • Just so that it's clear, the JavaVM doesn't load libvlccore to use libvlccore API through JNI, but instead loads libvlccore as a dependency of libvlc and provides libvlccore 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 on dlopen(), 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'' with libvlc, libvlccore and modules through a common dependency, a bit like libvlc_vdpau or libvlcpulse.

      Edited by Rémi Denis-Courmont
    • Hi,

      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 or vlc_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 on dlopen(), 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);
    • Please register or sign in to reply
  • Thomas Guillem
    Thomas Guillem @tguillem started a thread on commit f496362d
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 \
  • Jean-Baptiste Kempf marked this merge request as draft

    marked this merge request as draft

  • Alexandre Janniaux mentioned in merge request !3166

    mentioned in merge request !3166

  • Please register or sign in to reply
    Loading