gstreamer-java / gst1-java-core

Java bindings for GStreamer 1.x
GNU Lesser General Public License v3.0
194 stars 72 forks source link

Library Name Resolution + GioApi #230

Closed freelancer1845 closed 3 years ago

freelancer1845 commented 3 years ago

https://github.com/gstreamer-java/gst1-java-core/issues/229

  1. gstreamer => gstreamer-1.0-0.dll (msvc build)
  2. glib => libglib-2.0.dll (probably build by mingw) => Leading to Invalid Memory Access etc...

I added a short circuiting style library name resolving to GNative.loadLibrary that fixes this. But I don't know If you want to have that in your lib. I'm not sure whether this actually leads to more problems. Do you know cases where the library name patterns are different? The fix resolves the upper case to:

  1. gstreamer => gstreamer-1.0-0.dll
  2. glib => glib-2.0-0.dll

I think correctly resolving the MSVC library is important as on the gstreamer download page it is suggested to windows users to use the msvc build...

Side Note, maybe another issue: If gstreamer is installed on windows a environment variable is created containing the path of gstreamer.

GSTREAMER_1_0_ROOT_MSVC_X86_64

Maybe this can be used to automatically add the path to jna library resolving.

neilcsmith-net commented 3 years ago

Thanks! I'll take a look.

Not sure how safe the short circuiting is across all systems, or whether it's the best approach - even if the current approach is ugly!

Good spot on GIO. It was included as static in a PR, and was left like that because it worked until now. That is probably my preferred way to map things with JNA if possible, and is more efficient, but we're not really in a position to drop interface mapping yet.

The environment variable isn't that useful for JNA library resolving by itself, because it doesn't affect how GStreamer finds its own dependencies - quite often problems if multiple versions of libraries on the path with GStreamer and JNA picking different ones. There's a message on the list about that recently.

However, I would be tempted to consider either changing the loading order in the mapping string to put %s-0 before %s, and/or look at a different default mappings if GSTREAMER_1_0_ROOT_MSVC_X86_64 is set?

freelancer1845 commented 3 years ago

Yep, I agree, my solution is a little short sighted :D

I thought about changing the order... I guess it makes sense since the default lib names on windows would be %s-0, but then I again it could produce the same problem in the end^^

GNative.java

    public static Stream<String> libFormats(String libraryName) {
        return Arrays.stream(nameFormats).map(format -> String.format(format, libraryName));
    }

GioAPI.java

        static {
                GNative.libFormats("gio-2.0").filter(name -> {
                        try {
                                Native.register(name);
                                return true;
                        } catch (UnsatisfiedLinkError error) {
                                return false;
                        }
                }).findFirst().orElseThrow(() -> new UnsatisfiedLinkError("Failed to load gio-2.0"));
        }

It would be nice if GStreamer had an api that somehow locates the glib library it loaded (or sth. similar)

neilcsmith-net commented 3 years ago

At the moment, go with the original change for GIO - let's keep loading consistent, so switch to interface is good.

We may need to support loading GLib before GStreamer. So we may need to switch in the longer term into attempting a file search and loading the libraries with full path. Right now, simplest approach that works! :smile:

neilcsmith-net commented 3 years ago

Thanks for your initial work on this. I need to get a basic fix in place for testing something else on Windows, so have opened #232 instead. It changes the order of the name resolution, and also adds the ability to set the GStreamer path as a system property for Maven tests. I haven't needed the plugin version test change you have here. Could you clarify when it fails (version / build) for you? Thanks!

freelancer1845 commented 3 years ago

You're welcome, happy that I could help.

When I build gstreamer myself on windows the plugin version of "playback" is reported as 1.19.0.1

But now I think this only happens If you don't use a release build.

If I understand this correctly https://github.com/GStreamer/gst-plugins-base/blob/a89b4757f6c6aaddb916b2d2e6b01a96ada78a57/meson.build#L1-L16 it is derived from the gst version

neilcsmith-net commented 3 years ago

Ah, thanks, makes sense. I've updated #232 to include that change as well.

I'll close this PR, but will keep in mind some of the conversation about fixed name resolution - I think we need to rethink how names are resolved at some point, but hopefully the minor changes to the pattern and order cover us for now.